-----------------------------------------------------------
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.


Changes
-------

- update test cases


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 (updated)
-----

  
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

Reply via email to