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 2014/09/01 20:29:59 UTC
[1/2] git commit: Fix typo in JavaDoc
Repository: tapestry-5
Updated Branches:
refs/heads/master 42b0486e2 -> 021b23e18
Fix typo in JavaDoc
Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/57ced868
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/57ced868
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/57ced868
Branch: refs/heads/master
Commit: 57ced868d1aeea09c709cd1af7391d226b93bb3b
Parents: 42b0486
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Mon Sep 1 10:51:43 2014 -0700
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Mon Sep 1 10:51:43 2014 -0700
----------------------------------------------------------------------
.../org/apache/tapestry5/services/PersistentFieldStrategy.java | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/57ced868/tapestry-core/src/main/java/org/apache/tapestry5/services/PersistentFieldStrategy.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/services/PersistentFieldStrategy.java b/tapestry-core/src/main/java/org/apache/tapestry5/services/PersistentFieldStrategy.java
index 64774c4..76733ff 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/services/PersistentFieldStrategy.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/services/PersistentFieldStrategy.java
@@ -1,5 +1,3 @@
-// Copyright 2006, 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
@@ -41,7 +39,7 @@ public interface PersistentFieldStrategy
/**
* Discards any saved changes for the name page. There is no expectation that data already gathered from the
- * strategy and persumably dumped into component instance fields will be affected, but future field access (within
+ * strategy and presumably dumped into component instance fields will be affected, but future field access (within
* this request or a later one) will show no data for the indicated page.
*
* @param pageName logical name of page whose field persistent data should be discarded
[2/2] git commit: TAP5-2269: Client persistence strategy does not
detect changes to internal state of mutable objects
Posted by hl...@apache.org.
TAP5-2269: Client persistence strategy does not detect changes to internal state of mutable objects
Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/021b23e1
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/021b23e1
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/021b23e1
Branch: refs/heads/master
Commit: 021b23e18c1b67b0a932d19dc0080b82a82e4902
Parents: 57ced86
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Mon Sep 1 11:30:07 2014 -0700
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Mon Sep 1 11:30:07 2014 -0700
----------------------------------------------------------------------
54_RELEASE_NOTES.md | 7 +
.../ClientPersistentFieldStorageImpl.java | 44 ++++---
.../ClientPersistentFieldStorageImplTest.java | 128 +++++++++++++++----
3 files changed, 133 insertions(+), 46 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/021b23e1/54_RELEASE_NOTES.md
----------------------------------------------------------------------
diff --git a/54_RELEASE_NOTES.md b/54_RELEASE_NOTES.md
index 93d0646..19a598e 100644
--- a/54_RELEASE_NOTES.md
+++ b/54_RELEASE_NOTES.md
@@ -448,3 +448,10 @@ The automatic ValueEncoder from String to any Number type, or ti Boolean have
changed slightly. An empty input string is encoded to a null rather than being passed through
the type coercer. This reflects the desire that a submitted value (in a URL or a form) that is blank
is the same as no value: null.
+
+## Client Persistent Fields
+
+Tapestry now determines if a client-persistent field is mutable and contains changes, using the same
+checks as for session-persistent objects (the OptimizedSessionPersistedObject interface and so forth).
+Previously, Tapestry would load a mutable object into a field, but a change to the internal state
+of that mutable field would not always be preserved in the serialized object data attached to links.
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/021b23e1/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java
index 2d95767..a94ac4f 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java
@@ -1,5 +1,3 @@
-// Copyright 2007, 2008, 2009, 2011 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
@@ -19,10 +17,7 @@ import org.apache.tapestry5.ioc.ScopeConstants;
import org.apache.tapestry5.ioc.annotations.Scope;
import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
import org.apache.tapestry5.ioc.internal.util.InternalUtils;
-import org.apache.tapestry5.services.ClientDataEncoder;
-import org.apache.tapestry5.services.ClientDataSink;
-import org.apache.tapestry5.services.PersistentFieldChange;
-import org.apache.tapestry5.services.Request;
+import org.apache.tapestry5.services.*;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
@@ -66,7 +61,7 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
public PersistentFieldChange toChange(Object value)
{
return new PersistentFieldChangeImpl(componentId == null ? "" : componentId,
- fieldName, value);
+ fieldName, value);
}
@Override
@@ -102,8 +97,7 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
if (componentId == null)
{
if (other.componentId != null) return false;
- }
- else if (!componentId.equals(other.componentId)) return false;
+ } else if (!componentId.equals(other.componentId)) return false;
return true;
}
@@ -111,15 +105,18 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
private final ClientDataEncoder clientDataEncoder;
+ private final SessionPersistedObjectAnalyzer analyzer;
+
private final Map<Key, Object> persistedValues = CollectionFactory.newMap();
private String clientData;
private boolean mapUptoDate = false;
- public ClientPersistentFieldStorageImpl(Request request, ClientDataEncoder clientDataEncoder)
+ public ClientPersistentFieldStorageImpl(Request request, ClientDataEncoder clientDataEncoder, SessionPersistedObjectAnalyzer analyzer)
{
this.clientDataEncoder = clientDataEncoder;
+ this.analyzer = analyzer;
// This, here, is the problem of TAPESTRY-2501; this call can predate
// the check to set the character set based on meta data of the page.
@@ -233,12 +230,10 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
persistedValues.put(key, value);
}
- }
- catch (Exception ex)
+ } catch (Exception ex)
{
throw new RuntimeException("Serialized client state was corrupted. This may indicate that too much state is being stored, which can cause the encoded string to be truncated by the client web browser.", ex);
- }
- finally
+ } finally
{
InternalUtils.close(in);
}
@@ -246,6 +241,21 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
private void refreshClientData()
{
+ // TAP5-2269: Even in the absense of a change to a persistent field, a mutable persistent object
+ // may have changed.
+
+ if (clientData != null)
+ {
+ for (Object value : persistedValues.values())
+ {
+ if (analyzer.checkAndResetDirtyState(value))
+ {
+ clientData = null;
+ break;
+ }
+ }
+ }
+
// Client data will be null after a change to the map, or if there was no client data in the
// request. In any other case where the client data is non-null, it is by definition
// up-to date (since it is reset to null any time there's a change to the map).
@@ -276,12 +286,10 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
os.writeObject(e.getKey());
os.writeObject(e.getValue());
}
- }
- catch (Exception ex)
+ } catch (Exception ex)
{
throw new RuntimeException(ex.getMessage(), ex);
- }
- finally
+ } finally
{
InternalUtils.close(os);
}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/021b23e1/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java
index 0b24b38..3cd0ef9 100644
--- a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java
@@ -1,5 +1,3 @@
-// Copyright 2007-2013 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
@@ -17,14 +15,19 @@ package org.apache.tapestry5.internal.services;
import org.apache.tapestry5.Link;
import org.apache.tapestry5.internal.test.InternalBaseTestCase;
import org.apache.tapestry5.internal.util.Holder;
+import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
import org.apache.tapestry5.services.ClientDataEncoder;
import org.apache.tapestry5.services.PersistentFieldChange;
import org.apache.tapestry5.services.Request;
+import org.apache.tapestry5.services.SessionPersistedObjectAnalyzer;
import org.easymock.EasyMock;
import org.easymock.IAnswer;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
import java.util.List;
import static org.apache.tapestry5.ioc.internal.util.CollectionFactory.newList;
@@ -35,10 +38,13 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
{
private ClientDataEncoder clientDataEncoder;
+ private SessionPersistedObjectAnalyzer analyzer;
+
@BeforeClass
public void setup()
{
clientDataEncoder = getService(ClientDataEncoder.class);
+ analyzer = getService(SessionPersistedObjectAnalyzer.class);
}
@Test
@@ -49,7 +55,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
// Should do nothing.
@@ -58,21 +64,10 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
verify();
}
- @SuppressWarnings("unchecked")
- @Test
- public void store_and_restore_a_change()
+ private Holder<String> captureLinkModification(Link link)
{
- Request request = mockRequest(null);
- Link link = mockLink();
final Holder<String> holder = Holder.create();
- String pageName = "Foo";
- String componentId = "bar.baz";
- String fieldName = "biff";
- Object value = 99;
-
- // Use an IAnswer to capture the value.
-
link.addParameter(eq(ClientPersistentFieldStorageImpl.PARAMETER_NAME), isA(String.class));
setAnswer(new IAnswer<Void>()
{
@@ -86,9 +81,26 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
}
});
+ return holder;
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void store_and_restore_a_change()
+ {
+ Request request = mockRequest(null);
+ Link link = mockLink();
+
+ String pageName = "Foo";
+ String componentId = "bar.baz";
+ String fieldName = "biff";
+ Object value = 99;
+
+ final Holder<String> holder = captureLinkModification(link);
+
replay();
- ClientPersistentFieldStorage storage1 = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage1 = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
storage1.postChange(pageName, componentId, fieldName, value);
@@ -98,8 +110,6 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
verify();
- System.out.println(holder.get());
-
assertEquals(changes1.size(), 1);
PersistentFieldChange change1 = changes1.get(0);
@@ -113,7 +123,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage2 = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage2 = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
List<PersistentFieldChange> changes2 = newList(storage2.gatherFieldChanges(pageName));
@@ -142,7 +152,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
for (int k = 0; k < 3; k++)
{
@@ -174,7 +184,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
storage.postChange(pageName, componentId, fieldName, 99);
storage.postChange(pageName, componentId, fieldName, null);
@@ -201,7 +211,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
storage.postChange(pageName, componentId, fieldName, 99);
@@ -231,14 +241,13 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
try
{
storage.postChange("Foo", "bar.baz", "woops", badBody);
unreachable();
- }
- catch (IllegalArgumentException ex)
+ } catch (IllegalArgumentException ex)
{
assertEquals(
ex.getMessage(),
@@ -248,6 +257,70 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
verify();
}
+ public static class MutableObject implements Serializable
+ {
+ public String mutableValue;
+ }
+
+ // So if an object is stored in a persistent field and is mutable, then the field should not have to be modified
+ // to force a change, instead the value should be checked to see if it is dirty (assuming true), and reserialized.
+ @Test
+ public void modified_mutable_objects_are_reserialized()
+ {
+ Request request = mockRequest(null);
+ Link link = mockLink();
+
+ MutableObject mo = new MutableObject();
+
+ mo.mutableValue = "initial state";
+
+ String pageName = "Foo";
+ String componentId = "bar.baz";
+ String fieldName = "biff";
+
+ final Holder<String> holder1 = captureLinkModification(link);
+
+ final Holder<String> holder2 = captureLinkModification(link);
+
+ replay();
+
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
+
+ storage.postChange(pageName, componentId, fieldName, mo);
+
+ storage.updateLink(link);
+
+ mo.mutableValue = "modified state";
+
+ storage.updateLink(link);
+
+ verify();
+
+ System.out.printf("holder1=%s%nholder2=%s%n", holder1.get(), holder2.get());
+
+ assertNotEquals(holder1.get(), holder2.get(), "encoded client data should be different");
+
+ // Now check that it de-serializes to the correct data.
+
+ request = mockRequest(holder2.get());
+
+ replay();
+
+ storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
+
+ Collection<PersistentFieldChange> changes = storage.gatherFieldChanges(pageName);
+
+ assertEquals(changes.size(), 1);
+
+ PersistentFieldChange change = new ArrayList<PersistentFieldChange>(changes).get(0);
+
+ MutableObject mo2 = (MutableObject) change.getValue();
+
+ assertEquals(mo2.mutableValue, "modified state");
+
+ verify();
+ }
+
@Test
public void corrupt_client_data()
{
@@ -256,14 +329,13 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
try
{
storage.postChange("Foo", "bar.baz", "woops", 99);
unreachable();
- }
- catch (RuntimeException ex)
+ } catch (RuntimeException ex)
{
assertEquals(ex.getMessage(), "Serialized client state was corrupted. This may indicate that too much state is being stored, which can cause the encoded string to be truncated by the client web browser.");
assertNotNull(ex.getCause());
[2/2] git commit: TAP5-2269: Client persistence strategy does not
detect changes to internal state of mutable objects
Posted by hl...@apache.org.
TAP5-2269: Client persistence strategy does not detect changes to internal state of mutable objects
Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/021b23e1
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/021b23e1
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/021b23e1
Branch: refs/heads/master
Commit: 021b23e18c1b67b0a932d19dc0080b82a82e4902
Parents: 57ced86
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Mon Sep 1 11:30:07 2014 -0700
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Mon Sep 1 11:30:07 2014 -0700
----------------------------------------------------------------------
54_RELEASE_NOTES.md | 7 +
.../ClientPersistentFieldStorageImpl.java | 44 ++++---
.../ClientPersistentFieldStorageImplTest.java | 128 +++++++++++++++----
3 files changed, 133 insertions(+), 46 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/021b23e1/54_RELEASE_NOTES.md
----------------------------------------------------------------------
diff --git a/54_RELEASE_NOTES.md b/54_RELEASE_NOTES.md
index 93d0646..19a598e 100644
--- a/54_RELEASE_NOTES.md
+++ b/54_RELEASE_NOTES.md
@@ -448,3 +448,10 @@ The automatic ValueEncoder from String to any Number type, or ti Boolean have
changed slightly. An empty input string is encoded to a null rather than being passed through
the type coercer. This reflects the desire that a submitted value (in a URL or a form) that is blank
is the same as no value: null.
+
+## Client Persistent Fields
+
+Tapestry now determines if a client-persistent field is mutable and contains changes, using the same
+checks as for session-persistent objects (the OptimizedSessionPersistedObject interface and so forth).
+Previously, Tapestry would load a mutable object into a field, but a change to the internal state
+of that mutable field would not always be preserved in the serialized object data attached to links.
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/021b23e1/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java
index 2d95767..a94ac4f 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImpl.java
@@ -1,5 +1,3 @@
-// Copyright 2007, 2008, 2009, 2011 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
@@ -19,10 +17,7 @@ import org.apache.tapestry5.ioc.ScopeConstants;
import org.apache.tapestry5.ioc.annotations.Scope;
import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
import org.apache.tapestry5.ioc.internal.util.InternalUtils;
-import org.apache.tapestry5.services.ClientDataEncoder;
-import org.apache.tapestry5.services.ClientDataSink;
-import org.apache.tapestry5.services.PersistentFieldChange;
-import org.apache.tapestry5.services.Request;
+import org.apache.tapestry5.services.*;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
@@ -66,7 +61,7 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
public PersistentFieldChange toChange(Object value)
{
return new PersistentFieldChangeImpl(componentId == null ? "" : componentId,
- fieldName, value);
+ fieldName, value);
}
@Override
@@ -102,8 +97,7 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
if (componentId == null)
{
if (other.componentId != null) return false;
- }
- else if (!componentId.equals(other.componentId)) return false;
+ } else if (!componentId.equals(other.componentId)) return false;
return true;
}
@@ -111,15 +105,18 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
private final ClientDataEncoder clientDataEncoder;
+ private final SessionPersistedObjectAnalyzer analyzer;
+
private final Map<Key, Object> persistedValues = CollectionFactory.newMap();
private String clientData;
private boolean mapUptoDate = false;
- public ClientPersistentFieldStorageImpl(Request request, ClientDataEncoder clientDataEncoder)
+ public ClientPersistentFieldStorageImpl(Request request, ClientDataEncoder clientDataEncoder, SessionPersistedObjectAnalyzer analyzer)
{
this.clientDataEncoder = clientDataEncoder;
+ this.analyzer = analyzer;
// This, here, is the problem of TAPESTRY-2501; this call can predate
// the check to set the character set based on meta data of the page.
@@ -233,12 +230,10 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
persistedValues.put(key, value);
}
- }
- catch (Exception ex)
+ } catch (Exception ex)
{
throw new RuntimeException("Serialized client state was corrupted. This may indicate that too much state is being stored, which can cause the encoded string to be truncated by the client web browser.", ex);
- }
- finally
+ } finally
{
InternalUtils.close(in);
}
@@ -246,6 +241,21 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
private void refreshClientData()
{
+ // TAP5-2269: Even in the absense of a change to a persistent field, a mutable persistent object
+ // may have changed.
+
+ if (clientData != null)
+ {
+ for (Object value : persistedValues.values())
+ {
+ if (analyzer.checkAndResetDirtyState(value))
+ {
+ clientData = null;
+ break;
+ }
+ }
+ }
+
// Client data will be null after a change to the map, or if there was no client data in the
// request. In any other case where the client data is non-null, it is by definition
// up-to date (since it is reset to null any time there's a change to the map).
@@ -276,12 +286,10 @@ public class ClientPersistentFieldStorageImpl implements ClientPersistentFieldSt
os.writeObject(e.getKey());
os.writeObject(e.getValue());
}
- }
- catch (Exception ex)
+ } catch (Exception ex)
{
throw new RuntimeException(ex.getMessage(), ex);
- }
- finally
+ } finally
{
InternalUtils.close(os);
}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/021b23e1/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java
index 0b24b38..3cd0ef9 100644
--- a/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/internal/services/ClientPersistentFieldStorageImplTest.java
@@ -1,5 +1,3 @@
-// Copyright 2007-2013 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
@@ -17,14 +15,19 @@ package org.apache.tapestry5.internal.services;
import org.apache.tapestry5.Link;
import org.apache.tapestry5.internal.test.InternalBaseTestCase;
import org.apache.tapestry5.internal.util.Holder;
+import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
import org.apache.tapestry5.services.ClientDataEncoder;
import org.apache.tapestry5.services.PersistentFieldChange;
import org.apache.tapestry5.services.Request;
+import org.apache.tapestry5.services.SessionPersistedObjectAnalyzer;
import org.easymock.EasyMock;
import org.easymock.IAnswer;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collection;
import java.util.List;
import static org.apache.tapestry5.ioc.internal.util.CollectionFactory.newList;
@@ -35,10 +38,13 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
{
private ClientDataEncoder clientDataEncoder;
+ private SessionPersistedObjectAnalyzer analyzer;
+
@BeforeClass
public void setup()
{
clientDataEncoder = getService(ClientDataEncoder.class);
+ analyzer = getService(SessionPersistedObjectAnalyzer.class);
}
@Test
@@ -49,7 +55,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
// Should do nothing.
@@ -58,21 +64,10 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
verify();
}
- @SuppressWarnings("unchecked")
- @Test
- public void store_and_restore_a_change()
+ private Holder<String> captureLinkModification(Link link)
{
- Request request = mockRequest(null);
- Link link = mockLink();
final Holder<String> holder = Holder.create();
- String pageName = "Foo";
- String componentId = "bar.baz";
- String fieldName = "biff";
- Object value = 99;
-
- // Use an IAnswer to capture the value.
-
link.addParameter(eq(ClientPersistentFieldStorageImpl.PARAMETER_NAME), isA(String.class));
setAnswer(new IAnswer<Void>()
{
@@ -86,9 +81,26 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
}
});
+ return holder;
+ }
+
+ @SuppressWarnings("unchecked")
+ @Test
+ public void store_and_restore_a_change()
+ {
+ Request request = mockRequest(null);
+ Link link = mockLink();
+
+ String pageName = "Foo";
+ String componentId = "bar.baz";
+ String fieldName = "biff";
+ Object value = 99;
+
+ final Holder<String> holder = captureLinkModification(link);
+
replay();
- ClientPersistentFieldStorage storage1 = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage1 = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
storage1.postChange(pageName, componentId, fieldName, value);
@@ -98,8 +110,6 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
verify();
- System.out.println(holder.get());
-
assertEquals(changes1.size(), 1);
PersistentFieldChange change1 = changes1.get(0);
@@ -113,7 +123,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage2 = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage2 = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
List<PersistentFieldChange> changes2 = newList(storage2.gatherFieldChanges(pageName));
@@ -142,7 +152,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
for (int k = 0; k < 3; k++)
{
@@ -174,7 +184,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
storage.postChange(pageName, componentId, fieldName, 99);
storage.postChange(pageName, componentId, fieldName, null);
@@ -201,7 +211,7 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
storage.postChange(pageName, componentId, fieldName, 99);
@@ -231,14 +241,13 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
try
{
storage.postChange("Foo", "bar.baz", "woops", badBody);
unreachable();
- }
- catch (IllegalArgumentException ex)
+ } catch (IllegalArgumentException ex)
{
assertEquals(
ex.getMessage(),
@@ -248,6 +257,70 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
verify();
}
+ public static class MutableObject implements Serializable
+ {
+ public String mutableValue;
+ }
+
+ // So if an object is stored in a persistent field and is mutable, then the field should not have to be modified
+ // to force a change, instead the value should be checked to see if it is dirty (assuming true), and reserialized.
+ @Test
+ public void modified_mutable_objects_are_reserialized()
+ {
+ Request request = mockRequest(null);
+ Link link = mockLink();
+
+ MutableObject mo = new MutableObject();
+
+ mo.mutableValue = "initial state";
+
+ String pageName = "Foo";
+ String componentId = "bar.baz";
+ String fieldName = "biff";
+
+ final Holder<String> holder1 = captureLinkModification(link);
+
+ final Holder<String> holder2 = captureLinkModification(link);
+
+ replay();
+
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
+
+ storage.postChange(pageName, componentId, fieldName, mo);
+
+ storage.updateLink(link);
+
+ mo.mutableValue = "modified state";
+
+ storage.updateLink(link);
+
+ verify();
+
+ System.out.printf("holder1=%s%nholder2=%s%n", holder1.get(), holder2.get());
+
+ assertNotEquals(holder1.get(), holder2.get(), "encoded client data should be different");
+
+ // Now check that it de-serializes to the correct data.
+
+ request = mockRequest(holder2.get());
+
+ replay();
+
+ storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
+
+ Collection<PersistentFieldChange> changes = storage.gatherFieldChanges(pageName);
+
+ assertEquals(changes.size(), 1);
+
+ PersistentFieldChange change = new ArrayList<PersistentFieldChange>(changes).get(0);
+
+ MutableObject mo2 = (MutableObject) change.getValue();
+
+ assertEquals(mo2.mutableValue, "modified state");
+
+ verify();
+ }
+
@Test
public void corrupt_client_data()
{
@@ -256,14 +329,13 @@ public class ClientPersistentFieldStorageImplTest extends InternalBaseTestCase
replay();
- ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder);
+ ClientPersistentFieldStorage storage = new ClientPersistentFieldStorageImpl(request, clientDataEncoder, analyzer);
try
{
storage.postChange("Foo", "bar.baz", "woops", 99);
unreachable();
- }
- catch (RuntimeException ex)
+ } catch (RuntimeException ex)
{
assertEquals(ex.getMessage(), "Serialized client state was corrupted. This may indicate that too much state is being stored, which can cause the encoded string to be truncated by the client web browser.");
assertNotNull(ex.getCause());