You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2008/01/20 21:26:24 UTC

svn commit: r613670 - in /tapestry/tapestry5/trunk/tapestry-core/src: main/java/org/apache/tapestry/internal/services/ main/java/org/apache/tapestry/internal/test/ main/java/org/apache/tapestry/services/ test/java/org/apache/tapestry/internal/services/

Author: hlship
Date: Sun Jan 20 12:26:16 2008
New Revision: 613670

URL: http://svn.apache.org/viewvc?rev=613670&view=rev
Log:
TAPESTRY-1636: Template reloading in Tomcat doesn't work

Modified:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentTemplateSourceImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextResource.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/PageTesterContext.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Context.java
    tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextImplTest.java
    tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextResourceTest.java

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentTemplateSourceImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentTemplateSourceImpl.java?rev=613670&r1=613669&r2=613670&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentTemplateSourceImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ComponentTemplateSourceImpl.java Sun Jan 20 12:26:16 2008
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007 The Apache Software Foundation
+// Copyright 2006, 2007, 2008 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.
@@ -34,8 +34,7 @@
 /**
  * Service implementation that manages a cache of parsed component templates.
  */
-public final class ComponentTemplateSourceImpl extends InvalidationEventHubImpl implements
-                                                                                ComponentTemplateSource, UpdateListener
+public final class ComponentTemplateSourceImpl extends InvalidationEventHubImpl implements ComponentTemplateSource, UpdateListener
 {
 
     private final TemplateParser _parser;
@@ -45,10 +44,9 @@
     private final URLChangeTracker _tracker;
 
     /**
-     * Caches from a key (combining component name and locale) to a resource. Often, many different
-     * keys will point to the same resource (i.e., "foo:en_US", "foo:en_UK", and "foo:en" may all be
-     * parsed from the same "foo.tml" resource). The resource may end up being null, meaning the
-     * template does not exist in any locale.
+     * Caches from a key (combining component name and locale) to a resource. Often, many different keys will point to
+     * the same resource (i.e., "foo:en_US", "foo:en_UK", and "foo:en" may all be parsed from the same "foo.tml"
+     * resource). The resource may end up being null, meaning the template does not exist in any locale.
      */
     private final Map<MultiKey, Resource> _templateResources = newConcurrentMap();
 
@@ -85,8 +83,7 @@
         this(parser, locator, new URLChangeTracker());
     }
 
-    ComponentTemplateSourceImpl(TemplateParser parser, PageTemplateLocator locator,
-                                URLChangeTracker tracker)
+    ComponentTemplateSourceImpl(TemplateParser parser, PageTemplateLocator locator, URLChangeTracker tracker)
     {
         _parser = parser;
         _locator = locator;
@@ -94,9 +91,9 @@
     }
 
     /**
-     * Resolves the component name to a {@link Resource} and finds the localization of that resource
-     * (the combination of component name and locale is resolved to a resource). The localized
-     * resource is used as the key to a cache of {@link ComponentTemplate}s.
+     * Resolves the component name to a {@link Resource} and finds the localization of that resource (the combination of
+     * component name and locale is resolved to a resource). The localized resource is used as the key to a cache of
+     * {@link ComponentTemplate}s.
      * <p/>
      * If a template doesn't exist, then the missing ComponentTemplate is returned.
      */
@@ -136,8 +133,7 @@
 
         URL resourceURL = r.toURL();
 
-        if (resourceURL == null)
-            return _missingTemplate;
+        if (resourceURL == null) return _missingTemplate;
 
         _tracker.add(resourceURL);
 
@@ -160,16 +156,14 @@
             // then we've found a match (even if we had to ascend a couple of levels
             // to reach it).
 
-            if (localized != null)
-                return localized;
+            if (localized != null) return localized;
 
             // Not on the classpath, the the locator see if its a) a page and b) a resource inside
             // the context
 
             localized = _locator.findPageTemplateResource(model, locale);
 
-            if (localized != null)
-                return localized;
+            if (localized != null) return localized;
 
             // Otherwise, this component doesn't have its own template ... lets work up to its
             // base class and check there.
@@ -189,9 +183,9 @@
     }
 
     /**
-     * Checks to see if any parsed resource has changed. If so, then all internal caches are
-     * cleared, and an invalidation event is fired. This is brute force ... a more targeted
-     * dependency management strategy may come later.
+     * Checks to see if any parsed resource has changed. If so, then all internal caches are cleared, and an
+     * invalidation event is fired. This is brute force ... a more targeted dependency management strategy may come
+     * later.
      */
     public void checkForUpdates()
     {

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextImpl.java?rev=613670&r1=613669&r2=613670&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextImpl.java Sun Jan 20 12:26:16 2008
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007 The Apache Software Foundation
+// Copyright 2006, 2007, 2008 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.
@@ -20,6 +20,7 @@
 import org.apache.tapestry.services.Context;
 
 import javax.servlet.ServletContext;
+import java.io.File;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Collections;
@@ -47,6 +48,13 @@
         }
     }
 
