You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oodt.apache.org by Ross Laidlaw <rl...@gmail.com> on 2013/08/14 19:32:17 UTC
Review Request 13000: OODT-651: Improve parameter initialization,
validation and logging for the CAS-Product JAXRS service
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13000/
-----------------------------------------------------------
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
Re: Review Request 13000: OODT-651: Improve parameter initialization,
validation and logging for the CAS-Product JAXRS service
Posted by Chris Mattmann <ma...@apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 13000: OODT-651: Improve parameter initialization,
validation and logging for the CAS-Product JAXRS service
Posted by Rishi Verma <ri...@apache.org>.
-----------------------------------------------------------
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
>
>