http://codereview.appspot.com/391042/diff/1/2 File java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java (right):
http://codereview.appspot.com/391042/diff/1/2#newcode48 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java:48: HttpResponseMetadataHelper metadataHelper) { may want to make this @Nullable, and allow the hash to be omitted when not present. http://codereview.appspot.com/391042/diff/1/6 File java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseMetadataHelper.java (right): http://codereview.appspot.com/391042/diff/1/6#newcode48 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseMetadataHelper.java:48: public static HttpResponse updateMetadata(HttpResponse response, Map<String, String> values) { will this method ever be used with more than 1 k/v pair? or by any calling context that's not within this class itself? http://codereview.appspot.com/391042/diff/1/6#newcode61 java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpResponseMetadataHelper.java:61: public HttpResponse updateHash(HttpResponse response) { this method doesn't really use any class state, so you could either: A) make it static as well B) refactor the top of this method into String getHash(HttpResponse response), which in turn can be mocked (or overridden) in the DefaultRequestPipeline test so you don't have to calculate data hash to evaluate the test. I'd lean toward the latter. http://codereview.appspot.com/391042/show
