Repository: cayenne Updated Branches: refs/heads/master c4e8ef895 -> a0ef5ad24
CAY-2243 ObjectContext.getGraphManager().unregisterObject() inconsistencies Project: http://git-wip-us.apache.org/repos/asf/cayenne/repo Commit: http://git-wip-us.apache.org/repos/asf/cayenne/commit/a0ef5ad2 Tree: http://git-wip-us.apache.org/repos/asf/cayenne/tree/a0ef5ad2 Diff: http://git-wip-us.apache.org/repos/asf/cayenne/diff/a0ef5ad2 Branch: refs/heads/master Commit: a0ef5ad2411f05c23ce8e609a7eabe686b642be3 Parents: c4e8ef8 Author: Nikita Timofeev <[email protected]> Authored: Wed Mar 22 18:11:18 2017 +0300 Committer: Nikita Timofeev <[email protected]> Committed: Wed Mar 22 18:11:18 2017 +0300 ---------------------------------------------------------------------- .../cayenne/CayenneContextGraphManager.java | 5 +- .../org/apache/cayenne/access/DataContext.java | 4 +- .../cayenne/access/DataContextMergeHandler.java | 8 +- .../org/apache/cayenne/access/ObjectStore.java | 84 ++++++++------------ .../cayenne/CayenneContextGraphManagerTest.java | 67 ++++++++++++++++ .../access/DataContextObjectTrackingIT.java | 2 +- .../apache/cayenne/access/ObjectStoreIT.java | 2 +- .../apache/cayenne/access/ObjectStoreTest.java | 71 +++++++++++++++++ docs/doc/src/main/resources/RELEASE-NOTES.txt | 1 + 9 files changed, 182 insertions(+), 62 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/cayenne-server/src/main/java/org/apache/cayenne/CayenneContextGraphManager.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/CayenneContextGraphManager.java b/cayenne-server/src/main/java/org/apache/cayenne/CayenneContextGraphManager.java index ec97004..f4865c0 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/CayenneContextGraphManager.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/CayenneContextGraphManager.java @@ -44,7 +44,7 @@ import java.util.Iterator; import java.util.Map; /** - * A GraphMap extension that works together with ObjectContext to track persistent object + * A GraphMap extension that works together with {@link ObjectContext} to track persistent object * changes and send events. * * @since 1.2 @@ -108,6 +108,9 @@ final class CayenneContextGraphManager extends GraphMap { if (node != null) { stateLog.unregisterNode(nodeId); changeLog.unregisterNode(nodeId); + Persistent object = (Persistent)node; + object.setObjectContext(null); + object.setPersistenceState(PersistenceState.TRANSIENT); return node; } http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java b/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java index 264a7e8..ca7a3d9 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java @@ -612,8 +612,8 @@ public class DataContext extends BaseContext { /** * Unregisters a Collection of DataObjects from the DataContext and the - * underlying ObjectStore. This operation also unsets DataContext and - * ObjectId for each object and changes its state to TRANSIENT. + * underlying ObjectStore. This operation also unsets DataContext for + * each object and changes its state to TRANSIENT. * * @see #invalidateObjects(Collection) */ http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextMergeHandler.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextMergeHandler.java b/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextMergeHandler.java index 9d95863..6b422ea 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextMergeHandler.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextMergeHandler.java @@ -37,8 +37,7 @@ import org.apache.cayenne.reflect.ToOneProperty; import org.apache.cayenne.util.Util; /** - * A listener of GraphEvents sent by the DataChannel that merges changes to the - * DataContext. + * A listener of GraphEvents sent by the DataChannel that merges changes to the DataContext. * * @since 1.2 */ @@ -97,8 +96,7 @@ class DataContextMergeHandler implements GraphChangeHandler, DataChannelListener if (diff instanceof SnapshotEventDecorator) { SnapshotEvent decoratedEvent = ((SnapshotEventDecorator) diff).getEvent(); context.getObjectStore().processSnapshotEvent(decoratedEvent); - } - else { + } else { synchronized (context.getObjectStore()) { diff.apply(this); } @@ -154,7 +152,7 @@ class DataContextMergeHandler implements GraphChangeHandler, DataChannelListener public void nodeRemoved(Object nodeId) { ObjectStore os = context.getObjectStore(); synchronized (os) { - os.processDeletedID(nodeId); + os.processDeletedID((ObjectId)nodeId); } } http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectStore.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectStore.java b/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectStore.java index 693daf6..6901b77 100644 --- a/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectStore.java +++ b/cayenne-server/src/main/java/org/apache/cayenne/access/ObjectStore.java @@ -157,10 +157,8 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa if (objectDiff == null) { Persistent object = objectMap.get(nodeId); - if (object == null) { - throw new CayenneRuntimeException( - "No object is registered in context with Id " + nodeId); + throw new CayenneRuntimeException("No object is registered in context with Id " + nodeId); } if (object.getPersistenceState() == PersistenceState.COMMITTED) { @@ -287,9 +285,8 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa Collection<ObjectId> ids = new ArrayList<>(objects.size()); - Iterator it = objects.iterator(); - while (it.hasNext()) { - Persistent object = (Persistent) it.next(); + for (Object object1 : objects) { + Persistent object = (Persistent) object1; ObjectId id = object.getObjectId(); @@ -299,7 +296,6 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa ids.add(id); object.setObjectContext(null); - object.setObjectId(null); object.setPersistenceState(PersistenceState.TRANSIENT); } @@ -310,10 +306,10 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa // send an event for removed snapshots getDataRowCache().processSnapshotChanges( this, - Collections.EMPTY_MAP, - Collections.EMPTY_LIST, + Collections.<ObjectId, DataRow>emptyMap(), + Collections.<ObjectId>emptyList(), ids, - Collections.EMPTY_LIST); + Collections.<ObjectId>emptyList()); } } @@ -323,11 +319,11 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa * @since 1.1 */ public synchronized void objectsRolledBack() { - Iterator it = getObjectIterator(); + Iterator<Persistent> it = getObjectIterator(); // collect candidates while (it.hasNext()) { - Persistent object = (Persistent) it.next(); + Persistent object = it.next(); int objectState = object.getPersistenceState(); switch (objectState) { case PersistenceState.NEW: @@ -498,7 +494,7 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa /** * Returns an iterator over the registered objects. */ - public synchronized Iterator getObjectIterator() { + public synchronized Iterator<Persistent> getObjectIterator() { return objectMap.values().iterator(); } @@ -553,21 +549,17 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa */ synchronized void processSnapshotEvent(SnapshotEvent event) { - Map modifiedDiffs = event.getModifiedDiffs(); + Map<ObjectId, DataRow> modifiedDiffs = event.getModifiedDiffs(); if (modifiedDiffs != null && !modifiedDiffs.isEmpty()) { - Iterator oids = modifiedDiffs.entrySet().iterator(); - - while (oids.hasNext()) { - Map.Entry entry = (Map.Entry) oids.next(); - processUpdatedSnapshot(entry.getKey(), (DataRow) entry.getValue()); + for (Map.Entry<ObjectId, DataRow> entry : modifiedDiffs.entrySet()) { + processUpdatedSnapshot(entry.getKey(), entry.getValue()); } } - Collection deletedIDs = event.getDeletedIds(); + Collection<ObjectId> deletedIDs = event.getDeletedIds(); if (deletedIDs != null && !deletedIDs.isEmpty()) { - Iterator it = deletedIDs.iterator(); - while (it.hasNext()) { - processDeletedID(it.next()); + for (ObjectId deletedID : deletedIDs) { + processDeletedID(deletedID); } } @@ -604,10 +596,9 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa * * @since 1.2 */ - void processDeletedID(Object nodeId) { + void processDeletedID(ObjectId nodeId) { - // access object map directly - the method should be called in a synchronized - // context... + // access object map directly - the method should be called in a synchronized context... Persistent object = objectMap.get(nodeId); if (object != null) { @@ -662,11 +653,9 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa /** * @since 1.1 */ - void processInvalidatedIDs(Collection invalidatedIDs) { + void processInvalidatedIDs(Collection<ObjectId> invalidatedIDs) { if (invalidatedIDs != null && !invalidatedIDs.isEmpty()) { - Iterator it = invalidatedIDs.iterator(); - while (it.hasNext()) { - ObjectId oid = (ObjectId) it.next(); + for (ObjectId oid : invalidatedIDs) { DataObject object = (DataObject) getNode(oid); if (object == null) { @@ -713,17 +702,12 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa * * @since 1.1 */ - void processIndirectlyModifiedIDs(Collection indirectlyModifiedIDs) { - Iterator indirectlyModifiedIt = indirectlyModifiedIDs.iterator(); - while (indirectlyModifiedIt.hasNext()) { - ObjectId oid = (ObjectId) indirectlyModifiedIt.next(); - - // access object map directly - the method should be called in a synchronized - // context... + void processIndirectlyModifiedIDs(Collection<ObjectId> indirectlyModifiedIDs) { + for (ObjectId oid : indirectlyModifiedIDs) { + // access object map directly - the method should be called in a synchronized context... final DataObject object = (DataObject) objectMap.get(oid); - if (object == null - || object.getPersistenceState() != PersistenceState.COMMITTED) { + if (object == null || object.getPersistenceState() != PersistenceState.COMMITTED) { continue; } @@ -772,10 +756,9 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa * * @since 1.1 */ - void processUpdatedSnapshot(Object nodeId, DataRow diff) { + void processUpdatedSnapshot(ObjectId nodeId, DataRow diff) { - // access object map directly - the method should be called in a synchronized - // context... + // access object map directly - the method should be called in a synchronized context... DataObject object = (DataObject) objectMap.get(nodeId); // no object, or HOLLOW object require no processing @@ -791,17 +774,14 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa if (delegate.shouldMergeChanges(object, diff)) { ClassDescriptor descriptor = context .getEntityResolver() - .getClassDescriptor(((ObjectId) nodeId).getEntityName()); + .getClassDescriptor(nodeId.getEntityName()); // TODO: andrus, 5/26/2006 - call to 'getSnapshot' is expensive, // however my attempts to merge the 'diff' instead of snapshot - // via 'refreshObjectWithSnapshot' resulted in even worse - // performance. - // This sounds counterintuitive (Not sure if this is some HotSpot - // related glitch)... still keeping the old algorithm here until - // we - // switch from snapshot events to GraphEvents and all this code - // becomes obsolete. + // via 'refreshObjectWithSnapshot' resulted in even worse performance. + // This sounds counterintuitive (Not sure if this is some HotSpot related glitch)... + // still keeping the old algorithm here until we switch from snapshot events + // to GraphEvents and all this code becomes obsolete. DataRow snapshot = getSnapshot(object.getObjectId()); DataRowUtils.refreshObjectWithSnapshot( @@ -821,7 +801,7 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa if (delegate.shouldMergeChanges(object, diff)) { ClassDescriptor descriptor = context .getEntityResolver() - .getClassDescriptor(((ObjectId) nodeId).getEntityName()); + .getClassDescriptor(nodeId.getEntityName()); DataRowUtils.forceMergeWithSnapshot( context, descriptor, @@ -985,7 +965,7 @@ public class ObjectStore implements Serializable, SnapshotEventListener, GraphMa registerLifecycleEventInducedChange(diff); } - registerDiff(nodeId, diff); + registerDiff((ObjectId)nodeId, diff); } // an ObjectIdQuery optimized for retrieval of multiple snapshots - it can be reset http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/cayenne-server/src/test/java/org/apache/cayenne/CayenneContextGraphManagerTest.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/test/java/org/apache/cayenne/CayenneContextGraphManagerTest.java b/cayenne-server/src/test/java/org/apache/cayenne/CayenneContextGraphManagerTest.java new file mode 100644 index 0000000..1adfb52 --- /dev/null +++ b/cayenne-server/src/test/java/org/apache/cayenne/CayenneContextGraphManagerTest.java @@ -0,0 +1,67 @@ +/***************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.cayenne; + +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * @since 4.0 + */ +public class CayenneContextGraphManagerTest { + + private CayenneContextGraphManager graphManager; + + @Before + public void before() { + CayenneContext mockContext = mock(CayenneContext.class); + this.graphManager = new CayenneContextGraphManager(mockContext, false, false); + } + + @Test + public void testRegisterNode() { + + ObjectId id = new ObjectId("E1", "ID", 500); + Persistent object = mock(Persistent.class); + + graphManager.registerNode(id, object); + assertSame(object, graphManager.getNode(id)); + } + + @Test + public void testUnregisterNode() { + + ObjectId id = new ObjectId("E1", "ID", 500); + Persistent object = mock(Persistent.class); + + graphManager.registerNode(id, object); + Object unregistered = graphManager.unregisterNode(id); + assertSame(object, unregistered); + + verify(object, times(0)).setObjectId(null); + verify(object).setObjectContext(null); + verify(object).setPersistenceState(PersistenceState.TRANSIENT); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextObjectTrackingIT.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextObjectTrackingIT.java b/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextObjectTrackingIT.java index 5d48075..8ce1415 100644 --- a/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextObjectTrackingIT.java +++ b/cayenne-server/src/test/java/org/apache/cayenne/access/DataContextObjectTrackingIT.java @@ -113,7 +113,7 @@ public class DataContextObjectTrackingIT extends ServerCase { assertEquals(PersistenceState.TRANSIENT, obj.getPersistenceState()); assertNull(obj.getObjectContext()); - assertNull(obj.getObjectId()); + assertEquals(oid, obj.getObjectId()); assertNull(context.getGraphManager().getNode(oid)); assertNull(context.getObjectStore().getCachedSnapshot(oid)); } http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreIT.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreIT.java b/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreIT.java index 70a726c..8c768e8 100644 --- a/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreIT.java +++ b/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreIT.java @@ -87,7 +87,7 @@ public class ObjectStoreIT extends ServerCase { context.getObjectStore().objectsUnregistered(Collections.singletonList(object)); - assertNull(object.getObjectId()); + assertEquals(oid, object.getObjectId()); assertNull(context.getObjectStore().getNode(oid)); // in the future this may not be the case http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreTest.java ---------------------------------------------------------------------- diff --git a/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreTest.java b/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreTest.java new file mode 100644 index 0000000..b8097c2 --- /dev/null +++ b/cayenne-server/src/test/java/org/apache/cayenne/access/ObjectStoreTest.java @@ -0,0 +1,71 @@ +/***************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.cayenne.access; + +import java.util.HashMap; + +import org.apache.cayenne.ObjectId; +import org.apache.cayenne.PersistenceState; +import org.apache.cayenne.Persistent; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * @since 4.0 + */ +public class ObjectStoreTest { + + private ObjectStore objectStore; + + @Before + public void before() { + DataRowStore sharedCache = mock(DataRowStore.class); + this.objectStore = new ObjectStore(sharedCache, new HashMap<Object, Persistent>()); + } + + @Test + public void testRegisterNode() { + + ObjectId id = new ObjectId("E1", "ID", 500); + Persistent object = mock(Persistent.class); + + objectStore.registerNode(id, object); + assertSame(object, objectStore.getNode(id)); + } + + @Test + public void testUnregisterNode() { + + ObjectId id = new ObjectId("E1", "ID", 500); + Persistent object = mock(Persistent.class); + + objectStore.registerNode(id, object); + Object unregistered = objectStore.unregisterNode(id); + assertSame(object, unregistered); + + verify(object, times(0)).setObjectId(null); + verify(object).setObjectContext(null); + verify(object).setPersistenceState(PersistenceState.TRANSIENT); } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/cayenne/blob/a0ef5ad2/docs/doc/src/main/resources/RELEASE-NOTES.txt ---------------------------------------------------------------------- diff --git a/docs/doc/src/main/resources/RELEASE-NOTES.txt b/docs/doc/src/main/resources/RELEASE-NOTES.txt index 7c2cd8b..2d04acb 100644 --- a/docs/doc/src/main/resources/RELEASE-NOTES.txt +++ b/docs/doc/src/main/resources/RELEASE-NOTES.txt @@ -29,6 +29,7 @@ CAY-2272 ColumnSelect: methods to manually control DISTINCT clause Bug Fixes: CAY-2240 Modeler: issue with cursor rendering for EJBQL query +CAY-2243 ObjectContext.getGraphManager().unregisterObject() inconsistencies CAY-2256 Cannot Save/Insert an Object With null Flattened (complex) toOne Relationship (see also CAY-2146) CAY-2265 ServerRuntime.builder() fails to set default runtime name when a the project file doesn't follow recognized pattern
