tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hls...@apache.org
Subject git commit: TAP5-2377: ComponentClassResolver should ensure that page names, component types, mixin names uniquely identify a single class
Date Wed, 27 Aug 2014 00:31:54 GMT
Repository: tapestry-5
Updated Branches:
  refs/heads/master 898364269 -> 532cfae85


TAP5-2377: ComponentClassResolver should ensure that page names, component types, mixin names
uniquely identify a single class


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

Branch: refs/heads/master
Commit: 532cfae8540ec7d77d0ebdcc46d984788d4488cd
Parents: 8983642
Author: Howard M. Lewis Ship <hlship@apache.org>
Authored: Tue Aug 26 17:31:49 2014 -0700
Committer: Howard M. Lewis Ship <hlship@apache.org>
Committed: Tue Aug 26 17:31:49 2014 -0700

----------------------------------------------------------------------
 .../services/ComponentClassResolverImpl.java    | 119 +++++++++++++++----
 .../ComponentClassResolverImplTest.groovy       |  26 ++--
 2 files changed, 113 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/532cfae8/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassResolverImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassResolverImpl.java
b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassResolverImpl.java
index e81037e..4d29a3e 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassResolverImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassResolverImpl.java
@@ -1,5 +1,3 @@
-// Copyright 2006-2014 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
@@ -67,7 +65,7 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
     // is operating.
 
     private volatile boolean needsRebuild = true;
-    
+
     private final Collection<LibraryMapping> libraryMappings;
 
     private class Data
@@ -100,15 +98,28 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
          */
         private final Map<String, String> pageNameToCanonicalPageName = CollectionFactory.newCaseInsensitiveMap();
 
+
+        /**
+         * These are used to check for name overlaps: a single name (generated by different
paths) that maps to more than one class.
+         */
+        private Map<String, Set<String>> pageToClassNames = CollectionFactory.newCaseInsensitiveMap();
+
+        private Map<String, Set<String>> componentToClassNames = CollectionFactory.newCaseInsensitiveMap();
+
+        private Map<String, Set<String>> mixinToClassNames = CollectionFactory.newCaseInsensitiveMap();
+
+        private boolean invalid = false;
+
         private void rebuild(String pathPrefix, String rootPackage)
         {
-            fillNameToClassNameMap(pathPrefix, rootPackage, InternalConstants.PAGES_SUBPACKAGE,
pageToClassName);
-            fillNameToClassNameMap(pathPrefix, rootPackage, InternalConstants.COMPONENTS_SUBPACKAGE,
componentToClassName);
-            fillNameToClassNameMap(pathPrefix, rootPackage, InternalConstants.MIXINS_SUBPACKAGE,
mixinToClassName);
+            fillNameToClassNameMap(pathPrefix, rootPackage, InternalConstants.PAGES_SUBPACKAGE,
pageToClassName, pageToClassNames);
+            fillNameToClassNameMap(pathPrefix, rootPackage, InternalConstants.COMPONENTS_SUBPACKAGE,
componentToClassName, componentToClassNames);
+            fillNameToClassNameMap(pathPrefix, rootPackage, InternalConstants.MIXINS_SUBPACKAGE,
mixinToClassName, mixinToClassNames);
         }
 
         private void fillNameToClassNameMap(String pathPrefix, String rootPackage, String
subPackage,
-                                            Map<String, String> logicalNameToClassName)
+                                            Map<String, String> logicalNameToClassName,
+                                            Map<String, Set<String>> nameToClassNames)
         {
             String searchPackage = rootPackage + "." + subPackage;
             boolean isPage = subPackage.equals(InternalConstants.PAGES_SUBPACKAGE);
@@ -117,10 +128,10 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
 
             int startPos = searchPackage.length() + 1;
 
-            for (String name : classNames)
+            for (String className : classNames)
             {
-                String logicalName = toLogicalName(name, pathPrefix, startPos, true);
-                String unstrippedName = toLogicalName(name, pathPrefix, startPos, false);
+                String logicalName = toLogicalName(className, pathPrefix, startPos, true);
+                String unstrippedName = toLogicalName(className, pathPrefix, startPos, false);
 
                 if (isPage)
                 {
@@ -137,18 +148,22 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
 
                         if (!(lastTerm.equalsIgnoreCase(startPageName) && logicalNameToClassName.containsKey(reducedName)))
                         {
-                            logicalNameToClassName.put(reducedName, name);
+                            logicalNameToClassName.put(reducedName, className);
                             pageNameToCanonicalPageName.put(reducedName, logicalName);
+                            addNameMapping(nameToClassNames, reducedName, className);
                         }
                     }
 
-                    pageClassNameToLogicalName.put(name, logicalName);
+                    pageClassNameToLogicalName.put(className, logicalName);
                     pageNameToCanonicalPageName.put(logicalName, logicalName);
                     pageNameToCanonicalPageName.put(unstrippedName, logicalName);
                 }
 
-                logicalNameToClassName.put(logicalName, name);
-                logicalNameToClassName.put(unstrippedName, name);
+                logicalNameToClassName.put(logicalName, className);
+                logicalNameToClassName.put(unstrippedName, className);
+
+                addNameMapping(nameToClassNames, logicalName, className);
+                addNameMapping(nameToClassNames, unstrippedName, className);
             }
         }
 
