Skip to content

Commit 95c26e6

Browse files
dmealingclaude
andcommitted
Merge omdb-remediation Tier 2 (dead-code purge) + migration-removal plan
Purge: ~720 lines of dead/commented code removed across 6 OMDB runtime files (ExpressionParser, ManagedObject, QueryOptions, GenericSQLDriver, ObjectManagerDB, DirtyWriteException) — deletion-only, suites identical (om+omdb 68/0/0), no migration-subsystem files touched, reviewed safe. Plus the OMDB schema-migration subsystem removal plan (Option A, decisions resolved). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 parents 5f754ba + 7e54a17 commit 95c26e6

7 files changed

Lines changed: 110 additions & 724 deletions

File tree

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# OMDB Schema-Migration Subsystem Removal — Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax.
4+
5+
**Goal:** Remove OMDB's diff-and-converge **schema-migration engine** from the Java port. Schema migrations move to a **TS-only** approach ([[ts-only-schema-migrations-pivot]]); OMDB keeps its **runtime persistence** role only.
6+
7+
**Status:** PLAN (not yet executed). Carries one **load-bearing scope decision** (Option A vs B below) that must be confirmed before implementation.
8+
9+
---
10+
11+
## The scope decision (CONFIRM FIRST)
12+
13+
OMDB has TWO schema-touching capabilities. They are independent:
14+
15+
1. **Diff-and-converge MIGRATION engine**`migrate/*` (16 files), the `MigrationSqlRenderer` interface, the drivers' `render(Change)` methods, the `meta:migrate` Maven goal, and the Java migration-conformance scenarios. **This is what the TS-only pivot replaces.**
16+
2. **Runtime auto-create DDL**`MetaClassDBValidatorService` calling the drivers' *legacy* `createTable`/`createIndex`/`createForeignKey`. This bootstraps a schema at runtime/startup and is what the OMDB **test harness** (`AbstractOMDBTest`, ~10 tests) uses to create its tables. It does NOT use the `Change`/`render` engine.
17+
18+
- **Option A (RECOMMENDED — remove the engine only):** delete capability #1; **keep** capability #2 (validator + legacy `createTable` DDL). Rationale: low blast radius; the test harness keeps working; runtime auto-create is a legitimate dev/test convenience orthogonal to *migrations* (schema evolution), which is the thing going TS-only. The shared `defs/` model stays (it's also the runtime mapping model).
19+
- **Option B (purist — remove all schema/DDL generation):** also delete the validator + the legacy `createTable`/`createIndex`/`createForeignKey` DDL, making OMDB a pure data-access runtime. **Much larger:** ~10 omdb tests + the integration-tests harness lose their schema setup and must be re-tooled to create tables via raw SQL fixtures. Only do this if the intent is "OMDB never emits DDL at all."
20+
21+
**This plan implements Option A.** If B is chosen, add a Phase 0 that replaces `AbstractOMDBTest`'s validator-based setup with raw-SQL fixtures, then also delete the validator + legacy DDL methods.
22+
23+
---
24+
25+
## What gets removed (Option A)
26+
27+
**Delete (whole files):**
28+
- `omdb/.../manager/db/migrate/` — all 16: `AllowOptions`, `BlockedChangesError`, `Change`, `ChangeStatusRules`, `DiffResult`, `EmitResult`, `ExpectedSchemaBuilder`, `JdbcSqlTypes`, `MigrationEmitter`, `RenameHints`, `SchemaDiffer`, `SchemaIntrospector`, `SchemaMigrationEngine`, `SchemaSnapshot`, `SqlType`, `ViewBodyBuilder`.
29+
- `omdb/.../manager/db/MigrationSqlRenderer.java` (the interface drivers implement).
30+
- `maven-plugin/.../mojo/MetaDataMigrateMojo.java` (the `meta:migrate` goal).
31+
- omdb migration tests: `SchemaMigrationEngineTest`, `ExpectedSchemaBuilderTest`, `ViewMigrationTest` (+ any migrate-specific fixtures).
32+
- `integration-tests/.../MigrationScenarioRunner.java` + the migration-scenario invocations.
33+
34+
**Edit (remove migration bits, keep the rest):**
35+
- `GenericSQLDriver.java`, `PostgresDriver.java`, `DerbyDriver.java` — remove `implements MigrationSqlRenderer`, the `render(Change)` methods, and migration-only helpers (e.g. `pgType`/`derbyType` `SqlType` mappers used only by `render`). KEEP the legacy `createTable`/`createIndex`/`createForeignKey`/`createSequence` DDL (used by the validator) and ALL CRUD/query/codec/paging.
36+
- `DatabaseDriver.java` — drop any migration-only abstract methods/signatures if present.
37+
- `integration-tests` migration scenario wiring + any `meta:migrate` usage in test/build configs.
38+
- `omdb/.../driver/DriverFloatTypeTest.java` — remove its `migrate.*` references (keep the float-type assertions, re-pointed off `SqlType` if needed).
39+
40+
**KEEP (do not touch):**
41+
- `manager/db/defs/*` (TableDef/ColumnDef/IndexDef/etc.) — shared runtime mapping model.
42+
- `MetaClassDBValidatorService` + the legacy `createTable`/`createIndex`/`createForeignKey` driver DDL.
43+
- All CRUD/query/codec/transaction/paging/UUID code; the Spring Boot starter; `om`.
44+
45+
---
46+
47+
## Cross-cutting impact (handle as part of the plan)
48+
49+
- **Maven plugin:** `meta:gen`, `meta:verify`, `meta:editor` stay; `meta:migrate` goes. Update the plugin README/docs to drop `migrate`. Check the plugin POM/`@Mojo` registration and any shared base (`AbstractMetaDataMojo`) for migrate-only plumbing.
50+
- **Persistence-conformance:** Java currently runs "3 migration + 9 query" (per [[java-persistence-conformance-status]]). After removal Java runs **query only**; the corpus's migration scenarios remain for TS (and whichever ports still own migration). Update the `integration-tests` runner + the conformance memory/docs to reflect that Java no longer executes migration scenarios. **Decision:** confirm the corpus migration scenarios are NOT expected from Java anymore (vs. temporarily skipped).
51+
- **`spec/` + docs:** `meta migrate` references in CLAUDE.md ("Useful commands" mentions `meta migrate` — that's the TS CLI, leave), the Java maven-plugin docs, FR-003 docs, and `spec/roadmap.md` open-questions/migration notes. Update Java-migration claims; note migrations are TS-only. Reconcile [[fr003-migrate-cross-lang-divergences]].
52+
- **Memories to update after:** [[java-persistence-conformance-status]], [[ts-only-schema-migrations-pivot]], [[omdb-remediation-status]].
53+
54+
---
55+
56+
## Sequencing (Option A)
57+
58+
**Pre-flight verification (Task 0):**
59+
- [ ] Confirm `SqlType`/`JdbcSqlTypes` have NO runtime (non-migration) consumers: `grep -rn "SqlType\|JdbcSqlTypes" server/java --include=*.java | grep -v /target/ | grep -v /migrate/` → expect only `migrate/` + the drivers' `render` path. If a runtime consumer exists, STOP and re-scope.
60+
- [ ] Confirm `defs/` has runtime consumers (it does: SimpleMappingHandlerDB/ObjectMapping/validator/CRUD) → KEEP.
61+
- [ ] Snapshot baseline: `mvn -q -pl om,omdb,maven-plugin,core-spring,omdb-ktx test` green; `integration-tests` green (Docker).
62+
63+
**Unit 1 — drivers shed the renderer.**
64+
- [ ] Remove `implements MigrationSqlRenderer` + `render(Change)` + migration-only `SqlType` mappers from `GenericSQLDriver`/`PostgresDriver`/`DerbyDriver`; delete `MigrationSqlRenderer.java`. Keep legacy DDL + CRUD.
65+
- [ ] `mvn -q -pl omdb test` — fix any compile breakage (the validator/CRUD must still compile against the kept DDL). Commit.
66+
67+
**Unit 2 — delete the `migrate/` package + its tests.**
68+
- [ ] Delete `migrate/*` (16 files) + `SchemaMigrationEngineTest`/`ExpectedSchemaBuilderTest`/`ViewMigrationTest` + migrate fixtures. Fix `DriverFloatTypeTest` references.
69+
- [ ] `mvn -q -pl omdb test` green. Commit.
70+
71+
**Unit 3 — remove the Maven `meta:migrate` goal.**
72+
- [ ] Delete `MetaDataMigrateMojo`; remove migrate-only plumbing from `AbstractMetaDataMojo`/plugin config; update plugin README (drop `migrate`, keep gen/verify/editor).
73+
- [ ] `mvn -q -pl maven-plugin test` (or `install`) green. Commit.
74+
75+
**Unit 4 — integration-tests: drop Java migration scenarios.**
76+
- [ ] Remove `MigrationScenarioRunner` + migration-scenario invocations; keep query + api-contract scenarios. Adjust the conformance count (Java = query-only).
77+
- [ ] `mvn -f integration-tests/pom.xml test` (Docker) green. Commit.
78+
79+
**Unit 5 — docs + memories.**
80+
- [ ] Update maven-plugin docs, FR-003/spec/roadmap Java-migration claims, and the conformance memories. Commit.
81+
82+
**Final:** full reactor `mvn -q test` green; cross-cutting review + simplify; finish branch (merge forward + push).
83+
84+
---
85+
86+
## Decisions — RESOLVED 2026-05-30
87+
1. **Option A** — remove the migration ENGINE only; **keep** runtime auto-create DDL (validator + legacy `createTable`/`createIndex`/`createForeignKey`) and the shared `defs/` model. ✓
88+
2. **Persistence-conformance** — Java **permanently** stops running migration scenarios (not skipped); the corpus migration scenarios remain for TS (and other ports that own migration). ✓
89+
3. **Maven `meta:migrate`****fully removed; do NOT proxy the TS CLI.** Rationale: the TS `meta migrate` runs against a TS project shape (`metaobjects.config.ts`/`.metaobjects/`/TS CLI), would force Node+npm into every Maven build, and re-entangles the TS-owns-migrations boundary. Add a one-line plugin-README pointer ("schema migrations are managed by the TS toolchain"). **Flag (non-blocking):** a pure-Java shop is then left with runtime auto-create (dev) + externally-managed schema (prod), or adopting the TS CLI — revisit only if pure-Java adopters are a target.
90+
4. **`defs/`** — stays (runtime mapping model). ✓
91+
92+
## Risks / notes
93+
- The drivers are the trickiest edit — `render(Change)` and legacy `createTable` coexist in the same files; delete only the `Change`/render surface. The Tier-1 paging/codec work and Tier-0 fixes are all in CRUD/query paths and are unaffected.
94+
- This is **forward-only** removal — no rollback path (consistent with the TS-only direction). The deleted engine remains in git history if ever needed.

server/java/om/src/main/java/com/metaobjects/manager/QueryOptions.java

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ public class QueryOptions
2424
private Expression mExp = null;
2525
private SortOrder mOrder = null;
2626
private Range mRange = null;
27-
//private boolean mWriteableOnly = false;
2827
private Collection<MetaField> mFields = null;
2928
private boolean mWithLock = false;
3029

@@ -40,39 +39,15 @@ public QueryOptions( Expression exp )
4039
public QueryOptions( Expression exp, SortOrder order )
4140
{
4241
this( exp, order, null );
43-
//mExp = exp;
44-
//mOrder = order;
4542
}
4643

47-
//public QueryOptions( SortOrder order )
48-
//{
49-
// mOrder = order;
50-
//}
51-
5244
public QueryOptions( Expression exp, SortOrder order, Range range )
5345
{
5446
setExpression( exp );
5547
setSortOrder( order );
5648
setRange( range );
5749
}
5850

59-
//public QueryOptions( SortOrder order, Range range )
60-
//{
61-
// mOrder = order;
62-
// mRange = range;
63-
//}
64-
65-
//public QueryOptions( Range range )
66-
//{
67-
// mRange = range;
68-
//}
69-
70-
//public QueryOptions( Expression exp, Range range )
71-
//{
72-
// mExp = exp;
73-
// mRange = range;
74-
//}
75-
7651
public void setExpression( Expression exp )
7752
{
7853
mExp = exp;
@@ -128,14 +103,6 @@ public Collection<MetaField> getFields()
128103
return mFields;
129104
}
130105

131-
//public void setWriteableOnly( boolean writeableOnly ) {
132-
// mWriteableOnly = writeableOnly;
133-
//}
134-
135-
//public boolean getWriteableOnly() {
136-
// return mWriteableOnly;
137-
//}
138-
139106
public String toString()
140107
{
141108
return "Options{ EXP: " + getExpression() + "; ORDER: " + getSortOrder() + "; RANGE: " + getRange() + " }";

server/java/om/src/main/java/com/metaobjects/manager/exp/parser/ExpressionParser.java

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,8 @@
22

33
import java.text.DateFormat;
44

5-
import com.metaobjects.object.EntityMetaObject;
65
import com.metaobjects.object.MetaObject;
76
import com.metaobjects.field.MetaField;
8-
import com.metaobjects.field.DateField;
9-
import com.metaobjects.field.IntegerField;
10-
import com.metaobjects.field.LongField;
11-
import com.metaobjects.field.StringField;
127
import com.metaobjects.manager.exp.Expression;
138
import org.slf4j.Logger;
149
import org.slf4j.LoggerFactory;
@@ -264,20 +259,4 @@ protected boolean isOr( String str, int x ) {
264259
str.charAt( x+2 ) == '(' )) return true;
265260
return false;
266261
}
267-
268-
public static void main( String [] args ) throws ExpressionParseError
269-
{
270-
MetaObject mc = new EntityMetaObject("test");
271-
MetaField a = new LongField("id");
272-
MetaField b = new StringField("name");
273-
MetaField c = new IntegerField("value");
274-
MetaField d = new DateField("time");
275-
mc.addMetaField( a );
276-
mc.addMetaField( b );
277-
mc.addMetaField( c );
278-
mc.addMetaField( d );
279-
280-
Expression exp = ExpressionParser.getInstance().parse( mc, "( time > = '10/12/2006 14:55' and ( ( id = 5 or name = 'test me!' ) and value <> 20))" );
281-
log.debug("EXPRESSION: {}", exp);
282-
}
283262
}

