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 2006/10/19 20:33:51 UTC
svn commit: r465715 - in /tapestry/tapestry5/tapestry-core/trunk/src:
main/java/org/apache/tapestry/internal/model/
main/java/org/apache/tapestry/internal/services/
main/java/org/apache/tapestry/model/ main/java/org/apache/tapestry/util/
test/java/org/...
Author: hlship
Date: Thu Oct 19 11:33:48 2006
New Revision: 465715
URL: http://svn.apache.org/viewvc?view=rev&rev=465715
Log:
Handle the case where a parent class contains a persistent field with the same name as a persistent field in a subclass.
Modified:
tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/model/MutableComponentModelImpl.java
tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ComponentClassTransformerImpl.java
tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/PersistWorker.java
tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/model/MutableComponentModel.java
tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/util/IdAllocator.java
tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/model/MutableComponentModelImplTest.java
tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/util/IdAllocatorTest.java
Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/model/MutableComponentModelImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/model/MutableComponentModelImpl.java?view=diff&rev=465715&r1=465714&r2=465715
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/model/MutableComponentModelImpl.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/model/MutableComponentModelImpl.java Thu Oct 19 11:33:48 2006
@@ -25,12 +25,14 @@
import org.apache.tapestry.Resource;
import org.apache.tapestry.internal.annotations.SuppressNullCheck;
import org.apache.tapestry.internal.ioc.IOCUtilities;
+import org.apache.tapestry.internal.util.InternalUtils;
import org.apache.tapestry.model.ComponentModel;
import org.apache.tapestry.model.EmbeddedComponentModel;
import org.apache.tapestry.model.MutableComponentModel;
import org.apache.tapestry.model.MutableEmbeddedComponentModel;
import org.apache.tapestry.model.ParameterModel;
import org.apache.tapestry.util.Defense;
+import org.apache.tapestry.util.IdAllocator;
/**
* Internal implementation of {@link org.apache.tapestry.model.MutableComponentModel}.
@@ -43,6 +45,8 @@
private final String _componentClassName;
+ private final IdAllocator _persistentFieldNameAllocator = new IdAllocator();
+
private final Log _log;
private Map<String, ParameterModel> _parameters;
@@ -60,6 +64,16 @@
_log = log;
_baseResource = baseResource;
_parentModel = parentModel;
+
+ // Pre-allocate names from the parent, to avoid name collisions.
+
+ if (_parentModel != null)
+ {
+ for (String name : _parentModel.getPersistentFieldNames())
+ {
+ _persistentFieldNameAllocator.allocateId(name);
+ }
+ }
}
@Override
@@ -189,27 +203,22 @@
public List<String> getPersistentFieldNames()
{
- List<String> result = newList();
-
- if (_persistentFields != null)
- result.addAll(_persistentFields.keySet());
-
- if (_parentModel != null)
- result.addAll(_parentModel.getPersistentFieldNames());
-
- Collections.sort(result);
-
- return result;
+ // The name allocator is
+ return _persistentFieldNameAllocator.getAllocatedIds();
}
- public void setFieldPersistenceStrategy(String fieldName, String strategy)
+ public String setFieldPersistenceStrategy(String fieldName, String strategy)
{
- // TODO: Check for name collision here, or in parent
+ String stripped = InternalUtils.stripMemberPrefix(fieldName);
+
+ String logicalFieldName = _persistentFieldNameAllocator.allocateId(stripped);
if (_persistentFields == null)
_persistentFields = newMap();
- _persistentFields.put(fieldName, strategy);
+ _persistentFields.put(logicalFieldName, strategy);
+
+ return logicalFieldName;
}
}
Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ComponentClassTransformerImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ComponentClassTransformerImpl.java?view=diff&rev=465715&r1=465714&r2=465715
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ComponentClassTransformerImpl.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/ComponentClassTransformerImpl.java Thu Oct 19 11:33:48 2006
@@ -35,8 +35,6 @@
/**
* Implementation of {@link org.apache.tapestry.internal.services.ComponentClassTransformer}.
- *
- *
*/
@Concurrent
public class ComponentClassTransformerImpl implements ComponentClassTransformer,
@@ -91,13 +89,15 @@
Log log = _logSource.getLog(classname);
- // If the parent class is in a controlled package, it will already have been loaded and
- // transformed (that is driven by the ComponentInstantiatorSource).
+ // If the parent class is in a controlled package (and has the ComponentClass annotation),
+ // it will already have been loaded and transformed (that is driven by the
+ // ComponentInstantiatorSource).
InternalClassTransformation parentTransformation = _nameToClassTransformation
.get(parentClassname);
- // TODO: Check that the name is not already in the map.
+ // TODO: Check that the name is not already in the map. But I think that can't happen,
+ // because the classloader itself is synchronized.
InternalClassTransformation transformation = parentTransformation == null ? new InternalClassTransformationImpl(
ctClass, classLoader, log)
@@ -105,17 +105,18 @@
log);
// Not all classes in the packages are components. That's not just sloppy coding by
- // application developers, it also represents inner classes (including anonymous classes).
+ // application developers, it also represents inner classes (including anonymous inner
+ // classes).
if (transformation.getAnnotation(ComponentClass.class) == null)
return;
- // TODO: child class model should start as deep copy of parent model?
- // Or have pointer to parent model? Or something.
-
Resource baseResource = new ClasspathResource(classname.replace(".", "/") + ".class");
- MutableComponentModel model = new MutableComponentModelImpl(classname, log, baseResource, null);
+ ComponentModel parentModel = _nameToComponentModel.get(parentClassname);
+
+ MutableComponentModel model = new MutableComponentModelImpl(classname, log, baseResource,
+ parentModel);
_workerChain.transform(transformation, model);
Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/PersistWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/PersistWorker.java?view=diff&rev=465715&r1=465714&r2=465715
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/PersistWorker.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/internal/services/PersistWorker.java Thu Oct 19 11:33:48 2006
@@ -66,7 +66,7 @@
// Record the type of persistence, until needed later.
- model.setFieldPersistenceStrategy(fieldName, annotation.value());
+ String logicalFieldName = model.setFieldPersistenceStrategy(fieldName, annotation.value());
String defaultFieldName = transformation.addField(Modifier.PRIVATE, fieldType, fieldName
+ "_default");
@@ -88,7 +88,7 @@
BodyBuilder builder = new BodyBuilder();
builder.begin();
- builder.addln("%s.postFieldChange(\"%s\", ($w) $1);", resourcesFieldName, fieldName);
+ builder.addln("%s.postFieldChange(\"%s\", ($w) $1);", resourcesFieldName, logicalFieldName);
builder.addln("%s = $1;", fieldName);
builder.end();
@@ -103,16 +103,17 @@
// Check to see if there's a recorded change for this component, this field.
- builder.addln("if (%s.hasFieldChange(\"%s\"))", resourcesFieldName, fieldName);
+ builder.addln("if (%s.hasFieldChange(\"%s\"))", resourcesFieldName, logicalFieldName);
String wrapperType = TransformUtils.getWrapperTypeName(fieldType);
// Get the value, cast it to the correct type (or wrapper type)
builder.add(
- " %s = ((%s) %s.getFieldChange(\"%1$s\"))",
+ " %s = ((%s) %s.getFieldChange(\"%s\"))",
fieldName,
wrapperType,
- resourcesFieldName);
+ resourcesFieldName,
+ logicalFieldName);
// For primtive types, add in the method call to unwrap the wrapper type to a primitive type
Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/model/MutableComponentModel.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/model/MutableComponentModel.java?view=diff&rev=465715&r1=465714&r2=465715
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/model/MutableComponentModel.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/model/MutableComponentModel.java Thu Oct 19 11:33:48 2006
@@ -14,6 +14,9 @@
package org.apache.tapestry.model;
+import org.apache.tapestry.annotations.Persist;
+import org.apache.tapestry.internal.InternalComponentResources;
+
/**
* Mutable version of {@link org.apache.tapestry.model.ComponentModel} used during the
* transformation phase.
@@ -47,7 +50,18 @@
String componentClassName);
/**
- * @see ComponentModel#getFieldPersistenceStrategy(String)
+ * Used to define the field persistence strategy for a particular field name. Returns a logical
+ * name for the field, which is guaranteed to be unique (this is necessary for handling the case
+ * where a subclass has a persistent field with the same name as a persistent field from a
+ * super-class).
+ *
+ * @param fieldName
+ * the name of the field which is to be made persistent
+ * @param strategy
+ * the strategy for persisting the field, from {@link Persist#value()}
+ * @return a logical name for the field, to be used with
+ * {@link ComponentModel#getFieldPersistenceStrategy(String)}, and with
+ * {@link InternalComponentResources#postFieldChange(String, Object)}, etc.
*/
- void setFieldPersistenceStrategy(String fieldName, String strategy);
+ String setFieldPersistenceStrategy(String fieldName, String strategy);
}
Modified: tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/util/IdAllocator.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/util/IdAllocator.java?view=diff&rev=465715&r1=465714&r2=465715
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/util/IdAllocator.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/main/java/org/apache/tapestry/util/IdAllocator.java Thu Oct 19 11:33:48 2006
@@ -18,20 +18,23 @@
import java.util.HashMap;
import java.util.IdentityHashMap;
+import java.util.List;
import java.util.Map;
+import org.apache.tapestry.internal.ioc.IOCUtilities;
+
/**
* Used to "uniquify" names within a given context. A base name is passed in, and the return value
* is the base name, or the base name extended with a suffix to make it unique.
- *
- * @author Howard Lewis Ship
- * @since 3.0
+ * <p>
+ * This class is not threadsafe.
*/
public final class IdAllocator
{
private static final String SEPARATOR = "_";
+ /** Map from allocated id to a generator for names associated with the allocated id. */
private final Map<String, NameGenerator> _generatorMap;
private final String _namespace;
@@ -87,6 +90,12 @@
_generatorMap = generatorMap;
}
+ /** Returns a list of all allocated ids, sorted alphabetically. */
+ public List<String> getAllocatedIds()
+ {
+ return IOCUtilities.sortedKeys(_generatorMap);
+ }
+
/**
* Creates a clone of this IdAllocator instance, copying the allocator's namespace and key map.
*/
@@ -147,6 +156,12 @@
_generatorMap.put(result, g);
return result;
+ }
+
+ /** Checks to see if a given name has been allocated. */
+ public boolean isAllocated(String name)
+ {
+ return _generatorMap.containsKey(name);
}
/**
Modified: tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/model/MutableComponentModelImplTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/model/MutableComponentModelImplTest.java?view=diff&rev=465715&r1=465714&r2=465715
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/model/MutableComponentModelImplTest.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/internal/model/MutableComponentModelImplTest.java Thu Oct 19 11:33:48 2006
@@ -265,8 +265,26 @@
MutableComponentModel model = new MutableComponentModelImpl(CLASS_NAME, log, r, null);
- model.setFieldPersistenceStrategy("fred", "session");
- model.setFieldPersistenceStrategy("barney", "client");
+ assertEquals(model.setFieldPersistenceStrategy("fred", "session"), "fred");
+ assertEquals(model.setFieldPersistenceStrategy("barney", "client"), "barney");
+
+ assertEquals(model.getPersistentFieldNames(), Arrays.asList("barney", "fred"));
+
+ verify();
+ }
+
+ @Test
+ public void persistent_field_names_have_punctuation_stripped()
+ {
+ Resource r = newResource();
+ Log log = newLog();
+
+ replay();
+
+ MutableComponentModel model = new MutableComponentModelImpl(CLASS_NAME, log, r, null);
+
+ assertEquals(model.setFieldPersistenceStrategy("_fred", "session"), "fred");
+ assertEquals(model.setFieldPersistenceStrategy("_$barney", "client"), "barney");
assertEquals(model.getPersistentFieldNames(), Arrays.asList("barney", "fred"));
@@ -282,14 +300,35 @@
replay();
MutableComponentModel parent = new MutableComponentModelImpl(CLASS_NAME, log, r, null);
- MutableComponentModel model = new MutableComponentModelImpl(CLASS_NAME, log, r, parent);
- parent.setFieldPersistenceStrategy("wilma", "session");
+ assertEquals(parent.setFieldPersistenceStrategy("wilma", "session"), "wilma");
+
+ MutableComponentModel model = new MutableComponentModelImpl(CLASS_NAME, log, r, parent);
- model.setFieldPersistenceStrategy("fred", "session");
- model.setFieldPersistenceStrategy("barney", "client");
+ assertEquals(model.setFieldPersistenceStrategy("fred", "session"), "fred");
+ assertEquals(model.setFieldPersistenceStrategy("barney", "client"), "barney");
assertEquals(model.getPersistentFieldNames(), Arrays.asList("barney", "fred", "wilma"));
+
+ verify();
+ }
+
+ @Test
+ public void persistent_field_names_allocated_in_subclasses_are_unique()
+ {
+ Resource r = newResource();
+ Log log = newLog();
+
+ replay();
+
+ MutableComponentModel parent = new MutableComponentModelImpl(CLASS_NAME, log, r, null);
+ assertEquals(parent.setFieldPersistenceStrategy("wilma", "session"), "wilma");
+
+ MutableComponentModel model = new MutableComponentModelImpl(CLASS_NAME, log, r, parent);
+
+ assertEquals(model.setFieldPersistenceStrategy("wilma", "session"), "wilma_0");
+
+ assertEquals(model.getPersistentFieldNames(), Arrays.asList("wilma", "wilma_0"));
verify();
}
Modified: tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/util/IdAllocatorTest.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/util/IdAllocatorTest.java?view=diff&rev=465715&r1=465714&r2=465715
==============================================================================
--- tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/util/IdAllocatorTest.java (original)
+++ tapestry/tapestry5/tapestry-core/trunk/src/test/java/org/apache/tapestry/util/IdAllocatorTest.java Thu Oct 19 11:33:48 2006
@@ -14,27 +14,46 @@
package org.apache.tapestry.util;
-import static org.testng.Assert.assertEquals;
+import static org.apache.tapestry.util.CollectionFactory.newList;
+import java.util.List;
+
+import org.testng.Assert;
import org.testng.annotations.Test;
-/**
- * Tests the {@link org.apache.tapestry.util.IdAllocator}class.
- *
- * @author Howard Lewis Ship
- */
-public class IdAllocatorTest
+public class IdAllocatorTest extends Assert
{
@Test
public void simple()
{
IdAllocator a = new IdAllocator();
+ List<String> ids = newList();
+
+ assertFalse(a.isAllocated("name"));
assertEquals(a.allocateId("name"), "name");
+ assertTrue(a.isAllocated("name"));
+
+ ids.add("name");
for (int i = 0; i < 10; i++)
- assertEquals(a.allocateId("name"), "name_" + i);
+ {
+
+ String expected = "name_" + i;
+
+ assertFalse(a.isAllocated(expected));
+
+ String nextId = a.allocateId("name");
+
+ assertTrue(a.isAllocated(expected));
+
+ assertEquals(nextId, expected);
+
+ ids.add(nextId);
+ }
+
+ assertEquals(a.getAllocatedIds(), ids);
}
@Test