JdbcAccess: Defer exceptions when in a transaction
The insert/update/delete methods may throw OrmExceptions immediately
when a primary key constraint is violated or when attempting to modify
a row that does not exist. This violates the expectations of callers
who assume that no work is actually done until the transaction is
committed. Work around this in the simplest way possible, by holding
on to the first exception encountered during a transaction and
throwing it immediately from commit().
This enables a pattern where callers can "turn off" database access
entirely by always using transactions and simply never committing
them:
db.table().beginTransaction(key);
try {
db.table().insert(foo);
if (dbIsEnabled) {
db.commit();
}
} finally {
db.rollback();
}
db.table().beginTransaction(key);
try {
db.table().delete(foo.getKey());
if (dbIsEnabled) {
db.commit();
}
} finally {
db.rollback();
}
The problem with this pattern, prior to this change, is that if the
first transaction never inserted foo, then the delete call will
immediately throw OrmConcurrencyException due to foo's key not
existing.
Change-Id: Ic7ced328a4d3659cb8b7b00cb387142dbf8a0522
diff --git a/src/main/java/com/google/gwtorm/jdbc/JdbcAccess.java b/src/main/java/com/google/gwtorm/jdbc/JdbcAccess.java
index a97cfef..7335427 100644
--- a/src/main/java/com/google/gwtorm/jdbc/JdbcAccess.java
+++ b/src/main/java/com/google/gwtorm/jdbc/JdbcAccess.java
@@ -168,7 +168,9 @@
insertIndividually(instances);
}
} catch (SQLException e) {
- throw convertError("insert", e);
+ throwOrDefer(convertError("insert", e));
+ } catch (OrmException e) {
+ throwOrDefer(e);
}
}
@@ -227,7 +229,9 @@
updateIndividually(instances);
}
} catch (SQLException e) {
- throw convertError("update", e);
+ throwOrDefer(convertError("update", e));
+ } catch (OrmException e) {
+ throwOrDefer(e);
}
}
@@ -326,11 +330,15 @@
@Override
public void upsert(final Iterable<T> instances) throws OrmException {
- // Assume update first, it will cheaply tell us if the row is missing.
- Collection<T> inserts = attemptUpdate(instances);
+ try {
+ // Assume update first, it will cheaply tell us if the row is missing.
+ Collection<T> inserts = attemptUpdate(instances);
- if (inserts != null) {
- insert(inserts);
+ if (inserts != null) {
+ insert(inserts);
+ }
+ } catch (OrmException e) {
+ throwOrDefer(e);
}
}
@@ -391,7 +399,9 @@
deleteIndividually(instances);
}
} catch (SQLException e) {
- throw convertError("delete", e);
+ throwOrDefer(convertError("delete", e));
+ } catch (OrmException e) {
+ throwOrDefer(e);
}
}
@@ -453,6 +463,19 @@
}
}
+ private void throwOrDefer(OrmException e) throws OrmException {
+ try {
+ if (!schema.isInTransaction()) {
+ throw e;
+ }
+ } catch (SQLException e2) {
+ // Couldn't determine if we were in a transaction. Assume we are not, and
+ // just rethrow the original exception.
+ throw e; // Not e2.
+ }
+ schema.setTransactionException(e);
+ }
+
protected OrmException convertError(final String op, SQLException err) {
if (err.getCause() == null && err.getNextException() != null) {
// special case for IBM DB2. Exception cause is null:
diff --git a/src/main/java/com/google/gwtorm/jdbc/JdbcSchema.java b/src/main/java/com/google/gwtorm/jdbc/JdbcSchema.java
index 80dec0b..1173140 100644
--- a/src/main/java/com/google/gwtorm/jdbc/JdbcSchema.java
+++ b/src/main/java/com/google/gwtorm/jdbc/JdbcSchema.java
@@ -21,6 +21,8 @@
import com.google.gwtorm.schema.SequenceModel;
import com.google.gwtorm.schema.sql.SqlDialect;
import com.google.gwtorm.server.AbstractSchema;
+import com.google.gwtorm.server.OrmConcurrencyException;
+import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.Schema;
import com.google.gwtorm.server.StatementExecutor;
@@ -34,6 +36,7 @@
public abstract class JdbcSchema extends AbstractSchema {
private final Database<?> dbDef;
private Connection conn;
+ private OrmException transactionException;
protected JdbcSchema(final Database<?> d) throws OrmException {
dbDef = d;
@@ -51,7 +54,18 @@
@Override
public void commit() throws OrmException {
try {
- if (!conn.getAutoCommit()) {
+ if (isInTransaction()) {
+ if (transactionException != null) {
+ OrmException e = transactionException;
+ transactionException = null;
+ if (e instanceof OrmConcurrencyException) {
+ throw new OrmConcurrencyException(e.getMessage(), e);
+ } else if (e instanceof OrmDuplicateKeyException) {
+ throw new OrmDuplicateKeyException(e.getMessage(), e);
+ } else {
+ throw new OrmException(e.getMessage(), e);
+ }
+ }
conn.commit();
}
} catch (SQLException err) {
@@ -69,6 +83,7 @@
public void rollback() throws OrmException {
try {
if (!conn.getAutoCommit()) {
+ transactionException = null;
conn.rollback();
}
} catch (SQLException err) {
@@ -242,6 +257,7 @@
@Override
public void close() {
+ transactionException = null;
if (conn != null) {
try {
conn.close();
@@ -251,4 +267,15 @@
conn = null;
}
}
+
+ boolean isInTransaction() throws SQLException {
+ return !conn.getAutoCommit();
+ }
+
+ void setTransactionException(OrmException ex) {
+ // commit() needs a single cause, so just take the first.
+ if (transactionException == null) {
+ transactionException = Preconditions.checkNotNull(ex);
+ }
+ }
}
diff --git a/src/test/java/com/google/gwtorm/data/Person.java b/src/test/java/com/google/gwtorm/data/Person.java
index 38644da..13ff465 100644
--- a/src/test/java/com/google/gwtorm/data/Person.java
+++ b/src/test/java/com/google/gwtorm/data/Person.java
@@ -59,6 +59,10 @@
this.age = age;
}
+ public Key key() {
+ return name;
+ }
+
public String name() {
return name.get();
}
diff --git a/src/test/java/com/google/gwtorm/jdbc/AbstractTestJdbcAccess.java b/src/test/java/com/google/gwtorm/jdbc/AbstractTestJdbcAccess.java
index 2c08ef2..267bfcc 100644
--- a/src/test/java/com/google/gwtorm/jdbc/AbstractTestJdbcAccess.java
+++ b/src/test/java/com/google/gwtorm/jdbc/AbstractTestJdbcAccess.java
@@ -112,6 +112,7 @@
oneRow = dataProvider.createIterable(new Data(1));
twoRows = dataProvider.createIterable(new Data(1), new Data(2));
conn = mock(Connection.class);
+ when(conn.getAutoCommit()).thenReturn(true);
dialect = createDialect();
classUnderTest = createJdbcAccess(dialect, conn);
}
diff --git a/src/test/java/com/google/gwtorm/schema/sql/SqlDialectTest.java b/src/test/java/com/google/gwtorm/schema/sql/SqlDialectTest.java
index b191769..422ba0a 100644
--- a/src/test/java/com/google/gwtorm/schema/sql/SqlDialectTest.java
+++ b/src/test/java/com/google/gwtorm/schema/sql/SqlDialectTest.java
@@ -25,6 +25,7 @@
import com.google.gwtorm.data.PhoneBookDb2;
import com.google.gwtorm.jdbc.Database;
import com.google.gwtorm.jdbc.JdbcExecutor;
+import com.google.gwtorm.server.OrmConcurrencyException;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException;
@@ -33,8 +34,8 @@
import java.sql.Connection;
import java.sql.SQLException;
import java.util.ArrayList;
-import java.util.List;
import java.util.Collections;
+import java.util.List;
public abstract class SqlDialectTest {
protected JdbcExecutor executor;
@@ -124,6 +125,149 @@
}
}
+ @Test
+ public void testInsertExistingRowThrowsOrmDuplicateKeyException()
+ throws Exception {
+ PhoneBookDb p = phoneBook.open();
+ try {
+ p.updateSchema(executor);
+
+ Person bob = new Person(new Person.Key("Bob"), 18);
+ p.people().insert(Collections.singleton(bob));
+
+ try {
+ p.people().insert(Collections.singleton(bob));
+ fail("expected OrmDuplicateKeyException");
+ } catch (OrmDuplicateKeyException e) {
+ assertEquals(p.people().getRelationName(), e.getMessage());
+ }
+ } finally {
+ p.close();
+ }
+ }
+
+ @Test
+ public void testInsertExistingRowInTransactionThrowsOrmDuplicateKeyException()
+ throws Exception {
+ PhoneBookDb p = phoneBook.open();
+ try {
+ p.updateSchema(executor);
+
+ Person bob = new Person(new Person.Key("Bob"), 18);
+ p.people().insert(Collections.singleton(bob));
+
+ p.people().beginTransaction(bob.key());
+ try {
+ p.people().insert(Collections.singleton(bob));
+
+ try {
+ p.commit();
+ fail("expected OrmDuplicateKeyException");
+ } catch (OrmDuplicateKeyException e) {
+ assertEquals(p.people().getRelationName(), e.getMessage());
+ }
+ } finally {
+ p.rollback();
+ }
+ } finally {
+ p.close();
+ }
+ }
+
+ @Test
+ public void testUpdateNonexistentRowThrowsOrmConcurrencyException()
+ throws Exception {
+ PhoneBookDb p = phoneBook.open();
+ try {
+ p.updateSchema(executor);
+
+ Person bob = new Person(new Person.Key("Bob"), 18);
+
+ try {
+ p.people().update(Collections.singleton(bob));
+ fail("expected OrmConcurrencyException");
+ } catch (OrmConcurrencyException e) {
+ // Expected.
+ }
+ } finally {
+ p.close();
+ }
+ }
+
+ @Test
+ public void testUpdateNonexistentRowInTransactionThrowsOrmConcurrencyException()
+ throws Exception {
+ PhoneBookDb p = phoneBook.open();
+ try {
+ p.updateSchema(executor);
+
+ Person bob = new Person(new Person.Key("Bob"), 18);
+
+ p.people().beginTransaction(bob.key());
+ try {
+ p.people().update(Collections.singleton(bob));
+
+ try {
+ p.commit();
+ fail("expected OrmConcurrencyException");
+ } catch (OrmConcurrencyException e) {
+ // Expected.
+ }
+ } finally {
+ p.rollback();
+ }
+ } finally {
+ p.close();
+ }
+ }
+
+ @Test
+ public void testDeleteNonexistentRowThrowsOrmConcurrencyException()
+ throws Exception {
+ PhoneBookDb p = phoneBook.open();
+ try {
+ p.updateSchema(executor);
+
+ Person bob = new Person(new Person.Key("Bob"), 18);
+
+ try {
+ p.people().delete(Collections.singleton(bob));
+ fail("expected OrmConcurrencyException");
+ } catch (OrmConcurrencyException e) {
+ // Expected.
+ }
+ } finally {
+ p.close();
+ }
+ }
+
+ @Test
+ public void testDeleteNonexistentRowInTransactionThrowsOrmConcurrencyException()
+ throws Exception {
+ PhoneBookDb p = phoneBook.open();
+ try {
+ p.updateSchema(executor);
+
+ Person bob = new Person(new Person.Key("Bob"), 18);
+
+ p.people().beginTransaction(bob.key());
+ try {
+ p.people().delete(Collections.singleton(bob));
+
+ try {
+ p.commit();
+ fail("expected OrmConcurrencyException");
+ } catch (OrmConcurrencyException e) {
+ // Expected.
+ }
+ } finally {
+ p.rollback();
+ }
+ } finally {
+ p.close();
+ }
+ }
+
private void assertContainsString(String string, String substring) {
assertNotNull(string);
assertTrue(string.contains(substring));
