Skip to content

Commit 09ae8d5

Browse files
dmealingclaude
andcommitted
fix(omdb): FR-017 TPH — enforce the discriminator on the single-object + bulk write/read paths
Follow-up to the TPH persistence lane (review finding). Scoping previously lived only on the set-based query methods (getObjects/getObjectsCount/deleteObjects) + createObject, so a consumer calling the by-PK single-object API on a subtype VO bypassed the discriminator and could read/ overwrite/delete a SIBLING subtype's row in the shared table — a divergence from the Python/TS runtimes, which scope update/delete too. The guarantee now lives in the runtime, not just the test adapter: - loadObject — AND the discriminator into the by-PK read (a subtype can't load a sibling's row). - updateObject / deleteObject — a scoped existence pre-check (requireInSubtypeScope) refuses a cross-subtype PK with ObjectNotFoundException before the driver's PK-only write (the driver path has no expression hook to AND the discriminator into its WHERE). - createObjectsBulk — inject the discriminator per row (parallel to createObject). - updateObjectsBulk — pre-check each row (covers both the native bulk path and the per-object fallback). Also: getObjects no longer mutates the caller's QueryOptions — it scopes into a derived copy (scopedOptions), so scoping a read has no visible side effect on the argument. Helpers injectDiscriminator / requireInSubtypeScope / scopedOptions are no-ops for a non-TPH class. No-ops on the non-TPH path. QueryScenarioTests 23/0/0; omdb module 46/0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 05bd26c commit 09ae8d5

1 file changed

Lines changed: 77 additions & 14 deletions

File tree

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

Lines changed: 77 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,56 @@ private Expression scopeToSubtype(MetaObject mc, Expression exp) {
428428
return exp == null ? disc : exp.and(disc);
429429
}
430430

