From ec234f1f395a4b25a5f0a9aa1c283daa76586331 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Mon, 16 May 2016 15:50:11 +1200 Subject: [PATCH] #568 - ENH: Add ability to turn of getGeneratedKeys with @Transactional annotation (Also needs agent update) --- src/main/java/com/avaje/ebean/TxScope.java | 17 + .../avaje/ebean/annotation/Transactional.java | 10 + .../avaje/ebeaninternal/api/ScopeTrans.java | 340 +++++++++--------- .../ebeaninternal/api/ScopedTransaction.java | 5 + .../ebeaninternal/api/SpiTransaction.java | 11 +- .../server/transaction/JdbcTransaction.java | 5 + .../java/com/avaje/ebean/BaseTestCase.java | 2 +- .../batchinsert/TestBatchInsertSimple.java | 32 ++ 8 files changed, 253 insertions(+), 169 deletions(-) diff --git a/src/main/java/com/avaje/ebean/TxScope.java b/src/main/java/com/avaje/ebean/TxScope.java index 1b1ee4268..af1b56881 100644 --- a/src/main/java/com/avaje/ebean/TxScope.java +++ b/src/main/java/com/avaje/ebean/TxScope.java @@ -36,6 +36,8 @@ public final class TxScope { int batchSize; + boolean skipGeneratedKeys; + boolean readOnly; ArrayList> rollbackFor; @@ -188,6 +190,21 @@ public final class TxScope { return this; } + /** + * Set if the transaction should skip reading generated keys for inserts. + */ + public TxScope setSkipGeneratedKeys() { + this.skipGeneratedKeys = true; + return this; + } + + /** + * Return true if getGeneratedKeys should be skipped for this transaction. + */ + public boolean isSkipGeneratedKeys() { + return skipGeneratedKeys; + } + /** * Return if the transaction should be treated as read only. */ diff --git a/src/main/java/com/avaje/ebean/annotation/Transactional.java b/src/main/java/com/avaje/ebean/annotation/Transactional.java index 336313dbe..5de7f8b6c 100644 --- a/src/main/java/com/avaje/ebean/annotation/Transactional.java +++ b/src/main/java/com/avaje/ebean/annotation/Transactional.java @@ -90,6 +90,16 @@ public @interface Transactional { */ int batchSize() default 0; + /** + * Set to false when we want to skip getting generatedKeys. + *

+ * This is typically used in the case of large batch inserts where we get a + * performance benefit from not calling getGeneratedKeys (as we are going to + * insert a lot of rows and have no need for the Id values after the insert). + *

