You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by et...@apache.org on 2009/07/16 10:26:58 UTC

svn commit: r794588 - in /incubator/shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/ main/java/org/apache/shindig/gadgets/spec/ test/java/org/apache/shindig/gadgets/

Author: etnu
Date: Thu Jul 16 08:26:57 2009
New Revision: 794588

URL: http://svn.apache.org/viewvc?rev=794588&view=rev
Log:
Refactored factories for manifest files to share more code -- this also resulted in fixing a few bugs in message bundle handling:

- Improperly caching objects under bogus keys, resulting in many large objects going into the cache unnecessarily.
- Missing the all_COUNTRY locale from the merge chain (exact > lang_ALL > all_COUNTRY > all_ALL)
- Not enforcing negative caching for bad message bundles
- Improperly returning empty message bundles instead of giving an error when a message bundle fails to parse.


Added:
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java
Modified:
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/MessageBundleFactory.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.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/DefaultGadgetSpecFactoryTest.java
    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java

Added: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java?rev=794588&view=auto
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java (added)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/AbstractSpecFactory.java Thu Jul 16 08:26:57 2009
@@ -0,0 +1,210 @@
+/*
+ * 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 org.apache.shindig.common.cache.Cache;
+import org.apache.shindig.common.cache.SoftExpiringCache;
+import org.apache.shindig.common.uri.Uri;
+import org.apache.shindig.common.xml.XmlException;
+import org.apache.shindig.config.ContainerConfig;
+import org.apache.shindig.gadgets.http.HttpRequest;
+import org.apache.shindig.gadgets.http.HttpResponse;
+import org.apache.shindig.gadgets.http.RequestPipeline;
+import org.apache.shindig.gadgets.spec.SpecParserException;
+
+import java.util.concurrent.ExecutorService;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Basis for implementing GadgetSpec and MessageBundle factories.
+ *
+ * Automatically updates objects as needed asynchronously to provide optimal throughput.
+ */
+public abstract class AbstractSpecFactory<T> {
+  private static final Logger logger = Logger.getLogger(AbstractSpecFactory.class.getName());
+  private final Class<T> clazz;
+  private final ExecutorService executor;
+  private final RequestPipeline pipeline;
+  final SoftExpiringCache<Uri, Object> cache;
+  private final long refresh;
+
+  /**
+   * @param clazz the class for spec objects.
+   * @param executor for asynchronously updating specs
+   * @param pipeline the request pipeline for fetching new specs
+   * @param cache a cache for parsed spec objects
+   * @param refresh the frequency at which to update specs, independent of cache expiration policy
+   */
+  public AbstractSpecFactory(Class<T> clazz, ExecutorService executor, RequestPipeline pipeline,
+      Cache<Uri, Object> cache, long refresh) {
+    this.clazz = clazz;
+    this.executor = executor;
+    this.pipeline = pipeline;
+    this.cache = new SoftExpiringCache<Uri, Object>(cache);
+    this.refresh = refresh;
+  }
+
+  /**
+   * Attempt to fetch a spec, either from cache or from the network.
+   *
+   * Note that the {@code query} passed here will always be passed, unmodified, to
+   * {@link #parse(String, Query)}. This can be used to carry additional context information
+   * during parsing.
+   */
+  protected T getSpec(Query query) throws GadgetException {
+    Object obj = null;
+    if (!query.ignoreCache) {
+      SoftExpiringCache.CachedObject<Object> cached = cache.getElement(query.specUri);
+      if (cached != null) {
+        obj = cached.obj;
+        if (cached.isExpired) {
+          // We write to the cache to avoid any race conditions with multiple writers.
+          // This causes a double write, but that's better than a write per thread or synchronizing
+          // this block.
+          cache.addElement(query.specUri, obj, refresh);
+          executor.execute(new SpecUpdater(query, obj));
+        }
+      }
+    }
+
+    if (obj == null) {
+      try {
+        obj = fetchFromNetwork(query);
+      } catch (GadgetException e) {
+        obj = e;
+      }
+      cache.addElement(query.specUri, obj, refresh);
+    }
+
+    if (obj instanceof GadgetException) {
+      throw (GadgetException) obj;
+    }
+
+    // If there's a bug that puts the wrong object in here, we'll get a ClassCastException.
+    return clazz.cast(obj);
+  }
+
+  /**
+   * Retrieves a spec from the network, parses, and adds it to the cache.
+   */
+  protected T fetchFromNetwork(Query query) throws GadgetException {
+    HttpRequest request = new HttpRequest(query.specUri)
+        .setIgnoreCache(query.ignoreCache)
+        .setGadget(query.gadgetUri)
+        .setContainer(query.container);
+
+    // Since we don't allow any variance in cache time, we should just force the cache time
+    // globally. This ensures propagation to shared caches when this is set.
+    request.setCacheTtl((int) (refresh / 1000));
+
+    HttpResponse response = pipeline.execute(request);
+    if (response.getHttpStatusCode() != HttpResponse.SC_OK) {
+      throw new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
+                                "Unable to retrieve spec for " + query.specUri + ". HTTP error " +
+                                response.getHttpStatusCode());
+    }
+
+    try {
+      String content = response.getResponseAsString();
+      return parse(content, query);
+    } catch (XmlException e) {
+      throw new SpecParserException(e);
+    }
+  }
+
+  /**
+   * Parse and return a new spec object from the network.
+   *
+   * @param content the content located at specUri
+   * @param query same as was passed {@link #getSpec(Query)}
+   */
+  protected abstract T parse(String content, Query query) throws XmlException, GadgetException;
+
+  /**
+   * Holds information used to fetch a spec.
+   */
+  protected static class Query {
+    private Uri specUri = null;
+    private String container = ContainerConfig.DEFAULT_CONTAINER;
+    private Uri gadgetUri = null;
+    private boolean ignoreCache = false;
+
+    public Query setSpecUri(Uri specUri) {
+      this.specUri = specUri;
+      return this;
+    }
+
+    public Query setContainer(String container) {
+      this.container = container;
+      return this;
+    }
+
+    public Query setGadgetUri(Uri gadgetUri) {
+      this.gadgetUri = gadgetUri;
+      return this;
+    }
+
+    public Query setIgnoreCache(boolean ignoreCache) {
+      this.ignoreCache = ignoreCache;
+      return this;
+    }
+
+    public Uri getSpecUri() {
+      return specUri;
+    }
+
+    public String getContainer() {
+      return container;
+    }
+
+    public Uri getGadgetUri() {
+      return gadgetUri;
+    }
+
+    public boolean getIgnoreCache() {
+      return ignoreCache;
+    }
+  }
+
+  private class SpecUpdater implements Runnable {
+    private final Query query;
+    private final Object old;
+
+    public SpecUpdater(Query query, Object old) {
+      this.query = query;
+      this.old = old;
+    }
+
+    public void run() {
+      try {
+        T newSpec = fetchFromNetwork(query);
+        cache.addElement(query.specUri, newSpec, refresh);
+      } catch (GadgetException e) {
+        if (old != null) {
+          logger.log(Level.INFO, "Failed to update {0}. Using cached version.", query.specUri);
+          cache.addElement(query.specUri, old, refresh);
+        } else {
+          logger.log(Level.INFO, "Failed to update {0}. Applying negative cache.", query.specUri);
+          cache.addElement(query.specUri, e, refresh);
+        }
+      }
+    }
+  }
+}

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java?rev=794588&r1=794587&r2=794588&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java Thu Jul 16 08:26:57 2009
@@ -20,18 +20,13 @@
 
 import org.apache.shindig.common.cache.Cache;
 import org.apache.shindig.common.cache.CacheProvider;