431+
/**
432+
* FR-017 TPH: return a DERIVED {@link QueryOptions} with the discriminator AND'd into the
433+
* expression when {@code mc} is a subtype; otherwise the original {@code options} unchanged
434+
* (no allocation). Never mutates the caller's options — scoping a read must not have a visible
435+
* side effect on the argument.
436+
*/
437+
private QueryOptions scopedOptions(MetaObject mc, QueryOptions options) {
438+
Expression scoped = scopeToSubtype(mc, options.getExpression());
439+
if (scoped == options.getExpression()) return options; // non-TPH (or no change) — caller's object
440+
QueryOptions copy = new QueryOptions(scoped, options.getSortOrder(), options.getRange());
441+
copy.setDistinct(options.isDistinct());
442+
copy.setFields(options.getFields());
443+
copy.setWithLock(options.withLock());
444+
return copy;
445+
}
446+
447+
/**
448+
* FR-017 TPH: inject a subtype's discriminator value onto {@code obj} before a write (the
449+
* entity names the subtype; the caller never sets the discriminator field). No-op for a
450+
* non-TPH class.
451+
*/
452+
private void injectDiscriminator(MetaObject mc, Object obj) {
453+
TphHelper.TphSubtype tph = TphHelper.tphSubtypeOf(mc);
454+
if (tph != null) {
455+
mc.getMetaField(tph.field()).setString(obj, tph.value());
456+
}
457+
}
458+
459+
/**
460+
* FR-017 TPH: enforce the subtype scope on a by-PK single-object write ({@code updateObject} /
461+
* {@code deleteObject}), which go through the driver's PK-based path (no expression hook to AND
462+
* the discriminator into the WHERE). A cross-subtype PK — a row that exists in the shared table
463+
* but belongs to a different subtype — is invisible to the subtype scope, so the write must be
464+
* refused rather than corrupt/delete a sibling subtype's row. Mirrors the Python/TS runtime,
465+
* which scope the update/delete WHERE by discriminator. No-op for a non-TPH class.
466+
*/
467+
private void requireInSubtypeScope(ObjectConnection c, MetaObject mc, Object obj) {
468+
if (TphHelper.tphSubtypeOf(mc) == null) return;
469+
Connection conn = (Connection) c.getDatastoreConnection();
470+
ObjectMappingDB mapping = (ObjectMappingDB) getReadMapping(mc);
471+
Expression exp = scopeToSubtype(mc, buildPrimaryKeyExpressionFromObject(mc, obj));
472+
try {
473+
if (getTypedDatabaseDriver().readMany(conn, mc, mapping, new QueryOptions(exp)).isEmpty()) {
474+
throw new ObjectNotFoundException(obj); // cross-subtype (or absent) — refuse the write
475+
}
476+
} catch (SQLException e) {
477+
throw new PersistenceException("Unable to verify subtype scope for [" + obj + "]: " + e.getMessage(), e);
478+
}
479+
}
480+
431481
/**
432482
* Delete the objects from the datastore where the field has the specified
433483
* value
@@ -511,10 +561,10 @@ public Collection<?> getObjects(ObjectConnection c, MetaObject mc, QueryOptions
511561
// Check for a valid transaction if enforced
512562
checkTransaction(conn, false);
513563

514-
// FR-017 TPH: a subtype read is discriminator-scoped (a row of a different subtype
515-
// is invisible); the base entity (no @discriminatorValue) reads the whole table.
516-
Expression scoped = scopeToSubtype(mc, options.getExpression());
517-
if (scoped != options.getExpression()) options.setExpression(scoped);
564+
// FR-017 TPH: a subtype read is discriminator-scoped (a row of a different subtype is
565+
// invisible); the base entity (no @discriminatorValue) reads the whole table. Scope into a
566+
// DERIVED options object — never mutate the caller's QueryOptions.
567+
options = scopedOptions(mc, options);
518568

519569
//int failures = 0;
520570
//while( true )
@@ -569,8 +619,9 @@ public void loadObject(ObjectConnection c, Object o) throws MetaDataException {
569619
log.debug("Loading object [" + o + "] of class [" + mc + "]");
570620
}
571621

572-
// Create the Expression for the Primary Keys
573-
Expression exp = buildPrimaryKeyExpressionFromObject(mc, o);
622+
// Create the Expression for the Primary Keys (FR-017 TPH: AND the discriminator so a
623+
// subtype load can't read a sibling subtype's row sharing the single table).
624+
Expression exp = scopeToSubtype(mc, buildPrimaryKeyExpressionFromObject(mc, o));
574625

575626
// Try to read the object
576627
try {
@@ -607,11 +658,8 @@ public void createObject(ObjectConnection c, Object obj) throws PersistenceExcep
607658
}
608659

609660
// FR-017 TPH: a subtype create injects its discriminator value (the entity names
610-
// the subtype; the caller never sets `type`) BEFORE the write so it persists.
611-
TphHelper.TphSubtype tph = TphHelper.tphSubtypeOf(mc);
612-
if (tph != null) {
613-
mc.getMetaField(tph.field()).setString(obj, tph.value());
614-
}
661+
// the subtype; the caller never sets it) BEFORE the write so it persists.
662+
injectDiscriminator(mc, obj);
615663

616664
// Get the create mapping
617665
ObjectMappingDB mapping = (ObjectMappingDB) getCreateMapping(mc);
@@ -651,6 +699,10 @@ public void updateObject(ObjectConnection c, Object obj) throws PersistenceExcep
651699
throw new PersistenceException("Object of class [" + mc + "] is not writeable");
652700
}
653701

702+
// FR-017 TPH: refuse a cross-subtype by-PK update (the driver writes by PK only, so without
703+
// this a subtype VO could overwrite a sibling subtype's row in the shared table).
704+
requireInSubtypeScope(c, mc, obj);
705+
654706
// check the object manager
655707
//verifyObjectManager( obj );
656708

@@ -735,6 +787,10 @@ public void deleteObject(ObjectConnection c, Object obj) throws PersistenceExcep
735787

736788
//verifyObjectManager( obj );
737789

790+
// FR-017 TPH: refuse a cross-subtype by-PK delete (the driver deletes by PK only, so without
791+
// this a subtype VO could delete a sibling subtype's row in the shared table).
792+
requireInSubtypeScope(c, mc, obj);
793+
738794
// Get the update mapping
739795
ObjectMappingDB mapping = (ObjectMappingDB) getDeleteMapping(mc);
740796

@@ -1024,9 +1080,12 @@ public void createObjectsBulk(ObjectConnection c, MetaObject mc, Collection<Obje
10241080

10251081
Connection conn = (Connection) c.getDatastoreConnection();
10261082
checkTransaction(conn, true);
1027-
1083+
10281084
ObjectMappingDB mapping = (ObjectMappingDB) getCreateMapping(mc);
1029-
1085+
1086+
// FR-017 TPH: inject the discriminator on every row before the bulk write (parallel to createObject).
1087+
for (Object obj : objects) injectDiscriminator(mc, obj);
1088+
10301089
try {
10311090
// Use database driver for bulk creation if supported
10321091
if (getDatabaseDriver() instanceof BulkOperationSupport bulkDriver) {
@@ -1053,7 +1112,11 @@ public void updateObjectsBulk(ObjectConnection c, MetaObject mc, Collection<Obje
10531112
checkTransaction(conn, true);
10541113

10551114
ObjectMappingDB mapping = (ObjectMappingDB) getUpdateMapping(mc);
1056-
1115+
1116+
// FR-017 TPH: refuse any cross-subtype row up front (covers both the native bulk path and
1117+
// the per-object fallback) so a bulk update can't corrupt a sibling subtype's row by PK.
1118+
for (Object obj : objects) requireInSubtypeScope(c, mc, obj);
1119+
10571120
try {
10581121
// Use database driver for bulk updates if supported
10591122
if (getDatabaseDriver() instanceof BulkOperationSupport bulkDriver) {

0 commit comments

Comments
 (0)