-
Notifications
You must be signed in to change notification settings - Fork 1.4k
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cannot create a repository for entity with @IdClass [DATAJPA-50] #486
Comments
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 |
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 |
GWA commented Patrick, when you create an Entity with 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... |
GWA commented Olivier, in JpaMetamodelEntityInformation, the SingularAttribute is only used 3 times:
The method getIdAttribute seems never used... With this change, this issue should be fixed. Is it a good idea? |
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 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? |
GWA commented I guess you wanted to write :
JPA allow you to query an entity by the id class [find(Entity.class, Object pk)] Anyway,I've made a quick test to see if modification to make |
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:
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? |
GWA commented OK, i'm working on it. |
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 |
Oliver Drotbohm commented If you're already working on it. Please take into account that the basic |
wims.tijd commented so i switched from
Page<Rank> page = this.rankRepo.findAll(new PageRequest(0, 5)); sql server : |
GWA commented Proposed resolution |
GWA commented The DATAJPA-50.patch contains a solution to this issue. I've made the modification so that tests I added in the JPAPersistableEntityInformationUnitTests works. (Test of JPAEntityInformation against Then, I've tested the solution against real JPA vendor with a real entityManager... 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) |
GWA commented Adding missing file to the patch... |
Mathias Kluba commented IMOHO, it's not "minor" but "blocker" priority... |
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? |
Hari Gangadharan commented
Hari Gangadharan |
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 |
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. |
Jim Hazen commented This is a major issue for me as well. Also need a flat structure without modeled ID fields. Given |
Brendan Fagan commented I concur with this being a major issue. Can someone provide an updated status of this issue and its resolution? |
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 |
Oliver Drotbohm commented I've just pushed a first draft of <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:
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 |
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 |
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 |
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 |
Oliver Drotbohm commented
|
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 |
gaurav rajwanshi commented and at last it says build failure: /org/springframework/spring-instrument/3.0.6.RELEASE/spring-instrument-3.0.6.REL |
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 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 |
gaurav rajwanshi commented May be you can attach the SnapShot jar. I can check if that work in the case of |
Oliver Drotbohm commented Sure you've built the |
Oliver Drotbohm commented If you remove the |
Valentin Ozanne commented Hi Olivier, |
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 |
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
..... Public class CsReglaPK implements Serializable {
... |
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 |
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 |
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? |
Julio Salazar commented Trace: java.lang.IllegalStateException: No supertype found |
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); |
Julio Salazar commented Test case with spring-data-jpa-1.1.0.RC1 and hibernate 4.1.2.Final |
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 So what I am actually wondering is why our |
GWA opened DATAJPA-50 and commented
When I want to create a
JpaRepository
with an Entity like this:MyEntityPK
is a simple bean withString parent;
int order;
. Then i get the following error :Affects: 1.0 M2
Attachments:
Referenced from: commits 3962f1a
13 votes, 18 watchers
The text was updated successfully, but these errors were encountered: