Issue 1950: Duplicate objects in UnitOfWorkImpl.primaryKeyToNewObjects

- fix: add objects to primaryKeyToNewObjects in assignSequenceNumber as it is required for some use cases - added unit tests Signed-off-by: Patrick Schmitt <[email protected]>
eclipse-ee4j · Oct 2, 2023 · 826b289 · 826b289
1 parent b68e8ae
commit 826b289
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 1 deletion.
diff --git a/...e/persistence/testing/tests/unitofwork/UOWPrimaryKeyToNewObjectsDuplicateObjectsTest.java b/...e/persistence/testing/tests/unitofwork/UOWPrimaryKeyToNewObjectsDuplicateObjectsTest.java
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 1998, 2023 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0,
+ * or the Eclipse Distribution License v. 1.0 which is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
+ */
+package org.eclipse.persistence.testing.tests.unitofwork;
+
+import org.eclipse.persistence.internal.sessions.UnitOfWorkImpl;
+import org.eclipse.persistence.sessions.Session;
+import org.eclipse.persistence.testing.framework.TestCase;
+import org.eclipse.persistence.testing.models.employee.domain.Employee;
+
+import java.util.IdentityHashMap;
+
+/**
+ * This test is in response to issue 1950: Duplicate objects in UnitOfWorkImpl.primaryKeyToNewObjects
+ *
+ * When a new object is registered for persist by calling registerNewObjectForPersist
+ * the object is potentially added twice to primaryKeyToNewObjects. Once during assignSequenceNumber and
+ * then in registerNewObjectClone. This test verifies that the object is contained only once in primaryKeyToNewObjects.
+ */
+public class UOWPrimaryKeyToNewObjectsDuplicateObjectsTest extends TestCase {
+    public UOWPrimaryKeyToNewObjectsDuplicateObjectsTest() {
+        setDescription("This test verifies that no duplicates exist in primaryKeyToNewObjects after registering a new object");
+    }
+
+    @Override
+    public void reset() {
+        getAbstractSession().rollbackTransaction();
+        getSession().getIdentityMapAccessor().initializeAllIdentityMaps();
+    }
+
+    @Override
+    public void setup() {
+        getAbstractSession().beginTransaction();
+    }
+
+    @Override
+    public void test() {
+        Session session = getSession();
+        UnitOfWorkImpl uow = (UnitOfWorkImpl) session.acquireUnitOfWork();
+        Employee emp = new Employee();
+        emp.setFirstName("UOWPrimaryKeyToNewObjectsDuplicateObjectsTest");
+        uow.registerNewObjectForPersist(emp, new IdentityHashMap<>());
+        // there should be only one object in primaryKeyToNewObjects
+        assertEquals("Unexpected amount of entities in primaryKeyToNewObjects", 1,
+                uow.getPrimaryKeyToNewObjects().get(emp.getId()).size());
+    }
+}
diff --git a/...est/src/it/java/org/eclipse/persistence/testing/tests/unitofwork/UnitOfWorkTestSuite.java b/...est/src/it/java/org/eclipse/persistence/testing/tests/unitofwork/UnitOfWorkTestSuite.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2021 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2023 Oracle and/or its affiliates. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -70,6 +70,9 @@ public void addTests() {
 
         addTest(new UnregisterUnitOfWorkTest());
 
+        // Issue 1950 - Duplicate objects in UnitOfWorkImpl.primaryKeyToNewObjects
+        addTest(new UOWPrimaryKeyToNewObjectsDuplicateObjectsTest());
+
         // EL Bug 252047 - Mutable attributes are not cloned when isMutable is enabled on a Direct Mapping
         addTest(new CloneAttributeIfMutableTest());
 

diff --git a/...sistence.core/src/main/java/org/eclipse/persistence/internal/sessions/UnitOfWorkImpl.java b/...sistence.core/src/main/java/org/eclipse/persistence/internal/sessions/UnitOfWorkImpl.java
@@ -547,6 +547,8 @@ public Object assignSequenceNumber(Object object, ClassDescriptor descriptor) th
             ObjectBuilder builder = descriptor.getObjectBuilder();
             try {
                 value = builder.assignSequenceNumber(object, this);
+                getPrimaryKeyToNewObjects().putIfAbsent(value, new IdentityHashSet());
+                getPrimaryKeyToNewObjects().get(value).add(object);
             } catch (RuntimeException exception) {
                 handleException(exception);
             } finally {

diff --git a/.../java/org/eclipse/persistence/testing/tests/jpa/advanced/EntityManagerJUnitTestSuite.java b/.../java/org/eclipse/persistence/testing/tests/jpa/advanced/EntityManagerJUnitTestSuite.java
@@ -337,6 +337,7 @@ public static Test suiteSpring() {
         tests.add("testBeginTransactionClose");
         tests.add("testClose");
         tests.add("testPersistOnNonEntity");
+        tests.add("testPersistRemoveFind");
         tests.add("testWRITELock");
         tests.add("testOPTIMISTIC_FORCE_INCREMENTLock");
         tests.add("testReadOnlyTransactionalData");
@@ -4972,6 +4973,43 @@ public void testPersistOnNonEntity()
         Assert.assertTrue(testPass);
     }
 
+    /**
+     * Test for issue 1950 Duplicate objects in UnitOfWorkImpl.primaryKeyToNewObjects
+     *
+     * Objects may potentially be added to UnitOfWorkImpl.primaryKeyToNewObjects both during
+     * the call to UnitOfWorkImpl#assignSequenceNumber and then again in UnitOfWorkImpl#registerNewObjectClone.
+     * This can cause the EntityManager to return already removed entities, as only one of the saved
+     * references is removed.
+     */
+    public void testPersistRemoveFind()
+    {
+        EntityManager em = createEntityManager();
+        Employee employee = new Employee();
+        employee.setFirstName("Employee");
+        employee.setLastName("1");
+        Employee employee2 = new Employee();
+        employee2.setFirstName("Employee");
+        employee2.setLastName("2");
+        beginTransaction(em);
+        try {
+            em.persist(employee);
+           /* In order to hit the problematic code, we have to make sure there are still objects in the cache after
+            * we remove the first one, because otherwise the call to UnitOfWorkImpl#getObjectFromNewObjects
+            * will be skipped. Therefore, we register another employee object, which we won't remove. */
+            em.persist(employee2);
+            Employee clone = em.find(Employee.class, employee.getId());
+            // remove employee 1
+            em.remove(clone);
+            // a find call should not return employee 1, since we removed it earlier
+            clone = em.find(Employee.class, employee.getId());
+            assertNull("Removed employee was returned by em.find!", clone);
+        } catch (Exception e) {
+            fail("Unexpected exception thrown during test: " + e);
+        } finally {
+            rollbackTransaction(em);
+        }
+    }
+
     //detach(nonentity) throws illegalArgumentException
     public void testDetachNonEntity() {
         boolean testPass = false;