Updated Branches:
  refs/heads/master 1dd33e198 -> b91c11a15

Use a Dispatcher, not an AssetRequestHandler, to process module requests


Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/b91c11a1
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/b91c11a1
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/b91c11a1

Branch: refs/heads/master
Commit: b91c11a15c298a2bdacd6c143df303dde28e456d
Parents: 9c545d9
Author: Howard M. Lewis Ship <[email protected]>
Authored: Tue Jan 22 10:30:42 2013 -0800
Committer: Howard M. Lewis Ship <[email protected]>
Committed: Tue Jan 22 10:30:55 2013 -0800

----------------------------------------------------------------------
 .../javascript/ModuleAssetRequestHandler.java      |   90 ------------
 .../services/javascript/ModuleDispatcher.java      |  105 +++++++++++++++
 .../services/javascript/ModuleManagerImpl.java     |   11 +-
 .../tapestry5/internal/util/VirtualResource.java   |    4 +-
 .../apache/tapestry5/services/TapestryModule.java  |    3 +-
 .../services/javascript/JavaScriptModule.java      |    8 +-
 .../ModuleAssetRequestHandlerTest.groovy           |   29 +++--
 .../javascript/ModuleDispatcherTests.groovy        |   65 +++++++++
 8 files changed, 205 insertions(+), 110 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b91c11a1/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleAssetRequestHandler.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleAssetRequestHandler.java
 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleAssetRequestHandler.java
deleted file mode 100644
index 1d6a1b7..0000000
--- 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleAssetRequestHandler.java
+++ /dev/null
@@ -1,90 +0,0 @@
-// Copyright 2012, 2013 The Apache Software Foundation
-//
-// Licensed 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.tapestry5.internal.services.javascript;
-
-import org.apache.tapestry5.internal.services.AssetDispatcher;
-import org.apache.tapestry5.internal.services.ResourceStreamer;
-import org.apache.tapestry5.ioc.IOOperation;
-import org.apache.tapestry5.ioc.OperationTracker;
-import org.apache.tapestry5.ioc.Resource;
-import org.apache.tapestry5.services.Dispatcher;
-import org.apache.tapestry5.services.Request;
-import org.apache.tapestry5.services.Response;
-import org.apache.tapestry5.services.assets.AssetRequestHandler;
-import org.apache.tapestry5.services.javascript.ModuleManager;
-
-import java.io.IOException;
-
-/**
- * Handler contributed to {@link AssetDispatcher} with key "modules". It 
interprets the extra path as a module name,
- * and searches for the corresponding JavaScript module.
- */
-public class ModuleAssetRequestHandler implements AssetRequestHandler, 
Dispatcher
-{
-    private final ModuleManager moduleManager;
-
-    private final ResourceStreamer streamer;
-
-    private final OperationTracker tracker;
-
-    public ModuleAssetRequestHandler(ModuleManager moduleManager, 
ResourceStreamer streamer, OperationTracker tracker)
-    {
-        this.moduleManager = moduleManager;
-        this.streamer = streamer;
-        this.tracker = tracker;
-    }
-
-    public boolean dispatch(Request request, Response response) throws 
IOException
-    {
-        return false;
-    }
-
-    public boolean handleAssetRequest(Request request, Response response, 
String extraPath) throws IOException
-    {
-        // Ensure request ends with '.js'.  That's the extension tacked on by 
RequireJS because it expects there
-        // to be a hierarchy of static JavaScript files here.
-
-        int dotx = extraPath.lastIndexOf('.');
-
-        if (dotx < 0)
-        {
-            return false;
-        }
-
-        if (!extraPath.substring(dotx + 1).equals("js"))
-        {
-            return false;
-        }
-
-        final String moduleName = extraPath.substring(0, dotx);
-
-        return tracker.perform(String.format("Streaming module %s", 
extraPath), new IOOperation<Boolean>()
-        {
-            public Boolean perform() throws IOException
-            {
-                Resource resource = 
moduleManager.findResourceForModule(moduleName);
-
-                if (resource != null)
-                {
-                    streamer.streamResource(resource);
-
-                    return true;
-                }
-
-                return false;
-            }
-        });
-    }
-}

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b91c11a1/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleDispatcher.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleDispatcher.java
 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleDispatcher.java
