oodt-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rishi Verma" <rive...@apache.org>
Subject Re: Review Request 13000: OODT-651: Improve parameter initialization, validation and logging for the CAS-Product JAXRS service
Date Mon, 19 Aug 2013 22:00:48 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13000/#review25317
-----------------------------------------------------------

Ship it!


Hey Ross,

EXCELLENT work! You really went above and beyond here, to come up with an elegant solution
to resource management. I like your decision to customize and create your own 'CasProductJaxrsServlet'
servlet especially. 

Also, good attention to detail in updating test cases, updating Java docs, and centralizing
the location of configurable properties. 

Absolutely great work! Ship it!

Rishi 

- Rishi Verma


On Aug. 14, 2013, 5:32 p.m., Ross Laidlaw wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13000/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2013, 5:32 p.m.)
> 
> 
> Review request for oodt, Chris Mattmann and Rishi Verma.
> 
> 
> Bugs: OODT-651
>     https://issues.apache.org/jira/browse/OODT-651
> 
> 
> Repository: oodt
> 
> 
> Description
> -------
> 
> Summary
> -------
> 
> The aim of this patch is to make some improvements to the JAX-RS service in the CAS-Product
web application.  I've moved some initialization of various parameters from the resource classes
into a servlet class, so that it doesn't happen every time a call is made to the RESTful service.
 I've also added some extra logging, along with an example (Tomcat) logging.properties file.
 I've tidied up some of the code, e.g. reducing some duplication.  I've updated the comments
and improved the Javadocs.
> 
> 
> Details
> -------
> 
> - I moved all of the context parameter definitions from web.xml to context.xml so that
they're all in one place and possibly easier to keep track of.
> 
> - I created a new class CasProductJaxrsServlet that extends org.apache.cxf.jaxrs.servlet.CXFNonSpringJaxrsServlet.
 This new class is now our entry point to the JAX-RS service (configured in the web.xml file).
> 
> - The new servlet class is used to initialize and validate certain parameters, such as
the file manager's working directory and file manager client, based on context parameters
(now all defined in context.xml).  Previously these were initialized in the resource classes
each time an HTTP request was made.  After they have been initialized and validated, the working
directory and file manager client instance are stored as servlet context attributes, using
context.setAttribute(String name, Object value).
> 
> - I abstracted some of the duplicated code in each resource class into a new abstract
class called 'Resource'.  This class is used to inject the servlet context and retrieve some
of the initialized fields (i.e. the file manager working directory and client) using context.getAttribute(String
name).
> 
> - I added some extra logging in the resource and responder classes.  I also added an
example logging.properties file for a basic default setup when the web application is deployed.
> 
> - I corrected and updated some of the comments and Javadocs.
> 
> 
> Diffs
> -----
> 
>   trunk/webapp/fmprod/src/main/java/org/apache/oodt/cas/product/service/resources/DatasetResource.java
1513410 
>   trunk/webapp/fmprod/src/main/java/org/apache/oodt/cas/product/service/resources/ProductResource.java
1513410 
>   trunk/webapp/fmprod/src/main/java/org/apache/oodt/cas/product/service/resources/ReferenceResource.java
1513410 
>   trunk/webapp/fmprod/src/main/java/org/apache/oodt/cas/product/service/resources/Resource.java
PRE-CREATION 
>   trunk/webapp/fmprod/src/main/java/org/apache/oodt/cas/product/service/resources/TransferResource.java
1513410 
>   trunk/webapp/fmprod/src/main/java/org/apache/oodt/cas/product/service/responders/FileResponder.java
1513410 
>   trunk/webapp/fmprod/src/main/java/org/apache/oodt/cas/product/service/responders/ZipResponder.java
1513410 
>   trunk/webapp/fmprod/src/main/java/org/apache/oodt/cas/product/service/servlets/CasProductJaxrsServlet.java
PRE-CREATION 
>   trunk/webapp/fmprod/src/main/resources/logging.properties PRE-CREATION 
>   trunk/webapp/fmprod/src/main/webapp/META-INF/context.xml 1513410 
>   trunk/webapp/fmprod/src/main/webapp/WEB-INF/web.xml 1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/resources/DatasetResourceTest.java
1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/resources/ProductResourceTest.java
1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/resources/ReferenceResourceTest.java
1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/resources/ResourceTestBase.java
1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/responders/FileResponderTest.java
1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/responders/NullResponderTest.java
1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/responders/ResponderFactoryTest.java
1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/responders/UnrecognizedFormatResponderTest.java
1513410 
>   trunk/webapp/fmprod/src/test/java/org/apache/oodt/cas/product/service/responders/ZipResponderTest.java
1513410 
>   trunk/webapp/fmprod/src/test/resources/filemgr/policy/core/product-types.xml 1513410

>   trunk/webapp/fmprod/src/test/resources/test.logging.properties 1513410 
> 
> Diff: https://reviews.apache.org/r/13000/diff/
> 
> 
> Testing
> -------
> 
> I updated several of the JUnit tests for the resources and responders to take account
of the code changes.  All unit tests pass on my local machine (Mac OSX 10.8).
> 
> 
> Thanks,
> 
> Ross Laidlaw
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message