[OPENJPA-2963] Track dropped tables per database, not per JVM

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) {

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<String> _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<String, java.util.Set<String>> _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<String> 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 <code>openjpa.Id</code> 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.
+     * <p>
+     * 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<String> 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<Table> 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);

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"));
+    }
+}