Skip to content

Commit 8a93776

Browse files
yan-3005Siddhant
authored andcommitted
fix(workflows): make Flowable schema upgrades idempotent to survive partial migrations (#27234)
* fix(workflows): make Flowable schema upgrades idempotent to survive partial migrations Fixes #26048. When the server crashed mid-startup during a Flowable schema upgrade, the DB was left in a partially-migrated state. On restart, Flowable re-ran the same DDL and failed on already-existing objects (indexes, tables, columns), permanently wedging both the server and migrate --force. Changes: 1. WorkflowHandler: webserver now uses DB_SCHEMA_UPDATE_FALSE — it validates the schema but never runs DDL. Only migrate CLI uses DB_SCHEMA_UPDATE_TRUE. 2. OpenMetadataOperations: explicit WorkflowHandler.initialize(config, true) inside the migrate command so Flowable DDL always runs during migration. 3. WorkflowHandler: catches FlowableWrongDbException on webserver startup and rethrows with an actionable message directing the operator to run migrate. 4. IdempotentDdlDataSource + IdempotentDdlStatement: JDBC DataSource wrapper used exclusively in migration context. Intercepts execute(sql) for CREATE INDEX, CREATE TABLE, and ALTER TABLE ADD COLUMN and pre-checks existence via standard DatabaseMetaData (getIndexInfo, getTables, getColumns) before executing. If the object already exists it logs a skip and returns — no SQL state codes, no string matching, works on MySQL and PostgreSQL. Unit tests cover schema-update mode selection in both contexts. * fix(workflows): address review comments on idempotent DDL wrapper - Extract shouldSkip() helper; apply idempotency checks to all execute() and executeUpdate() overloads, not just execute(String) - Tighten ALTER TABLE regex with negative lookahead to exclude SQL keywords (CONSTRAINT, PRIMARY, UNIQUE, FOREIGN, CHECK, INDEX, KEY) from being matched as column names - IdempotentDdlDataSource now wraps a DataSource delegate instead of calling DriverManager directly; uses migrationDataSource() helper in WorkflowHandler to resolve from existing DataSource or JDBC params - Fix InvocationTargetException wrapping in Connection proxy — unwrap cause so callers receive the original SQLException - Wrap all createStatement() variants in the proxy, not just the no-arg form - Contextual error message in WorkflowHandler — distinguish between server startup and migration context - Add IdempotentDdlStatementTest: 11 tests covering skip/execute for CREATE INDEX, CREATE UNIQUE INDEX, CREATE TABLE, ALTER TABLE ADD COLUMN, keyword-guarded ALTER TABLE, executeUpdate overload, and pass-through * fix(workflows): include DB/library versions in FlowableWrongDbException message * test(workflows): add IdempotentDdlDataSourceTest for proxy wrapping and exception surfacing * test(workflows): assert exception identity in proxy exception-surfacing tests * fix(workflows): catalog-aware identifier normalization in IdempotentDdlStatement On MySQL with lower_case_table_names=0 (default on Linux), table names are stored as-is and catalog=null metadata lookups can miss existing objects. - Use connection.getCatalog() for all getIndexInfo/getTables/getColumns calls - Normalize identifiers via DatabaseMetaData.storesLowerCaseIdentifiers() / storesUpperCaseIdentifiers() instead of unconditional toLowerCase() - stripIdentifierQuotes() handles backtick, double-quote and bracket quoting - extractObjectName() handles schema-qualified names (schema.table) - columnExists now iterates and normalizes COLUMN_NAME from ResultSet - Test: added MySQL uppercase storage case to IdempotentDdlStatementTest * fix(workflows): null guard in shouldSkip, drop-create Flowable init, robust test indexing - shouldSkip() returns false immediately for null SQL, preserving JDBC contract (delegate handles null and throws the driver's own error) - drop-create command now calls WorkflowHandler.initialize(config, true) after native migrations so it produces a fully startable DB including Flowable tables - WorkflowHandlerSchemaUpdateTest: replace brittle get(1) with getLast() so the test is not sensitive to how many StandaloneProcessEngineConfiguration instances are constructed before initializeNewProcessEngine runs
1 parent 4b41cf1 commit 8a93776

7 files changed

Lines changed: 1135 additions & 42 deletions

File tree

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* Copyright 2021 Collate
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
12+
*/
13+
14+
package org.openmetadata.service.governance.workflows;
15+
16+
import java.io.PrintWriter;
17+
import java.lang.reflect.InvocationTargetException;
18+
import java.lang.reflect.Method;
19+
import java.lang.reflect.Proxy;
20+
import java.sql.Connection;
21+
import java.sql.SQLException;
22+
import java.sql.SQLFeatureNotSupportedException;
23+
import java.sql.Statement;
24+
import java.util.logging.Logger;
25+
import javax.sql.DataSource;
26+
27+
/**
28+
* A {@link DataSource} wrapper that intercepts every {@link Connection} it vends and ensures
29+
* that all {@link Statement} variants created from that connection go through
30+
* {@link IdempotentDdlStatement}. Only used in migration context so Flowable upgrade scripts
31+
* skip already-existing DDL objects instead of failing.
32+
*/
33+
final class IdempotentDdlDataSource implements DataSource {
34+
35+
private final DataSource delegate;
36+
37+
IdempotentDdlDataSource(DataSource delegate) {
38+
this.delegate = delegate;
39+
}
40+
41+
@Override
42+
public Connection getConnection() throws SQLException {
43+
return wrapConnection(delegate.getConnection());
44+
}
45+
46+
@Override
47+
public Connection getConnection(String username, String password) throws SQLException {
48+
return wrapConnection(delegate.getConnection(username, password));
49+
}
50+
51+
private Connection wrapConnection(Connection real) {
52+
return (Connection)
53+
Proxy.newProxyInstance(
54+
real.getClass().getClassLoader(),
55+
new Class<?>[] {Connection.class},
56+
(proxy, method, args) -> {
57+
if ("createStatement".equals(method.getName())) {
58+
Statement stmt = (Statement) invokeDelegate(method, real, args);
59+
return new IdempotentDdlStatement(stmt, real);
60+
}
61+
return invokeDelegate(method, real, args);
62+
});
63+
}
64+
65+
/**
66+
* Invokes a method on the real connection, unwrapping any {@link InvocationTargetException}
67+
* so callers receive the original exception type (e.g. {@link SQLException}).
68+
*/
69+
private static Object invokeDelegate(Method method, Object target, Object[] args)
70+
throws Throwable {
71+
try {
72+
return method.invoke(target, args);
73+
} catch (InvocationTargetException e) {
74+
throw e.getCause();
75+
}
76+
}
77+
78+
@Override
79+
public PrintWriter getLogWriter() throws SQLException {
80+
return delegate.getLogWriter();
81+
}
82+
83+
@Override
84+
public void setLogWriter(PrintWriter out) throws SQLException {
85+
delegate.setLogWriter(out);
86+
}
87+
88+
@Override
89+
public void setLoginTimeout(int seconds) throws SQLException {
90+
delegate.setLoginTimeout(seconds);
91+
}
92+
93+
@Override
94+
public int getLoginTimeout() throws SQLException {
95+
return delegate.getLoginTimeout();
96+
}
97+
98+
@Override
99+
public Logger getParentLogger() throws SQLFeatureNotSupportedException {
100+
return delegate.getParentLogger();
101+
}
102+
103+
@Override
104+
public <T> T unwrap(Class<T> iface) throws SQLException {
105+
return delegate.unwrap(iface);
106+
}
107+
108+
@Override
109+
public boolean isWrapperFor(Class<?> iface) throws SQLException {
110+
return delegate.isWrapperFor(iface);
111+
}
112+
}

0 commit comments

Comments
 (0)