Cannot create a repository for entity with @IdClass [DATAJPA-50] about spring-data-jpa HOT 43 CLOSED

Oliver Drotbohm commented

It seems that Hibernate does not build the JPA metamodel correctly. After a quick search I found various open, unresolved, ignored issues in Hibernate JIRA, e.g. and they don't seem to be eager to fix those as long as the TCK's not barking (see that ticket for example). I'd suggest you rather hold a field of the id class and annotate it with @EmbeddedId as a workaround

from spring-data-jpa.

Patrick Radtke commented

I'm having the same issue, and what I'm confused about is that things seem to work just fine with hibernate+JPA with @IdClass on its own. I can use an EntityManager and find my entities, it is only when I declare an Interface extending JpaRepository that the error happens. I'm not sure why Hibernate would build the metamodel differently when Spring-JPA is used, as compared to when I use the EntityManager directly

from spring-data-jpa.

GWA commented

Patrick, when you create an Entity with @IdClass, Hibernate declare the id as a set of attributes (single Id or EmbeddedId are a single attribute).

When Spring data JPA try to get the Type of the id there is logically an error as you cannot determine a single type from a set of type...

Now, I don't know what are exactly in the JPA specs, but the fact that the specs define a way of getting more than 1 Id attributes means that dataJPA will be in trouble in such case...

from spring-data-jpa.

GWA commented

Olivier, in JpaMetamodelEntityInformation, the SingularAttribute is only used 3 times:

• in the method 'getIdType'
• in the method 'getId'
• in the method 'getIdAttribute'

The method getIdAttribute seems never used...
The getIdType method code can be replaced by :
"
Type typeSC=this.getClass().getGenericSuperclass();
return (Class<ID>)((ParameterizedType)typeSC).getActualTypeArguments()[1].getClass();
"
I don't know when the getId is used (is it mandatory that this method return something?)
but is it possible to change the code to :
If the IdType is valid then use it to get ID else return an exception?
(Or then read the IdClassAttributes and get each attributes and set them into a newly create ID)

With this change, this issue should be fixed.

Is it a good idea?

from spring-data-jpa.

Oliver Drotbohm commented

The repository approach expects you to work with a type representing the entities ID. I know that JPA allows to simply don't have a dedicated id class and annotate various properties with @IdClass but I think this is an intrinsically bad idea as this will let you carry around low-level parts of the id where it's actually a domain concept. It's a bit similar to the the decision to introduce a dedicated Pageable type (although this of course is not related to the spec).

As we expect a single id type in the repository's type signature we aren't able to cover that case I think. How do you set up the repository interface at all?

from spring-data-jpa.

GWA commented

I guess you wanted to write :

• JPA allow to don't declare an unique id attribute by annotating various properties with @Id but then a unique id class is mandatory (and is provided with @IdClass).

JPA allow you to query an entity by the id class [find(Entity.class, Object pk)]
You can retrieve the pk from an entity either directly (like in the JpaMetamodelEntityInformation you wrote) or by reconstructing it (Id class is Serializable and must contains the same properties (same name, same type, getter and setter) as the properties annotated by @Id)

Anyway,I've made a quick test to see if modification to make @IdClass works is possible. (yes it is, but I don't heavily tested that).
I've just added a 'fallback' mode in the JpaMetamodelEntityInformation class in case there is not a single SingularAttribute.
I attached the patch if you interested to see all the needed changes.

from spring-data-jpa.

Oliver Drotbohm commented

The idea seems tempting and the patch also seems to take a reasonable approach. However we probably still have to cover more corner cases as the properties in the id class can be of the id type of the property of the entity:

class FooPK {
  String fooA;
  String fooB;
}

@IdClass(FooPK.class) 
class Foo {
  @Id String fooA;
  @Id Bar fooB;
}

@Entity
class Bar {
  @Id String barId;
}

In that case (as it can be found in chapter 2.4.1.3 of the spec) we'd have to recursively resolve the id value.

Is there a chance you can come up with unit tests for the changes you did already so that I can get my hands on extending and polishing the stuff a little?

from spring-data-jpa.

GWA commented

OK, i'm working on it.

from spring-data-jpa.

GWA commented

Ups, I've made a little mistake with the previous patch... (The idea is not so bad, but I inverted some logic... and test with wrong input don't help to detect that...)

I've read more carefully the spec (eyes open this time :) ) and found out that the getIdClassAttributes return the attributes from the IdClass, not the entityclass.

So I will completely changes my code to get the idClass directly from those attributes. (the type of those attributes must be the id class...), and I guess we could also use the JpaClassUtils like a 'factory' to resolve recursively the id...

Anyway, I will work on it to get you a better patch

from spring-data-jpa.

Oliver Drotbohm commented