+ */ + boolean getGeneratedKeys() default true; + /** * The transaction isolation level this transaction should have. *

diff --git a/src/main/java/com/avaje/ebeaninternal/api/ScopeTrans.java b/src/main/java/com/avaje/ebeaninternal/api/ScopeTrans.java index 46ee68aa5..ecdda3085 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/ScopeTrans.java +++ b/src/main/java/com/avaje/ebeaninternal/api/ScopeTrans.java @@ -1,49 +1,49 @@ package com.avaje.ebeaninternal.api; -import java.util.ArrayList; - import com.avaje.ebean.TxScope; import com.avaje.ebean.config.PersistBatch; +import java.util.ArrayList; + /** * Used internally to handle the scoping of transactions for methods. */ public class ScopeTrans implements Thread.UncaughtExceptionHandler { - private static final int OPCODE_ATHROW = 191; + private static final int OPCODE_ATHROW = 191; - private final SpiTransactionScopeManager scopeMgr; + private final SpiTransactionScopeManager scopeMgr; - /** - * The suspended transaction (can be null). - */ - private final SpiTransaction suspendedTransaction; + /** + * The suspended transaction (can be null). + */ + private final SpiTransaction suspendedTransaction; - /** - * The transaction in scope (can be null). - */ - private final SpiTransaction transaction; + /** + * The transaction in scope (can be null). + */ + private final SpiTransaction transaction; - /** - * If true by default rollback on Checked exceptions. - */ - private final boolean rollbackOnChecked; + /** + * If true by default rollback on Checked exceptions. + */ + private final boolean rollbackOnChecked; - /** - * True if the transaction was created and hence should be committed - * on finally if it hasn't already been rolled back. - */ - private final boolean created; + /** + * True if the transaction was created and hence should be committed + * on finally if it hasn't already been rolled back. + */ + private final boolean created; - /** - * Explicit set of Exceptions that DO NOT cause a rollback to occur. - */ - private final ArrayList> noRollbackFor; + /** + * Explicit set of Exceptions that DO NOT cause a rollback to occur. + */ + private final ArrayList> noRollbackFor; - /** - * Explicit set of Exceptions that DO cause a rollback to occur. - */ - private final ArrayList> rollbackFor; + /** + * Explicit set of Exceptions that DO cause a rollback to occur. + */ + private final ArrayList> rollbackFor; private PersistBatch restoreBatch; @@ -51,29 +51,32 @@ public class ScopeTrans implements Thread.UncaughtExceptionHandler { private int restoreBatchSize; - /** - * Flag set when a rollback has occurred. - */ - private boolean rolledBack; - - - public ScopeTrans(boolean rollbackOnChecked, boolean created, SpiTransaction transaction, TxScope txScope, - SpiTransaction suspendedTransaction, SpiTransactionScopeManager scopeMgr) { + private Boolean restoreBatchGeneratedKeys; - this.rollbackOnChecked = rollbackOnChecked; - this.created = created; - this.transaction = transaction; - this.suspendedTransaction = suspendedTransaction; - this.scopeMgr = scopeMgr; - - this.noRollbackFor = txScope.getNoRollbackFor(); - this.rollbackFor = txScope.getRollbackFor(); + /** + * Flag set when a rollback has occurred. + */ + private boolean rolledBack; + + + public ScopeTrans(boolean rollbackOnChecked, boolean created, SpiTransaction transaction, TxScope txScope, + SpiTransaction suspendedTransaction, SpiTransactionScopeManager scopeMgr) { + + this.rollbackOnChecked = rollbackOnChecked; + this.created = created; + this.transaction = transaction; + this.suspendedTransaction = suspendedTransaction; + this.scopeMgr = scopeMgr; + + this.noRollbackFor = txScope.getNoRollbackFor(); + this.rollbackFor = txScope.getRollbackFor(); if (transaction != null) { if (!created && txScope.isBatchSet() || txScope.isBatchOnCascadeSet() || txScope.isBatchSizeSet()) { restoreBatch = transaction.getBatch(); restoreBatchOnCascade = transaction.getBatchOnCascade(); restoreBatchSize = transaction.getBatchSize(); + restoreBatchGeneratedKeys = transaction.getBatchGetGeneratedKeys(); } if (txScope.isBatchSet()) { transaction.setBatch(txScope.getBatch()); @@ -84,70 +87,74 @@ public class ScopeTrans implements Thread.UncaughtExceptionHandler { if (txScope.isBatchSizeSet()) { transaction.setBatchSize(txScope.getBatchSize()); } + if (txScope.isSkipGeneratedKeys()) { + transaction.setBatchGetGeneratedKeys(false); + } } - } + } - /** - * Return the current/active transaction. - */ - protected SpiTransaction getTransaction() { - return transaction; - } + /** + * Return the current/active transaction. + */ + protected SpiTransaction getTransaction() { + return transaction; + } - /** - * Called when the Thread catches any uncaught exception. - * For example, an unexpected NullPointerException or Error. - */ - public void uncaughtException(Thread thread, Throwable e) { - - // rollback transaction if required - caughtThrowable(e); - - // reinstate suspended transaction - onFinally(); - } - - /** - * Returned via RETURN or expected Exception from the method. - * @param returnOrThrowable the return value or Throwable - * @param opCode indicates - */ - public void onExit(Object returnOrThrowable, int opCode) { - - if (opCode == OPCODE_ATHROW){ - // exited with a Throwable - caughtThrowable((Throwable)returnOrThrowable); - } - onFinally(); - } - - - /** - * Commit if the transaction exists and has not already been rolled back. - * Also reinstate the suspended transaction if there was one. - */ - public void onFinally() { + /** + * Called when the Thread catches any uncaught exception. + * For example, an unexpected NullPointerException or Error. + */ + public void uncaughtException(Thread thread, Throwable e) { - try { - if (!rolledBack) { - commitTransaction(); - } - } finally { - restoreSuspended(); - } - } + // rollback transaction if required + caughtThrowable(e); - protected void restoreSuspended() { - if (suspendedTransaction != null){ + // reinstate suspended transaction + onFinally(); + } + + /** + * Returned via RETURN or expected Exception from the method. + * + * @param returnOrThrowable the return value or Throwable + * @param opCode indicates + */ + public void onExit(Object returnOrThrowable, int opCode) { + + if (opCode == OPCODE_ATHROW) { + // exited with a Throwable + caughtThrowable((Throwable) returnOrThrowable); + } + onFinally(); + } + + + /** + * Commit if the transaction exists and has not already been rolled back. + * Also reinstate the suspended transaction if there was one. + */ + public void onFinally() { + + try { + if (!rolledBack) { + commitTransaction(); + } + } finally { + restoreSuspended(); + } + } + + protected void restoreSuspended() { + if (suspendedTransaction != null) { // put the previously suspended transaction // back onto the ThreadLocal or equivalent scopeMgr.replace(suspendedTransaction); } - } + } - protected void commitTransaction() { - if (created) { + protected void commitTransaction() { + if (created) { transaction.commit(); } else { if (restoreBatch != null) { @@ -159,78 +166,81 @@ public class ScopeTrans implements Thread.UncaughtExceptionHandler { if (restoreBatchSize > 0) { transaction.setBatchSize(restoreBatchSize); } + if (restoreBatchGeneratedKeys != null) { + transaction.setBatchGetGeneratedKeys(restoreBatchGeneratedKeys); + } } - } + } - /** - * An Error was caught and this ALWAYS causes a rollback to occur. - * Returns the error and this should be thrown by the calling code. - */ - public Error caughtError(Error e) { - rollback(e); - return e; - } - - /** - * An Exception was caught and may or may not cause a rollback to occur. - * Returns the exception and this should be thrown by the calling code. - */ - public T caughtThrowable(T e) { - - if (isRollbackThrowable(e)) { - rollback(e); - } - return e; - } + /** + * An Error was caught and this ALWAYS causes a rollback to occur. + * Returns the error and this should be thrown by the calling code. + */ + public Error caughtError(Error e) { + rollback(e); + return e; + } - protected void rollback(Throwable e) { - if (transaction != null && transaction.isActive()) { - // transaction is null for NOT_SUPPORTED and sometimes SUPPORTS - // and Inactive (already rolled back) if nested REQUIRED - transaction.rollback(e); - } - rolledBack = true; - } + /** + * An Exception was caught and may or may not cause a rollback to occur. + * Returns the exception and this should be thrown by the calling code. + */ + public T caughtThrowable(T e) { - /** - * Return true if this throwable should cause a rollback to occur. - */ - private boolean isRollbackThrowable(Throwable e) { + if (isRollbackThrowable(e)) { + rollback(e); + } + return e; + } - if (e instanceof Error){ - return true; - } - - if (noRollbackFor != null){ - for (int i = 0; i < noRollbackFor.size(); i++) { - if (noRollbackFor.get(i).equals(e.getClass())) { - - // explicit no rollback for this one - return false; - } - } - } + protected void rollback(Throwable e) { + if (transaction != null && transaction.isActive()) { + // transaction is null for NOT_SUPPORTED and sometimes SUPPORTS + // and Inactive (already rolled back) if nested REQUIRED + transaction.rollback(e); + } + rolledBack = true; + } + + /** + * Return true if this throwable should cause a rollback to occur. + */ + private boolean isRollbackThrowable(Throwable e) { + + if (e instanceof Error) { + return true; + } + + if (noRollbackFor != null) { + for (int i = 0; i < noRollbackFor.size(); i++) { + if (noRollbackFor.get(i).equals(e.getClass())) { + + // explicit no rollback for this one + return false; + } + } + } + + if (rollbackFor != null) { + for (int i = 0; i < rollbackFor.size(); i++) { + if (rollbackFor.get(i).equals(e.getClass())) { + // explicit rollback for this one + return true; + } + } + } + + + if (e instanceof RuntimeException) { + return true; + + } else { + // checked exceptions... + // EJB defaults this to false which is not intuitive IMO + // Ebean makes this configurable (default to true) + return rollbackOnChecked; + } + } - if (rollbackFor != null){ - for (int i = 0; i < rollbackFor.size(); i++) { - if (rollbackFor.get(i).equals(e.getClass())) { - // explicit rollback for this one - return true; - } - } - } - - - if (e instanceof RuntimeException) { - return true; - - } else { - // checked exceptions... - // EJB defaults this to false which is not intuitive IMO - // Ebean makes this configurable (default to true) - return rollbackOnChecked; - } - } - } diff --git a/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java b/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java index 176227b7b..17116e2c0 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java @@ -230,6 +230,11 @@ public class ScopedTransaction implements SpiTransaction { transaction.setBatchGetGeneratedKeys(getGeneratedKeys); } + @Override + public Boolean getBatchGetGeneratedKeys() { + return transaction.getBatchGetGeneratedKeys(); + } + @Override public void setBatchFlushOnMixed(boolean batchFlushOnMixed) { transaction.setBatchFlushOnMixed(batchFlushOnMixed); diff --git a/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java b/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java index cb8ccc1d2..e51719579 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java @@ -1,8 +1,5 @@ package com.avaje.ebeaninternal.api; -import java.sql.Connection; -import java.util.List; - import com.avaje.ebean.Transaction; import com.avaje.ebean.annotation.DocStoreMode; import com.avaje.ebean.bean.PersistenceContext; @@ -12,6 +9,9 @@ import com.avaje.ebeaninternal.server.core.PersistRequest; import com.avaje.ebeaninternal.server.core.PersistRequestBean; import com.avaje.ebeaninternal.server.persist.BatchControl; +import java.sql.Connection; +import java.util.List; + /** * Extends Transaction with additional API required on server. *

@@ -125,6 +125,11 @@ public interface SpiTransaction extends Transaction { */ int getBatchSize(); + /** + * Return the getGeneratedKeys setting for this transaction. + */ + Boolean getBatchGetGeneratedKeys(); + /** * Modify and return the current 'depth' of the transaction. *

diff --git a/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java b/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java index 85fbbbbe0..d49c86bbe 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java @@ -500,6 +500,11 @@ public class JdbcTransaction implements SpiTransaction { return batchOnCascadeMode; } + @Override + public Boolean getBatchGetGeneratedKeys() { + return batchGetGeneratedKeys; + } + @Override public void setBatchGetGeneratedKeys(boolean getGeneratedKeys) { this.batchGetGeneratedKeys = getGeneratedKeys; diff --git a/src/test/java/com/avaje/ebean/BaseTestCase.java b/src/test/java/com/avaje/ebean/BaseTestCase.java index 077591532..b1d6026c7 100644 --- a/src/test/java/com/avaje/ebean/BaseTestCase.java +++ b/src/test/java/com/avaje/ebean/BaseTestCase.java @@ -15,7 +15,7 @@ public class BaseTestCase { logger.debug("... preStart"); if (!AgentLoader.loadAgentFromClasspath("avaje-ebeanorm-agent","debug=0;packages=com.avaje.tests.**,org.avaje.test.**")) { logger.info("avaje-ebeanorm-agent not found in classpath - not dynamically loaded"); - } + } } /** diff --git a/src/test/java/com/avaje/tests/batchinsert/TestBatchInsertSimple.java b/src/test/java/com/avaje/tests/batchinsert/TestBatchInsertSimple.java index 0e9774101..55157fb47 100644 --- a/src/test/java/com/avaje/tests/batchinsert/TestBatchInsertSimple.java +++ b/src/test/java/com/avaje/tests/batchinsert/TestBatchInsertSimple.java @@ -13,6 +13,9 @@ import java.util.ArrayList; import java.util.List; import java.util.Random; +//import static org.assertj.core.api.Assertions.assertThat; +//import static org.junit.Assert.assertNull; + public class TestBatchInsertSimple extends BaseTestCase { Random random = new Random(); @@ -63,6 +66,35 @@ public class TestBatchInsertSimple extends BaseTestCase { } } +// @Test +// public void testTransactional_skipGeneratedBeans() { +// +// if (isMsSqlServer()) return; +// +// List beans = saveWithFullBatchMode_skipGeneratedKeys(); +// for (UTMaster bean : beans) { +// assertNull(bean.getId()); +// } +// } +// +// @Transactional(batch=PersistBatch.ALL, batchSize=50, getGeneratedKeys = false) +// public List saveWithFullBatchMode_skipGeneratedKeys() { +// +// Transaction transaction = server().currentTransaction(); +// SpiTransaction spiTxn = (SpiTransaction)transaction; +// Boolean generatedKeys = spiTxn.getBatchGetGeneratedKeys(); +// +// assertThat(generatedKeys).isFalse(); +// +// List beans = new ArrayList(); +// for (int i = 0; i < 4; i++) { +// beans.add(createMaster(i)); +// } +// +// server().saveAll(beans); +// return beans; +// } + @Test public void testJdbcBatchPerRequestWithMasterOnly() {