server/java/om/src/main/java/com/metaobjects/object/managed/ManagedObject.java

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,10 @@
1414
import java.util.*;
1515
import java.util.concurrent.ConcurrentHashMap;
1616

17-
//import com.metaobjects.manager.*;
18-
1917
public class ManagedObject implements Map<String, Object>, Serializable, MetaObjectAware {
2018

2119
private static final long serialVersionUID = 6888178049723946186L;
2220

23-
//private static Log log = LogFactory.getLog( MetaObject.class );
2421
public class Value implements Serializable {
2522

2623
private static final long serialVersionUID = 746942287755477951L;
@@ -77,21 +74,18 @@ public void setNew(boolean state) {
7774
}
7875
}
7976

80-
//private String mId = null;
8177
private final Map<String, Value> mAttributes = new ConcurrentHashMap<String, Value>();
82-
78+
8379
private boolean mDeleted = false;
8480
private boolean mModified = false;
8581
private boolean mNew = true;
8682
// The MetaObject is transient, but the loader name, package name, and class name are needed
8783
private transient MetaObject mMetaObject = null;
8884
private String mLoaderName = null;
89-
//private String mPackageName = null;
9085
private String mObjectName = null;
9186
private transient ObjectManager mManager = null;
9287

9388
public ManagedObject() {
94-
//mId = "";
9589
mNew = true;
9690
}
9791

@@ -105,7 +99,6 @@ public ObjectManager getObjectManager() {
10599

106100
public String getObjectRef()
107101
{
108-
//return mId;
109102
try {
110103
return getObjectManager().getObjectRef( this ).toString();
111104
} catch( Exception e ) {
@@ -157,10 +150,6 @@ public String getMetaDataLoaderName() {
157150
return mLoaderName;
158151
}
159152

160-
//public String getMetaPackageName()
161-
//{
162-
// return mPackageName;
163-
//}
164153
public String getMetaObjectName() {
165154
return mObjectName;
166155
}
@@ -215,18 +204,6 @@ protected Value getObjectAttributeValue(String name) //throws ValueException
215204
}
216205
}
217206

218-
/*public Object getObjectValue( String name )
219-
throws MetaException
220-
{
221-
return getMetaField( name ).getValue( this );
222-
}
223-
224-
public void setObjectValue( String name, Object value )
225-
throws MetaException
226-
{
227-
getMetaField( name ).setValue( this, value );
228-
}*/
229-
230207
//////////////////////////////////////////////////////////////
231208
// PRIMITIVE SETTER VALUES
232209
public void setBoolean(String name, boolean value) //throws ValueException
@@ -470,15 +447,7 @@ public String toString() {
470447
MetaObject mc = getMetaData();
471448

472449
b.append("[");
473-
//String ref = getObjectRef();
474-
//if ( ref != null )
475-
// b.append( ref );
476-
//else
477-
//{
478-
//b.append( "{NEW}" );
479-
//b.append( '@' );
480450
b.append(mc.getName());
481-
//}
482451
b.append("]");
483452

484453
boolean first = true;

0 commit comments

Comments
 (0)