Fully support and test IdClass usages

[beikov]

Currently we often assume that there is only a single id mapped in a JPA type although there could be multiple. We should fix these cases and write some tests that use IdClass to make sure it works.

[jwgmeligmeyling]

After a quick search, the most problematic method seems to be public static SingularAttribute<?, ?> getIdAttribute(IdentifiableType<?> entityType). It has about 40 uses throughout the code, so a fix for this is going to be a quite scattered throughout code.

Primarily I think this method fails to address two things:

• No exception if multiple of the singular attributes are an id attribute, because we just return and use the first found id property, which will consecutively be used throughout the queries, potentially reading and writing rows that were not intended to be touched.
• No special handling for IdentifiableType#getIdClassAttributes() which returns all SingularAttributes if @IdClass is used (which is mandatory if multiple @Id attributes exist)

We probably need a new method public static Set<SingularAttribute<?, ?>> getIdAttributes(IdentifiableType<?> entityType).

For this I primarily see two approaches:

1. Add this additional method and use where it makes sense / where the most blocking exceptions happen (this could lead to further IdClass incompatibilities)
2. Deprecate public static SingularAttribute<?, ?> getIdAttribute(IdentifiableType<?> entityType), make it return getIdAttributes()[0] if size() == 1, else throw an exception (to prevent unexpected edge cases). Slowly rewrite all places where getIdAttribute is used.

Small list of things that almost certainly won't work right with IdClass:

• Page count query
• Page id query
• values queries
• Modifying criteria queries
• Updatable Entity views

[beikov]

I think most uses can be adapted to work properly, except for values queries and com.blazebit.persistence.view.impl.EntityViewManagerImpl#find(javax.persistence.EntityManager, com.blazebit.persistence.view.EntityViewSetting<T,com.blazebit.persistence.CriteriaBuilder<T>>, java.lang.Object), but that's ok. I'd remove the method in the end and throw customized exceptions for the values use and find by id use.
The commit looks like a good start, but the real work will be to adapt all uses 😁
I'm happy to answer questions and help you with obstacles 😄

[jwgmeligmeyling]

I'll extend my PR gradually as I try rewriting all occasions of getIdAttribute(). I mainly made this test commit to check whether any existing tests pass incorrectly, but that doesn't seem to be the case.

Note to self: also look at uses of private boolean isId(Type<?> type, Expression idExpression) in JoinManager.

[ieugen]

I just ran into this issue while trying out this library. This is a show stopper for us.

[beikov]

@ieugen can you elaborate what it is that isn't working for you? The support for keyset pagination with id class attributes is currently on master and will be in the next release.

[jwgmeligmeyling]

I can revive the PR after I finish working on #571.

[ieugen]

Well, my ap won't even start. I have a bunch of entities, one with composit pk using IdClass in Postgres, as i provided link. Lets call it CompositA.

In another entity I have a @OnetoOne with CompositA with field composit. I have two join columns to make the mapping work between the two entities. One for id and one for org id.

When the app starts it fails to create CBF object from blaze. It can't find field composit.id. Removing blaze fixes it.

https://vladmihalcea.com/how-to-map-a-composite-identifier-using-an-automatically-generatedvalue-with-jpa-and-hibernate/

[beikov]

Can you please share the stacktrace so we can further analyze this?

[ieugen]

Sorry, I migrated away from this and did not save the stack trace. Next time I will be more careful.
I used spring boot 2.0.4 with spring data jpa with hibernate 5.2.x and blaze 1.2.x .

[beikov]

That's too bad, on master we already have support for the most important parts of IdClass support but it's just not released yet. I'll ping you when it's released and hope you can give it another shot!

[jwgmeligmeyling]

Based on the description I think this is an issue with compound keys (or multiple JoinColumns) rather than IdClass specifically. I'll experiment with some test cases.

[ieugen]

Yes, that is where I had issues, the 2 join columns.

[jwgmeligmeyling]

The IdClassEntity in the test suite has an association mapping that references a compound key using @IdClass: https://github.com/Blazebit/blaze-persistence/blob/master/core/testsuite/src/main/java/com/blazebit/persistence/testsuite/entity/IdClassEntity.java#L87-L97 . This leads to believe that your problem must be resolved on the current master branch. There are however some features currently unsupported for IdClass entities. These should throw a clear error message ("Can't access a single id attribute as the entity has multiple id attributes i.e. uses @IdClass!") indicating that its currently unsupported. As I am using quite some IdClass entities myself, I am invested in adding these features for IdClass entities too. Nevertheless, none of these known limitations should prevent the metamodel to be built and the program to initialize.

