----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28434/#review62997 -----------------------------------------------------------
Thank you for jumping in Stanley! server/src/main/java/org/apache/sqoop/server/RequestContext.java <https://reviews.apache.org/r/28434/#comment105168> Do you think that it would make sense to make this method private, so that users of this class don't have the option to call the getReader() themselves and have to go through methods of this class (wrapper) to obtain all they need? server/src/main/java/org/apache/sqoop/server/RequestContext.java <https://reviews.apache.org/r/28434/#comment105173> I'm wondering whether this method would return the JSON correctly as expected if I would send POST request as this one: POST /page?argument=value HTTP/1.0 {"json":"value"} E.g. in this case I would expect that the getParametersMap will return Map of size two and then it will depend on the Map ordering whether we get the correct one or not. Let's perhaps add unit tests to ensure that we have the behaviour that we're expecting? - Jarek Cecho On Nov. 25, 2014, 8:34 a.m., Qian Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28434/ > ----------------------------------------------------------- > > (Updated Nov. 25, 2014, 8:34 a.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1795 > https://issues.apache.org/jira/browse/SQOOP-1795 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Sqoop client posts parameters as JSON to server. As it is not query string > based pattern, HttpServletRequest is not able to return the JSON using > `getParameterValue(...)`. The current solution is to call `getReader()` to > get raw post data. There is a danger, if the method is NOT called at the > first place. You do not know, whether the reading position is at the > beginning. You might get unexpected result without notice. > > Expectedly: > 1. For query string based pattern, caller should always use > `getParameterValue(...)`. > 2. For raw post data usage, the post data is parsed as the first parameter's > key. > > The patch proposes to add `RequestContext.getFirstParameterName()`. so that > we can keep fingers away from `getReader()`. > > > Diffs > ----- > > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > 75a069a227b04e045b76c09e721deb45a35cb9eb > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > d2d080bbe2c2f55feb349089b93da7f41bc13bc6 > server/src/main/java/org/apache/sqoop/server/RequestContext.java > d0963f5bcd09a5f31601f677304165d513cc7de3 > > Diff: https://reviews.apache.org/r/28434/diff/ > > > Testing > ------- > > All passed. (Nits: adding unit tests) > > > Thanks, > > Qian Xu > >