@@ -236,6 +251,63 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
         {
             return string.regionMatches(true, string.length() - suffix.length(), suffix,
0, suffix.length());
         }
+
+        private void addNameMapping(Map<String, Set<String>> map, String name,
String className)
+        {
+            Set<String> classNames = map.get(name);
+
+            if (classNames == null)
+            {
+                classNames = CollectionFactory.newSet();
+                map.put(name, classNames);
+            }
+
+            classNames.add(className);
+        }
+
+        private void validate()
+        {
+            validate("page name", pageToClassNames);
+            validate("component type", componentToClassNames);
+            validate("mixin type", mixinToClassNames);
+
+            // No longer needed after validation.
+            pageToClassNames = null;
+            componentToClassNames = null;
+            mixinToClassNames = null;
+
+            if (invalid)
+            {
+                throw new IllegalStateException("You must correct these validation issues
to proceed.");
+            }
+        }
+
+        private void validate(String category, Map<String, Set<String>> map)
+        {
+            boolean header = false;
+
+            for (String name : F.flow(map.keySet()).sort())
+            {
+                Set<String> classNames = map.get(name);
+
+                if (classNames.size() == 1)
+                {
+                    continue;
+                }
+
+                if (!header)
+                {
+                    logger.error(String.format("Some %s(s) map to more than one Java class.",
category));
+                    header = true;
+                    invalid = true;
+                }
+
+                logger.error(String.format("%s '%s' maps to %s",
+                        InternalUtils.capitalize(category),
+                        name,
+                        InternalUtils.joinSorted(classNames)));
+            }
+        }
     }
 
     private volatile Data data = new Data();
@@ -258,7 +330,7 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
         for (LibraryMapping mapping : mappings)
         {
             String libraryName = mapping.libraryName;
-            
+
             List<String> packages = this.libraryNameToPackageNames.get(libraryName);
 
             if (packages == null)
@@ -301,8 +373,7 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
     }
 
     /**
-     * Invoked from within a withRead() block, checks to see if a rebuild is needed, and
then performs the rebuild
-     * within a withWrite() block.
+     * Returns the current data, or atomically rebuilds it. In rare race conditions, the
data may be rebuilt more than once, overlapping.
      */
     private Data getData()
     {
@@ -320,9 +391,13 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
             String folder = prefix + "/";
 
             for (String packageName : packages)
+            {
                 newData.rebuild(folder, packageName);
+            }
         }
 