new file mode 100644
index 0000000..b81dd0f
--- /dev/null
+++ 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleDispatcher.java
@@ -0,0 +1,105 @@
+// Copyright 2012, 2013 The Apache Software Foundation
+//
+// Licensed 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.tapestry5.internal.services.javascript;
+
+import org.apache.tapestry5.SymbolConstants;
+import org.apache.tapestry5.internal.services.AssetDispatcher;
+import org.apache.tapestry5.internal.services.ResourceStreamer;
+import org.apache.tapestry5.ioc.IOOperation;
+import org.apache.tapestry5.ioc.OperationTracker;
+import org.apache.tapestry5.ioc.Resource;
+import org.apache.tapestry5.ioc.annotations.Symbol;
+import org.apache.tapestry5.services.Dispatcher;
+import org.apache.tapestry5.services.PathConstructor;
+import org.apache.tapestry5.services.Request;
+import org.apache.tapestry5.services.Response;
+import org.apache.tapestry5.services.javascript.ModuleManager;
+
+import java.io.IOException;
+
+/**
+ * Handler contributed to {@link AssetDispatcher} with key "modules". It 
interprets the extra path as a module name,
+ * and searches for the corresponding JavaScript module.
+ */
+public class ModuleDispatcher implements Dispatcher
+{
+    private final ModuleManager moduleManager;
+
+    private final ResourceStreamer streamer;
+
+    private final OperationTracker tracker;
+
+    private final String prefix;
+
+    public ModuleDispatcher(ModuleManager moduleManager,
+                            ResourceStreamer streamer,
+                            PathConstructor pathConstructor,
+                            @Symbol(SymbolConstants.APPLICATION_VERSION)
+                            String applicationVersion,
+                            OperationTracker tracker)
+    {
+        this.moduleManager = moduleManager;
+        this.streamer = streamer;
+        this.tracker = tracker;
+
+        prefix = pathConstructor.constructDispatchPath("modules", 
applicationVersion, "");
+    }
+
+    public boolean dispatch(Request request, Response response) throws 
IOException
+    {
+        String requestPath = request.getPath();
+
+        if (!requestPath.startsWith(prefix))
+        {
+            return false;
+        }
+
+        String extraPath = requestPath.substring(prefix.length());
+
+        // Ensure request ends with '.js'.  That's the extension tacked on by 
RequireJS because it expects there
+        // to be a hierarchy of static JavaScript files here.
+
+        int dotx = extraPath.lastIndexOf('.');
+
+        if (dotx < 0)
+        {
+            return false;
+        }
+
+        if (!extraPath.substring(dotx + 1).equals("js"))
+        {
+            return false;
+        }
+
+        final String moduleName = extraPath.substring(0, dotx);
+
+        return tracker.perform(String.format("Streaming module %s", 
extraPath), new IOOperation<Boolean>()
+        {
+            public Boolean perform() throws IOException
+            {
+                Resource resource = 
moduleManager.findResourceForModule(moduleName);
+
+                if (resource != null)
+                {
+                    streamer.streamResource(resource);
+
+                    return true;
+                }
+
+                return false;
+            }
+        });
+    }
+}

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b91c11a1/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
index f52cafa..13dac35 100644
--- 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
+++ 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
@@ -1,4 +1,4 @@
-// Copyright 2012 The Apache Software Foundation
+// Copyright 2012, 2013 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -28,7 +28,7 @@ import org.apache.tapestry5.json.JSONArray;
 import org.apache.tapestry5.json.JSONLiteral;
 import org.apache.tapestry5.json.JSONObject;
 import org.apache.tapestry5.services.AssetSource;
-import org.apache.tapestry5.services.assets.AssetPathConstructor;
+import org.apache.tapestry5.services.PathConstructor;
 import org.apache.tapestry5.services.assets.StreamableResourceSource;
 import org.apache.tapestry5.services.javascript.JavaScriptModuleConfiguration;
 import org.apache.tapestry5.services.javascript.ModuleManager;
@@ -56,7 +56,10 @@ public class ModuleManagerImpl implements ModuleManager
     // Note: ConcurrentHashMap does not support null as a value, alas. We use 
classpathRoot as a null.
     private final Map<String, Resource> cache = 
CollectionFactory.newConcurrentMap();
 
