Bug 544202 - Fix CacheKey.protectedForeignKeys update

Signed-off-by: Alexandre Jacob <alexandref.jacob@gmail.com>

foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/descriptors/ObjectBuilder.java

@@ -1,6 +1,6 @@
 /*
- * Copyright (c) 1998, 2018 Oracle and/or its affiliates. All rights reserved.
- * Copyright (c) 1998, 2018 IBM Corporation. All rights reserved.
+ * Copyright (c) 1998, 2019 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2019 IBM Corporation. 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
@@ -27,6 +27,8 @@
 //       - 499335: Multiple embeddable fields can't reference same object
 //     02/20/2018-2.7 Will Dazey
 //       - 529602: Added support for CLOBs in DELETE statements for Oracle
+//     07/02/2019-3.0 Alexandre Jacob
+//       - 544202: Fixed refresh of foreign key values when cacheKey was invalidated
 package org.eclipse.persistence.internal.descriptors;
 
 import java.io.*;
@@ -988,6 +990,15 @@ protected Object buildObject(boolean returnCacheKey, ObjectBuildingQuery query,
 
             FetchGroup fetchGroup = query.getExecutionFetchGroup(concreteDescriptor);
 
+            // 544202: cachedForeignKeys should be refreshed when cacheKey was invalidated
+            if (domainWasMissing || shouldRetrieveBypassCache
+                || cacheKey.getInvalidationState() == CacheKey.CACHE_KEY_INVALID) {
+
+                if (isProtected && (cacheKey != null)) {
+                    cacheForeignKeyValues(databaseRow, cacheKey, session);
+                }
+            }
+
             if (domainWasMissing || shouldRetrieveBypassCache) {
                 cacheHit = false;
                 if (domainObject == null || shouldStoreBypassCache) {
@@ -1016,9 +1027,6 @@ protected Object buildObject(boolean returnCacheKey, ObjectBuildingQuery query,
                     cacheKey.setObject(domainObject);
                 }
                 concreteObjectBuilder.buildAttributesIntoObject(domainObject, cacheKey, databaseRow, query, joinManager, fetchGroup, false, session);
-                if (isProtected && (cacheKey != null)) {
-                    cacheForeignKeyValues(databaseRow, cacheKey, session);
-                }
                 if (shouldMaintainCache && !shouldStoreBypassCache) {
                     // Set the fetch group to the domain object, after built.
                     if ((fetchGroup != null) && concreteDescriptor.hasFetchGroupManager()) {

jpa/eclipselink.jpa.test.jse/src/org/eclipse/persistence/jpa/test/sharedcache/TestSharedCacheWithNonCacheable.java

@@ -0,0 +1,188 @@
+/********************************************************************************
+ * Copyright (c) 2019 IBM Corporation. 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
+ ******************************************************************************/
+
+// Contributors:
+//     07/02/2019-3.0 Alexandre Jacob
+//        - 544202: Fixed refresh of foreign key values when cacheKey was invalidated
+
+package org.eclipse.persistence.jpa.test.sharedcache;
+
+import static org.junit.Assert.assertEquals;
+
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+
+import org.eclipse.persistence.jpa.JpaEntityManager;
+import org.eclipse.persistence.jpa.test.framework.DDLGen;
+import org.eclipse.persistence.jpa.test.framework.Emf;
+import org.eclipse.persistence.jpa.test.framework.EmfRunner;
+import org.eclipse.persistence.jpa.test.framework.Property;
+import org.eclipse.persistence.jpa.test.sharedcache.model.Student;
+import org.eclipse.persistence.jpa.test.sharedcache.model.Teacher;
+import org.eclipse.persistence.sessions.Session;
+import org.eclipse.persistence.sessions.SessionProfiler;
+import org.eclipse.persistence.tools.profiler.PerformanceMonitor;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+@RunWith(EmfRunner.class)
+public class TestSharedCacheWithNonCacheable {
+
+    @Emf(createTables = DDLGen.DROP_CREATE, classes = { Teacher.class, Student.class },
+        properties = {
+            @Property(name = "eclipselink.logging.level", value = "FINE"),
+            @Property(name = "eclipselink.logging.parameters", value = "true"),
+            @Property(name = "eclipselink.cache.shared.default", value = "false"),
+            @Property(name = "eclipselink.profiler", value = "PerformanceMonitor")
+        })
+    private EntityManagerFactory emf;
+
+    private Session session;
+
+    private PerformanceMonitor performanceMonitor;
+
+    @Before
+    public void setup() {
+        session = ((JpaEntityManager) emf.createEntityManager()).getServerSession();
+        performanceMonitor = (PerformanceMonitor) session.getProfiler();
+
+        // We populate DB with 2 students and 1 teacher
+        Student student1 = new Student(1, "Picasso");
+        Student student2 = new Student(2, "Pablo");
+
+        Teacher teacher = new Teacher(1, "Foo", student1);
+
+        final EntityManager em = this.emf.createEntityManager();
+        final EntityTransaction transaction = em.getTransaction();
+
+        transaction.begin();
+        em.persist(student1);
+        em.persist(student2);
+        em.persist(teacher);
+        transaction.commit();
+
+        em.close();
+    }
+
+    @Test
+    public void testSharedCacheWithNonCacheable() throws Exception {
+        // 1 : We want to get our Teacher 1. He is supposed to be in the shared cache.
+        // The Student linked to our Teacher should not be in the cache. (@Noncacheable)
+        {
+            Teacher teacher = this.findTeacher(1);
+
+            assertEquals(teacher.getId(), 1);
+            assertEquals(teacher.getName(), "Foo");
+
+            // Our Teacher IS in shared cache
+            assertEquals(performanceMonitor.getOperationTime(SessionProfiler.CacheHits), 1L); // + 1
+
+            Student student = teacher.getStudent();
+
+            assertEquals(student.getId(), 1); // trigger lazy loading of our Student
+
+            // Our Student is NOT in shared cache
+            assertEquals(performanceMonitor.getOperationTime(SessionProfiler.CacheHits), 1L); // no change
+        }
+
+        // 2 : We change our Teacher 1 in native SQL. The name and the linked student are changed.
+        {
+            EntityManager em = emf.createEntityManager();
+
+            EntityTransaction transaction = em.getTransaction();
+
+            transaction.begin();
+            em.createNativeQuery("update TEACHER set NAME = ?, STUDENT_ID = ? where ID = ?")
+                .setParameter(1, "Bar")
+                .setParameter(2, 2)
+                .setParameter(3, 1)
+                .executeUpdate();
+            transaction.commit();
+
+            em.close();
+        }
+
+        // 3 : We want to get our Teacher 1 ONE MORE TIME. He is still supposed to be in the shared cache.
+        // The Student linked to our Teacher should not be in the cache. (@Noncacheable)
+        {
+            Teacher teacher = this.findTeacher(1);
+
+            assertEquals(teacher.getId(), 1);
+            assertEquals(teacher.getName(), "Foo");
+
+            // Our Teacher IS in shared cache
+            assertEquals(performanceMonitor.getOperationTime(SessionProfiler.CacheHits), 2L); // + 1
+
+            Student student = teacher.getStudent();
+
+            assertEquals(student.getId(), 1); // trigger lazy loading of our Student
+
+            // Our Student is NOT in shared cache
+            assertEquals(performanceMonitor.getOperationTime(SessionProfiler.CacheHits), 2L); // no change
+        }
+
+        // 4 : Now we clear shared cache.
+        {
+            emf.getCache().evict(Teacher.class);
+        }
+
+        // 5 : We want to get our Teacher 1 for the THIRD TIME. He is not in the shared cache anymore (invalidated)
+        // Data should reflect our update from (2)
+        {
+            Teacher teacher = this.findTeacher(1);
+
+            assertEquals(teacher.getId(), 1);
+            assertEquals(teacher.getName(), "Bar"); // Updated
+
+            // Our Teacher is NOT in shared cache
+            assertEquals(performanceMonitor.getOperationTime(SessionProfiler.CacheHits), 2L); // no change
+
+            Student student = teacher.getStudent();
+
+            assertEquals(student.getId(), 2); // trigger lazy loading of our Student
+
+            // Our Student is NOT in shared cache
+            assertEquals(performanceMonitor.getOperationTime(SessionProfiler.CacheHits), 2L); // no change
+        }
+
+        // 6 : We want to get our Teacher 1 for the FOURTH TIME. He is back in the shared.
+        // Data should reflect our update from (2)
+        {
+            Teacher teacher = this.findTeacher(1);
+
+            assertEquals(teacher.getId(), 1);
+            assertEquals(teacher.getName(), "Bar"); // Updated
+
+            // Our Teacher IS in shared cache
+            assertEquals(performanceMonitor.getOperationTime(SessionProfiler.CacheHits), 3L); // + 1
+
+            Student student = teacher.getStudent();
+
+            // Before correction of bug 544202 this value was 1 because CacheKey.protectedForeignKeys was never updated
+            assertEquals(student.getId(), 2); // trigger lazy loading of our Student
+
+            // Our Student is NOT in shared cache
+            assertEquals(performanceMonitor.getOperationTime(SessionProfiler.CacheHits), 3L); // no change
+        }
+    }
+
+    private Teacher findTeacher(int id) {
+        final EntityManager em = this.emf.createEntityManager();
+        Teacher teacher = em.find(Teacher.class, 1);
+        teacher.getStudent().getId();
+        em.close();
+
+        return teacher;
+    }
+}

