Skip to content

Commit 8302bcd

Browse files
dmealingclaude
andcommitted
Merge omdb-final-cleanup: TimeField round-trip + validator propagation
Two final OMDB-remediation fixes: - DataConverter passes DataTypes.CUSTOM values through (return val) instead of throwing — unblocks TimeField (LocalTime). Completes the round-trip: + Types.TIME in SimpleMappingHandlerDB.getSQLType/getSQLLength and Derby/Postgres createTable DDL. - MetaClassDBValidatorService.verifyMapping no longer swallows exceptions (restored the MetaDataException rethrow) so TableDoesNotExistException surfaces when a table is missing and autoCreate=false. metadata + omdb suites green (omdb 35/35). Reviewed (spec ✅ / quality Approved) + simplified. Follow-up note: the TimeField SQL-type hook is instanceof-specific; a unified codec-registry SQL-type extension point would also cover ClassField (the only other CUSTOM field, not used in OMDB DDL today) — deferred, non-blocking. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 parents fa94419 + 78910a9 commit 8302bcd

10 files changed

Lines changed: 190 additions & 17 deletions

File tree

server/java/metadata/src/main/java/com/metaobjects/util/DataConverter.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ public static Object toType( DataTypes dataType, Object val ) {
5151
case STRING_ARRAY: return toStringArray( val );
5252
case OBJECT_ARRAY: return toObjectArray( val );
5353

54-
case CUSTOM: throw new IllegalStateException( "Cannot convert to a custom type, value passed: [" + val + "]" );
54+
// Custom types are opaque to the generic converter; their JDBC binding is
55+
// handled by the per-type codec (e.g. TimeCodec for TimeField/LocalTime).
56+
case CUSTOM: return val;
5557

5658
default: throw new IllegalStateException( "Unknown type (" + dataType + "), cannot convert object [" + val + "]" );
5759
}

server/java/metadata/src/test/java/com/metaobjects/util/DataConverterTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package com.metaobjects.util;
22

3+
import com.metaobjects.DataTypes;
34
import com.metaobjects.loader.LoaderOptions;
45
import org.junit.Test;
56

7+
import java.time.LocalTime;
68
import java.util.ArrayList;
79
import java.util.Date;
810
import java.util.List;
@@ -329,6 +331,24 @@ public void testToStringArray() {
329331
assertEquals("42", numberResult.get(0));
330332
}
331333

