On Wed, Sep 10, 2008 at 4:44 PM, John Hjelmstad <[EMAIL PROTECTED]> wrote:
> 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? Actually, I was looking at MakeRequestServlet instead of MakeRequestHandler and getting their responsibilities confused. The verification in MakeRequestHandler is fine. > 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); > > > > > > > > > > > >
