Skip to content
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

JPA orm.xml file handling to support multiple database vendors #14762

Closed
mgwillem opened this issue Feb 2, 2021 · 36 comments · Fixed by #17334
Closed

JPA orm.xml file handling to support multiple database vendors #14762

mgwillem opened this issue Feb 2, 2021 · 36 comments · Fixed by #17334
Assignees
Labels
area/hibernate-orm Hibernate ORM kind/enhancement New feature or request
Milestone

Comments

@mgwillem
Copy link

mgwillem commented Feb 2, 2021

Description
I need to implement a solution that supports multiple database vendors like Oracle, MS SQLServer, DB2, etc.

Moreover I need to re-use existing "legacy" JARs containing JPA entities with annotations and orm.xml files.
Such "legacy" JARs are used by existing Java EE 8 applications.

Existing database schemas use sequences for Oracle PK and identities for SQLServer PK.
To handle such differences, we use annotations to define Oracle sequences .. and we use the orm.xml file to define MS SQLServer identities.

Moreover the orm.xml file is used for setting persistence-unit/schema as well. It means it is not only a PK override issue.

The orm.xml file is a standard JPA feature that is very useful (imho) and I believe this file support is mandatory for such use-cases.

Please keep in mind that migration of legacy apps/libraries is an important point here.

Thank you!

@mgwillem mgwillem added the kind/enhancement New feature or request label Feb 2, 2021
@ghost ghost added the triage/needs-triage label Feb 2, 2021
@gastaldi
Copy link
Contributor

gastaldi commented Feb 2, 2021

@mgwillem can you provide a sample orm.xml that you use in your application?

@gastaldi gastaldi added area/hibernate-orm Hibernate ORM and removed triage/needs-triage labels Feb 2, 2021
@mgwillem
Copy link
Author

mgwillem commented Feb 2, 2021

Here is an example of such orm.xml file for the SQLServer support

<?xml version="1.0" encoding="UTF-8"?>

<entity-mappings
        xmlns="http://java.sun.com/xml/ns/persistence/orm"
        version="2.0">

    <description>Override Annotations...</description>

    <persistence-unit-metadata>
        <persistence-unit-defaults>
            <schema>dbo</schema>
        </persistence-unit-defaults>
    </persistence-unit-metadata>

    <entity class="org.foo.Foo" access="FIELD" metadata-complete="false">
        <attributes>
            <id name="id">
                <generated-value strategy="IDENTITY" />
            </id>
        </attributes>
    </entity>

    <entity class="org.foo.Bar" access="FIELD" metadata-complete="false">
        <attributes>
            <id name="id">
                <generated-value strategy="IDENTITY" />
            </id>
        </attributes>
    </entity>
</entity-mappings>

@Sanne
Copy link
Member

Sanne commented Feb 2, 2021

I agree we should do this, but let me clarify some restrictions:

  • The orm.xml resource will be ignored at runtime: I would prefer we pre-parse it during the Quarkus augmentation
  • We can allow adding/updating it during "dev mode"
  • It should still be possible to build a Quarkus application which does not need to initialize the XML parser libraries at runtime (even if the app is usingorm.xml backed mappings)

I hope that's agreable?

@mgwillem
Copy link
Author

mgwillem commented Feb 2, 2021

Yes the orm.xml resource could be ignored at runtime. In my use-case I will generate one JAR per database vendor.
It seems we have a deal :)

Do you think we could define a "milestone" for this feature request ?

@mgwillem
Copy link
Author

mgwillem commented Feb 5, 2021

Hi ! I'm curious to know if this orm.xml file support could be added soon in Quakus. What do you think ?

@Sanne
Copy link
Member

Sanne commented Feb 5, 2021

I must admit I'm not sure about the timeline.

There's a couple more urgent tasks that my team needs to finish first, and I personally need to take some time off - so it depends if I can delegate this one, but it's not simple to do as far as I remember from the initial POC :) But that's been a couple years ago so I suppose it's possible that this is much simpler noways, as our tools improved and we have a better understanding of GraalVM we have today.

I'll try a better estimate next week; ideally if I can get a better picture and delegate this to another team member it is more likely to get done quickly.

@mgwillem
Copy link
Author

mgwillem commented Feb 5, 2021

Thanks @Sanne for your update! I hope it will be easier to implement than expected :)

