Updated Branches:
  refs/heads/master ae5e4ad0e -> 04bf4d862

Do not send expiration date header with modules

Rely instead on ETags to know to respond with a 304


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

Branch: refs/heads/master
Commit: e43eae0a7b4c7e7dea56fe69fff13ea7603cbe0d
Parents: ae5e4ad
Author: Howard M. Lewis Ship <[email protected]>
Authored: Wed Apr 3 06:32:43 2013 -0700
Committer: Howard M. Lewis Ship <[email protected]>
Committed: Wed Apr 3 06:32:43 2013 -0700

----------------------------------------------------------------------
 .../internal/services/ResourceStreamer.java        |   51 ++++++++++++--
 .../internal/services/ResourceStreamerImpl.java    |   42 ++++++++----
 .../services/assets/StackAssetRequestHandler.java  |   18 ++++-
 .../services/javascript/ModuleDispatcher.java      |    6 ++-
 4 files changed, 91 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/e43eae0a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamer.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamer.java
 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamer.java
index 7200b07..cf4eefd 100644
--- 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamer.java
+++ 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamer.java
@@ -14,37 +14,72 @@
 
 package org.apache.tapestry5.internal.services;
 
-import java.io.IOException;
-
-import javax.servlet.http.HttpServletResponse;
-
 import org.apache.tapestry5.ioc.Resource;
 import org.apache.tapestry5.services.assets.StreamableResource;
 import org.apache.tapestry5.services.assets.StreamableResourceSource;
 
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.Set;
+
 /**
  * Responsible for streaming the contents of a resource to the client. The 
{@link org.apache.tapestry5.ioc.Resource} to
  * stream is a {@link 
org.apache.tapestry5.ioc.internal.util.ClasspathResource} or {@link 
ContextResource}.
- * 
+ *
  * @since 5.1.0.0
  */
 public interface ResourceStreamer
 {
     /**
+     * Used to customize the behavior of {@link 
#streamResource(org.apache.tapestry5.ioc.Resource, java.util.Set)}.
+     *
+     * @since 5.4
+     */
+    enum Options
+    {
+        /**
+         * Omit the normal expiration date header; this is appropriate for 
JavaScript modules, which cannot include
+         * their own checksum in the URL (instead, we rely on ETags to prevent 
unwanted data transfer).
+         */
+        OMIT_EXPIRATION
+    }
+
+    /**
      * Streams the content of the resource to the client (or sends
      * an alternative response such as {@link 
HttpServletResponse#SC_NOT_MODIFIED}). Encapsulates logic for compression
      * and for caching.
-     * 
+     *
      * @see StreamableResourceSource
      */
     void streamResource(Resource resource) throws IOException;
 
     /**
-     * Streams a resource that has been assembled elsewhere.
-     * 
+     * Streams the content of the resource to the client (or sends
+     * an alternative response such as {@link 
HttpServletResponse#SC_NOT_MODIFIED}). Encapsulates logic for compression
+     * and for caching.
+     *
+     * @see StreamableResourceSource
+     */
+    void streamResource(Resource resource, Set<Options> options) throws 
IOException;
+
+    /**
+     * Streams a resource that has been assembled elsewhere, using default 
options of none.
+     *
      * @param resource
      * @throws IOException
      * @since 5.3
      */
     void streamResource(StreamableResource resource) throws IOException;
+
+    /**
+     * Streams a resource that has been assembled elsewhere.
+     *
+     * @param resource
+     *         content to stream
+     * @param options
+     *         any special options related to streaming the resource
+     * @throws IOException
+     * @since 5.4
+     */
+    void streamResource(StreamableResource resource, Set<Options> options) 
throws IOException;
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/e43eae0a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamerImpl.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamerImpl.java
 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamerImpl.java
index 8daec39..6020adc 100644
--- 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamerImpl.java
+++ 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ResourceStreamerImpl.java
@@ -32,6 +32,8 @@ import 
org.apache.tapestry5.services.assets.StreamableResourceSource;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.util.EnumSet;
+import java.util.Set;
 
 public class ResourceStreamerImpl implements ResourceStreamer
 {
@@ -51,6 +53,8 @@ public class ResourceStreamerImpl implements ResourceStreamer
 
     private final ResourceChangeTracker resourceChangeTracker;
 
+    private final Set<Options> defaultOptions = EnumSet.noneOf(Options.class);
+
     public ResourceStreamerImpl(Request request,
 
                                 Response response,
@@ -76,7 +80,12 @@ public class ResourceStreamerImpl implements ResourceStreamer
         this.resourceChangeTracker = resourceChangeTracker;
     }
 
-    public void streamResource(final Resource resource) throws IOException
+    public void streamResource(Resource resource) throws IOException
+    {
+        streamResource(resource, defaultOptions);
+    }
+
+    public void streamResource(final Resource resource, final Set<Options> 
options) throws IOException
     {
         if (!resource.exists())
         {
@@ -88,20 +97,29 @@ public class ResourceStreamerImpl implements 
ResourceStreamer
         {
             public Void perform() throws IOException
             {
-                StreamableResourceProcessing processing = 
analyzer.isGZipSupported() ? StreamableResourceProcessing.COMPRESSION_ENABLED
+                StreamableResourceProcessing processing = 
analyzer.isGZipSupported()
+                        ? StreamableResourceProcessing.COMPRESSION_ENABLED
                         : StreamableResourceProcessing.COMPRESSION_DISABLED;
 
                 StreamableResource streamable = 
streamableResourceSource.getStreamableResource(resource, processing, 
resourceChangeTracker);
 
-                streamResource(streamable);
+                streamResource(streamable, options);
 
                 return null;
             }
         });
     }
 
-    public void streamResource(StreamableResource streamable) throws 
IOException
+    public void streamResource(StreamableResource resource) throws IOException
     {
+        streamResource(resource, defaultOptions);
+    }
+
+    public void streamResource(StreamableResource streamable, Set<Options> 
options) throws IOException
+    {
+        assert streamable != null;
+        assert options != null;
+
         long lastModified = streamable.getLastModified();
 
         long ifModifiedSince = 0;
@@ -116,13 +134,10 @@ public class ResourceStreamerImpl implements 
ResourceStreamer
             ifModifiedSince = -1;
         }
 
-        if (ifModifiedSince > 0)
+        if (ifModifiedSince > 0 && ifModifiedSince >= lastModified)
         {
-            if (ifModifiedSince >= lastModified)
-            {
-                response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
-                return;
-            }
+            response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
+            return;
         }
 
         // ETag should be surrounded with quotes.
@@ -135,10 +150,10 @@ public class ResourceStreamerImpl implements 
ResourceStreamer
 
         // If the client can send the correct ETag token, then its cache 
already contains the correct
         // content.
-
         String providedToken = request.getHeader("If-None-Match");
 
-        if (providedToken != null && providedToken.equals(token)) {
+        if (token.equals(providedToken))
+        {
             response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
             return;
         }
@@ -150,7 +165,7 @@ public class ResourceStreamerImpl implements 
ResourceStreamer
         response.setDateHeader("Last-Modified", lastModified);
 
 
-        if (productionMode)
+        if (productionMode && !options.contains(Options.OMIT_EXPIRATION))
         {
             // Starting in 5.4, this is a lot less necessary; any change to a 
Resource will result
             // in a new asset URL with the changed checksum incorporated into 
the URL.
@@ -170,4 +185,5 @@ public class ResourceStreamerImpl implements 
ResourceStreamer
 
         os.close();
     }
+
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/e43eae0a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/StackAssetRequestHandler.java
----------------------------------------------------------------------
diff --git 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/StackAssetRequestHandler.java
 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/StackAssetRequestHandler.java
index 39f1e31..7edff7f 100644
--- 
a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/StackAssetRequestHandler.java
+++ 
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/assets/StackAssetRequestHandler.java
@@ -64,7 +64,7 @@ public class StackAssetRequestHandler implements 
AssetRequestHandler
 
     public boolean handleAssetRequest(Request request, Response response, 
final String extraPath) throws IOException
     {
-        return tracker.perform(String.format("Streaming asset stack %s", 
extraPath),
+        return tracker.perform(String.format("Streaming JavaScript asset stack 
%s", extraPath),
                 new IOOperation<Boolean>()
                 {
                     public Boolean perform() throws IOException
@@ -85,7 +85,7 @@ public class StackAssetRequestHandler implements 
AssetRequestHandler
                 });
     }
 
-    private StreamableResource getResource(String extraPath, boolean 
compressed) throws IOException
+    private StreamableResource getResource(String extraPath, final boolean 
compressed) throws IOException
     {
         Matcher matcher = pathPattern.matcher(extraPath);
 
@@ -98,14 +98,24 @@ public class StackAssetRequestHandler implements 
AssetRequestHandler
 
         String checksum = matcher.group(1);
         String localeName = matcher.group(2);
-        String stackName = matcher.group(3);
+        final String stackName = matcher.group(3);
 
         // Yes, I have a big regret that the JavaScript stack stuff relies on 
this global, rather than
         // having it passed around properly.
 
         localizationSetter.setNonPersistentLocaleFromLocaleName(localeName);
 
-        StreamableResource resource = 
javaScriptStackAssembler.assembleJavaScriptResourceForStack(stackName, 
compressed);
+        StreamableResource resource =
+                tracker.perform(String.format("Assembling JavaScript asset 
stack '%s' (%s)",
+                        stackName, localeName),
+                        new IOOperation<StreamableResource>()
+                        {
+                            public StreamableResource perform() throws 
IOException
+                            {
+                                return 
javaScriptStackAssembler.assembleJavaScriptResourceForStack(stackName, 
compressed);
+
+                            }
+                        });
 
         return checksum.equals(resource.getChecksum())
                 ? resource

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/e43eae0a/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
index 284e23c..3b2864d 100644
--- 
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
@@ -28,6 +28,8 @@ import org.apache.tapestry5.services.Response;
 import org.apache.tapestry5.services.javascript.ModuleManager;
 
 import java.io.IOException;
+import java.util.EnumSet;
+import java.util.Set;
 
 /**
  * Handler contributed to {@link AssetDispatcher} with key "modules". It 
interprets the extra path as a module name,
@@ -43,6 +45,8 @@ public class ModuleDispatcher implements Dispatcher
 
     private final String prefix;
 
+    private final Set<ResourceStreamer.Options> omitExpiration = 
EnumSet.of(ResourceStreamer.Options.OMIT_EXPIRATION);
+
     public ModuleDispatcher(ModuleManager moduleManager,
                             ResourceStreamer streamer,
                             PathConstructor pathConstructor,
@@ -95,7 +99,7 @@ public class ModuleDispatcher implements Dispatcher
 
                 if (resource != null)
                 {
-                    streamer.streamResource(resource);
+                    streamer.streamResource(resource, omitExpiration);
 
                     return true;
                 }

Reply via email to