----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13000/#review25396 -----------------------------------------------------------
Ship it! Ship It! - Chris Mattmann 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 > >