@mgwillem
Copy link
Author

Hi! Any news regarding this feature support? Thanks

@sithmein
Copy link

sithmein commented Mar 1, 2021

So this basically means there is no way to use entities from external libraries with JPA in Quarkus? This is bad.

@yrodiere yrodiere assigned yrodiere and unassigned Sanne Mar 2, 2021
@Sanne
Copy link
Member

Sanne commented Mar 2, 2021

@mgwillem sorry I'm the slow cog here. @yrodiere volunteered to take it over.

@sithmein if by "external libraries" you mean 3rd party classes which can't be annotated, then yes there's this current limitation, but it shouldn't be too hard to fix. At least, conceptually, there's nothing fundamentally hard about it other than someone needing to do the work of wiring up the existing support of Hibernate ORM to our custom bootstrap code.

HTH

@yrodiere
Copy link
Member

@mgwillem Could you please detail what your classpath looks like? What are the JARs involved, where are the orm.xml files and there persistence.xml files, and which persistence.xml file references which orm.xml file?

The JPA spec allows many different setups, and it gives the tools to achieve these setups by allowing a persistence.xml to specify a list of JARs.

But currently, we don't support that in Quarkus, so we can end up including orm.xml files that shouldn't be included. And that's a very real problem: if a META-INF/orm.xml file in the classpath defines a default schema, you definitely don't want to apply it to the wrong persistence unit.

Here are the use cases that I can think of:

  1. Single-JAR application, configured through either application.properties or persistence.xml.
  2. Multi-JAR application configured through either application.properties or persistence.xml, where the orm.xml files have unique names across all JARs.
  3. Multi-JAR application configured through a persistence.xml, where the orm.xml files may not have unique names, but are always located in the same JAR as the corresponding persistence.xml.
  4. Multi-JAR application where the orm.xml files do not have unique names, and are not necessarily located in the same JAR as the persistence.xml they need to be used with, or are used in a persistence unit that doesn't have a persistence.xml.

Use cases 1 and 2 can be covered easily (barring any problem with the XML parsers, but that's another matter).

Use case 3 requires the configuration of the persistence unit through a persistence.xml, but could be covered as well, as long as we have a way to detect where each file (persistence.xml or orm.xml) comes from (which JAR).

Use case 4 cannot be addressed unless we add support for assigning JARs to a persistence unit, so that we can filter out irrelevant orm.xml files. And from what I can see, that doesn't look like something we want to do... @Sanne?

@Sanne
Copy link
Member

Sanne commented Mar 18, 2021

by spec 8.2.1 the default name would be META-INF/orm.xml, and is able to avoid ambiguity by storing this resource in the root of the persistence unit (e.g. in the same jar as persistence.xml and its entities).

So I'd say you don't have to implement "case 4" as that was never supported in other systems either.

@yrodiere
Copy link
Member

by spec 8.2.1 the default name would be META-INF/orm.xml, and is able to avoid ambiguity by storing this resource in the root of the persistence unit (e.g. in the same jar as persistence.xml and its entities).

I wish :)

An object/relational mapping XML file named orm.xml may be specified in the META-INF directory in the root of the persistence unit or in the META-INF directory of any jar file referenced by the persistence.xml.

Taken from the ORM documentation, quoting the spec: https://docs.jboss.org/hibernate/orm/5.4/userguide/html_single/Hibernate_User_Guide.html#bootstrap-jpa-xml-files

@Sanne
Copy link
Member

Sanne commented Mar 22, 2021

@yrodiere I don't understand your point for "I wish" - could you clarify?

It seems to me that it's avoiding ambiguity, what am I missing?

@yrodiere
Copy link
Member

Sorry if that wasn't clear, my point was just below. By spec, orm.xml does not even have to be in the same JAR as the persistence.xml. Users can avoid ambiguity by specifying the relevant JARs in their persistence.xml.

So from my understanding, this JAR content is spec-compliant:

  • my-app.jar
    • persistence.xml
      • PU1
        • <mapping-file>META-INF/orm.xml</mapping-file>
        • <jar-file>pu1.jar</jar-file>
      • PU2
        • <mapping-file>META-INF/orm.xml</mapping-file>
        • <jar-file>pu2.jar</jar-file>
  • pu1.jar
    • META-INF/orm.xml
  • pu2.jar
    • META-INF/orm.xml