+    public File getRealFile(String path)
+    {
+        String realPath = _servletContext.getRealPath(path);
+
+        return realPath == null ? null : new File(realPath);
+    }
+
     public String getInitParameter(String name)
     {
         return _servletContext.getInitParameter(name);
@@ -75,10 +83,8 @@
             {
                 // Folders are queued up for further expansion.
 
-                if (match.endsWith("/"))
-                    queue.push(match);
-                else
-                    result.add(match);
+                if (match.endsWith("/")) queue.push(match);
+                else result.add(match);
             }
 
         }

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextResource.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextResource.java?rev=613670&r1=613669&r2=613670&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextResource.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/services/ContextResource.java Sun Jan 20 12:26:16 2008
@@ -1,4 +1,4 @@
-// Copyright 2006 The Apache Software Foundation
+// Copyright 2006, 2008 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.
@@ -19,6 +19,8 @@
 import static org.apache.tapestry.ioc.internal.util.Defense.notNull;
 import org.apache.tapestry.services.Context;
 
+import java.io.File;
+import java.net.MalformedURLException;
 import java.net.URL;
 
 /**
@@ -57,7 +59,29 @@
         // and HttpServletContext.getResource() does. This is what I mean when I say that
         // a framework is an accumulation of the combined experience of many users and developers.
 
-        return _context.getResource("/" + getPath());
+        String contextPath = "/" + getPath();
+
+        // Always prefer the actual file to the URL.  This is critical for templates to
+        // reload inside Tomcat.
+
+        File file = _context.getRealFile(contextPath);
+
+        if (file != null && file.exists())
+        {
+            try
+            {
+                return file.toURL();
+            }
+            catch (MalformedURLException ex)
+            {
+                throw new RuntimeException(ex);
+            }
+        }
+
+        // But, when packaged inside a WAR or JAR, the File will not be available, so use whatever
+        // URL we get ... but reloading won't work.
+
+        return _context.getResource(contextPath);
     }
 
     @Override

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/PageTesterContext.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/PageTesterContext.java?rev=613670&r1=613669&r2=613670&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/PageTesterContext.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/test/PageTesterContext.java Sun Jan 20 12:26:16 2008
@@ -1,4 +1,4 @@
-// Copyright 2007 The Apache Software Foundation
+// Copyright 2007, 2008 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.
@@ -64,4 +64,11 @@
         throw new UnsupportedOperationException("getAttribute() is not supported for ContextForPageTester.");
     }
 
+    /**
+     * Always returns null.
+     */
+    public File getRealFile(String path)
+    {
+        return null;
+    }
 }

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Context.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Context.java?rev=613670&r1=613669&r2=613670&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Context.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/services/Context.java Sun Jan 20 12:26:16 2008
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007 The Apache Software Foundation
+// Copyright 2006, 2007, 2008 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.
@@ -14,33 +14,43 @@
 
 package org.apache.tapestry.services;
 
