diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCBrokerFactory.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCBrokerFactory.java index 974f32686a..4bef41ac07 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCBrokerFactory.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/JDBCBrokerFactory.java @@ -264,7 +264,7 @@ private void mapSchemaGenerationToSynchronizeMappings(JDBCConfiguration conf) { // preserve tracking from prior generateSchema() calls. if (conf.getDatabaseActionConstant() != 0 || conf.getScriptsActionConstant() != 0) { - SchemaTool.clearDroppedTables(); + SchemaTool.clearDroppedTables(conf); } String actions = ""; if (conf.getDatabaseAction() != null) { diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SchemaTool.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SchemaTool.java index 1461e4878a..6181b4cc19 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SchemaTool.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SchemaTool.java @@ -108,15 +108,111 @@ public class SchemaTool { // Only active when SpecCompliantSchemaGeneration is enabled (TCK mode). // Prevents buildSchema/add from re-creating tables that were explicitly // dropped by schema gen scripts within the same schema generation flow. - private static final java.util.Set _droppedTables = - java.util.Collections.synchronizedSet(new java.util.HashSet<>()); + // + // The tracking has to outlive the configuration that wrote it, since + // Persistence.generateSchema() closes its factory and a later factory is + // expected to see what it dropped. It is therefore static, but keyed by + // the database it describes, so that two persistence units on different + // databases neither consume nor clear one another's entries. + private static final java.util.Map> _droppedTables = + new java.util.HashMap<>(); + + /** + * The set of tables dropped on the database the given configuration + * connects to. Callers must hold the monitor of {@link #_droppedTables}. + */ + private static java.util.Set droppedTables(JDBCConfiguration conf) { + return _droppedTables.computeIfAbsent(trackingKey(conf), + k -> new java.util.HashSet<>()); + } + + /** + * The identity the tracking is partitioned by: the configuration id, which + * for a persistence unit is its name (see PersistenceUnitInfoImpl, which + * defaults openjpa.Id to the unit name). Two factories for one + * unit therefore share their tracking, which is what lets a factory see + * what a closed {@code Persistence.generateSchema()} factory dropped, while + * two units cannot clear or consume one another's entries. + *

+ * Configurations with no id fall back to the connection they name, and + * those that name neither share one entry, which is the behaviour the + * tracking had before it was partitioned. + */ + static String trackingKey(JDBCConfiguration conf) { + String[] candidates = new String[] { + conf.getId(), + conf.getConnectionFactory2Name(), conf.getConnection2URL(), + conf.getConnectionFactoryName(), conf.getConnectionURL(), + }; + for (String candidate : candidates) { + if (!StringUtil.isEmpty(candidate)) { + return candidate; + } + } + return ""; + } /** - * Clear the dropped tables tracking set. Called at the start of each - * schema generation flow to prevent cross-EMF contamination. + * Record the effect of a DDL statement executed from a schema generation + * script: a table it drops is tracked, a table it creates is forgotten. */ + static void trackScriptDdl(JDBCConfiguration conf, String sql) { + String upper = sql.toUpperCase(Locale.ROOT).trim(); + if (upper.startsWith("DROP TABLE")) { + String tableName = sql.trim() + .substring("DROP TABLE".length()).trim() + .replaceAll("(?i)\\s*(IF EXISTS|CASCADE).*", "") + .trim().toUpperCase(Locale.ROOT); + synchronized (_droppedTables) { + droppedTables(conf).add(tableName); + } + } else if (upper.startsWith("CREATE TABLE")) { + String tableName = sql.trim() + .substring("CREATE TABLE".length()).trim() + .split("\\s*\\(")[0].trim().toUpperCase(Locale.ROOT); + synchronized (_droppedTables) { + droppedTables(conf).remove(tableName); + } + } + } + + /** + * Whether the given table was dropped by a schema generation script on the + * database this configuration connects to, and must therefore not be + * created again. Outside spec compliant schema generation the entry is + * consumed, so it suppresses one create only. + */ + static boolean isDroppedTable(JDBCConfiguration conf, String tableName) { + synchronized (_droppedTables) { + java.util.Set dropped = droppedTables(conf); + return conf.isSpecCompliantSchemaGeneration() + ? dropped.contains(tableName) + : dropped.remove(tableName); + } + } + + /** + * Clear the tables tracked as dropped on the database the given + * configuration connects to. Called at the start of each schema + * generation flow to prevent cross-EMF contamination. + */ + public static void clearDroppedTables(JDBCConfiguration conf) { + synchronized (_droppedTables) { + _droppedTables.remove(trackingKey(conf)); + } + } + + /** + * Clear the dropped tables tracking for every database. + * + * @deprecated use {@link #clearDroppedTables(JDBCConfiguration)}, which + * does not discard the tracking of unrelated persistence units. + */ + @Deprecated public static void clearDroppedTables() { - _droppedTables.clear(); + synchronized (_droppedTables) { + _droppedTables.clear(); + } } protected final JDBCConfiguration _conf; @@ -1238,14 +1334,11 @@ protected void dropTables(Collection tables, SchemaGroup change) public boolean createTable(Table table) throws SQLException { String tableName = table.getFullIdentifier().getName().toUpperCase(Locale.ROOT); - if (_log.isTraceEnabled()) { - _log.trace("createTable: " + tableName + " action=" + _action - + " droppedTables=" + _droppedTables); - } - if (ACTION_ADD.equals(_action) - && (_conf.isSpecCompliantSchemaGeneration() - ? _droppedTables.contains(tableName) - : _droppedTables.remove(tableName))) { + if (ACTION_ADD.equals(_action) && isDroppedTable(_conf, tableName)) { + if (_log.isTraceEnabled()) { + _log.trace("createTable: " + tableName + " action=" + _action + + " was dropped by a schema generation script; not created"); + } return false; } return executeSQL(_dict.getCreateTableSQL(table, _db)); @@ -1517,19 +1610,7 @@ protected boolean executeSQL(String[] sql) statement.executeUpdate(s); // Track DROP/CREATE TABLE for drop-then-rebuild flows if (ACTION_EXECUTE_SCRIPT.equals(_action)) { - String upper = s.toUpperCase(Locale.ROOT).trim(); - if (upper.startsWith("DROP TABLE")) { - String tableName = s.trim() - .substring("DROP TABLE".length()).trim() - .replaceAll("(?i)\\s*(IF EXISTS|CASCADE).*", "") - .trim().toUpperCase(Locale.ROOT); - _droppedTables.add(tableName); - } else if (upper.startsWith("CREATE TABLE")) { - String tableName = s.trim() - .substring("CREATE TABLE".length()).trim() - .split("\\s*\\(")[0].trim().toUpperCase(Locale.ROOT); - _droppedTables.remove(tableName); - } + trackScriptDdl(_conf, s); } if (_log.isTraceEnabled()) { _log.trace("DDL executed successfully: " + s); diff --git a/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/schema/TestSchemaToolDroppedTables.java b/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/schema/TestSchemaToolDroppedTables.java new file mode 100644 index 0000000000..dfad635cf7 --- /dev/null +++ b/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/schema/TestSchemaToolDroppedTables.java @@ -0,0 +1,122 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.openjpa.jdbc.schema; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.apache.openjpa.jdbc.conf.JDBCConfiguration; +import org.apache.openjpa.jdbc.conf.JDBCConfigurationImpl; +import org.junit.After; +import org.junit.Test; + +/** + * The tables a schema generation script drops are tracked statically, because + * the tracking has to outlive the configuration that wrote it. It must + * therefore be partitioned, so that one persistence unit cannot consume or + * clear the entries of another. + */ +public class TestSchemaToolDroppedTables { + + @After + public void clearTracking() { + SchemaTool.clearDroppedTables(); + } + + private JDBCConfiguration conf(String id, String url) { + JDBCConfigurationImpl conf = new JDBCConfigurationImpl(); + conf.setId(id); + if (url != null) { + conf.setConnectionURL(url); + } + return conf; + } + + @Test + public void testKeyIsTheConfigurationId() { + assertEquals("unit-a", SchemaTool.trackingKey(conf("unit-a", "jdbc:h2:mem:x"))); + } + + @Test + public void testKeyFallsBackToTheConnectionWithoutAnId() { + assertEquals("jdbc:h2:mem:x", SchemaTool.trackingKey(conf(null, "jdbc:h2:mem:x"))); + } + + @Test + public void testKeyIsStableWithNeitherIdNorConnection() { + assertEquals(SchemaTool.trackingKey(conf(null, null)), + SchemaTool.trackingKey(conf(null, null))); + } + + @Test + public void testOnePersistenceUnitDoesNotSeeAnothersDrop() { + JDBCConfiguration a = conf("unit-a", "jdbc:h2:mem:a"); + JDBCConfiguration b = conf("unit-b", "jdbc:h2:mem:b"); + + SchemaTool.trackScriptDdl(a, "DROP TABLE FOO"); + + assertFalse("unit-b must not see unit-a's drop", + SchemaTool.isDroppedTable(b, "FOO")); + assertTrue("unit-a must see its own drop", + SchemaTool.isDroppedTable(a, "FOO")); + } + + @Test + public void testTwoFactoriesOfOneUnitShareTheTracking() { + // this is what lets a factory see what a closed generateSchema() + // factory dropped; the two are separate configuration instances + SchemaTool.trackScriptDdl(conf("unit-a", "jdbc:h2:mem:a"), "DROP TABLE FOO"); + + assertTrue(SchemaTool.isDroppedTable(conf("unit-a", "jdbc:h2:mem:a"), "FOO")); + } + + @Test + public void testClearingOneUnitLeavesAnotherIntact() { + JDBCConfiguration a = conf("unit-a", "jdbc:h2:mem:a"); + JDBCConfiguration b = conf("unit-b", "jdbc:h2:mem:b"); + SchemaTool.trackScriptDdl(a, "DROP TABLE FOO"); + SchemaTool.trackScriptDdl(b, "DROP TABLE FOO"); + + SchemaTool.clearDroppedTables(b); + + assertTrue("clearing unit-b must not discard unit-a's tracking", + SchemaTool.isDroppedTable(a, "FOO")); + assertFalse(SchemaTool.isDroppedTable(b, "FOO")); + } + + @Test + public void testCreatingTheTableAgainForgetsIt() { + JDBCConfiguration a = conf("unit-a", "jdbc:h2:mem:a"); + SchemaTool.trackScriptDdl(a, "DROP TABLE FOO"); + SchemaTool.trackScriptDdl(a, "CREATE TABLE FOO (ID INTEGER)"); + + assertFalse(SchemaTool.isDroppedTable(a, "FOO")); + } + + @Test + public void testEntryIsConsumedWhenNotSpecCompliant() { + JDBCConfiguration a = conf("unit-a", "jdbc:h2:mem:a"); + SchemaTool.trackScriptDdl(a, "DROP TABLE FOO"); + + assertTrue(SchemaTool.isDroppedTable(a, "FOO")); + assertFalse("the entry suppresses one create only", + SchemaTool.isDroppedTable(a, "FOO")); + } +}