You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by et...@apache.org on 2008/09/21 04:14:39 UTC

svn commit: r697440 - in /incubator/shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/ main/java/org/apache/shindig/gadgets/rewrite/ main/java/org/apache/shindig/gadgets/rewrite/lexer/ main/java/org/apache/shindig/gadgets/spec/ test/...

Author: etnu
Date: Sat Sep 20 19:14:38 2008
New Revision: 697440

URL: http://svn.apache.org/viewvc?rev=697440&view=rev
Log:
Cleaned up Default/Caching RewriterRegistry to actually support more than one rewriter; this is essential for supporting proxied content.

During this change I discovered many issues that need to be resolved before we can roll out our first release. There is a whole lot of dead code lying around, along with a bunch of critical integration points that are very difficult (or impossible) to actually integrate against.

Added:
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java
Modified:
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.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/rewrite/ContentRewriter.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterRegistry.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/lexer/DefaultContentRewriter.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.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/rewrite/CachingContentRewriterRegistryTest.java
    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistryTest.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/ProxyHandlerTest.java

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java?rev=697440&r1=697439&r2=697440&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java Sat Sep 20 19:14:38 2008
@@ -18,26 +18,34 @@
  */
 package org.apache.shindig.gadgets;
 
-import org.apache.commons.io.IOUtils;
 import org.apache.shindig.common.crypto.BasicBlobCrypter;
 import org.apache.shindig.common.crypto.BlobCrypter;
 import org.apache.shindig.common.crypto.Crypto;
 import org.apache.shindig.common.util.ResourceLoader;
 import org.apache.shindig.gadgets.http.HttpResponse;
+import org.apache.shindig.gadgets.oauth.BasicOAuthStore;
 import org.apache.shindig.gadgets.oauth.BasicOAuthStoreConsumerKeyAndSecret;
 import org.apache.shindig.gadgets.oauth.OAuthStore;
-import org.apache.shindig.gadgets.oauth.BasicOAuthStore;
 import org.apache.shindig.gadgets.oauth.BasicOAuthStoreConsumerKeyAndSecret.KeyType;
+import org.apache.shindig.gadgets.rewrite.ContentRewriter;
+import org.apache.shindig.gadgets.rewrite.lexer.DefaultContentRewriter;
 
+import com.google.common.collect.Lists;
 import com.google.inject.AbstractModule;
 import com.google.inject.CreationException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.TypeLiteral;
 import com.google.inject.name.Names;
 import com.google.inject.spi.Message;
 
+import org.apache.commons.io.IOUtils;
+
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Properties;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
@@ -52,26 +60,30 @@
 
   private static final Logger logger
       = Logger.getLogger(DefaultGuiceModule.class.getName());
-  
+
   private final Properties properties;
   private final String oauthConfig;
-  
+
   private final static String DEFAULT_PROPERTIES = "gadgets.properties";
   private final static String OAUTH_CONFIG = "config/oauth.json";
-  
+
   public final static String OAUTH_STATE_CRYPTER_ANNOTATION = "shindig.oauth.state-key";
   public final static String OAUTH_SIGNING_KEY_NAME = "shindig.signing.key-name";
   public final static String OAUTH_SIGNING_KEY_FILE = "shindig.signing.key-file";
 
   /** {@inheritDoc} */
   @Override
-  protected void configure() {    
+  protected void configure() {
     Names.bindProperties(this.binder(), properties);
 
     ExecutorService service = Executors.newCachedThreadPool();
     bind(Executor.class).toInstance(service);
     bind(ExecutorService.class).toInstance(service);
-    
+
+    bind(new TypeLiteral<List<ContentRewriter>>(){}).toProvider(ContentRewritersProvider.class);
+
+    // TODO: This is not the proper way to use a Guice module. It needs to be fixed before we can
+    // do a release.
     try {
       configureOAuthStore();
     } catch (Throwable t) {
@@ -86,22 +98,25 @@
       // because we can't initialize the OAuth config.
       logger.log(Level.WARNING, "Failed to initialize Crypter", t);
     }
-    
+
     // We perform static injection on HttpResponse for cache TTLs.
     requestStaticInjection(HttpResponse.class);
   }
-  
+
   /**
    * Create a store for OAuth consumer keys and access tokens.  By default consumer keys are read
    * from config/oauth.json, and access tokens are stored in memory.
-   * 
+   *
    * We read the default key from disk, in a location specified in our properties file.
+   *
+   * TODO: This doesn't belong here! It can't be reused by anyone who wants to customize shindig,
+   * which *FORCES* everyone to re-implement it.
    */
   private void configureOAuthStore() throws GadgetException {
     BasicOAuthStore store = new BasicOAuthStore();
     bind(OAuthStore.class).toInstance(store);
     store.initFromConfigString(oauthConfig);
-    
+
     String keyName = properties.getProperty(OAUTH_SIGNING_KEY_NAME);
     String keyFile = properties.getProperty(OAUTH_SIGNING_KEY_FILE);
     BasicOAuthStoreConsumerKeyAndSecret defaultKey = null;
@@ -129,7 +144,7 @@
       		OAUTH_SIGNING_KEY_NAME + "=mykey\n");
     }
   }
