From e173f6e9a905333494ab21ac5f307da0d0941aec Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 12 Oct 2021 15:10:36 +1300 Subject: [PATCH] Tidy only DefaultPersistenceContext after merge of #2411 --- .../server/core/OrmQueryRequest.java | 1 - .../DefaultPersistenceContext.java | 29 ++----- .../tests/basic/TestPersistenceContext.java | 80 +++++++++---------- 3 files changed, 46 insertions(+), 64 deletions(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java index 6ef1e352b..718df243c 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java @@ -2,7 +2,6 @@ package io.ebeaninternal.server.core; import io.ebean.*; import io.ebean.bean.BeanCollection; -import io.ebean.bean.EntityBean; import io.ebean.bean.PersistenceContext; import io.ebean.cache.QueryCacheEntry; import io.ebean.common.BeanList; diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/DefaultPersistenceContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/DefaultPersistenceContext.java index 1ff3d44fa..d10d92ab2 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/DefaultPersistenceContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/DefaultPersistenceContext.java @@ -1,7 +1,6 @@ package io.ebeaninternal.server.transaction; import io.ebean.bean.EntityBean; -import io.ebean.bean.PersistenceContext; import io.ebeaninternal.api.SpiBeanType; import io.ebeaninternal.api.SpiBeanTypeManager; import io.ebeaninternal.api.SpiPersistenceContext; @@ -30,7 +29,7 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { private final HashMap, ClassContext> typeCache = new HashMap<>(); private final ReentrantLock lock = new ReentrantLock(); - private final ReferenceQueue queue = new ReferenceQueue(); + private final ReferenceQueue queue = new ReferenceQueue<>(); /** * When we are inside an iterate loop, we will add only WeakReferences. This @@ -66,9 +65,6 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { } } - /** - * Set an object into the PersistenceContext. - */ @Override public void put(Class rootType, Object id, Object bean) { lock.lock(); @@ -116,9 +112,6 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { } } - /** - * Return the number of beans of the given type in the persistence context. - */ @Override public int size(Class rootType) { lock.lock(); @@ -131,9 +124,6 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { } } - /** - * Clear the PersistenceContext. - */ @Override public void clear() { lock.lock(); @@ -210,10 +200,10 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { private void expungeStaleEntries() { Reference ref; while ((ref = queue.poll()) != null) { - ((BeanRef)ref).expunge(); + ((BeanRef) ref).expunge(); } } - + @Override public String toString() { lock.lock(); @@ -228,18 +218,16 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { private ClassContext classContext(Class rootType) { return typeCache.computeIfAbsent(rootType, k -> new ClassContext(k, queue)); } - + private static class ClassContext { private final Map map = new HashMap<>(); private final Class rootType; private final ReferenceQueue queue; private Set deleteSet; - private boolean useReferences; private int weakCount; - private ClassContext(Class rootType, ReferenceQueue queue) { this.rootType = rootType; this.queue = queue; @@ -249,12 +237,11 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { * When called with "true", initialize referenceQueue and store BeanRefs instead * of real object references. */ - public ClassContext useReferences(boolean useReferences) { + private ClassContext useReferences(boolean useReferences) { this.useReferences = useReferences; return this; } - @Override public String toString() { return "size:" + map.size() + " (" + weakCount + " weak)"; @@ -263,7 +250,7 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { private Object get(Object id) { Object ret = map.get(id); if (ret instanceof BeanRef) { - return ((BeanRef)ret).get(); + return ((BeanRef) ret).get(); } else { return ret; } @@ -291,7 +278,7 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { private void put(Object id, Object b) { if (useReferences) { weakCount++; - map.put(id, new BeanRef(this, id, b, queue)); + map.put(id, new BeanRef(this, id, b, queue)); } else { map.put(id, b); } @@ -327,7 +314,7 @@ public final class DefaultPersistenceContext implements SpiPersistenceContext { void dirtyBeans(SpiBeanTypeManager manager, List list) { final SpiBeanType beanType = manager.beanType(rootType); for (Object value : map.values()) { - if (queue != null && value instanceof BeanRef) { + if (value instanceof BeanRef) { value = ((BeanRef) value).get(); if (value == null) continue; } diff --git a/ebean-test/src/test/java/org/tests/basic/TestPersistenceContext.java b/ebean-test/src/test/java/org/tests/basic/TestPersistenceContext.java index 4d79e5b89..9a53847b4 100644 --- a/ebean-test/src/test/java/org/tests/basic/TestPersistenceContext.java +++ b/ebean-test/src/test/java/org/tests/basic/TestPersistenceContext.java @@ -2,11 +2,9 @@ package org.tests.basic; import io.ebean.BaseTestCase; import io.ebean.DB; -import io.ebean.Query; import io.ebean.Transaction; import io.ebeaninternal.api.SpiPersistenceContext; import io.ebeaninternal.api.SpiTransaction; - import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.tests.model.basic.Customer; @@ -17,33 +15,25 @@ import java.util.List; import java.util.WeakHashMap; import java.util.concurrent.atomic.AtomicInteger; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; -public class TestPersistenceContext extends BaseTestCase { +class TestPersistenceContext extends BaseTestCase { @Test - public void test() { - + void test() { ResetBasicData.reset(); - // implicit transaction with its own - // persistence context + // implicit transaction with its own persistence context Order oBefore = DB.find(Order.class, 1); - - Order order = null; - // start a persistence context DB.beginTransaction(); + + Order order; try { - order = DB.find(Order.class, 1); - - // not the same instance ...as a different - // persistence context - assertTrue(order != oBefore); - + // not the same instance ...as a different persistence context + assertNotSame(order, oBefore); // finds an existing bean in the persistence context // ... so doesn't even execute a query @@ -51,19 +41,17 @@ public class TestPersistenceContext extends BaseTestCase { Order o3 = DB.reference(Order.class, 1); // all the same instance - assertTrue(order == o2); - assertTrue(order == o3); + assertSame(order, o2); + assertSame(order, o3); } finally { DB.endTransaction(); } - // implicit transaction with its own - // persistence context + // implicit transaction with its own persistence context Order oAfter = DB.find(Order.class, 1); - - assertTrue(oAfter != oBefore); - assertTrue(oAfter != order); + assertNotSame(oAfter, oBefore); + assertNotSame(oAfter, order); // start a persistence context DB.beginTransaction(); @@ -76,13 +64,16 @@ public class TestPersistenceContext extends BaseTestCase { .setUseCache(false) .setId(id) .findOne(); + assert customer != null; System.gc(); Order order2 = DB.find(Order.class, orderId); + assert order2 != null; Customer customer2 = order2.getCustomer(); + assert customer2 != null; assertEquals(customer.getId(), customer2.getId()); - assertTrue(customer == customer2); + assertSame(customer, customer2); } finally { DB.endTransaction(); @@ -120,11 +111,10 @@ public class TestPersistenceContext extends BaseTestCase { }); } } - + @Disabled // run manually @Test - void testPcScopes() throws InterruptedException { - + void testPcScopes_with_weakReferences() throws InterruptedException { for (int i = 0; i < 5000; i++) { Customer c = new Customer(); c.setName("Customer #" + i); @@ -141,12 +131,15 @@ public class TestPersistenceContext extends BaseTestCase { c.setSmallnote("one of the first 100"); } + // use lastBean to hold onto a weak reference bean Customer[] lastBean = new Customer[1]; + // findEach switches on use of weak reference in persistence context DB.find(Customer.class).setLazyLoadBatchSize(1).findEach(customer -> { if (customer.getId() <= 100) { assertEquals("one of the first 100", customer.getSmallnote()); // nested finds - DB.find(Order.class).where().eq("customer", customer).findEach(20, consumer->{}); + DB.find(Order.class).where().eq("customer", customer).findEach(20, consumer -> { + }); } else { assertNotEquals("one of the first 100", customer.getSmallnote()); } @@ -154,30 +147,33 @@ public class TestPersistenceContext extends BaseTestCase { }); SpiPersistenceContext pc = ((SpiTransaction) txn).getPersistenceContext(); - System.out.println(pc.toString()); // Customer=size:5000 (4900 weak), Order=size:100 (100 weak) - + // the first 100 customers using strong references + assertThat(pc.toString()).contains("Customer=size:5000 (4900 weak)"); + assertThat(pc.toString()).contains("Order=size:100 (100 weak)"); + System.gc(); Thread.sleep(100); pc.get(Customer.class, 1); // trigger expungeStaleEntries - pc.get(Order.class, 1); // Checkme: Would it be a problem, that every classContext has its own queue - System.out.println(pc.toString()); // Customer=size:101 (1 weak), no orders + // pc.get(Order.class, 1); + assertThat(pc.toString()).contains("Customer=size:101 (1 weak)"); + assertThat(pc.toString()).contains("Order=size:0 (0 weak)"); first100 = DB.find(Customer.class).where().le("id", 100).findList(); for (Customer c : first100) { assertEquals("one of the first 100", c.getSmallnote()); } Customer lastBeanFromDb = DB.find(Customer.class).setId(lastBean[0].getId()).findOne(); - assertTrue(lastBeanFromDb == lastBean[0]); - + assertSame(lastBeanFromDb, lastBean[0]); + // read 200 - DB.find(Customer.class).where().le("id", 200).findList(); - System.out.println(pc.toString()); // Customer=size:201 (1 weak) - - lastBean[0] = null; + DB.find(Customer.class).where().le("id", 200).findList(); + assertThat(pc.toString()).contains("Customer=size:201 (1 weak)"); + + lastBean[0] = null; // allow GC on this one lastBeanFromDb = null; System.gc(); Thread.sleep(100); - System.out.println(pc.toString()); // Customer=size:200 (0 weak) + assertThat(pc.toString()).contains("Customer=size:200 (0 weak)"); } } }