-    public ModuleManagerImpl(AssetPathConstructor constructor, AssetSource 
assetSource,
+    public ModuleManagerImpl(PathConstructor constructor,
+                             @Symbol(SymbolConstants.APPLICATION_VERSION)
+                             String applicationVersion,
+                             AssetSource assetSource,
                              @Path("${" + SymbolConstants.REQUIRE_JS + "}")
                              Asset requireJS,
                              Map<String, JavaScriptModuleConfiguration> 
configuration,
@@ -71,7 +74,7 @@ public class ModuleManagerImpl implements ModuleManager
         this.globalMessages = globalMessages;
         this.compactJSON = compactJSON;
 
-        this.requireConfig = 
buildRequireJSConfig(constructor.constructAssetPath("modules", ""), 
configuration, !productionMode);
+        this.requireConfig = 
buildRequireJSConfig(constructor.constructClientPath("modules", 
applicationVersion, ""), configuration, !productionMode);
 
         classpathRoot = assetSource.resourceForPath("");
 

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b91c11a1/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/VirtualResource.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/VirtualResource.java
 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/VirtualResource.java
index 82b3808..0fc0738 100644
--- 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/VirtualResource.java
+++ 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/VirtualResource.java
@@ -1,4 +1,4 @@
-// Copyright 2012 The Apache Software Foundation
+// Copyright 2012, 2013 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -30,7 +30,7 @@ import java.util.Locale;
  * the contents of the virtual resource.
  *
  * @see org.apache.tapestry5.services.javascript.ModuleManager
- * @see 
org.apache.tapestry5.internal.services.javascript.ModuleAssetRequestHandler
+ * @see org.apache.tapestry5.internal.services.javascript.ModuleDispatcher
  * @since 5.4
  */
 public abstract class VirtualResource implements Resource

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b91c11a1/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java 
b/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
index a75dbc2..3eebd3a 100644
--- 
a/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
+++ 
b/tapestry-core/src/main/java/org/apache/tapestry5/services/TapestryModule.java
@@ -380,6 +380,7 @@ public final class TapestryModule
         binder.bind(PageActivationContextCollector.class, 
PageActivationContextCollectorImpl.class);
         binder.bind(StringInterner.class, StringInternerImpl.class);
         binder.bind(ValueEncoderSource.class, ValueEncoderSourceImpl.class);
+        binder.bind(PathConstructor.class, PathConstructorImpl.class);
     }
 
     // ========================================================================
@@ -395,7 +396,7 @@ public final class TapestryModule
     // ========================================================================
 
     /**
-     * Contributes the factory for serveral built-in binding prefixes ("asset",
+     * Contributes the factory for several built-in binding prefixes ("asset",
      * "block", "component", "literal", prop",
      * "nullfieldstrategy", "message", "validate", "translate", "var").
      */

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b91c11a1/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/JavaScriptModule.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/JavaScriptModule.java
 
b/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/JavaScriptModule.java
index 5fb1fbe..23cb900 100644
--- 
a/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/JavaScriptModule.java
+++ 
b/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/JavaScriptModule.java
@@ -28,13 +28,13 @@ import org.apache.tapestry5.ioc.OrderedConfiguration;
 import org.apache.tapestry5.ioc.Resource;
 import org.apache.tapestry5.ioc.ServiceBinder;
 import org.apache.tapestry5.ioc.annotations.Contribute;
+import org.apache.tapestry5.ioc.annotations.Primary;
 import org.apache.tapestry5.ioc.annotations.Symbol;
 import org.apache.tapestry5.ioc.services.FactoryDefaults;
 import org.apache.tapestry5.ioc.services.SymbolProvider;
 import org.apache.tapestry5.ioc.util.IdAllocator;
 import org.apache.tapestry5.json.JSONObject;
 import org.apache.tapestry5.services.*;
-import org.apache.tapestry5.services.assets.AssetRequestHandler;
 import org.apache.tapestry5.services.compatibility.Compatibility;
 import org.apache.tapestry5.services.compatibility.Trait;
 import org.apache.tapestry5.services.messages.ComponentMessagesSource;
@@ -211,10 +211,10 @@ public class JavaScriptModule
     }
 
     @Contribute(Dispatcher.class)
-    @AssetRequestDispatcher
-    public static void handleModuleAssetRequests(MappedConfiguration<String, 
AssetRequestHandler> configuration)
+    @Primary
+    public static void handleModuleRequests(OrderedConfiguration<Dispatcher> 
configuration)
     {
-        configuration.addInstance("modules", ModuleAssetRequestHandler.class);
+        configuration.addInstance("Module", ModuleDispatcher.class, 
"before:PageRender");
     }
 
     @Contribute(ModuleManager.class)

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b91c11a1/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleAssetRequestHandlerTest.groovy
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleAssetRequestHandlerTest.groovy
 
b/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleAssetRequestHandlerTest.groovy
index 2065dbf..4940cca 100644
--- 
a/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleAssetRequestHandlerTest.groovy
+++ 
b/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleAssetRequestHandlerTest.groovy
@@ -1,9 +1,9 @@
-package org.apache.tapestry5.services.javascript;
+package org.apache.tapestry5.services.javascript
 