-  
+
   /**
    * Create a crypter to handle OAuth state.  This can be loaded from disk, if
    * shindig.oauth.state-key-file is specified in your gadgets.properties file, or it can be
@@ -151,7 +166,7 @@
       logger.info("Creating OAuth state crypter with random key");
       oauthCrypter = new BasicBlobCrypter(
           Crypto.getRandomBytes(BasicBlobCrypter.MASTER_KEY_MIN_LEN));
-    }        
+    }
     bind(BlobCrypter.class).annotatedWith(
         Names.named(OAUTH_STATE_CRYPTER_ANNOTATION))
         .toInstance(oauthCrypter);
@@ -184,4 +199,17 @@
     this.properties = properties;
     this.oauthConfig = oauthConfig;
   }
+
+  private static class ContentRewritersProvider implements Provider<List<ContentRewriter>> {
+    private final List<ContentRewriter> rewriters;
+
+    @Inject
+    public ContentRewritersProvider(DefaultContentRewriter optimizingRewriter) {
+      rewriters = Lists.<ContentRewriter>newArrayList(optimizingRewriter);
+    }
+
+    public List<ContentRewriter> get() {
+      return rewriters;
+    }
+  }
 }

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=697440&r1=697439&r2=697440&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 Sat Sep 20 19:14:38 2008
@@ -17,8 +17,8 @@
  */
 package org.apache.shindig.gadgets.rewrite;
 
+import org.apache.shindig.common.cache.Cache;
 import org.apache.shindig.common.cache.CacheProvider;
-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;
@@ -26,9 +26,6 @@
 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.DefaultContentRewriterRegistry;
-import org.apache.shindig.gadgets.rewrite.ContentRewriter;
-import org.apache.shindig.gadgets.spec.GadgetSpec;
 
 import com.google.inject.Inject;
 import com.google.inject.name.Named;
@@ -36,44 +33,43 @@
 import java.util.List;
 
 /**
- * Standard implementation of a content rewriter with caching logic layered atop it.
- * Uses {@code BasicContentRewriterRegistry} for actual rewriting, and a
- * {@code TtlCache}, whose underlying persistence is provided by {@code CacheProvider},
- * as the cache.
+ * Enhances {@code DefaultContentRewriterRegistry} by adding a caching layer.
+ *
+ * Entries are cached using a hash of their contents and all registered rewriters. This ensures
+ * that changes to either the content or the rewriters will invalidate the cache, as well as
+ * optimizing cache size by ensuring that duplicate inputs only have to be rewritten once.
  */
 public class CachingContentRewriterRegistry extends DefaultContentRewriterRegistry {
-  
-  private final TtlCache<String, String> rewrittenCache;
+
+  private final Cache<String, String> rewrittenCache;
   private String rewritersKey;
-  
+
   @Inject
-  public CachingContentRewriterRegistry(ContentRewriter firstRewriter,
+  public CachingContentRewriterRegistry(List<? extends ContentRewriter> rewriters,
       GadgetHtmlParser htmlParser,
       CacheProvider cacheProvider,
-      @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);
+      @Named("shindig.rewritten-content.cache.capacity")int capacity) {
+    super(rewriters, htmlParser);
+
+    rewrittenCache = cacheProvider.createCache(capacity);
   }
 
   protected String getGadgetCacheKey(Gadget gadget) {
-    return getRewritersKey() + ":" + HashUtil.checksum(gadget.getContent().getBytes());
+    return getRewritersKey() + ':' + HashUtil.checksum(gadget.getContent().getBytes());
   }
-  
+
   protected String getHttpResponseCacheKey(HttpRequest req, HttpResponse response) {
-    return getRewritersKey() + ":" + req.getUri().toString() + ":" + 
+    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());
+            .append('-').append(rewriter.getClass().hashCode());
       }
       rewritersKey = keyBuilder.toString();
     }
@@ -82,68 +78,49 @@
 
   /** {@inheritDoc} */
   @Override
-  public boolean rewriteGadget(Gadget gadget)
-      throws GadgetException {
+  public boolean rewriteGadget(Gadget gadget) throws GadgetException {
     if (gadget.getContext().getIgnoreCache()) {
       return super.rewriteGadget(gadget);
     }
-    
+
     String cacheKey = getGadgetCacheKey(gadget);
     String cached = rewrittenCache.getElement(cacheKey);
-    
+
     if (cached != null) {
       gadget.setContent(cached);
       return true;
     }
-    
+
     // 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);
+      rewrittenCache.addElement(cacheKey, gadget.getContent());
     }
 
     return rewritten;
   }
-  
+
   /** {@inheritDoc} */
   @Override
   public HttpResponse rewriteHttpResponse(HttpRequest req, HttpResponse resp) {
-    if (req.getIgnoreCache()) {
+    if (req.getIgnoreCache() || resp.getCacheTtl() <= 0) {
       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);
+      rewrittenCache.addElement(cacheKey, rewritten.getResponseAsString());
     }
-    
+
     return rewritten;
   }
 }

Added: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java?rev=697440&view=auto
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java (added)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CaptureRewriter.java Sat Sep 20 19:14:38 2008
@@ -0,0 +1,51 @@
+/*
+ * 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 org.apache.shindig.gadgets.Gadget;
+import org.apache.shindig.gadgets.MutableContent;
+import org.apache.shindig.gadgets.http.HttpRequest;
+import org.apache.shindig.gadgets.http.HttpResponse;
+
+/**
+ * Utility rewriter for testing.
+ */
+public class CaptureRewriter implements ContentRewriter {
+  private boolean rewroteView = false;
+  private boolean rewroteResponse = false;
+
+  public RewriterResults rewrite(HttpRequest request, HttpResponse original,
+      MutableContent content) {
+    rewroteResponse = true;
+    return null;
+  }
+
+  public boolean responseWasRewritten() {
+    return rewroteResponse;
+  }
+
+  public RewriterResults rewrite(Gadget gadget) {
+    rewroteView = true;
+    return null;
+  }
+
+  public boolean viewWasRewritten() {
+    return rewroteView;
+  }
+}
\ No newline at end of file

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriter.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriter.java?rev=697440&r1=697439&r2=697440&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriter.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriter.java Sat Sep 20 19:14:38 2008
@@ -21,35 +21,28 @@
 import org.apache.shindig.gadgets.MutableContent;
 import org.apache.shindig.gadgets.http.HttpRequest;
 import org.apache.shindig.gadgets.http.HttpResponse;
