http://codereview.appspot.com/1712043/diff/5001/6003 File java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java (right):
http://codereview.appspot.com/1712043/diff/5001/6003#newcode123 java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/AccelHandler.java:123: Uri normalizedUri = Uri.parse(proxiedUri.getQueryParameter( surround this with try/catch(UriException) for error handling. http://codereview.appspot.com/1712043/diff/5001/6005 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/AccelUriManager.java (right): http://codereview.appspot.com/1712043/diff/5001/6005#newcode50 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/AccelUriManager.java:50: public Uri parseAndNormalize(Gadget gadget, Uri requestUri) throws GadgetException { While we're at it, we should refactor this class a bit to better reflect the needs of Accel: 1. parse method should take an HttpRequest rather than a Gadget. then perhaps (these can be in a followup as well) 2. Don't extend DefaultProxyUriManager, just @Inject a ProxyUriManager to which you delegate when useful. 3. [optional, but fits the pattern] Make AccelUriManager an interface, with DefaultAccelUriManager implementing it. ^^ these two help isolate AccelUriManager from potential future issues having to do w/ DefaultProxyUriManager impl changes. http://codereview.appspot.com/1712043/diff/5001/6005#newcode65 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/AccelUriManager.java:65: String accelPath = config.getString(container, PROXY_PATH_PARAM); You should also just calculate these once in the ctor. http://codereview.appspot.com/1712043/diff/5001/6005#newcode67 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/AccelUriManager.java:67: return accelHost.equals(requestUri.getAuthority()) && +1, at least for testing purposes. You might also, out of conservatism, choose to equalsIgnoreCase http://codereview.appspot.com/1712043/diff/5001/6004 File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java (right): http://codereview.appspot.com/1712043/diff/5001/6004#newcode66 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultProxyUriManager.java:66: protected final ContainerConfig config; not needed; just store your own ContainerConfig reference in AccelUriManager. http://codereview.appspot.com/1712043/show
