Author: johnh
Date: Wed Aug 27 17:50:01 2008
New Revision: 689683

URL: http://svn.apache.org/viewvc?rev=689683&view=rev
Log:
Refactoring BasicGadgetSpecFactory and BasicMessageBundleFactory, per TODOs in 
the code. Both now
inherit from the abstract templatized base class CachingWebRetrievalFactory, to 
which is provided
the return value of the cache lookup and the type of the object used for 
lookup. All other logic
was identical, so was pulled up.

Refactoring done in preparation for yet another caching factory, this time for 
rewritten content (which
may require the cache key type - currently fixed at URI - to be templatized as 
well).


Added:
    
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/CachingWebRetrievalFactory.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/BasicMessageBundleFactory.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=689683&r1=689682&r2=689683&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
 Wed Aug 27 17:50:01 2008
@@ -17,6 +17,7 @@
  */
 package org.apache.shindig.gadgets;
 
+import org.apache.shindig.common.cache.CacheProvider;
 import org.apache.shindig.gadgets.spec.GadgetSpec;
 import org.apache.shindig.gadgets.spec.LocaleSpec;
 import org.apache.shindig.gadgets.spec.MessageBundle;
@@ -28,7 +29,15 @@
  * Core implementation of MessageBundleFactory that ensures proper 
MessageBundle creation and
  * delegates caching and network retrieval to concreate implementations.
  */
