Hey Kevin: I'm a little confused here... the AbstractHttpCache changes simply sub in ContentRewriterRegistry for the singleton ContentRewriter, and I added a test for AbstractHttpCache ensuring precisely what you're asking for. (?)
John On Mon, Aug 25, 2008 at 10:25 PM, Kevin Brown <[EMAIL PROTECTED]> wrote: > The changes to AbstractHttpCache bypass rewriting entirely, spending a lot > of effort to do nothing. You should add some tests to verify that rewriting > is actually performed if you're changing this code. > > On Mon, Aug 25, 2008 at 7:45 PM, <[EMAIL PROTECTED]> wrote: > > > Author: johnh > > Date: Mon Aug 25 19:45:55 2008 > > New Revision: 688943 > > > > URL: http://svn.apache.org/viewvc?rev=688943&view=rev > > Log: > > Modularization of ContentRewriter functionality. > > > > Rather than a single ContentRewriter being injected into the appropriate > > code paths, a ContentRewriterRegistry > > is injected instead, enabling a list of ContentRewriters rather than only > > one. > > > > By default, BasicContentRewriterRegistry is added, whose default > > constructor has a (pre-existing) ContentRewriter > > instance injected into it, maintaining exact rewriting and execution > > semantics. > > > > Rewriting code in HttpResponse and HttpRequest has been removed in favor > of > > method-injection of ContentRewriterRegistry > > into AbstractHttpCache, cleaning up code a bit. New test > > AbstractHttpCacheTest verifies proper rewriting behavior, > > including injection along the way. > > > > > > > > Added: > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistry.java > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterRegistry.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/BasicContentRewriterRegistryTest.java > > Modified: > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractMessageBundleFactory.java > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetServer.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/http/BasicHttpCache.java > > > > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpRequest.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/main/java/org/apache/shindig/gadgets/spec/MessageBundle.java > > > > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java > > > > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.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/HttpRequestTest.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 > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractMessageBundleFactory.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractMessageBundleFactory.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractMessageBundleFactory.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractMessageBundleFactory.java > > Mon Aug 25 19:45:55 2008 > > @@ -21,7 +21,6 @@ > > import org.apache.shindig.gadgets.spec.LocaleSpec; > > import org.apache.shindig.gadgets.spec.MessageBundle; > > > > -import java.util.List; > > import java.util.Locale; > > import java.net.URI; > > > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java > > Mon Aug 25 19:45:55 2008 > > @@ -20,19 +20,18 @@ > > > > import org.apache.shindig.common.cache.Cache; > > import org.apache.shindig.common.cache.CacheProvider; > > -import org.apache.shindig.common.cache.LruCache; > > import org.apache.shindig.common.uri.Uri; > > 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 org.apache.shindig.gadgets.spec.GadgetSpec; > > import org.apache.shindig.gadgets.spec.View; > > > > import com.google.inject.Inject; > > import com.google.inject.Singleton; > > import com.google.inject.name.Named; > > -import com.sun.jmx.remote.util.CacheMap; > > > > import java.net.URI; > > import java.util.ArrayList; > > @@ -53,7 +52,7 @@ > > private static final Logger logger = > > Logger.getLogger(BasicGadgetSpecFactory.class.getName()); > > > > private final HttpFetcher fetcher; > > - private final ContentRewriter rewriter; > > + private final ContentRewriterRegistry rewriterRegistry; > > private final Executor executor; > > private final long minTtl; > > private final long maxTtl; > > @@ -148,8 +147,12 @@ > > throw new > > GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT, > > "Unable to retrieve remote gadget > > content."); > > } > > - if (rewriter != null) { > > - v.setRewrittenContent(rewriter.rewriteGadgetView(spec, > > v.getContent(), "text/html")); > > + if (rewriterRegistry != null) { > > + String content = v.getContent(); > > + for (ContentRewriter rewriter : > rewriterRegistry.getRewriters()) > > { > > + content = rewriter.rewriteGadgetView(spec, content, > > "text/html"); > > + } > > + v.setRewrittenContent(content); > > } > > } > > } > > @@ -170,13 +173,13 @@ > > @Inject > > public BasicGadgetSpecFactory(HttpFetcher fetcher, > > CacheProvider cacheProvider, > > - ContentRewriter rewriter, > > + ContentRewriterRegistry > rewriterRegistry, > > Executor executor, > > > > @Named("shindig.gadget-spec.cache.capacity")int gadgetSpecCacheCapacity, > > > > @Named("shindig.gadget-spec.cache.minTTL")long minTtl, > > > > @Named("shindig.gadget-spec.cache.maxTTL")long maxTtl) { > > this.fetcher = fetcher; > > - this.rewriter = rewriter; > > + this.rewriterRegistry = rewriterRegistry; > > this.executor = executor; > > this.cache = cacheProvider.createCache(gadgetSpecCacheCapacity); > > this.minTtl = minTtl; > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetServer.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetServer.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetServer.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetServer.java > > Mon Aug 25 19:45:55 2008 > > @@ -25,7 +25,6 @@ > > import org.apache.shindig.gadgets.spec.Auth; > > import org.apache.shindig.gadgets.spec.Feature; > > import org.apache.shindig.gadgets.spec.GadgetSpec; > > -import org.apache.shindig.gadgets.spec.LocaleSpec; > > import org.apache.shindig.gadgets.spec.MessageBundle; > > import org.apache.shindig.gadgets.spec.Preload; > > > > > > 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=688943&r1=688942&r2=688943&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 > > Mon Aug 25 19:45:55 2008 > > @@ -17,6 +17,11 @@ > > */ > > package org.apache.shindig.gadgets.http; > > > > +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 > > @@ -24,6 +29,13 @@ > > */ > > public abstract class AbstractHttpCache implements HttpCache { > > > > + private ContentRewriterRegistry rewriterRegistry; > > + > > + @Inject > > + public void setRewriterRegistry(ContentRewriterRegistry registry) { > > + rewriterRegistry = registry; > > + } > > + > > public final HttpResponse getResponse(HttpCacheKey key, HttpRequest > > request) { > > if (key.isCacheable()) { > > String keyString = key.toString(); > > @@ -108,7 +120,7 @@ > > > > // Return the rewritten version if requested > > if (!request.getIgnoreCache() && > > - request.getContentRewriter() != null && > > + rewriterRegistry != null && > > response.getRewritten() != null && > > response.getRewritten().getContentLength() > 0) { > > return response.getRewritten(); > > @@ -122,8 +134,15 @@ > > */ > > protected HttpResponse rewrite(HttpRequest request, HttpResponse > > response) { > > // TODO - Make this sensitive to custom rewriting rules > > - if (response.getRewritten() == null && request.getContentRewriter() > != > > null) { > > - return request.getContentRewriter().rewrite(request, response); > > + if (response.getRewritten() == null && > > + rewriterRegistry != null) { > > + HttpResponse rewritten = response; > > + for (ContentRewriter rewriter : rewriterRegistry.getRewriters()) { > > + rewritten = rewriter.rewrite(request, rewritten); > > + } > > + if (response.getRewritten() != null) { > > + return response; > > + } > > } > > return null; > > } > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpCache.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpCache.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpCache.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/BasicHttpCache.java > > Mon Aug 25 19:45:55 2008 > > @@ -19,7 +19,6 @@ > > > > import org.apache.shindig.common.cache.Cache; > > import org.apache.shindig.common.cache.CacheProvider; > > -import org.apache.shindig.common.cache.LruCache; > > > > import com.google.inject.Inject; > > import com.google.inject.name.Named; > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpRequest.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpRequest.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpRequest.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/HttpRequest.java > > Mon Aug 25 19:45:55 2008 > > @@ -25,7 +25,6 @@ > > import org.apache.shindig.common.ContainerConfig; > > import org.apache.shindig.common.SecurityToken; > > import org.apache.shindig.common.uri.Uri; > > -import org.apache.shindig.gadgets.rewrite.ContentRewriter; > > > > import java.io.ByteArrayInputStream; > > import java.io.IOException; > > @@ -64,8 +63,6 @@ > > private boolean signOwner = true; > > private boolean signViewer = true; > > > > - // TODO: Remove this when new rewriting infrastructure is in place. > > - private ContentRewriter contentRewriter; > > private String rewriteMimeType; > > > > /** > > @@ -90,7 +87,6 @@ > > securityToken = request.securityToken; > > signOwner = request.signOwner; > > signViewer = request.signViewer; > > - contentRewriter = request.contentRewriter; > > rewriteMimeType = request.rewriteMimeType; > > } > > > > @@ -241,15 +237,6 @@ > > } > > > > /** > > - * @param contentRewriter The rewriter to use for the object retrieved > > by this request. > > - * TODO: Move this to new rewriting facility. > > - */ > > - public HttpRequest setContentRewriter(ContentRewriter contentRewriter) > { > > - this.contentRewriter = contentRewriter; > > - return this; > > - } > > - > > - /** > > * @param rewriteMimeType The assumed content type of the response to > be > > rewritten. Overrides > > * any values set in the Content-Type response header. > > * > > @@ -391,13 +378,6 @@ > > } > > > > /** > > - * @return The rewriter to be used on any response objects. > > - */ > > - public ContentRewriter getContentRewriter() { > > - return contentRewriter; > > - } > > - > > - /** > > * @return The content type to assume when rewriting. > > * > > * TODO: Move this to new rewriting facility. > > > > Added: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistry.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistry.java?rev=688943&view=auto > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistry.java > > (added) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistry.java > > Mon Aug 25 19:45:55 2008 > > @@ -0,0 +1,53 @@ > > +/* > > + * Licensed to the Apache Software Foundation (ASF) under one > > + * or more contributor license agreements. See the NOTICE file > > + * distributed with this work for additional information > > + * regarding copyright ownership. The ASF licenses this file > > + * to you under the Apache License, Version 2.0 (the > > + * "License"); you may not use this file except in compliance > > + * with the License. You may obtain a copy of the License at > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, > > + * software distributed under the License is distributed on an > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > > + * KIND, either express or implied. See the License for the > > + * specific language governing permissions and limitations under the > > License. > > + */ > > +package org.apache.shindig.gadgets.rewrite; > > + > > +import com.google.inject.Inject; > > + > > +import java.util.Collections; > > +import java.util.LinkedList; > > +import java.util.List; > > + > > +/** > > + * Registry into which is injected a single rewriter, which > > + * bootstraps the rewriters list. This enables modularization > > + * of [EMAIL PROTECTED] ContentRewriter} instances without changing > > + * Guice injection bindings. The class also provides a method > > + * for manipulating a simple list of rewriters. > > + */ > > +public class BasicContentRewriterRegistry implements > > ContentRewriterRegistry { > > + private final List<ContentRewriter> rewriters; > > + > > + @Inject > > + public BasicContentRewriterRegistry(ContentRewriter firstRewriter) { > > + rewriters = new LinkedList<ContentRewriter>(); > > + appendRewriter(firstRewriter); > > + } > > + > > + /** [EMAIL PROTECTED] */ > > + public List<ContentRewriter> getRewriters() { > > + return Collections.unmodifiableList(rewriters); > > + } > > + > > + public void appendRewriter(ContentRewriter rewriter) { > > + if (rewriter != null) { > > + rewriters.add(rewriter); > > + } > > + } > > + > > +} > > > > Added: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterRegistry.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterRegistry.java?rev=688943&view=auto > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterRegistry.java > > (added) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterRegistry.java > > Mon Aug 25 19:45:55 2008 > > @@ -0,0 +1,30 @@ > > +/* > > + * Licensed to the Apache Software Foundation (ASF) under one > > + * or more contributor license agreements. See the NOTICE file > > + * distributed with this work for additional information > > + * regarding copyright ownership. The ASF licenses this file > > + * to you under the Apache License, Version 2.0 (the > > + * "License"); you may not use this file except in compliance > > + * with the License. You may obtain a copy of the License at > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, > > + * software distributed under the License is distributed on an > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > > + * KIND, either express or implied. See the License for the > > + * specific language governing permissions and limitations under the > > License. > > + */ > > +package org.apache.shindig.gadgets.rewrite; > > + > > +import com.google.inject.ImplementedBy; > > + > > +import java.util.List; > > + > > [EMAIL PROTECTED](BasicContentRewriterRegistry.class) > > +public interface ContentRewriterRegistry { > > + /** > > + * @return An immutable list of all content rewriters > > + */ > > + public List<ContentRewriter> getRewriters(); > > +} > > > > 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=688943&r1=688942&r2=688943&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 > > Mon Aug 25 19:45:55 2008 > > @@ -30,7 +30,6 @@ > > 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.ContentRewriter; > > import org.apache.shindig.gadgets.spec.Auth; > > import org.apache.shindig.gadgets.spec.Preload; > > > > @@ -42,7 +41,6 @@ > > > > import java.io.IOException; > > import java.util.Collections; > > -import java.util.List; > > import java.util.Map; > > > > import javax.servlet.http.HttpServletRequest; > > @@ -71,15 +69,12 @@ > > > > private final SecurityTokenDecoder securityTokenDecoder; > > private final ContentFetcherFactory contentFetcherFactory; > > - private final ContentRewriter rewriter; > > > > @Inject > > public MakeRequestHandler(ContentFetcherFactory contentFetcherFactory, > > - SecurityTokenDecoder securityTokenDecoder, > > - ContentRewriter rewriter) { > > + SecurityTokenDecoder securityTokenDecoder) { > > this.contentFetcherFactory = contentFetcherFactory; > > this.securityTokenDecoder = securityTokenDecoder; > > - this.rewriter = rewriter; > > } > > > > /** > > @@ -128,9 +123,6 @@ > > } > > > > Uri url = validateUrl(request.getParameter(URL_PARAM)); > > - String method = request.getMethod(); > > - Map<String, List<String>> headers = null; > > - byte[] postBody = null; > > > > HttpRequest req = new HttpRequest(url) > > .setMethod(getParameter(request, METHOD_PARAM, "GET")) > > @@ -163,7 +155,6 @@ > > if (request.getParameter(GADGET_PARAM) != null) { > > req.setGadget(Uri.parse(request.getParameter(GADGET_PARAM))); > > } > > - req.setContentRewriter(rewriter); > > > > // Allow the rewriter to use an externally forced mime type. This is > > needed > > // allows proper rewriting of <script src="x"/> where x is returned > > with > > > > 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=688943&r1=688942&r2=688943&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 > > Mon Aug 25 19:45:55 2008 > > @@ -55,7 +55,6 @@ > > // This is a limitation of Guice, but this workaround...works. > > private final HttpFetcher fetcher; > > private final LockedDomainService lockedDomainService; > > - private final ContentRewriter rewriter; > > > > @Inject > > public ProxyHandler(HttpFetcher fetcher, > > @@ -63,7 +62,6 @@ > > ContentRewriter rewriter) { > > this.fetcher = fetcher; > > this.lockedDomainService = lockedDomainService; > > - this.rewriter = rewriter; > > } > > > > /** > > @@ -79,8 +77,6 @@ > > req.setGadget(Uri.parse(request.getParameter(GADGET_PARAM))); > > } > > > > - req.setContentRewriter(rewriter); > > - > > // Allow the rewriter to use an externally forced mime type. This is > > needed > > // allows proper rewriting of <script src="x"/> where x is returned > > with > > // a content type like text/html which unfortunately happens all too > > often > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/MessageBundle.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/MessageBundle.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/MessageBundle.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/MessageBundle.java > > Mon Aug 25 19:45:55 2008 > > @@ -23,7 +23,6 @@ > > import org.w3c.dom.Element; > > import org.w3c.dom.NodeList; > > > > -import java.net.URI; > > import java.util.Collections; > > import java.util.HashMap; > > import java.util.Map; > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java > > Mon Aug 25 19:45:55 2008 > > @@ -26,6 +26,8 @@ > > import org.apache.shindig.gadgets.http.HttpResponse; > > import org.apache.shindig.gadgets.http.HttpResponseBuilder; > > import org.apache.shindig.gadgets.rewrite.ContentRewriter; > > +import org.apache.shindig.gadgets.rewrite.ContentRewriterRegistry; > > +import org.apache.shindig.gadgets.rewrite.BasicContentRewriterRegistry; > > import org.apache.shindig.gadgets.spec.GadgetSpec; > > import org.easymock.EasyMock; > > import static org.easymock.EasyMock.expect; > > @@ -86,11 +88,13 @@ > > > > private final HttpFetcher fetcher = > > EasyMock.createNiceMock(HttpFetcher.class); > > private final CaptureRewriter rewriter = new CaptureRewriter(); > > + private final ContentRewriterRegistry rewriterRegistry = > > + new BasicContentRewriterRegistry(rewriter); > > > > private final CacheProvider cacheProvider = new DefaultCacheProvider(); > > > > private final BasicGadgetSpecFactory specFactory > > - = new BasicGadgetSpecFactory(fetcher, cacheProvider, rewriter, > > FAKE_EXECUTOR, 5, -1000, 1000); > > + = new BasicGadgetSpecFactory(fetcher, cacheProvider, > > rewriterRegistry, FAKE_EXECUTOR, 5, -1000, 1000); > > > > @Test > > public void specFetched() throws Exception { > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java > > Mon Aug 25 19:45:55 2008 > > @@ -20,13 +20,11 @@ > > > > import org.apache.shindig.common.cache.CacheProvider; > > import org.apache.shindig.common.cache.DefaultCacheProvider; > > -import org.apache.shindig.common.xml.XmlUtil; > > 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.http.HttpResponseBuilder; > > import org.apache.shindig.gadgets.spec.GadgetSpec; > > -import org.apache.shindig.gadgets.spec.LocaleSpec; > > import org.apache.shindig.gadgets.spec.MessageBundle; > > import org.easymock.EasyMock; > > import static org.easymock.EasyMock.expect; > > > > 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=688943&r1=688942&r2=688943&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 > > Mon Aug 25 19:45:55 2008 > > @@ -27,6 +27,7 @@ > > import org.apache.shindig.gadgets.http.ContentFetcherFactory; > > import org.apache.shindig.gadgets.http.HttpFetcher; > > import org.apache.shindig.gadgets.oauth.OAuthFetcher; > > +import org.apache.shindig.gadgets.rewrite.BasicContentRewriterRegistry; > > import org.apache.shindig.gadgets.rewrite.ContentRewriter; > > import org.apache.shindig.gadgets.rewrite.NoOpContentRewriter; > > import org.apache.shindig.gadgets.servlet.GadgetRenderingTask; > > @@ -72,7 +73,7 @@ > > } > > }; > > public final GadgetSpecFactory specFactory = new > BasicGadgetSpecFactory( > > - fetcher, cacheProvider, new NoOpContentRewriter(), executor, 0, > 0L, > > 0L); > > + fetcher, cacheProvider, new BasicContentRewriterRegistry(null), > > executor, 0, 0L, 0L); > > > > > > public GadgetTestFixture() { > > > > Added: > > > 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=688943&view=auto > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java > > (added) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/AbstractHttpCacheTest.java > > Mon Aug 25 19:45:55 2008 > > @@ -0,0 +1,106 @@ > > +/* > > + * Licensed to the Apache Software Foundation (ASF) under one > > + * or more contributor license agreements. See the NOTICE file > > + * distributed with this work for additional information > > + * regarding copyright ownership. The ASF licenses this file > > + * to you under the Apache License, Version 2.0 (the > > + * "License"); you may not use this file except in compliance > > + * with the License. You may obtain a copy of the License at > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, > > + * software distributed under the License is distributed on an > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > > + * KIND, either express or implied. See the License for the > > + * specific language governing permissions and limitations under the > > License. > > + */ > > +package org.apache.shindig.gadgets.http; > > + > > +import org.apache.shindig.gadgets.spec.GadgetSpec; > > +import org.apache.shindig.gadgets.rewrite.ContentRewriter; > > + > > +import static org.easymock.EasyMock.expect; > > +import static org.easymock.classextension.EasyMock.replay; > > +import org.easymock.classextension.EasyMock; > > + > > +import com.google.inject.AbstractModule; > > +import com.google.inject.Guice; > > +import com.google.inject.Injector; > > + > > +import junit.framework.TestCase; > > + > > +import java.util.HashMap; > > +import java.util.Map; > > + > > +/** > > + * Sanity test to ensure that Guice injection of a default > > + * ContentRewriter leads to its use by an AbstractHttpCache > > + * in properly rewriting cacheable content. > > + */ > > +public class AbstractHttpCacheTest extends TestCase { > > + private Injector injector; > > + > > + protected void setUp() throws Exception { > > + injector = Guice.createInjector(new TestCacheModule()); > > + } > > + > > + public void testCacheWithRewritingOps() { > > + // Setup: could move this elsewhere, but no real need right now. > > + HttpCacheKey key = EasyMock.createNiceMock(HttpCacheKey.class); > > + expect(key.isCacheable()).andReturn(true).anyTimes(); > > + HttpRequest request = EasyMock.createNiceMock(HttpRequest.class); > > + expect(request.getIgnoreCache()).andReturn(false).anyTimes(); > > + replay(key, request); > > + HttpResponse response = new > > HttpResponseBuilder().setHttpStatusCode(200) > > + > > > .setResponse("foo".getBytes()).setExpirationTime(Integer.MAX_VALUE).create(); > > + > > + // 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)); > > + assertSame(response, ahc.removeResponse(key)); > > + } > > + > > + private static class TestHttpCache extends AbstractHttpCache { > > + private final Map<String, HttpResponse> map; > > + > > + public TestHttpCache() { > > + super(); > > + map = new HashMap<String, HttpResponse>(); > > + } > > + > > + public void addResponseImpl(String key, HttpResponse response) { > > + map.put(key, response); > > + } > > + > > + public HttpResponse getResponseImpl(String key) { > > + return map.get(key); > > + } > > + > > + public HttpResponse removeResponseImpl(String key) { > > + return map.remove(key); > > + } > > + > > + } > > + > > + private static String PFX_STR = "--prefixtest--"; > > + private static class TestContentRewriter implements ContentRewriter { > > + public String rewriteGadgetView(GadgetSpec spec, String content, > > String mime) { > > + return PFX_STR + content; > > + } > > + > > + public HttpResponse rewrite(HttpRequest req, HttpResponse resp) { > > + return new > > HttpResponseBuilder().setHttpStatusCode(resp.getHttpStatusCode()) > > + .setResponse((PFX_STR + > > resp.getResponseAsString()).getBytes()).create(); > > + } > > + } > > + > > + private static class TestCacheModule extends AbstractModule { > > + protected void configure() { > > + bind(ContentRewriter.class).to(TestContentRewriter.class); > > + } > > + } > > +} > > > > Modified: > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpRequestTest.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpRequestTest.java?rev=688943&r1=688942&r2=688943&view=diff > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpRequestTest.java > > (original) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/HttpRequestTest.java > > Mon Aug 25 19:45:55 2008 > > @@ -90,7 +90,6 @@ > > .setGadget(DEFAULT_URI) > > .setMethod("POST") > > .setPostBody(POST_BODY.getBytes()) > > - .setContentRewriter(null) > > .setRewriteMimeType("text/fake") > > .setSecurityToken(AnonymousSecurityToken.getInstance()) > > .setSignOwner(false) > > @@ -104,7 +103,6 @@ > > assertEquals(request.getGadget(), request2.getGadget()); > > assertEquals(request.getMethod(), request2.getMethod()); > > assertEquals(request.getPostBodyAsString(), > > request2.getPostBodyAsString()); > > - assertEquals(request.getContentRewriter(), > > request2.getContentRewriter()); > > assertEquals(request.getRewriteMimeType(), > > request2.getRewriteMimeType()); > > assertEquals(request.getSecurityToken(), > request2.getSecurityToken()); > > assertEquals(request.getSignOwner(), request2.getSignOwner()); > > > > Added: > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistryTest.java > > URL: > > > http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistryTest.java?rev=688943&view=auto > > > > > ============================================================================== > > --- > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistryTest.java > > (added) > > +++ > > > incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BasicContentRewriterRegistryTest.java > > Mon Aug 25 19:45:55 2008 > > @@ -0,0 +1,50 @@ > > +/* > > + * Licensed to the Apache Software Foundation (ASF) under one > > + * or more contributor license agreements. See the NOTICE file > > + * distributed with this work for additional information > > + * regarding copyright ownership. The ASF licenses this file > > + * to you under the Apache License, Version 2.0 (the > > + * "License"); you may not use this file except in compliance > > + * with the License. You may obtain a copy of the License at > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, > > + * software distributed under the License is distributed on an > > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > > + * KIND, either express or implied. See the License for the > > + * specific language governing permissions and limitations under the > > License. > > + */ > > +package org.apache.shindig.gadgets.rewrite; > > + > > +import junit.framework.TestCase; > > + > > +public class BasicContentRewriterRegistryTest extends TestCase { > > + public void testNoArgsCreatedBasicRegistry() { > > + BasicContentRewriterRegistry r = new > > BasicContentRewriterRegistry(null); > > + assertNotNull(r.getRewriters()); > > + assertEquals(0, r.getRewriters().size()); > > + } > > + > > + public void testSingleValuedBasicRegistry() { > > + BasicContentRewriterRegistry r = new BasicContentRewriterRegistry( > > + new NoOpContentRewriter()); > > + assertNotNull(r.getRewriters()); > > + assertEquals(1, r.getRewriters().size()); > > + assertTrue(r.getRewriters().get(0) instanceof NoOpContentRewriter); > > + } > > + > > + public void testBasicContentRegistryWithAdds() { > > + ContentRewriter cr0 = new NoOpContentRewriter(); > > + BasicContentRewriterRegistry r = new > > BasicContentRewriterRegistry(cr0); > > + ContentRewriter cr1 = new NoOpContentRewriter(); > > + ContentRewriter cr2 = new NoOpContentRewriter(); > > + r.appendRewriter(cr1); > > + r.appendRewriter(cr2); > > + assertNotNull(r.getRewriters()); > > + assertEquals(3, r.getRewriters().size()); > > + assertSame(cr0, r.getRewriters().get(0)); > > + assertSame(cr1, r.getRewriters().get(1)); > > + assertSame(cr2, r.getRewriters().get(2)); > > + } > > +} > > > > 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=688943&r1=688942&r2=688943&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 > > Mon Aug 25 19:45:55 2008 > > @@ -52,7 +52,7 @@ > > private static final SecurityToken DUMMY_TOKEN = new FakeGadgetToken(); > > > > private final MakeRequestHandler handler = new > > MakeRequestHandler(contentFetcherFactory, > > - securityTokenDecoder, rewriter); > > + securityTokenDecoder); > > > > private void expectGetAndReturnBody(String response) throws Exception { > > expectGetAndReturnBody(fetcher, response); > > > > 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=688943&r1=688942&r2=688943&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 > > Mon Aug 25 19:45:55 2008 > > @@ -48,7 +48,7 @@ > > > > private final MakeRequestServlet servlet = new MakeRequestServlet(); > > private final MakeRequestHandler handler = new > > MakeRequestHandler(contentFetcherFactory, > > - securityTokenDecoder, rewriter); > > + securityTokenDecoder); > > > > private final HttpRequest internalRequest = new > HttpRequest(REQUEST_URL); > > private final HttpResponse internalResponse = new > > HttpResponse(RESPONSE_BODY); > > > > > > >

