new patch coming..
http://codereview.appspot.com/1032045/diff/1/4 File java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java (right): http://codereview.appspot.com/1032045/diff/1/4#newcode95 java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java:95: if ((content.indexOf('[') != -1) && content.indexOf('[') < content.indexOf('{')) { On 2010/05/01 02:41:06, johnfargo wrote:
duplicated elsewhere in the code: should be broken out into at least
an isBatch
method
Done. http://codereview.appspot.com/1032045/diff/1/4#newcode108 java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java:108: } On 2010/05/01 02:41:06, johnfargo wrote:
this whole if/else block is identical to that found in doPost, but
with a JSONP
wrapper. let's just wrap ServletResponse in a helper class that either
does a
Jsonp wrap or not -- or break this out into a separate method that
takes boolean
isJsonp.
I combined the doGet/doPost methods into a single one since there's a bunch of other duplicate code too. http://codereview.appspot.com/1032045/diff/1/4#newcode115 java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java:115: throw new IllegalArgumentException("could not find data to process"); On 2010/05/01 02:41:06, johnfargo wrote:
IMO would be preferable not to throw a RuntimeException
Done. Replaced with explicit error return http://codereview.appspot.com/1032045/show
