[OPENJPA-2985] Implement the mandatory JPA 3.2 methods that threw UnsupportedOperationException - #156
Open
rzo1 wants to merge 5 commits into
Open
[OPENJPA-2985] Implement the mandatory JPA 3.2 methods that threw UnsupportedOperationException#156rzo1 wants to merge 5 commits into
rzo1 wants to merge 5 commits into
Conversation
EntityManagerImpl.createQuery(TypedQueryReference<T>) no longer throws UnsupportedOperationException. It now: - calls assertNotCloseInvoked() first, so a closed EntityManager yields IllegalStateException; - rejects a null reference (and a null reference name) with IllegalArgumentException; - delegates to createNamedQuery(name, resultType) so the named query metadata (query string, hints, flush mode, max results, lock mode) is applied and the reference's result type becomes the query result class; an unknown query name propagates as ArgumentException, which extends IllegalArgumentException, matching the createNamedQuery contract; - applies the hints carried by the reference afterwards, so they take precedence over hints declared on the @NamedQuery itself; failures there go through translateException like the rest of the named query path. A null result type falls back to createNamedQuery(name) defensively. Tested by the new TestTypedQueryReference in org.apache.openjpa.persistence.query, which covers query creation and execution from a reference, the applied result class, reference hints surviving in getHints(), positional parameters of the named query, null reference, unknown query name and a closed EntityManager. All seven cases failed against the previous UnsupportedOperationException.
EntityManagerFactoryImpl.getNamedQueries(Class) no longer throws UnsupportedOperationException. It now forces loading of the persistent types (mirroring the private MetaDataRepository.resolveAll() that getQueryMetaData() uses, so a cold repository does not yield an empty result), iterates the query metadata and returns a freshly built map of TypedQueryReference for every named query whose declared result type is assignable to the requested type. The declared result type is QueryMetaData.getResultType(), falling back to getCandidateType(), which makes the predicate exactly "createNamedQuery(name, resultType) is legal". Named queries without any declared result type are never returned, for any type including Object.class. The returned map is a copy; each reference carries the query name, the result type and an unmodifiable copy of the query hints. A null result type is rejected with IllegalArgumentException and a closed factory raises the usual IllegalStateException. For this to be answerable from metadata, AnnotationPersistenceMetaDataParser.parseNamedQueries() now honours the JPA 3.2 @NamedQuery.resultClass() the same way parseNamedNativeQueries() already honours @NamedNativeQuery.resultClass(): a managed result class is stored as the candidate type, any other non-void class as the result type. This is the identical branch that QueryImpl.setResultClass() performs for createNamedQuery(name, resultClass), so behaviour matches an explicit result class at the call site; note that setting the candidate type also makes JPQLExpressionBuilder skip its own candidate inference, which is the same situation as today's createNamedQuery(name, X.class). XMLPersistenceMetaDataParser is deliberately untouched: the bundled orm_3_2.xsd.rsrc declares only a "name" attribute on <named-query>, so a "result-class" attribute would fail schema validation before reaching the parser. Refreshing the schema is left to a separate issue; until then XML-declared named queries have no declared result type and are therefore not returned by getNamedQueries(). New TypedQueryReferenceImpl is an immutable TypedQueryReference with equals/hashCode/toString. Tested by the new TestGetNamedQueries (with NamedQueryRefEntity, which declares named queries with an entity resultClass plus a hint, a scalar resultClass, no resultClass, and a native query with a resultClass): covers the parser change, entity/scalar/supertype lookups, exclusion of untyped queries, hints and their immutability, the returned map being a copy, null argument handling, a cold second factory, addNamedQuery(), and round-tripping a reference through EntityManager.createQuery().
…Option...) find(EntityGraph, Object, FindOption...) threw UnsupportedOperationException. It now looks up an instance of the root entity type of the given graph (EntityGraphImpl.getEntityType()) by primary key and applies the graph as a JPA 3.2 load graph: every attribute named by the graph and, recursively, by its subgraphs and key subgraphs is added to the fetch plan, while attributes outside the graph keep their declared fetch behaviour - the default fetch group is deliberately left in place, which is what distinguishes a load graph from a fetch graph. Details: - The graph is applied to a fetch plan pushed for the duration of the call only, so neither the entity manager's fetch plan nor its maximum fetch depth is affected afterwards. The maximum fetch depth is raised only when the graph is nested deeper than a currently configured finite depth; an infinite depth (-1) is never lowered. - Attributes are resolved through ClassMetaData/FieldMetaData and added by their full name (declaring type + field), so attributes inherited from a mapped superclass or a superclass entity are matched correctly. - A subgraph's declared class type is preferred, falling back to the declared type of the owning value's element/key, because for a plural attribute EntityGraphImpl derives the subgraph type from Attribute.getJavaType(), which is the collection type. - Recursion is bounded by a path scoped identity guard, so a graph that references itself terminates. - A null graph, a null primary key, a non-entity root type and a foreign EntityGraph implementation all raise IllegalArgumentException; the message for a foreign implementation mirrors EntityManagerFactoryImpl.addNamedEntityGraph. - The FindOption parsing of find(Class, Object, FindOption...) was extracted into a shared private helper so both overloads behave identically. The only observable delta on the existing overload is that an explicit null option array is now a no-op instead of an NPE, matching lock(Object, LockModeType, LockOption...). Tested by the new TestEntityGraphFind (10 tests) in the existing entitygraph test package; EGDepartment gained a lazy inverse collection so a graph has something to change. The tests assert the load state via the state manager's loaded bit set and pair every positive assertion with a fresh entity manager control that asserts the attribute is NOT loaded without the graph, covering a flat graph, a subgraph, a cyclic graph, find options, a missing row and the three IllegalArgumentException cases.
Contributor
|
Build need to be fixed :) |
The getNamedQueries(Class) implementation guarded its lazy metadata load with a dedicated "private final Object _namedQueriesLock = new Object()". EntityManagerFactoryImpl is serialized field by field (it implements OpenJPAEntityManagerFactory, which extends Serializable, and the class declares no writeObject/readObject), so that non-transient bare Object field made the whole factory unserializable: java.io.NotSerializableException: java.lang.Object. The targeted tests for the new API never serialize a factory, so they stayed green; CI runs the full suite and org.apache.openjpa.persistence.simple.TestSerializedFactory, which writes the EntityManagerFactory to an ObjectOutputStream, failed on all four jobs. The neighbouring _entityGraphs guard did not have this problem because it locks on a ConcurrentHashMap, which is itself serializable. Fix: use a java.util.concurrent.locks.ReentrantLock as the monitor, the same idiom AbstractBrokerFactory already uses for its internal lock in the kernel. ReentrantLock is Serializable and deserializes unlocked, so the field can stay final and non-transient, no serialization hooks are needed, and the double-checked initialization (volatile flag read outside, re-checked inside the lock, written last) is unchanged. No new test: TestSerializedFactory already reproduces the failure exactly and passes with the fix.
solomax
reviewed
Aug 20, 2026
| props.put(JPAProperties.LOCK_SCOPE, pls); | ||
| } else if (opt instanceof Timeout timeout) { | ||
| props.put(JPAProperties.LOCK_TIMEOUT, timeout.milliseconds()); | ||
| if (options != null) { |
Contributor
There was a problem hiding this comment.
Maybe this one can be flattened a bit with:
if (options == null) {
return mode;
}
? :)
solomax
reviewed
Aug 20, 2026
| throw new IllegalArgumentException("resultType is required"); | ||
| _name = name; | ||
| _resultType = resultType; | ||
| _hints = (hints == null || hints.isEmpty()) |
Contributor
There was a problem hiding this comment.
Suggested change
| _hints = (hints == null || hints.isEmpty()) | |
| _hints = (hints == null || hints.isEmpty()) ? Map.of() : Map.copyOf(hints) |
solomax
approved these changes
Aug 20, 2026
solomax
left a comment
solomax
left a comment
Contributor
There was a problem hiding this comment.
I've added couple suggestions :)
hopefully others can review as well :)
- EntityManagerImpl.parseFindOptions(): flatten the null check into an early return instead of wrapping the whole loop in "if (options != null)". - TypedQueryReferenceImpl: use Map.of() for the empty-hints branch. Deviation: the non-empty branch keeps unmodifiableMap(new LinkedHashMap<>(hints)) instead of Map.copyOf(hints), because Map.copyOf randomizes iteration order per JVM run while createQuery(TypedQueryReference) replays the hints in that order and OpenJPA has aliased hint keys (openjpa.FetchPlan.LockTimeout / jakarta.persistence.lock.timeout, likewise for QueryTimeout) where the last write wins, and because Map.copyOf rejects null keys and values that this public constructor has always tolerated. TestGetNamedQueries now pins the order.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the three mandatory JPA 3.2 methods that were still throwing
UnsupportedOperationException("Not yet implemented (JPA 3.2)"), one commit each with tests:EntityManager.createQuery(TypedQueryReference),EntityManagerFactory.getNamedQueries(Class)andEntityManager.find(EntityGraph, Object, FindOption...).createQuery(TypedQueryReference)delegates tocreateNamedQuery(name, resultType)and applies the reference's hints.getNamedQueries(Class)needed metadata support first, soAnnotationPersistenceMetaDataParsernow honours the JPA 3.2@NamedQuery.resultClass()the same way it already honoured@NamedNativeQuery.resultClass(); the method then returns a fresh map ofTypedQueryReferencefor every named query whose declared result type is assignable to the requested one.find(EntityGraph, ...)resolves the root type from the graph and applies it as a load graph to the pushed fetch plan (with a cycle guard), which is the first place anEntityGraphactually influences loading in OpenJPA — until nowEntityGraphImplwas a pure data holder.Two known gaps worth a reviewer's opinion: named queries without a declared result type are excluded from
getNamedQueriesfor every requested type includingObject.class, and because the bundledorm_3_2.xsd.rsrcstill carries the 3.1-shapednamed-querytype withoutresult-class, XML-declared named queries can never be returned — that stale schema looks like a separate follow-up.