If you're already working on it. Please take into account that the basic EntityInformation implementation checks whether the id returned is null to decide whether an object is considered new we probably need to have to dig into the just instantiated value object and check the contained fields for being null in turn

from spring-data-jpa.

wims.tijd commented

so i switched from @IdClass to @EmbeddedId and this seemed to work,
now i added a simple test to get a Page result :
@Entity
public class Rank {

@EmbeddedId
private RankId id;

Page<Rank> page = this.rankRepo.findAll(new PageRequest(0, 5));
mysql :
select distinct count((rank0_.C_CODE, rank0_.N_CODE, rank0_.B_TM)) as col_0_0_ from T_RANK rank0_ limit ?
WARN JDBCExceptionReporter:100 - SQL Error: 1241, SQLState: 21000
ERROR JDBCExceptionReporter:101 - Operand should contain 1 column(s)

sql server :
SQL Error: 170, SQLState: 37000
ERROR JDBCExceptionReporter:101 - Line 1: Incorrect syntax near ','

from spring-data-jpa.

GWA commented

Proposed resolution

from spring-data-jpa.

GWA commented

The DATAJPA-50.patch contains a solution to this issue.
Now here the history of this patch:

I've made the modification so that tests I added in the JPAPersistableEntityInformationUnitTests works. (Test of JPAEntityInformation against @IdClass based Entity, with a mocked entityManager)

Then, I've tested the solution against real JPA vendor with a real entityManager...
I've got some error due to the lack of standard for the getIdClassAttributes method (SingularType returned are from the entity? the IdClass? ...)

It's why I wrote a JPAPersistableEntityInformationIntegrationTests that test the class against real jpa vendor. This test use a specific persitence file (persistence_support.xml) a specific orm file (orm_support.xml) and a specific application context (appcontext_support.xml).

In appcontext_support.xml I've set the jpa vendor to hibernate, but I've also tested eclipselink and openjpa. With openjpa, the test succeed. With eclipselink, sometime a LoadTimeWeaver error can occurs (WHY????) (when this error don't occurs, the tests succeed...).

Anyway, I think the correction is pretty correct (but you will probably need to polish the test configuration)

from spring-data-jpa.

GWA commented

Adding missing file to the patch...

from spring-data-jpa.

Mathias Kluba commented

IMOHO, it's not "minor" but "blocker" priority...
In my case, I can't use the workaround, because my objects should be "flat"

from spring-data-jpa.

Oliver Drotbohm commented

I was about to get my hands onto this for the upcoming 1.1.0.M2 release but it seems due to some changes in the codebase the patch attached does not apply cleanly anymore.

Is there a chance you massage it a bit to be applicable to the current master?

from spring-data-jpa.

Hari Gangadharan commented

@Oliver: +1 for this from me. This is a show stopper. I cannot use spring data unless this is taken care of. I can work on this if you give me pointers on where to look.

Hari Gangadharan

from spring-data-jpa.

GWA commented

Sorry Olivier, I didn't see the last comment.

I will rework this patch I expect to get a new patch in the middle of next week

from spring-data-jpa.

GWA commented

Hello Olivier,

Here is the patch that work on the current head.

I've noticed that the 'exist' method on SimpleJpaRepository will NOT work with composite PK entities.

I've written 2 TODO in the class. We should open a jira for those.

from spring-data-jpa.

Jim Hazen commented

This is a major issue for me as well. Also need a flat structure without modeled ID fields. Given @Id + an IdClass wrapper, I'd expect a repository implementation to be able to synthesize an id field internally if necessary, applying the same dirty checking rules as for @EmbeddedId's. Isn't this what Hibernate does under the hood anyway?

from spring-data-jpa.

Brendan Fagan commented

I concur with this being a major issue. Can someone provide an updated status of this issue and its resolution?

from spring-data-jpa.

Jeff Nickoloff commented

I'm just wondering how many times GWA is going to have to update his patch before its applied. Know you're busy Olivier and I love this project. I'll wait as long as I have to but an update would be great

from spring-data-jpa.

Oliver Drotbohm commented

I've just pushed a first draft of @IdClass support to a feature branch. I've also deployed a snapshot with the classifier idclass to the snapshot repository. So should be able to try this build using:

<dependency>
  <groupId>org.springframework.data</groupId>
  <artifactId>spring-data-jpa</artifactId>
  <version>1.1.0.BUILD-SNAPSHOT</version>
  <classifier>idclass</classifier>
</dependency>

A few comments regarding the implementation. What I've done so far is coming up with a slightly less complicated implementation of the following behaviour:

1. generally support usage of @IdClass on entities which means JpaEntityInformation method getIdType(…) returns the annotated class
2. getId(…) creates an instance of the id type and copies all properties declared in the id type from the entity to the id instance. It returns null in case all of the partial id properties are null as well.
3. SimpleJpaRepository.exists(…) falls back to loading the entity and checking it for null in case of the usage of an @IdClass

I haven't actually dealt with the derived entities case (JPA spec 2.4.1) yet to get early feedback and potentially improve the implementation. So feel free to give it a try. Coming up with the test cases for the functionality currently implemented I've stumbled above quite a few issues within the JPA implementations (Hibernate not returning the id type for IdentifiableType.getIdType() when @IdClass is used (workaround in the implementation), Hibernate not being able to bind auto-generated values (see bug), EclipseLink not allowing to to auto-generate more than one partial id attributes)) that you might face as well (actually I am wondering who has successfully used @IdClass and id generation with any JPA implementation at all)

from spring-data-jpa.

gaurav rajwanshi commented

Thank Oliver,

I have tried using the dependency details you have provided. However Not able to build using these details. Please provide the correct updated maven artifact details

from spring-data-jpa.

Oliver Drotbohm commented

Sure you have the snapshot repo declared inside your pom? If it still fails, would you mind simply checking out the sources and build the project using mvn clean install?

from spring-data-jpa.

gaurav rajwanshi commented

Thanks for the quick response.

Please let me know details of the version control system so that i can check out the jars.

Gaurav

from spring-data-jpa.

Oliver Drotbohm commented

$ git clone git://github.com/SpringSource/spring-data-jpa.git
…
$ git checkout DATAJPA-50
$ mvn clean install

from spring-data-jpa.

gaurav rajwanshi commented

Thanks Oliver,

Please see if you fix the snapshot so that I can directly get it from the maven repository.

Its trying to build all spring projects when I am running the mvn clean install

from spring-data-jpa.

gaurav rajwanshi commented

and at last it says build failure:

/org/springframework/spring-instrument/3.0.6.RELEASE/spring-instrument-3.0.6.REL
EASE.jar
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 11:42.970s
[INFO] Finished at: Thu Jan 12 12:52:26 GMT 2012
[INFO] Final Memory: 23M/55M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.
5:test (default-test) on project spring-data-jpa: There are test failures.
[ERROR]
[ERROR] Please refer to C:\DATAJPA-50\target\surefire-reports for the individual
test results.
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e swit
ch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please rea
d the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureExc
eption

from spring-data-jpa.

gaurav rajwanshi commented

Hi Oliver,

I am able to manage to mvn install the source in my eclipse. However after that i trapped into the same old issue of having @IdClass on my entity for which I am creating DAO

Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'iRefValueDAO': FactoryBean threw exception on object creation; nested exception is java.lang.IllegalStateException: No supertype found

from spring-data-jpa.

gaurav rajwanshi commented

May be you can attach the SnapShot jar. I can check if that work in the case of @IdClass issue

from spring-data-jpa.

Oliver Drotbohm commented

Sure you've built the DATAJPA-50 branch? Have a look at the commit and test cases included

from spring-data-jpa.

Oliver Drotbohm commented

If you remove the <classifier /> element from the JAR plugin configuration you should be able to reference the built artifact without the classifier in the dependency declaration

from spring-data-jpa.

Valentin Ozanne commented

Hi Olivier,
I test your fix and it seems to solve my problem (the same as described here)! Do you know if a release will be soon available ?
Thanks

from spring-data-jpa.

Oliver Drotbohm commented

Merged the feature branch into master and deployed the artifact to the snapshot repository. Resolving this as fixed for now. Feel free to open up a new ticket for more advanced use cases

from spring-data-jpa.

Julio Salazar commented

Hi Olivier,

I have an issue that is related to these patch (I tried also with the version 1.1.0.RC1) , if I try to do pagination over a class with @IdClass (See class definition) the count to determine how many rows are fetched is using 2 columns instead of 1 and in Oracle it throws "ORA-00907: missing right parenthesis tips" , do you have an idea how to solve this issue ?. Thanks in advance.

Pageable pageRequest = new PageRequest(0, 1000 );
    Page<CsRegla> list = dataStore.findAll(pageRequest); --SimpleJPARepository

• Count
  select count((csregla0_.C_REGLAS, csregla0_.C_PLANES)) as col_0_0_ from XXXX.CS_REGLA csregla0_
• Class definition:

@IdClass(domain.CsReglaPK.class)
@Entity
@Table(schema = "XXXX", name = "CS_REGLA")
@XmlAccessorType(XmlAccessType.FIELD)
@XmlType(namespace = "domain", name = "CsRegla")
@XmlRootElement(namespace = "domain")
public class CsRegla implements Persistable<CsReglaPK> {
private static final long serialVersionUID = 1L;
private boolean isNew;

@Column(name = "C_REGLAS", length = 50, nullable = false)
@Basic(fetch = FetchType.EAGER)
@Id
@XmlElement
String creglas;

@Column(name = "C_PLANES", nullable = false)
@Basic(fetch = FetchType.EAGER)
@Id
@XmlElement
Integer cplanes;

.....

Public class CsReglaPK implements Serializable {

@Column(name = "C_REGLAS", length = 50, nullable = false)
@Basic(fetch = FetchType.EAGER)
@Id
public String creglas;

@Column(name = "C_PLANES", nullable = false)
@Basic(fetch = FetchType.EAGER)
@Id
public Integer cplanes;

...

from spring-data-jpa.

Jim Hazen commented

Julio, if you're using Hibernate as your JPA provider, make sure you're using version 4+. There was an issue in the 3.x series that caused the SQL generated by Spring Data's count pagination queries to fail. Instead of count(*), count (PK1, PK2) was being generated and that confused a lot of DB's. I had this same issue with compound key pagination on mySQL and an upgrade to Hibernate 4 resolved it

from spring-data-jpa.

Julio Salazar commented

Thanks a lot Jim, the upgrade to 4.1.2.Final solved part of the issue (count problem solved) , right now I'm getting the first bug
java.lang.IllegalStateException: No supertype found
at org.hibernate.ejb.metamodel.AbstractIdentifiableType.requireSupertype(AbstractIdentifiableType.java:85)
I have already installed the latest version from the master branch of GITHub of spring-data-jpa-1.1.0.BUILD-SNAPSHOT but maybe that release doesn't have the DATAJPA-50 update. Do you know where I can find these patch release ?

from spring-data-jpa.

Oliver Drotbohm commented

Have a look at the fix for this ticket which was released with 1.1.0.RC1 (as indicated in this ticket, see above). Does your scenario differ from the ones provided in the test cases?

from spring-data-jpa.

Julio Salazar commented

Trace: java.lang.IllegalStateException: No supertype found
at org.hibernate.ejb.metamodel.AbstractIdentifiableType.requireSupertype(AbstractIdentifiableType.java:85)
at org.hibernate.ejb.metamodel.AbstractIdentifiableType.getIdType(AbstractIdentifiableType.java:173)
at org.hibernate.ejb.criteria.expression.function.AggregationFunction$COUNT.renderArguments(AggregationFunction.java:112)
at org.hibernate.ejb.criteria.expression.function.ParameterizedFunctionExpression.render(ParameterizedFunctionExpression.java:96)
at org.hibernate.ejb.criteria.expression.function.BasicFunctionExpression.renderProjection(BasicFunctionExpression.java:73)
at org.hibernate.ejb.criteria.QueryStructure.render(QueryStructure.java:252)
at org.hibernate.ejb.criteria.CriteriaQueryImpl.render(CriteriaQueryImpl.java:340)
at org.hibernate.ejb.criteria.CriteriaQueryCompiler.compile(CriteriaQueryCompiler.java:217)
at org.hibernate.ejb.AbstractEntityManagerImpl.createQuery(AbstractEntityManagerImpl.java:628)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.springframework.orm.jpa.ExtendedEntityManagerCreator$ExtendedEntityManagerInvocationHandler.invoke(ExtendedEntityManagerCreator.java:365)
at $Proxy26.createQuery(Unknown Source)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.springframework.orm.jpa.SharedEntityManagerCreator$SharedEntityManagerInvocationHandler.invoke(SharedEntityManagerCreator.java:240)

from spring-data-jpa.

Julio Salazar commented

Oliver, my scenario is almost the same as yours but the differences are on my test cases because I'm testing the method findAll (Pageable), I think that this problem is related to an issue on hibernate.

https://hibernate.onjira.com/browse/HHH-7216

Attach you a small project that shows the error with spring-data-JPA 1.1.0.RC1 and Hibernate 4.1.2.Final

Pageable pageRequest = new PageRequest(1, 10);
Page<CsRegla> reglas = repository.findAll(pageRequest);

from spring-data-jpa.

Julio Salazar commented

Test case with spring-data-jpa-1.1.0.RC1 and hibernate 4.1.2.Final

from spring-data-jpa.

Oliver Drotbohm commented

So looking at the stack traces I'd argue this has nothing to do with the original posters issue as this broke when looking up the id metadata already. What you're seing is an exception on query execution most likely to be caused by the Hibernate issue you linked to. Honestly, implementing the support for this I stumbled over various issues with various persistence providers (see my comment) that I wonder how anyone could reasonably use @IdClass without suffering severe design implications.

So what I am actually wondering is why our hibernate-4 branch is actually not suffering from this problem as it seems to be okay in Bamboo

from spring-data-jpa.