Set javax.persistence.jtaDataSource to null, when provided and empty

[ArneLimburg]

My persistence.xml looks like

<persistence-unit name="..." transaction-type="JTA">
    <jta-data-source>...</jta-data-source>

In the test I want to override it with RESOURCE_LOCAL. At least in Hibernate this can be done by setting the following properties:

Map<String, Object> persistenceProperties = new HashMap<>();
persistenceProperties.put("javax.persistence.transactionType", "RESOURCE_LOCAL");
persistenceProperties.put("javax.persistence.jtaDataSource", null);
persistenceProperties.put("javax.persistence.jdbc.url", "...");
...
EntityManagerFactory emf = Persistence.createEntityManagerFactory(..., persistenceProperties);

I want to achieve the same behavior with @PersistenceProperty, but I cannot set the jtaDataSource to null, which is essential (at least for Hibernate).

My proposal is to set the "javax.persistence.jtaDataSource" property value to null, if it is present in the properties and is an empty string. This way, it could be removed by providing an empty string.

@PersistenceContext(unitName = "...", , properties = {
    @PersistenceProperty(name = "javax.persistence.transactionType", value = "RESOURCE_LOCAL"),
    @PersistenceProperty(name = "javax.persistence.jtaDataSource", value = ""),
                        ...})
private EntityManager entityManager;

[dadrus]

Hi @ArneLimburg. Excuse me please for the belated answer. To be honest, I wonder about the purpose. I mean, it looks like you're trying to reuse your production persistence.xml file for test purposes, which is IMHO not a good idea. In addition the documentation of jpa-unit clearly states, which transaction type must be used:

Like in any JPA application, you have to define a persistence.xml file in the META-INF directory which includes the JPA provider and persistence-unit configuration. For test purposes the transaction-type of the configured persistence-unit must be RESOURCE_LOCAL.

But maybe I overlook something. So can you please help me understanding your case better?

p.s. the builds fail if somebody not part of maintainers list is pushing an MR. I have to fix the configuration for these types of MRs.

[ArneLimburg]

Yes, the use case is to use the production persistence.xml in tests. Otherwise you would have to put all your entities into the test persistence.xml explicitly, because they reside not at the same classpath location as the test persistence.xml.

I see no problem, using the production persistence.xml for tests, if you change the transaction type and database connection. I guess that's why the property "javax.persistence.transactionType" was specified.

[dadrus]

I guess, those who've written the JPA specification have not though about config update at runtime. It is however another story :)

Do you have a stack trace for me to see what happens, when you don't set javax.persistence.jtaDataSource to null. Maybe it is worth to update PersistenceUnitDescriptorImpl class and have jpa-unit more smart without relying on additional configuration. I mean javax.persistence.jtaDataSource can anyway not be used with javax.persistence.transactionType=RESOURCE_LOCAL.

[ArneLimburg]

javax.persistence.jtaDataSource can't be set to null, since the annotation has no default value ;-)
When I leave the property out, Hibernate tries to access jndi
org.hibernate.service.spi.ServiceException: Unable to create requested service [org.hibernate.engine.jdbc.env.spi.JdbcEnvironment] Caused by: org.hibernate.engine.jndi.JndiException: Error parsing JNDI name [java:/jdbc/deliveryDS] Caused by: javax.naming.NoInitialContextException: Need to specify class name in environment or system property, or as an applet parameter, or in an application resource file: java.naming.factory.initial
Same when the value is set to empty.

[dadrus]

What happens if you update the constructor of PersistenceUnitDescriptorImpl like following?

public PersistenceUnitDescriptorImpl(final Element element, final Map<String, Object> properties) {
        this.properties = new HashMap<>(properties);
        classList = new ArrayList<>();
        parse(element);
        
        // cannot use JTA 
        this.properties.remove("javax.persistence.jtaDataSource");
}

[ArneLimburg]

Guess the same will happen.
But if you like to fix it there, the following would work, I guess:

public PersistenceUnitDescriptorImpl(final Element element, final Map<String, Object> properties) {
        this.properties = new HashMap<>(properties);
        classList = new ArrayList<>();
        parse(element);
        
        // cannot use JTA 
        this.properties.put("javax.persistence.jtaDataSource", null);
}

If that would be ok for you, I'll change the pull request.
Or maybe less invasive:

public PersistenceUnitDescriptorImpl(final Element element, final Map<String, Object> properties) {
        this.properties = new HashMap<>(properties);
        classList = new ArrayList<>();
        parse(element);
        
        // cannot use JTA
        if (properties.containsKey("javax.persistence.jtaDataSource") && properties.get("javax.persistence.jtaDataSource").isEmpty()) {
            this.properties.put("javax.persistence.jtaDataSource", null);
        }
}

[dadrus]

You're right. the call to Persistence.createEntityManagerFactory will load the persistence.xml once again.

Less invasive is better :)

// cannot use JTA
if (properties.containsKey("javax.persistence.jtaDataSource")) {
    this.properties.put("javax.persistence.jtaDataSource", null);
}

I removed && properties.get("javax.persistence.jtaDataSource").isEmpty() as the project is compiled using java 8 which does not have the updated API to support isEmpty.

So if you don't mind, update your MR to include this change.

[ArneLimburg]

I'll create a new Pull Request for that.

[dadrus]

👍

[ArneLimburg]

Just updated my proposal to match the pull request