[OPENJPA-2978] Fix SchemaManagerImpl nits and unsupported-validation reporting - #155
[OPENJPA-2978] Fix SchemaManagerImpl nits and unsupported-validation reporting#155rzo1 wants to merge 2 commits into
Conversation
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.
…ation 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.
| @Override | ||
| public void createPersistenceStructure(boolean createSchemas) { | ||
| throw new UnsupportedOperationException(); | ||
| throw new UnsupportedOperationException("This BrokerFactory does not implement schema creation."); |
There was a problem hiding this comment.
this class is abstract maybe these methods need to be just removed?
There was a problem hiding this comment.
These can't just be removed: AbstractStoreBrokerFactory (openjpa-kernel/src/main/java/org/apache/openjpa/abstractstore/AbstractStoreBrokerFactory.java:51) is a concrete main-code subclass that implements none of the four, as is the BrokerFactory stub in TestPersistenceManagerFactoryImpl, so both would stop compiling and we'd end up pasting the same four throws into AbstractStoreBrokerFactory rather than deleting them. The methods are also new on the interface (@SInCE 4.2.0, unreleased), so any out-of-tree factory extending AbstractBrokerFactory compiles today only because these defaults exist - dropping them turns a clear UnsupportedOperationException into an AbstractMethodError at getSchemaManager() time. I'd rather keep one default here than duplicate it per subclass; this commit only makes the exception say which operation is missing. If you'd still prefer compile-time enforcement, that's a separate change that also has to decide what AbstractStoreBrokerFactory does for schema operations.
2 participants
Cleans up the reported nits in
SchemaManagerImpl(stray double semicolon, redundant(Exception)cast, javadoc typo) and stopsvalidate()from wrapping anUnsupportedOperationExceptionintoSchemaValidationException("Schema could not be validated: null")— a store that does not implement validation now reports a missing capability, consistently withcreate(),drop()andtruncate(). The message-lessUnsupportedOperationExceptions inAbstractBrokerFactorygot real messages, and a newTestSchemaManagerImplcovers both the unsupported and the real-failure path.The
Strucuture/Struturetypos from the review comment are already fixed on master (commit cbb19f7, "[OPENJPA-2940] Fixes typos in BrokerFactory"), so no rename was needed.