[
https://issues.apache.org/jira/browse/TOMEE-4650?focusedWorklogId=1032356&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1032356
]
ASF GitHub Bot logged work on TOMEE-4650:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 27/Jul/26 08:13
Start Date: 27/Jul/26 08:13
Worklog Time Spent: 10m
Work Description: rzo1 commented on PR #2847:
URL: https://github.com/apache/tomee/pull/2847#issuecomment-5088905089
Could you split this? There are two unrelated changes here and they're in
very different states.
**Part 1 — the `ReloadableEntityManagerFactory.close()` guard — I'd merge
today.** It's small,
correct, uses the non-lazy `delegate` field rather than `delegate()` so it
doesn't force
initialisation, and the test genuinely fails without it. That's the part
that fixes the reported
undeploy noise.
**Part 2 — `JpaCDIExtension` — needs rework.** It's modelled on
`ConcurrencyCDIExtension`, which is
the right reference, but several of the guards that make that class safe
were dropped in the copy.
Blocking:
- For every PU without `<qualifier>`, `validateAndCreateQualifiers` returns
`{@Any, @Default}` and
`registerBeans` adds `EntityManagerFactory` and `EntityManager` beans with
those qualifiers, with
no `getBeans()` check. Beans added via `AfterBeanDiscovery.addBean()` are
ordinary enabled beans,
not built-ins, so nothing prefers an application producer over them —
that's an
`AmbiguousResolutionException` against the `@Produces EntityManager`
pattern, which is about as
common as CDI/JPA patterns get.
Two tests in this very module should now fail deployment:
`ProducedExtendedEmTest`
(`EntityManagerProducer.produceEm()` + `@Inject EntityManager` in `A`, PU
`cdi-em-extended`) and
`ResourceLocalCdiEmTest` (`EMFProducer.em()` + `@Inject EntityManager` in
`PersistManager`, PU
`rl-unit`). Neither is `@Ignore`d, and the extension does run in them —
`OptimizedLoaderService.loadExtensions` adds it unconditionally at :130
and `OpenEJBLifecycle`
sets `CURRENT_APP_INFO` at :190 before `deployer.deploy()`.
`ConcurrencyCDIExtension.registerDefaultBeanIfMissing` (:359) is exactly
the guard that's missing —
it takes the `BeanManager` and skips when `beanManager.getBeans(type,
Default.Literal.INSTANCE)`
is non-empty. `OpenEJBLifecycle.addInternalBeans` (:244-258) uses the same
idiom.
- The loop `for (final PersistenceUnitInfo unitInfo :
appInfo.persistenceUnits)` has no dedup of
qualifier sets and no filter on `unitInfo.webappName`. So two unqualified
PUs in one app register
duplicate `@Default` beans, and in an EAR a webapp gets beans for its
siblings' PUs —
`TomcatWebAppBuilder:1454` sets `CURRENT_APP_INFO` to the whole EAR's
`AppInfo` in the per-webapp
`!webAppAlone` branch, while the same method correctly filters on
`unitInfo.webappName` at :1425
for EMF creation. `ConcurrencyCDIExtension` scopes this with
`isVisibleInCurrentApp(resource, currentAppIds)`
(:102); there's no equivalent here.
- The annotation proxy breaks the `Annotation` equals/hashCode contract:
`equals` is
`annotationType.isInstance(args[0])` and `hashCode` is
`annotationType.hashCode()`. The spec
mandates 0 for a marker annotation, and `equals` ignoring member values
makes it asymmetric with a
real annotation instance — so a qualifier with members can select the
wrong PU. The correct
implementation is ~150 lines away in `ConcurrencyCDIExtension`
(`annotationEquals` :235,
`annotationHashCode` :254, `annotationToString`) and was replaced here by
two one-liners. Please
reuse it rather than reimplementing.
Related omission: `ConcurrencyCDIExtension.validateAndCreateQualifiers`
(:184-195) rejects
qualifiers with members lacking defaults and members lacking
`@Nonbinding`, via two
`addDefinitionError` calls. `JpaCDIExtension.validateAndCreateQualifiers`
stops at the `@Qualifier`
check. So `@Qualifier @interface Unit { String value(); }` gives
`getDefaultValue() == null`, the
proxy returns null from `value()`, and you get either an NPE inside OWB or
a bean that can never
be matched — with no diagnostic. The javadoc on `createAnnotation` asserts
"the CDI qualifier
rules guarantee [a default] to exist"; that guarantee doesn't exist.
Also:
- `jakarta.persistence.qualifiers` and `jakarta.persistence.scope` aren't
spec properties. I unpacked
`jakarta.persistence-api-3.2.0.jar` — neither string appears anywhere in
the jar, and
`PersistenceConfiguration` declares constants for every standard override
property (JDBC_*,
LOCK_TIMEOUT, QUERY_TIMEOUT, SCHEMAGEN_*, VALIDATION_*, CACHE_MODE) with
nothing for these two.
`persistence_3_2.xsd` defines only the `<qualifier>`/`<scope>` elements.
Please don't mint new
property names under the `jakarta.*` namespace — `openejb.*` is the right
prefix for a
TomEE-specific carrier.
- The `SchemaManager` bean can only ever inject null: `addUtilityBean(...,
SchemaManager.class, EntityManagerFactory::getSchemaManager)`
on the reloadable EMF. Either wire it properly or drop it.
- The `jakarta.transaction`-absent fallback is unreachable — openejb-core
has a hard dependency on it
(`cdi/transactional/TransactionContext` imports
`TransactionManager`/`TransactionScoped` directly
and extends `AbstractContext(TransactionScoped.class)`); the module can't
load without it. Its only
possible effect is a silent scope change, and the javadoc describes
reflective member access that
isn't happening.
- `<scope>` isn't validated as an actual CDI scope, unlike the `<qualifier>`
right above it. A typo
there fails much later and much less clearly.
- The JNDI prefix is hardcoded rather than using
`JndiConstants.PERSISTENCE_UNIT_NAMING_CONTEXT`.
- `qualifierSelectsTheMatchingPersistenceUnit` is tautological — it would
pass against a stub.
Question rather than a finding: `produceWith(instance ->
lookupEntityManagerFactory(unitInfo.id).createEntityManager())`
hands out the provider EM unwrapped by `JtaEntityManager`, so it bypasses
TomEE's JTA integration —
and `resolveEntityManagerScope` never consults `unitInfo.transactionType`
(populated at
`AppInfoBuilder:685`), so a RESOURCE_LOCAL unit gets `TransactionScoped`.
`TransactionContext.isActive()`
(:50-60) returns false with no JTA transaction, so that EM is unusable
outside one. Is
`TransactionScoped` mandated unconditionally by the platform spec here, with
apps expected to declare
`<scope>` for RESOURCE_LOCAL? The 3.2 XSD (:117) documents `<scope>` with no
stated default, so I
couldn't settle it from the schema alone.
Low-priority, only reachable via the JPA JMX operations: the `@Dependent`
`CriteriaBuilder`/`Metamodel`/`Cache`/`PersistenceUnitUtil` beans call the
accessor once at
instantiation, so they capture the *current* delegate.
`ReloadableEntityManagerFactory.reload()`
(:383-389) replaces the delegate without closing the old one, so after a
reload an
`@ApplicationScoped` bean holds a `CriteriaBuilder` bound to the superseded
EMF while
`@Inject EntityManagerFactory` follows the new one. Resolving through the
EMF per call, or a
documented limitation, would avoid the inconsistency.
Issue Time Tracking
-------------------
Worklog Id: (was: 1032356)
Time Spent: 20m (was: 10m)
> Undeploy closes an already-closed EntityManagerFactory; PU CDI qualifier
> beans missing
> --------------------------------------------------------------------------------------
>
> Key: TOMEE-4650
> URL: https://issues.apache.org/jira/browse/TOMEE-4650
> Project: TomEE
> Issue Type: Bug
> Reporter: Markus Jung
> Assignee: Markus Jung
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> When a test closes a container-managed {{EntityManagerFactory}} (EMF) itself,
> TomEE's undeploy path calls {{close()}} on it again.
> {{Assembler.destroyApplication}} then fails with "Attempting to execute an
> operation on a closed EntityManagerFactory". The test method itself passes;
> only the undeploy step after it errors. TomEE must check whether the EMF is
> already closed before calling {{close()}} on it during undeploy.
> Separately, TomEE does not register the CDI qualifier beans that Jakarta
> Persistence 3.2 requires for {{persistence.xml}}-declared units. When an app
> injects {{EntityManagerFactory}}, {{EntityManager}}, or
> {{PersistenceUnitUtil}} with a qualifier such as {{@CtsEm2Qualifier}},
> deployment fails with {{UnsatisfiedResolutionException}}. TomEE is missing
> this part of the Jakarta Persistence 3.2 CDI integration.
> Both problems show up on Plume (EclipseLink) and on the webprofile
> distribution (OpenJPA) alike, so neither is a persistence provider defect.
> h2. Steps to reproduce / TCK reference
> *
> {{ee.jakarta.tck.persistence.core.entityManagerFactoryCloseExceptions.ClientPmservletTest}}
> and {{ClientPuservletTest}} — excluded in
> {{runner-webprofile/exclusions/persistence-javatest.txt}} in the
> apache/tomee-tck harness repo. The {{exceptionsTest}} methods pass; the class
> reports an undeploy error.
> * {{ee.jakarta.tck.persistence.ee.cdi.ServletEMLookupTest}} — excluded in
> {{runner-webprofile/exclusions/persistence-servlet.txt}}. Deployment fails
> with {{UnsatisfiedResolutionException}} for {{@CtsEm2Qualifier}}.
> Remove the matching lines from both exclusion files once fixed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
