Closed spring-projects-issues closed 11 years ago
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
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
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...
GWA commented
Olivier, in JpaMetamodelEntityInformation, the SingularAttribute is only used 3 times:
The method getIdAttribute seems never used...
The getIdType method code can be replaced by :
"
Type typeSC=this.getClass().getGenericSuperclass();
return (Class\
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 @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?
GWA commented
I guess you wanted to write :
@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.
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?
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 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
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\
sql server : SQL Error: 170, SQLState: 37000 ERROR JDBCExceptionReporter:101 - Line 1: Incorrect syntax near ','
GWA commented
Proposed resolution
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)
GWA commented
Adding missing file to the patch...
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"
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
@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
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 @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?
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 @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:
@IdClass
on entities which means JpaEntityInformation
method getIdType(…)
returns the annotated classgetId(…)
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.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)
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 mvn clean install
?
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
$ git clone git://github.com/SpringSource/spring-data-jpa.git
…
$ git checkout DATAJPA-50
$ mvn clean install
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 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
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
gaurav rajwanshi commented
May be you can attach the SnapShot jar. I can check if that work in the case of @IdClass
issue
Oliver Drotbohm commented
Sure you've built the DATAJPA-50
branch? Have a look at the commit and test cases included
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
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
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 @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
@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\
@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;
...
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 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 ?
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 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)
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\
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 @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
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 https://github.com/spring-projects/spring-data-jpa/commit/3962f1ab31b7d9ab2ad650a720b10a0617419d53
13 votes, 18 watchers