Author: ddumont
Date: Mon Apr 30 15:16:42 2012
New Revision: 1332240
URL: http://svn.apache.org/viewvc?rev=1332240&view=rev
Log:
Use refreshInterval in makeRequest sparingly.
Modified:
shindig/trunk/features/src/main/javascript/features/core.io/io.js
shindig/trunk/features/src/test/javascript/features/core.io/iotest.js
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpCache.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java
Modified: shindig/trunk/features/src/main/javascript/features/core.io/io.js
URL:
http://svn.apache.org/viewvc/shindig/trunk/features/src/main/javascript/features/core.io/io.js?rev=1332240&r1=1332239&r2=1332240&view=diff
==============================================================================
--- shindig/trunk/features/src/main/javascript/features/core.io/io.js (original)
+++ shindig/trunk/features/src/main/javascript/features/core.io/io.js Mon Apr
30 15:16:42 2012
@@ -367,11 +367,6 @@ gadgets.io = function() {
if (params['AUTHORIZATION'] && params['AUTHORIZATION'] !== 'NONE') {
auth = params['AUTHORIZATION'].toLowerCase();
st = shindig.auth.getSecurityToken();
- } else {
- // Unauthenticated GET requests are cacheable
- if (httpMethod === 'GET' && typeof refreshInterval == 'undefined') {
- refreshInterval = 3600;
- }
}
// Include owner information?
@@ -433,20 +428,16 @@ gadgets.io = function() {
// FIXME -- processResponse is not used in call
if (!respondWithPreload(paramData, params, callback)) {
- if (httpMethod === 'GET' && refreshInterval > 0) {
- // this content should be cached
- // Add paramData to the URL
- var extraparams = '?refresh=' + refreshInterval + '&' +
- gadgets.io.encodeValues(paramData);
+ if (httpMethod == 'GET' && typeof(refreshInterval) != 'undefined') {
+ paramData['refresh'] = refreshInterval; // gadget requested cache
override.
+ }
+ if (httpMethod === 'GET' && !paramData['authz']) {
+ var extraparams = '?' + gadgets.io.encodeValues(paramData);
makeXhrRequest(url, proxyUrl + extraparams, callback,
null, 'GET', params, processResponse);
-
} else {
var extraparams = gadgets.io.encodeValues(paramData);
- if (httpMethod === 'GET' && refreshInterval == 0) {
- extraparams = extraparams + "&refresh=0";
- }
makeXhrRequest(url, proxyUrl, callback,
extraparams, 'POST', params,
processResponse);
Modified: shindig/trunk/features/src/test/javascript/features/core.io/iotest.js
URL:
http://svn.apache.org/viewvc/shindig/trunk/features/src/test/javascript/features/core.io/iotest.js?rev=1332240&r1=1332239&r2=1332240&view=diff
==============================================================================
--- shindig/trunk/features/src/test/javascript/features/core.io/iotest.js
(original)
+++ shindig/trunk/features/src/test/javascript/features/core.io/iotest.js Mon
Apr 30 15:16:42 2012
@@ -114,7 +114,6 @@ IoTest.prototype.setArg = function(req,
};
IoTest.prototype.setStandardArgs = function(req, inBody) {
- this.setArg(req, inBody, "refresh", "3600");
this.setArg(req, inBody, "st", "");
this.setArg(req, inBody, "contentType", "TEXT");
this.setArg(req, inBody, "authz", "");
@@ -176,11 +175,10 @@ IoTest.prototype.testNoMethod_nonDefault
};
IoTest.prototype.testNoMethod_disableRefresh = function() {
- var req = new fakeXhr.Expectation("POST", "http://example.com/json";);
- this.setStandardArgs(req, true);
- req.setBodyArg("url", "http://target.example.com/somepage";);
- req.setBodyArg("refresh", "0");
- req.setHeader("Content-Type", "application/x-www-form-urlencoded");
+ var req = new fakeXhr.Expectation("GET", "http://example.com/json";);
+ this.setStandardArgs(req, false);
+ req.setQueryArg("url", "http://target.example.com/somepage";);
+ req.setQueryArg("refresh", "0");
var resp = this.makeFakeResponse(
"{ 'http://target.example.com/somepage' : { 'body' : 'some data', 'rc' :
200 }}");
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java?rev=1332240&r1=1332239&r2=1332240&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java
Mon Apr 30 15:16:42 2012
@@ -103,7 +103,7 @@ public abstract class AbstractHttpCache
return null;
}
- public boolean addResponse(HttpRequest request, HttpResponse response) {
+ public HttpResponse addResponse(HttpRequest request, HttpResponse response) {
HttpResponseBuilder responseBuilder;
boolean storeStrictNoCacheResources = (refetchStrictNoCacheAfterMs >= 0);
if (isCacheable(request, response, storeStrictNoCacheResources)) {
@@ -113,7 +113,7 @@ public abstract class AbstractHttpCache
responseBuilder = new HttpResponseBuilder(response);
}
} else {
- return false;
+ return null;
}
int forcedTtl = request.getCacheTtl();
if (forcedTtl != -1) {
@@ -122,7 +122,7 @@ public abstract class AbstractHttpCache
response = responseBuilder.create();
String keyString = createKey(request);
addResponseImpl(keyString, response);
- return true;
+ return response; // cached and possibly modified
}
@VisibleForTesting
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java?rev=1332240&r1=1332239&r2=1332240&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
Mon Apr 30 15:16:42 2012
@@ -232,7 +232,10 @@ public class DefaultRequestPipeline impl
if (fetchedResponse.getCacheTtl() > 0) {
fetchedResponse = invalidationService.markResponse(request,
fetchedResponse);
}
- httpCache.addResponse(request, fetchedResponse);
+ HttpResponse cached = httpCache.addResponse(request, fetchedResponse);
+ if (cached != null) {
+ fetchedResponse = cached; // possibly modified response.
+ }
}
return fetchedResponse;
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpCache.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpCache.java?rev=1332240&r1=1332239&r2=1332240&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpCache.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpCache.java
Mon Apr 30 15:16:42 2012
@@ -32,9 +32,9 @@ public interface HttpCache {
/**
* Add a request/response pair to the cache.
*
- * @return true if the response was cached, false if the response was not
cached.
+ * @return The response that was cached, null otherwise.
*/
- boolean addResponse(HttpRequest request, HttpResponse response);
+ HttpResponse addResponse(HttpRequest request, HttpResponse response);
HttpResponse removeResponse(HttpRequest key);
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java?rev=1332240&r1=1332239&r2=1332240&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java
Mon Apr 30 15:16:42 2012
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertEqu
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotNull;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
@@ -309,10 +310,10 @@ public class AbstractHttpCacheTest {
HttpResponse response = new HttpResponse("normal");
String key = cache.createKey(request);
- assertTrue("response should have been cached", cache.addResponse(request,
response));
+ assertNotNull("response should have been cached",
cache.addResponse(request, response));
assertEquals(response, cache.map.get(key));
- assertTrue("response should have been cached",
+ assertNotNull("response should have been cached",
extendedStrictNoCacheTtlCache.addResponse(request, response));
assertEquals(response, extendedStrictNoCacheTtlCache.map.get(key));
}
@@ -323,10 +324,10 @@ public class AbstractHttpCacheTest {
.setIgnoreCache(true);
HttpResponse response = new HttpResponse("does not matter");
- assertFalse("response should not have been cached",
cache.addResponse(request, response));
+ assertNull("response should not have been cached",
cache.addResponse(request, response));
assertEquals(0, cache.map.size());
- assertFalse("response should not have been cached",
+ assertNull("response should not have been cached",
extendedStrictNoCacheTtlCache.addResponse(request, response));
assertEquals(0, extendedStrictNoCacheTtlCache.map.size());
}
@@ -337,10 +338,10 @@ public class AbstractHttpCacheTest {
HttpResponse response = new
HttpResponseBuilder().setStrictNoCache().create();
String key = cache.createKey(request);
- assertFalse(cache.addResponse(request, response));
+ assertNull(cache.addResponse(request, response));
assertEquals(0, cache.map.size());
- assertTrue("response should have been cached",
+ assertNotNull("response should have been cached",
extendedStrictNoCacheTtlCache.addResponse(request, response));
assertEquals(
extendedStrictNoCacheTtlCache.buildStrictNoCacheHttpResponse(response).create(),
@@ -352,10 +353,10 @@ public class AbstractHttpCacheTest {
HttpRequest request = new HttpRequest(DEFAULT_URI);
HttpResponse response = new
HttpResponseBuilder().setHttpStatusCode(HttpResponse.SC_NOT_MODIFIED).create();
- assertFalse(cache.addResponse(request, response));
+ assertNull(cache.addResponse(request, response));
assertEquals(0, cache.map.size());
- assertFalse(extendedStrictNoCacheTtlCache.addResponse(request, response));
+ assertNull(extendedStrictNoCacheTtlCache.addResponse(request, response));
assertEquals(0, extendedStrictNoCacheTtlCache.map.size());
}
@@ -365,10 +366,10 @@ public class AbstractHttpCacheTest {
.setMethod("POST");
HttpResponse response = new HttpResponse("does not matter");
- assertFalse(cache.addResponse(request, response));
+ assertNull(cache.addResponse(request, response));
assertEquals(0, cache.map.size());
- assertFalse(extendedStrictNoCacheTtlCache.addResponse(request, response));
+ assertNull(extendedStrictNoCacheTtlCache.addResponse(request, response));
assertEquals(0, extendedStrictNoCacheTtlCache.map.size());
}
@@ -380,10 +381,10 @@ public class AbstractHttpCacheTest {
HttpResponse response = new HttpResponse("normal");
String key = cache.createKey(request);
- assertTrue(cache.addResponse(request, response));
+ assertNotNull(cache.addResponse(request, response));
assertEquals(response, cache.map.get(key));
- assertTrue(extendedStrictNoCacheTtlCache.addResponse(request, response));
+ assertNotNull(extendedStrictNoCacheTtlCache.addResponse(request,
response));
assertEquals(response, extendedStrictNoCacheTtlCache.map.get(key));
}
@@ -395,11 +396,11 @@ public class AbstractHttpCacheTest {
String key = cache.createKey(request);
HttpResponse response = new HttpResponse("result");
- assertTrue(cache.addResponse(request, response));
+ assertNotNull(cache.addResponse(request, response));
assertEquals("public,max-age=10",
cache.map.get(key).getHeader("Cache-Control"));
- assertTrue(extendedStrictNoCacheTtlCache.addResponse(request, response));
+ assertNotNull(extendedStrictNoCacheTtlCache.addResponse(request,
response));
assertEquals("public,max-age=10",
extendedStrictNoCacheTtlCache.map.get(key).getHeader("Cache-Control"));
}
@@ -415,11 +416,11 @@ public class AbstractHttpCacheTest {
.setStrictNoCache()
.create();
- assertTrue(cache.addResponse(request, response));
+ assertNotNull(cache.addResponse(request, response));
assertEquals("public,max-age=10",
cache.map.get(key).getHeader("Cache-Control"));
- assertTrue(extendedStrictNoCacheTtlCache.addResponse(request, response));
+ assertNotNull(extendedStrictNoCacheTtlCache.addResponse(request,
response));
assertEquals("public,max-age=10",
extendedStrictNoCacheTtlCache.map.get(key).getHeader("Cache-Control"));
}
@@ -431,11 +432,11 @@ public class AbstractHttpCacheTest {
String key = cache.createKey(request);
HttpResponse response = new HttpResponse("no headers");
- assertTrue(cache.addResponse(request, response));
+ assertNotNull(cache.addResponse(request, response));
assertEquals("no headers", cache.map.get(key).getResponseAsString());
- assertTrue(extendedStrictNoCacheTtlCache.addResponse(request, response));
+ assertNotNull(extendedStrictNoCacheTtlCache.addResponse(request,
response));
assertEquals("no headers",
extendedStrictNoCacheTtlCache.map.get(key).getResponseAsString());
}
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java?rev=1332240&r1=1332239&r2=1332240&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java
Mon Apr 30 15:16:42 2012
@@ -324,10 +324,10 @@ public class DefaultRequestPipelineTest
protected FakeHttpCache() {}
- public boolean addResponse(HttpRequest request, HttpResponse response) {
+ public HttpResponse addResponse(HttpRequest request, HttpResponse
response) {
writeCount++;
data.put(request.getUri(), response);
- return true;
+ return response;
}
public HttpResponse getResponse(HttpRequest request) {
