Skip to content

Commit 933c157

Browse files
dmealingclaude
andcommitted
fix(omdb): DB validator propagates errors instead of swallowing (TableDoesNotExistException reachable)
MetaClassDBValidatorService.verifyMapping() wrapped validateDefinition() in an empty catch block (the rethrow was commented out), silently discarding TableDoesNotExistException when autoCreate=false and a required table was absent. Real SQLExceptions were also lost. Restore the rethrow so all exceptions surface as MetaDataException with the original cause attached. MetaDataException is unchecked (extends RuntimeException) so no method signature changes are required. Tests: new MetaClassDBValidatorServiceTest drives the validator with a stub DerbyDriver whose checkTable() always returns false and autoCreate=false, asserting that init() throws a MetaDataException whose cause chain contains TableDoesNotExistException. Full omdb suite: 35 tests, 0 failures (was 34 before the new test). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1355c8b commit 933c157

2 files changed

Lines changed: 111 additions & 1 deletion

File tree

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* Copyright (c) 2026 Doug Mealing LLC. All Rights Reserved.
3+
*
4+
* Fix B — verifies that MetaClassDBValidatorService.verifyMapping propagates
5+
* exceptions instead of swallowing them silently.
6+
*
7+
* Covers the bug where the empty catch block around validateDefinition() meant
8+
* that TableDoesNotExistException (thrown when autoCreate=false and the table is
9+
* absent) was discarded without any diagnostic or propagation.
10+
*/
11+
package com.metaobjects.manager.db.validator;
12+
13+
import com.metaobjects.MetaDataException;
14+
import com.metaobjects.loader.MetaDataLoader;
15+
import com.metaobjects.manager.db.ObjectManagerDB;
16+
import com.metaobjects.manager.db.driver.DerbyDriver;
17+
import com.metaobjects.manager.db.defs.TableDef;
18+
import com.metaobjects.registry.MetaDataLoaderRegistry;
19+
import com.metaobjects.registry.ServiceRegistryFactory;
20+
import org.junit.Test;
21+
22+
import javax.sql.DataSource;
23+
import java.io.PrintWriter;
24+
import java.sql.*;
25+
import java.util.logging.Logger;
26+
27+
import static org.junit.Assert.*;
28+
29+
/**
30+
* Unit tests for MetaClassDBValidatorService exception propagation (Fix B).
31+
*/
32+
public class MetaClassDBValidatorServiceTest {
33+
34+
/**
35+
* When autoCreate=false and the underlying driver reports that a table does not
36+
* exist, validateDefinition throws TableDoesNotExistException. Before the fix,
37+
* verifyMapping silently swallowed this; after the fix it must propagate as a
38+
* MetaDataException wrapping the original cause.
39+
*/
40+
@Test
41+
public void verifyMappingPropagatesTableDoesNotExistException() throws Exception {
42+
MetaDataLoaderRegistry registry = new MetaDataLoaderRegistry(ServiceRegistryFactory.getDefault());
43+
MetaDataLoader loader = MetaDataLoader.fromResources(
44+
"test-validator", java.util.List.of("meta.fruit.json"));
45+
registry.registerLoader(loader);
46+
47+
// A DerbyDriver subclass whose checkTable always returns false (table absent).
48+
DerbyDriver absentDriver = new DerbyDriver() {
49+
@Override
50+
public boolean checkTable(Connection c, TableDef table) throws SQLException {
51+
return false;
52+
}
53+
};
54+
55+
String dbFile = "validator-test-" + System.currentTimeMillis();
56+
Class.forName("org.apache.derby.jdbc.EmbeddedDriver");
57+
// Create the DB so getConnection works; we don't need actual tables.
58+
DriverManager.getConnection("jdbc:derby:memory:" + dbFile + ";create=true").close();
59+
60+
try {
61+
DataSource ds = new DataSource() {
62+
@Override public Connection getConnection() throws SQLException {
63+
return DriverManager.getConnection("jdbc:derby:memory:" + dbFile + ";create=true");
64+
}
65+
@Override public Connection getConnection(String u, String p) throws SQLException { return getConnection(); }
66+
@Override public PrintWriter getLogWriter() { return new PrintWriter(System.out); }
67+
@Override public void setLogWriter(PrintWriter out) {}
68+
@Override public void setLoginTimeout(int s) {}
69+
@Override public int getLoginTimeout() { return 100; }
70+
@Override public Logger getParentLogger() { throw new UnsupportedOperationException(); }
71+
@Override public <T> T unwrap(Class<T> iface) { throw new UnsupportedOperationException(); }
72+
@Override public boolean isWrapperFor(Class<?> iface) { return false; }
73+
};
74+
75+
ObjectManagerDB omdb = new ObjectManagerDB();
76+
omdb.setDatabaseDriver(absentDriver);
77+
omdb.setDataSource(ds);
78+
omdb.init();
79+
80+
MetaClassDBValidatorService vs = new MetaClassDBValidatorService();
81+
vs.setObjectManager(omdb);
82+
vs.setAutoCreate(false); // must NOT auto-create: triggers the throw path
83+
vs.setMetaDataLoaderRegistry(registry);
84+
85+
try {
86+
vs.init();
87+
fail("init() must throw when autoCreate=false and the table is absent; "
88+
+ "verifyMapping must propagate TableDoesNotExistException, not swallow it");
89+
} catch (MetaDataException e) {
90+
// Expected: verifyMapping wraps and re-throws as MetaDataException.
91+
assertTrue("cause chain must contain TableDoesNotExistException",
92+
isCausedBy(e, TableDoesNotExistException.class));
93+
}
94+
} finally {
95+
// Drop the in-memory DB.
96+
try { DriverManager.getConnection("jdbc:derby:memory:" + dbFile + ";drop=true"); }
97+
catch (SQLNonTransientConnectionException ignored) {}
98+
loader.destroy();
99+
}
100+
}
101+
102+
/** Returns true if any cause in the chain is assignable to the given type. */
103+
private static boolean isCausedBy(Throwable t, Class<?> type) {
104+
while (t != null) {
105+
if (type.isInstance(t)) return true;
106+
t = t.getCause();
107+
}
108+
return false;
109+
}
110+
}

0 commit comments

Comments
 (0)