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/04/15 19:55:54 UTC
svn commit: r648355 - in /tapestry/tapestry5/trunk/tapestry-core/src:
main/java/org/apache/tapestry/annotations/
main/java/org/apache/tapestry/internal/transform/ test/app1/
test/java/org/apache/tapestry/integration/
test/java/org/apache/tapestry/integ...
Author: hlship
Date: Tue Apr 15 10:55:40 2008
New Revision: 648355
URL: http://svn.apache.org/viewvc?rev=648355&view=rev
Log:
TAPESTRY-2338: Cached values for methods annotated with @Cached do not reset at end of Ajax request
Added:
tapestry/tapestry5/trunk/tapestry-core/src/test/app1/CleanCacheDemo.tml
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/CleanCacheDemo.java
Modified:
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/Cached.java
tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/transform/CachedWorker.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java
tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/Start.java
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/Cached.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/Cached.java?rev=648355&r1=648354&r2=648355&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/Cached.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/annotations/Cached.java Tue Apr 15 10:55:40 2008
@@ -14,28 +14,25 @@
package org.apache.tapestry.annotations;
-import java.lang.annotation.Documented;
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
+import java.lang.annotation.*;
-/** Indicates that a method should only be evaluated once and the result cached.
- * All further calls to the method will return the cached result. Note that this
- * annotation is inheritence-safe; if a subclass calls a superclass method that
- * has \@Cached then the value the subclass method gets is the cached value.
- * <p>
- * The watch parameter can be passed a binding expression
- * which will be evaluated each time the method is called. The method will then
- * only be executed the first time it is called and after that only when the
- * value of the binding changes. This can be used, for instance, to have the
- * method only evaluated once per iteration of a loop by setting watch to
- * the value or index of the loop.
+/**
+ * Indicates that a method should only be evaluated once and the result cached. All further calls to the method will
+ * return the cached result. Note that this annotation is inheritence-safe; if a subclass calls a superclass method that
+ * has \@Cached then the value the subclass method gets is the cached value.
+ * <p/>
+ * The watch parameter can be passed a binding expression which will be evaluated each time the method is called. The
+ * method will then only be executed the first time it is called and after that only when the value of the binding
+ * changes. This can be used, for instance, to have the method only evaluated once per iteration of a loop by setting
+ * watch to the value or index of the loop.
*/
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
@Documented
-public @interface Cached {
- /** The optional binding to watch (default binding prefix is "prop") */
- String watch() default "";
+public @interface Cached
+{
+ /**
+ * The optional binding to watch (default binding prefix is "prop").
+ */
+ String watch() default "";
}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/transform/CachedWorker.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/transform/CachedWorker.java?rev=648355&r1=648354&r2=648355&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/transform/CachedWorker.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/main/java/org/apache/tapestry/internal/transform/CachedWorker.java Tue Apr 15 10:55:40 2008
@@ -14,115 +14,127 @@
package org.apache.tapestry.internal.transform;
-import static java.lang.reflect.Modifier.PRIVATE;
-
-import java.util.List;
-
import org.apache.tapestry.Binding;
import org.apache.tapestry.TapestryConstants;
import org.apache.tapestry.annotations.Cached;
import org.apache.tapestry.ioc.util.BodyBuilder;
import org.apache.tapestry.model.MutableComponentModel;
-import org.apache.tapestry.services.BindingSource;
-import org.apache.tapestry.services.ClassTransformation;
-import org.apache.tapestry.services.ComponentClassTransformWorker;
-import org.apache.tapestry.services.TransformConstants;
-import org.apache.tapestry.services.TransformMethodSignature;
-import org.apache.tapestry.services.TransformUtils;
-
-/** Caches method return values for methods annotated with {@link Cached}. */
-public class CachedWorker implements ComponentClassTransformWorker {
- private final BindingSource _bindingSource;
-
- public CachedWorker(BindingSource bindingSource) {
- super();
- this._bindingSource = bindingSource;
- }
-
- public void transform(ClassTransformation transformation, MutableComponentModel model) {
- List<TransformMethodSignature> methods = transformation.findMethodsWithAnnotation(Cached.class);
- if (methods.isEmpty())
- return;
-
- for(TransformMethodSignature method : methods) {
- if (method.getReturnType().equals("void"))
- throw new IllegalArgumentException(TransformMessages.cachedMethodMustHaveReturnValue(method));
-
- if (method.getParameterTypes().length != 0)
- throw new IllegalArgumentException(TransformMessages.cachedMethodsHaveNoParameters(method));
-
- String propertyName = method.getMethodName();
-
- // add a property to store whether or not the method has been called
- String fieldName = transformation.addField(PRIVATE, method.getReturnType(), propertyName);
- String calledField = transformation.addField(PRIVATE, "boolean", fieldName + "$called");
-
- Cached once = transformation.getMethodAnnotation(method, Cached.class);
- String bindingField = null;
- String bindingValueField = null;
- boolean watching = once.watch().length() > 0;
- if (watching) {
- // add fields to store the binding and the value
- bindingField = transformation.addField(PRIVATE, Binding.class.getCanonicalName(), fieldName + "$binding");
- bindingValueField = transformation.addField(PRIVATE, "java.lang.Object", fieldName + "$bindingValue");
-
- String bindingSourceField = transformation.addInjectedField(BindingSource.class, fieldName + "$bindingsource", _bindingSource);
-
- String body = String.format("%s = %s.newBinding(\"Watch expression\", %s, \"%s\", \"%s\");",
- bindingField,
- bindingSourceField,
- transformation.getResourcesFieldName(),
- TapestryConstants.PROP_BINDING_PREFIX,
- once.watch());
- transformation.extendMethod(TransformConstants.CONTAINING_PAGE_DID_LOAD_SIGNATURE, body);
- }
-
- BodyBuilder b = new BodyBuilder();
-
- // on cleanup, reset the field values
- b.begin();
-
- if (!TransformUtils.isPrimitive(method.getReturnType()))
- b.addln("%s = null;", fieldName);
- b.addln("%s = false;", calledField);
-
- if (watching)
- b.addln("%s = null;", bindingValueField);
-
- b.end();
- transformation.extendMethod(TransformConstants.POST_RENDER_CLEANUP_SIGNATURE, b.toString());
-
- // prefix the existing method to cache the result
- b.clear();
- b.begin();
-
- // if it has been called and watch is set and the old value is the same as the new value then return
- // get the old value and cache it
- /* NOTE: evaluates the binding twice when checking the new value.
- * this is probably not a problem because in most cases properties
- * that are being watched are not expensive operations. plus, we
- * never guaranteed that it would be called exactly once when
- * watching.
- */
- if (watching) {
- b.addln("if (%s && %s == %s.get()) return %s;",
- calledField, bindingValueField, bindingField, fieldName);
- b.addln("%s = %s.get();", bindingValueField, bindingField);
- }
- else {
- b.addln("if (%s) return %s;", calledField, fieldName);
- }
-
- b.addln("%s = true;", calledField);
- b.end();
- transformation.prefixMethod(method, b.toString());
-
- // cache the return value
- b.clear();
- b.begin();
- b.addln("%s = $_;", fieldName);
- b.end();
- transformation.extendExistingMethod(method, b.toString());
- }
- }
+import org.apache.tapestry.services.*;
+
+import static java.lang.reflect.Modifier.PRIVATE;
+import java.util.List;
+
+/**
+ * Caches method return values for methods annotated with {@link Cached}.
+ */
+public class CachedWorker implements ComponentClassTransformWorker
+{
+ private final BindingSource _bindingSource;
+
+ public CachedWorker(BindingSource bindingSource)
+ {
+ _bindingSource = bindingSource;
+ }
+
+ public void transform(ClassTransformation transformation, MutableComponentModel model)
+ {
+ List<TransformMethodSignature> methods = transformation.findMethodsWithAnnotation(Cached.class);
+ if (methods.isEmpty())
+ return;
+
+ for (TransformMethodSignature method : methods)
+ {
+ if (method.getReturnType().equals("void"))
+ throw new IllegalArgumentException(TransformMessages.cachedMethodMustHaveReturnValue(method));
+
+ if (method.getParameterTypes().length != 0)
+ throw new IllegalArgumentException(TransformMessages.cachedMethodsHaveNoParameters(method));
+
+ String propertyName = method.getMethodName();
+
+ // add a property to store whether or not the method has been called
+ String fieldName = transformation.addField(PRIVATE, method.getReturnType(), propertyName);
+ String calledField = transformation.addField(PRIVATE, "boolean", fieldName + "$called");
+
+ Cached once = transformation.getMethodAnnotation(method, Cached.class);
+ String bindingField = null;
+ String bindingValueField = null;
+ boolean watching = once.watch().length() > 0;
+
+ if (watching)
+ {
+ // add fields to store the binding and the value
+ bindingField = transformation.addField(PRIVATE, Binding.class.getCanonicalName(),
+ fieldName + "$binding");
+ bindingValueField = transformation.addField(PRIVATE, "java.lang.Object", fieldName + "$bindingValue");
+
+ String bindingSourceField = transformation.addInjectedField(BindingSource.class,
+ fieldName + "$bindingsource",
+ _bindingSource);
+
+ String body = String.format("%s = %s.newBinding(\"Watch expression\", %s, \"%s\", \"%s\");",
+ bindingField,
+ bindingSourceField,
+ transformation.getResourcesFieldName(),
+ TapestryConstants.PROP_BINDING_PREFIX,
+ once.watch());
+
+ transformation.extendMethod(TransformConstants.CONTAINING_PAGE_DID_LOAD_SIGNATURE, body);
+ }
+
+ BodyBuilder b = new BodyBuilder();
+
+ // on cleanup, reset the field values
+ b.begin();
+
+ if (!TransformUtils.isPrimitive(method.getReturnType()))
+ b.addln("%s = null;", fieldName);
+ b.addln("%s = false;", calledField);
+
+ if (watching)
+ b.addln("%s = null;", bindingValueField);
+
+ b.end();
+
+ // TAPESTRY-2338: Cleanup at page detach, not render cleanup. In an Ajax request, the rendering
+ // objects may reference properties of components that don't render and so won't execute the
+ // PostCleanupRender phase.
+
+ transformation.extendMethod(TransformConstants.CONTAINING_PAGE_DID_DETACH_SIGNATURE, b.toString());
+
+ // prefix the existing method to cache the result
+ b.clear();
+ b.begin();
+
+ // if it has been called and watch is set and the old value is the same as the new value then return
+ // get the old value and cache it
+ /* NOTE: evaluates the binding twice when checking the new value.
+ * this is probably not a problem because in most cases properties
+ * that are being watched are not expensive operations. plus, we
+ * never guaranteed that it would be called exactly once when
+ * watching.
+ */
+ if (watching)
+ {
+ b.addln("if (%s && %s == %s.get()) return %s;",
+ calledField, bindingValueField, bindingField, fieldName);
+ b.addln("%s = %s.get();", bindingValueField, bindingField);
+ }
+ else
+ {
+ b.addln("if (%s) return %s;", calledField, fieldName);
+ }
+
+ b.addln("%s = true;", calledField);
+ b.end();
+ transformation.prefixMethod(method, b.toString());
+
+ // cache the return value
+ b.clear();
+ b.begin();
+ b.addln("%s = $_;", fieldName);
+ b.end();
+ transformation.extendExistingMethod(method, b.toString());
+ }
+ }
}
Added: tapestry/tapestry5/trunk/tapestry-core/src/test/app1/CleanCacheDemo.tml
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/app1/CleanCacheDemo.tml?rev=648355&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/app1/CleanCacheDemo.tml (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/app1/CleanCacheDemo.tml Tue Apr 15 10:55:40 2008
@@ -0,0 +1,24 @@
+<html t:type="border" xmlns:t="http://tapestry.apache.org/schema/tapestry_5_0_0.xsd">
+
+
+ <p>
+ The following two numbers will be identical, due to @Cache:
+ </p>
+
+ <div t:type="zone" t:id="aZone">
+ <ul>
+ <li id="time1">${timeNanos}</li>
+ <li id="time2">${timeNanos}</li>
+ </ul>
+
+
+ </div>
+
+ <p>
+ Clicking
+ <a t:type="actionlink" t:id="updateZone" href="#" t:zone="aZone">update</a>
+ will refresh the numbers.
+ </p>
+
+
+</html>
\ No newline at end of file
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java?rev=648355&r1=648354&r2=648355&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/IntegrationTests.java Tue Apr 15 10:55:40 2008
@@ -1445,7 +1445,7 @@
public void action_links_on_custom_url()
{
open(BASE_URL + "nested/actiondemo/");
-
+
clickAndWait("link=2");
assertTextPresent("Number: 2");
@@ -1897,12 +1897,26 @@
{
start("Cached Annotation");
- assertText("//span[@id='value']", "000");
- assertText("//span[@id='value2size']", "111");
+ assertText("value", "000");
+ assertText("value2size", "111");
assertText("//span[@class='watch'][1]", "0");
assertText("//span[@class='watch'][2]", "0");
assertText("//span[@class='watch'][3]", "1");
+
+ clickAndWait("link=Back to index");
+
+ // TAPESTRY-2338: Make sure the data is cleared.
+
+ clickAndWait("link=Cached Annotation");
+
+ assertText("value", "000");
+ assertText("value2size", "111");
+
+ assertText("//span[@class='watch'][1]", "0");
+ assertText("//span[@class='watch'][2]", "0");
+ assertText("//span[@class='watch'][3]", "1");
+
}
/**
@@ -1912,7 +1926,16 @@
public void override_method_with_cached()
{
start("Cached Annotation2");
- assertText("//span[@id='value']", "111");
+
+ assertText("value", "111");
+
+ clickAndWait("link=Back to index");
+
+ // TAPESTRY-2338: Make sure the data is cleared.
+
+ clickAndWait("link=Cached Annotation2");
+
+ assertText("value", "111");
}
private void sleep(long timeout)
@@ -1924,5 +1947,36 @@
catch (InterruptedException ex)
{
}
+ }
+
+ /**
+ * TAPESTRY-2338
+ */
+ @Test
+ public void cached_properties_cleared_at_end_of_request()
+ {
+ start("Clean Cache Demo");
+
+ String time1_1 = getText("time1");
+ String time1_2 = getText("time1");
+
+ // Don't know what they are but they should be the same.
+
+ assertEquals(time1_2, time1_1);
+
+ click("link=update");
+
+ sleep(250);
+
+ String time2_1 = getText("time1");
+ String time2_2 = getText("time1");
+
+ // Check that @Cache is still working
+
+ assertEquals(time2_2, time2_1);
+
+ assertFalse(time2_1.equals(time1_1),
+ "After update the nanoseconds time did not change, meaning @Cache was broken.");
+
}
}
Added: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/CleanCacheDemo.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/CleanCacheDemo.java?rev=648355&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/CleanCacheDemo.java (added)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/CleanCacheDemo.java Tue Apr 15 10:55:40 2008
@@ -0,0 +1,36 @@
+// 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.tapestry.integration.app1.pages;
+
+import org.apache.tapestry.annotations.Cached;
+import org.apache.tapestry.annotations.Component;
+import org.apache.tapestry.corelib.components.Zone;
+
+public class CleanCacheDemo
+{
+ @Component
+ private Zone aZone;
+
+ Object onActionFromUpdateZone()
+ {
+ return aZone;
+ }
+
+ @Cached
+ public long getTimeNanos()
+ {
+ return System.nanoTime();
+ }
+}
Modified: tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/Start.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/Start.java?rev=648355&r1=648354&r2=648355&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/Start.java (original)
+++ tapestry/tapestry5/trunk/tapestry-core/src/test/java/org/apache/tapestry/integration/app1/pages/Start.java Tue Apr 15 10:55:40 2008
@@ -63,6 +63,8 @@
private static final List<Item> ITEMS = CollectionFactory.newList(
new Item("actionpage", "Action Page", "tests fixture for ActionLink component"),
+ new Item("cleancachedemo", "Clean Cache Demo", "cache cleared properly during Ajax calls"),
+
new Item("numberbeaneditordemo", "Number BeanEditor Demo",
"use of nulls and wrapper types with BeanEditor"),
@@ -84,7 +86,8 @@
new Item("nested/AssetDemo", "AssetDemo", "declaring an image using Assets"),
- new Item("nested/ActionDemo", "Action With Context Demo", "using action links with context on page with activation context"),
+ new Item("nested/ActionDemo", "Action With Context Demo",
+ "using action links with context on page with activation context"),
new Item("blockdemo", "BlockDemo", "use of blocks to control rendering"),
@@ -246,13 +249,14 @@
new Item("TrackEditor", "Generic Page Class Demo",
"demo use of generics with component classes and, particularily, with property types"),
- new Item("IndirectProtectedFields", "Protected Fields Demo", "demo exception when component class contains protected fields"),
+ new Item("IndirectProtectedFields", "Protected Fields Demo",
+ "demo exception when component class contains protected fields"),
new Item("injectcomponentdemo", "Inject Component Demo",
"inject component defined in template"),
-
+
new Item("cachedpage", "Cached Annotation", "Caching method return values"),
-
+
new Item("cachedpage2", "Cached Annotation2", "Caching method return values w/ inheritence")
);
@@ -277,7 +281,7 @@
{
_item = item;
}
-
+
@InjectPage
private SecurePage _securePage;