TAP5-2284: The Hibernate ENTITY session persistent strategy should store transient entities as-is
Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/c83d91bc Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/c83d91bc Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/c83d91bc Branch: refs/heads/master Commit: c83d91bca2c64895d5d69efe8b6f89e8dba1fa59 Parents: 1a54c61 Author: Howard M. Lewis Ship <[email protected]> Authored: Fri Feb 7 14:21:41 2014 -0500 Committer: Howard M. Lewis Ship <[email protected]> Committed: Fri Feb 7 14:21:41 2014 -0500 ---------------------------------------------------------------------- ...tityApplicationStatePersistenceStrategy.java | 42 +++++---------- .../EntityPersistentFieldStrategy.java | 20 ++++--- .../internal/hibernate/PersistedEntity.java | 16 +++--- .../hibernate/PersistedTransientEntity.java | 40 ++++++++++++++ .../internal/hibernate/SessionRestorable.java | 30 +++++++++++ .../TapestryHibernateIntegrationTests.java | 25 +++++---- .../EntityPersistentFieldStrategyTest.java | 56 ++++++++++++++++++-- .../org/example/app0/pages/PersistEntity.java | 7 +-- .../java/org/example/app0/pages/SSOEntity.java | 15 ++---- .../src/test/webapp/PersistEntity.tml | 1 - .../src/test/webapp/SSOEntity.tml | 1 - 11 files changed, 177 insertions(+), 76 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityApplicationStatePersistenceStrategy.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityApplicationStatePersistenceStrategy.java b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityApplicationStatePersistenceStrategy.java index f6a4eaa..82d1f47 100644 --- a/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityApplicationStatePersistenceStrategy.java +++ b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityApplicationStatePersistenceStrategy.java @@ -17,9 +17,7 @@ package org.apache.tapestry5.internal.hibernate; import org.apache.tapestry5.internal.services.SessionApplicationStatePersistenceStrategy; import org.apache.tapestry5.services.ApplicationStateCreator; import org.apache.tapestry5.services.Request; -import org.hibernate.HibernateException; - -import java.io.Serializable; +import org.hibernate.Session; /** * Persists Hibernate entities as SSOs by storing their primary key in the {@link org.apache.tapestry5.services.Session}. @@ -29,12 +27,13 @@ import java.io.Serializable; public class EntityApplicationStatePersistenceStrategy extends SessionApplicationStatePersistenceStrategy { - private final org.hibernate.Session hibernateSession; + private final EntityPersistentFieldStrategy delegate; - public EntityApplicationStatePersistenceStrategy(Request request, org.hibernate.Session hibernateSession) + public EntityApplicationStatePersistenceStrategy(Request request, Session hibernateSession) { super(request); - this.hibernateSession = hibernateSession; + + delegate = new EntityPersistentFieldStrategy(hibernateSession, null); } @SuppressWarnings("unchecked") @@ -42,13 +41,11 @@ public class EntityApplicationStatePersistenceStrategy extends SessionApplicatio { final Object persistedValue = getOrCreate(ssoClass, creator); - if (persistedValue instanceof PersistedEntity) + if (persistedValue instanceof SessionRestorable) { - final PersistedEntity persisted = (PersistedEntity) persistedValue; - - Object restored = persisted.restore(this.hibernateSession); + Object restored = delegate.convertPersistedToApplicationValue(persistedValue); - //shall we maybe throw an exception instead? + // Maybe throw an exception instead? if (restored == null) { set(ssoClass, null); @@ -64,27 +61,16 @@ public class EntityApplicationStatePersistenceStrategy extends SessionApplicatio public <T> void set(Class<T> ssoClass, T sso) { final String key = buildKey(ssoClass); - Object entity; - if (sso != null) + if (sso == null) { - try - { - final String entityName = this.hibernateSession.getEntityName(sso); - final Serializable id = this.hibernateSession.getIdentifier(sso); - - entity = new PersistedEntity(entityName, id); - } catch (final HibernateException ex) - { - // if entity not attached to a Hibernate Session yet, store it as usual sso - entity = sso; - } - } else - { - entity = sso; + getSession().setAttribute(key, null); + return; } - getSession().setAttribute(key, entity); + Object persistable = delegate.convertApplicationValueToPersisted(sso); + + getSession().setAttribute(key, persistable); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategy.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategy.java b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategy.java index d651352..5be27af 100644 --- a/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategy.java +++ b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategy.java @@ -1,4 +1,4 @@ -// Copyright 2008m 2913 The Apache Software Foundation +// Copyright 2008-2014 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. @@ -40,24 +40,32 @@ public class EntityPersistentFieldStrategy extends AbstractSessionPersistentFiel @Override protected Object convertApplicationValueToPersisted(Object newValue) { + assert newValue != null; + + if (!session.contains(newValue)) + { + return new PersistedTransientEntity(newValue); + } + try { String entityName = session.getEntityName(newValue); Serializable id = session.getIdentifier(newValue); return new PersistedEntity(entityName, id); - } - catch (HibernateException ex) + } catch (HibernateException ex) { - throw new IllegalArgumentException(String.format("Failed persisting an entity in the session. Only entities attached to a Hibernate Session can be persisted. entity: %s", newValue), ex); + throw new IllegalArgumentException(String.format("Failed persisting an entity in the session. entity: %s", newValue), ex); } } @Override protected Object convertPersistedToApplicationValue(Object persistedValue) { - PersistedEntity persisted = (PersistedEntity) persistedValue; + assert persistedValue != null; + + SessionRestorable persisted = (SessionRestorable) persistedValue; - return persisted.restore(session); + return persisted.restoreWithSession(session); } } http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedEntity.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedEntity.java b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedEntity.java index 51d3088..a4d5fb2 100644 --- a/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedEntity.java +++ b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedEntity.java @@ -1,4 +1,4 @@ -// Copyright 2008, 2010, 2013 The Apache Software Foundation +// Copyright 2008-2014 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. @@ -15,6 +15,7 @@ package org.apache.tapestry5.internal.hibernate; import org.apache.tapestry5.annotations.ImmutableSessionPersistedObject; +import org.apache.tapestry5.ioc.internal.util.InternalUtils; import org.hibernate.Session; import java.io.Serializable; @@ -23,7 +24,7 @@ import java.io.Serializable; * Encapsulates a Hibernate entity name with an entity id. */ @ImmutableSessionPersistedObject -public class PersistedEntity implements Serializable +public class PersistedEntity implements SessionRestorable { private static final long serialVersionUID = 897120520279686518L; @@ -33,19 +34,22 @@ public class PersistedEntity implements Serializable public PersistedEntity(String entityName, Serializable id) { + assert InternalUtils.isNonBlank(entityName); + assert id != null; + this.entityName = entityName; this.id = id; } - public Object restore(Session session) + public Object restoreWithSession(Session session) { try { return session.get(entityName, id); - } - catch (Exception ex) + } catch (Exception ex) { - throw new RuntimeException(String.format("Failed to load session-persisted entity %s(%s): %s", entityName, id, ex)); + throw new RuntimeException(String.format("Failed to load session-persisted entity %s(%s): %s", entityName, id, ex), + ex); } } http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedTransientEntity.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedTransientEntity.java b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedTransientEntity.java new file mode 100644 index 0000000..78977b0 --- /dev/null +++ b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/PersistedTransientEntity.java @@ -0,0 +1,40 @@ +// Copyright 2014 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.internal.hibernate; + +import org.hibernate.Session; + +public class PersistedTransientEntity implements SessionRestorable +{ + private final Object transientEntity; + + public PersistedTransientEntity(Object transientEntity) + { + assert transientEntity != null; + + this.transientEntity = transientEntity; + } + + public Object restoreWithSession(Session session) + { + return transientEntity; + } + + @Override + public String toString() + { + return String.format("PersistedTransientEntity<%s>", transientEntity); + } +} http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/SessionRestorable.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/SessionRestorable.java b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/SessionRestorable.java new file mode 100644 index 0000000..c3cf82c --- /dev/null +++ b/tapestry-hibernate/src/main/java/org/apache/tapestry5/internal/hibernate/SessionRestorable.java @@ -0,0 +1,30 @@ +// Copyright 2014 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.internal.hibernate; + +import org.hibernate.Session; + +import java.io.Serializable; + +/** + * Interface for serializable objects stored in the HTTP Session that can be restored to active state via + * the Hibernate {@linkplain org.hibernate.Session}. + * + * @since 5.4 + */ +public interface SessionRestorable extends Serializable +{ + Object restoreWithSession(Session session); +} http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/test/java/org/apache/tapestry5/hibernate/integration/TapestryHibernateIntegrationTests.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/test/java/org/apache/tapestry5/hibernate/integration/TapestryHibernateIntegrationTests.java b/tapestry-hibernate/src/test/java/org/apache/tapestry5/hibernate/integration/TapestryHibernateIntegrationTests.java index d5f9540..e573704 100644 --- a/tapestry-hibernate/src/test/java/org/apache/tapestry5/hibernate/integration/TapestryHibernateIntegrationTests.java +++ b/tapestry-hibernate/src/test/java/org/apache/tapestry5/hibernate/integration/TapestryHibernateIntegrationTests.java @@ -1,4 +1,4 @@ -// Copyright 2008, 2012, 2013 The Apache Software Foundation +// Copyright 2008-2014 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. @@ -15,6 +15,7 @@ package org.apache.tapestry5.hibernate.integration; import org.apache.tapestry5.internal.hibernate.PersistedEntity; +import org.apache.tapestry5.internal.hibernate.PersistedTransientEntity; import org.apache.tapestry5.test.SeleniumTestCase; import org.apache.tapestry5.test.TapestryTestConfiguration; import org.example.app0.entities.User; @@ -24,6 +25,10 @@ import org.testng.annotations.Test; @TapestryTestConfiguration(webAppFolder = "src/test/webapp") public class TapestryHibernateIntegrationTests extends SeleniumTestCase { + + private final String PERSISTENT_ENTITY_CLASS_NAME = PersistedEntity.class.getName(); + private final String PERSISTED_TRANSIENT_ENTITY_CLASS_NAME = PersistedTransientEntity.class.getName(); + public void valueencode_all_entity_types() throws Exception { open("/encodeentities"); @@ -60,39 +65,33 @@ public class TapestryHibernateIntegrationTests extends SeleniumTestCase assertText("//span[@id='name']", "name"); clickAndWait("link=delete"); assertEquals(getText("//span[@id='name']").length(), 0); - - // transient objects cannot be persisted - clickAndWait("link=set to transient"); - assertTextPresent("Error persisting"); } public void sso_entities() { open("/ssoentity"); assertEquals(getText("//span[@id='name']").length(), 0); - assertText("//span[@id='persistedEntityClassName']", User.class.getName()); + assertText("//span[@id='persistedEntityClassName']", PERSISTED_TRANSIENT_ENTITY_CLASS_NAME); clickAndWait("link=persist entity"); assertText("//span[@id='name']", "name"); - assertText("//span[@id='persistedEntityClassName']", PersistedEntity.class.getName()); + assertText("//span[@id='persistedEntityClassName']", PERSISTENT_ENTITY_CLASS_NAME); // can set back to null clickAndWait("link=set to null"); assertEquals(getText("//span[@id='name']").length(), 0); - assertText("//span[@id='persistedEntityClassName']", User.class.getName()); + assertText("//span[@id='persistedEntityClassName']", PERSISTED_TRANSIENT_ENTITY_CLASS_NAME); clickAndWait("link=persist entity"); assertText("//span[@id='name']", "name"); - assertText("//span[@id='persistedEntityClassName']", PersistedEntity.class.getName()); + assertText("//span[@id='persistedEntityClassName']", PERSISTENT_ENTITY_CLASS_NAME); clickAndWait("link=delete"); assertEquals(getText("//span[@id='name']").length(), 0); - assertText("//span[@id='persistedEntityClassName']", User.class.getName()); + assertText("//span[@id='persistedEntityClassName']", PERSISTED_TRANSIENT_ENTITY_CLASS_NAME); clickAndWait("link=persist entity"); assertText("//span[@id='name']", "name"); - assertText("//span[@id='persistedEntityClassName']", PersistedEntity.class.getName()); - clickAndWait("link=set to transient"); - assertText("//span[@id='persistedEntityClassName']", User.class.getName()); + assertText("//span[@id='persistedEntityClassName']", PERSISTENT_ENTITY_CLASS_NAME); } /** http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/test/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategyTest.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/test/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategyTest.java b/tapestry-hibernate/src/test/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategyTest.java index 3b9b008..5bd2e91 100644 --- a/tapestry-hibernate/src/test/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategyTest.java +++ b/tapestry-hibernate/src/test/java/org/apache/tapestry5/internal/hibernate/EntityPersistentFieldStrategyTest.java @@ -1,4 +1,4 @@ -// Copyright 2008 The Apache Software Foundation +// Copyright 2008, 2014 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. @@ -28,6 +28,7 @@ public class EntityPersistentFieldStrategyTest extends TapestryTestCase Session session = newMock(Session.class); EntityPersistentFieldStrategy strategy = new EntityPersistentFieldStrategy(session, null); + expect(session.contains(nonEntity)).andReturn(true); expect(session.getEntityName(nonEntity)).andThrow(new HibernateException("error")); replay(); @@ -37,12 +38,59 @@ public class EntityPersistentFieldStrategyTest extends TapestryTestCase strategy.postChange("pageName", "", "fieldName", nonEntity); unreachable(); - } - catch (IllegalArgumentException ex) + } catch (IllegalArgumentException ex) { - assertEquals(ex.getMessage(), "Failed persisting an entity in the session. Only entities attached to a Hibernate Session can be persisted. entity: foo"); + assertEquals(ex.getMessage(), "Failed persisting an entity in the session. entity: foo"); } verify(); } + + public void transient_entity() + { + SampleEntity entity = new SampleEntity(); + Session session = newMock(Session.class); + EntityPersistentFieldStrategy strategy = new EntityPersistentFieldStrategy(session, null); + + expect(session.contains(entity)).andReturn(false); + + replay(); + + Object persisted = strategy.convertApplicationValueToPersisted(entity); + + assert persisted instanceof SessionRestorable; + + Object restored = strategy.convertPersistedToApplicationValue(persisted); + + assertSame(entity, restored); + + verify(); + } + + public void persistent_entity() + { + SampleEntity entity = new SampleEntity(); + SampleEntity restoredEntity = new SampleEntity(); + Session session = newMock(Session.class); + EntityPersistentFieldStrategy strategy = new EntityPersistentFieldStrategy(session, null); + + expect(session.contains(entity)).andReturn(true); + expect(session.getEntityName(entity)).andReturn("SampleEntity"); + expect(session.getIdentifier(entity)).andReturn(42); + + expect(session.get("SampleEntity", 42)).andReturn(restoredEntity); + + replay(); + + Object persisted = strategy.convertApplicationValueToPersisted(entity); + + assert persisted instanceof SessionRestorable; + + Object restored = strategy.convertPersistedToApplicationValue(persisted); + + assertSame(restored, restoredEntity); + + verify(); + + } } http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/test/java/org/example/app0/pages/PersistEntity.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/test/java/org/example/app0/pages/PersistEntity.java b/tapestry-hibernate/src/test/java/org/example/app0/pages/PersistEntity.java index 50cc55a..484f666 100644 --- a/tapestry-hibernate/src/test/java/org/example/app0/pages/PersistEntity.java +++ b/tapestry-hibernate/src/test/java/org/example/app0/pages/PersistEntity.java @@ -1,4 +1,4 @@ -// Copyright 2008 The Apache Software Foundation +// Copyright 2008-2014 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. @@ -56,11 +56,6 @@ public class PersistEntity // No commit, so no real change. } - void onSetToTransient() - { - user = new User(); - } - void onSetToNull() { user = null; http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/test/java/org/example/app0/pages/SSOEntity.java ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/test/java/org/example/app0/pages/SSOEntity.java b/tapestry-hibernate/src/test/java/org/example/app0/pages/SSOEntity.java index be2047c..b59f6fa 100644 --- a/tapestry-hibernate/src/test/java/org/example/app0/pages/SSOEntity.java +++ b/tapestry-hibernate/src/test/java/org/example/app0/pages/SSOEntity.java @@ -1,4 +1,4 @@ -// Copyright 2009 The Apache Software Foundation +// Copyright 2009-2014 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. @@ -13,18 +13,16 @@ // limitations under the License. package org.example.app0.pages; -import java.util.List; - -import org.apache.tapestry5.annotations.Persist; import org.apache.tapestry5.annotations.Property; import org.apache.tapestry5.annotations.SessionState; -import org.apache.tapestry5.internal.hibernate.PersistedEntity; import org.apache.tapestry5.ioc.annotations.Inject; import org.apache.tapestry5.services.Request; import org.apache.tapestry5.services.Session; import org.example.app0.entities.User; import org.example.app0.services.UserDAO; +import java.util.List; + public class SSOEntity { @SessionState @@ -51,12 +49,7 @@ public class SSOEntity { user = null; } - - void onSetToTransient() - { - user = new User(); - } - + void onDelete() { List<User> users = userDAO.findAll(); http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/test/webapp/PersistEntity.tml ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/test/webapp/PersistEntity.tml b/tapestry-hibernate/src/test/webapp/PersistEntity.tml index 6329cf5..8fba8a6 100644 --- a/tapestry-hibernate/src/test/webapp/PersistEntity.tml +++ b/tapestry-hibernate/src/test/webapp/PersistEntity.tml @@ -5,6 +5,5 @@ <p><t:eventlink event="changeName">change the name</t:eventlink></p> <p><t:eventlink event="setToNull">set to null</t:eventlink></p> <p><t:eventlink event="delete">delete</t:eventlink></p> - <p><t:eventlink event="setToTransient">set to transient</t:eventlink></p> </body> </html> \ No newline at end of file http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/c83d91bc/tapestry-hibernate/src/test/webapp/SSOEntity.tml ---------------------------------------------------------------------- diff --git a/tapestry-hibernate/src/test/webapp/SSOEntity.tml b/tapestry-hibernate/src/test/webapp/SSOEntity.tml index f7b0cf1..f749a25 100644 --- a/tapestry-hibernate/src/test/webapp/SSOEntity.tml +++ b/tapestry-hibernate/src/test/webapp/SSOEntity.tml @@ -4,7 +4,6 @@ <p>persisted entity class name: <span id="persistedEntityClassName">${persistedEntityClassName}</span></p> <p><t:eventlink event="persistEntity">persist entity</t:eventlink></p> <p><t:eventlink event="setToNull">set to null</t:eventlink></p> - <p><t:eventlink event="setToTransient">set to transient</t:eventlink></p> <p><t:eventlink event="delete">delete</t:eventlink></p> </body> </html> \ No newline at end of file
