-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 544202 - Fix CacheKey.protectedForeignKeys update #369
Open
ajacob
wants to merge
1
commit into
eclipse-ee4j:master
Choose a base branch
from
ajacob:544202_master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
186 changes: 186 additions & 0 deletions
186
...it/java/org/eclipse/persistence/jpa/test/sharedcache/TestSharedCacheWithNonCacheable.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,186 @@ | ||
/* | ||
* Copyright (c) 2020 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 | ||
*/ | ||
|
||
// Contributors: | ||
// 04/19/2020-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 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; | ||
|
||
import jakarta.persistence.EntityManager; | ||
import jakarta.persistence.EntityManagerFactory; | ||
import jakarta.persistence.EntityTransaction; | ||
|
||
@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 PerformanceMonitor performanceMonitor; | ||
|
||
@Before | ||
public void setUp() { | ||
Session 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, id); | ||
teacher.getStudent().getId(); | ||
em.close(); | ||
|
||
return teacher; | ||
} | ||
} |
54 changes: 54 additions & 0 deletions
54
....jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/sharedcache/model/Student.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/* | ||
* Copyright (c) 2020 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 | ||
*/ | ||
|
||
// Contributors: | ||
// 04/19/2020-3.0 Alexandre Jacob | ||
// - 544202: Fixed refresh of foreign key values when cacheKey was invalidated | ||
|
||
package org.eclipse.persistence.jpa.test.sharedcache.model; | ||
|
||
import jakarta.persistence.Entity; | ||
import jakarta.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; | ||
} | ||
} |
75 changes: 75 additions & 0 deletions
75
....jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/sharedcache/model/Teacher.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
/* | ||
* Copyright (c) 2020 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 | ||
*/ | ||
|
||
// Contributors: | ||
// 04/19/2020-3.0 Alexandre Jacob | ||
// - 544202: Fixed refresh of foreign key values when cacheKey was invalidated | ||
|
||
package org.eclipse.persistence.jpa.test.sharedcache.model; | ||
|
||
import org.eclipse.persistence.annotations.Cache; | ||
import org.eclipse.persistence.annotations.Noncacheable; | ||
import org.eclipse.persistence.config.CacheIsolationType; | ||
|
||
import jakarta.persistence.Cacheable; | ||
import jakarta.persistence.Entity; | ||
import jakarta.persistence.Id; | ||
import jakarta.persistence.ManyToOne; | ||
|
||
@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; | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you follow the convention used in other files wrt formatting of license header? I do not think this is going to pass the copyright check - verify by running
mvn -Pcoding-standards glassfish-copyright-mave-plugin:check
ormvn -Pcoding-standards glassfish-copyright-mave-plugin:copyright
(the latter will show wrong files but won't fail the build in case of errors) - note that the check may take up to 2hours to finishThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @lukasj, thank you for your attention, I fixed the copyright as you suggested.
I ran:
mvn -X -Pcoding-standards glassfish-copyright:copyright -pl foundation/org.eclipse.persistence.core
And can confirm it ends with a
BUILD SUCCESS
and no reported errors.I rebased master on my branch in order to be up to date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated PR's description. Everything was already in bugzilla + test case but it's better to sum up here for future reference