----------------------------------------------------------- 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 > >
