You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cayenne.apache.org by nt...@apache.org on 2017/03/22 15:11:25 UTC
cayenne git commit: CAY-2243
ObjectContext.getGraphManager().unregisterObject() inconsistencies
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 <st...@gmail.com>
Authored: Wed Mar 22 18:11:18 2017 +0300
Committer: Nikita Timofeev <st...@gmail.com>
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