-import org.apache.shindig.common.cache.SoftExpiringCache;
 import org.apache.shindig.common.uri.Uri;
 import org.apache.shindig.common.xml.XmlException;
 import org.apache.shindig.common.xml.XmlUtil;
-import org.apache.shindig.gadgets.http.HttpRequest;
-import org.apache.shindig.gadgets.http.HttpResponse;
 import org.apache.shindig.gadgets.http.RequestPipeline;
-import org.apache.shindig.gadgets.spec.ApplicationManifest;
 import org.apache.shindig.gadgets.spec.GadgetSpec;
 import org.apache.shindig.gadgets.spec.SpecParserException;
 
-import com.google.common.base.Preconditions;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import com.google.inject.name.Named;
@@ -39,38 +34,27 @@
 import org.w3c.dom.Element;
 
 import java.util.concurrent.ExecutorService;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 /**
  * Default implementation of a gadget spec factory.
  */
 @Singleton
-public class DefaultGadgetSpecFactory implements GadgetSpecFactory {
+public class DefaultGadgetSpecFactory extends AbstractSpecFactory<GadgetSpec>
+    implements GadgetSpecFactory {
   public static final String CACHE_NAME = "gadgetSpecs";
-
-  static final String VERSION_PARAM = "version";
-  static final String LABEL_PARAM = "label";
-  static final String DEFAULT_LABEL = "production";
   static final String RAW_GADGETSPEC_XML_PARAM_NAME = "rawxml";
   static final Uri RAW_GADGET_URI = Uri.parse("http://localhost/raw.xml");
 
-  private final Logger logger = Logger.getLogger(DefaultGadgetSpecFactory.class.getName());
-  private final ExecutorService executor;
-  private final RequestPipeline pipeline;
-  final SoftExpiringCache<Uri, Object> cache;
-  private final long refresh;
-
   @Inject
   public DefaultGadgetSpecFactory(ExecutorService executor,
                                   RequestPipeline pipeline,
                                   CacheProvider cacheProvider,
                                   @Named("shindig.cache.xml.refreshInterval") long refresh) {
-    this.executor = executor;
-    this.pipeline = pipeline;
-    Cache<Uri, Object> baseCache = cacheProvider.createCache(CACHE_NAME);
-    this.cache = new SoftExpiringCache<Uri, Object>(baseCache);
-    this.refresh = refresh;
+    super(GadgetSpec.class, executor, pipeline, makeCache(cacheProvider), refresh);
+  }
+
+  private static Cache<Uri, Object> makeCache(CacheProvider cacheProvider) {
+    return cacheProvider.createCache(CACHE_NAME);
   }
 
   public GadgetSpec getGadgetSpec(GadgetContext context) throws GadgetException {
@@ -86,145 +70,19 @@
       }
     }
 
-    return fetchObject(context.getUrl(), context, false);
-  }
-
-  private GadgetSpec getSpecFromManifest(ApplicationManifest manifest, GadgetContext context)
-      throws GadgetException {
-    String version = context.getParameter(VERSION_PARAM);
-
-    if (version == null) {
-      // TODO: The label param should only be used for metadata calls. This should probably be
-      // exposed up a layer in the stack, perhaps at the interface level.
-      String label = firstNonNull(context.getParameter(LABEL_PARAM), DEFAULT_LABEL);
-
-      version = manifest.getVersion(label);
-
-      if (version == null) {
-        throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
-            "Unable to find a suitable version for the given manifest.");
-      }
-    }
-
-    Uri specUri = manifest.getGadget(version);
-
-    if (specUri == null) {
-      throw new GadgetException(GadgetException.Code.INVALID_PARAMETER,
-          "No gadget spec available for the given version.");
-    }
-
-    return fetchObject(specUri, context, true);
-  }
-
-  private GadgetSpec fetchObject(Uri uri, GadgetContext context, boolean noManifests)
-      throws GadgetException {
-
-    Object obj = null;
-    if (!context.getIgnoreCache()) {
-      SoftExpiringCache.CachedObject<Object> cached = cache.getElement(uri);
-      if (cached != null) {
-        obj = cached.obj;
-        if (cached.isExpired) {
-          // We write to the cache to avoid any race conditions with multiple writers.
-          // This causes a double write, but that's better than a write per thread or synchronizing
-          // this block.
-          cache.addElement(uri, obj, refresh);
-          executor.execute(new ObjectUpdater(uri, context, obj));
-        }
-      }
-    }
-
-    if (obj == null) {
-      try {
-        obj = fetchFromNetwork(uri, context);
-      } catch (GadgetException e) {
-        obj = e;
-      }
-
-      cache.addElement(uri, obj, refresh);
-    }
-
-    if (obj instanceof GadgetSpec) {
-      return (GadgetSpec) obj;
-    }
-
-    if (obj instanceof ApplicationManifest) {
-      if (noManifests) {
-        throw new SpecParserException("Manifests may not reference other manifests.");
-      }
-
-      return getSpecFromManifest((ApplicationManifest) obj, context);
-    }
-
-    if (obj instanceof GadgetException) {
-      throw (GadgetException) obj;
-    }
-
-    // Some big bug.
-    throw new GadgetException(GadgetException.Code.INTERNAL_SERVER_ERROR,
-        "Unknown object type stored for input URI " + uri);
-  }
-
-  /**
-   * Retrieves a gadget specification from the Internet, processes its views and
-   * adds it to the cache.
-   */
-  private Object fetchFromNetwork(Uri uri, GadgetContext context) throws GadgetException {
-    HttpRequest request = new HttpRequest(uri)
-        .setIgnoreCache(context.getIgnoreCache())
-        .setGadget(uri)
-        .setContainer(context.getContainer());
-
-    // Since we don't allow any variance in cache time, we should just force the cache time
-    // globally. This ensures propagation to shared caches when this is set.
-    request.setCacheTtl((int) (refresh / 1000));
-
-    HttpResponse response = pipeline.execute(request);
-    if (response.getHttpStatusCode() != HttpResponse.SC_OK) {
-      throw new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
-                                "Unable to retrieve gadget xml. HTTP error " +
-                                response.getHttpStatusCode());
-    }
-
-    try {
-      String content = response.getResponseAsString();
-      Element element = XmlUtil.parse(content);
-      if (ApplicationManifest.NAMESPACE.equals(element.getNamespaceURI())) {
-        return new ApplicationManifest(uri, element);
-      }
-      return new GadgetSpec(uri, element, content);
-    } catch (XmlException e) {
-      throw new SpecParserException(e);
-    }
-  }
+    Uri gadgetUri = context.getUrl();
 
