http://codereview.appspot.com/3734041/diff/127001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java (right):
http://codereview.appspot.com/3734041/diff/127001/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java#newcode190 java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatVisitor.java:190: for(Uri uri : concatUri.getUris()) { Space before ( http://codereview.appspot.com/3734041/diff/127001/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java File java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java (right): http://codereview.appspot.com/3734041/diff/127001/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java#newcode130 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:130: uriBuilder = makeUriBuilder(ctx, concatHost, concatPath); Move the newline on line 131 to before line 130, since we are trying to reinitialize everything starting from line 130. http://codereview.appspot.com/3734041/diff/127001/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java#newcode167 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:167: if (versions != null && versions.size() > 0) { Will versions.size() ever be greater than 1 here? http://codereview.appspot.com/3734041/diff/127001/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java#newcode182 java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManager.java:182: while((resourceUri = uri.getQueryParameter(i.toString())) != null) { Space after while http://codereview.appspot.com/3734041/diff/127001/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManagerTest.java File java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManagerTest.java (right): http://codereview.appspot.com/3734041/diff/127001/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManagerTest.java#newcode309 java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultConcatUriManagerTest.java:309: public void dontConcatMultipleResourceBeyoundUrlLimit() throws Exception { Yet to go through this test in detail, but does it assume that splitJsEnabled is true or false? Can we add tests for both scenarios because even in the non-json case, if there are enough adjacent js/css files, we can easily exceed the GET URL limit and ideally, these cases should be fixed by this patch. http://codereview.appspot.com/3734041/
