Author: ddumont
Date: Fri Jan 18 23:59:32 2013
New Revision: 1435421
URL: http://svn.apache.org/viewvc?rev=1435421&view=rev
Log:
SHINDIG-1890 If-Modified-Since header not handled properly in gadgets js servlet
Committed for Marshall Shi
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistry.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistryTest.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsServletTest.java
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistry.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistry.java?rev=1435421&r1=1435420&r2=1435421&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistry.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistry.java
Fri Jan 18 23:59:32 2013
@@ -29,18 +29,29 @@ import java.util.List;
*/
public class DefaultJsProcessorRegistry implements JsProcessorRegistry {
+ private final List<JsProcessor> preProcessors;
private final List<JsProcessor> optionalProcessors;
private final List<JsProcessor> requiredProcessors;
@Inject
public DefaultJsProcessorRegistry(
+ @Named("shindig.js.pre-processors") List<JsProcessor> preProcessors,
@Named("shindig.js.optional-processors") List<JsProcessor>
optionalProcessors,
@Named("shindig.js.required-processors") List<JsProcessor>
requiredProcessors) {
+ this.preProcessors = preProcessors;
this.optionalProcessors = optionalProcessors;
this.requiredProcessors = requiredProcessors;
}
public void process(JsRequest request, JsResponseBuilder response) throws
JsException {
+ // JsProcessor defined in preProcessors can determine whether the js
process really need to happen
+ // Typically, IfModifiedSinceProcessor is one of the preProcessors, if it
sets a 304 status code,
+ // all the remaining JsProcessors in optional and required won't be
started.
+ for (JsProcessor processor : preProcessors) {
+ if (!processor.process(request, response)){
+ return;
+ }
+ }
for (JsProcessor processor : optionalProcessors) {
if (!processor.process(request, response)) {
break;
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java?rev=1435421&r1=1435420&r2=1435421&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java
Fri Jan 18 23:59:32 2013
@@ -38,12 +38,19 @@ public class JsServingPipelineModule ext
@Provides
@Inject
+ @Named("shindig.js.pre-processors")
+ public List<JsProcessor> provideProcessors(
+ IfModifiedSinceProcessor ifModifiedSinceProcessor) {
+ return ImmutableList.<JsProcessor>of(ifModifiedSinceProcessor);
+ }
+
+ @Provides
+ @Inject
@Named("shindig.js.optional-processors")
public List<JsProcessor> provideProcessors(
AddJslInfoVariableProcessor addJslInfoVariableProcessor,
DeferJsProcessor deferJsProcessor,
JsLoadProcessor jsLoaderGeneratorProcessor,
- IfModifiedSinceProcessor ifModifiedSinceProcessor,
GetJsContentProcessor getJsContentProcessor,
CajaJsSubtractingProcessor cajaJsSubtractingProcessor,
ExportJsProcessor exportJsProcessor,
@@ -56,7 +63,6 @@ public class JsServingPipelineModule ext
addJslInfoVariableProcessor,
deferJsProcessor,
jsLoaderGeneratorProcessor,
- ifModifiedSinceProcessor,
getJsContentProcessor,
cajaJsSubtractingProcessor,
exportJsProcessor,
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java?rev=1435421&r1=1435420&r2=1435421&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java
Fri Jan 18 23:59:32 2013
@@ -102,6 +102,10 @@ public class JsServlet extends InjectedS
protected void emitJsResponse(JsResponse jsResponse, HttpServletRequest req,
HttpServletResponse resp) throws IOException {
+ if (jsResponse.getStatusCode() == 304) {
+ resp.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
+ return;
+ }
if (jsResponse.getStatusCode() == 200 && jsResponse.toJsString().length()
== 0) {
resp.setStatus(HttpServletResponse.SC_NOT_FOUND);
return;
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistryTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistryTest.java?rev=1435421&r1=1435420&r2=1435421&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistryTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/DefaultJsProcessorRegistryTest.java
Fri Jan 18 23:59:32 2013
@@ -37,6 +37,7 @@ public class DefaultJsProcessorRegistryT
private IMocksControl control;
private JsRequest request;
private JsResponseBuilder response;
+ private JsProcessor processor0;
private JsProcessor processor1;
private JsProcessor processor2;
private JsProcessor processor3;
@@ -47,16 +48,23 @@ public class DefaultJsProcessorRegistryT
control = EasyMock.createControl();
request = control.createMock(JsRequest.class);
response = new JsResponseBuilder();
+ processor0 = control.createMock(JsProcessor.class);
processor1 = control.createMock(JsProcessor.class);
processor2 = control.createMock(JsProcessor.class);
processor3 = control.createMock(JsProcessor.class);
registry = new DefaultJsProcessorRegistry(
+ ImmutableList.of(processor0),
ImmutableList.of(processor1, processor2),
ImmutableList.of(processor3));
}
@Test
public void testProcessorModifiesResponse() throws Exception {
+ JsProcessor preprocessor = new JsProcessor() {
+ public boolean process(JsRequest request, JsResponseBuilder builder) {
+ return true;
+ }
+ };
JsProcessor processor = new JsProcessor() {
public boolean process(JsRequest request, JsResponseBuilder builder) {
builder.clearJs().appendJs(JS_CODE, "js");
@@ -64,6 +72,7 @@ public class DefaultJsProcessorRegistryT
}
};
registry = new DefaultJsProcessorRegistry(
+ ImmutableList.of(preprocessor),
ImmutableList.of(processor),
ImmutableList.<JsProcessor>of());
control.replay();
@@ -74,6 +83,7 @@ public class DefaultJsProcessorRegistryT
@Test
public void testTwoProcessorsAreRunOneAfterAnother() throws Exception {
+ EasyMock.expect(processor0.process(request, response)).andReturn(true);
EasyMock.expect(processor1.process(request, response)).andReturn(true);
EasyMock.expect(processor2.process(request, response)).andReturn(true);
EasyMock.expect(processor3.process(request, response)).andReturn(true);
@@ -84,10 +94,19 @@ public class DefaultJsProcessorRegistryT
@Test
public void testProcessorStopsProcessingWhenItReturnsFalse() throws
Exception {
+ EasyMock.expect(processor0.process(request, response)).andReturn(true);
EasyMock.expect(processor1.process(request, response)).andReturn(false);
EasyMock.expect(processor3.process(request, response)).andReturn(true);
control.replay();
registry.process(request, response);
control.verify();
}
+
+ @Test
+ public void testProcessorStopsProcessingWhenPreProcessorsReturnsFalse()
throws Exception {
+ EasyMock.expect(processor0.process(request, response)).andReturn(false);
+ control.replay();
+ registry.process(request, response);
+ control.verify();
+ }
}
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsServletTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsServletTest.java?rev=1435421&r1=1435420&r2=1435421&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsServletTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsServletTest.java
Fri Jan 18 23:59:32 2013
@@ -80,8 +80,8 @@ public class JsServletTest extends Servl
jsLoadProcessor = new JsLoadProcessor(jsUriManagerMock, jsloadTtlSecs,
true);
JsProcessorRegistry jsProcessorRegistry =
new DefaultJsProcessorRegistry(
- ImmutableList.<JsProcessor>of(jsLoadProcessor, new
IfModifiedSinceProcessor(),
- getJsProcessorMock, new AddOnloadFunctionProcessor()),
+ ImmutableList.<JsProcessor>of(new IfModifiedSinceProcessor()),
+ ImmutableList.<JsProcessor>of(jsLoadProcessor, getJsProcessorMock,
new AddOnloadFunctionProcessor()),
ImmutableList.<JsProcessor>of());
jsServingPipeline = new DefaultJsServingPipeline(jsProcessorRegistry);
@@ -249,7 +249,7 @@ public class JsServletTest extends Servl
builder.addError("Something bad happened");
return false;
}};
- JsProcessorRegistry jsProcessorRegistry = new DefaultJsProcessorRegistry(
+ JsProcessorRegistry jsProcessorRegistry = new
DefaultJsProcessorRegistry(ImmutableList.<JsProcessor> of(),
ImmutableList.<JsProcessor> of(errorProcessor),
ImmutableList.<JsProcessor> of());
JsServingPipeline pipeline = new
DefaultJsServingPipeline(jsProcessorRegistry);