-  private class ObjectUpdater implements Runnable {
-    private final Uri uri;
-    private final GadgetContext context;
-    private final Object old;
-
-    public ObjectUpdater(Uri uri, GadgetContext context, Object old) {
-      this.uri = uri;
-      this.context = context;
-      this.old = old;
-    }
-
-    public void run() {
-      try {
-        Object newObject = fetchFromNetwork(uri, context);
-        cache.addElement(uri, newObject, refresh);
-      } catch (GadgetException e) {
-        if (old != null) {
-          logger.log(Level.INFO, "Failed to update {0}. Using cached version.", uri);
-          cache.addElement(uri, old, refresh);
-        } else {
-          logger.log(Level.INFO, "Failed to update {0}. Applying negative cache.", uri);
-          cache.addElement(uri, e, refresh);
-        }
-      }
-    }
-  }
-  private static <T> T firstNonNull(T first, T second) {
-    return first != null ? first : Preconditions.checkNotNull(second);
+    Query query = new Query()
+        .setSpecUri(gadgetUri)
+        .setContainer(context.getContainer())
+        .setGadgetUri(gadgetUri)
+        .setIgnoreCache(context.getIgnoreCache());
+    return super.getSpec(query);
+  }
+
+  @Override
+  protected GadgetSpec parse(String content, Query query) throws XmlException, GadgetException {
+    Element element = XmlUtil.parse(content);
+    return new GadgetSpec(query.getSpecUri(), element, content);
   }
-}
+}
\ No newline at end of file

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java?rev=794588&r1=794587&r2=794588&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultMessageBundleFactory.java Thu Jul 16 08:26:57 2009
@@ -20,11 +20,8 @@
 
 import org.apache.shindig.common.cache.Cache;
 import org.apache.shindig.common.cache.CacheProvider;
-import org.apache.shindig.common.cache.SoftExpiringCache;
-import org.apache.shindig.common.cache.SoftExpiringCache.CachedObject;
 import org.apache.shindig.common.uri.Uri;
-import org.apache.shindig.gadgets.http.HttpRequest;
-import org.apache.shindig.gadgets.http.HttpResponse;
+import org.apache.shindig.config.ContainerConfig;
 import org.apache.shindig.gadgets.http.RequestPipeline;
 import org.apache.shindig.gadgets.spec.GadgetSpec;
 import org.apache.shindig.gadgets.spec.LocaleSpec;
@@ -35,112 +32,68 @@
 import com.google.inject.name.Named;
 
 import java.util.Locale;
-import java.util.logging.Logger;
+import java.util.concurrent.ExecutorService;
 
 /**
  * Default implementation of a message bundle factory.
- *
- * Containers wishing to implement custom bundle fetching behavior should override
- * {@link #fetchBundle}.
  */
 @Singleton
