----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37934/#review98361 -----------------------------------------------------------
lens-api/src/main/java/org/apache/lens/api/query/save/ListResponse.java (line 30) <https://reviews.apache.org/r/37934/#comment154786> This response can be generified to anything that is supposed to return a list. Can we probably make this `ListResponse<T>` and use `ListResponse<SavedQuery>` wherever required in this patch. lens-api/src/main/java/org/apache/lens/api/query/save/Parameter.java (line 42) <https://reviews.apache.org/r/37934/#comment154788> final? lens-api/src/main/java/org/apache/lens/api/query/save/ParameterParserResponse.java (line 42) <https://reviews.apache.org/r/37934/#comment154789> change the comment to remove `if valid` lens-api/src/main/java/org/apache/lens/api/query/save/ResourceModifiedResponse.java (lines 43 - 53) <https://reviews.apache.org/r/37934/#comment154790> CAPITAL CASE plase lens-api/src/main/java/org/apache/lens/api/query/save/SavedQuery.java (line 50) <https://reviews.apache.org/r/37934/#comment154791> `Parameter` has `name`, `displayName` but not `description`. `SavedQuery` has `name`, `description` but not `displayName`. Should they both have all three? Thinking more on this, `XField` has all three, so maybe they can extend that. But if that happens, we'll be better off renaming `XField` to something that reveals it has 3 compulsary methods. lens-api/src/main/resources/lens-errors.conf (lines 27 - 28) <https://reviews.apache.org/r/37934/#comment154793> `javax.ws.rs.core.Response.Status`: OK(200, "OK"), CREATED(201, "Created"), ACCEPTED(202, "Accepted"), NO_CONTENT(204, "No Content"), RESET_CONTENT(205, "Reset Content"), PARTIAL_CONTENT(206, "Partial Content"), MOVED_PERMANENTLY(301, "Moved Permanently"), FOUND(302, "Found"), SEE_OTHER(303, "See Other"), NOT_MODIFIED(304, "Not Modified"), USE_PROXY(305, "Use Proxy"), TEMPORARY_REDIRECT(307, "Temporary Redirect"), BAD_REQUEST(400, "Bad Request"), UNAUTHORIZED(401, "Unauthorized"), PAYMENT_REQUIRED(402, "Payment Required"), FORBIDDEN(403, "Forbidden"), NOT_FOUND(404, "Not Found"), METHOD_NOT_ALLOWED(405, "Method Not Allowed"), NOT_ACCEPTABLE(406, "Not Acceptable"), PROXY_AUTHENTICATION_REQUIRED(407, "Proxy Authentication Required"), REQUEST_TIMEOUT(408, "Request Timeout"), CONFLICT(409, "Conflict"), GONE(410, "Gone"), LENGTH_REQUIRED(411, "Length Required"), PRECONDITION_FAILED(412, "Precondition Failed"), REQUEST_ENTITY_TOO_LARGE(413, "Request Entity Too Large"), REQUEST_URI_TOO_LONG(414, "Request-URI Too Long"), UNSUPPORTED_MEDIA_TYPE(415, "Unsupported Media Type"), REQUESTED_RANGE_NOT_SATISFIABLE(416, "Requested Range Not Satisfiable"), EXPECTATION_FAILED(417, "Expectation Failed"), INTERNAL_SERVER_ERROR(500, "Internal Server Error"), NOT_IMPLEMENTED(501, "Not Implemented"), BAD_GATEWAY(502, "Bad Gateway"), SERVICE_UNAVAILABLE(503, "Service Unavailable"), GATEWAY_TIMEOUT(504, "Gateway Timeout"), HTTP_VERSION_NOT_SUPPORTED(505, "HTTP Version Not Supported"); Let's keep the names consistent, and try to re-use from there if possible? lens-api/src/main/resources/lens-errors.conf <https://reviews.apache.org/r/37934/#comment154796> Why do we need to remove it? lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryHelper.java (line 88) <https://reviews.apache.org/r/37934/#comment154798> Just a thought here: `parameter.getDataType()` is converted to `ParameterDataTypeEncoder` and sample value is asked. perhaps both(data type and encoder) can be merged? lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryHelper.java (line 97) <https://reviews.apache.org/r/37934/#comment154801> why two values? lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryHelper.java (lines 112 - 113) <https://reviews.apache.org/r/37934/#comment154818> Validations should be on constructor level. lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryHelper.java (line 118) <https://reviews.apache.org/r/37934/#comment154816> Can we reuse `LensJAXBValidationException` here? lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryHelper.java (lines 132 - 134) <https://reviews.apache.org/r/37934/#comment154817> Instead of validating outside of `parameter`, creation of `Parameter` Object itself should be valid. As in these checks should be at the constructor level. lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryService.java (line 63) <https://reviews.apache.org/r/37934/#comment154819> Instead of `long`, can we re-use `QueryHandle`? lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/ParameterCollectionException.java (line 42) <https://reviews.apache.org/r/37934/#comment154822> can we pass error info object to super instead of just a message? - Rajat Khandelwal On Sept. 10, 2015, 12:53 a.m., Amruth Sampath wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37934/ > ----------------------------------------------------------- > > (Updated Sept. 10, 2015, 12:53 a.m.) > > > Review request for lens, Amareshwari Sriramadasu, Pranav Agarwal, Rajat > Khandelwal, and sharad agarwal. > > > Repository: lens > > > Description > ------- > > Refer to the JIRA description - https://issues.apache.org/jira/browse/LENS-742 > (Note : Sharing and CLI are not a part of this patch) > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/error/LensCommonErrorCode.java > 754e6e1 > lens-api/src/main/java/org/apache/lens/api/query/save/ListResponse.java > PRE-CREATION > lens-api/src/main/java/org/apache/lens/api/query/save/Parameter.java > PRE-CREATION > > lens-api/src/main/java/org/apache/lens/api/query/save/ParameterCollectionType.java > PRE-CREATION > > lens-api/src/main/java/org/apache/lens/api/query/save/ParameterDataType.java > PRE-CREATION > > lens-api/src/main/java/org/apache/lens/api/query/save/ParameterParserResponse.java > PRE-CREATION > > lens-api/src/main/java/org/apache/lens/api/query/save/ResourceModifiedResponse.java > PRE-CREATION > lens-api/src/main/java/org/apache/lens/api/query/save/SavedQuery.java > PRE-CREATION > lens-api/src/main/resources/lens-errors.conf 5428041 > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > fb11f93 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryHelper.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/SavedQueryService.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/MissingParameterException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/ParameterCollectionException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/ParameterValueException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/PrivilegeException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/SavedQueryNotFound.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/exception/ValueEncodeException.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/param/ParameterCollectionTypeEncoder.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/param/ParameterDataTypeEncoder.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/param/ParameterParser.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/query/save/param/ParameterResolver.java > PRE-CREATION > > lens-server-api/src/test/java/org/apache/lens/server/api/query/save/TestParameterParser.java > PRE-CREATION > > lens-server-api/src/test/java/org/apache/lens/server/api/query/save/TestParameterResolution.java > PRE-CREATION > lens-server/enunciate.xml 94b5199 > > lens-server/src/main/java/org/apache/lens/server/query/save/SavedQueryApp.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/query/save/SavedQueryDao.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/query/save/SavedQueryResource.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/query/save/SavedQueryServiceImpl.java > PRE-CREATION > lens-server/src/main/java/org/apache/lens/server/util/UtilityMethods.java > 5d77eb7 > lens-server/src/main/resources/lensserver-default.xml 5d33eda > > lens-server/src/test/java/org/apache/lens/server/query/save/TestSavedQueryService.java > PRE-CREATION > lens-server/src/test/resources/lens-site.xml 4cf94d5 > src/site/apt/admin/config.apt b163a3a > src/site/apt/user/index.apt 6a86b1b > > Diff: https://reviews.apache.org/r/37934/diff/ > > > Testing > ------- > > Have added unit test cases for parsing, resolution of parameter and service > testing. > > > Thanks, > > Amruth Sampath > >