Same with custom (non-default) file names:

  • my-app.jar
    • persistence.xml
      • PU1
        • <mapping-file>META-INF/my-orm.xml</mapping-file>
        • <jar-file>pu1.jar</jar-file>
      • PU2
        • <mapping-file>META-INF/my-orm.xml</mapping-file>
        • <jar-file>pu2.jar</jar-file>
  • pu1.jar
    • META-INF/my-orm.xml
  • pu2.jar
    • META-INF/my-orm.xml

Looking at the (complex) code that handles JARs and entity/resource scanning in ORM, it does seem to handle this kind of setup.

I can tell that we won't be able to provide something that is both simple (flat classpath, simple entity/resource scanner) and adheres to the spec. And that's fine I guess, we don't want to handle every single legacy quirk that JEE allows, but then the whole of this issue was to support legacy setups, so I'm not sure where we should draw the line :)

I'll start with a simple implementation where we just look for mapping files everywhere, and fail if there are multiple resources for a given mapping file name. That should cover use cases 1 and 2.

Then I'll see about use case 3: when looking for a mapping file, I'll consider that a file in the same JAR as the persistence.xml gets precedence over all others with the same name.

That should be a decent compromise, I think. And anyway, we cannot support more without supporting <jar-file>.

@mgwillem
Copy link
Author

mgwillem commented Mar 22, 2021 via email

@yrodiere
Copy link
Member

Does it answer to your question ? :)

Almost, thanks. One remaining question: where is the persistence.xml? In which file(s)? libA.jar, libB.jar, or your application jar?

@sithmein
Copy link

In my use case there wouldn't be a persistence.xml at all because Quarkus should set up everything.

@yrodiere
Copy link
Member

In my use case there wouldn't be a persistence.xml at all because Quarkus should set up everything.

Sure. I have that in mind too: allow listing mapping files in the Quarkus configuration.

The legacy use cases are much more tricky though, so I'd like to know where the persistence.xml file is in @mgwillem 's case...

@gavinking
Copy link

Sorry if that wasn't clear, my point was just below. By spec, orm.xml does not even have to be in the same JAR as the persistence.xml. Users can avoid ambiguity by specifying the relevant JARs in their persistence.xml.

That seems like a very fine setup in Java EE where you have assemblies (EARs and WARs) that contain jars. And the domain model might be in the jar along with its orm.xml, and the persistence.xml would quite likely be in the assembly.

But do we have that sort of thing here?

@mgwillem
Copy link
Author

mgwillem commented Mar 22, 2021 via email

@yrodiere
Copy link
Member

But do we have that sort of thing here?

No, we don't have <jar-file> support, even when using persistence.xml. There's an exception somewhere being thrown as soon as we detect <jar-file> being used.

So that was my point: I don't think we can (fully) support use case 4 without <jar-file> support.

Maybe that's fine, maybe that's not; in any case, supporting the other use cases will be a lot of work already, so I'll stick to that for now.

Fortunately for @mgwillem , it seems he doesn't need that as his orm.xml files are in the root file of his persistence units:

libA.jar and libB.jar contain one META-INF/persistence.xml file each.
In my use case the application jar does not contain any persistence file.

Alright, thank you! That's use case 3, then.

@yrodiere
Copy link
Member

yrodiere commented Apr 19, 2021

I submitted a PR to support XML mapping: #16630

Anyone interested in the feature, please try it out and report about your results!

You will need to build the main branch of Hibernate ORM locally first, though. See the PR description. => That's no longer true, just building Quarkus locally will be enough.

@agoncal
Copy link
Contributor

agoncal commented May 17, 2021

I do have the same problem now. I have a legacy.jar that I cannot touch (nor add any configuration file) with a few entities and POJOs that need to be mapped and queried in my app. I've tried adding a persistence.xml file with <mapping-file> targetting orm.xm but didn't work.

Hoping the PR will get in soon ;o) Let me know, I'm ready to test it if you need someone

@Sanne
Copy link
Member

Sanne commented May 17, 2021

hi @agoncal ! Feel free to test the PR - it looks good, the reason for which it wasn't merged yet should not be a problem for you in practice.

@agoncal
Copy link
Contributor

