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/11/08 01:13:35 UTC
svn commit: r712317 - in /tapestry/tapestry5/trunk/tapestry-core/src:
main/java/org/apache/tapestry5/annotations/
main/java/org/apache/tapestry5/internal/services/
main/java/org/apache/tapestry5/services/
main/resources/org/apache/tapestry5/internal/se...
Author: hlship
Date: Fri Nov 7 16:13:34 2008
New Revision: 712317
URL: http://svn.apache.org/viewvc?rev=712317&view=rev
Log:
TAP5-301: When @Property generated accessor methods will overwrite user generated accessors then either warn the user or do not generate them
Added:
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/GetterMethodAlreadyExists.java
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/PageActivationContext.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/Property.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/services/ServicesMessages.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/ClassTransformation.java
tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties
tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FormFragmentDemo.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Music.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/music/MusicDetails2.java
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/PageActivationContext.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/PageActivationContext.java?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/PageActivationContext.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/PageActivationContext.java Fri Nov 7 16:13:34 2008
@@ -22,16 +22,24 @@
/**
* Annotation for a field for which the page activation context handlers (onActivate and onPassivate) should be created.
- * In order to use this annotation you must contribute a {@link ValueEncoder} for the class of the annotated property.
+ * In order to use this annotation you must contribute a {@link org.apache.tapestry5.ValueEncoder} for the class of the
+ * annotated property.
+ * <p/>
+ * You should not use this annotation within a class that already has an onActivate() or onPassivate() method; doing so
+ * will result in a runtime exception.
*/
@Target(FIELD)
@Documented
@Retention(RUNTIME)
public @interface PageActivationContext
{
- /** Whether to create an activate event handler. */
+ /**
+ * Whether to create an activate event handler.
+ */
boolean activate() default true;
- /** Whether to create a passivate event handler */
+ /**
+ * Whether to create a passivate event handler
+ */
boolean passivate() default true;
}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/Property.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/Property.java?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/Property.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/annotations/Property.java Fri Nov 7 16:13:34 2008
@@ -24,6 +24,9 @@
* Annotation for fields for which accessor methods (getters and setters) should be created. This can help when
* defining the kind of placeholder properties often used in components, though the disadvantage is that you can't
* access the fields in a unit test, and you may get compiler warnings about unused private variables.
+ * <p/>
+ * The annotation <em>will not</em> overwrite an existing getter or setter method; if you put a Property annotation on a
+ * field that already has a getter or a setter you will see a runtime exception.
*/
@Target(FIELD)
@Documented
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=712317&r1=712316&r2=712317&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 Nov 7 16:13:34 2008
@@ -635,9 +635,54 @@
addMethodToDescription(action, signature, methodBody);
}
- public void addTransformedMethod(TransformMethodSignature methodSignature, String methodBody)
+ public void addTransformedMethod(TransformMethodSignature signature, String methodBody)
{
- addOrReplaceMethod(methodSignature, methodBody, false);
+ failIfFrozen();
+
+ CtClass returnType = findCtClass(signature.getReturnType());
+ CtClass[] parameters = buildCtClassList(signature.getParameterTypes());
+ CtClass[] exceptions = buildCtClassList(signature.getExceptionTypes());
+
+
+ try
+ {
+ CtMethod existing = ctClass.getDeclaredMethod(signature.getMethodName(), parameters);
+
+ if (existing != null)
+ throw new RuntimeException(ServicesMessages.addNewMethodConflict(signature));
+ }
+ catch (NotFoundException ex)
+ {
+ // That's ok. Kind of sloppy to rely on a thrown exception; wish getDeclaredMethod()
+ // would return null for
+ // that case. Alternately, we could maintain a set of the method signatures of declared
+ // or added methods.
+ }
+
+ try
+ {
+ CtMethod method = new CtMethod(returnType, signature.getMethodName(), parameters, ctClass);
+
+ // TODO: Check for duplicate method add
+
+ method.setModifiers(signature.getModifiers());
+
+ method.setBody(methodBody);
+ method.setExceptionTypes(exceptions);
+
+ ctClass.addMethod(method);
+ }
+ catch (CannotCompileException ex)
+ {
+ throw new MethodCompileException(ServicesMessages.methodCompileError(signature, methodBody, ex), methodBody,
+ ex);
+ }
+ catch (NotFoundException ex)
+ {
+ throw new RuntimeException(ex);
+ }
+
+ addMethodToDescription("add transformed", signature, methodBody);
}
private CtClass[] buildCtClassList(String[] typeNames)
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ServicesMessages.java Fri Nov 7 16:13:34 2008
@@ -422,4 +422,9 @@
{
return MESSAGES.format("document-missing-html-root", rootElementName);
}
+
+ public static String addNewMethodConflict(TransformMethodSignature signature)
+ {
+ return MESSAGES.format("add-new-method-conflict", signature);
+ }
}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/ClassTransformation.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/ClassTransformation.java?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/ClassTransformation.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry5/services/ClassTransformation.java Fri Nov 7 16:13:34 2008
@@ -129,9 +129,9 @@
/**
* Claims a field so as to ensure that only a single annotation is applied to any single field. When a
- * transformation occurs (driven by a field annotation), the field is claimed (using the annotation object
- * as the tag). If a field has multiple conflicting annotations, this will be discovered
- * when the code attempts to claim the field a second time.
+ * transformation occurs (driven by a field annotation), the field is claimed (using the annotation object as the
+ * tag). If a field has multiple conflicting annotations, this will be discovered when the code attempts to claim
+ * the field a second time.
*
* @param fieldName the name of the field that is being claimed
* @param tag a non-null object that represents why the field is being tagged (this is typically a specific
@@ -285,7 +285,7 @@
/**
* As with {@link #addMethod(TransformMethodSignature, String)}, but field references inside the method
- * <em>will</em> be transformed.
+ * <em>will</em> be transformed, and the method <em>must not already exist</em>.
*/
void addTransformedMethod(TransformMethodSignature methodSignature, String methodBody);
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/resources/org/apache/tapestry5/internal/services/ServicesStrings.properties Fri Nov 7 16:13:34 2008
@@ -96,3 +96,4 @@
forbid-instantiate-component-class=Component class %s may not be instantiated directly. You should use an @InjectPage or @InjectComponent annotation instead.
event-not-handled=Request event '%s' (on component %s) was not handled; you must provide a matching event handler method in the component or in one of its containers.
document-missing-html-root=The root element of the rendered document was <%s>, not <html>. A root element of <html> is needed when linking JavaScript and stylesheet resources.
+add-new-method-conflict=Unable to add new method %s as it already exists.
\ No newline at end of file
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/app1/Start.tml Fri Nov 7 16:13:34 2008
@@ -89,6 +89,10 @@
<a href="componentsNotInTemplateDemo">Components Not In Template Demo</a>
-- demo exception when a component is defined in the class but not present in the template
</li>
+ <li>
+ <a href="GetterMethodAlreadyExists">Getter Method Already Exists</a>
+ -- demo @Property attempting to overwrite an existing getter method
+ </li>
</ul>
</html>
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/IntegrationTests.java Fri Nov 7 16:13:34 2008
@@ -2503,4 +2503,14 @@
assertText("name-value", "Betty");
assertText("last-clicked", "Barney");
}
+
+ /**
+ * TAP5-309
+ */
+ public void conflict_between_property_annotation_and_existing_method()
+ {
+ start("Getter Method Already Exists");
+
+ assertTextPresent("Unable to add new method public final java.lang.String getName() as it already exists.");
+ }
}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FormFragmentDemo.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FormFragmentDemo.java?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FormFragmentDemo.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FormFragmentDemo.java Fri Nov 7 16:13:34 2008
@@ -37,11 +37,6 @@
@InjectPage
private FormFragmentOutput outputPage;
- public SubscribeData getSubscribe()
- {
- return subscribe;
- }
-
void onPrepare()
{
subscribe = new SubscribeData();
Added: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/GetterMethodAlreadyExists.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/GetterMethodAlreadyExists.java?rev=712317&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/GetterMethodAlreadyExists.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/GetterMethodAlreadyExists.java Fri Nov 7 16:13:34 2008
@@ -0,0 +1,33 @@
+// 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.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.apache.tapestry5.integration.app1.pages;
+
+import org.apache.tapestry5.annotations.Property;
+
+public class GetterMethodAlreadyExists
+{
+ @Property
+ private String name;
+
+ public String getName()
+ {
+ return name;
+ }
+
+ public void setName(String name)
+ {
+ this.name = name;
+ }
+}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Music.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Music.java?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Music.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Music.java Fri Nov 7 16:13:34 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.
@@ -14,6 +14,7 @@
package org.apache.tapestry5.integration.app1.pages;
+import org.apache.tapestry5.annotations.Property;
import org.apache.tapestry5.integration.app1.data.Track;
import org.apache.tapestry5.integration.app1.services.MusicLibrary;
import org.apache.tapestry5.ioc.annotations.Inject;
@@ -25,20 +26,11 @@
@Inject
private MusicLibrary library;
+ @Property
private Track track;
public List<Track> getTracks()
{
return library.getTracks();
}
-
- public Track getTrack()
- {
- return track;
- }
-
- public void setTrack(Track track)
- {
- this.track = track;
- }
}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/music/MusicDetails2.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/music/MusicDetails2.java?rev=712317&r1=712316&r2=712317&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/music/MusicDetails2.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/music/MusicDetails2.java Fri Nov 7 16:13:34 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.
@@ -20,18 +20,8 @@
import org.apache.tapestry5.integration.app1.data.Track;
public class MusicDetails2
-{
+{
@Property
@PageActivationContext
private Track track;
-
- void onActivate(Track track)
- {
- throw new RuntimeException("onActivate invoked unexpectedly");
- }
-
- Object onPassivate()
- {
- throw new RuntimeException("onPassivate invoked unexpectedly");
- }
}