You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openjpa.apache.org by rz...@apache.org on 2026/09/02 19:18:27 UTC
(openjpa) branch OPENJPA-2962 created (now e2ecff51f)
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a change to branch OPENJPA-2962
in repository https://gitbox.apache.org/repos/asf/openjpa.git
at e2ecff51f [OPENJPA-2962] Do not create a broker to obtain a class loader
This branch includes the following new commits:
new e2ecff51f [OPENJPA-2962] Do not create a broker to obtain a class loader
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(openjpa) 01/01: [OPENJPA-2962] Do not create a broker to obtain a class loader
Posted by rz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch OPENJPA-2962
in repository https://gitbox.apache.org/repos/asf/openjpa.git
commit e2ecff51f6c1c889bdce5a342ca3e1be698fce04
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Wed Sep 2 20:42:15 2026 +0200
[OPENJPA-2962] Do not create a broker to obtain a class loader
The four schema management methods each built a Broker through
newBrokerImpl() only to call getClassLoader() on it. That loader was always
null: BrokerImpl assigns it in initialize(), which newBrokerImpl() does not
reach, so the broker answered a question it could not answer and was then
dropped. They now pass the thread context class loader, which is the value
initialize() would have assigned and which postCreationCallback() already
passes for the same synchronizeMappings() calls.
Closing the broker instead was not an option: close() runs free(), which
dereferences a cache that an uninitialized broker has not got, so it throws.
Nothing heavyweight leaked, though, since without initialize() there is no
store manager, no connection and no registration with the factory; what the
call cost was a finalizable object per invocation whose finalizer threw and
was discarded.
The only behaviour that changes is that a class loader now reaches
MappingTool, so a custom openjpa.Sequence class resolves where it
previously fell back to the bootstrap loader.
The ticket also names dropPersistenceStrucuture and
validatePersistenceStruture; cbb19f777 has since corrected both spellings.
---
.../openjpa/jdbc/kernel/JDBCBrokerFactory.java | 31 +++--
.../kernel/TestJDBCBrokerFactorySchemaSpi.java | 135 +++++++++++++++++++++
2 files changed, 157 insertions(+), 9 deletions(-)
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 974f32686..fdaa546a6 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
@@ -40,7 +40,6 @@ import org.apache.openjpa.jdbc.meta.MappingTool;
import org.apache.openjpa.jdbc.schema.SchemaTool;
import org.apache.openjpa.kernel.AbstractBrokerFactory;
import org.apache.openjpa.kernel.Bootstrap;
-import org.apache.openjpa.kernel.Broker;
import org.apache.openjpa.kernel.BrokerImpl;
import org.apache.openjpa.kernel.StoreManager;
import org.apache.openjpa.lib.conf.ConfigurationProvider;
@@ -160,35 +159,49 @@ public class JDBCBrokerFactory extends AbstractBrokerFactory {
}
}
+ /**
+ * Return the class loader for the {@link org.apache.openjpa.kernel.BrokerFactory}
+ * schema management operations, which are not associated with any broker.
+ *
+ * This is the thread context class loader, which is what
+ * {@code BrokerImpl.initialize()} assigns to the loader a broker later reports from
+ * {@code Broker#getClassLoader()}, and what {@link #postCreationCallback()} already
+ * passes here. No broker is created for this: a broker from {@code newBrokerImpl()}
+ * has not been initialized, so its class loader is still null and it supplies no
+ * loader at all (OPENJPA-2962).
+ */
+ private static ClassLoader schemaManagementClassLoader() {
+ return Thread.currentThread().getContextClassLoader();
+ }
+
@Override
public void createPersistenceStructure(boolean createSchemas) {
JDBCConfiguration conf = (JDBCConfiguration) getConfiguration();
- Broker broker = super.newBrokerImpl(conf.getConnectionUserName(), conf.getConnectionPassword());
String baseAction = createSchemas ? "createDB, add": MappingTool.ACTION_ADD;
- synchronizeMappings(broker.getClassLoader(), conf, String.format("buildSchema(ForeignKeys=true,schemaAction='%s')", baseAction));
+ synchronizeMappings(schemaManagementClassLoader(), conf,
+ String.format("buildSchema(ForeignKeys=true,schemaAction='%s')", baseAction));
}
@Override
public void dropPersistenceStructure(boolean dropSchemas) {
JDBCConfiguration conf = (JDBCConfiguration) getConfiguration();
- Broker broker = super.newBrokerImpl(conf.getConnectionUserName(), conf.getConnectionPassword());
String baseAction = dropSchemas ? "drop, dropDB": MappingTool.ACTION_DROP;
- synchronizeMappings(broker.getClassLoader(), conf, String.format("buildSchema(ForeignKeys=true,schemaAction='%s')", baseAction));
+ synchronizeMappings(schemaManagementClassLoader(), conf,
+ String.format("buildSchema(ForeignKeys=true,schemaAction='%s')", baseAction));
}
@Override
public void validatePersistenceStructure() throws Exception {
JDBCConfiguration conf = (JDBCConfiguration) getConfiguration();
- Broker broker = super.newBrokerImpl(conf.getConnectionUserName(), conf.getConnectionPassword());
- synchronizeMappings(broker.getClassLoader(), conf, "validate(ForeignKeys=true)");
+ synchronizeMappings(schemaManagementClassLoader(), conf, "validate(ForeignKeys=true)");
}
@Override
public void truncateData() {
JDBCConfiguration conf = (JDBCConfiguration) getConfiguration();
- Broker broker = super.newBrokerImpl(conf.getConnectionUserName(), conf.getConnectionPassword());
String baseAction = "refresh,deleteTableContents";
- synchronizeMappings(broker.getClassLoader(), conf, String.format("buildSchema(ForeignKeys=true,schemaAction='%s')", baseAction));
+ synchronizeMappings(schemaManagementClassLoader(), conf,
+ String.format("buildSchema(ForeignKeys=true,schemaAction='%s')", baseAction));
}
protected boolean synchronizeMappings(ClassLoader loader, JDBCConfiguration conf) {
diff --git a/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestJDBCBrokerFactorySchemaSpi.java b/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestJDBCBrokerFactorySchemaSpi.java
new file mode 100644
index 000000000..8be328844
--- /dev/null
+++ b/openjpa-jdbc/src/test/java/org/apache/openjpa/jdbc/kernel/TestJDBCBrokerFactorySchemaSpi.java
@@ -0,0 +1,135 @@
+/*
+ * 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.kernel;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
+import org.apache.openjpa.jdbc.conf.JDBCConfigurationImpl;
+import org.apache.openjpa.kernel.BrokerImpl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Verifies that the {@code BrokerFactory} schema management operations do not
+ * instantiate a {@link BrokerImpl} and that they pass the thread context class
+ * loader on to the mapping tool (OPENJPA-2962).
+ *
+ * This test needs no database: {@code synchronizeMappings} is stubbed out.
+ */
+public class TestJDBCBrokerFactorySchemaSpi {
+
+ static final AtomicInteger CLONES = new AtomicInteger();
+
+ /**
+ * Plugin class for {@code openjpa.BrokerImpl}. The factory obtains brokers by
+ * cloning the configured template instance, so counting clones counts the
+ * brokers a schema operation creates.
+ */
+ public static class CountingBroker extends BrokerImpl {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public Object clone() throws CloneNotSupportedException {
+ CLONES.incrementAndGet();
+ return super.clone();
+ }
+ }
+
+ /**
+ * Captures the arguments handed to the mapping synchronization instead of
+ * running it against a database.
+ */
+ static class CapturingFactory extends JDBCBrokerFactory {
+ private static final long serialVersionUID = 1L;
+
+ ClassLoader loader;
+ String action;
+ int calls;
+
+ CapturingFactory(JDBCConfiguration conf) {
+ super(conf);
+ }
+
+ @Override
+ protected boolean synchronizeMappings(ClassLoader loader, JDBCConfiguration conf, String action) {
+ this.loader = loader;
+ this.action = action;
+ this.calls++;
+ return false;
+ }
+ }
+
+ private CapturingFactory factory;
+
+ @Before
+ public void setUp() {
+ JDBCConfigurationImpl conf = new JDBCConfigurationImpl();
+ conf.setBrokerImpl(CountingBroker.class.getName());
+ CLONES.set(0);
+ factory = new CapturingFactory(conf);
+ }
+
+ private void assertNoBroker(String expectedAction) {
+ assertEquals("schema management must not instantiate a Broker", 0, CLONES.get());
+ assertEquals(1, factory.calls);
+ assertSame(Thread.currentThread().getContextClassLoader(), factory.loader);
+ assertEquals(expectedAction, factory.action);
+ }
+
+ @Test
+ public void testCreatePersistenceStructure() {
+ factory.createPersistenceStructure(false);
+ assertNoBroker("buildSchema(ForeignKeys=true,schemaAction='add')");
+ }
+
+ @Test
+ public void testCreatePersistenceStructureWithSchemas() {
+ factory.createPersistenceStructure(true);
+ assertNoBroker("buildSchema(ForeignKeys=true,schemaAction='createDB, add')");
+ }
+
+ @Test
+ public void testDropPersistenceStructure() {
+ factory.dropPersistenceStructure(false);
+ assertNoBroker("buildSchema(ForeignKeys=true,schemaAction='drop')");
+ }
+
+ @Test
+ public void testDropPersistenceStructureWithSchemas() {
+ factory.dropPersistenceStructure(true);
+ assertNoBroker("buildSchema(ForeignKeys=true,schemaAction='drop, dropDB')");
+ }
+
+ @Test
+ public void testValidatePersistenceStructure() throws Exception {
+ factory.validatePersistenceStructure();
+ assertNoBroker("validate(ForeignKeys=true)");
+ }
+
+ @Test
+ public void testTruncateData() {
+ factory.truncateData();
+ assertNoBroker("buildSchema(ForeignKeys=true,schemaAction='refresh,deleteTableContents')");
+ }
+}