<exclude-unlisted-classes> is read incorrectly in PersistenceUnitReader [SPR-10767]

[spring-projects-issues]

Nick Williams opened SPR-10767 and commented

This exact same problem also exists in Hibernate ORM (see attached reference URL).

In JPA 1.0's persistence.xml, <exclude-unlisted-classes> defaulted to false (providers always scanned for entities). So, you could have the following four uses of <exclude-unlisted-classes> with the indicated meanings:

1. Omitted means don't exclude / do scan.
2. <exclude-unlisted-classes /> means exclude / don't scan.
3. <exclude-unlisted-classes>false</exclude-unlisted-classes> means don't exclude / do scan.
4. <exclude-unlisted-classes>true</exclude-unlisted-classes> means exclude / don't scan.

As of JPA 2.0, <exclude-unlisted-classes> was changed to default to true:

1. Omitted means exclude / don't scan.
2. <exclude-unlisted-classes /> means exclude / don't scan.
3. <exclude-unlisted-classes>false</exclude-unlisted-classes> means don't exclude / do scan.
4. <exclude-unlisted-classes>true</exclude-unlisted-classes> means exclude / don't scan.

However, PersistenceUnitReader sets the value of ExcludeUnlistedClasses to true if <exclude-unlisted-classes> is merely present, regardless of its value...

...
        Element excludeUnlistedClasses = DomUtils.getChildElementByTagName(persistenceUnit, EXCLUDE_UNLISTED_CLASSES);
        if (excludeUnlistedClasses != null) {
            unitInfo.setExcludeUnlistedClasses(true);
        }
...

...and leaves it as the default value (false in MutablePersistenceUnitInfo) if <exclude-unlisted-classes> is missing. This is incorrect. It will result in the default value being incorrect in JPA 2.0/2.1, and in the value being incorrectly set if persistence.xml contains <exclude-unlisted-classes>false</exclude-unlisted-classes>.

---

Affects: 3.1 GA, 3.2 GA, 4.0 M1

Reference URL: https://hibernate.atlassian.net/browse/HHH-8364

Issue Links:

• Spring no longer recognizes mapped classes [SPR-10826] #15452 Spring no longer recognizes mapped classes

[spring-projects-issues]

Nick Williams commented

Pull request #318 submitted. Changes need to be merged into master and then also into 3.1.x and 3.2.x. I have signed and agree to the terms of the SpringSource Individual Contributor License Agreement.

[spring-projects-issues]

Phil Webb commented

I slightly tweaked the pull request so that MutablePersistenceUnitInfo remains with excludeUnlistedClasses defaulting to false.

Applied to master and 3.2.x. We have not immediate plans for a 3.1.x release so I have not backported that far.

[spring-projects-issues]

Nick Williams commented

Okay.

Just FYI, in Spring 4.0 the persistenceXMLSchemaVersion property in MutablePersistenceUnitInfo defaults to "2.0". Thus, it would only be logical that its excludeUnlistedClasses property default to true (appropriate for the version). That's why I changed the default value. However, in Spring 3.2 the persistenceXMLSchemaVersion property defaults to "1.0", so you are correct that excludeUnlistedClasses should still default to false in that case.

[spring-projects-issues]

Juergen Hoeller commented

Guys, there seems to be a misunderstanding here: The default value of an xsd element only applies if that element is present but has no custom value in its body.
See for example: http://www.codesynthesis.com/pipermail/xsd-users/2008-August/001885.html

In JPA 2.0, they simply corrected the default value for <exclude-unlisted-classes/> in the xsd: If no custom value given, this implies <exclude-unlisted-classes>true</exclude-unlisted-classes>. There was no intention to change actual default behavior, just taking existing behavior as defined in 1.0 and correctly reflecting it in the xsd definition.

So while the boolean parsing in this pull request is certainly correct, the default in case of the element not being present must remain "false". Otherwise, with JPA 2.0, there won't be any scanning for @Entity classes by default. I'm afraid we broke plenty of persistence units there in 4.0 M2; I'll fix this in time for 3.2.4 and also for 4.0 RC1.

Juergen

[spring-projects-issues]

Nick Williams commented

Well that's just frustrating. Whose bright idea was it to make the default semantics different between elements and attributes!? Grrr...

[spring-projects-issues]

Juergen Hoeller commented

Indeed, those xsd semantics are quite non-obvious. I've only really noticed after checking the spec whether they really meant to change the default there.

Anyway, fixed for 3.2.4 and 4.0 RC1 - hopefully correct in all cases now.

Juergen

[spring-projects-issues]

Nick Williams commented

I went to review your commits to do a sanity check on them and make sure I concur, but the most recent commit is five hours ago. Have you just not pushed it yet?

[spring-projects-issues]

Juergen Hoeller commented

Not pushed yet since I'm still doing some tests that the old - wrong - default in the JPA 1.0 persistence.xsd doesn't get in the way with our explicit parsing now...

Coming in about half an hour, I'd say.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

It's finally pushed now.

[spring-projects-issues]

Nick Williams commented

Looks good. I was gonna apologize for creating extra work for you, but it would seem that it was so easy to fix, my pull request was still a net plus. :-)

[spring-projects-issues]

Juergen Hoeller commented

You had a point with the parsing of a specified boolean value there, so the pull request was nevertheless very much worth it.

Quite odd to discover that gap so late, actually, given that we have been doing JPA 1.0 since 2006 and JPA 2.0 since 2009...

Juergen

[spring-projects-issues]

Nick Williams commented

Hibernate has the EXACT SAME BUG with its <exclude-unlisted-classes> parsing, and that has also been there about 5-7 years. I had wondered if one of y'all copied the code from the other so long ago.

A Hibernate bug was actually created about this in 2006 (https://hibernate.atlassian.net/browse/EJB-224), but it was marked as a duplicated of another bug (that it wasn't a duplicate of, IMO), and (surprise, surprise) fixing the duplicated bug did not fix EJB-224. I filed https://hibernate.atlassian.net/browse/HHH-8364 the same day I filed this bug, and I'm updating it with this new information now.

[spring-projects-issues]

Mauro Molinari commented

Well, it was a real headache to understand this mess between specification vs XSD inconsistencies, XML Schema interpretation, parsing bugs of both Spring and Hibernate, another inconsistency I found in Eclipse Dali Persistence Tools regarding this infamous tag...

But, at the end of all this long story, my question is: does the Spring Framework 4 documentation need then to be updated with regards to this tag handling? I mean, look at this:
http://docs.spring.io/spring/docs/current/spring-framework-reference/htmlsingle/#orm-jpa-setup-lcemfb
Here, the docs still says that "<exclude-unlisted-classes> false </exclude-unlisted-classes/> is not supported", but I would really need it to be supported in order to avoid the confusion of omitting this tag completely in different environments.

[spring-projects-issues]

Juergen Hoeller commented

Good catch. I'll reword that ref doc note on exclude-unlisted-classes accordingly, since a false value is being supported now.

Juergen