-
-import 
org.apache.tapestry5.internal.services.javascript.ModuleAssetRequestHandler
+import org.apache.tapestry5.internal.services.javascript.ModuleDispatcher
 import org.apache.tapestry5.ioc.internal.QuietOperationTracker
 import org.apache.tapestry5.ioc.test.TestBase
+import org.apache.tapestry5.services.PathConstructor
 import org.testng.annotations.DataProvider
 import org.testng.annotations.Test
 
@@ -11,9 +11,17 @@ class ModuleAssetRequestHandlerTest extends TestBase {
 
     @Test(dataProvider = "unknownPaths")
     void "invalid extension is ignored"(path) {
-        def handler = new ModuleAssetRequestHandler(null, null, null)
+        def pc = newMock PathConstructor
+
+        def handler = new ModuleDispatcher(null, null, pc, "123", new 
QuietOperationTracker())
+
+        expect(pc.constructDispatchPath("module", "123", "")).andReturn 
"/modules/123/"
+
+        replay()
 
         assertEquals handler.handleAssetRequest(null, null, path), false
+
+        verify()
     }
 
     @DataProvider
@@ -22,23 +30,26 @@ class ModuleAssetRequestHandlerTest extends TestBase {
             "foo/bar.xyz",
             "foo",
             "foo/bar",
-            "/",
             ""
-        ].collect({ it -> [it] as Object[]}) as Object[][]
+        ].collect({ it -> ["/modules/123/$it"] as Object[] }) as Object[][]
     }
 
     @Test
     void "returns false if no module is found"() {
 
-        def manager = newMock(ModuleManager)
+        def pc = newMock PathConstructor
+
+        def manager = newMock ModuleManager
+
+        def handler = new ModuleDispatcher(manager, null, pc, "123", new 
QuietOperationTracker())
 
-        def handler = new ModuleAssetRequestHandler(manager, null, new 
QuietOperationTracker())
+        expect(pc.constructDispatchPath("module", "123", "")).andReturn 
"/modules/123/"
 
         expect(manager.findResourceForModule("foo/bar")).andReturn null
 
         replay()
 
-        assertEquals handler.handleAssetRequest(null, null, "foo/bar.js"), 
false
+        assertEquals handler.handleAssetRequest(null, null, 
"/modules/123/foo/bar.js"), false
 
         verify()
     }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b91c11a1/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleDispatcherTests.groovy
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleDispatcherTests.groovy
 
b/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleDispatcherTests.groovy
new file mode 100644
index 0000000..4d39119
--- /dev/null
+++ 
b/tapestry-core/src/test/groovy/org/apache/tapestry5/services/javascript/ModuleDispatcherTests.groovy
@@ -0,0 +1,65 @@
+package org.apache.tapestry5.services.javascript
+
+import org.apache.tapestry5.internal.services.javascript.ModuleDispatcher
+import org.apache.tapestry5.ioc.internal.QuietOperationTracker
+import org.apache.tapestry5.ioc.test.TestBase
+import org.apache.tapestry5.services.PathConstructor
+import org.apache.tapestry5.services.Request
+import org.testng.annotations.DataProvider
+import org.testng.annotations.Test
+
+class ModuleDispatcherTests extends TestBase {
+
+    @Test(dataProvider = "unknownPaths")
+    void "invalid extension is ignored"(path) {
+        def pc = newMock PathConstructor
+
+        def request = newMock Request
+
+        expect(pc.constructDispatchPath("modules", "123", "")).andReturn 
"/modules/123/"
+
+        expect(request.path).andReturn path
+
+        replay()
+
+        def handler = new ModuleDispatcher(null, null, pc, "123", new 
QuietOperationTracker())
+
+        assertEquals handler.dispatch(request, null), false
+
+        verify()
+    }
+
+    @DataProvider
+    Object[][] unknownPaths() {
+        [
+            "foo/bar.xyz",
+            "foo",
+            "foo/bar",
+            ""
+        ].collect({ it -> ["/modules/123/$it"] as Object[] }) as Object[][]
+    }
+
+    @Test
+    void "returns false if no module is found"() {
+
+        def pc = newMock PathConstructor
+
+        def manager = newMock ModuleManager
+
+        def request = newMock Request
+
+        expect(pc.constructDispatchPath("modules", "123", "")).andReturn 
"/modules/123/"
+
+        expect(request.path).andReturn("/modules/123/foo/bar.js")
+
+        expect(manager.findResourceForModule("foo/bar")).andReturn null
+
+        replay()
+
+        def handler = new ModuleDispatcher(manager, null, pc, "123", new 
QuietOperationTracker())
+
+        assertEquals handler.dispatch(request, null), false
+
+        verify()
+    }
+}

Reply via email to