You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2008/09/12 20:15:29 UTC

svn commit: r694761 - in /tapestry/tapestry5/trunk: tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ut...

Author: hlship
Date: Fri Sep 12 11:15:28 2008
New Revision: 694761

URL: http://svn.apache.org/viewvc?rev=694761&view=rev
Log:
TAPESTRY-2236: Strip trailing punctuation (_ and $) from member names, as is done with leading punctuation

Modified:
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BlockInjectionProvider.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java
    tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/PropertyWorker.java
    tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/InternalUtils.java
    tapestry/tapestry5/trunk/tapestry-ioc/src/test/conf/testng.xml
    tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/InternalUtilsTest.java

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BlockInjectionProvider.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BlockInjectionProvider.java?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BlockInjectionProvider.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/BlockInjectionProvider.java Fri Sep 12 11:15:28 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.
@@ -63,7 +63,6 @@
     {
         if (annotation != null) return annotation.value();
 
-        return InternalUtils.stripMemberPrefix(fieldName);
+        return InternalUtils.stripMemberName(fieldName);
     }
-
 }

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/InternalClassTransformationImpl.java Fri Sep 12 11:15:28 2008
@@ -442,7 +442,7 @@
 
     public String newMemberName(String prefix, String baseName)
     {
-        return newMemberName(prefix + "_" + InternalUtils.stripMemberPrefix(baseName));
+        return newMemberName(prefix + "_" + InternalUtils.stripMemberName(baseName));
     }
 
     public void addImplementedInterface(Class interfaceClass)

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ComponentWorker.java Fri Sep 12 11:15:28 2008
@@ -54,7 +54,7 @@
 
             String id = annotation.id();
 
-            if (InternalUtils.isBlank(id)) id = InternalUtils.stripMemberPrefix(fieldName);
+            if (InternalUtils.isBlank(id)) id = InternalUtils.stripMemberName(fieldName);
 
             String type = transformation.getFieldType(fieldName);
 
@@ -113,5 +113,4 @@
             embedded.addParameter(kv.getKey(), kv.getValue());
         }
     }
-
 }

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/InjectComponentWorker.java Fri Sep 12 11:15:28 2008
@@ -1,4 +1,4 @@
-// Copyright  2008 The Apache Software Foundation
+// Copyright 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.
@@ -43,7 +43,7 @@
 
             String componentId = annotation.value();
             if (InternalUtils.isBlank(componentId))
-                componentId = InternalUtils.stripMemberPrefix(fieldName);
+                componentId = InternalUtils.stripMemberName(fieldName);
 
             transformation.makeReadOnly(fieldName);
 

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/ParameterWorker.java Fri Sep 12 11:15:28 2008
@@ -175,7 +175,6 @@
                            resourcesFieldName, parameterName, bindingFactoryFieldName, defaultPrefix, defaultBinding);
 
             return;
-
         }
 
         if (autoconnect)
@@ -311,7 +310,7 @@
     {
         if (InternalUtils.isNonBlank(annotatedName)) return annotatedName;
 
-        return InternalUtils.stripMemberPrefix(fieldName);
+        return InternalUtils.stripMemberName(fieldName);
     }
 
     public static void bind(String parameterName, InternalComponentResources resources, Object value)

Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/PropertyWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/PropertyWorker.java?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/PropertyWorker.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/transform/PropertyWorker.java Fri Sep 12 11:15:28 2008
@@ -38,7 +38,7 @@
         {
             Property annotation = transformation.getFieldAnnotation(fieldName, Property.class);
 
-            String propertyName = InternalUtils.capitalize(InternalUtils.stripMemberPrefix(fieldName));
+            String propertyName = InternalUtils.capitalize(InternalUtils.stripMemberName(fieldName));
 
             String fieldType = transformation.getFieldType(fieldName);
 
@@ -56,13 +56,12 @@
             {
                 TransformMethodSignature setter
                         = new TransformMethodSignature(Modifier.PUBLIC | Modifier.FINAL, "void", "set" + propertyName,
-                                                       new String[] { fieldType }, null);
+                                                       new String[] {fieldType}, null);
 
                 transformation.addTransformedMethod(setter, fieldName + " = $1;");
             }
 
             // The field is NOT claimed, because we want annotation for the fields to operate.
         }