334+
/**
335+
* CUSTOM type is opaque to the generic converter; values must pass through unchanged.
336+
* This allows per-type codecs (e.g. TimeCodec for TimeField) to handle their own
337+
* JDBC binding without being blocked by the generic converter.
338+
*/
339+
@Test
340+
public void testCustomTypePassesThrough() {
341+
LocalTime localTime = LocalTime.of(13, 45, 30);
342+
Object result = DataConverter.toType(DataTypes.CUSTOM, localTime);
343+
assertSame("CUSTOM type must return the exact same LocalTime instance", localTime, result);
344+
345+
Object arbitrary = new Object();
346+
Object result2 = DataConverter.toType(DataTypes.CUSTOM, arbitrary);
347+
assertSame("CUSTOM type must return the exact same Object instance", arbitrary, result2);
348+
349+
assertNull("CUSTOM type with null input must return null", DataConverter.toType(DataTypes.CUSTOM, null));
350+
}
351+
332352
@Test
333353
public void testToObjectArray() {
334354
// Test null input

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import com.metaobjects.MetaDataException;
1414

1515
import com.metaobjects.field.MetaField;
16+
import com.metaobjects.field.TimeField;
1617
import com.metaobjects.identity.MetaIdentity;
1718
import com.metaobjects.identity.PrimaryIdentity;
1819
import com.metaobjects.manager.ObjectManager;
@@ -266,6 +267,8 @@ protected boolean isJsonbField(MetaField mf) {
266267
protected int getSQLType( MetaField mf ) {
267268
// jsonb fields are stored as text (VARCHAR/CLOB) — no native jsonb on Derby
268269
if (isJsonbField(mf)) return Types.VARCHAR;
270+
// TimeField uses DataTypes.CUSTOM; its SQL column type is TIME.
271+
if (mf instanceof TimeField) return Types.TIME;
269272
switch( mf.getDataType() )
270273
{
271274
case BOOLEAN: return Types.BIT;
@@ -285,6 +288,8 @@ protected int getSQLType( MetaField mf ) {
285288
protected int getSQLLength( MetaField mf ) {
286289
// jsonb: store as CLOB (length > Derby's VARCHAR max of 32672 triggers CLOB in DerbyDriver)
287290
if (isJsonbField(mf)) return 65536;
291+
// TimeField uses DataTypes.CUSTOM; TIME columns have no meaningful length parameter.
292+
if (mf instanceof TimeField) return 0;
288293
switch( mf.getDataType() )
289294
{
290295
case BOOLEAN: return 1;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ public void createTable(Connection c, TableDef table) throws SQLException {
8686
case Types.FLOAT -> query.append("REAL");
8787
case Types.DOUBLE -> query.append("DOUBLE");
8888
case Types.TIMESTAMP -> query.append("TIMESTAMP");
89+
case Types.TIME -> query.append("TIME");
8990
case Types.VARCHAR -> {
9091
if (col.getLength() > 32700) {
9192
query.append("CLOB");

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ public void createTable(Connection c, TableDef table) throws SQLException {
8989
case Types.FLOAT -> query.append("REAL");
9090
case Types.DOUBLE -> query.append("DOUBLE PRECISION");
9191
case Types.TIMESTAMP -> query.append("TIMESTAMP WITH TIME ZONE");
92+
case Types.TIME -> query.append("TIME");
9293
case Types.VARCHAR -> {
9394
if (col.getLength() > 10485760) { // 10MB limit for VARCHAR
9495
query.append("TEXT");

server/java/omdb/src/main/java/com/metaobjects/manager/db/validator/MetaClassDBValidatorService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ private void verifyMapping(Connection c, MetaObject mc, DatabaseDriver dd, Objec
180180
}
181181
}
182182
catch( Exception e ) {
183-
//throw new MetaDataException( "Error validating mapping [" + mapping + "] for MetaClass [" + mc + "]: " + e.getMessage(), e );
183+
throw new MetaDataException( "Error validating mapping [" + mapping + "] for MetaClass [" + mc + "]: " + e.getMessage(), e );
184184
}
185185
}
186186

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import javax.sql.DataSource;
2222
import java.io.PrintWriter;
2323
import java.sql.*;
24+
import java.time.LocalTime;
2425
import java.util.ArrayList;
2526
import java.util.Collection;
2627
import java.util.List;
@@ -109,6 +110,7 @@ private ValueObject sample(MetaObject mo, String label) {
109110
vo.setObject("amount", new java.math.BigDecimal("1.00"));
110111
vo.setString("label", label);
111112
vo.setDate("createdAt", new java.util.Date(1_700_000_000_000L));
113+
vo.setObject("startTime", LocalTime.of(8, 0, 0));
112114
return vo;
113115
}
114116

server/java/omdb/src/test/java/com/metaobjects/manager/db/codec/JdbcCodecRoundTripTest.java

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ public void primitivesRoundTripThroughCodecs() throws Exception {
109109
vo.setString("label", "hello-codec");
110110
Date created = new Date(1_700_000_000_000L);
111111
vo.setDate("createdAt", created);
112+
vo.setObject("startTime", LocalTime.of(8, 0, 0)); // TimeCodec
112113

113114
omdb.createObject(oc, vo);
114115

@@ -136,22 +137,54 @@ public void primitivesRoundTripThroughCodecs() throws Exception {
136137
}
137138

138139
/**
139-
* Task 1.5 (remediation) — TimeCodec symmetry at the codec/JDBC boundary.
140+
* Full OMDB end-to-end round-trip for {@code field.time} — write a {@link LocalTime}
141+
* through {@code omdb.createObject} and read it back through {@code omdb.getObjects}.
140142
*
141-
* <p>A full OMDB round-trip for {@code field.time} is blocked by a
142-
* {@code MetaField}/{@code DataConverter} limitation: {@link TimeField}'s
143-
* data type is {@code DataTypes.CUSTOM}, and {@code DataConverter.toType(CUSTOM, …)}
144-
* throws {@code IllegalStateException}. So {@code MetaField.setObject} (which
145-
* {@code TimeCodec.readInto} calls) cannot accept a {@code LocalTime} on a real
146-
* TimeField. That is unrelated to the codec's correctness.
147-
*
148-
* <p>This test therefore drives the codec's real JDBC IO directly against an
149-
* embedded-Derby {@code TIME} column: write a {@link LocalTime} via
150-
* {@link JdbcCodecs.TimeCodec#write} and read it back via
151-
* {@link JdbcCodecs.TimeCodec#readInto}, using a thin {@link TimeField} subclass
152-
* whose {@code setObject}/{@code getObject} store the value verbatim (bypassing
153-
* the unrelated CUSTOM-type {@code DataConverter} hop). This proves the codec's
154-
* {@code LocalTime → java.sql.Time → LocalTime} conversion is symmetric.
143+
* <p>This exercises the complete path that was previously blocked:
144+
* {@code TimeCodec.readInto} → {@code MetaField.setObject} →
145+
* {@code DataConverter.toType(CUSTOM, localTime)}, which now passes through
146+
* unchanged instead of throwing {@code IllegalStateException}.
147+
*/
148+
@Test
149+
public void timeFieldRoundTripThroughOMDB() throws Exception {
150+
MetaObject mo = registry.findMetaObjectByName("codectest::Sample");
151+
assertNotNull(mo);
152+
153+
LocalTime original = LocalTime.of(9, 30, 0);
154+
155+
ObjectConnection oc = omdb.getConnection();
156+
try {
157+
ValueObject vo = (ValueObject) mo.newInstance();
158+
// Set all NOT-NULL columns required by the CODEC_SAMPLE schema.
159+
String label = "time-roundtrip-" + System.currentTimeMillis();
160+
vo.setString("label", label);
161+
vo.setInt("count", 1);
162+
vo.setLong("bignum", 1L);
163+
vo.setBoolean("active", false);
164+
vo.setDouble("ratio", 0d);
165+
vo.setFloat("rate", 0f);
166+
vo.setObject("amount", java.math.BigDecimal.ZERO);
167+
vo.setDate("createdAt", new Date(0));
168+
vo.setObject("startTime", original);
169+
170+
omdb.createObject(oc, vo);
171+
172+
Collection<?> rows = omdb.getObjects(oc, mo,
173+
new QueryOptions(new Expression("label", label, Expression.EQUAL)));
174+
assertEquals("exactly one row written", 1, rows.size());
175+
176+
ValueObject read = (ValueObject) rows.iterator().next();
177+
assertEquals("TimeField LocalTime must survive full OMDB write+read round-trip",
178+
original, read.getObject("startTime"));
179+
} finally {
180+
omdb.releaseConnection(oc);
181+
}
182+
}
183+
184+
/**
185+
* TimeCodec symmetry at the raw codec/JDBC boundary (codec unit test, not OMDB
186+
* end-to-end). Uses a verbatim-storing TimeField subclass to isolate ONLY the
187+
* codec's {@code LocalTime → java.sql.Time → LocalTime} JDBC conversion.
155188
*/
156189
@Test
157190
public void timeCodecIsSymmetricAtTheJdbcBoundary() throws Exception {
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Copyright (c) 2026 Doug Mealing LLC. All Rights Reserved.
3+
*/
4+
package com.metaobjects.manager.db.validator;
5+
6+
import com.metaobjects.MetaDataException;
7+
import com.metaobjects.loader.MetaDataLoader;
8+
import com.metaobjects.manager.db.ObjectManagerDB;
9+
import com.metaobjects.manager.db.driver.DerbyDriver;
10+
import com.metaobjects.manager.db.defs.TableDef;
11+
import com.metaobjects.registry.MetaDataLoaderRegistry;
12+
import com.metaobjects.registry.ServiceRegistryFactory;
13+
import org.junit.Test;
14+
15+
import javax.sql.DataSource;
16+
import java.io.PrintWriter;
17+
import java.sql.*;
18+
import java.util.logging.Logger;
19+
20+
import static org.junit.Assert.*;
21+
22+
/**
23+
* Unit tests for MetaClassDBValidatorService exception propagation (Fix B).
24+
*/
25+
public class MetaClassDBValidatorServiceTest {
26+
27+
/**
28+
* When autoCreate=false and the underlying driver reports that a table does not
29+
* exist, validateDefinition throws TableDoesNotExistException. Before the fix,
30+
* verifyMapping silently swallowed this; after the fix it must propagate as a
31+
* MetaDataException wrapping the original cause.
32+
*/
33+
@Test
34+
public void verifyMappingPropagatesTableDoesNotExistException() throws Exception {
35+
MetaDataLoaderRegistry registry = new MetaDataLoaderRegistry(ServiceRegistryFactory.getDefault());
36+
MetaDataLoader loader = MetaDataLoader.fromResources(
37+
"test-validator", java.util.List.of("meta.fruit.json"));
38+
registry.registerLoader(loader);
39+
40+
// A DerbyDriver subclass whose checkTable always returns false (table absent).
41+
DerbyDriver absentDriver = new DerbyDriver() {
42+
@Override
43+
public boolean checkTable(Connection c, TableDef table) throws SQLException {
44+
return false;
45+
}
46+
};
47+
48+
String dbFile = "validator-test-" + System.currentTimeMillis();
49+
Class.forName("org.apache.derby.jdbc.EmbeddedDriver");
50+
// Create the DB so getConnection works; we don't need actual tables.
51+
DriverManager.getConnection("jdbc:derby:memory:" + dbFile + ";create=true").close();
52+
53+
try {
54+
DataSource ds = new DataSource() {
55+
@Override public Connection getConnection() throws SQLException {
56+
return DriverManager.getConnection("jdbc:derby:memory:" + dbFile + ";create=true");
57+
}
58+
@Override public Connection getConnection(String u, String p) throws SQLException { return getConnection(); }
59+
@Override public PrintWriter getLogWriter() { return new PrintWriter(System.out); }
60+
@Override public void setLogWriter(PrintWriter out) {}
61+
@Override public void setLoginTimeout(int s) {}
62+
@Override public int getLoginTimeout() { return 100; }
63+
@Override public Logger getParentLogger() { throw new UnsupportedOperationException(); }
64+
@Override public <T> T unwrap(Class<T> iface) { throw new UnsupportedOperationException(); }
65+
@Override public boolean isWrapperFor(Class<?> iface) { return false; }
66+
};
67+
68+
ObjectManagerDB omdb = new ObjectManagerDB();
69+
omdb.setDatabaseDriver(absentDriver);
70+
omdb.setDataSource(ds);
71+
omdb.init();
72+
73+
MetaClassDBValidatorService vs = new MetaClassDBValidatorService();
74+
vs.setObjectManager(omdb);
75+
vs.setAutoCreate(false); // must NOT auto-create: triggers the throw path
76+
vs.setMetaDataLoaderRegistry(registry);
77+
78+
try {
79+
vs.init();
80+
fail("init() must throw when autoCreate=false and the table is absent; "
81+
+ "verifyMapping must propagate TableDoesNotExistException, not swallow it");
82+
} catch (MetaDataException e) {
83+
// Expected: verifyMapping wraps and re-throws as MetaDataException.
84+
assertTrue("cause chain must contain TableDoesNotExistException",
85+
isCausedBy(e, TableDoesNotExistException.class));
86+
}
87+
} finally {
88+
// Drop the in-memory DB.
89+
try { DriverManager.getConnection("jdbc:derby:memory:" + dbFile + ";drop=true"); }
90+
catch (SQLNonTransientConnectionException ignored) {}
91+
loader.destroy();
92+
}
93+
}
94+
95+
/** Returns true if any cause in the chain is assignable to the given type. */
96+
private static boolean isCausedBy(Throwable t, Class<?> type) {
97+
while (t != null) {
98+
if (type.isInstance(t)) return true;
99+
t = t.getCause();
100+
}
101+
return false;
102+
}
103+
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@
6868
"@column": "createdAt"
6969
}
7070
},
71+
{
72+
"field.time": {
73+
"name": "startTime",
74+
"@column": "startTime"
75+
}
76+
},
7177
{
7278
"identity.primary": {
7379
"name": "primary",

0 commit comments

Comments
 (0)