Skip to content

Improved support for OpenJPA persistence units. #8370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tehnic-take3
Copy link

Adding better creating/editing support for OpenJPA persistence units.


^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

  1. Was this PR correctly labeled, did the right tests run? When did they run?
  2. Is this PR squashed?
  3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
  4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

@matthiasblaesing matthiasblaesing added Java EE/Jakarta EE [ci] enable enterprise job enterprise [ci] enable enterprise job labels Mar 29, 2025
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking care. In general the update looks like the right idea. However I suggest to adjust the logic:

I think what was meant originally was: If there is no version present or the target version is below JPA 2.1 use the legacy/openjpa specific statements, else usw the standard ones.

So what do you think about this suggestion:

  1. Update the superclass implementation for getTableGenerationPropertyName , getTableGenerationDropCreateValue and getTableGenerationCreateValue in org.netbeans.modules.j2ee.persistence.provider.Provider to use the isJakartaNamespace method and return the correct value. That way you not only fix OpenJPA, but any JPA provider.
  2. Simplify the three methods in OpenJPAProvider to only handle the special cases where version is null, Persistence.VERSION_2_0 or Persistence.VERSION_1_0 with the OpenJPA specific code. Everything else can be delegated to the superclass.

@tehnic-take3
Copy link
Author

I agree with your comments, essentially i only added 3.0 and 3.1 to existing OpenJPA implementation without any refactoring.

Before i touch the superclass i need to review the unit tests to avoid any breaking changes.

Anyway i found another point i need to understand/solve:

In ui/resources/persistence/persinstence-3.1.xml:

xsi:schemaLocation="https://jakarta.ee/xml/ns/persistence https://jakarta.ee/xml/ns/persistence/persistence_3_0.xsd"
version="3.1">

This creates an invalid persistence unit (validation exception), at least for OpenJPA, because version="3.1" is not allowed in persistence_3_0.xsd:

<xsd:attribute name="version" type="persistence:versionType"
fixed="3.0" use="required"/>

According to my interpretation of JPA 3.1, version="3.0" must be used for persistence.xml, but version="3.1" must be used in orm.xml (due to the new generator UUID). In order to not break existing usage, i'm considering simply not implementing OpenJPA 3.1 (OpenJPA 3.0 is enough).

@matthiasblaesing
Copy link
Contributor

I agree, looking at the the specification page: https://jakarta.ee/specifications/persistence/3.1/, it seems that the persistence.xml retained version 3.0. This raises the question whether persistence-3.1.xml should just not be there, because it does not exist.

@tehnic-take3
Copy link
Author

Well, Eclipselink 4.0.5 accepts this "malformed" PU.

I tested with an invalid transaction-type too:
OpenJPA throws a SAXParseException very early on loading and Eclipselink throws an IllegalArgumentException later on usage.

I guess Eclipselink does not verify persistence.xml at all.

I considered to remove persistence-3.1.xml completely too, but maybe another implementation (Eclipselink?) uses persistence:Version internally and removing it would break existing usage.

Also Persistence.VERSION_3.1 is used in a lot of other places within j2ee.persistence and enterprise.
I found there more places where i should add OpenJPA 3.0 too.

For this reason i would let the current persistence-3.1.xml, inclusive usage, as is and add only OpenJPA 3.0.

I not checked/tested any other implementation (hibernate, ...).

I'll try to implement your suggested changes and a more complete patch until 15.04. (feature freeze) in another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enterprise [ci] enable enterprise job Java EE/Jakarta EE [ci] enable enterprise job
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants