You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by jo...@apache.org on 2011/04/01 20:17:23 UTC

svn commit: r1087837 - in /shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/js/ main/java16/org/apache/shindig/gadgets/js/ main/java16/org/apache/shindig/gadgets/rewrite/js/ test/java/org/apache/shindig/gadgets/js/

Author: johnh
Date: Fri Apr  1 18:17:22 2011
New Revision: 1087837

URL: http://svn.apache.org/viewvc?rev=1087837&view=rev
Log:
2 part CL:

1. Closure Compiler by default evidently does not (always?) export
common externs, ie. dom and other APIs. These are all provided in
Closure Compilers' JARs, and per authors are canonically loaded by the
command line runner (here, used as a helper method). These are added as
externs for all compilation to prevent things such as document.location,
String.methods, Array.methods, Object.methods and so on from undue
obfuscation.

2. Exports also pulled in from features' <api><uses...> directives.


Modified:
    shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java
    shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/js/JsCompilerModule.java
    shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java
    shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java

Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java?rev=1087837&r1=1087836&r2=1087837&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/CompilationProcessor.java Fri Apr  1 18:17:22 2011
@@ -17,33 +17,64 @@
  */
 package org.apache.shindig.gadgets.js;
 
-import java.util.List;
+import com.google.common.collect.Lists;
 
+import org.apache.shindig.gadgets.GadgetContext;
+import org.apache.shindig.gadgets.RenderingContext;
+import org.apache.shindig.gadgets.features.ApiDirective;
+import org.apache.shindig.gadgets.features.FeatureRegistry;
+import org.apache.shindig.gadgets.features.FeatureRegistry.FeatureBundle;
+import org.apache.shindig.gadgets.features.FeatureRegistry.LookupResult;
 import org.apache.shindig.gadgets.rewrite.js.JsCompiler;
 
-import com.google.common.collect.ImmutableList;
 import com.google.inject.Inject;
 
+import java.util.List;
+
 public class CompilationProcessor implements JsProcessor {
   private final JsCompiler compiler;
+  private final FeatureRegistry registry;
   
   @Inject
-  public CompilationProcessor(JsCompiler compiler) {
+  public CompilationProcessor(JsCompiler compiler, FeatureRegistry registry) {
     this.compiler = compiler;
+    this.registry = registry;
   }
       
   /**
    * Compile content in the inbound JsResponseBuilder.
-   * TODO: Re-add support for externs here if they're ever used.
    * TODO: Convert JsCompiler to take JsResponseBuilder directly rather than Iterable<JsContent>
    */
   public boolean process(JsRequest request, JsResponseBuilder builder)
       throws JsException {
-    List<String> externs = ImmutableList.of();
+    List<String> featureExterns = getFeatureExterns(builder.build().getAllJsContent(), request);
     JsResponse result = compiler.compile(request.getJsUri(),
-        builder.build().getAllJsContent(), externs);
+        builder.build().getAllJsContent(), featureExterns);
     builder.clearJs().appendAllJs(result.getAllJsContent());
     return true;
   }
 
+  private List<String> getFeatureExterns(Iterable<JsContent> content, final JsRequest request) {
+    List<String> result = Lists.newArrayList();
+    List<String> featureNames = Lists.newArrayList();
+    for (JsContent js : content) {
+      if (js.getFeature() != null) {
+        featureNames.add(js.getFeature());
+      }
+    }
+    LookupResult lookup = registry.getFeatureResources(new GadgetContext() {
+      @Override
+      public RenderingContext getRenderingContext() {
+        return request.getJsUri().getContext();
+      }
+      @Override
+      public String getContainer() {
+        return request.getJsUri().getContainer();
+      }
+    }, featureNames, Lists.<String>newArrayList());
+    for (FeatureBundle bundle : lookup.getBundles()) {
+      result.addAll(bundle.getApis(ApiDirective.Type.JS, false));
+    }
+    return result;
+  }
 }

Modified: shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/js/JsCompilerModule.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/js/JsCompilerModule.java?rev=1087837&r1=1087836&r2=1087837&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/js/JsCompilerModule.java (original)
+++ shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/js/JsCompilerModule.java Fri Apr  1 18:17:22 2011
@@ -21,11 +21,9 @@ import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
 import com.google.inject.Provides;
 
-import org.apache.shindig.gadgets.rewrite.js.ExportJsCompiler;
+import org.apache.shindig.gadgets.rewrite.js.ClosureJsCompiler;
 import org.apache.shindig.gadgets.rewrite.js.JsCompiler;
 
-import java.util.List;
-
 /**
  * Guice configuration for JS compilation.
  */
@@ -38,7 +36,7 @@ public class JsCompilerModule extends Ab
 
   @Provides
   @Inject
-  public JsCompiler provideJsCompiler(ExportJsCompiler compiler) {
+  public JsCompiler provideJsCompiler(ClosureJsCompiler compiler) {
     return compiler;
   }
   

Modified: shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java?rev=1087837&r1=1087836&r2=1087837&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java (original)
+++ shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java Fri Apr  1 18:17:22 2011
@@ -25,6 +25,7 @@ import com.google.common.collect.Maps;
 import com.google.inject.Inject;
 import com.google.javascript.jscomp.BasicErrorManager;
 import com.google.javascript.jscomp.CheckLevel;
+import com.google.javascript.jscomp.CommandLineRunner;
 import com.google.javascript.jscomp.CompilationLevel;
 import com.google.javascript.jscomp.Compiler;
 import com.google.javascript.jscomp.CompilerOptions;
@@ -67,6 +68,16 @@ public class ClosureJsCompiler implement
               + "for(;parts.length&&(part=parts.shift());){if(!parts.length){"
               + "cur[part]=obj;}else{cur=cur[part]||(cur[part]={})}}};", "[goog.exportSymbol]");
   
