Merge omdb-remediation Tier 0: verified OMDB correctness fixes + remediation plan

Tier 0 (5 verified bugs): ManagedObject.remove wrong-target read;
ExpressionParser no longer lower-cases literals/NULL; generic getNextAutoId
uses long (test-only fallback); readMany applies in-memory range when a
dialect can't page; non-synchronized lazy driver accessor. Plus the
prioritized remediation plan (migration tasks dropped per the TS-only
migrations pivot).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: dmealing

docs/superpowers/plans/2026-05-30-omdb-remediation.md

@@ -45,7 +45,10 @@ public static ExpressionParser getInstance() {
 
   public Expression parse( MetaObject mc, String expressionString ) throws ExpressionParseError
   {
-    return parse( mc, expressionString.toLowerCase(), 0, 0 ).getExp();
+    // NOTE: the query string is NOT lower-cased — doing so corrupts quoted literals
+    // and the NULL sentinel. Only the and/or keywords are matched case-insensitively
+    // (see isAnd/isOr); field-name lookup stays case-sensitive (metadata names are).
+    return parse( mc, expressionString, 0, 0 ).getExp();
   }
 
   protected final static int ACTION_AND = 1;
@@ -83,13 +86,13 @@ else if ( c == ')' ) {
       else if ( c == ' ' ) {
         x++;
       }
-      else if ( c == 'a' && isAnd( str, x )){
+      else if ( ( c == 'a' || c == 'A' ) && isAnd( str, x )){
         action = ACTION_AND;
         x += 3;
         if ( a == null ) throw new ExpressionParseError( "error.parse.unexpected", x, x+3 );
         n = x;
       }
-      else if ( c == 'o' && isOr( str, x )){
+      else if ( ( c == 'o' || c == 'O' ) && isOr( str, x )){
         action = ACTION_OR;
         x += 2;
         if ( a == null ) throw new ExpressionParseError( "error.parse.unexpected", x, x+2 );
@@ -111,7 +114,11 @@ else if ( c == 'o' && isOr( str, x )){
         //if ( str.charAt( r.getEnd() ) != ')' )
         x = r.getEnd();
 
-        log.debug("END: [{}] {}", str.charAt(x), str.substring(x));
+        // Guard the debug log: a top-level expression can return x == str.length()
+        // (end-of-string), so charAt(x) would be out of bounds.
+        if ( log.isDebugEnabled() && x < str.length() ) {
+          log.debug("END: [{}] {}", str.charAt(x), str.substring(x));
+        }
 
         n = x;
       }
@@ -244,15 +251,15 @@ protected ExpResult buildExpresion( MetaField mf, String type, String name, int
 
   protected boolean isAnd( String str, int x ) {
     if (( str.length() - x ) < 4 ) return false;
-    if ( str.substring( x, x+3 ).equals( "and" )
+    if ( str.substring( x, x+3 ).equalsIgnoreCase( "and" )
         && ( str.charAt( x+3 ) == ' ' ||
             str.charAt( x+3 ) == '(' )) return true;
     return false;
   }
 
   protected boolean isOr( String str, int x ) {
     if (( str.length() - x ) < 3 ) return false;
-    if ( str.substring( x, x+2 ).equals( "or" )
+    if ( str.substring( x, x+2 ).equalsIgnoreCase( "or" )
         && ( str.charAt( x+2 ) == ' ' ||
             str.charAt( x+2 ) == '(' )) return true;
     return false;

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

@@ -616,7 +616,7 @@ public Object remove(Object key) {
         if (mf == null) {
             return null;
         }
-        Object o = mf.getObject(key);
+        Object o = mf.getObject(this);
         mf.setObject(this, null);
         return o;
     }

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

@@ -0,0 +1,60 @@
+package com.metaobjects.manager.exp.parser;
+
+import com.metaobjects.field.IntegerField;
+import com.metaobjects.field.StringField;
+import com.metaobjects.manager.exp.Expression;
+import com.metaobjects.object.EntityMetaObject;
+import com.metaobjects.object.MetaObject;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Regression tests for {@link ExpressionParser}: it previously lower-cased the entire
+ * query string, corrupting quoted literals and making the {@code NULL} sentinel
+ * unreachable. Only the {@code and}/{@code or} keywords are matched case-insensitively;
+ * field identifiers and literal values preserve their original case.
+ */
+public class ExpressionParserCaseTest {
+
+    private MetaObject mc;
+
+    @Before
+    public void setUp() {
+        mc = new EntityMetaObject("test");
+        mc.addMetaField(new StringField("name"));
+        mc.addMetaField(new IntegerField("value"));
+    }
+
+    @Test
+    public void preservesMixedCaseStringLiteral() throws ExpressionParseError {
+        Expression exp = ExpressionParser.getInstance().parse(mc, "name = 'Test Me'");
+
+        Assert.assertEquals("name", exp.getField());
+        Assert.assertEquals("string value preserves original case", "Test Me", exp.getValue());
+        Assert.assertEquals(Expression.EQUAL, exp.getCondition());
+    }
+
+    @Test
+    public void nullSentinelProducesNullValuedExpression() throws ExpressionParseError {
+        Expression exp = ExpressionParser.getInstance().parse(mc, "name = NULL");
+
+        Assert.assertEquals("name", exp.getField());
+        Assert.assertNull("NULL sentinel yields a null value", exp.getValue());
+    }
+
+    @Test
+    public void andKeywordIsCaseInsensitive() throws ExpressionParseError {
+        Expression exp = ExpressionParser.getInstance().parse(mc, "( name = 'Foo' AND value = 5 )");
+
+        // A grouped/compound expression is returned without error.
+        Assert.assertNotNull(exp);
+    }
+
+    @Test
+    public void orKeywordIsCaseInsensitive() throws ExpressionParseError {
+        Expression exp = ExpressionParser.getInstance().parse(mc, "( name = 'Foo' Or value = 5 )");
+
+        Assert.assertNotNull(exp);
+    }
+}

server/java/om/src/test/java/com/metaobjects/manager/exp/parser/ExpressionParserCaseTest.java

@@ -0,0 +1,36 @@
+package com.metaobjects.object.managed;
+
+import com.metaobjects.field.StringField;
+import com.metaobjects.object.MetaObject;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Regression test for {@link ManagedObject#remove(Object)} which previously read the
+ * prior field value from the field-name key String instead of from {@code this}.
+ */
+public class ManagedObjectRemoveTest {
+
+    private MetaObject metaObject;
+
+    @Before
+    public void setUp() {
+        metaObject = new ManagedMetaObject("RemoveTestEntity");
+        metaObject.addMetaField(new StringField("name"));
+    }
+
+    @Test
+    public void removeReturnsPriorValueAndClearsField() {
+        ManagedObject obj = new ManagedObject();
+        obj.setMetaData(metaObject);
+
+        obj.put("name", "Test Me");
+        Assert.assertEquals("precondition: value is set", "Test Me", obj.get("name"));
+
+        Object removed = obj.remove("name");
+
+        Assert.assertEquals("remove() returns the prior value read from this", "Test Me", removed);
+        Assert.assertNull("field is cleared after remove()", obj.get("name"));
+    }
+}

server/java/om/src/test/java/com/metaobjects/object/managed/ManagedObjectRemoveTest.java

@@ -54,7 +54,9 @@ public class ObjectManagerDB extends ObjectManager implements DBOperations {
     private final ConcurrentHashMap<MetaObject, Optional<ObjectMapping>> deleteMappings = new ConcurrentHashMap<>();
 
     private MappingHandler mMappingHandler = null;
-    private DatabaseDriver mDriver = null;
+    private volatile DatabaseDriver mDriver = null;
+    /** Guards lazy initialization of {@link #mDriver} in {@link #getDatabaseDriver()}. */
+    private final Object driverInitLock = new Object();
     private DataSource mSource = null;
     private boolean enforceTransaction = false;
     //private static Cache<String, MetaObject> templateCache = new Cache<String, MetaObject>(true, 3000, 1500);
@@ -191,13 +193,21 @@ public void setDatabaseDriver(Object dd) {
     }
 
     @Override
-    public synchronized Object getDatabaseDriver() {
-        if (mDriver == null) {
-            mDriver = new GenericSQLDriver();
-            mDriver.setManager(this);
+    public Object getDatabaseDriver() {
+        // Double-checked locking on a volatile field: the common (already-initialized)
+        // case is a lock-free volatile read, avoiding contention on this hot CRUD path.
+        DatabaseDriver d = mDriver;
+        if (d == null) {
+            synchronized (driverInitLock) {
+                d = mDriver;
+                if (d == null) {
+                    d = new GenericSQLDriver();
+                    d.setManager(this);
+                    mDriver = d;
+                }
+            }
         }
-
-        return mDriver;
+        return d;
     }
 
     /**

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

@@ -308,7 +308,16 @@ protected String getLastAutoId(Connection conn, ColumnDef col) throws SQLExcepti
     }
 
     /**
-     * Gets the next sequence for a given MetaClass
+     * Generic {@code MAX(col) + 1} fallback for the next auto-id.
+     *
+     * <p><strong>Single-writer / test-only.</strong> This is the base-class fallback used
+     * when a dialect provides no native sequence or identity column. It is <em>not</em>
+     * concurrency-safe: two concurrent writers can read the same {@code MAX} and collide.
+     * Real dialects override id generation with database sequences or identity/auto-increment
+     * columns; this exists only for simple embedded/test scenarios with a single writer.</p>
+     *
+     * <p>The {@code MAX} result is parsed as a {@code long} so BIGINT / {@code LongField}
+     * ids above {@link Integer#MAX_VALUE} are not truncated.</p>
      */
     protected String getNextAutoId(Connection conn, ColumnDef col) throws SQLException {
         // String table = getManager().getTableName( mc );
@@ -343,9 +352,9 @@ protected String getNextAutoId(Connection conn, ColumnDef col) throws SQLExcepti
                         return "1";
                     }
 
-                    int i = Integer.parseInt(tmp) + 1;
+                    long i = Long.parseLong(tmp) + 1;
 
-                    return "" + i;
+                    return Long.toString(i);
                 } finally {
                     rs.close();
                 }
@@ -759,10 +768,7 @@ public Collection<Object> readMany(Connection c, MetaObject mc,
             ResultSet rs = s.executeQuery();
 
             // If the range is not supported in the query, then we need to do it manually
-            Range range = null;
-            if (!supportsRangeInQuery()) {
-                options.getRange();
-            }
+            Range range = (!supportsRangeInQuery()) ? options.getRange() : null;
 
             int index = 1;
 

server/java/omdb/src/main/java/com/metaobjects/manager/db/driver/GenericSQLDriver.java

@@ -0,0 +1,75 @@
+package com.metaobjects.manager.db.driver;
+
+import com.metaobjects.manager.db.defs.ColumnDef;
+import com.metaobjects.manager.db.defs.NameDef;
+import com.metaobjects.manager.db.defs.TableDef;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.sql.SQLNonTransientConnectionException;
+import java.sql.Statement;
+import java.sql.Types;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Regression test for {@link GenericSQLDriver#getNextAutoId} which previously parsed the
+ * {@code MAX(col)} result with {@link Integer#parseInt}, overflowing BIGINT / LongField ids
+ * above {@link Integer#MAX_VALUE}. The fallback now parses as {@code long}.
+ *
+ * <p>Uses embedded in-memory Derby (no Testcontainers required), so it runs in the
+ * standard {@code omdb} unit suite.</p>
+ */
+public class GenericSQLDriverAutoIdTest {
+
+    private static String dbFile;
+
+    /** Exposes the protected fallback for direct testing. */
+    private static final class TestDriver extends GenericSQLDriver {
+        String nextAutoId(Connection c, ColumnDef col) throws SQLException {
+            return getNextAutoId(c, col);
+        }
+    }
+
+    @BeforeClass
+    public static void setUp() throws Exception {
+        Class.forName("org.apache.derby.jdbc.EmbeddedDriver");
+        dbFile = "autoid-testing-" + System.currentTimeMillis();
+        try (Connection c = getConnection(); Statement s = c.createStatement()) {
+            s.execute("CREATE TABLE BIG_SEQ ( ID BIGINT )");
+            // A value well above Integer.MAX_VALUE (2147483647).
+            s.execute("INSERT INTO BIG_SEQ ( ID ) VALUES ( 5000000000 )");
+        }
+    }
+
+    @AfterClass
+    public static void tearDown() {
+        try {
+            DriverManager.getConnection("jdbc:derby:memory:" + dbFile + ";drop=true");
+        } catch (SQLNonTransientConnectionException ignored) {
+        } catch (SQLException ignored) {
+        }
+    }
+
+    private static Connection getConnection() throws SQLException {
+        return DriverManager.getConnection("jdbc:derby:memory:" + dbFile + ";create=true");
+    }
+
+    @Test
+    public void nextAutoIdDoesNotOverflowAboveIntMax() throws Exception {
+        ColumnDef col = new ColumnDef("ID", Types.BIGINT);
+        TableDef table = new TableDef(new NameDef(null, "BIG_SEQ"));
+        col.setBaseTable(table);
+
+        TestDriver driver = new TestDriver();
+        try (Connection c = getConnection()) {
+            String next = driver.nextAutoId(c, col);
+            // 5,000,000,000 + 1 — would have thrown NumberFormatException / truncated under int parsing.
+            assertEquals("5000000001", next);
+        }
+    }
+}