-import org.apache.shindig.gadgets.rewrite.lexer.DefaultContentRewriter;
-
-import com.google.inject.ImplementedBy;
 
 /**
  * Standard interface for content rewriters
  */
-
-@ImplementedBy(DefaultContentRewriter.class)
-
 public interface ContentRewriter {
 
   /**
    * Rewrite the original content located at source.
-   * 
+   *
    * @param request Originating request, as context.
    * @param original Original HTTP response, for context.
    * @param content Original content.
    * @return Object indicating results cacheability, or null (indicates not cacheable).
    */
-  public RewriterResults rewrite(HttpRequest request,
-      HttpResponse original, MutableContent content);
+  RewriterResults rewrite(HttpRequest request, HttpResponse original, MutableContent content);
 
   /**
    * Rewrite the gadget. The Gadget object's manipulation methods are used
    * for the bulk of this.
-   * 
+   *
    * @param gadget Gadget to rewrite.
    * @return Object indicating results cacheability, or null (indicates not cacheable).
    */
-  public RewriterResults rewrite(Gadget gadget);
+  RewriterResults rewrite(Gadget gadget);
 }

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java?rev=697440&r1=697439&r2=697440&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterFeature.java Sat Sep 20 19:14:38 2008
@@ -29,7 +29,12 @@
 
 /**
  * Parser for the "content-rewrite" feature. The supported params are
- * include-urls,exclude-urls,include-tags. Default values are container specific
+ * include-urls,exclude-urls,include-tags. Default values are container specific.
+ *
+ * TODO: This really needs to be fixed, because it makes GadgetSpec mutable. It is *ONLY* needed
+ * by code in the rewrite package, and that code isn't even being used, and can't be used the way
+ * that they are currently written -- they require values from the gadget during construction, which
+ * are, of course, unavailable.
  */
 public class ContentRewriterFeature {
 
@@ -190,13 +195,13 @@
     }
     return fingerprint;
   }