[ghost]

I tried all combinations but none of it is working for the composite primary key. it fails with below exceptions.

• Composite id mapping defined in the IdView and reference this IdView inside EntityView using @IdMapping("this")

@EntityView(CompositeEntity.class)
public interface CompositeKeyView
{
	@Mapping("compositeKey.key1")
	public LzauhtIdView getFirstKey();
	
	@Mapping("compositeKey.key2")
	public LzauhtIdView getSecondKey();
}

@EntityView(CompositeEntity.class)
public interface CompositeEntityView
{
	@IdMapping("this")
	public CompositeKeyView getId();
}

  Exception for Embedded/Embeddable composite key is:
  Caused by: java.lang.IllegalArgumentException: Attribute 'this' not found on type 'CompositeEntity'
	at com.blazebit.persistence.parser.PathTargetResolvingExpressionVisitor.visit(PathTargetResolvingExpressionVisitor.java:225)
	at com.blazebit.persistence.parser.expression.PropertyExpression.accept(PropertyExpression.java:41)

  For IdClass Mapping, it fails with below exception:
  Caused by: java.lang.IllegalStateException: Can't access a single id attribute as the entity has multiple id attributes i.e. uses @IdClass!
	at com.blazebit.persistence.parser.util.JpaMetamodelUtils.getSingleIdAttribute(JpaMetamodelUtils.java:298)
	at com.blazebit.persistence.deltaspike.data.impl.handler.EntityViewRepositoryHandler.idAttribute(EntityViewRepositoryHandler.java:99)
	at com.blazebit.persistence.deltaspike.data.base.handler.AbstractEntityViewAwareRepositoryHandler.findAll(AbstractEntityViewAwareRepositoryHandler.java:194)

• Composite id mapping defined in the EntityView with @IdMapping("compositeKey.key1") and @IdMapping("compositeKey.key2")

@EntityView(CompositeEntity.class)
public interface CompositeEntityView
{
	@IdMapping("compositeKey.key1")
	public LzauhtIdView getFirstKey();
	
	@IdMapping("compositeKey.key2")
	public LzauhtIdView getSecondKey();
}

 Exception is
 Caused by: java.lang.IllegalArgumentException: There are error(s) in entity views!
 Conflicting attribute mapping for attribute 'firstKey' at the methods [CompositeEntityView.getFirstKey, CompositeEntityView.getSecondKey] for managed view type 'CompositeEntityView'
	at com.blazebit.persistence.view.impl.EntityViewManagerImpl.<init>(EntityViewManagerImpl.java:179)
	at com.blazebit.persistence.view.impl.EntityViewConfigurationImpl.createEntityViewManager(EntityViewConfigurationImpl.java:160)

It seems I am not using the mapping correctly. @beikov, can you guide with the mapping for composite primary key.

[jwgmeligmeyling]

Right now we support IdClass everywhere in Blaze-Persist Core, but Blaze-Persist EntityViews does not support compound keys using IdClass yet. A lot of progress has been made to eventually support it, such as making sure Blaze-Persist core is ready in the last 1.3.0 release. Alex and I have contributed to that as well and also done some preliminary work on supporting IdClass entities in EntityViews. Christian has worked on the JpaUtils#expandBindings method that can be used to expand the id class attributes in the where clause of the entity view query.

But today we don't support id class entities yet, and for compound keys in entityviews you're limited to using EmbeddedId instead. Which is by the way more performant and better supported in Hibernate too.

[beikov]

If compositeKey is an embedded id, you should be able to use it like this

@EntityView(CompositeEntityId.class)
public interface CompositeKeyView
{
	@Mapping("key1")
	public LzauhtIdView getFirstKey();
	
	@Mapping("key2")
	public LzauhtIdView getSecondKey();
}

@EntityView(CompositeEntity.class)
public interface CompositeEntityView
{
	@IdMapping("compositeKey")
	public CompositeKeyView getId();
}

[ghost]

Thanks @jwgmeligmeyling, @beikov
It worked with EmbeddedId composite key.

[jwgmeligmeyling]

I frequently check for IdClass support in Blaze-Persistence core. But we haven't had much time to look into doing this for EV's as well. Since, I believe it has actually gotten a fair bit more complicated with all the work on updatable EVs.

Should we close this issue? Or at least change the labels so it indicates that the work needs to be done in EV and not core.