Author: mhermanto
Date: Wed May 18 22:17:39 2011
New Revision: 1124452
URL: http://svn.apache.org/viewvc?rev=1124452&view=rev
Log:
New JS pipeline now respects/ignores unknown feature.
http://codereview.appspot.com/4515109/
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/GetJsContentProcessor.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessorTest.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/GetJsContentProcessorTest.java
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java?rev=1124452&r1=1124451&r2=1124452&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessor.java
Wed May 18 22:17:39 2011
@@ -19,8 +19,6 @@
package org.apache.shindig.gadgets.js;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Joiner;
-import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.inject.Inject;
@@ -28,11 +26,9 @@ import org.apache.commons.lang.StringEsc
import org.apache.shindig.gadgets.GadgetContext;
import org.apache.shindig.gadgets.features.FeatureRegistry;
import org.apache.shindig.gadgets.features.FeatureRegistry.FeatureBundle;
-import org.apache.shindig.gadgets.http.HttpResponse;
import org.apache.shindig.gadgets.uri.JsUriManager.JsUri;
import java.util.Collection;
-import java.util.List;
import java.util.Set;
/**
@@ -42,7 +38,6 @@ import java.util.Set;
*/
public class AddJslLoadedVariableProcessor implements JsProcessor {
private static final String CODE_ID = "[jsload-loaded-info]";
- private static final Joiner UNKNOWN_FEATURE_ERR = Joiner.on(", ");
@VisibleForTesting
static final String TEMPLATE =
@@ -55,7 +50,7 @@ public class AddJslLoadedVariableProcess
this.registry = featureRegistry;
}
- public boolean process(JsRequest jsRequest, JsResponseBuilder builder)
throws JsException {
+ public boolean process(JsRequest jsRequest, JsResponseBuilder builder) {
JsUri jsUri = jsRequest.getJsUri();
if (!jsUri.isNohint()) {
Set<String> result = getBundleNames(jsUri, false);
@@ -67,16 +62,11 @@ public class AddJslLoadedVariableProcess
}
// TODO: factor this logic into somewhere shared. it's now used in
GetJsContentProcessor.
- private Set<String> getBundleNames(JsUri jsUri, boolean loaded) throws
JsException {
+ private Set<String> getBundleNames(JsUri jsUri, boolean loaded) {
GadgetContext ctx = new JsGadgetContext(jsUri);
- Collection<String> libs = loaded ? jsUri.getLoadedLibs() :
jsUri.getLibs();
- List<String> unsupported = Lists.newLinkedList();
- FeatureRegistry.LookupResult lookup = registry.getFeatureResources(ctx,
libs, unsupported);
- if (!unsupported.isEmpty()) {
- String message = loaded ? "loaded" : "requested";
- throw new JsException(HttpResponse.SC_BAD_REQUEST,
- "Unknown " + message + " feature(s): " +
UNKNOWN_FEATURE_ERR.join(unsupported));
- }
+ Collection<String> libs = loaded ? jsUri.getLoadedLibs() : jsUri.getLibs();
+ // TODO: possibly warn on unknown/unrecognized libs.
+ FeatureRegistry.LookupResult lookup = registry.getFeatureResources(ctx,
libs, null);
Set<String> ret = Sets.newLinkedHashSet(); // ordered set for testability.
for (FeatureBundle bundle : lookup.getBundles()) {
ret.add(bundle.getName());
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/GetJsContentProcessor.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/GetJsContentProcessor.java?rev=1124452&r1=1124451&r2=1124452&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/GetJsContentProcessor.java
(original)
+++
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/GetJsContentProcessor.java
Wed May 18 22:17:39 2011
@@ -18,8 +18,6 @@
package org.apache.shindig.gadgets.js;
-import com.google.common.base.Joiner;
-import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.inject.Inject;
@@ -30,12 +28,10 @@ import org.apache.shindig.gadgets.featur
import org.apache.shindig.gadgets.features.FeatureRegistry.FeatureBundle;
import org.apache.shindig.gadgets.features.FeatureRegistryProvider;
import org.apache.shindig.gadgets.features.FeatureResource;
-import org.apache.shindig.gadgets.http.HttpResponse;
import org.apache.shindig.gadgets.rewrite.js.JsCompiler;
import org.apache.shindig.gadgets.uri.JsUriManager.JsUri;
import org.apache.shindig.gadgets.uri.UriStatus;
-import java.util.Collection;
import java.util.List;
import java.util.Set;
@@ -43,8 +39,6 @@ import java.util.Set;
* Retrieves the requested Javascript code using a {@link JsProcessor}.
*/
public class GetJsContentProcessor implements JsProcessor {
- private static final Joiner UNKNOWN_FEATURE_ERR = Joiner.on(", ");
-
private final FeatureRegistryProvider registryProvider;
private final JsCompiler compiler;
@@ -67,8 +61,12 @@ public class GetJsContentProcessor imple
} catch (GadgetException e) {
throw new JsException(e.getHttpStatusCode(), e.getMessage());
}
- List<FeatureBundle> requestedBundles = getAllBundles(registry, ctx,
jsUri.getLibs(), false);
- List<FeatureBundle> loadedBundles = getAllBundles(registry, ctx,
jsUri.getLoadedLibs(), true);
+
+ // TODO: possibly warn on unknown/unrecognized libs.
+ List<FeatureBundle> requestedBundles = registry.getFeatureResources(
+ ctx, jsUri.getLibs(), null).getBundles();
+ List<FeatureBundle> loadedBundles = registry.getFeatureResources(
+ ctx, jsUri.getLoadedLibs(), null).getBundles();
Set<String> loadedFeatures = Sets.newHashSet();
for (FeatureBundle bundle : loadedBundles) {
@@ -97,18 +95,6 @@ public class GetJsContentProcessor imple
return true;
}
- private List<FeatureBundle> getAllBundles(FeatureRegistry registry,
GadgetContext ctx,
- Collection<String> requested, boolean loaded) throws JsException {
- List<String> unsupported = Lists.newLinkedList();
- FeatureRegistry.LookupResult lookup = registry.getFeatureResources(ctx,
requested, unsupported);
- if (!unsupported.isEmpty()) {
- String message = loaded ? "loaded" : "requested";
- throw new JsException(HttpResponse.SC_BAD_REQUEST,
- "Unknown " + message + " feature(s): " +
UNKNOWN_FEATURE_ERR.join(unsupported));
- }
- return lookup.getBundles();
- }
-
/**
* Sets the cache TTL depending on the value of the {@link UriStatus} object.
*
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessorTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessorTest.java?rev=1124452&r1=1124451&r2=1124452&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessorTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AddJslLoadedVariableProcessorTest.java
Wed May 18 22:17:39 2011
@@ -21,6 +21,7 @@ package org.apache.shindig.gadgets.js;
import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.isA;
+import static org.easymock.EasyMock.isNull;
import static org.junit.Assert.assertEquals;
import com.google.common.collect.ImmutableList;
@@ -43,8 +44,6 @@ import java.util.List;
* Tests for {@link AddJslLoadedVariableProcessor}.
*/
public class AddJslLoadedVariableProcessorTest {
- private static final List<String> EMPTY_STRING_LIST =
ImmutableList.<String>of();
-
private static final String REQ_1_LIB = "foo";
private static final String REQ_1_DEP_LIB = "bar";
private static final String REQ_2_LIB = "gig";
@@ -96,6 +95,7 @@ public class AddJslLoadedVariableProcess
EasyMock.expect(request.getJsUri()).andReturn(jsUri);
}
+ @SuppressWarnings("unchecked")
private void setUpRegistry() {
FeatureBundle bundle1 = mockBundle(REQ_1_LIB);
FeatureBundle bundle2 = mockBundle(REQ_1_DEP_LIB);
@@ -103,10 +103,10 @@ public class AddJslLoadedVariableProcess
FeatureBundle bundle4 = mockBundle(LOAD_LIB);
LookupResult reqResult = mockLookupResult(Lists.newArrayList(bundle1,
bundle2, bundle3));
expect(registry.getFeatureResources(isA(JsGadgetContext.class),
eq(REQ_LIBS),
- eq(EMPTY_STRING_LIST))).andReturn(reqResult);
+ isNull(List.class))).andReturn(reqResult);
LookupResult loadResult = mockLookupResult(Lists.newArrayList(bundle4));
expect(registry.getFeatureResources(isA(JsGadgetContext.class),
eq(LOAD_LIBS),
- eq(EMPTY_STRING_LIST))).andReturn(loadResult);
+ isNull(List.class))).andReturn(loadResult);
}
private LookupResult mockLookupResult(List<FeatureBundle> bundles) {
Modified:
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/GetJsContentProcessorTest.java
URL:
http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/GetJsContentProcessorTest.java?rev=1124452&r1=1124451&r2=1124452&view=diff
==============================================================================
---
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/GetJsContentProcessorTest.java
(original)
+++
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/GetJsContentProcessorTest.java
Wed May 18 22:17:39 2011
@@ -22,6 +22,7 @@ import static org.easymock.EasyMock.crea
import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.isA;
+import static org.easymock.EasyMock.isNull;
import static org.junit.Assert.*;
import java.util.List;
@@ -151,6 +152,7 @@ public class GetJsContentProcessorTest {
JsContent.fromFeature(JS_CODE2, "source2", bundle, resource2)));
}
+ @SuppressWarnings("unchecked")
private void setupJsUriAndRegistry(UriStatus uriStatus,
List<String> reqLibs, List<FeatureBundle> reqLookupBundles,
List<String> loadLibs, List<FeatureBundle> loadLookupBundles) {
@@ -168,9 +170,9 @@ public class GetJsContentProcessorTest {
LookupResult loadLookup = mockLookupResult(loadLookupBundles);
expect(registry.getFeatureResources(isA(JsGadgetContext.class),
eq(reqLibs),
- eq(EMPTY_STRING_LIST))).andReturn(reqLookup);
+ isNull(List.class))).andReturn(reqLookup);
expect(registry.getFeatureResources(isA(JsGadgetContext.class),
eq(loadLibs),
- eq(EMPTY_STRING_LIST))).andReturn(loadLookup);
+ isNull(List.class))).andReturn(loadLookup);
}
private LookupResult mockLookupResult(List<FeatureBundle> bundles) {