-
     }
 }

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/InternalUtils.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/InternalUtils.java?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/InternalUtils.java (original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/InternalUtils.java Fri Sep 12 11:15:28 2008
@@ -33,6 +33,8 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.*;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Utilities used within various internal implemenations of Tapestry IOC and the rest of the tapestry-core framework.
@@ -46,6 +48,11 @@
     private static final String NAME_PREFIX = "_$";
 
     /**
+     * Pattern used to eliminate leading and trailing underscores and dollar signs.
+     */
+    private static final Pattern NAME_PATTERN = Pattern.compile("^[_|$]*([\\w|$]+?)[_|$]*$", Pattern.CASE_INSENSITIVE);
+
+    /**
      * Converts a method to a user presentable string using a {@link ClassFactory} to obtain a {@link Location} (where
      * possible). {@link #asString(Method)} is used under the covers, to present a detailed, but not excessive,
      * description of the class, method and parameters.
@@ -99,28 +106,18 @@
     }
 
     /**
-     * Strips leading punctuation ("_" and "$") from the provided name.
+     * Strips leading "_" and "$" and trailing "_" from the name.
      */
-    public static String stripMemberPrefix(String memberName)
+    public static String stripMemberName(String memberName)
     {
-        StringBuilder builder = new StringBuilder(memberName);
-
-        // There may be other prefixes we want to strip off, at some point!
-
-        // Strip off leading characters defined by NAME_PREFIX
-
-        // This code is really ugly and needs to be fixed.
+        Defense.notBlank(memberName, "memberName");
 
-        while (true)
-        {
-            char ch = builder.charAt(0);
-
-            if (InternalUtils.NAME_PREFIX.indexOf(ch) < 0) break;
+        Matcher matcher = NAME_PATTERN.matcher(memberName);
 
-            builder.deleteCharAt(0);
-        }
+        if (!matcher.matches())
+            throw new IllegalArgumentException(String.format("Input '%s' is not a valid Java identifier.", memberName));
 
-        return builder.toString();
+        return matcher.group(1);
     }
 
     /**
@@ -128,7 +125,7 @@
      */
     public static String createMemberName(String memberName)
     {
-        return NAME_PREFIX + stripMemberPrefix(memberName);
+        return NAME_PREFIX + stripMemberName(memberName);
     }
 
     /**
@@ -178,7 +175,6 @@
             {
                 return findAnnotation(parameterAnnotations, annotationClass);
             }
-
         };
 
         // At some point, it would be nice to eliminate InjectService, and rely
@@ -417,7 +413,6 @@
             {
                 throw new UnsupportedOperationException();
             }
-
         };
     }
 
@@ -576,6 +571,5 @@
                     "Constructor %s is not public and may not be used for autobuilding an instance of the class. " +
                             "You should make the constructor public, or mark an alternate public constructor with the @Inject annotation.",
                     constructor));
-
     }
 }

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/test/conf/testng.xml
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/test/conf/testng.xml?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/test/conf/testng.xml (original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/test/conf/testng.xml Fri Sep 12 11:15:28 2008
@@ -15,7 +15,7 @@
    limitations under the License.
 -->
 
-<suite name="Tapestry IOC" thread-count="10" annotations="1.5" verbose="2" parallel="tests">
+<suite name="Tapestry IOC" annotations="1.5" verbose="2">
     <test name="Public APIs">
         <packages>
             <package name="org.apache.tapestry5.ioc"/>

Modified: tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/InternalUtilsTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/InternalUtilsTest.java?rev=694761&r1=694760&r2=694761&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/InternalUtilsTest.java (original)
+++ tapestry/tapestry5/trunk/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/util/InternalUtilsTest.java Fri Sep 12 11:15:28 2008
@@ -96,22 +96,44 @@
     @Test
     public void array_size_when_non_null()
     {
-        Object[] array = { 1, 2, 3 };
+        Object[] array = {1, 2, 3};
 
         assertEquals(InternalUtils.size(array), 3);
     }
 
-    @Test(dataProvider = "memberPrefixData")
-    public void strip_member_prefix(String input, String expected)
+    @Test(dataProvider = "memberNameData")
+    public void strip_member_name(String input, String expected)
     {
-        assertEquals(InternalUtils.stripMemberPrefix(input), expected);
+        assertEquals(InternalUtils.stripMemberName(input), expected);
     }
 
-    @DataProvider(name = "memberPrefixData")
-    public Object[][] memberPrefixData()
+    @DataProvider(name = "memberNameData")
+    public Object[][] memberNameData()
     {
-        return new Object[][] { { "simple", "simple" }, { "_name", "name" }, { "$name", "name" },
-                { "$_$__$__$_$___$_$_$_$$name$", "name$" } };
+        return new Object[][] {
+                {"simple", "simple"},
+                {"_name", "name"},
+                {"$name", "name"},
+                {"ruby_style", "ruby_style"},
+                {"__$ruby_style_", "ruby_style"},
+                {"$_$__$__$_$___$_$_$_$$name$", "name"},
+                {"foo_", "foo"},
+                {"_foo_", "foo"}
+        };
+    }
+
+    @Test
+    public void strip_illegal_member_name()
+    {
+        try
+        {
+            InternalUtils.stripMemberName("!foo");
+            unreachable();
+        }
+        catch (IllegalArgumentException ex)
+        {
+            assertEquals(ex.getMessage(), "Input '!foo' is not a valid Java identifier.");
+        }
     }
 
     @Test
@@ -153,7 +175,6 @@
     {
         List<String> many = Arrays.asList("fred", "barney", "", "wilma");
         assertEquals(InternalUtils.join(many), "fred, barney, (blank), wilma");
-
     }
 
     @Test
@@ -175,7 +196,6 @@
         List<String> unsorted = Arrays.asList("betty", "fred", "barney", "", "wilma");
 
         assertEquals(InternalUtils.joinSorted(unsorted), "(blank), barney, betty, fred, wilma");
-
     }
 
     @Test(dataProvider = "capitalize_inputs")
@@ -187,8 +207,8 @@
     @DataProvider(name = "capitalize_inputs")
     public Object[][] capitalize_inputs()
     {
-        return new Object[][] { { "hello", "Hello" }, { "Goodbye", "Goodbye" }, { "", "" }, { "a", "A" },
-                { "A", "A" } };
+        return new Object[][] {{"hello", "Hello"}, {"Goodbye", "Goodbye"}, {"", ""}, {"a", "A"},
+                {"A", "A"}};
     }
 
     @Test
@@ -333,7 +353,7 @@
 
         try
         {
-            InternalUtils.validateMarkerAnnotations(new Class[] { Inject.class, NotRetainedRuntime.class });
+            InternalUtils.validateMarkerAnnotations(new Class[] {Inject.class, NotRetainedRuntime.class});
             unreachable();
         }
         catch (IllegalArgumentException ex)
@@ -432,7 +452,6 @@
         {
             assertMessageContains(ex,
                                   "Constructor protected org.apache.tapestry5.ioc.internal.util.InternalUtilsTest$PublicInnerClass() is not public and may not be used for autobuilding an instance of the class.");
-
         }
     }
 }