jpa/eclipselink.jpa.test.jse/src/org/eclipse/persistence/jpa/test/sharedcache/model/Student.java

@@ -0,0 +1,54 @@
+/********************************************************************************
+ * Copyright (c) 2019 IBM Corporation. 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
+ ******************************************************************************/
+
+// Contributors:
+//     07/02/2019-3.0 Alexandre Jacob
+//        - 544202: Fixed refresh of foreign key values when cacheKey was invalidated
+
+package org.eclipse.persistence.jpa.test.sharedcache.model;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+@Entity
+public class Student {
+
+    @Id
+    private int id;
+
+    private String name;
+
+    public Student() {
+
+    }
+
+    public Student(int id, String name) {
+        this.id = id;
+        this.name = name;
+    }
+
+    public int getId() {
+        return id;
+    }
+
+    public void setId(int id) {
+        this.id = id;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+}

jpa/eclipselink.jpa.test.jse/src/org/eclipse/persistence/jpa/test/sharedcache/model/Teacher.java

@@ -0,0 +1,75 @@
+/********************************************************************************
+ * Copyright (c) 2019 IBM Corporation. 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
+ ******************************************************************************/
+
+// Contributors:
+//     07/02/2019-3.0 Alexandre Jacob
+//        - 544202: Fixed refresh of foreign key values when cacheKey was invalidated
+
+package org.eclipse.persistence.jpa.test.sharedcache.model;
+
+import javax.persistence.Cacheable;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.ManyToOne;
+
+import org.eclipse.persistence.annotations.Cache;
+import org.eclipse.persistence.annotations.Noncacheable;
+import org.eclipse.persistence.config.CacheIsolationType;
+
+@Entity
+@Cacheable(value = true)
+@Cache(isolation = CacheIsolationType.SHARED)
+public class Teacher {
+
+    @Id
+    private int id;
+
+    private String name;
+
+    @Noncacheable
+    @ManyToOne
+    private Student student;
+
+    public Teacher() {
+
+    }
+
+    public Teacher(int id, String name, Student student) {
+        this.id = id;
+        this.name = name;
+        this.student = student;
+    }
+
+    public int getId() {
+        return id;
+    }
+
+    public void setId(int id) {
+        this.id = id;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    public Student getStudent() {
+        return student;
+    }
+
+    public void setStudent(Student student) {
+        this.student = student;
+    }
+}