Hi,
I created the issue https://issues.apache.org/jira/browse/TAP5-2619. I
also added a patch for fixing it. I am attaching the patch file to this
email as well. I also copied the issue description to the end of this email.
As I'm planning in applying the same fix for my companies product, I
would like feedback on the correctness and stability of this fix.
Regards,
Raigo
*What happens*
A page has two class fields. One is a flash persisted field (call it
A) and the other is a session persisted field (call it B). In a single
request i set field A and read field B. In the next request field A
has a value of null (the value is gone).
*What I expect to happen*
In the next request field A should have the value it was set in the
previous request.
**I've done some research into the causes of this bug and a fix for it.
*When did the bug appear*
I'm certain enough that the bug appeared with the commit
435997738cee2cb9ea8b4b6ec3e5227dfdb2dc91. In it, restoring the state
of persisted values became more lazy. If I revert the lazyness with
restoring the persisted field state at page attach, the bug disappears.
*What causes the bug*
There is an operation the result of this is cached. This is called
gatherChanges from persistentFieldManager. This is operation has the
side-effect of nulling the session value of all flash persisted fields
of the page.
Previously this operation was called at the page attach stage. Now it
is more lazy and is called when a persisted field is read. But not
when it is set. In the buggy use case, the flash persisted field sets
the session value, reading the other persisted field calls
gatherChanges. The flash persisted field clears the field value that
was just set. By the next request, the flash persisted value is gone.
*How to fix*
The fix is to call and cache gatherChanges at setting a persisted
field in addition to getting it. Then the order of operation
guarantees correct result.
>From 0dd7a02351203da64733be4ee0f357bf10e5ff5a Mon Sep 17 00:00:00 2001
From: Raigo Aljand <ra...@smartaccounts.eu>
Date: Thu, 30 Jan 2020 10:39:34 +0200
Subject: [PATCH] TAP5-2619: Flash persisted value cleared too early
Restore the proper order of operations: first gatherChanges, then
setting the session value.
---
.../tapestry5/internal/structure/PageImpl.java | 8 +++++++-
.../integration/app1/pages/FlashDemo.java | 18 ++++++++++++++----
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java
index 6b9a90f12..0b3cc8ea7 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/PageImpl.java
@@ -1,4 +1,4 @@
-// Copyright 2006-2014 The Apache Software Foundation
+// Copyright 2020 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.
@@ -263,6 +263,12 @@ public class PageImpl implements Page
throw new RuntimeException(StructureMessages.persistChangeBeforeLoadComplete());
}
+ /*
+ * See TAP5-2619. Reading another persisted value caused a flash persisted value
+ * to sometimes be cleared too early.
+ */
+ fieldBundle.createObject();
+
persistentFieldManager.postChange(name, resources, fieldName, newValue);
}
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FlashDemo.java b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FlashDemo.java
index a39990cfb..8097fd04f 100644
--- a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FlashDemo.java
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/FlashDemo.java
@@ -1,4 +1,4 @@
-// Copyright 2007 The Apache Software Foundation
+// Copyright 2020 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,16 +14,20 @@
package org.apache.tapestry5.integration.app1.pages;
-import org.apache.tapestry5.annotations.Meta;
+import java.util.Objects;
+
+import org.apache.tapestry5.PersistenceConstants;
import org.apache.tapestry5.annotations.Persist;
import org.apache.tapestry5.integration.app1.base.BaseComponent;
-@Meta("tapestry.persistence-strategy=flash")
public class FlashDemo extends BaseComponent
{
- @Persist
+ @Persist(PersistenceConstants.FLASH)
private String message;
+ @Persist(PersistenceConstants.SESSION)
+ private String persistField;
+
public String getMessage()
{
return message;
@@ -32,5 +36,11 @@ public class FlashDemo extends BaseComponent
void onAction()
{
message = "You clicked the link!";
+
+ /*
+ * See TAP5-2619. Reading another persisted value caused a flash persisted value
+ * to sometimes be cleared too early.
+ */
+ Objects.toString(persistField);
}
}
--
2.17.1
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tapestry.apache.org
For additional commands, e-mail: dev-h...@tapestry.apache.org