+  private static final List<JSSourceFile> DEFAULT_EXTERNS;
+  
+  static {
+    try {
+      DEFAULT_EXTERNS = CommandLineRunner.getDefaultExterns();
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+  
   @VisibleForTesting
   static final String CACHE_NAME = "CompiledJs";
 
@@ -148,6 +159,7 @@ public class ClosureJsCompiler implement
     if (!jsUri.isDebug() || options.isExternExportsEnabled()) {
       List<JSSourceFile> allExterns = Lists.newArrayList();
       allExterns.add(JSSourceFile.fromCode("externs", externStr));
+      allExterns.addAll(DEFAULT_EXTERNS);
 
       List<JsContent> allContent = Lists.newLinkedList(content);
       if (options.isExternExportsEnabled()) {

Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java?rev=1087837&r1=1087836&r2=1087837&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/CompilationProcessorTest.java Fri Apr  1 18:17:22 2011
@@ -20,6 +20,7 @@ package org.apache.shindig.gadgets.js;
 import static org.easymock.EasyMock.createControl;
 import static org.easymock.EasyMock.eq;
 import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.isA;
 import static org.easymock.EasyMock.same;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -28,6 +29,12 @@ import static org.junit.Assert.assertTru
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.shindig.gadgets.GadgetContext;
+import org.apache.shindig.gadgets.RenderingContext;
+import org.apache.shindig.gadgets.features.ApiDirective;
+import org.apache.shindig.gadgets.features.FeatureRegistry;
+import org.apache.shindig.gadgets.features.FeatureRegistry.FeatureBundle;
+import org.apache.shindig.gadgets.features.FeatureRegistry.LookupResult;
 import org.apache.shindig.gadgets.rewrite.js.JsCompiler;
 import org.apache.shindig.gadgets.uri.JsUriManager.JsUri;
 import org.easymock.IMocksControl;
@@ -39,26 +46,35 @@ import com.google.common.collect.Immutab
 public class CompilationProcessorTest {
   private IMocksControl control;
   private JsCompiler compiler;
+  private FeatureRegistry registry;
   private CompilationProcessor processor;
   
   @Before
   public void setUp() throws Exception {
     control = createControl();
     compiler = control.createMock(JsCompiler.class);
-    processor = new CompilationProcessor(compiler);
+    registry = control.createMock(FeatureRegistry.class);
+    processor = new CompilationProcessor(compiler, registry);
   }
   
   @Test
   public void compilerIsRun() throws Exception {
     JsUri jsUri = control.createMock(JsUri.class);
+    LookupResult lookupResult = control.createMock(LookupResult.class);
+    FeatureBundle bundle = control.createMock(FeatureBundle.class);
+    ImmutableList<String> externs = ImmutableList.of("e1", "e2");
+    expect(bundle.getApis(ApiDirective.Type.JS, false)).andReturn(externs);
+    expect(lookupResult.getBundles()).andReturn(ImmutableList.<FeatureBundle>of(bundle));
+    expect(registry.getFeatureResources(isA(GadgetContext.class), eq(ImmutableList.of("f1", "f2")),
+        eq(ImmutableList.<String>of()))).andReturn(lookupResult);
     JsResponseBuilder builder =
         new JsResponseBuilder().setCacheTtlSecs(1234).setStatusCode(200)
-          .appendJs("content1:", "source1").appendJs("content2", "source2");
+          .appendJs(JsContent.fromFeature("content1:", "source1", "f1", null))
+          .appendJs(JsContent.fromFeature("content2", "source2", "f2", null));
     JsResponse outputResponse = new JsResponseBuilder().appendJs("content3", "s3").build();
     JsRequest request = control.createMock(JsRequest.class);
     expect(request.getJsUri()).andReturn(jsUri);
-    List<String> emptyList = ImmutableList.of();
-    expect(compiler.compile(same(jsUri), eq(builder.build().getAllJsContent()), eq(emptyList)))
+    expect(compiler.compile(same(jsUri), eq(builder.build().getAllJsContent()), eq(externs)))
         .andReturn(outputResponse);
     
     control.replay();
@@ -80,13 +96,19 @@ public class CompilationProcessorTest {
   @Test(expected = JsException.class)
   public void compilerExceptionThrows() throws Exception {
     JsUri jsUri = control.createMock(JsUri.class);
+    LookupResult lookupResult = control.createMock(LookupResult.class);
+    FeatureBundle bundle = control.createMock(FeatureBundle.class);
+    ImmutableList<String> externs = ImmutableList.of();
+    expect(bundle.getApis(ApiDirective.Type.JS, false)).andReturn(externs);
+    expect(lookupResult.getBundles()).andReturn(ImmutableList.<FeatureBundle>of(bundle));
+    expect(registry.getFeatureResources(isA(GadgetContext.class), eq(ImmutableList.of("f1")),
+        eq(ImmutableList.<String>of()))).andReturn(lookupResult);
     JsResponseBuilder builder =
         new JsResponseBuilder().setCacheTtlSecs(1234).setStatusCode(200)
-          .appendJs("content1:", "source1").appendJs("content2", "source2");
+          .appendJs(JsContent.fromFeature("content1:", "source1", "f1", null));
     JsRequest request = control.createMock(JsRequest.class);
     expect(request.getJsUri()).andReturn(jsUri);
-    List<String> emptyList = ImmutableList.of();
-    expect(compiler.compile(same(jsUri), eq(builder.build().getAllJsContent()), eq(emptyList)))
+    expect(compiler.compile(same(jsUri), eq(builder.build().getAllJsContent()), eq(externs)))
         .andThrow(new JsException(400, "foo"));
     control.replay();
     processor.process(request, builder);