Now that rewriting is being done in appropriate places, there need to be tests to verify that rewriting is actually being invoked appropriately. We had several regressions in the other rewriter because of code changes that caused rewriting to be skipped.
On Wed, Sep 10, 2008 at 3:23 PM, <[EMAIL PROTECTED]> wrote: > Author: johnh > Date: Wed Sep 10 15:23:40 2008 > New Revision: 694035 > > URL: http://svn.apache.org/viewvc?rev=694035&view=rev > Log: > Moving rewriting logic out of AbstractHttpCache and into > [Caching]ContentRewriterRegistry. Caching of rewritten content > no longer occurs on an HttpResponse object (to be cleaned up), and is > essentially just a rewriting optimization. > > * AbstractHttpCache significantly cleaned up. > * CachingContentRewriterRegistry subclasses BasicContentRewriterRegistry > rather than funky CachingWebRetrievalFactory. > * CCRR uses TtlCache under the hood for actual caching. > * MakeRequestHandler and ProxyHandler use injected ContentRewriterRegistry > for rewriting. > > > Modified: > > > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestServletTest.java > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java > > Modified: > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java > (original) > +++ > incubator/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/cache/TtlCache.java > Wed Sep 10 15:23:40 2008 > @@ -109,11 +109,10 @@ > * minTtl, minTtl is used. If above maxTtl, maxTtl is used. > * @param key Element key. > * @param val Element value to cache. > - * @param lifetime Intended lifetime, in millis, of the element's entry. > + * @param expiration Time, in millis, that the value is to expire. > */ > - public void addElement(K key, V val, long lifetime) { > + public void addElement(K key, V val, long expiration) { > long now = timeSource.currentTimeMillis(); > - long expiration = lifetime; > expiration = Math.max(now + minTtl, Math.min(now + maxTtl, > expiration)); > TimeoutPair<V> entry = new TimeoutPair<V>(val, expiration); > synchronized(baseCache) { > > Modified: > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/AbstractHttpCache.java > Wed Sep 10 15:23:40 2008 > @@ -17,40 +17,28 @@ > */ > package org.apache.shindig.gadgets.http; > > -import org.apache.shindig.gadgets.MutableContent; > -import org.apache.shindig.gadgets.rewrite.ContentRewriter; > -import org.apache.shindig.gadgets.rewrite.ContentRewriterRegistry; > - > -import com.google.inject.Inject; > - > /** > * Base class for content caches. Defines cache expiration rules and > - * and restrictions on allowed content. Also enforces rewriting > - * on cacheable content. > - * TODO: separate this logic from rewriting - it's confusing > + * and restrictions on allowed content. > */ > public abstract class AbstractHttpCache implements HttpCache { > > - private ContentRewriterRegistry rewriterRegistry; > - > - @Inject > - public void setRewriterRegistry(ContentRewriterRegistry registry) { > - rewriterRegistry = registry; > - } > + // Implement these methods to create a concrete HttpCache class. > + protected abstract HttpResponse getResponseImpl(String key); > + protected abstract void addResponseImpl(String key, HttpResponse > response); > + protected abstract HttpResponse removeResponseImpl(String key); > > public final HttpResponse getResponse(HttpCacheKey key, HttpRequest > request) { > if (key.isCacheable()) { > String keyString = key.toString(); > HttpResponse cached = getResponseImpl(keyString); > if (responseStillUsable(cached)) { > - return checkRewrite(keyString, request, cached); > + return cached; > } > } > return null; > } > > - protected abstract HttpResponse getResponseImpl(String key); > - > public HttpResponse addResponse(HttpCacheKey key, HttpRequest request, > HttpResponse response) { > if (key.isCacheable() && response != null) { > // !!! Note that we only rewrite cacheable content. Move this call > above the if > @@ -62,24 +50,17 @@ > } > > response = responseBuilder.create(); > - HttpResponse rewritten = checkRewrite(key.toString(), request, > response); > - if (rewritten == response) { > - // Nothing rewritten (and thus cached). Cache the entry. > - addResponseImpl(key.toString(), response); > - } else { > - return rewritten; > - } > + addResponseImpl(key.toString(), response); > > if (!request.getIgnoreCache() && > response.getRewritten() != null) { > return response.getRewritten(); > } > } > + > return response; > } > > - protected abstract void addResponseImpl(String key, HttpResponse > response); > - > public HttpResponse removeResponse(HttpCacheKey key) { > String keyString = key.toString(); > HttpResponse response = getResponseImpl(keyString); > @@ -90,8 +71,6 @@ > return null; > } > > - protected abstract HttpResponse removeResponseImpl(String key); > - > /** > * Utility function to verify that an entry is cacheable and not expired > * @return true If the response can be used. > @@ -102,50 +81,4 @@ > } > return response.getCacheExpiration() > System.currentTimeMillis(); > } > - > - /** > - * Add rewritten content to the response if its not there and > - * we can add it. (Re-)cache if we created rewritten content. > - * Return the appropriately re-written version if requested. > - * @return Original response object if not rewritten; rewritten object > if so. > - */ > - protected HttpResponse checkRewrite(String key, > - HttpRequest request, HttpResponse response) { > - if (response == null) { > - return null; > - } > - > - // Perform a rewrite and store the content back to the cache if the > - // content is actually rewritten > - if (response.getRewritten() == null) { > - HttpResponse rewritten = rewrite(request, response); > - > - if (rewritten != null) { > - // TODO: Remove this and other rewriting logic from http cache > when ready. > - response = new > HttpResponseBuilder(response).setRewritten(rewritten).create(); > - addResponseImpl(key, response); > - } > - } > - > - // Return the rewritten version if requested > - if (!request.getIgnoreCache() && > - rewriterRegistry != null && > - response.getRewritten() != null && > - response.getRewritten().getContentLength() > 0) { > - return response.getRewritten(); > - } > - > - return response; > - } > - > - /** > - * Rewrite the content. > - * @return rewritten HttpResponse object, if rewriting occurred. > - */ > - protected HttpResponse rewrite(HttpRequest request, HttpResponse > response) { > - if (rewriterRegistry != null) { > - return rewriterRegistry.rewriteHttpResponse(request, response); > - } > - return null; > - } > } > > Modified: > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistry.java > Wed Sep 10 15:23:40 2008 > @@ -17,15 +17,14 @@ > */ > package org.apache.shindig.gadgets.rewrite; > > -import java.util.List; > -import java.util.logging.Logger; > - > import org.apache.shindig.common.cache.CacheProvider; > -import org.apache.shindig.gadgets.CachingWebRetrievalFactory; > +import org.apache.shindig.common.cache.TtlCache; > +import org.apache.shindig.common.util.HashUtil; > import org.apache.shindig.gadgets.Gadget; > import org.apache.shindig.gadgets.GadgetException; > import org.apache.shindig.gadgets.http.HttpRequest; > import org.apache.shindig.gadgets.http.HttpResponse; > +import org.apache.shindig.gadgets.http.HttpResponseBuilder; > import org.apache.shindig.gadgets.parse.GadgetHtmlParser; > import org.apache.shindig.gadgets.rewrite.BasicContentRewriterRegistry; > import org.apache.shindig.gadgets.rewrite.ContentRewriter; > @@ -34,93 +33,115 @@ > import com.google.inject.Inject; > import com.google.inject.name.Named; > > +import java.util.List; > + > /** > - * Implementation of a content rewriter registry that delegates to > - * [EMAIL PROTECTED] BasicContentRewriterRegistry} for base operations, but > also > - * provides a layer of caching atop that. > + * Standard implementation of a content rewriter with caching logic > layered atop it. > + * Uses [EMAIL PROTECTED] BasicContentRewriterRegistry} for actual > rewriting, and a > + * [EMAIL PROTECTED] TtlCache}, whose underlying persistence is provided by > [EMAIL PROTECTED] > CacheProvider}, > + * as the cache. > */ > -public class CachingContentRewriterRegistry > - extends CachingWebRetrievalFactory<String, Gadget, String> > - implements ContentRewriterRegistry { > +public class CachingContentRewriterRegistry extends > BasicContentRewriterRegistry { > > - static final Logger logger = > Logger.getLogger(CachingContentRewriterRegistry.class.getName()); > - private final BasicContentRewriterRegistry baseRegistry; > + private final TtlCache<String, String> rewrittenCache; > + private String rewritersKey; > > @Inject > public CachingContentRewriterRegistry(ContentRewriter firstRewriter, > GadgetHtmlParser htmlParser, > CacheProvider cacheProvider, > - @Named("shindig.gadget-spec.cache.capacity")int capacity, > - @Named("shindig.gadget-spec.cache.minTTL")long minTtl, > - @Named("shindig.gadget-spec.cache.maxTTL")long maxTtl) { > - // Cache configuration values are currently identical to those for the > spec > - // cache in BasicGadgetSpecFactory for backward compatibility > (ensuring that if > - // caching is set up for specs, it's set up for rewritten content as > well) > - // TODO: create separate rewritten-content config values. > - super(cacheProvider, capacity, minTtl, maxTtl); > - baseRegistry = new BasicContentRewriterRegistry(firstRewriter, > htmlParser); > - } > - > - @Override > - protected String getCacheKeyFromQueryObj(Gadget gadget) { > - // Cache by URI + View. Since we always append a view, there should be > no > - // key conflicts associated with this operation. > - return gadget.getSpec().getUrl().toString() + "#v=" + > gadget.getCurrentView().getName(); > - } > - > - @Override > - protected Logger getLogger() { > - return logger; > - } > - > - @Override > - protected FetchedObject<String> retrieveRawObject(Gadget gadget, > - boolean ignoreCache) throws GadgetException { > - // Always attempt to rewrite the inbound gadget object. > - // Even if that fails, the non-rewritten Gadget should be cached, > - // to avoid superfluous rewrites later. > - baseRegistry.rewriteGadget(gadget); > - > - // Expiration time of 30 minutes by default. This number is arbitrary. > - // TODO: Make this value configurable. > - long expiration = System.currentTimeMillis() + (1000 * 60 * 30); > - Object expirationObj = > gadget.getSpec().getAttribute(GadgetSpec.EXPIRATION_ATTRIB); > - if (expirationObj instanceof Long) { > - expiration = (Long)expirationObj; > - } > - > - return new FetchedObject<String>(gadget.getContent(), expiration); > + @Named("shindig.rewritten-content.cache.capacity")int capacity, > + @Named("shindig.rewritten-content.cache.minTTL")long minTtl, > + @Named("shindig.rewritten-content.cache.maxTTL")long maxTtl) { > + super(firstRewriter, htmlParser); > + rewrittenCache = new TtlCache<String, String>(cacheProvider, capacity, > minTtl, maxTtl); > + } > + > + protected String getGadgetCacheKey(Gadget gadget) { > + return getRewritersKey() + ":" + > HashUtil.checksum(gadget.getContent().getBytes()); > } > > - /** [EMAIL PROTECTED] */ > - public List<ContentRewriter> getRewriters() { > - return baseRegistry.getRewriters(); > + protected String getHttpResponseCacheKey(HttpRequest req, HttpResponse > response) { > + return getRewritersKey() + ":" + req.getUri().toString() + ":" + > + HashUtil.checksum(response.getResponseAsString().getBytes()); > + } > + > + private String getRewritersKey() { > + if (rewritersKey == null) { > + // No need for lock: "rewriter key" generation is idempotent > + StringBuilder keyBuilder = new StringBuilder(); > + List<ContentRewriter> rewriters = getRewriters(); > + for (ContentRewriter rewriter : rewriters) { > + keyBuilder.append(rewriter.getClass().getCanonicalName()) > + .append("-").append(rewriter.getClass().hashCode()); > + } > + rewritersKey = keyBuilder.toString(); > + } > + return rewritersKey; > } > > /** [EMAIL PROTECTED] */ > public boolean rewriteGadget(Gadget gadget) > throws GadgetException { > - String cached = doCachedFetch(gadget, > gadget.getContext().getIgnoreCache()); > + if (gadget.getContext().getIgnoreCache()) { > + return super.rewriteGadget(gadget); > + } > + > + String cacheKey = getGadgetCacheKey(gadget); > + String cached = rewrittenCache.getElement(cacheKey); > > - // At present, the output of rewriting is just the string contained > within > - // the Gadget object. Thus, a successful cache hit results in copying > over the > - // rewritten value to the input gadget object. > - // TODO: Clean up the ContentRewriter interface so rewriting "output" > is clearer. > - // TODO: If necessary later, copy other modified contents to Gadget > object. > if (cached != null) { > gadget.setContent(cached); > return true; > } > > - return baseRegistry.rewriteGadget(gadget); > + // Do a fresh rewrite and cache the results. > + boolean rewritten = super.rewriteGadget(gadget); > + if (rewritten) { > + // Only cache if the rewriters did something. > + long expiration = 0; > + Object expirationObj = > gadget.getSpec().getAttribute(GadgetSpec.EXPIRATION_ATTRIB); > + if (expirationObj instanceof Long) { > + expiration = (Long)expirationObj; > + } > + rewrittenCache.addElement(cacheKey, gadget.getContent(), > expiration); > + } > + > + return rewritten; > } > > /** [EMAIL PROTECTED] */ > public HttpResponse rewriteHttpResponse(HttpRequest req, HttpResponse > resp) { > - return baseRegistry.rewriteHttpResponse(req, resp); > - } > - > - public void appendRewriter(ContentRewriter rewriter) { > - baseRegistry.appendRewriter(rewriter); > + if (req.getIgnoreCache()) { > + return super.rewriteHttpResponse(req, resp); > + } > + > + String cacheKey = getHttpResponseCacheKey(req, resp); > + String cached = rewrittenCache.getElement(cacheKey); > + > + if (cached != null) { > + return new > HttpResponseBuilder(resp).setResponseString(cached).create(); > + } > + > + HttpResponse rewritten = super.rewriteHttpResponse(req, resp); > + if (rewritten != null) { > + // Favor forced cache TTL from request first, then > + // the response's cache expiration, then the response's cache TTL > + long forceTtl = req.getCacheTtl(); > + long expiration = 0; > + if (forceTtl > 0) { > + expiration = System.currentTimeMillis() + forceTtl; > + } else { > + expiration = resp.getCacheExpiration(); > + } > + if (expiration == -1) { > + expiration = System.currentTimeMillis() + resp.getCacheTtl(); > + } > + > + // All these are bounded by the provided TTLs > + rewrittenCache.addElement(cacheKey, rewritten.getResponseAsString(), > expiration); > + } > + > + return rewritten; > } > } > > Modified: > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java > Wed Sep 10 15:23:40 2008 > @@ -29,6 +29,7 @@ > import org.apache.shindig.gadgets.http.HttpRequest; > import org.apache.shindig.gadgets.http.HttpResponse; > import org.apache.shindig.gadgets.oauth.OAuthArguments; > +import org.apache.shindig.gadgets.rewrite.ContentRewriterRegistry; > import org.apache.shindig.gadgets.spec.Auth; > > import com.google.inject.Inject; > @@ -49,7 +50,7 @@ > * Unlike ProxyHandler, this may perform operations such as OAuth or signed > fetch. > */ > @Singleton > -public class MakeRequestHandler extends ProxyBase{ > +public class MakeRequestHandler extends ProxyBase { > // Relaxed visibility for ease of integration. Try to avoid relying on > these. > public static final String UNPARSEABLE_CRUFT = "throw 1; < don't be evil' > >"; > public static final String POST_DATA_PARAM = "postData"; > @@ -63,10 +64,13 @@ > public static final String AUTHZ_PARAM = "authz"; > > private final ContentFetcherFactory contentFetcherFactory; > + private final ContentRewriterRegistry contentRewriterRegistry; > > @Inject > - public MakeRequestHandler(ContentFetcherFactory contentFetcherFactory) { > + public MakeRequestHandler(ContentFetcherFactory contentFetcherFactory, > + ContentRewriterRegistry contentRewriterRegistry) { > this.contentFetcherFactory = contentFetcherFactory; > + this.contentRewriterRegistry = contentRewriterRegistry; > } > > /** > @@ -94,6 +98,11 @@ > > // Serialize the response > HttpResponse results = fetcher.fetch(rcr); > + > + // Rewrite the response > + if (contentRewriterRegistry != null) { > + results = contentRewriterRegistry.rewriteHttpResponse(rcr, results); > + } > > // Serialize the response > String output = convertResponseToJson(authToken, request, results); > > Modified: > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java > Wed Sep 10 15:23:40 2008 > @@ -28,7 +28,7 @@ > import org.apache.shindig.gadgets.http.HttpFetcher; > import org.apache.shindig.gadgets.http.HttpRequest; > import org.apache.shindig.gadgets.http.HttpResponse; > -import org.apache.shindig.gadgets.rewrite.ContentRewriter; > +import org.apache.shindig.gadgets.rewrite.ContentRewriterRegistry; > > import javax.servlet.http.HttpServletRequest; > import javax.servlet.http.HttpServletResponse; > @@ -55,13 +55,15 @@ > // This is a limitation of Guice, but this workaround...works. > private final HttpFetcher fetcher; > private final LockedDomainService lockedDomainService; > + private final ContentRewriterRegistry contentRewriterRegistry; > > @Inject > public ProxyHandler(HttpFetcher fetcher, > LockedDomainService lockedDomainService, > - ContentRewriter rewriter) { > + ContentRewriterRegistry contentRewriterRegistry) { > this.fetcher = fetcher; > this.lockedDomainService = lockedDomainService; > + this.contentRewriterRegistry = contentRewriterRegistry; > } > > /** > @@ -116,10 +118,12 @@ > > HttpRequest rcr = buildHttpRequest(request); > HttpResponse results = fetcher.fetch(rcr); > + if (contentRewriterRegistry != null) { > + results = contentRewriterRegistry.rewriteHttpResponse(rcr, results); > + } > > setResponseHeaders(request, response, results); > > - > for (Map.Entry<String, List<String>> entry : > results.getHeaders().entrySet()) { > String name = entry.getKey(); > if (!DISALLOWED_RESPONSE_HEADERS.contains(name.toLowerCase())) { > > Modified: > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java > Wed Sep 10 15:23:40 2008 > @@ -64,7 +64,7 @@ > } > } > > - protected static class CaptureRewriter implements ContentRewriter { > + public static class CaptureRewriter implements ContentRewriter { > private boolean rewroteView = false; > private boolean rewroteResponse = false; > > > Modified: > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java > Wed Sep 10 15:23:40 2008 > @@ -46,7 +46,7 @@ > injector = Guice.createInjector(new TestCacheModule()); > } > > - public void testCacheWithRewritingOps() { > + public void testCache() { > // Setup: could move this elsewhere, but no real need right now. > HttpCacheKey key = EasyMock.createNiceMock(HttpCacheKey.class); > expect(key.isCacheable()).andReturn(true).anyTimes(); > @@ -59,10 +59,12 @@ > > // Actual test. > AbstractHttpCache ahc = injector.getInstance(TestHttpCache.class); > - HttpResponse rewritten = ahc.addResponse(key, request, response); > - assertNotSame(rewritten, response); > - assertEquals(PFX_STR + "foo", rewritten.getResponseAsString()); > - assertSame(rewritten, ahc.getResponse(key, request)); > + HttpResponse added = ahc.addResponse(key, request, response); > + assertNotSame(added, response); > + > + // Not rewritten (anymore). > + assertEquals("foo", added.getResponseAsString()); > + assertSame(added, ahc.getResponse(key, request)); > assertEquals(response, ahc.removeResponse(key)); > } > > > Modified: > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/CachingContentRewriterRegistryTest.java > Wed Sep 10 15:23:40 2008 > @@ -21,8 +21,12 @@ > import static org.easymock.classextension.EasyMock.replay; > > import org.apache.shindig.common.cache.DefaultCacheProvider; > +import org.apache.shindig.common.uri.Uri; > import org.apache.shindig.gadgets.Gadget; > import org.apache.shindig.gadgets.GadgetContext; > +import org.apache.shindig.gadgets.http.HttpRequest; > +import org.apache.shindig.gadgets.http.HttpResponse; > +import org.apache.shindig.gadgets.http.HttpResponseBuilder; > import org.apache.shindig.gadgets.spec.GadgetSpec; > import org.apache.shindig.gadgets.spec.View; > import org.easymock.classextension.EasyMock; > @@ -60,6 +64,10 @@ > expect(spec.getUrl()).andReturn(new URI("http://gadget.org/gadget.xml > ")).anyTimes(); > GadgetContext context = EasyMock.createNiceMock(GadgetContext.class); > > expect(context.getView()).andReturn(GadgetSpec.DEFAULT_VIEW).anyTimes(); > + HttpRequest request = new HttpRequest(Uri.parse(" > http://request.org/cgi-bin/request.py";)); > + request.setCacheTtl(Integer.MAX_VALUE); > + HttpResponse resp = new > HttpResponseBuilder().setResponseString(inputContent) > + > .setCacheTtl(-1).setExpirationTime(-1).setHttpStatusCode(200).create(); > replay(context, view, spec); > > Gadget gadget = new Gadget(context, spec, null, null, null); > @@ -69,6 +77,11 @@ > assertTrue(r.rewriteGadget(gadget)); > assertEquals(rewrittenContent, gadget.getContent()); > > + // Likewise for the http response > + HttpResponse rewrittenResp = r.rewriteHttpResponse(request, resp); > + assertNotSame(rewrittenResp, resp); > + assertEquals(rewrittenContent, rewrittenResp.getResponseAsString()); > + > r.appendRewriter(new AppendingRewriter("-end")); > > // Should also be rewritten the second time, but with the previous > @@ -76,5 +89,8 @@ > Gadget nextGadget = new Gadget(context, spec, null, null, null); > assertTrue(r.rewriteGadget(nextGadget)); > assertEquals(rewrittenContent, nextGadget.getContent()); > + > + HttpResponse rewrittenResp2 = r.rewriteHttpResponse(request, resp); > + assertEquals(rewrittenContent, rewrittenResp2.getResponseAsString()); > } > } > > Modified: > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java > Wed Sep 10 15:23:40 2008 > @@ -31,6 +31,7 @@ > import org.apache.shindig.gadgets.http.HttpRequest; > import org.apache.shindig.gadgets.http.HttpResponse; > import org.apache.shindig.gadgets.http.HttpResponseBuilder; > +import org.apache.shindig.gadgets.rewrite.BasicContentRewriterRegistry; > import org.apache.shindig.gadgets.spec.Auth; > > import com.google.common.collect.Lists; > @@ -52,7 +53,8 @@ > private static final String RESPONSE_BODY = "makeRequest response body"; > private static final SecurityToken DUMMY_TOKEN = new FakeGadgetToken(); > > - private final MakeRequestHandler handler = new > MakeRequestHandler(contentFetcherFactory); > + private final MakeRequestHandler handler = new > MakeRequestHandler(contentFetcherFactory, > + new BasicContentRewriterRegistry(rewriter, null)); > > private void expectGetAndReturnBody(String response) throws Exception { > expectGetAndReturnBody(fetcher, response); > @@ -100,6 +102,7 @@ > JSONObject results = extractJsonFromResponse(); > assertEquals(HttpResponse.SC_OK, results.getInt("rc")); > assertEquals(RESPONSE_BODY, results.get("body")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testExplicitHeaders() throws Exception { > @@ -128,6 +131,7 @@ > JSONObject results = extractJsonFromResponse(); > assertEquals(HttpResponse.SC_OK, results.getInt("rc")); > assertEquals(RESPONSE_BODY, results.get("body")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testPostRequest() throws Exception { > @@ -140,6 +144,7 @@ > > assertEquals(HttpResponse.SC_OK, results.getInt("rc")); > assertEquals(RESPONSE_BODY, results.get("body")); > + assertTrue(rewriter.responseWasRewritten()); > } > > @Test > @@ -172,6 +177,7 @@ > assertEquals(entryLink, entry.getString("Link")); > assertNull("getSummaries has the wrong default value (should be > false).", > entry.optString("Summary", null)); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testFetchFeedWithParameters() throws Exception { > @@ -218,6 +224,7 @@ > assertEquals(entryLink, entry.getString("Link")); > assertTrue("getSummaries not parsed correctly.", entry.has("Summary")); > assertEquals(entrySummary, entry.getString("Summary")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testFetchEmptyDocument() throws Exception { > @@ -229,6 +236,7 @@ > > assertEquals(HttpResponse.SC_OK, results.getInt("rc")); > assertEquals("", results.get("body")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testSignedGetRequest() throws Exception { > @@ -245,6 +253,7 @@ > JSONObject results = extractJsonFromResponse(); > > assertEquals(RESPONSE_BODY, results.get("body")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testSignedPostRequest() throws Exception { > @@ -263,6 +272,7 @@ > assertEquals(RESPONSE_BODY, results.get("body")); > assertFalse("A security token was returned when it was not requested.", > results.has("st")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testChangeSecurityToken() throws Exception { > @@ -281,6 +291,7 @@ > > assertEquals(RESPONSE_BODY, results.get("body")); > assertEquals("updated", results.getString("st")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testDoOAuthRequest() throws Exception { > @@ -301,6 +312,7 @@ > > assertEquals(HttpResponse.SC_OK, results.getInt("rc")); > assertEquals(RESPONSE_BODY, results.get("body")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testInvalidSigningTypeTreatedAsNone() throws Exception { > @@ -313,6 +325,7 @@ > > assertEquals(HttpResponse.SC_OK, results.getInt("rc")); > assertEquals(RESPONSE_BODY, results.get("body")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testBadHttpResponseIsPropagated() throws Exception { > @@ -324,6 +337,7 @@ > JSONObject results = extractJsonFromResponse(); > > assertEquals(HttpResponse.SC_INTERNAL_SERVER_ERROR, > results.getInt("rc")); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testBadSecurityTokenThrows() throws Exception { > @@ -355,5 +369,6 @@ > JSONObject results = extractJsonFromResponse(); > > assertEquals(RESPONSE_BODY, results.getString("foo")); > + assertTrue(rewriter.responseWasRewritten()); > } > } > > Modified: > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestServletTest.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestServletTest.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestServletTest.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestServletTest.java > Wed Sep 10 15:23:40 2008 > @@ -48,7 +48,7 @@ > = Collections.enumeration(Collections.<String>emptyList()); > > private final MakeRequestServlet servlet = new MakeRequestServlet(); > - private final MakeRequestHandler handler = new > MakeRequestHandler(contentFetcherFactory); > + private final MakeRequestHandler handler = new > MakeRequestHandler(contentFetcherFactory, null); > > private final HttpRequest internalRequest = new HttpRequest(REQUEST_URL); > private final HttpResponse internalResponse = new > HttpResponse(RESPONSE_BODY); > > Modified: > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java > Wed Sep 10 15:23:40 2008 > @@ -23,6 +23,7 @@ > import org.apache.shindig.gadgets.http.HttpRequest; > import org.apache.shindig.gadgets.http.HttpResponse; > import org.apache.shindig.gadgets.http.HttpResponseBuilder; > +import org.apache.shindig.gadgets.rewrite.BasicContentRewriterRegistry; > import static org.easymock.EasyMock.expect; > > import javax.servlet.http.HttpServletResponse; > @@ -36,7 +37,7 @@ > private final static String DATA_ONE = "hello world"; > > private final ProxyHandler proxyHandler > - = new ProxyHandler(fetcher, lockedDomainService, rewriter); > + = new ProxyHandler(fetcher, lockedDomainService, new > BasicContentRewriterRegistry(rewriter, null)); > > private void expectGetAndReturnData(String url, byte[] data) throws > Exception { > HttpRequest req = new HttpRequest(Uri.parse(url)); > @@ -69,6 +70,7 @@ > verify(); > > assertEquals(HttpServletResponse.SC_NOT_MODIFIED, > recorder.getHttpStatusCode()); > + assertFalse(rewriter.responseWasRewritten()); > } > > public void testLockedDomainEmbed() throws Exception { > @@ -81,6 +83,7 @@ > verify(); > > assertEquals(DATA_ONE, recorder.getResponseAsString()); > + assertTrue(rewriter.responseWasRewritten()); > } > > public void testLockedDomainFailedEmbed() throws Exception { > @@ -115,5 +118,6 @@ > > assertEquals(contentType, recorder.getHeader("Content-Type")); > assertEquals(magicGarbage, recorder.getHeader("X-Magic-Garbage")); > + assertTrue(rewriter.responseWasRewritten()); > } > } > > Modified: > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java > URL: > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java?rev=694035&r1=694034&r2=694035&view=diff > > ============================================================================== > --- > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java > (original) > +++ > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java > Wed Sep 10 15:23:40 2008 > @@ -44,7 +44,7 @@ > private static final String ERROR_MESSAGE = "Broken!"; > > private final ProxyHandler proxyHandler > - = new ProxyHandler(fetcher, lockedDomainService, rewriter); > + = new ProxyHandler(fetcher, lockedDomainService, null); > private final ProxyServlet servlet = new ProxyServlet(); > private final HttpRequest internalRequest = new HttpRequest(REQUEST_URL); > private final HttpResponse internalResponse = new > HttpResponse(RESPONSE_BODY); > > >