-public class DefaultMessageBundleFactory implements MessageBundleFactory {
+public class DefaultMessageBundleFactory extends AbstractSpecFactory<MessageBundle>
+    implements MessageBundleFactory {
   private static final Locale ALL_ALL = new Locale("all", "ALL");
   public static final String CACHE_NAME = "messageBundles";
-  static final Logger LOG = Logger.getLogger(DefaultMessageBundleFactory.class.getName());
-  private final RequestPipeline pipeline;
-  final SoftExpiringCache<String, MessageBundle> cache;
-  private final long refresh;
 
   @Inject
-  public DefaultMessageBundleFactory(RequestPipeline pipeline,
+  public DefaultMessageBundleFactory(ExecutorService executor,
+                                     RequestPipeline pipeline,
                                      CacheProvider cacheProvider,
                                      @Named("shindig.cache.xml.refreshInterval") long refresh) {
-    this.pipeline = pipeline;
-    Cache<String, MessageBundle> baseCache = cacheProvider.createCache(CACHE_NAME);
-    this.cache = new SoftExpiringCache<String, MessageBundle>(baseCache);
-    this.refresh = refresh;
+    super(MessageBundle.class, executor, pipeline, makeCache(cacheProvider), refresh);
   }
 
-  public MessageBundle getBundle(GadgetSpec spec, Locale locale, boolean ignoreCache)
-      throws GadgetException {
-
-    if (ignoreCache) {
-      return getNestedBundle(spec, locale, true);
-    }
+  private static Cache<Uri, Object> makeCache(CacheProvider cacheProvider) {
+    return cacheProvider.createCache(CACHE_NAME);
+  }
 
-    String key = spec.getUrl().toString() + '.' + locale.toString();
-    CachedObject<MessageBundle> cached = cache.getElement(key);
+  @Override
+  protected MessageBundle parse(String content, Query query) throws GadgetException {
+    return new MessageBundle(((LocaleQuery) query).locale, content);
+  }
 
-    MessageBundle bundle;
-    if (cached == null || cached.isExpired) {
-      try {
-        bundle = getNestedBundle(spec, locale, ignoreCache);
-      } catch (GadgetException e) {
-        // Enforce negative caching.
-        if (cached != null) {
-          LOG.info("MessageBundle fetch failed for " + key + " - using cached.");
-          bundle = cached.obj;
-        } else {
-          // We create this dummy spec to avoid the cost of re-parsing when a remote site is out.
-          LOG.info("MessageBundle fetch failed for " + key + " - using default.");
-          bundle = MessageBundle.EMPTY;
-        }
-      }
-      cache.addElement(key, bundle, refresh);
-    } else {
-      bundle = cached.obj;
-    }
+  public MessageBundle getBundle(GadgetSpec spec, Locale locale, boolean ignoreCache)
+      throws GadgetException {
+    MessageBundle exact = getBundleFor(spec, locale, ignoreCache);
+    MessageBundle lang = getBundleFor(spec, new Locale(locale.getLanguage(), "ALL"), ignoreCache);
+    MessageBundle country = getBundleFor(spec, new Locale("all", locale.getCountry()), ignoreCache);
+    MessageBundle all = getBundleFor(spec, ALL_ALL, ignoreCache);
 
-    return bundle;
+    return new MessageBundle(all, country, lang, exact);
   }
 
-  private MessageBundle getNestedBundle(GadgetSpec spec, Locale locale, boolean ignoreCache)
+  private MessageBundle getBundleFor(GadgetSpec spec, Locale locale, boolean ignoreCache)
       throws GadgetException {
-    MessageBundle parent = getParentBundle(spec, locale, ignoreCache);
-    MessageBundle child = null;
     LocaleSpec localeSpec = spec.getModulePrefs().getLocale(locale);
     if (localeSpec == null) {
-      return parent == null ? MessageBundle.EMPTY : parent;
-    }
-    Uri messages = localeSpec.getMessages();
-    if (messages == null || messages.toString().length() == 0) {
-      child = localeSpec.getMessageBundle();
-    } else {
-      child = fetchBundle(localeSpec, ignoreCache);
+      return MessageBundle.EMPTY;
     }
-    return new MessageBundle(parent, child);
-  }
 
-  private MessageBundle getParentBundle(GadgetSpec spec, Locale locale, boolean ignoreCache)
-      throws GadgetException {
-    if (locale.getLanguage().equalsIgnoreCase("all")) {
-      // Top most locale already.
-      return null;
+    if (localeSpec.getMessages().toString().length() == 0) {
+      return localeSpec.getMessageBundle();
     }
 
-    if (locale.getCountry().equalsIgnoreCase("ALL")) {
-      return getBundle(spec, ALL_ALL, ignoreCache);
-    }
+    LocaleQuery query = new LocaleQuery();
+    query.setSpecUri(localeSpec.getMessages())
+         .setGadgetUri(spec.getUrl())
+         // TODO: Get the real container that was used during the request here.
+         .setContainer(ContainerConfig.DEFAULT_CONTAINER)
+         .setIgnoreCache(ignoreCache);
+    query.locale = localeSpec;
 
-    return getBundle(spec, new Locale(locale.getLanguage(), "ALL"), ignoreCache);
+    return super.getSpec(query);
   }
 
-  protected MessageBundle fetchBundle(LocaleSpec locale, boolean ignoreCache)
-      throws GadgetException {
-    Uri url = locale.getMessages();
-    HttpRequest request = new HttpRequest(url).setIgnoreCache(ignoreCache);
-    // Since we don't allow any variance in cache time, we should just force the cache time
-    // globally. This ensures propagation to shared caches when this is set.
-    request.setCacheTtl((int) (refresh / 1000));
-
-    HttpResponse response = pipeline.execute(request);
-    if (response.getHttpStatusCode() != HttpResponse.SC_OK) {
-      throw new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
-          "Unable to retrieve message bundle xml. HTTP error " +
-          response.getHttpStatusCode());
-    }
-
-    return new MessageBundle(locale, response.getResponseAsString());
+  private static class LocaleQuery extends Query {
+    // We just use this to hold the locale used in the original query so that parsing can see it.
+    LocaleSpec locale;
   }
 }

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/MessageBundleFactory.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/MessageBundleFactory.java?rev=794588&r1=794587&r2=794588&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/MessageBundleFactory.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/MessageBundleFactory.java Thu Jul 16 08:26:57 2009
@@ -18,10 +18,11 @@
  */
 package org.apache.shindig.gadgets;
 
-import com.google.inject.ImplementedBy;
 import org.apache.shindig.gadgets.spec.GadgetSpec;
 import org.apache.shindig.gadgets.spec.MessageBundle;
 
+import com.google.inject.ImplementedBy;
+
 import java.util.Locale;
 
 /**
@@ -31,9 +32,10 @@
 public interface MessageBundleFactory {
   /**
    * Retrieves a messagMessageBundle for the provided GadgetSpec and Locale. Implementations must be
-   * sure to perform proper merging of message bundles of lower specifity with exact matches.
+   * sure to perform proper merging of message bundles of lower specifity with exact matches
+   * (exact > lang only > country only > all / all)
    *
-   * @param spec The gadet to inspect for Locales.
+   * @param spec The gadget to inspect for Locales.
    * @param locale The language and country to get a message bundle for.
    * @param ignoreCache  True to bypass any caching of message bundles for debugging purposes.
    * @return The newly created MesageBundle.

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java?rev=794588&r1=794587&r2=794588&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/spec/LocaleSpec.java Thu Jul 16 08:26:57 2009
@@ -21,6 +21,7 @@
 
 import org.w3c.dom.Element;
 
+import java.util.Locale;
 import java.util.Map;
 
 /**
@@ -33,6 +34,10 @@
  * No user pref substitution.
  */
 public class LocaleSpec {
+  private final Locale locale;
+  private final String languageDirection;
+  private final Uri messages;
+  private final MessageBundle messageBundle;
 
   /**
    * @param specUrl The url that the spec is loaded from. messages is assumed
@@ -40,18 +45,20 @@
    * @throws SpecParserException If language_direction is not valid
    */
   public LocaleSpec(Element element, Uri specUrl) throws SpecParserException {
-    language = XmlUtil.getAttribute(element, "lang", "all").toLowerCase();
-    country = XmlUtil.getAttribute(element, "country", "ALL").toUpperCase();
+    String language = XmlUtil.getAttribute(element, "lang", "all").toLowerCase();
+    String country = XmlUtil.getAttribute(element, "country", "ALL").toUpperCase();
+    this.locale = new Locale(language, country);
+
     languageDirection = XmlUtil.getAttribute(element, "language_direction", "ltr");
     if (!("ltr".equals(languageDirection) || "rtl".equals(languageDirection))) {
       throw new SpecParserException("Locale/@language_direction must be ltr or rtl");
     }
-    String messages = XmlUtil.getAttribute(element, "messages");
-    if (messages == null) {
+    String messagesString = XmlUtil.getAttribute(element, "messages");
+    if (messagesString == null) {
       this.messages = Uri.parse("");
     } else {
       try {
-        this.messages = specUrl.resolve(Uri.parse(messages));
+        this.messages = specUrl.resolve(Uri.parse(messagesString));
       } catch (IllegalArgumentException e) {
         throw new SpecParserException("Locale@messages url is invalid.");
       }
@@ -59,26 +66,27 @@
     messageBundle = new MessageBundle(element);
   }
 
+  public Locale getLocale() {
+    return locale;
+  }
+
   /**
    * Locale@lang
    */
-  private final String language;
   public String getLanguage() {
-    return language;
+    return locale.getLanguage();
   }
 
   /**
    * Locale@country
    */
-  private final String country;
   public String getCountry() {
-    return country;
+    return locale.getCountry();
   }
 
   /**
    * Locale@language_direction
    */
-  private final String languageDirection;
   public String getLanguageDirection() {
     return languageDirection;
   }
@@ -86,7 +94,6 @@
   /**
    * Locale@messages
    */
-  private final Uri messages;
   public Uri getMessages() {
     return messages;
   }
@@ -94,7 +101,6 @@
   /**
    * Locale/msg
    */
-  private final MessageBundle messageBundle;
   public MessageBundle getMessageBundle() {
     return messageBundle;
   }
@@ -103,8 +109,8 @@
   public String toString() {
     StringBuilder buf = new StringBuilder();
     buf.append("<Locale")
-       .append(" lang='").append(language).append('\'')
-       .append(" country='").append(country).append('\'')
+       .append(" lang='").append(getLanguage()).append('\'')
+       .append(" country='").append(getCountry()).append('\'')
        .append(" language_direction='").append(languageDirection).append('\'')
        .append(" messages='").append(messages).append("'>\n");
     for (Map.Entry<String, String> entry : messageBundle.getMessages().entrySet()) {

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=794588&r1=794587&r2=794588&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 Thu Jul 16 08:26:57 2009
@@ -17,15 +17,14 @@
  */
 package org.apache.shindig.gadgets.spec;
 
+import org.apache.shindig.common.JsonSerializer;
 import org.apache.shindig.common.xml.XmlException;
 import org.apache.shindig.common.xml.XmlUtil;
-
 import org.apache.shindig.gadgets.parse.DefaultHtmlSerializer;
 
-import com.google.common.collect.Maps;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 
-import org.json.JSONObject;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
@@ -38,13 +37,14 @@
  * Represents a messagebundle structure.
  */
 public class MessageBundle {
-
   public static final MessageBundle EMPTY = new MessageBundle();
 
   private static final DefaultHtmlSerializer htmlSerializer = new DefaultHtmlSerializer();
   private final ImmutableMap<String, String> messages;
   private final String languageDirection;
-  private final String jsonString;
+
+  /* lazily created cache of the json-encoded form of the bundle */
+  private String jsonString;
 
    /**
    * Constructs a message bundle from input xml (fetched from an external file).
@@ -62,7 +62,6 @@
           + ": " + e.getMessage());
     }
     messages = parseMessages(doc);
-    jsonString = new JSONObject(messages).toString();
     languageDirection = locale.getLanguageDirection();
   }
 
@@ -75,7 +74,6 @@
   public MessageBundle(LocaleSpec locale, Map<String, String> map) {
      messages = ImmutableMap.copyOf(map);
      languageDirection = locale.getLanguageDirection();
-     jsonString = new JSONObject(messages).toString();
    }
 
   /**
@@ -85,30 +83,22 @@
    */
   public MessageBundle(Element element) throws SpecParserException {
     messages = parseMessages(element);
-    jsonString = new JSONObject(messages).toString();
     languageDirection = XmlUtil.getAttribute(element, "language_direction", "ltr");
   }
 
   /**
-   * Create a MessageBundle by merging child messages into the parent.
+   * Create a MessageBundle by merging multiple bundles together.
    *
-   * @param parent The base bundle.
-   * @param child The bundle containing overriding messages.
+   * @param bundles the bundles to merge, in order
    */
-  public MessageBundle(MessageBundle parent, MessageBundle child) {
+  public MessageBundle(MessageBundle... bundles) {
     Map<String, String> merged = Maps.newHashMap();
     String dir = null;
-
-    if (parent != null) {
-      merged.putAll(parent.messages);
-      dir = parent.languageDirection;
-    }
-    if (child != null) {
-      merged.putAll(child.messages);
-      dir = child.languageDirection;
+    for (MessageBundle bundle : bundles) {
+      merged.putAll(bundle.messages);
+      dir = bundle.languageDirection;
     }
     messages = ImmutableMap.copyOf(merged);
-    jsonString = new JSONObject(messages).toString();
     languageDirection = dir;
   }
 
@@ -139,6 +129,9 @@
    * @return json representation of the message bundler
    */
   public String toJSONString() {
+    if (jsonString == null) {
+      jsonString = JsonSerializer.serialize(messages);
+    }
     return jsonString;
   }
 
@@ -172,7 +165,7 @@
             htmlSerializer.serialize(msgChildren.item(child), sw);
           }
         } catch (IOException e) {
-          throw new SpecParserException("Unexpected error getting value of msg node", 
+          throw new SpecParserException("Unexpected error getting value of msg node",
                                         new XmlException(e));
         }
       }

Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactoryTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactoryTest.java?rev=794588&r1=794587&r2=794588&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactoryTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactoryTest.java Thu Jul 16 08:26:57 2009
@@ -19,7 +19,6 @@
 package org.apache.shindig.gadgets;
 
 import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.isA;
 import static org.easymock.classextension.EasyMock.replay;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
@@ -29,13 +28,11 @@
 import org.apache.shindig.common.cache.SoftExpiringCache;
 import org.apache.shindig.common.testing.TestExecutorService;
 import org.apache.shindig.common.uri.Uri;
-import org.apache.shindig.common.xml.XmlUtil;
 import org.apache.shindig.config.ContainerConfig;
 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.http.RequestPipeline;
-import org.apache.shindig.gadgets.spec.ApplicationManifest;
 import org.apache.shindig.gadgets.spec.GadgetSpec;
 import org.apache.shindig.gadgets.spec.SpecParserException;
 
@@ -48,24 +45,9 @@
  */
 public class DefaultGadgetSpecFactoryTest {
   private static final Uri SPEC_URL = Uri.parse("http://example.org/gadget.xml");
-  private static final Uri ALT_SPEC_URL = Uri.parse("http://example.org/gadget2.xml");
-  private static final Uri MANIFEST_URI = Uri.parse("http://example.org/manifest.xml");
   private static final String LOCAL_CONTENT = "Hello, local content!";
   private static final String ALT_LOCAL_CONTENT = "Hello, local content!";
   private static final String RAWXML_CONTENT = "Hello, rawxml content!";
-  private static final String MANIFEST_XML
-      = "<app xmlns='" + ApplicationManifest.NAMESPACE + "'>" +
-        "<gadget>" +
-        "  <label>production</label>" +
-        "  <version>1.0</version>" +
-        "  <spec>" + SPEC_URL + "</spec>" +
-        "</gadget>" +
-        "<gadget>" +
-        "  <label>development</label>" +
-        "  <version>2.0</version>" +
-        "  <spec>" + ALT_SPEC_URL + "</spec>" +
-        "</gadget>" +
-        "</app>";
   private static final String LOCAL_SPEC_XML
       = "<Module>" +
         "  <ModulePrefs title='GadgetSpecFactoryTest'/>" +
@@ -153,75 +135,7 @@
     assertEquals(LOCAL_CONTENT, spec.getView(GadgetSpec.DEFAULT_VIEW).getContent());
   }
 
-  @Test
-  public void specFetchedFromManifest() throws Exception {
-    HttpRequest gadgetRequest = createIgnoreCacheRequest();
-    HttpResponse gadgetResponse = new HttpResponse(LOCAL_SPEC_XML);
-    expect(pipeline.execute(gadgetRequest)).andReturn(gadgetResponse);
-
-    HttpRequest manifestRequest = createIgnoreCacheRequest();
-    manifestRequest.setUri(MANIFEST_URI);
-    HttpResponse manifestResponse = new HttpResponse(MANIFEST_XML);
-    expect(pipeline.execute(manifestRequest)).andReturn(manifestResponse);
-
-    replay(pipeline);
-
-    GadgetSpec spec = specFactory.getGadgetSpec(createContext(MANIFEST_URI, true));
-
-    assertEquals(LOCAL_CONTENT, spec.getView(GadgetSpec.DEFAULT_VIEW).getContent());
-  }
-
-  @Test(expected = SpecParserException.class)
-  public void nestedManifestThrows() throws Exception {
-    HttpResponse manifestResponse = new HttpResponse(MANIFEST_XML);
-    expect(pipeline.execute(isA(HttpRequest.class))).andReturn(manifestResponse).anyTimes();
-
-    replay(pipeline);
-
-    specFactory.getGadgetSpec(createContext(MANIFEST_URI, true));
-  }
-
-  @Test
-  public void manifestFetchedWithDefaults() throws Exception {
-    ApplicationManifest manifest
-        = new ApplicationManifest(MANIFEST_URI, XmlUtil.parse(MANIFEST_XML));
-    specFactory.cache.addElement(MANIFEST_URI, manifest, 1000);
-
-    GadgetSpec cachedSpec = new GadgetSpec(SPEC_URL, LOCAL_SPEC_XML);
-    specFactory.cache.addElement(SPEC_URL, cachedSpec, 1000);
-
-    GadgetSpec spec = specFactory.getGadgetSpec(createContext(MANIFEST_URI, false));
-
-    assertEquals(LOCAL_CONTENT, spec.getView(GadgetSpec.DEFAULT_VIEW).getContent());
-  }
-
-  @Test
-  public void manifestFetchedByVersion() throws Exception {
-    ApplicationManifest manifest
-        = new ApplicationManifest(MANIFEST_URI, XmlUtil.parse(MANIFEST_XML));
-    specFactory.cache.addElement(MANIFEST_URI, manifest, 1000);
-
-    GadgetSpec cachedSpec = new GadgetSpec(ALT_SPEC_URL, ALT_LOCAL_SPEC_XML);
-    specFactory.cache.addElement(ALT_SPEC_URL, cachedSpec, 1000);
-
-    GadgetSpec spec = specFactory.getGadgetSpec(new GadgetContext() {
-      @Override
-      public Uri getUrl() {
-        return MANIFEST_URI;
-      }
-
-      @Override
-      public String getParameter(String name) {
-        if (name.equals(DefaultGadgetSpecFactory.VERSION_PARAM)) {
-          return "2.0";
-        }
-        return null;
-      }
-    });
-
-    assertEquals(ALT_LOCAL_CONTENT, spec.getView(GadgetSpec.DEFAULT_VIEW).getContent());
-  }
-
+  // TODO: Move these tests into AbstractSpecFactoryTest
   @Test
   public void specRefetchedAsync() throws Exception {
     HttpRequest request = createCacheableRequest();

Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java?rev=794588&r1=794587&r2=794588&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/DefaultMessageBundleFactoryTest.java Thu Jul 16 08:26:57 2009
@@ -23,11 +23,12 @@
 import static org.easymock.EasyMock.verify;
 import static org.easymock.classextension.EasyMock.replay;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertNull;
 
 import org.apache.shindig.common.cache.Cache;
 import org.apache.shindig.common.cache.CacheProvider;
 import org.apache.shindig.common.cache.LruCacheProvider;
+import org.apache.shindig.common.testing.TestExecutorService;
 import org.apache.shindig.common.uri.Uri;
 import org.apache.shindig.common.util.TimeSource;
 import org.apache.shindig.gadgets.http.HttpRequest;
@@ -53,12 +54,17 @@
   private static final String MSG_0_NAME = "messageZero";
   private static final String MSG_1_NAME = "message1";
   private static final String MSG_2_NAME = "message 2";
+  private static final String MSG_3_NAME = "message 3";
   private static final String MSG_0_VALUE = "Message 0 VALUE";
-  private static final String MSG_0_ALT_VALUE = "Message 0 Alternative VALUE";
+  private static final String MSG_0_LANG_VALUE = "Message 0 language VALUE";
+  private static final String MSG_0_COUNTRY_VALUE = "Message 0 country VALUE";
+  private static final String MSG_0_ALL_VALUE = "Message 0 a VALUE";
   private static final String MSG_1_VALUE = "msg one val";
   private static final String MSG_2_VALUE = "message two val.";
+  private static final String MSG_3_VALUE = "message three value";
 
-  private static final Locale PARENT_LOCALE = new Locale("en", "ALL");
+  private static final Locale COUNTRY_LOCALE = new Locale("all", "US");
+  private static final Locale LANG_LOCALE = new Locale("en", "ALL");
   private static final Locale LOCALE = new Locale("en", "US");
 
   private static final String BASIC_BUNDLE
@@ -71,10 +77,14 @@
       = "<Module>" +
         "<ModulePrefs title='foo'>" +
         " <Locale lang='all' country='ALL'>" +
-        "  <msg name='" + MSG_0_NAME + "'>" + MSG_0_VALUE + "</msg>" +
+        "  <msg name='" + MSG_0_NAME + "'>" + MSG_0_ALL_VALUE + "</msg>" +
+        " </Locale>" +
+        " <Locale lang='all' country='" + LOCALE.getCountry() + "'>" +
+        "  <msg name='" + MSG_0_NAME + "'>" + MSG_0_COUNTRY_VALUE + "</msg>" +
+        "  <msg name='" + MSG_3_NAME + "'>" + MSG_3_VALUE + "</msg>" +
         " </Locale>" +
         " <Locale lang='" + LOCALE.getLanguage() + "'>" +
-        "  <msg name='" + MSG_0_NAME + "'>" + MSG_0_ALT_VALUE + "</msg>" +
+        "  <msg name='" + MSG_0_NAME + "'>" + MSG_0_LANG_VALUE + "</msg>" +
         "  <msg name='" + MSG_1_NAME + "'>" + MSG_1_VALUE + "</msg>" +
         "  <msg name='" + MSG_2_NAME + "'>" + MSG_2_VALUE + "</msg>" +
         " </Locale>" +
@@ -91,7 +101,7 @@
   private final Cache<String, MessageBundle> cache
       = cacheProvider.createCache(DefaultMessageBundleFactory.CACHE_NAME);
   private final DefaultMessageBundleFactory bundleFactory
-      = new DefaultMessageBundleFactory(pipeline, cacheProvider, MAX_AGE);
+      = new DefaultMessageBundleFactory(new TestExecutorService(), pipeline, cacheProvider, MAX_AGE);
   private final GadgetSpec gadgetSpec;
 
   public DefaultMessageBundleFactoryTest() {
@@ -104,7 +114,7 @@
 
 
   @Test
-  public void getBundle() throws Exception {
+  public void getExactBundle() throws Exception {
     HttpResponse response = new HttpResponse(BASIC_BUNDLE);
     expect(pipeline.execute(isA(HttpRequest.class))).andReturn(response);
     replay(pipeline);
@@ -114,33 +124,50 @@
     assertEquals(MSG_0_VALUE, bundle.getMessages().get(MSG_0_NAME));
     assertEquals(MSG_1_VALUE, bundle.getMessages().get(MSG_1_NAME));
     assertEquals(MSG_2_VALUE, bundle.getMessages().get(MSG_2_NAME));
+    assertEquals(MSG_3_VALUE, bundle.getMessages().get(MSG_3_NAME));
   }
 
   @Test
-  public void getBundleFromCache() throws Exception {
-    HttpResponse response = new HttpResponse(BASIC_BUNDLE);
-    expect(pipeline.execute(isA(HttpRequest.class))).andReturn(response).once();
-    replay(pipeline);
-
-    MessageBundle bundle0 = bundleFactory.getBundle(gadgetSpec, LOCALE, false);
-    MessageBundle bundle1 = bundleFactory.getBundle(gadgetSpec, LOCALE, false);
+  public void getLangBundle() throws Exception {
+    MessageBundle bundle = bundleFactory.getBundle(gadgetSpec, LANG_LOCALE, true);
 
-    assertSame("Different objects returned out of the cache", bundle0, bundle1);
+    assertEquals(MSG_0_LANG_VALUE, bundle.getMessages().get(MSG_0_NAME));
+    assertEquals(MSG_1_VALUE, bundle.getMessages().get(MSG_1_NAME));
+    assertEquals(MSG_2_VALUE, bundle.getMessages().get(MSG_2_NAME));
+    assertNull(bundle.getMessages().get(MSG_3_NAME));
   }
 
   @Test
-  public void getParentBundle() throws Exception {
-    MessageBundle bundle = bundleFactory.getBundle(gadgetSpec, PARENT_LOCALE, true);
+  public void getCountryBundle() throws Exception {
+    MessageBundle bundle = bundleFactory.getBundle(gadgetSpec, COUNTRY_LOCALE, true);
 
-    assertEquals(MSG_0_ALT_VALUE, bundle.getMessages().get(MSG_0_NAME));
-    assertEquals(MSG_1_VALUE, bundle.getMessages().get(MSG_1_NAME));
-    assertEquals(MSG_2_VALUE, bundle.getMessages().get(MSG_2_NAME));
+    assertEquals(MSG_0_COUNTRY_VALUE, bundle.getMessages().get(MSG_0_NAME));
+    assertNull(bundle.getMessages().get(MSG_1_NAME));
+    assertNull(bundle.getMessages().get(MSG_2_NAME));
+    assertEquals(MSG_3_VALUE, bundle.getMessages().get(MSG_3_NAME));
   }
 
   @Test
   public void getAllAllBundle() throws Exception {
     MessageBundle bundle = bundleFactory.getBundle(gadgetSpec, new Locale("all", "ALL"), true);
-    assertEquals(MSG_0_VALUE, bundle.getMessages().get(MSG_0_NAME));
+    assertEquals(MSG_0_ALL_VALUE, bundle.getMessages().get(MSG_0_NAME));
+    assertNull(bundle.getMessages().get(MSG_1_NAME));
+    assertNull(bundle.getMessages().get(MSG_2_NAME));
+    assertNull(bundle.getMessages().get(MSG_3_NAME));
+  }
+
+  @Test
+  public void getBundleFromCache() throws Exception {
+    HttpResponse response = new HttpResponse(BASIC_BUNDLE);
+    expect(pipeline.execute(isA(HttpRequest.class))).andReturn(response).once();
+    replay(pipeline);
+
+    MessageBundle bundle0 = bundleFactory.getBundle(gadgetSpec, LOCALE, false);
+    MessageBundle bundle1 = bundleFactory.getBundle(gadgetSpec, LOCALE, false);
+
+    verify(pipeline);
+
+    assertEquals(bundle0.getMessages().get(MSG_0_NAME), bundle1.getMessages().get(MSG_0_NAME));
   }
 
   @Test
@@ -182,30 +209,26 @@
 
     verify(pipeline);
 
-    assertSame("Did not respond from cache when refresh failed.", bundle0, bundle1);
+    assertEquals(bundle0.getMessages().get(MSG_0_NAME), bundle1.getMessages().get(MSG_0_NAME));
   }
 
-  @Test
-  public void badResponseIsEmptyWhenNotInCache() throws Exception {
+  @Test(expected=GadgetException.class)
+  public void badResponsePropagatesException() throws Exception {
     HttpResponse badResponse = HttpResponse.error();
 
     expect(pipeline.execute(isA(HttpRequest.class)))
         .andReturn(badResponse).once();
     replay(pipeline);
 
-    MessageBundle bundle = bundleFactory.getBundle(gadgetSpec, LOCALE, false);
-
-    verify(pipeline);
-
-    assertEquals(0, bundle.getMessages().size());
+    bundleFactory.getBundle(gadgetSpec, LOCALE, false);
   }
 
   @Test
   public void ttlPropagatesToFetcher() throws Exception {
     CapturingFetcher capturingFetcher = new CapturingFetcher();
 
-    MessageBundleFactory factory
-        = new DefaultMessageBundleFactory(capturingFetcher, cacheProvider, MAX_AGE);
+    MessageBundleFactory factory = new DefaultMessageBundleFactory(
+        new TestExecutorService(), capturingFetcher, cacheProvider, MAX_AGE);
 
     factory.getBundle(gadgetSpec, LOCALE, false);
 
@@ -223,6 +246,6 @@
       return new HttpResponse(BASIC_BUNDLE);
     }
 
-    public void normalizeProtocol(HttpRequest request) throws GadgetException { }
+    public void normalizeProtocol(HttpRequest request) { }
   }
 }