Beyond checking to ensure that rewriters are actually being run over the appropriate objects (Gadget, in GadgetServerTest, and HttpResponse, in MakeRequestHandlerTest and ProxyHandlerTest, each of which verify that rewriter.[object]WasRewritten(), or not, as appropriate), what do you have in mind? At present there's no additional selection/applicability logic in the ContentRewriter or ContentRewriterRegistry interface(s) -- every Rewriter just a shot at each object, and it's up to those Rewriter(s) to choose whether or not to do anything, so I'm not sure what else I can test aside from ensuring that rewriter objects are called in the first place.
John On Wed, Sep 10, 2008 at 4:37 PM, Kevin Brown <[EMAIL PROTECTED]> wrote: > 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); > > > > > > >
