jungm commented on PR #2847:
URL: https://github.com/apache/tomee/pull/2847#issuecomment-5144026339

   Thanks — split done, and you were right on the blocking point.
   
   I reproduced the `AmbiguousResolutionException` before touching anything: 
with the extension in place, `ProducedExtendedEmTest` fails deployment with 
`There is more than one Bean with type jakarta.persistence.EntityManager 
Qualifiers: [@Default]`. That's a regression I should have caught, and it's the 
clearest argument for splitting.
   
   **This PR** is now only the `close()` guard — two lines plus its test. 
`ProducedExtendedEmTest` and `ResourceLocalCdiEmTest` are green again.
   
   **The extension** is preserved on `claude/tomee-4650-jpa-cdi-extension` and 
will return as its own PR. Taking your points: the missing 
`registerDefaultBeanIfMissing`-style `getBeans()` guard, 
`isVisibleInCurrentApp`-style app scoping plus the `webappName` filter and 
qualifier-set dedup, reusing `ConcurrencyCDIExtension`'s 
`annotationEquals`/`annotationHashCode` rather than my one-liners, and its full 
`validateAndCreateQualifiers` including the defaults/`@Nonbinding` checks.
   
   On the smaller items — you're right that 
`jakarta.persistence.qualifiers`/`.scope` aren't spec properties. I took them 
from the Platform spec's "Additional EntityManagerFactory Properties" table in 
`CDI-JPA.adoc`, but that's a platform-spec table, not something the API jar 
defines, and minting `jakarta.*` names was the wrong call regardless; I'll move 
them to `openejb.*`. Also agreed on `JndiConstants`, dropping the unreachable 
`jakarta.transaction` fallback, validating `<scope>`, the dead `SchemaManager` 
wiring, and that `qualifierSelectsTheMatchingPersistenceUnit` is tautological.
   
   On your question about `TransactionScoped` and RESOURCE_LOCAL: the platform 
spec states the `EntityManager` bean's scope is the `<scope>` element "or 
`jakarta.transaction.TransactionScoped` if no scope is explicitly specified", 
with no carve-out for RESOURCE_LOCAL — so unconditional as written. But that 
yields an unusable bean for a RESOURCE_LOCAL unit, which reads like a spec gap 
rather than an intended outcome. I'll consult `unitInfo.transactionType` and 
raise it upstream rather than guess. The unwrapped-EM point stands too — it 
should go through `JtaEntityManager`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to