rzo1 commented on code in PR #179:
URL: https://github.com/apache/openjpa/pull/179#discussion_r3926860704


##########
openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/schema/SchemaTool.java:
##########
@@ -108,15 +108,66 @@ 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(databaseKey(conf),
+            k -> new java.util.HashSet<>());
+    }
+
+    /**
+     * An identifier for the database a configuration connects to. Schema
+     * generation reads through the second data source, so its connection
+     * properties are preferred. Configurations that name no connection at all
+     * share one entry, which is the behaviour tracking had before it was
+     * partitioned.
+     */
+    private static String databaseKey(JDBCConfiguration conf) {
+        String[] candidates = new String[] {
+            conf.getConnectionFactory2Name(), conf.getConnection2URL(),

Review Comment:
   You are right, and I could not find such a proof either — a JNDI name or URL 
is not a reliable identity for a database. Switched to the configuration id as 
you suggest: PersistenceUnitInfoImpl:450 defaults `openjpa.Id` to the 
persistence-unit name, so that is the unit name for a JPA persistence unit, 
with the connection names kept only as a fallback for a configuration that has 
no id.
   
   That keeps the property the tracking depends on: two factories of the *same* 
unit still share entries, which is what lets a later factory see what a closed 
`Persistence.generateSchema()` factory dropped 
(`TestSchemaGenDrop.testDropViaGenerateSchema` pins this), while two units can 
no longer clear or consume each other's.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to