From 4f18dc339f42d38e113db1964a9a473b5b561397 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Fri, 25 Aug 2023 20:56:55 +1200 Subject: [PATCH] Follow up for #3188 Fix findFutureList() and findFutureIds() when txn in scope Apply the same fix to findFutureList() and findFutureIds() --- .../io/ebeaninternal/api/SpiEbeanServer.java | 13 +--- .../java/io/ebeaninternal/api/SpiQuery.java | 11 --- .../server/core/DefaultServer.java | 31 ++++++--- .../server/query/CallableQueryIds.java | 8 ++- .../server/query/CallableQueryList.java | 13 ++-- .../server/querydefn/DefaultOrmQuery.java | 19 +----- .../xtest/internal/api/TDSpiEbeanServer.java | 4 +- .../tests/query/TestFindFutureRowCount.java | 68 ++++++++++++++++++- 8 files changed, 106 insertions(+), 61 deletions(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java b/ebean-core/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java index 9ea75f685..1399940b0 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java @@ -378,20 +378,11 @@ public interface SpiEbeanServer extends SpiServer, ExtendedServer, BeanCollectio */ List findList(SpiQuery query, Transaction transaction); - /** - * Deprecated migrate to using {@link Query#usingTransaction(Transaction)}. - */ FutureRowCount findFutureCount(SpiQuery query); - /** - * Deprecated migrate to using {@link Query#usingTransaction(Transaction)}. - */ - FutureIds findFutureIds(SpiQuery query, Transaction transaction); + FutureIds findFutureIds(SpiQuery query); - /** - * Deprecated migrate to using {@link Query#usingTransaction(Transaction)}. - */ - FutureList findFutureList(SpiQuery query, Transaction transaction); + FutureList findFutureList(SpiQuery query); /** * Deprecated migrate to using {@link Query#usingTransaction(Transaction)}. diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/SpiQuery.java b/ebean-core/src/main/java/io/ebeaninternal/api/SpiQuery.java index 6f6a6ae1b..4ac3823ad 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/SpiQuery.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/SpiQuery.java @@ -877,17 +877,6 @@ public interface SpiQuery extends Query, SpiQueryFetch, TxnProfileEventCod */ boolean isDisableReadAudit(); - /** - * Return true if this is a query executing in the background. - */ - boolean isFutureFetch(); - - /** - * Set to true to indicate the query is executing in a background thread - * asynchronously. - */ - void setFutureFetch(boolean futureFetch); - /** * Set the readEvent for future queries (as prepared in foreground thread). */ diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index 125d82c39..bf916be8b 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java @@ -1272,7 +1272,6 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { @Override public FutureRowCount findFutureCount(SpiQuery query) { SpiQuery copy = query.copy(); - copy.setFutureFetch(true); boolean createdTransaction = false; SpiTransaction transaction = query.transaction(); if (transaction == null) { @@ -1288,19 +1287,25 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { } @Override - public FutureIds findFutureIds(SpiQuery query, Transaction transaction) { + public FutureIds findFutureIds(SpiQuery query) { SpiQuery copy = query.copy(); - copy.setFutureFetch(true); - Transaction newTxn = createTransaction(); - QueryFutureIds queryFuture = new QueryFutureIds<>(new CallableQueryIds<>(this, copy, newTxn)); + boolean createdTransaction = false; + SpiTransaction transaction = query.transaction(); + if (transaction == null) { + transaction = currentServerTransaction(); + if (transaction == null) { + transaction = (SpiTransaction) createTransaction(); + createdTransaction = true; + } + } + QueryFutureIds queryFuture = new QueryFutureIds<>(new CallableQueryIds<>(this, copy, transaction, createdTransaction)); backgroundExecutor.execute(queryFuture.futureTask()); return queryFuture; } @Override - public FutureList findFutureList(SpiQuery query, Transaction transaction) { + public FutureList findFutureList(SpiQuery query) { SpiQuery spiQuery = query.copy(); - spiQuery.setFutureFetch(true); // FutureList query always run in it's own persistence content spiQuery.setPersistenceContext(new DefaultPersistenceContext()); if (!spiQuery.isDisableReadAudit()) { @@ -1308,8 +1313,16 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { desc.readAuditFutureList(spiQuery); } // Create a new transaction solely to execute the findList() at some future time - Transaction newTxn = createTransaction(); - QueryFutureList queryFuture = new QueryFutureList<>(new CallableQueryList<>(this, spiQuery, newTxn)); + boolean createdTransaction = false; + SpiTransaction transaction = query.transaction(); + if (transaction == null) { + transaction = currentServerTransaction(); + if (transaction == null) { + transaction = (SpiTransaction) createTransaction(); + createdTransaction = true; + } + } + QueryFutureList queryFuture = new QueryFutureList<>(new CallableQueryList<>(this, spiQuery, transaction, createdTransaction)); backgroundExecutor.execute(queryFuture.futureTask()); return queryFuture; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/CallableQueryIds.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/CallableQueryIds.java index c236ccb6d..6a4391bc6 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/CallableQueryIds.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/CallableQueryIds.java @@ -12,9 +12,11 @@ import java.util.concurrent.Callable; */ public final class CallableQueryIds extends CallableQuery implements Callable> { + private final boolean createdTransaction; - public CallableQueryIds(SpiEbeanServer server, SpiQuery query, Transaction t) { + public CallableQueryIds(SpiEbeanServer server, SpiQuery query, Transaction t, boolean createdTransaction) { super(server, query, t); + this.createdTransaction = createdTransaction; } /** @@ -28,7 +30,9 @@ public final class CallableQueryIds extends CallableQuery implements Calla try { return server.findIdsWithCopy(query, transaction); } finally { - transaction.end(); + if (createdTransaction) { + transaction.end(); + } } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/CallableQueryList.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/CallableQueryList.java index 350cd67f0..53f56b319 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/CallableQueryList.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/CallableQueryList.java @@ -9,13 +9,14 @@ import java.util.concurrent.Callable; /** * Represent the findList query as a Callable. - * - * @param the entity bean type */ public final class CallableQueryList extends CallableQuery implements Callable> { - public CallableQueryList(SpiEbeanServer server, SpiQuery query, Transaction t) { + private final boolean createdTransaction; + + public CallableQueryList(SpiEbeanServer server, SpiQuery query, Transaction t, boolean createdTransaction) { super(server, query, t); + this.createdTransaction = createdTransaction; } /** @@ -26,10 +27,10 @@ public final class CallableQueryList extends CallableQuery implements Call try { return server.findList(query, transaction); } finally { - // cleanup the underlying connection - transaction.end(); + if (createdTransaction) { + transaction.end(); + } } } - } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java b/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java index c62f608a1..3850a1600 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java @@ -86,11 +86,6 @@ public class DefaultOrmQuery extends AbstractQuery implements SpiQuery { */ private boolean distinct; - /** - * Set to true if this is a future fetch using background threads. - */ - private boolean futureFetch; - /** * Only used for read auditing with findFutureList() query. */ @@ -1549,12 +1544,12 @@ public class DefaultOrmQuery extends AbstractQuery implements SpiQuery { @Override public final FutureIds findFutureIds() { - return server.findFutureIds(this, transaction); + return server.findFutureIds(this); } @Override public final FutureList findFutureList() { - return server.findFutureList(this, transaction); + return server.findFutureList(this); } @Override @@ -1922,16 +1917,6 @@ public class DefaultOrmQuery extends AbstractQuery implements SpiQuery { return disableReadAudit; } - @Override - public final boolean isFutureFetch() { - return futureFetch; - } - - @Override - public final void setFutureFetch(boolean backgroundFetch) { - this.futureFetch = backgroundFetch; - } - @Override public final void setFutureFetchAudit(ReadEvent event) { this.futureFetchAudit = event; diff --git a/ebean-test/src/test/java/io/ebean/xtest/internal/api/TDSpiEbeanServer.java b/ebean-test/src/test/java/io/ebean/xtest/internal/api/TDSpiEbeanServer.java index 8516fa300..33de0b7ba 100644 --- a/ebean-test/src/test/java/io/ebean/xtest/internal/api/TDSpiEbeanServer.java +++ b/ebean-test/src/test/java/io/ebean/xtest/internal/api/TDSpiEbeanServer.java @@ -661,12 +661,12 @@ public class TDSpiEbeanServer extends TDSpiServer implements SpiEbeanServer { } @Override - public FutureIds findFutureIds(SpiQuery query, Transaction transaction) { + public FutureIds findFutureIds(SpiQuery query) { return null; } @Override - public FutureList findFutureList(SpiQuery query, Transaction transaction) { + public FutureList findFutureList(SpiQuery query) { return null; } diff --git a/ebean-test/src/test/java/org/tests/query/TestFindFutureRowCount.java b/ebean-test/src/test/java/org/tests/query/TestFindFutureRowCount.java index 626071bfc..4193add25 100644 --- a/ebean-test/src/test/java/org/tests/query/TestFindFutureRowCount.java +++ b/ebean-test/src/test/java/org/tests/query/TestFindFutureRowCount.java @@ -1,12 +1,12 @@ package org.tests.query; -import io.ebean.DB; -import io.ebean.FutureRowCount; -import io.ebean.Transaction; +import io.ebean.*; import io.ebean.xtest.BaseTestCase; import org.junit.jupiter.api.Test; import org.tests.model.basic.EBasic; +import java.util.List; + import static org.assertj.core.api.Assertions.assertThat; class TestFindFutureRowCount extends BaseTestCase { @@ -46,4 +46,66 @@ class TestFindFutureRowCount extends BaseTestCase { assertThat(futureCountUsingTxn.get()).isEqualTo(1); } } + + @Test + void findFutureIds_when_inTransaction() throws Exception { + try (Transaction transaction = DB.beginTransaction()) { + EBasic basic = new EBasic("findFutureIds_when_inTransaction"); + DB.save(basic); + + List ids = DB.find(EBasic.class) + .where().eq("name", "findFutureIds_when_inTransaction") + .findIds(); + + Object expectedIdValue = ids.get(0); + + FutureIds futureIds = DB.find(EBasic.class) + .where().eq("name", "findFutureIds_when_inTransaction") + .findFutureIds(); + + List fids = futureIds.get(); + assertThat(fids).hasSize(1); + assertThat(fids.get(0)).isEqualTo(expectedIdValue); + + FutureIds futureIdsUsingTxn = DB.find(EBasic.class) + .usingTransaction(transaction) + .where().eq("name", "findFutureIds_when_inTransaction") + .findFutureIds(); + + List fids2 = futureIdsUsingTxn.get(); + assertThat(fids2).hasSize(1); + assertThat(fids2.get(0)).isEqualTo(expectedIdValue); + } + } + + @Test + void findFutureList_when_inTransaction() throws Exception { + try (Transaction transaction = DB.beginTransaction()) { + EBasic basic = new EBasic("findFutureList_when_inTransaction"); + DB.save(basic); + + List list = DB.find(EBasic.class) + .where().eq("name", "findFutureList_when_inTransaction") + .findList(); + + Object expectedIdValue = list.get(0).getId(); + + FutureList futureIds = DB.find(EBasic.class) + .where().eq("name", "findFutureList_when_inTransaction") + .findFutureList(); + + List fids = futureIds.get(); + assertThat(fids).hasSize(1); + assertThat(fids.get(0).getId()).isEqualTo(expectedIdValue); + + FutureList futureUsingTxn = DB.find(EBasic.class) + .usingTransaction(transaction) + .where().eq("name", "findFutureList_when_inTransaction") + .findFutureList(); + + List fids2 = futureUsingTxn.get(); + assertThat(fids2).hasSize(1); + assertThat(fids2.get(0).getId()).isEqualTo(expectedIdValue); + } + } }