> On Jan. 14, 2013, 4:15 p.m., Ryan Baxter wrote: > > Marshall I am seeing some compilation problems when applying this patch can > > you take a look? > > > > > > > > testProcessorModifiesResponse(org.apache.shindig.gadgets.js.DefaultJsProcessorRegistryTest): > > Unresolved compilation problem: > > The constructor DefaultJsProcessorRegistry(ImmutableList<JsProcessor>, > > ImmutableList<JsProcessor>) is undefined > > > > > > testTwoProcessorsAreRunOneAfterAnother(org.apache.shindig.gadgets.js.DefaultJsProcessorRegistryTest): > > Unresolved compilation problem: > > The constructor DefaultJsProcessorRegistry(ImmutableList<JsProcessor>, > > ImmutableList<JsProcessor>) is undefined > > > > > > testProcessorStopsProcessingWhenItReturnsFalse(org.apache.shindig.gadgets.js.DefaultJsProcessorRegistryTest): > > Unresolved compilation problem: > > The constructor DefaultJsProcessorRegistry(ImmutableList<JsProcessor>, > > ImmutableList<JsProcessor>) is undefined
Ryan, I skipped the unit test last time, now they are fixed. - Marshall ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8766/#review15314 ----------------------------------------------------------- On Jan. 15, 2013, 2:22 a.m., Marshall Shi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8766/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2013, 2:22 a.m.) > > > Review request for shindig, Ryan Baxter, Dan Dumont, Stanton Sievers, and > Rich Thompson. > > > Description > ------- > > The gadgets js servlet response can never be 304. Even if the > If-Modified-Since header present in the request header and the gadgets js > content has been cached in browser. > > Shindig provides required js processors and optional js processors. In > current implementation, the IfModifiedSinceProcessor is doing the right thing > to set a 304 response status code and stop the other optional js processors. > But CompilationProcessor, which is a required js processor, will always run > and always use a 200 to overwrite the 304 status code. Thus in the gadget js > servlet layer, it won't return 304. > > The proposed fix is to move IfModifiedSinceProcessor to a third type of > processor: preprocessors. If one of the preprocessors return false, the > entire process will be returned. So the closure compilor won't start. In the > gadget js servlet, also add a check for 304 status code. > > > This addresses bug SHINDIG-1890. > https://issues.apache.org/jira/browse/SHINDIG-1890 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistry.java > 1406188 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java > 1406188 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java > 1406188 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistryTest.java > 1406188 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsServletTest.java > 1406188 > > Diff: https://reviews.apache.org/r/8766/diff/ > > > Testing > ------- > > > Thanks, > > Marshall Shi > >
