EntityManagerFactoryImpl.getIdentifier uses deprecated version of getIdentifier

Description

When trying to get the identifier of an entity from the PersistenceUnitUtil, I get a NullPointerException. This is because the implementation of getIdentifier uses the deprecated version of ClassMetadata.getIdentifier() which takes no session, and subsequently the implementation of it explicitly sets the session to null. When you get to the IncrediblySillyJpaMapsIdMappedIdentifierValueMarshaller.getIdentifier()-method, persistEventListeners( session ) is called, which then throws the NullPointerException.

This problem is documented as such:

// we need a session to handle this use case if ( session == null ) { throw new AssertionError("Deprecated version of getIdentifier (no session) was used but session was required"); }

However, this happens after the persistEventListeners( session )-call. Shouldn't the new version of getIdentifier() be used when there already is support for this in the code?

The implementation of PersistenceUnitUtil.getIdentifier() in EntityManagerFactoryImpl::HibernatePersistenceUnitUtil:

@Override public Object getIdentifier(Object entity) { final Class entityClass = Hibernate.getClass( entity ); final ClassMetadata classMetadata = emf.getSessionFactory().getClassMetadata( entityClass ); if ( classMetadata == null ) { throw new IllegalArgumentException( entityClass + " is not an entity" ); } //TODO does that work for @IdClass? return classMetadata.getIdentifier( entity ); }

Side note: I can see there is a "TODO:"-comment here, and the answer is no. This has to do with the problem in HHH-10623. It should work for simple IdClass'es though (with no ManyToOne-relationships in the IdClass).

Attachments

1
  • 29 Nov 2016, 11:24 AM

Activity

Show:

Former userMay 10, 2017 at 10:14 PM

Fixed in 5.1 branch as well.

Steve EbersoleDecember 19, 2016 at 8:38 PM

So I looked at the test case added to this issue. Simply accounting for enhancement and proxies as discussed here, would not fix the test. The test builds a transient entity and passes that to javax.persistence.PersistenceUnitUtil#getIdentifier. The class is not enhanced, and as it is transient it can't be a proxy; so here we do not know the Session.

I see the problem and just need to think through the right fix.

Steve EbersoleDecember 5, 2016 at 1:10 PM
Edited

+1000. We can suggest edits of those Javadocs again to the spec group when we start seriously discussing 2.1+

Patrik BakkenDecember 5, 2016 at 7:13 AM

Ok, I understand. I definitely think that the support for entities which happen to be byte code enhanced should be implemented since Hibernate support byte code enhancement. I do agree this is a very poor assumption by the spec though.

I also think this should be documented in the API.

Steve EbersoleDecember 3, 2016 at 4:03 PM

I set the fix version to 5.2.6 mainly to define an end-date to the decision here. Either we do "something", or we reject this. Of course what "something" is ends up being part of that decision.

Fixed

Details

Assignee

Reporter

Fix versions

Affects versions

Priority

Created November 24, 2016 at 11:56 AM
Updated May 22, 2017 at 5:14 AM
Resolved May 10, 2017 at 10:14 PM

Flag notifications