+import java.io.File;
 import java.net.URL;
 import java.util.List;
 
 /**
- * An API agnostic version of {@link javax.servlet.ServletContext}, used to bridge the gaps between
- * the Servlet API and the Portlet API.
+ * An API agnostic version of {@link javax.servlet.ServletContext}, used to bridge the gaps between the Servlet API and
+ * the Portlet API.
  */
 public interface Context
 {
     /**
-     * Returns a URL to a resource stored within the context. The path should start with a leading
-     * slash.
+     * Returns a URL to a resource stored within the context. The path should start with a leading slash.
      *
-     * @param path
+     * @param path to the resource (with a leading slash)
      * @return the URL for the path, or null if the path does not correspond to a file.
      */
     URL getResource(String path);
 
     /**
+     * Attempts to find the actual file, on the file system, that would be provided by the servlet container for the
+     * given path (which must start with a leading slash). This may return null if no such file exists, or if the
+     * resource in question is packaged inside a WAR.  If packaged inside a WAR, the contents may be accessed via {@link
+     * #getResource(String)}.
+     *
+     * @param path to  the resource (with a leading slash)
+     * @return the underlying File, or null if no such file
+     */
+    File getRealFile(String path);
+
+    /**
      * Returns an initial parameter value defined by servlet.
      */
     String getInitParameter(String name);
 
     /**
-     * Looks for resources within the web application within the supplied path. The list will be
-     * recurively expanded, as necessary. The path must start with a leading slash, and usually ends
-     * with a slash as well.
+     * Looks for resources within the web application within the supplied path. The list will be recurively expanded, as
+     * necessary. The path must start with a leading slash, and usually ends with a slash as well.
      *
      * @param path to search for (should start with a leading slash)
      * @return the matches, sorted alphabetically

Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextImplTest.java?rev=613670&r1=613669&r2=613670&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextImplTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextImplTest.java Sun Jan 20 12:26:16 2008
@@ -1,4 +1,4 @@
-// Copyright 2007 The Apache Software Foundation
+// Copyright 2007, 2008 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.
@@ -20,6 +20,8 @@
 import org.testng.annotations.Test;
 
 import javax.servlet.ServletContext;
+import java.io.File;
+import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Arrays;
@@ -78,30 +80,16 @@
     {
         ServletContext servletContext = newServletContext();
 
-        train_getResourcePaths(
-                servletContext,
-                "/foo",
-                "/foo/alpha.tml",
-                "/foo/beta/",
-                "/foo/gamma.tml");
-        train_getResourcePaths(
-                servletContext,
-                "/foo/beta/",
-                "/foo/beta/b.tml",
-                "/foo/beta/a.tml",
-                "/foo/beta/c/");
+        train_getResourcePaths(servletContext, "/foo", "/foo/alpha.tml", "/foo/beta/", "/foo/gamma.tml");
+        train_getResourcePaths(servletContext, "/foo/beta/", "/foo/beta/b.tml", "/foo/beta/a.tml", "/foo/beta/c/");
         train_getResourcePaths(servletContext, "/foo/beta/c/", "/foo/beta/c/c.tml");
 
         replay();
 
         List<String> actual = new ContextImpl(servletContext).getResourcePaths("/foo");
 
-        assertEquals(actual, Arrays.asList(
-                "/foo/alpha.tml",
-                "/foo/beta/a.tml",
-                "/foo/beta/b.tml",
-                "/foo/beta/c/c.tml",
-                "/foo/gamma.tml"));
+        assertEquals(actual, Arrays.asList("/foo/alpha.tml", "/foo/beta/a.tml", "/foo/beta/b.tml", "/foo/beta/c/c.tml",
+                                           "/foo/gamma.tml"));
 
         verify();
     }
@@ -145,13 +133,58 @@
 
     }
 
+    @Test
+    public void get_real_file_exists() throws IOException
+    {
+        String path = "/foo.gif";
+        File file = File.createTempFile("foo", "gif");
+        String realPath = file.getPath();
+
+        ServletContext servletContext = newServletContext();
+
+        train_getRealPath(servletContext, path, realPath);
+
+        replay();
+
+        Context c = new ContextImpl(servletContext);
+
+        File f = c.getRealFile(path);
+
+        assertEquals(f, file);
+
+
+        verify();
+    }
+
+    @Test
+    public void get_real_file_missing()
+    {
+        String path = "/foo.gif";
+
+        ServletContext servletContext = newServletContext();
+
+        train_getRealPath(servletContext, path, null);
+
+        replay();
+
+        Context c = new ContextImpl(servletContext);
+
+        assertNull(c.getRealFile(path));
+
+        verify();
+    }
+
+    private void train_getRealPath(ServletContext servletContext, String path, String realPath)
+    {
+        expect(servletContext.getRealPath(path)).andReturn(realPath);
+    }
+
     protected final ServletContext newServletContext()
     {
         return newMock(ServletContext.class);
     }
 
-    protected final void train_getResourcePaths(ServletContext context, String path,
-                                                String... paths)
+    protected final void train_getResourcePaths(ServletContext context, String path, String... paths)
     {
         Set<String> set = CollectionFactory.newSet(Arrays.asList(paths));
 

Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextResourceTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextResourceTest.java?rev=613670&r1=613669&r2=613670&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextResourceTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/internal/services/ContextResourceTest.java Sun Jan 20 12:26:16 2008
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007 The Apache Software Foundation
+// Copyright 2006, 2007, 2008 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.
@@ -19,17 +19,21 @@
 import org.apache.tapestry.services.Context;
 import org.testng.annotations.Test;
 
+import java.io.File;
 import java.net.URL;
 
 public class ContextResourceTest extends InternalBaseTestCase
 {
     @Test
-    public void get_url() throws Exception
+    public void get_url_no_real_file() throws Exception
     {
+        String path = "/foo/Bar.txt";
         URL url = getClass().getResource("ContextResourceTest.class");
 
         Context context = mockContext();
 
+        expect(context.getRealFile(path)).andReturn(null);
+
         expect(context.getResource("/foo/Bar.txt")).andReturn(url);
 
         replay();
@@ -37,6 +41,26 @@
         Resource r = new ContextResource(context, "foo/Bar.txt");
 
         assertSame(r.toURL(), url);
+
+        verify();
+    }
+
+    @Test
+    public void get_url_file_exists() throws Exception
+    {
+        File f = File.createTempFile("Bar", ".txt");
+
+        String path = "/foo/Bar.txt";
+
+        Context context = mockContext();
+
+        expect(context.getRealFile(path)).andReturn(f);
+
+        replay();
+
+        Resource r = new ContextResource(context, "foo/Bar.txt");
+
+        assertEquals(r.toURL(), f.toURL());
 
         verify();
     }