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 2014/08/27 02:31:54 UTC
git commit: TAP5-2377: ComponentClassResolver should ensure that page
names, component types, mixin names uniquely identify a single class
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 <hl...@apache.org>
Authored: Tue Aug 26 17:31:49 2014 -0700
Committer: Howard M. Lewis Ship <hl...@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()