Skip to content

Commit f7e59c5

Browse files
dmealingclaude
andcommitted
fix(omdb): bulk-create fallback defers commit/rollback to a caller-managed transaction
createObjectsBatchFallback unconditionally committed on success and rolled back on failure. When the connection arrived already non-autocommit — reachable when an inTransaction(...) lambda (which sets autoCommit=false) calls createObjectsBulk and the driver has no native bulk support — this prematurely committed the OUTER transaction on success and double-rolled-back on failure. Gate the commit and rollback on originalAutoCommit (the flag already captured): manage the transaction boundary only when THIS method opened it. When the caller owns the transaction, issue no commit and no rollback here and let exceptions propagate, mirroring single createObject. The autoCommit restore in finally was already gated. Tests (embedded Derby): a bulk insert inside an inTransaction lambda whose later statement fails must NOT survive the outer rollback; and a successful fallback under a caller-managed (autoCommit=false) connection must not commit on its own (the owner's rollback removes the rows). Both fail before the gating fix (rows survive) and pass after. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 57ec64f commit f7e59c5

3 files changed

Lines changed: 126 additions & 8 deletions

File tree

server/java/omdb/src/main/java/com/metaobjects/manager/db/ObjectManagerDB.java

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1251,8 +1251,16 @@ public void updateObjectsBulk(ObjectConnection c, MetaObject mc, Collection<Obje
12511251
*
12521252
* <p>The per-1000-row mid-commit of the prior implementation was removed:
12531253
* intermediate commits made earlier chunks unrecoverable on a later
1254-
* failure, defeating atomicity. Auto-commit is disabled for the batch and
1255-
* restored in {@code finally}.
1254+
* failure, defeating atomicity.
1255+
*
1256+
* <p><strong>Transaction ownership.</strong> Commit / rollback / auto-commit
1257+
* restore are performed ONLY when this method opened the transaction (the
1258+
* connection arrived with auto-commit on). When the caller already owns the
1259+
* transaction — e.g. inside an {@code ObjectManager.inTransaction(...)} lambda
1260+
* which sets auto-commit off — this method commits nothing, rolls back nothing,
1261+
* and lets exceptions propagate so the caller's transaction boundary stays
1262+
* authoritative (no premature commit, no double rollback). This mirrors single
1263+
* {@link #createObject}, which never manages a commit of its own.
12561264
*/
12571265
private void createObjectsBatchFallback(ObjectConnection c, MetaObject mc, ObjectMappingDB mapping, Collection<Object> objects) throws SQLException {
12581266
Connection conn = (Connection) c.getDatastoreConnection();
@@ -1273,13 +1281,24 @@ private void createObjectsBatchFallback(ObjectConnection c, MetaObject mc, Objec
12731281
postPersistence(c, mc, obj, CREATE);
12741282
}
12751283

1276-
conn.commit();
1284+
// Only commit when THIS method opened the transaction. If the caller
1285+
// arrived already non-autocommit (e.g. inside an inTransaction lambda),
1286+
// the boundary is theirs to manage — committing here would prematurely
1287+
// commit the outer unit of work.
1288+
if (originalAutoCommit) {
1289+
conn.commit();
1290+
}
12771291
} catch (SQLException | RuntimeException e) {
1278-
// All-or-nothing: undo every row inserted in this batch.
1279-
try {
1280-
conn.rollback();
1281-
} catch (SQLException rollbackEx) {
1282-
e.addSuppressed(rollbackEx);
1292+
// All-or-nothing: undo every row inserted in this batch — but only when
1293+
// we own the transaction. When the caller owns it, let the exception
1294+
// propagate untouched (the caller rolls back); rolling back here would
1295+
// double-rollback the outer transaction.
1296+
if (originalAutoCommit) {
1297+
try {
1298+
conn.rollback();
1299+
} catch (SQLException rollbackEx) {
1300+
e.addSuppressed(rollbackEx);
1301+
}
12831302
}
12841303
throw e;
12851304
} finally {

server/java/omdb/src/test/java/com/metaobjects/manager/db/BulkCreateFallbackTest.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ private ValueObject sample(MetaObject mo, String label) {
105105
vo.setLong("bignum", 1L);
106106
vo.setBoolean("active", true);
107107
vo.setDouble("ratio", 1.0d);
108+
vo.setFloat("rate", 1.0f);
109+
vo.setObject("amount", new java.math.BigDecimal("1.00"));
108110
vo.setString("label", label);
109111
vo.setDate("createdAt", new java.util.Date(1_700_000_000_000L));
110112
return vo;
@@ -146,6 +148,91 @@ public void happyPathBulkCreatePassesLiveConnectionToHooks() throws Exception {
146148
omdb.postConns.forEach(c -> assertNotNull("postPersistence received a live (non-null) ObjectConnection", c));
147149
}
148150

151+
/**
152+
* Task 1.6 (remediation) — when the bulk fallback runs inside a
153+
* caller-managed transaction (an {@code inTransaction} lambda), it must NOT
154+
* commit the outer unit of work. A later failure in the same lambda must
155+
* roll back EVERYTHING, including the bulk-inserted rows. If the fallback
156+
* prematurely committed, those rows would survive the rollback.
157+
*/
158+
@Test
159+
public void bulkInsideInTransactionDefersToCallerOnLaterFailure() throws Exception {
160+
MetaObject mo = registry.findMetaObjectByName("codectest::Sample");
161+
162+
ObjectConnection before = omdb.getConnection();
163+
long start;
164+
try {
165+
start = omdb.getObjects(before, mo,
166+
new com.metaobjects.manager.QueryOptions(
167+
new com.metaobjects.manager.exp.Expression("label", "txn-bulk-", com.metaobjects.manager.exp.Expression.START_WITH))).size();
168+
} finally {
169+
omdb.releaseConnection(before);
170+
}
171+
172+
try {
173+
omdb.inTransaction(c -> {
174+
Collection<Object> batch = new ArrayList<>();
175+
for (int i = 0; i < 4; i++) batch.add(sample(mo, "txn-bulk-" + i));
176+
omdb.createObjectsBulk(c, mo, batch); // hits the fallback (Derby has no native bulk)
177+
// Now fail LATER in the same lambda. If the bulk path committed,
178+
// these rows survive; if it correctly deferred, the inTransaction
179+
// rollback removes them.
180+
throw new IllegalStateException("later work fails");
181+
});
182+
fail("expected the lambda's later failure to propagate");
183+
} catch (IllegalStateException expected) {
184+
// good — inTransaction rolled the whole unit back
185+
}
186+
187+
ObjectConnection verify = omdb.getConnection();
188+
try {
189+
long after = omdb.getObjects(verify, mo,
190+
new com.metaobjects.manager.QueryOptions(
191+
new com.metaobjects.manager.exp.Expression("label", "txn-bulk-", com.metaobjects.manager.exp.Expression.START_WITH))).size();
192+
assertEquals("bulk insert inside inTransaction must NOT survive the outer rollback "
193+
+ "(the fallback must not prematurely commit a caller-managed transaction)",
194+
start, after);
195+
} finally {
196+
omdb.releaseConnection(verify);
197+
}
198+
}
199+
200+
/**
201+
* Task 1.6 (remediation), connection-level corroboration — when the
202+
* connection arrives already non-autocommit (caller owns the transaction),
203+
* a SUCCESSFUL fallback must NOT commit on its own. We prove "no premature
204+
* commit" from the OWNER connection: roll back after a successful fallback;
205+
* if the fallback had committed, the rows would survive the rollback. They
206+
* must not. (We deliberately avoid a second connection here: embedded Derby
207+
* takes a write lock on the uncommitted rows, so a concurrent read on the
208+
* same in-memory DB would block on lock timeout rather than see "0 rows".)
209+
*/
210+
@Test
211+
public void successfulFallbackDoesNotCommitUnderCallerManagedTransaction() throws Exception {
212+
MetaObject mo = registry.findMetaObjectByName("codectest::Sample");
213+
214+
ObjectConnection oc = omdb.getConnection();
215+
try {
216+
oc.setAutoCommit(false); // caller takes ownership of the transaction
217+
Collection<Object> batch = new ArrayList<>();
218+
for (int i = 0; i < 3; i++) batch.add(sample(mo, "nocommit-" + i));
219+
omdb.createObjectsBulk(oc, mo, batch); // fallback runs successfully but must NOT commit
220+
221+
// Owner rolls back. If the fallback prematurely committed, these rows
222+
// would persist through the rollback; with the fix they vanish.
223+
oc.rollback();
224+
225+
long visible = omdb.getObjects(oc, mo,
226+
new com.metaobjects.manager.QueryOptions(
227+
new com.metaobjects.manager.exp.Expression("label", "nocommit-", com.metaobjects.manager.exp.Expression.START_WITH))).size();
228+
assertEquals("a successful fallback must NOT commit when the caller owns the transaction; "
229+
+ "the owner's rollback must remove every fallback-inserted row",
230+
0, visible);
231+
} finally {
232+
omdb.releaseConnection(oc);
233+
}
234+
}
235+
149236
@Test
150237
public void midBatchFailureRollsBackEntireBatch() throws Exception {
151238
MetaObject mo = registry.findMetaObjectByName("codectest::Sample");

server/java/omdb/src/test/resources/meta.codec.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,18 @@
5050
"@maxLength": 100
5151
}
5252
},
53+
{
54+
"field.float": {
55+
"name": "rate",
56+
"@column": "rate"
57+
}
58+
},
59+
{
60+
"field.decimal": {
61+
"name": "amount",
62+
"@column": "amount"
63+
}
64+
},
5365
{
5466
"field.date": {
5567
"name": "createdAt",

0 commit comments

Comments
 (0)