agoncal commented May 17, 2021

@Sanne I just gave it a try and it works!

I had the following exception:

        [error]: Build step io.quarkus.hibernate.orm.deployment.HibernateOrmProcessor#defineJpaEntities threw an exception: io.quarkus.deployment.configuration.ConfigurationError: Unable to properly register the hierarchy of the following JPA classes as they are not in the Jandex index:
        - org.agoncal.quarkus.jdbc.Customer
Consider adding them to the index either by creating a Jandex index for your dependency via the Maven plugin, an empty META-INF/beans.xml or quarkus.index-dependency properties.

So I had to add the following to the application.properties:

quarkus.index-dependency."customer".group-id=org.agoncal.course.quarkus
quarkus.index-dependency."customer".artifact-id=jdbc

Works like a charm ! And best of the best, I can create a PanacheRepositoryBase out of this mapped POJO !

@yrodiere
Copy link
Member

Thanks for testing @agoncal ; happy to see it works for you.

Hopefully we'll get rid of the remaining metaspace problem soon (it may be unrelated, but it's blocking the PR pending investigation). Then we'll merge the PR.

@Sanne
Copy link
Member

Sanne commented May 17, 2021

that Jandex error makes me think we might want to automatically add them to the index, if we can identify the artifact

@agoncal
Copy link
Contributor

agoncal commented May 17, 2021

That would be awesome. As a developer I don't want to know this level of details of Quarkus internals.

@yrodiere
Copy link
Member

@Sanne Maybe. But we cannot add them to the index until we start processing the mapping files and going through the entity hierarchy. I suspect we cannot add them to the index at this point, otherwise this error message wouldn't be here in the first place?

@yrodiere
Copy link
Member

Created #17285

@agoncal
Copy link
Contributor

agoncal commented May 18, 2021

I found something weird. nullable=false doesn't seem to get mapped. I have the following POJO on a separate JAR file:

public class Artist {

  private Long id;
  private String name;
  private String bio;
  private Instant createdDate = Instant.now();
  // Getters and setter
}

When I add this orm.xml file to my project, Artist gets mapped with all the defaults:

  <entity class="org.agoncal.quarkus.jdbc.Artist">
    <attributes>
      <id name="id">
        <generated-value strategy="AUTO"/>
      </id>
    </attributes>
  </entity>

create table Artist
(
    id          int8 not null,
    bio         varchar(255),
    createdDate timestamp,
    name        varchar(255),
    primary key (id)
)

Now, if I customize the mapping, the name of the table, name of the columns, the length of the columns do get changed, but not the nullability (name and created_date should not be null):

  <entity class="org.agoncal.quarkus.jdbc.Artist">
    <table name="t_artists"/>
    <attributes>
      <id name="id">
        <generated-value strategy="AUTO"/>
      </id>
      <basic name="name">
        <column length="50" nullable="false"/>
      </basic>
      <basic name="bio">
        <column length="3000"/>
      </basic>
      <basic name="createdDate">
        <column name="created_date" nullable="false"/>
      </basic>
    </attributes>
  </entity>

create table t_artists
(
    id           int8 not null,
    bio          varchar(3000),
    created_date timestamp,
    name         varchar(50),
    primary key (id)
)

Is my syntax right ? Am I doing something wrong or is the mapping not working correctly ?

@yrodiere
Copy link
Member

yrodiere commented May 18, 2021

@agoncal This looks odd, though I doubt it's caused by Quarkus... Then again, looking at the ORM code, I don't see how length could work while nullable does not; they are literally handled 5 lines apart from each other.

Do you have a quick reproducer? If so, please open a ticket either for Quarkus or ORM, I'll try to debug it.

@agoncal
Copy link
Contributor

agoncal commented May 18, 2021

@yrodiere Yes, I managed to reproduce it in a much simpler example https://github.com/agoncal/agoncal-bug-quarkus-orm

@yrodiere
Copy link
Member

@agoncal Thanks for the reproducer, I'm having a look right now. Sorry about the delay, there was quite a few issues to handle from last week.

@yrodiere
Copy link
Member

yrodiere commented May 20, 2021

@agoncal The problem was in Quarkus after all. Fix is #17383. Thanks for reporting this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-orm Hibernate ORM kind/enhancement New feature or request
Projects
None yet
7 participants