http://codereview.appspot.com/1770043/diff/13001/14001 File main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (left):
http://codereview.appspot.com/1770043/diff/13001/14001#oldcode211 main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:211: throws IOException { This method (and it's constituent method calls) wasn't throwing IOException. Good to have this cleaned up. http://codereview.appspot.com/1770043/diff/13001/14001 File main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (right): http://codereview.appspot.com/1770043/diff/13001/14001#newcode86 main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:86: IOUtils.copy(results.getResponse(), response.getOutputStream()); You can add another small function (10 odd lines) for block #75-#86 and name it CreateResponse() http://codereview.appspot.com/1770043/diff/13001/14001#newcode90 main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:90: protected HttpRequest buildHttpRequest(HttpServletRequest request) Move this overloaded method 'buildHttpRequest' near the other method (that takes two args). http://codereview.appspot.com/1770043/diff/13001/14001#newcode108 main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:108: // throw appropriate gadget exception. Are we throwing GadgetException ? If not, revisit the comment. http://codereview.appspot.com/1770043/diff/13001/14003 File main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java (right): http://codereview.appspot.com/1770043/diff/13001/14003#newcode100 main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java:100: response.getWriter().write(UNPARSEABLE_CRUFT + output); Block #91-#100 -> CreateResponse() http://codereview.appspot.com/1770043/diff/13001/14004 File main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java (right): http://codereview.appspot.com/1770043/diff/13001/14004#newcode203 main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java:203: protected HttpRequest buildHttpRequest(HttpServletRequest request) Don't you want to make this method abstract. If for a particular case, this method implementation is null, let that specific ProxyHandler define this will null body. Otherwise, just don't declare this method in this abstract class (interface) (if you feel that this is not generic enough. Also in that case, make this method private in the other ProxyHandlers). http://codereview.appspot.com/1770043/diff/13001/14005 File main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (right): http://codereview.appspot.com/1770043/diff/13001/14005#newcode175 main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java:175: HttpRequest rcr = buildHttpRequest(request, proxyUri, proxyUri.getResource()); What does 'rcr' signify/mean ? http://codereview.appspot.com/1770043/show