-public abstract class AbstractMessageBundleFactory implements 
MessageBundleFactory {
+public abstract class AbstractMessageBundleFactory
+    extends CachingWebRetrievalFactory<MessageBundle, LocaleSpec>
+    implements MessageBundleFactory {
+  
+  protected AbstractMessageBundleFactory(CacheProvider cacheProvider,
+      int capacity, long minTtl, long maxTtl) {
+    super(cacheProvider, capacity, minTtl, maxTtl);
+  }
+  
   private static final Locale ALL_ALL = new Locale("all", "ALL");
 
   public MessageBundle getBundle(GadgetSpec spec, Locale locale, boolean 
ignoreCache)

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=689683&r1=689682&r2=689683&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
 Wed Aug 27 17:50:01 2008
@@ -18,7 +18,6 @@
  */
 package org.apache.shindig.gadgets;
 
-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.http.HttpFetcher;
@@ -38,27 +37,22 @@
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
-import java.util.logging.Logger;
 
 /**
  * Basic implementation of a gadget spec factory.
- *
- * TODO: This needs to be unified with message bundle fetching. We've 
basically copied the class in
- * two places, which is horrible.
  */
 @Singleton
-public class BasicGadgetSpecFactory implements GadgetSpecFactory {
-
-  private static final Logger logger = 
Logger.getLogger(BasicGadgetSpecFactory.class.getName());
+public class BasicGadgetSpecFactory extends 
CachingWebRetrievalFactory<GadgetSpec, URI> 
+    implements GadgetSpecFactory {
 
   private final HttpFetcher fetcher;
   private final ContentRewriterRegistry rewriterRegistry;
   private final ExecutorService executor;
-  private final long minTtl;
-  private final long maxTtl;
 
-  // A cache of GadgetSpecs with expirations
-  private final Cache<URI, TimeoutPair> cache;
+  @Override
+  protected URI getCacheKeyFromQueryObj(URI queryObj) {
+    return queryObj;
+  }
 
   public GadgetSpec getGadgetSpec(GadgetContext context) throws 
GadgetException {
     return getGadgetSpec(context.getUrl(), context.getIgnoreCache());
@@ -68,49 +62,15 @@
    * Retrieves a gadget specification from the cache or from the Internet.
    */
   public GadgetSpec getGadgetSpec(URI url, boolean ignoreCache) throws 
GadgetException {
-    if (ignoreCache) {
-      return fetchFromWeb(url, true);
-    }
-
-    GadgetSpec spec = null;
-    long expiration = -1;
-
-    // Attempt to retrieve the gadget spec from the cache.
-    synchronized (cache) {
-      TimeoutPair gadgetSpecEntry = cache.getElement(url);
-      if (gadgetSpecEntry != null) {
-        spec = gadgetSpecEntry.spec;
-        expiration = gadgetSpecEntry.timeout;
-      }
-    }
-
-    long now = System.currentTimeMillis();
-
-    // If the gadget spec is not in the cache or has expired, fetch it from 
its URI.
-    if (spec == null || expiration < now) {
-      try {
-        return fetchFromWeb(url, false);
-      } catch (GadgetException e) {
-        if (spec == null) {
-          throw e;
-        } else {
-          logger.info("Gadget spec fetch failed for " + url + " -  using 
cached ");
-          // Try again later...
-          synchronized (cache) {
-            cache.addElement(url, new TimeoutPair(spec, now + minTtl));
-          }
-        }
-      }
-    }
-
-    return spec;
+    return doCachedFetch(url, ignoreCache);
   }
 
   /**
    * Retrieves a gadget specification from the Internet, processes its views 
and
    * adds it to the cache.
    */
-  private GadgetSpec fetchFromWeb(URI url, boolean ignoreCache) throws 
GadgetException {
+  protected FetchedObject<GadgetSpec> fetchFromWeb(URI url, boolean 
ignoreCache)
+      throws GadgetException {
     HttpRequest request = new 
HttpRequest(Uri.fromJavaUri(url)).setIgnoreCache(ignoreCache);
     HttpResponse response = fetcher.fetch(request);
     if (response.getHttpStatusCode() != HttpResponse.SC_OK) {
@@ -157,17 +117,7 @@
       }
     }
 
-    // We enforce the lower bound limit here for situations where a remote 
server temporarily serves
-    // the wrong cache control headers. This allows any distributed caches to 
be updated and for the
-    // updates to eventually cascade back into the factory.
-    long now = System.currentTimeMillis();
-    long expiration = response.getCacheExpiration();
-    expiration = Math.max(now + minTtl, Math.min(now + maxTtl, expiration));
-    synchronized (cache) {
-      cache.addElement(url, new TimeoutPair(spec, expiration));
-    }
-
-    return spec;
+    return new FetchedObject<GadgetSpec>(spec, response.getCacheExpiration());
   }
 
   @Inject
@@ -178,21 +128,9 @@
                                 
@Named("shindig.gadget-spec.cache.capacity")int gadgetSpecCacheCapacity,
                                 @Named("shindig.gadget-spec.cache.minTTL")long 
minTtl,
                                 @Named("shindig.gadget-spec.cache.maxTTL")long 
maxTtl) {
+    super(cacheProvider, gadgetSpecCacheCapacity, minTtl, maxTtl);
     this.fetcher = fetcher;
     this.rewriterRegistry = rewriterRegistry;
     this.executor = executor;
-    this.cache = cacheProvider.createCache(gadgetSpecCacheCapacity);
-    this.minTtl = minTtl;
-    this.maxTtl = maxTtl;
-  }
-
-  private static class TimeoutPair {
-    private final GadgetSpec spec;
-    private final long timeout;
-
-    private TimeoutPair(GadgetSpec spec, long timeout) {
-      this.spec = spec;
-      this.timeout = timeout;
-    }
   }
 }

Modified: 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java?rev=689683&r1=689682&r2=689683&view=diff
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java
 (original)
+++ 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java
 Wed Aug 27 17:50:01 2008
@@ -18,7 +18,6 @@
  */
 package org.apache.shindig.gadgets;
 
-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.http.HttpFetcher;
@@ -32,61 +31,18 @@
 import com.google.inject.name.Named;
 
 import java.net.URI;
-import java.util.logging.Logger;
 
 /**
- * Basic implementation of a message bundle factory
+ * Basic implementation of a message bundle factory.
  */
 @Singleton
 public class BasicMessageBundleFactory extends AbstractMessageBundleFactory {
 
-  private static final Logger LOG = 
Logger.getLogger(BasicMessageBundleFactory.class.getName());
-
   private final HttpFetcher fetcher;
 
-  private final Cache<URI, TimeoutPair> cache;
-
-  private final long minTtl;
-  private final long maxTtl;
-
-  protected MessageBundle fetchBundle(LocaleSpec locale, boolean ignoreCache)
-      throws GadgetException {
-    if (ignoreCache) {
-      return fetchFromWeb(locale, true);
-    }
-
-    URI messages = locale.getMessages();
-
-    MessageBundle bundle = null;
-    long expiration = -1;
-    synchronized (cache) {
-      TimeoutPair entry = cache.getElement(messages);
-      if (entry != null) {
-        bundle = entry.bundle;
-        expiration = entry.timeout;
-      }
-    }
-
-    long now = System.currentTimeMillis();
-    if (bundle == null || expiration < now) {
-      try {
-        return fetchFromWeb(locale, false);
-      } catch (GadgetException e) {
-        if (bundle == null) {
-          throw e;
-        } else {
-          LOG.info("Message bundle fetch failed for " + messages + " -  using 
cached ");
-          // Try again later...
-          synchronized (cache) {
-            cache.addElement(messages, new TimeoutPair(bundle, now + minTtl));
-          }
-        }
-      }
-    }
-    return bundle;
-  }
-
-  private MessageBundle fetchFromWeb(LocaleSpec locale, boolean ignoreCache) 
throws GadgetException {
+  @Override
+  protected FetchedObject<MessageBundle> fetchFromWeb(LocaleSpec locale,
+      boolean ignoreCache) throws GadgetException {
     URI url = locale.getMessages();
     HttpRequest request = new 
HttpRequest(Uri.fromJavaUri(url)).setIgnoreCache(ignoreCache);
     HttpResponse response = fetcher.fetch(request);
@@ -97,17 +53,17 @@
     }
 
     MessageBundle bundle  = new MessageBundle(locale, 
response.getResponseAsString());
+    return new FetchedObject<MessageBundle>(bundle, 
response.getCacheExpiration());
+  }
 
-    // We enforce the lower bound limit here for situations where a remote 
server temporarily serves
-    // the wrong cache control headers. This allows any distributed caches to 
be updated and for the
-    // updates to eventually cascade back into the factory.
-    long now = System.currentTimeMillis();
-    long expiration = response.getCacheExpiration();
-    expiration = Math.max(now + minTtl, Math.min(now + maxTtl, expiration));
-    synchronized (cache) {
-      cache.addElement(url, new TimeoutPair(bundle, expiration));
-    }
-    return bundle;
+  @Override
+  protected URI getCacheKeyFromQueryObj(LocaleSpec queryObj) {
+    return queryObj.getMessages();
+  }
+
+  protected MessageBundle fetchBundle(LocaleSpec locale, boolean ignoreCache)
+      throws GadgetException {
+    return doCachedFetch(locale, ignoreCache);
   }
 
   @Inject
@@ -116,19 +72,7 @@
                                    
@Named("shindig.message-bundle.cache.capacity")int capacity,
                                    
@Named("shindig.message-bundle.cache.minTTL")long minTtl,
                                    
@Named("shindig.message-bundle.cache.maxTTL")long maxTtl) {
+    super(cacheProvider, capacity, minTtl, maxTtl);
     this.fetcher = fetcher;
-    this.cache = cacheProvider.createCache(capacity);
-    this.minTtl = minTtl;
-    this.maxTtl = maxTtl;
-  }
-
-  private static class TimeoutPair {
-    private MessageBundle bundle;
-    private long timeout;
-
-    private TimeoutPair(MessageBundle bundle, long timeout) {
-      this.bundle = bundle;
-      this.timeout = timeout;
-    }
   }
 }

Added: 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/CachingWebRetrievalFactory.java
URL: 
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/CachingWebRetrievalFactory.java?rev=689683&view=auto
==============================================================================
--- 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/CachingWebRetrievalFactory.java
 (added)
+++ 
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/CachingWebRetrievalFactory.java
 Wed Aug 27 17:50:01 2008
@@ -0,0 +1,116 @@
+/*
+ * 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;
+
+import java.net.URI;
+import java.util.logging.Logger;
+
+import org.apache.shindig.common.cache.Cache;
+import org.apache.shindig.common.cache.CacheProvider;
+import org.apache.shindig.gadgets.GadgetException;
+
+public abstract class CachingWebRetrievalFactory<T, Q> {
+  // Subclasses must override these.
+  protected abstract FetchedObject<T> fetchFromWeb(Q queryObj, boolean 
ignoreCache) throws GadgetException;
+  protected abstract URI getCacheKeyFromQueryObj(Q queryObj);
+  
+  private static final Logger logger = 
Logger.getLogger(CachingWebRetrievalFactory.class.getName());
+  private final Cache<URI, TimeoutPair<T>> cache;
+  private final long minTtl, maxTtl;
+
+  protected CachingWebRetrievalFactory(CacheProvider cacheProvider,
+      int capacity, long minTtl, long maxTtl) {
+    this.cache = cacheProvider.createCache(capacity);
+    this.minTtl = minTtl;
+    this.maxTtl = maxTtl;
+  }
+  
+  protected T doCachedFetch(Q queryObj, boolean ignoreCache) throws 
GadgetException {
+    if (ignoreCache) {
+      return fetchObjectAndCache(queryObj, ignoreCache);
+    }
+    
+    T resultObj = null;
+    long expiration = -1;
+    URI cacheKey = getCacheKeyFromQueryObj(queryObj);
+    
+    synchronized(cache) {
+      TimeoutPair<T> cachedEntry = cache.getElement(cacheKey);
+      if (cachedEntry != null) {
+        resultObj = cachedEntry.cachedObj;
+        expiration = cachedEntry.timeout;
+      }
+    }
+    
+    // If the obj is not in the cache or has expired, fetch it from its URI.
+    long now = System.currentTimeMillis();
+    
+    if (resultObj == null || expiration < now) {
+      try {
+        return fetchObjectAndCache(queryObj, false);
+      } catch (GadgetException e) {
+        if (resultObj == null) {
+          throw e;
+        } else {
+          logger.info("Object fetch failed for " + cacheKey + " -  using 
cached ");
+          // Try again later...
+          synchronized (cache) {
+            cache.addElement(cacheKey, new TimeoutPair<T>(resultObj, now + 
minTtl));
+          }
+        }
+      }
+    }
+    
+    return resultObj;
+  }
+  
+  private T fetchObjectAndCache(Q queryObj, boolean ignoreCache) throws 
GadgetException {
+    FetchedObject<T> fetched = fetchFromWeb(queryObj, ignoreCache);
+    
+    long now = System.currentTimeMillis();
+    long expiration = fetched.expirationTime;
+    expiration = Math.max(now + minTtl, Math.min(now + maxTtl, expiration));
+    synchronized (cache) {
+      cache.addElement(getCacheKeyFromQueryObj(queryObj),
+          new TimeoutPair<T>(fetched.fetchedObj, expiration));
+    }
+    
+    return fetched.fetchedObj;
+  }
+  
+  protected static class FetchedObject<T> {
+    private T fetchedObj;
+    private long expirationTime;
+    
+    protected FetchedObject(T fetchedObj, long expirationTime) {
+      this.fetchedObj = fetchedObj;
+      this.expirationTime = expirationTime;
+    }
+  }
+  
+  private static class TimeoutPair<T> {
+    private T cachedObj;
+    private long timeout;
+
+    private TimeoutPair(T cachedObj, long timeout) {
+      this.cachedObj = cachedObj;
+      this.timeout = timeout;
+    }
+  }
+}


Reply via email to