From 8719892cdadd22aa2b9d614c4a428b9decea8ec6 Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Thu, 20 Aug 2026 12:36:48 +0200 Subject: [PATCH 1/2] [OPENJPA-2978] Fix typos and stray semicolon in SchemaManagerImpl Removes the stray second semicolon in truncate(), drops the redundant (Exception) cast when constructing SchemaValidationException, and corrects the 'concretelly' javadoc typo. The Strucuture/Struture method name typos reported in review were already corrected on master and need no change. --- .../org/apache/openjpa/persistence/SchemaManagerImpl.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/SchemaManagerImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/SchemaManagerImpl.java index bbf6c78c8..0d3b6dad2 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/SchemaManagerImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/SchemaManagerImpl.java @@ -25,7 +25,7 @@ /** * Implements a no-op SchemaManager object that will throw - * UnsupportedOperationException if not concretelly implemented + * UnsupportedOperationException if not concretely implemented * by the given persistence layer. * * @author Paulo Cristovão Filho @@ -54,13 +54,12 @@ public void validate() throws SchemaValidationException { _factory.validatePersistenceStructure(); } catch (Exception ex) { throw new SchemaValidationException( - String.format("Schema could not be validated: %s", ex.getLocalizedMessage()), - (Exception) ex); + String.format("Schema could not be validated: %s", ex.getLocalizedMessage()), ex); } } @Override public void truncate() { - _factory.truncateData();; + _factory.truncateData(); } } From d5143a43156dd830c6f8226ba0f76f40f944e146 Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Thu, 20 Aug 2026 12:40:09 +0200 Subject: [PATCH 2/2] [OPENJPA-2978] Do not report unsupported schema validation as a validation failure SchemaManagerImpl.validate() caught every Exception and rewrapped it into a SchemaValidationException. A store which does not implement schema validation throws UnsupportedOperationException from AbstractBrokerFactory, so a missing capability was reported to the caller as SchemaValidationException("Schema could not be validated: null") - which per the jakarta.persistence 3.2 javadoc asserts that a database object is missing or has an unexpected definition, although nothing was ever inspected. validate() now lets UnsupportedOperationException reach the caller unchanged, the same way create(), drop() and truncate() already do, and keeps wrapping every other failure (MetaDataException / IllegalStateException from the JDBC path) in SchemaValidationException. The wrapped message falls back to the exception class name when the exception carries no message, so it can never read "...: null" again. For consistency the four unsupported operations in AbstractBrokerFactory now carry a message instead of being thrown bare, which improves the create, drop and truncate paths as well. Adds TestSchemaManagerImpl, covering the unsupported case for all four operations, the wrapping of a real validation failure and the message fallback. --- .../openjpa/kernel/AbstractBrokerFactory.java | 8 +- .../persistence/SchemaManagerImpl.java | 11 +- .../persistence/TestSchemaManagerImpl.java | 140 ++++++++++++++++++ 3 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 openjpa-persistence/src/test/java/org/apache/openjpa/persistence/TestSchemaManagerImpl.java diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java index f8fbff089..d7ca65848 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/AbstractBrokerFactory.java @@ -475,22 +475,22 @@ public void unlock() { @Override public void createPersistenceStructure(boolean createSchemas) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("This BrokerFactory does not implement schema creation."); } @Override public void dropPersistenceStructure(boolean dropSchemas) { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("This BrokerFactory does not implement schema dropping."); } @Override public void validatePersistenceStructure() throws Exception { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("This BrokerFactory does not implement schema validation."); } @Override public void truncateData() { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException("This BrokerFactory does not implement data truncation."); } /** diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/SchemaManagerImpl.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/SchemaManagerImpl.java index 0d3b6dad2..2d61446a2 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/SchemaManagerImpl.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/SchemaManagerImpl.java @@ -52,12 +52,21 @@ public void drop(boolean dropSchemas) { public void validate() throws SchemaValidationException { try { _factory.validatePersistenceStructure(); + } catch (UnsupportedOperationException uoe) { + // the store does not implement schema validation at all. That is a missing capability, + // not a validation failure, so report it the same way create(), drop() and truncate() do. + throw uoe; } catch (Exception ex) { throw new SchemaValidationException( - String.format("Schema could not be validated: %s", ex.getLocalizedMessage()), ex); + String.format("Schema could not be validated: %s", describe(ex)), ex); } } + private static String describe(Exception ex) { + String message = ex.getLocalizedMessage(); + return message != null ? message : ex.getClass().getName(); + } + @Override public void truncate() { _factory.truncateData(); diff --git a/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/TestSchemaManagerImpl.java b/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/TestSchemaManagerImpl.java new file mode 100644 index 000000000..2e9444a01 --- /dev/null +++ b/openjpa-persistence/src/test/java/org/apache/openjpa/persistence/TestSchemaManagerImpl.java @@ -0,0 +1,140 @@ +/* + * 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.persistence; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import org.apache.openjpa.conf.OpenJPAConfigurationImpl; +import org.apache.openjpa.kernel.AbstractBrokerFactory; +import org.apache.openjpa.kernel.StoreManager; +import org.junit.Test; + +import jakarta.persistence.SchemaManager; +import jakarta.persistence.SchemaValidationException; + +/** + * Tests that {@link SchemaManagerImpl} reports a store which does not implement the + * schema operations as an unsupported operation rather than as a schema failure. + */ +public class TestSchemaManagerImpl { + + /** + * A minimal, non-JDBC {@link AbstractBrokerFactory} which inherits the default + * (unsupported) implementations of the four schema operations. + */ + private static final class NoSchemaBrokerFactory extends AbstractBrokerFactory { + private static final long serialVersionUID = 1L; + + private NoSchemaBrokerFactory() { + super(new OpenJPAConfigurationImpl()); + } + + @Override + protected StoreManager newStoreManager() { + return null; + } + } + + /** + * A factory whose validation fails the way a real store reports schema drift. + */ + private static final class FailingBrokerFactory extends AbstractBrokerFactory { + private static final long serialVersionUID = 1L; + + static final IllegalStateException FAILURE = new IllegalStateException("column FOO is missing"); + + private FailingBrokerFactory() { + super(new OpenJPAConfigurationImpl()); + } + + @Override + public void validatePersistenceStructure() throws Exception { + throw FAILURE; + } + + @Override + protected StoreManager newStoreManager() { + return null; + } + } + + /** + * A factory whose validation fails without carrying any message at all. + */ + private static final class SilentlyFailingBrokerFactory extends AbstractBrokerFactory { + private static final long serialVersionUID = 1L; + + private SilentlyFailingBrokerFactory() { + super(new OpenJPAConfigurationImpl()); + } + + @Override + public void validatePersistenceStructure() throws Exception { + throw new IllegalStateException(); + } + + @Override + protected StoreManager newStoreManager() { + return null; + } + } + + @Test + public void testValidateReportsUnsupportedOperationInsteadOfValidationFailure() { + SchemaManager schemaManager = new SchemaManagerImpl(new NoSchemaBrokerFactory()); + + UnsupportedOperationException uoe = + assertThrows(UnsupportedOperationException.class, schemaManager::validate); + assertNotNull("the unsupported operation must carry a message", uoe.getMessage()); + assertTrue(uoe.getMessage(), uoe.getMessage().contains("schema validation")); + } + + @Test + public void testCreateDropAndTruncateReportUnsupportedOperation() { + SchemaManager schemaManager = new SchemaManagerImpl(new NoSchemaBrokerFactory()); + + assertThrows(UnsupportedOperationException.class, () -> schemaManager.create(true)); + assertThrows(UnsupportedOperationException.class, () -> schemaManager.drop(true)); + assertThrows(UnsupportedOperationException.class, schemaManager::truncate); + } + + @Test + public void testValidateWrapsRealValidationFailures() { + SchemaManager schemaManager = new SchemaManagerImpl(new FailingBrokerFactory()); + + SchemaValidationException sve = + assertThrows(SchemaValidationException.class, schemaManager::validate); + assertEquals("Schema could not be validated: column FOO is missing", sve.getMessage()); + assertEquals(1, sve.getFailures().length); + assertSame(FailingBrokerFactory.FAILURE, sve.getFailures()[0]); + } + + @Test + public void testValidateNeverReportsANullMessage() { + SchemaManager schemaManager = new SchemaManagerImpl(new SilentlyFailingBrokerFactory()); + + SchemaValidationException sve = + assertThrows(SchemaValidationException.class, schemaManager::validate); + assertEquals("Schema could not be validated: java.lang.IllegalStateException", sve.getMessage()); + } +}