-  
+
   public static class Factory {
     private final String defaultIncludeUrls;
     private final String defaultExcludeUrls;
     private final String defaultExpires;
     private final Set<String> defaultIncludeTags;
-    
+
     public Factory(String includeUrls, String excludeUrls, String expires,
         Set<String> includeTags) {
       defaultIncludeUrls = includeUrls;
@@ -204,7 +209,7 @@
       defaultExpires = expires;
       defaultIncludeTags = includeTags;
     }
-    
+
     public ContentRewriterFeature get(GadgetSpec spec) {
       ContentRewriterFeature rewriterFeature =
         (ContentRewriterFeature)spec.getAttribute("content-rewrite");

Modified: 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=697440&r1=697439&r2=697440&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterRegistry.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ContentRewriterRegistry.java Sat Sep 20 19:14:38 2008
@@ -17,13 +17,16 @@
  */
 package org.apache.shindig.gadgets.rewrite;
 
-import com.google.inject.ImplementedBy;
-
 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 com.google.inject.ImplementedBy;
+
+/**
+ * Performs rewriting operations by invoking one or more ContentRewriters.
+ */
 @ImplementedBy(DefaultContentRewriterRegistry.class)
 public interface ContentRewriterRegistry {
   /**
@@ -32,8 +35,8 @@
    * @return True if rewriting occurred
    * @throws GadgetException Potentially passed through from rewriters
    */
-  public boolean rewriteGadget(Gadget gadget) throws GadgetException;
-  
+  boolean rewriteGadget(Gadget gadget) throws GadgetException;
+
   /**
    * Rewrites an {@code HttpResponse} object with the given request as context,
    * using the registered rewriters.
@@ -41,5 +44,5 @@
    * @param resp Original response object.
    * @return Rewritten response object, or resp if not modified.
    */
-  public HttpResponse rewriteHttpResponse(HttpRequest req, HttpResponse resp);
+  HttpResponse rewriteHttpResponse(HttpRequest req, HttpResponse resp);
 }

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java?rev=697440&r1=697439&r2=697440&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistry.java Sat Sep 20 19:14:38 2008
@@ -17,12 +17,6 @@
  */
 package org.apache.shindig.gadgets.rewrite;
 
-import com.google.inject.Inject;
-
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
-
 import org.apache.shindig.gadgets.Gadget;
 import org.apache.shindig.gadgets.GadgetException;
 import org.apache.shindig.gadgets.MutableContent;
@@ -31,68 +25,62 @@
 import org.apache.shindig.gadgets.http.HttpResponseBuilder;
 import org.apache.shindig.gadgets.parse.GadgetHtmlParser;
 
+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 {@code ContentRewriter} instances without changing
- * Guice injection bindings. The class also provides a method
- * for manipulating a simple list of rewriters. It does not
- * support caching of rewritten contents in any way.
+ * Basic registry -- just iterates over rewriters and invokes them sequentially.
+ *
+ * TODO: Make abstract and bind CachingContentRewriterRegistry as the default.
  */
 public class DefaultContentRewriterRegistry implements ContentRewriterRegistry {
-  private final List<ContentRewriter> rewriters;
-  private final GadgetHtmlParser htmlParser;
-  
+  protected final List<ContentRewriter> rewriters;
+  protected final GadgetHtmlParser htmlParser;
+
   @Inject
-  public DefaultContentRewriterRegistry(ContentRewriter firstRewriter,
+  public DefaultContentRewriterRegistry(List<? extends ContentRewriter> rewriters,
       GadgetHtmlParser htmlParser) {
-    this.rewriters = new LinkedList<ContentRewriter>();
-    this.htmlParser = htmlParser;
-    appendRewriter(firstRewriter);
-  }
-  
-  /** {@inheritDoc} */
-  public List<ContentRewriter> getRewriters() {
-    return Collections.unmodifiableList(rewriters);
-  }
-  
-  public void appendRewriter(ContentRewriter rewriter) {
-    if (rewriter != null) {
-      rewriters.add(rewriter);
+    if (rewriters == null) {
+      rewriters = Collections.emptyList();
     }
+    this.rewriters = new LinkedList<ContentRewriter>(rewriters);
+    this.htmlParser = htmlParser;
   }
-  
+
   /** {@inheritDoc} */
   public boolean rewriteGadget(Gadget gadget) throws GadgetException {
     String originalContent = gadget.getContent();
-    
+
     if (originalContent == null) {
       // Nothing to rewrite.
       return false;
     }
 
-    for (ContentRewriter rewriter : getRewriters()) {
+    for (ContentRewriter rewriter : rewriters) {
       rewriter.rewrite(gadget);
     }
-    
+
     return !originalContent.equals(gadget.getContent());
   }
-  
+
   /** {@inheritDoc} */
   public HttpResponse rewriteHttpResponse(HttpRequest req, HttpResponse resp) {
     MutableContent mc = new MutableContent(htmlParser);
     String originalContent = resp.getResponseAsString();
     mc.setContent(originalContent);
-    
-    for (ContentRewriter rewriter : getRewriters()) {
+
+    for (ContentRewriter rewriter : rewriters) {
       rewriter.rewrite(req, resp, mc);
     }
-    
+
     String rewrittenContent = mc.getContent();
     if (rewrittenContent.equals(originalContent)) {
       return resp;
     }
-    
+
     return new HttpResponseBuilder(resp).setResponseString(rewrittenContent).create();
   }
 

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/lexer/DefaultContentRewriter.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/lexer/DefaultContentRewriter.java?rev=697440&r1=697439&r2=697440&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/lexer/DefaultContentRewriter.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/lexer/DefaultContentRewriter.java Sat Sep 20 19:14:38 2008
@@ -17,10 +17,6 @@
  */
 package org.apache.shindig.gadgets.rewrite.lexer;
 
-import com.google.inject.Inject;
-import com.google.inject.Singleton;
-import com.google.inject.name.Named;
-
 import org.apache.shindig.gadgets.Gadget;
 import org.apache.shindig.gadgets.GadgetException;
 import org.apache.shindig.gadgets.GadgetSpecFactory;
@@ -35,6 +31,10 @@
 import org.apache.shindig.gadgets.rewrite.RewriterResults;
 import org.apache.shindig.gadgets.spec.GadgetSpec;
 
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.google.inject.name.Named;
+
 import java.io.ByteArrayOutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Reader;
@@ -106,7 +106,7 @@
     } catch (GadgetException ge) {
       // Couldn't retrieve gadgetSpec
     }
-    
+
     return RewriterResults.cacheableIndefinitely();
   }
 
@@ -125,14 +125,8 @@
       return false;
     }
 
-    // Store the feature in the spec so we dont keep parsing it
-    ContentRewriterFeature rewriterFeature = (ContentRewriterFeature) spec
-        .getAttribute("content-rewrite");
-    if (rewriterFeature == null) {
-      rewriterFeature = new ContentRewriterFeature(spec, includeUrls, excludeUrls, expires,
-          includeTags);
-      spec.setAttribute("content-rewrite", rewriterFeature);
-    }
+    ContentRewriterFeature rewriterFeature
+        = new ContentRewriterFeature(spec, includeUrls, excludeUrls, expires, includeTags);
 
     if (!rewriterFeature.isRewriteEnabled()) {
       return false;

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.java?rev=697440&r1=697439&r2=697440&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/GadgetSpec.java Sat Sep 20 19:14:38 2008
@@ -21,6 +21,7 @@
 import org.apache.shindig.common.xml.XmlException;
 import org.apache.shindig.common.xml.XmlUtil;
 import org.apache.shindig.gadgets.Substitutions;
+
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
@@ -41,7 +42,6 @@
 public class GadgetSpec {
   public static final String DEFAULT_VIEW = "default";
   public static final Locale DEFAULT_LOCALE = new Locale("all", "ALL");
-  public static final String EXPIRATION_ATTRIB = "expiration";
 
   /**
    * The url for this gadget spec.
@@ -103,7 +103,7 @@
   public Object getAttribute(String key) {
     return attributes.get(key);
   }
-  
+
   public void setAttribute(String key, Object o) {
     attributes.put(key, o);
   }

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=697440&r1=697439&r2=697440&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 Sat Sep 20 19:14:38 2008
@@ -26,65 +26,61 @@
 import org.apache.shindig.common.util.FakeTimeSource;
 import org.apache.shindig.gadgets.http.ContentFetcherFactory;
 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.oauth.OAuthFetcher;
-import org.apache.shindig.gadgets.rewrite.DefaultContentRewriterRegistry;
+import org.apache.shindig.gadgets.rewrite.CaptureRewriter;
 import org.apache.shindig.gadgets.rewrite.ContentRewriter;
-import org.apache.shindig.gadgets.rewrite.RewriterResults;
+import org.apache.shindig.gadgets.rewrite.ContentRewriterRegistry;
+import org.apache.shindig.gadgets.rewrite.DefaultContentRewriterRegistry;
 
+import java.util.Arrays;
 import java.util.concurrent.ExecutorService;
 
+// DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
 public abstract class GadgetTestFixture extends EasyMockTestCase {
   // TODO: Remove all of these.
+
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final GadgetServer gadgetServer;
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final ContentFetcherFactory fetcherFactory = mock(ContentFetcherFactory.class);
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final HttpFetcher fetcher = mock(HttpFetcher.class);
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final OAuthFetcher oauthFetcher = mock(OAuthFetcher.class);
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final GadgetBlacklist blacklist = mock(GadgetBlacklist.class);
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   private final CacheProvider cacheProvider = new DefaultCacheProvider();
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final MessageBundleFactory bundleFactory =
       new BasicMessageBundleFactory(fetcher, cacheProvider, 0, 0L, 0L);
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final GadgetFeatureRegistry registry;
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final ContainerConfig containerConfig = mock(ContainerConfig.class);
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final CaptureRewriter rewriter = new CaptureRewriter();
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
+  public final ContentRewriterRegistry rewriterRegistry
+      = new DefaultContentRewriterRegistry(Arrays.<ContentRewriter>asList(rewriter), null);
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final FakeTimeSource timeSource = new FakeTimeSource();
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final ExecutorService executor = new TestExecutorService();
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public final GadgetSpecFactory specFactory = new BasicGadgetSpecFactory(
       fetcher, cacheProvider, executor, 0, 0L, 0L);
-
+  // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
   public GadgetTestFixture() {
     try {
+      // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
       registry = new GadgetFeatureRegistry(null, fetcher);
+      // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
       gadgetServer = new GadgetServer(executor, registry, blacklist,
-          containerConfig, new DefaultContentRewriterRegistry(rewriter, null),
-          null, fetcherFactory, specFactory, bundleFactory);
+          containerConfig, rewriterRegistry, null, fetcherFactory, specFactory, bundleFactory);
+      // DO NOT ADD ANYTHING ELSE TO THIS CLASS. IT IS GOING AWAY SOON!!!
     } catch (Exception e) {
       throw new RuntimeException(e);
     }
   }
-
-  public static class CaptureRewriter implements ContentRewriter {
-    private boolean rewroteView = false;
-    private boolean rewroteResponse = false;
-
-    public RewriterResults rewrite(HttpRequest request, HttpResponse original,
-        MutableContent content) {
-      rewroteResponse = true;
-      return null;
-    }
-
-    public boolean responseWasRewritten() {
-      return rewroteResponse;
-    }
-
-    public RewriterResults rewrite(Gadget gadget) {
-      rewroteView = true;
-      return null;
-    }
-
-    public boolean viewWasRewritten() {
-      return rewroteView;
-    }
-  }
 }

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=697440&r1=697439&r2=697440&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 Sat Sep 20 19:14:38 2008
@@ -17,80 +17,228 @@
  */
 package org.apache.shindig.gadgets.rewrite;
 
-import static org.easymock.EasyMock.expect;
-import static org.easymock.classextension.EasyMock.replay;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
-import org.apache.shindig.common.cache.DefaultCacheProvider;
+import org.apache.shindig.common.ContainerConfig;
+import org.apache.shindig.common.cache.Cache;
+import org.apache.shindig.common.cache.CacheProvider;
 import org.apache.shindig.common.uri.Uri;
 import org.apache.shindig.gadgets.Gadget;
 import org.apache.shindig.gadgets.GadgetContext;
+import org.apache.shindig.gadgets.MutableContent;
 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;
 
-import junit.framework.TestCase;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
+import edu.emory.mathcs.backport.java.util.Collections;
+
+import org.easymock.classextension.EasyMock;
+import org.easymock.classextension.IMocksControl;
+import org.junit.Test;
 
 import java.net.URI;
+import java.util.List;
+import java.util.Map;
+
+public class CachingContentRewriterRegistryTest {
+  private final List<CaptureRewriter> rewriters
+      = Lists.newArrayList(new CaptureRewriter(), new ModifyingCaptureContentRewriter());
+  private final FakeCacheProvider provider = new FakeCacheProvider();
+  private final ContentRewriterRegistry registry
+      = new CachingContentRewriterRegistry(rewriters, null, provider, 100);
+  private final IMocksControl control = EasyMock.createNiceControl();
+  private final ContainerConfig config = control.createMock(ContainerConfig.class);
+
+  @Test
+  @SuppressWarnings("unchecked")
+  public void gadgetGetsCached() throws Exception {
+    String body = "Hello, world";
+    String xml = "<Module><ModulePrefs title=''/><Content>" + body + "</Content></Module>";
+    GadgetSpec spec = new GadgetSpec(URI.create("#"), xml);
+    GadgetContext context = new GadgetContext();
+    Gadget gadget = new Gadget(context, spec, Collections.emptyList(), config, null);
+
+    control.replay();
+
+    registry.rewriteGadget(gadget);
+
+    // TODO: We're not actually testing the TTL of the entries here.
+    assertEquals(1, provider.readCount);
+    assertEquals(1, provider.writeCount);
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  public void gadgetFetchedFromCache() throws Exception {
+    String body = "Hello, world";
+    String xml = "<Module><ModulePrefs title=''/><Content>" + body + "</Content></Module>";
+    GadgetSpec spec = new GadgetSpec(URI.create("#"), xml);
+
+    GadgetContext context = new GadgetContext();
+
+    control.replay();
+
+    // We have to re-create Gadget objects because they get mutated directly, which is really
+    // inconsistent with the behavior of rewriteHttpResponse.
+    Gadget gadget = new Gadget(context, spec, Collections.emptyList(), config, null);
+    registry.rewriteGadget(gadget);
+    gadget = new Gadget(context, spec, Collections.emptyList(), config, null);
+    registry.rewriteGadget(gadget);
+    gadget = new Gadget(context, spec, Collections.emptyList(), config, null);
+    registry.rewriteGadget(gadget);
+
+    // TODO: We're not actually testing the TTL of the entries here.
+    assertEquals(3, provider.readCount);
+    assertEquals(1, provider.writeCount);
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  public void noCacheGadgetDoesNotGetCached() throws Exception {
+    String body = "Hello, world";
+    String xml = "<Module><ModulePrefs title=''/><Content>" + body + "</Content></Module>";
+    GadgetSpec spec = new GadgetSpec(URI.create("#"), xml);
+    GadgetContext context = new GadgetContext() {
+      @Override
+      public boolean getIgnoreCache() {
+        return true;
+      }
+    };
+
+    Gadget gadget = new Gadget(context, spec, Collections.emptyList(), config, null);
+
+    control.replay();
+
+    registry.rewriteGadget(gadget);
+
+    assertTrue("Rewriting not performed on uncacheable content.",
+        rewriters.get(0).viewWasRewritten());
+    assertEquals(0, provider.readCount);
+    assertEquals(0, provider.writeCount);
+  }
+
+  @Test
+  public void httpResponseGetsCached() throws Exception {
+    String body = "Hello, world";
+    HttpRequest request = new HttpRequest(Uri.parse("#"));
+    HttpResponse response = new HttpResponse(body);
+
+    registry.rewriteHttpResponse(request, response);
+
+    assertEquals(1, provider.readCount);
+    assertEquals(1, provider.writeCount);
+  }
+
+  @Test
+  public void httpResponseFetchedFromCache() throws Exception {
+    String body = "Hello, world";
+    HttpRequest request = new HttpRequest(Uri.parse("#"));
+    HttpResponse response = new HttpResponse(body);
+
+    registry.rewriteHttpResponse(request, response);
+    registry.rewriteHttpResponse(request, response);
+    registry.rewriteHttpResponse(request, response);
+
+    assertEquals(3, provider.readCount);
+    assertEquals(1, provider.writeCount);
+  }
+
+  @Test
+  public void noCacheHttpResponseDoesNotGetCached() throws Exception {
+    String body = "Hello, world";
+    HttpRequest request = new HttpRequest(Uri.parse("#")).setIgnoreCache(true);
+    HttpResponse response = new HttpResponse(body);
+
+    registry.rewriteHttpResponse(request, response);
+
+    assertTrue("Rewriting not performed on uncacheable content.",
+        rewriters.get(0).responseWasRewritten());
+    assertEquals(0, provider.readCount);
+    assertEquals(0, provider.writeCount);
+  }
+
+  @Test
+  public void changingRewritersBustsCache() {
+    // What we're testing here is actually impossible (you can't swap the registry on a running
+    // application), but there's a good chance that implementations will be using a shared cache,
+    // which persists across server restarts / reconfigurations. This verifies that the entries in
+    // the cache will be invalidated if the rewriters change.
+
+    // Just HTTP here; we'll assume this code is common between both methods.
+    String body = "Hello, world";
+    HttpRequest request = new HttpRequest(Uri.parse("#"));
+    HttpResponse response = new HttpResponse(body);
+
+    registry.rewriteHttpResponse(request, response);
+
+    assertEquals(1, provider.readCount);
+    assertEquals(1, provider.writeCount);
+
+    // The new registry is created using one additional rewriter, but the same cache.
+    rewriters.add(new CaptureRewriter());
+    ContentRewriterRegistry newRegistry
+        = new CachingContentRewriterRegistry(rewriters, null, provider, 100);
+
+    newRegistry.rewriteHttpResponse(request, response);
+
+    assertEquals(2, provider.readCount);
+    assertEquals(2, provider.writeCount);
+    assertFalse("Cache was written using identical keys.",
+        provider.keys.get(0).equals(provider.keys.get(1)));
+  }
+
+  private static class FakeCacheProvider implements CacheProvider {
+    private final Map<String, Object> entries = Maps.newHashMap();
+    private final List<String> keys = Lists.newLinkedList();
+    private int readCount = 0;
+    private int writeCount = 0;
+    private final Cache<String, Object> cache = new Cache<String, Object>() {
+      public void addElement(String key, Object value) {
+        entries.put(key, value);
+        keys.add(key);
+        writeCount++;
+      }
+
+      public Object getElement(String key) {
+        readCount++;
+        return entries.get(key);
+      }
+
+      public Object removeElement(String key) {
+        return entries.remove(key);
+      }
+    };
+
+    public <K, V> Cache<K, V> createCache(int capacity, String name) {
+      throw new UnsupportedOperationException();
+    }
+
+    @SuppressWarnings("unchecked")
+    public <K, V> Cache<K, V> createCache(int capacity) {
+      return (Cache<K, V>)cache;
+    }
+  }
+
+  private static class ModifyingCaptureContentRewriter extends CaptureRewriter {
+
+    @Override
+    public RewriterResults rewrite(HttpRequest request, HttpResponse original,
+        MutableContent content) {
+      super.rewrite(request, original, content);
+      content.setContent(content.getContent() + "-modified");
+      return RewriterResults.cacheableIndefinitely();
+    }
 
-public class CachingContentRewriterRegistryTest extends TestCase {
-  public void testCachedRewriteIsReturned() throws Exception {
-    // Sort of a weird test, but gets the basic idea of caching across.
-    // Perform a rewrite, then update the rewriter in such a way that
-    // would yield a different rewritten result, but ensure that
-    // the result of the rewriter is the old version, which hasn't
-    // yet expired. To ensure no expiry, set expiration date
-    // to the largest possible date.
-    CachingContentRewriterRegistry r = new CachingContentRewriterRegistry(null,
-        null, new DefaultCacheProvider(), 100, 0, Integer.MAX_VALUE);
-    StringBuilder appendFull = new StringBuilder();
-    for (int i = 0; i < 3; ++i) {
-      String appendNew = "-" + i;
-      appendFull.append(appendNew);
-      r.appendRewriter(new AppendingRewriter(appendNew));
+    @Override
+    public RewriterResults rewrite(Gadget gadget) {
+      super.rewrite(gadget);
+      gadget.setContent(gadget.getContent() + "-modified");
+      return RewriterResults.cacheableIndefinitely();
     }
-    String inputContent = "foo";
-    String rewrittenContent = inputContent + appendFull.toString();
-    
-    GadgetSpec spec = EasyMock.createNiceMock(GadgetSpec.class);
-    View view = EasyMock.createNiceMock(View.class);
-    expect(view.getName()).andReturn(GadgetSpec.DEFAULT_VIEW).anyTimes();
-    expect(view.getType()).andReturn(View.ContentType.HTML).anyTimes();
-    expect(view.getContent()).andReturn(inputContent).anyTimes();
-    expect(spec.getView(GadgetSpec.DEFAULT_VIEW)).andReturn(view).anyTimes();
-    expect(spec.getAttribute(GadgetSpec.EXPIRATION_ATTRIB)).andReturn(Long.MAX_VALUE).anyTimes();
-    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);
-    assertEquals(inputContent, gadget.getContent());
-    
-    // Should be rewritten the first time.
-    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
-    // expected rewritten content value.
-    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/rewrite/DefaultContentRewriterRegistryTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistryTest.java?rev=697440&r1=697439&r2=697440&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistryTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/DefaultContentRewriterRegistryTest.java Sat Sep 20 19:14:38 2008
@@ -17,81 +17,65 @@
  */
 package org.apache.shindig.gadgets.rewrite;
 
-import org.easymock.classextension.EasyMock;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.classextension.EasyMock.replay;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
+import org.apache.shindig.common.ContainerConfig;
+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 junit.framework.TestCase;
+import edu.emory.mathcs.backport.java.util.Collections;
 
-public class DefaultContentRewriterRegistryTest extends TestCase {
-  public void testNoArgsCreatedBasicRegistry() {
-    DefaultContentRewriterRegistry r = new DefaultContentRewriterRegistry(null, null);
-    assertNotNull(r.getRewriters());
-    assertEquals(0, r.getRewriters().size());
-  }
-  
-  public void testSingleValuedBasicRegistry() {
-    DefaultContentRewriterRegistry r = new DefaultContentRewriterRegistry(
-        new NoOpContentRewriter(), null);
-    assertNotNull(r.getRewriters());
-    assertEquals(1, r.getRewriters().size());
-    assertTrue(r.getRewriters().get(0) instanceof NoOpContentRewriter);
-  }
-  
-  public void testBasicContentRegistryWithAdds() {
-    ContentRewriter cr0 = new NoOpContentRewriter();
-    DefaultContentRewriterRegistry r = new DefaultContentRewriterRegistry(cr0, null);
-    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));
+import org.easymock.classextension.EasyMock;
+import org.easymock.classextension.IMocksControl;
+import org.junit.Test;
+
+import java.net.URI;
+import java.util.Arrays;
+import java.util.List;
+
+public class DefaultContentRewriterRegistryTest {
+  private final List<CaptureRewriter> rewriters
+      = Arrays.asList(new CaptureRewriter(), new CaptureRewriter());
+  private final ContentRewriterRegistry registry
+      = new DefaultContentRewriterRegistry(rewriters, null);
+  private final IMocksControl control = EasyMock.createNiceControl();
+  private final ContainerConfig config = control.createMock(ContainerConfig.class);
+
+  @Test
+  @SuppressWarnings("unchecked")
+  public void rewriteGadget() throws Exception {
+    String body = "Hello, world";
+    String xml = "<Module><ModulePrefs title=''/><Content>" + body + "</Content></Module>";
+    GadgetSpec spec = new GadgetSpec(URI.create("#"), xml);
+    GadgetContext context = new GadgetContext();
+    Gadget gadget = new Gadget(context, spec, Collections.emptyList(), config, null);
+
+    control.replay();
+
+    registry.rewriteGadget(gadget);
+
+    assertTrue("First rewriter not invoked.", rewriters.get(0).viewWasRewritten());
+    assertTrue("Second rewriter not invoked.", rewriters.get(1).viewWasRewritten());
+
+    assertEquals(body, gadget.getContent());
   }
-  
-  public void testRunGadgetAndHttpResponseRewrites() throws Exception {
-    DefaultContentRewriterRegistry r = new DefaultContentRewriterRegistry(null, null);
-    StringBuilder appendFull = new StringBuilder();
-    for (int i = 0; i < 3; ++i) {
-      String appendNew = "-" + i;
-      appendFull.append(appendNew);
-      r.appendRewriter(new AppendingRewriter(appendNew));
-    }
-    String inputContent = "foo";
-    String rewrittenContent = inputContent + appendFull.toString();
-    
-    GadgetSpec spec = EasyMock.createNiceMock(GadgetSpec.class);
-    View view = EasyMock.createNiceMock(View.class);
-    expect(view.getName()).andReturn(GadgetSpec.DEFAULT_VIEW).anyTimes();
-    expect(view.getType()).andReturn(View.ContentType.HTML).anyTimes();
-    expect(view.getContent()).andReturn(inputContent).anyTimes();
-    expect(spec.getView(GadgetSpec.DEFAULT_VIEW)).andReturn(view).anyTimes();
-    GadgetContext context = EasyMock.createNiceMock(GadgetContext.class);
-    expect(context.getView()).andReturn(GadgetSpec.DEFAULT_VIEW).anyTimes();
-    replay(context, view, spec);
-    
-    Gadget gadget = new Gadget(context, spec, null, null, null);
-    assertEquals(inputContent, gadget.getContent());
-    assertTrue(r.rewriteGadget(gadget));
-    assertEquals(rewrittenContent, gadget.getContent());
-    
-    HttpResponse resp = new HttpResponseBuilder().setResponseString(inputContent).create();
-    assertEquals(inputContent, resp.getResponseAsString());
-    HttpRequest req = EasyMock.createNiceMock(HttpRequest.class);  // use mock to be lazy
-    HttpResponse rewritten = r.rewriteHttpResponse(req, resp);
-    assertNotSame(resp, rewritten);
-    assertEquals(rewrittenContent, rewritten.getResponseAsString());
+
+  @Test
+  public void rewriteHttpResponse() throws Exception {
+    String body = "Hello, world";
+    HttpRequest request = new HttpRequest(Uri.parse("#"));
+    HttpResponse response = new HttpResponse(body);
+
+    HttpResponse rewritten = registry.rewriteHttpResponse(request, response);
+
+    assertTrue("First rewriter not invoked.", rewriters.get(0).responseWasRewritten());
+    assertTrue("Second rewriter not invoked.", rewriters.get(1).responseWasRewritten());
+
+    assertEquals(response, rewritten);
   }
-  
 }

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=697440&r1=697439&r2=697440&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 Sat Sep 20 19:14:38 2008
@@ -30,7 +30,6 @@
 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.DefaultContentRewriterRegistry;
 
 import org.json.JSONArray;
 import org.json.JSONException;
@@ -48,8 +47,8 @@
   private static final String RESPONSE_BODY = "makeRequest response body";
   private static final SecurityToken DUMMY_TOKEN = new FakeGadgetToken();
 
-  private final MakeRequestHandler handler = new MakeRequestHandler(fetcherFactory,
-      new DefaultContentRewriterRegistry(rewriter, null));
+  private final MakeRequestHandler handler
+      = new MakeRequestHandler(fetcherFactory, rewriterRegistry);
 
   private void expectGetAndReturnBody(String response) throws Exception {
     expectGetAndReturnBody(AuthType.NONE, response);
@@ -359,7 +358,7 @@
     assertEquals(RESPONSE_BODY, results.getString("foo"));
     assertTrue(rewriter.responseWasRewritten());
   }
-  
+
   public void testSetCookiesReturned() throws Exception {
     HttpRequest internalRequest = new HttpRequest(REQUEST_URL);
     HttpResponse response = new HttpResponseBuilder()
@@ -367,7 +366,7 @@
         .addHeader("Set-Cookie", "foo=bar; Secure")
         .addHeader("Set-Cookie", "name=value")
         .create();
-    
+
     expect(fetcherFactory.fetch(internalRequest)).andReturn(response);
     replay();
 
@@ -381,14 +380,14 @@
     assertEquals("foo=bar; Secure", cookies.get(0));
     assertEquals("name=value", cookies.get(1));
   }
-  
+
   public void testLocationReturned() throws Exception {
     HttpRequest internalRequest = new HttpRequest(REQUEST_URL);
     HttpResponse response = new HttpResponseBuilder()
         .setResponse("foo".getBytes("UTF-8"))
         .addHeader("Location", "somewhere else")
         .create();
-    
+
     expect(fetcherFactory.fetch(internalRequest)).andReturn(response);
     replay();
 

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=697440&r1=697439&r2=697440&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 Sat Sep 20 19:14:38 2008
@@ -18,26 +18,27 @@
  */
 package org.apache.shindig.gadgets.servlet;
 
+import static org.easymock.EasyMock.expect;
+
 import org.apache.shindig.common.uri.Uri;
 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.rewrite.DefaultContentRewriterRegistry;
-import static org.easymock.EasyMock.expect;
 
-import javax.servlet.http.HttpServletResponse;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.servlet.http.HttpServletResponse;
+
 public class ProxyHandlerTest extends ServletTestFixture {
   private final static String URL_ONE = "http://www.example.org/test.html";
   private final static String DATA_ONE = "hello world";
 
   private final ProxyHandler proxyHandler
-      = new ProxyHandler(fetcher, lockedDomainService, new DefaultContentRewriterRegistry(rewriter, null));
+      = new ProxyHandler(fetcher, lockedDomainService, rewriterRegistry);
 
   private void expectGetAndReturnData(String url, byte[] data) throws Exception {
     HttpRequest req = new HttpRequest(Uri.parse(url));