+        newData.validate();
+
         showChanges("pages", data.pageToClassName, newData.pageToClassName);
         showChanges("components", data.componentToClassName, newData.componentToClassName);
         showChanges("mixins", data.mixinToClassName, newData.mixinToClassName);
@@ -341,9 +416,13 @@ public class ComponentClassResolverImpl implements ComponentClassResolver,
Inval
 
     /**
      * Log (at INFO level) the changes between the two logical-name-to-class-name maps
-     * @param title the title of the things in the maps (e.g. "pages" or "components")
-     * @param savedMap the old map
-     * @param newMap the new map
+     *
+     * @param title
+     *         the title of the things in the maps (e.g. "pages" or "components")
+     * @param savedMap
+     *         the old map
+     * @param newMap
+     *         the new map
      */
     private void showChanges(String title, Map<String, String> savedMap, Map<String,
String> newMap)
     {

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/532cfae8/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/ComponentClassResolverImplTest.groovy
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/ComponentClassResolverImplTest.groovy
b/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/ComponentClassResolverImplTest.groovy
index 7b666e1..9b7d4a2 100644
--- a/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/ComponentClassResolverImplTest.groovy
+++ b/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/ComponentClassResolverImplTest.groovy
@@ -8,6 +8,7 @@ import org.apache.tapestry5.services.ComponentClassResolver
 import org.apache.tapestry5.services.LibraryMapping
 import org.easymock.EasyMock
 import org.slf4j.Logger
+import org.slf4j.LoggerFactory
 import org.testng.annotations.Test
 
 import static org.easymock.EasyMock.isA
@@ -165,34 +166,35 @@ class ComponentClassResolverImplTest extends InternalBaseTestCase {
         verify()
     }
 
-    /**
-     * TAP5-1444
-     */
     @Test
-    void index_page_precedence() {
+    void name_clashes_are_identified() {
         ClassNameLocator locator = newClassNameLocator()
-        Logger logger = compliantLogger()
+        Logger logger = newMock Logger
 
-        def classNames = ["${APP_ROOT_PACKAGE}.pages.sub.HomePage", "${APP_ROOT_PACKAGE}.pages.sub.SubIndex"]
as String[]
+        def classNames = ["${APP_ROOT_PACKAGE}.pages.Foo", "${APP_ROOT_PACKAGE}.pages.foo.Index"]
as String[]
 
         train_locateComponentClassNames(locator, "${APP_ROOT_PACKAGE}.pages", classNames)
 
+        expect(logger.error(EasyMock.isA(String))).atLeastOnce()
+
         replay()
 
         List<LibraryMapping> mappings = APP_ROOT_PACKAGE_MAPPINGS
 
         ComponentClassResolver resolver = new ComponentClassResolverImpl(logger, locator,
"HomePage", mappings)
 
-        assertTrue(resolver.isPageName("sub/HomePage"))
-        assertTrue(resolver.isPageName("sub/subIndex"))
-        assertEquals(resolver.resolvePageNameToClassName("sub/HomePage"), "${APP_ROOT_PACKAGE}.pages.sub.HomePage")
-        assertEquals(resolver.resolvePageNameToClassName("sub/SubIndex"), "${APP_ROOT_PACKAGE}.pages.sub.SubIndex")
-        assertEquals(resolver.resolvePageNameToClassName("sub/Index"), "${APP_ROOT_PACKAGE}.pages.sub.SubIndex")
-        assertEquals(resolver.resolvePageNameToClassName("sub"), "${APP_ROOT_PACKAGE}.pages.sub.SubIndex")
+        try {
+            resolver.isPageName "foo"
+            unreachable()
+        }
+        catch (IllegalStateException ex) {
+            assertMessageContains ex, "correct these validation issues"
+        }
 
         verify()
     }
 
+
     @Test
     void page_name_in_subfolder() {
         ClassNameLocator locator = newClassNameLocator()


Mime
View raw message