[hibernate-dev] Still true with Javassist? Any final method prevents Hibernate from creating single-ended association proxies
Hi all, excuse me if this list isn't indented for such a question, but I asked this question about a year ago in the forums without any answers and I think it should be easy to answer for any Hibernate core developer: According to Hibernate's (3.6.5) reference documentaion (Section 21.1.3, Single-ended association proxies), such a single-ended association proxy can't be constructed by Hibernate if it contains "any final methods". My question is, does this restriction apply to getters/setters of persistent fields only or really to any method in an entity class? As far as I can see Javassist is able to modify final methods as well and even if it were not, why should a method like public final String toString() { return "..."; } prevent Hibernate from using a proxy? Thanks in advance, Robin. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Still true with Javassist? Any final method prevents Hibernate from creating single-ended association proxies
If your toString method ends up calling some of the entity state (likely), the initialization won't be triggered since you will call your getters from inside the instance. So it's a good general rule to mandate non final methods on entities. Emmanuel On 8 juil. 2011, at 09:59, Robin Sander wrote: > > Hi all, > > excuse me if this list isn't indented for such a question, but I asked this > question about a year ago in the forums > without any answers and I think it should be easy to answer for any Hibernate > core developer: > > According to Hibernate's (3.6.5) reference documentaion (Section 21.1.3, > Single-ended association proxies), > such a single-ended association proxy can't be constructed by Hibernate if it > contains "any final methods". > > My question is, does this restriction apply to getters/setters of persistent > fields only or really to any method in an entity class? > As far as I can see Javassist is able to modify final methods as well and > even if it were not, why should a method like > public final String toString() { > return "..."; > } > prevent Hibernate from using a proxy? > > Thanks in advance, > > Robin. > > > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Still true with Javassist? Any final method prevents Hibernate from creating single-ended association proxies
Isn't that totally independent from a method beeing final or not? If I have a method: public int getSum() { return field1 + field2; } Then this would trigger the initialization of the entity (in JavassistLazyInitializer I assume) and public final int getSum() { return field1 + field2; } would not? (if field1 and field2 are persistent fields and field-based access is used) Only yesterday I've stumpled upon a related issue in equals(): I've compared a persistent field of two instances of the same entity and that _did not_ trigger the initialization of the other instance proxy. So there I had to use "If (this.field.equals(other.getField())) { ..." instead of If (this.field.equals(other.field)) { ..." and no method of that entity was final. On 08.07.2011, at 11:26, Emmanuel Bernard wrote: > If your toString method ends up calling some of the entity state (likely), > the initialization won't be triggered since you will call your getters from > inside the instance. So it's a good general rule to mandate non final methods > on entities. > > Emmanuel > > On 8 juil. 2011, at 09:59, Robin Sander wrote: > >> >> Hi all, >> >> excuse me if this list isn't indented for such a question, but I asked this >> question about a year ago in the forums >> without any answers and I think it should be easy to answer for any >> Hibernate core developer: >> >> According to Hibernate's (3.6.5) reference documentaion (Section 21.1.3, >> Single-ended association proxies), >> such a single-ended association proxy can't be constructed by Hibernate if >> it contains "any final methods". >> >> My question is, does this restriction apply to getters/setters of persistent >> fields only or really to any method in an entity class? >> As far as I can see Javassist is able to modify final methods as well and >> even if it were not, why should a method like >> public final String toString() { >> return "..."; >> } >> prevent Hibernate from using a proxy? >> >> Thanks in advance, >> >> Robin. >> >> >> ___ >> hibernate-dev mailing list >> hibernate-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Still true with Javassist? Any final method prevents Hibernate from creating single-ended association proxies
On 8 juil. 2011, at 11:44, Robin Sander wrote: > > Isn't that totally independent from a method beeing final or not? > If I have a method: > > public int getSum() { > return field1 + field2; > } > > Then this would trigger the initialization of the entity (in > JavassistLazyInitializer I assume) > and > > public final int getSum() { > return field1 + field2; > } > > would not? (if field1 and field2 are persistent fields and field-based access > is used) correct. we can't intercept a final method and thus can't initialize when you call getSum() > > Only yesterday I've stumpled upon a related issue in equals(): I've compared > a persistent field of two instances > of the same entity and that _did not_ trigger the initialization of the other > instance proxy. So there I had to use > "If (this.field.equals(other.getField())) { ..." instead of If > (this.field.equals(other.field)) { ..." > and no method of that entity was final. Not the same but related. you are bypassing the proxy and thus don't initialize. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Still true with Javassist? Any final method prevents Hibernate from creating single-ended association proxies
Many thanks for your answers! To sum this up: Final methods don't prevent Hibernate from creating a proxy in general but unless those methods don't use any state of the entity this is highly inadvisable. Since I have several methods in base classes which indeed don't use any entity state this is good news for me. So this boils down whether a final method can be intercepted or not. But isn't Javassist capable of doing so? And if that's true, are there any plans to support intercepting final methods as well? On 08.07.2011, at 12:11, Emmanuel Bernard wrote: > > On 8 juil. 2011, at 11:44, Robin Sander wrote: > >> >> Isn't that totally independent from a method beeing final or not? >> If I have a method: >> >> public int getSum() { >> return field1 + field2; >> } >> >> Then this would trigger the initialization of the entity (in >> JavassistLazyInitializer I assume) >> and >> >> public final int getSum() { >> return field1 + field2; >> } >> >> would not? (if field1 and field2 are persistent fields and field-based >> access is used) > > correct. we can't intercept a final method and thus can't initialize when you > call getSum() > >> >> Only yesterday I've stumpled upon a related issue in equals(): I've compared >> a persistent field of two instances >> of the same entity and that _did not_ trigger the initialization of the >> other instance proxy. So there I had to use >> "If (this.field.equals(other.getField())) { ..." instead of If >> (this.field.equals(other.field)) { ..." >> and no method of that entity was final. > > Not the same but related. you are bypassing the proxy and thus don't > initialize. > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Still true with Javassist? Any final method prevents Hibernate from creating single-ended association proxies
> So this boils down whether a final method can be intercepted or not. But > isn't Javassist capable of doing so? I imagine you can intercept a final method via bytecode enhancement but we do proxy generation, not bytecode enhancement for many reasons. (we do have a bytecode enhancement mode but barely anybody uses it, it's more marketing than useful IMO). BTW do you mind writing a blog on what you've learned? ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Still true with Javassist? Any final method prevents Hibernate from creating single-ended association proxies
On 08.07.2011, at 14:02, Emmanuel Bernard wrote: > >> So this boils down whether a final method can be intercepted or not. But >> isn't Javassist capable of doing so? > > I imagine you can intercept a final method via bytecode enhancement but we do > proxy generation, not bytecode enhancement for many reasons. > (we do have a bytecode enhancement mode but barely anybody uses it, it's more > marketing than useful IMO). Now _that_ is the point! :-) > > BTW do you mind writing a blog on what you've learned? > I don't have any blog but I've got an the very same question open at stackoverflow and I will add my conclusion there as my own answer. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Patch for HHH-6361 / HHH-6349 : Envers loses audit information due to a Hibernate core bug
As Eric wrote this fix is needed to fix an Envers issue (HHH-6349), however I don't feel competent to review the patch for hibernate-core. So maybe someone else could try? :) Thanks, Adam On Jul 4, 2011, at 3:20 PM, Scheper, Erik-Berndt wrote: > Hi, > I'd like someone to review my proposed fix of HHH-6361 > (https://github.com/hibernate/hibernate-core/pull/117) for the 3.6 branch. > >> From a Hibernate core perspective, this may seem like a minor issue, but it >> is the cause of HHH-6349, where Envers forever loses the audit information >> when objects were added to or removed from a collection. Since there is no >> way to retrieve this information from the database at a later moment, this >> is really bad from the Envers (auditing) perspective. > > The proposed fix causes both the provided testcase for HHH-6361 (the > Hibernate core issue) and the testcase for HHH-6349 (the Envers issue) to > pass by ensuring that after a merge() operation the snapshot value of the > collection, as obtained by collectionEntry.getSnapshot(), corresponds with > the database contents. This is a good thing, of course. > > However, apart from the possible performance implication (though I'm not sure > if there's a remedy for this), I am a bit worried about the fact that I had > to fix a unit test in the Hibernate testsuite > (org.hibernate.test.manytomanyassociationclass.compositeid.ManyToManyAssociationClassCompositeIdTest) > to make the 3.6 build succeed. As a rule, that's a bad sign. > > What happens is that after the patch this test crashes with an NPE in the > hashCode() method of MembershipWithCompositeId.Id class. The reason of the > NPE is that MembershipWithCompositeId now has a non-null "Id" property, with > a null value of the userId and groupId properties. Even though it was easy to > fix the test by overriding the deleteMembership()-method in > ManyToManyAssociationClassCompositeIdTest by setting the "Id" property of > MembershipWithCompositeId to null, this doesn't feel good to me. > > I'd really like to see a fix for HHH-6361 without these changes in > ManyToManyAssociationClassCompositeIdTest, but I couldn't find one. Any help > there would be appreciated of course. > > After an initial review, I'd be more than happy to provide a fix for the > Hibernate 4.0.x series, which is also plagued by the same bug. > > Regards, > Erik-Berndt > > Disclaimer: > This message contains information that may be privileged or confidential and > is the property of Sogeti Nederland B.V. or its Group members. It is intended > only for the person to whom it is addressed. If you are not the intended > recipient, you are not authorized to read, print, retain, copy, disseminate, > distribute, or use this message or any part thereof. If you receive this > message in error, please notify the sender immediately and delete all copies > of this message. > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev -- Adam Warski http://twitter.com/#!/adamwarski http://www.warski.org http://www.softwaremill.eu ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
[hibernate-dev] MethodLevel Constraints for method level validations
Do you think we should support the following? @DateInOrder void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); ie like we have properly level and class level constraints, we could get method level constraints receiving all the parameters and raising constraints violations if necessary. Emmanuel ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] MethodLevel Constraints for method level validations
Cool Stuff !! +1 2011/7/8 Emmanuel Bernard > Do you think we should support the following? > > @DateInOrder > void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); > > ie like we have properly level and class level constraints, we could get > method level constraints receiving all the parameters and raising > constraints violations if necessary. > > Emmanuel > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] MethodLevel Constraints for method level validations
Good idea. On Fri, 08 Jul 2011 16:02:03 +0200, Emmanuel Bernard wrote: > Do you think we should support the following? > > @DateInOrder > void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); > > ie like we have properly level and class level constraints, we could get > method level constraints receiving all the parameters and raising > constraints violations if necessary. > > Emmanuel > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Patch for HHH-6361 / HHH-6349 : Envers loses audit information due to a Hibernate core bug
Looks good. I added a few minor comments. I'd like to see the Map sizing thing addressed and the 4.0 port done, but I have no problems with these changes. On Fri 08 Jul 2011 07:32:25 AM CDT, Adam Warski wrote: > As Eric wrote this fix is needed to fix an Envers issue (HHH-6349), however I > don't feel competent to review the patch for hibernate-core. > So maybe someone else could try? :) > > Thanks, > Adam > > On Jul 4, 2011, at 3:20 PM, Scheper, Erik-Berndt wrote: > >> Hi, >> I'd like someone to review my proposed fix of HHH-6361 >> (https://github.com/hibernate/hibernate-core/pull/117) for the 3.6 branch. >> >>> From a Hibernate core perspective, this may seem like a minor issue, but >>> it is the cause of HHH-6349, where Envers forever loses the audit >>> information when objects were added to or removed from a collection. Since >>> there is no way to retrieve this information from the database at a later >>> moment, this is really bad from the Envers (auditing) perspective. >> >> The proposed fix causes both the provided testcase for HHH-6361 (the >> Hibernate core issue) and the testcase for HHH-6349 (the Envers issue) to >> pass by ensuring that after a merge() operation the snapshot value of the >> collection, as obtained by collectionEntry.getSnapshot(), corresponds with >> the database contents. This is a good thing, of course. >> >> However, apart from the possible performance implication (though I'm not >> sure if there's a remedy for this), I am a bit worried about the fact that I >> had to fix a unit test in the Hibernate testsuite >> (org.hibernate.test.manytomanyassociationclass.compositeid.ManyToManyAssociationClassCompositeIdTest) >> to make the 3.6 build succeed. As a rule, that's a bad sign. >> >> What happens is that after the patch this test crashes with an NPE in the >> hashCode() method of MembershipWithCompositeId.Id class. The reason of the >> NPE is that MembershipWithCompositeId now has a non-null "Id" property, with >> a null value of the userId and groupId properties. Even though it was easy >> to fix the test by overriding the deleteMembership()-method in >> ManyToManyAssociationClassCompositeIdTest by setting the "Id" property of >> MembershipWithCompositeId to null, this doesn't feel good to me. >> >> I'd really like to see a fix for HHH-6361 without these changes in >> ManyToManyAssociationClassCompositeIdTest, but I couldn't find one. Any help >> there would be appreciated of course. >> >> After an initial review, I'd be more than happy to provide a fix for the >> Hibernate 4.0.x series, which is also plagued by the same bug. >> >> Regards, >> Erik-Berndt >> >> Disclaimer: >> This message contains information that may be privileged or confidential and >> is the property of Sogeti Nederland B.V. or its Group members. It is >> intended only for the person to whom it is addressed. If you are not the >> intended recipient, you are not authorized to read, print, retain, copy, >> disseminate, distribute, or use this message or any part thereof. If you >> receive this message in error, please notify the sender immediately and >> delete all copies of this message. >> ___ >> hibernate-dev mailing list >> hibernate-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev -- st...@hibernate.org http://hibernate.org ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
[hibernate-dev] Search: to follow or not to follow latest Lucene index format
So the testsuite is using an helper method "org.hibernate.search.test.SearchTestCase.getTargetLuceneVersion()" for all those cases in which the unfriendly Lucene API demands a version. The test method is currently returning this constant: Version.LUCENE_31 The ConfigContext is using a default of Version.LUCENE_30 if the user doesn't specify any version in the configuration. ..and I recently updated the Lucene dependency to 3.3, without changing these values. Now if we really want to keep them in sync, we could use Version.LUCENE_CURRENT; but this is deprecated, not because it's going to be removed but because there are implications in using it, so it's discouraged. It's also nice to be able to verify compatibility of Search's use of Lucene when using a backwards compatible index format. I think we're not tracking the latest version of Lucene with this flag, because we wanted to make sure we where backwards compatible. Given we're now having the option to break backwards compatibility, should we start tracking the latest version of Lucene, and log a warning if the configuration value is not set? As the Lucene developers suggest, we should recommend coding this value in the configuration. Also because Hibernate Search now seems to work equally well with all Lucene 3.0.x, 3.1.x and 3.3.x, we should either consider only the latest supported, or test for the others too: worth having an environment parameter to be able to run the testsuite with older Lucene versions, or run it in mixed environments of newer Lucene versions but configured as older versions? Cheers, Sanne ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Search: to follow or not to follow latest Lucene index format
> > It's also nice to be able to verify compatibility of Search's use of > Lucene when using a backwards compatible index format. We don't have tests (today) that keep old index formats and run against the new version of HSearch. So the verification is an illusion :) > > I think we're not tracking the latest version of Lucene with this > flag, because we wanted to make sure we where backwards compatible. I thought CURRENT would be removed by the Lucene team actually. > Given we're now having the option to break backwards compatibility, > should we start tracking the latest version of Lucene, and log a > warning if the configuration value is not set? > As the Lucene developers suggest, we should recommend coding this > value in the configuration. Right we can improve the doc here. > > Also because Hibernate Search now seems to work equally well with all > Lucene 3.0.x, 3.1.x and 3.3.x, we should either consider only the > latest supported, or test for the others too: > worth having an environment parameter to be able to run the testsuite > with older Lucene versions, or run it in mixed environments of newer > Lucene versions but configured as older versions? It's a lot of work/resource for not that much value IMO ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] MethodLevel Constraints for method level validations
BTW I forgot but do we support Constructor validation? On 8 juil. 2011, at 16:02, Emmanuel Bernard wrote: > Do you think we should support the following? > > @DateInOrder > void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); > > ie like we have properly level and class level constraints, we could get > method level constraints receiving all the parameters and raising constraints > violations if necessary. > > Emmanuel > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] MethodLevel Constraints for method level validations
I think you should. 2011/7/8 Emmanuel Bernard > BTW I forgot but do we support Constructor validation? > > On 8 juil. 2011, at 16:02, Emmanuel Bernard wrote: > > > Do you think we should support the following? > > > > @DateInOrder > > void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); > > > > ie like we have properly level and class level constraints, we could get > method level constraints receiving all the parameters and raising > constraints violations if necessary. > > > > Emmanuel > > ___ > > hibernate-dev mailing list > > hibernate-dev@lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Feedback on Bean Validation JAX-RS integration
On 30 juin 2011, at 18:25, Hardy Ferentschik wrote: > I don't really understand though what the "Client API" really does, but maybe > some > syncing/brainstorming in the programmatic API area could be useful for BV and > JAX-RS. I agree, I think it would be better not to support such a model if they cannot somehow type-safely share the contract between the client and the server. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Search: to follow or not to follow latest Lucene index format
2011/7/8 Emmanuel Bernard : >> >> It's also nice to be able to verify compatibility of Search's use of >> Lucene when using a backwards compatible index format. > > We don't have tests (today) that keep old index formats and run against the > new version of HSearch. So the verification is an illusion :) I don't think we have to verify binary formats, that's Lucene's own business. I'm just wondering how to deal with this nasty API properly. >> I think we're not tracking the latest version of Lucene with this >> flag, because we wanted to make sure we where backwards compatible. > > I thought CURRENT would be removed by the Lucene team actually. No, I specifically asked this on #lucene, the intention is to keep it but discourage people from using it. >> Given we're now having the option to break backwards compatibility, >> should we start tracking the latest version of Lucene, and log a >> warning if the configuration value is not set? >> As the Lucene developers suggest, we should recommend coding this >> value in the configuration. > > Right we can improve the doc here. And log a warning: if people where configuring it explicitly, then Search would be able to continue providing "drop in" upgrades (even better than before). >> >> Also because Hibernate Search now seems to work equally well with all >> Lucene 3.0.x, 3.1.x and 3.3.x, we should either consider only the >> latest supported, or test for the others too: >> worth having an environment parameter to be able to run the testsuite >> with older Lucene versions, or run it in mixed environments of newer >> Lucene versions but configured as older versions? > > It's a lot of work/resource for not that much value IMO. Ok, then I'll just make sure the constants are in sync and pointing to CURRENT. Sanne ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Feedback on Bean Validation JAX-RS integration
Many thanks guys. The good news is that we all are very much aligned in what we think. I am writing an email to send this feedback to the JAX-RS team. Emmanuel On 1 juil. 2011, at 14:55, Hardy Ferentschik wrote: > On Thu, 30 Jun 2011 22:04:39 +0200, Gunnar Morling > wrote: > >> Some minor remarks: >> >> * In listing 2 setEmail() instead of a getter is annotated. >> >> I'm not sure whether this is intentionally, otherwise it would make >> sense to use getters as in BV. > > Good catch. I was assuming getters. Need to check whether this is really > intentionally. > > >> * "The order in which validations are checked is as follows: >> constraint validations on fields and properties are checked first, on >> resource classes are checked next" >> >> I don't think we could currently ensure such an order in HV but I >> guess it could be implemented. The question is whether this is really >> required. > > I understood this to be more of a JAX-RS thing. I thought that there is a > way for same in the framework to distinguish between form parameter processing > and "resource class populating". If so it is just a question where and how > to call the validator. After re-reading the examples I am not so sure > anymore. The examples just use MyResourceClass, but still seem to distinguish > between resource class and non resource class. > > >> * "If the getUser() method in [the implementing class] MyResourceClass >> is decorated with any annotation ... all of the annotations in the >> [implemented] interface will be ignored. > >> I think this might require some more consideration. In HV we decided >> against such an approach, as preconditions shouldn't be strengthened >> in sub-types in order to obey to the Liskov substitution principle >> [1]. >> Instead of the very conservative approach currently taken by HV (not >> allowing parameter constraints for one method in several places in a >> hierarchy) > > I wonder what they think about this approach? > > --Hardy ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Patch for HHH-6361 / HHH-6349 : Envers loses audit information due to a Hibernate core bug
Ok, thanks for looking into this, Steve. Actually I was already working on the 4.0 port and I'll definitely look into the Map sizing and take that into account. Unfortunately I won't have time to fix that this weekend, so I'll probably submit updated / new pull requests for master and 3.6 either on Monday or Tuesday. Regards, Erik-Berndt Van: hibernate-dev-boun...@lists.jboss.org [hibernate-dev-boun...@lists.jboss.org] namens Steve Ebersole [st...@hibernate.org] Verzonden: vrijdag 8 juli 2011 17:59 Aan: hibernate-dev@lists.jboss.org Onderwerp: Re: [hibernate-dev] Patch for HHH-6361 / HHH-6349 : Envers loses audit information due to a Hibernate core bug Looks good. I added a few minor comments. I'd like to see the Map sizing thing addressed and the 4.0 port done, but I have no problems with these changes. On Fri 08 Jul 2011 07:32:25 AM CDT, Adam Warski wrote: > As Eric wrote this fix is needed to fix an Envers issue (HHH-6349), however I > don't feel competent to review the patch for hibernate-core. > So maybe someone else could try? :) > > Thanks, > Adam > > On Jul 4, 2011, at 3:20 PM, Scheper, Erik-Berndt wrote: > >> Hi, >> I'd like someone to review my proposed fix of HHH-6361 >> (https://github.com/hibernate/hibernate-core/pull/117) for the 3.6 branch. >> >>> From a Hibernate core perspective, this may seem like a minor issue, but >>> it is the cause of HHH-6349, where Envers forever loses the audit >>> information when objects were added to or removed from a collection. Since >>> there is no way to retrieve this information from the database at a later >>> moment, this is really bad from the Envers (auditing) perspective. >> >> The proposed fix causes both the provided testcase for HHH-6361 (the >> Hibernate core issue) and the testcase for HHH-6349 (the Envers issue) to >> pass by ensuring that after a merge() operation the snapshot value of the >> collection, as obtained by collectionEntry.getSnapshot(), corresponds with >> the database contents. This is a good thing, of course. >> >> However, apart from the possible performance implication (though I'm not >> sure if there's a remedy for this), I am a bit worried about the fact that I >> had to fix a unit test in the Hibernate testsuite >> (org.hibernate.test.manytomanyassociationclass.compositeid.ManyToManyAssociationClassCompositeIdTest) >> to make the 3.6 build succeed. As a rule, that's a bad sign. >> >> What happens is that after the patch this test crashes with an NPE in the >> hashCode() method of MembershipWithCompositeId.Id class. The reason of the >> NPE is that MembershipWithCompositeId now has a non-null "Id" property, with >> a null value of the userId and groupId properties. Even though it was easy >> to fix the test by overriding the deleteMembership()-method in >> ManyToManyAssociationClassCompositeIdTest by setting the "Id" property of >> MembershipWithCompositeId to null, this doesn't feel good to me. >> >> I'd really like to see a fix for HHH-6361 without these changes in >> ManyToManyAssociationClassCompositeIdTest, but I couldn't find one. Any help >> there would be appreciated of course. >> >> After an initial review, I'd be more than happy to provide a fix for the >> Hibernate 4.0.x series, which is also plagued by the same bug. >> >> Regards, >> Erik-Berndt >> >> Disclaimer: >> This message contains information that may be privileged or confidential and >> is the property of Sogeti Nederland B.V. or its Group members. It is >> intended only for the person to whom it is addressed. If you are not the >> intended recipient, you are not authorized to read, print, retain, copy, >> disseminate, distribute, or use this message or any part thereof. If you >> receive this message in error, please notify the sender immediately and >> delete all copies of this message. >> ___ >> hibernate-dev mailing list >> hibernate-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev -- st...@hibernate.org http://hibernate.org ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev Disclaimer: This message contains information that may be privileged or confidential and is the property of Sogeti Nederland B.V. or its Group members. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message. ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Feedback on Bean Validation JAX-RS integration
FYI my reply Hi guys, This is a very good proposal and quite aligned with Bean Validation (BV) ideas and with future plans we have for Bean Validation 1.1. In particular, we plan to add method validations (somewhat close to how we have designed it in Hibernate Validator (HV) http://docs.jboss.org/hibernate/validator/4.2/reference/en-US/html_single/#validator-customoptions-methodvalidation ). We probably should discuss so that JAX-RS (and maybe other specs) could reuse the same way of describing these constraints on method parameters and return value to gain consistency. I/we have reserves wrt client validation. I've detailed them below. The rest is feedback or comments on specific issues. Users need to be able to disable validation (even if it's on default) and select the groups they want to see validated (even if the Default group is used by default). @Validate(of=Extended.class) public void registerUser( @NotNull @FormParam("firstName") String firstName ) { ... } @Validate(DISABLED) public void registerUser( @NotNull @FormParam("firstName") String firstName ) { ... } Such a system will have to be standardized I think for the method validation feature we plan to add to BV 1.1. Listening 2 got me confused: do you really want to put the constraint annotation on the setter instead of the getter? I don't think that's totally inline with BV and is not consistent: at the very least the @Email annotation should be a parameter annotation public void setEmail(@Email String email) { } * Entity Validation All of these approaches to declare constraints on method parameters (and return values) will be explored and hopefully standardized in Bean Validation. What I see is that you also have a need to ensure that upon a method call, attributes of the hosting class should be validated. I need to think about that. Intuitively, I don't think this should be standardized in BV but rather left to the integrating framework. But imagine if we manage to standardize this approach in Bean Validation for all frameworks in need for lifecycle driven validation (at least the declaration approach), we would get a huge consistency across the platform. There is "@Provider" in one of your examples. What's the meaning for JAX-RS, why would a constraint validator need to be a provider as well? (It's usually an "implementation detail" attached to a @Constraint and is never seen by client libraries. "JAX-RS implementations MUST report an error if the type of the validation does not match the return type of the resource method. In the example above, the validator for the @CheckUser annotation must be defined for the type User." => Bean Validation must do that already, JAX-RS can delegate the work to BV. BV need to support Constructor validation (something I forgot) "The order in which validations are checked is as follows: constraint validations on fields and properties are checked first, on resource classes are checked next and on method parameters are checked last." What's the reasoning behind enforcing an order? In BV so far, we say that we will return all violations but that the order is undefined. "The rule for inheritance of constraint annotations is the same as that for all the other JAX-RS annotations (see Section 3.6). Namely, constraint annotations on methods and method parameters are inherited from interfaces and super-classes, with the latter taking precedence over the former when sharing common methods. " Interesting, we need to investigate that. Where do these rules come from? In Hibernate Validator we have stricter rules. In HV we decided against your approach, as preconditions shouldn't be strengthened in sub-types in order to obey to the Liskov substitution principle. Instead of the very conservative approach currently taken by HV (not allowing parameter constraints for one method in several places in a hierarchy) one could also OR-join the constraints from a method hierarchy causing the "weakest" contract in the hierarchy becoming effective. That's the way taken by most DbC frameworks. s/ConstraintValidationException/ConstraintViolationException/ "A constraint violation error reported for the return type of a resource method should not return a 400 (Bad Request) HTTP code. Instead, this condition should be reported as a 500 (Internal Server Error) response. However, it isn't clear how to write an exception mapper that can discern request vs. response violations." I'm not sure I follow. I agree that input violations should be 400 and output violations should be 500. Why do you say that writing an exception mapper would be hard? Is exception mapper a specific feature of JAX-RS? Client side validation On the client side, you don't know the server contract right, ie the client and the server don't share the contract in a type-safe way?, ie you don't proxy a method call from the client via some REST call and call the same method on the server side with an actual imp
Re: [hibernate-dev] MethodLevel Constraints for method level validations
+1 Good idea!! --Kevin Le 8 juil. 2011 à 16:03, Emmanuel Bernard a écrit : > Do you think we should support the following? > > @DateInOrder > void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); > > ie like we have properly level and class level constraints, we could get > method level constraints receiving all the parameters and raising constraints > violations if necessary. > > Emmanuel > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] MethodLevel Constraints for method level validations
I've created http://opensource.atlassian.com/projects/hibernate/browse/BVAL-232 On 8 juil. 2011, at 21:02, Kevin Pollet wrote: > +1 > Good idea!! > > --Kevin > > Le 8 juil. 2011 à 16:03, Emmanuel Bernard a écrit : > >> Do you think we should support the following? >> >> @DateInOrder >> void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); >> >> ie like we have properly level and class level constraints, we could get >> method level constraints receiving all the parameters and raising >> constraints violations if necessary. >> >> Emmanuel >> ___ >> hibernate-dev mailing list >> hibernate-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev > > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
Re: [hibernate-dev] Patch for HHH-6361 / HHH-6349 : Envers loses audit information due to a Hibernate core bug
No worries. Its 2 weeks till the next release window. Thanks for your work on this and the others! On Fri 08 Jul 2011 01:45:31 PM CDT, Scheper, Erik-Berndt wrote: > Ok, thanks for looking into this, Steve. Actually I was already working on > the 4.0 port and I'll definitely look into the Map sizing and take that into > account. > Unfortunately I won't have time to fix that this weekend, so I'll probably > submit updated / new pull requests for master and 3.6 either on Monday or > Tuesday. > > Regards, > > Erik-Berndt > > > Van: hibernate-dev-boun...@lists.jboss.org > [hibernate-dev-boun...@lists.jboss.org] namens Steve Ebersole > [st...@hibernate.org] > Verzonden: vrijdag 8 juli 2011 17:59 > Aan: hibernate-dev@lists.jboss.org > Onderwerp: Re: [hibernate-dev] Patch for HHH-6361 / HHH-6349 : Envers loses > audit information due to a Hibernate core bug > > Looks good. I added a few minor comments. I'd like to see the Map > sizing thing addressed and the 4.0 port done, but I have no problems > with these changes. > > On Fri 08 Jul 2011 07:32:25 AM CDT, Adam Warski wrote: >> As Eric wrote this fix is needed to fix an Envers issue (HHH-6349), however >> I don't feel competent to review the patch for hibernate-core. >> So maybe someone else could try? :) >> >> Thanks, >> Adam >> >> On Jul 4, 2011, at 3:20 PM, Scheper, Erik-Berndt wrote: >> >>> Hi, >>> I'd like someone to review my proposed fix of HHH-6361 >>> (https://github.com/hibernate/hibernate-core/pull/117) for the 3.6 branch. >>> From a Hibernate core perspective, this may seem like a minor issue, but it is the cause of HHH-6349, where Envers forever loses the audit information when objects were added to or removed from a collection. Since there is no way to retrieve this information from the database at a later moment, this is really bad from the Envers (auditing) perspective. >>> >>> The proposed fix causes both the provided testcase for HHH-6361 (the >>> Hibernate core issue) and the testcase for HHH-6349 (the Envers issue) to >>> pass by ensuring that after a merge() operation the snapshot value of the >>> collection, as obtained by collectionEntry.getSnapshot(), corresponds with >>> the database contents. This is a good thing, of course. >>> >>> However, apart from the possible performance implication (though I'm not >>> sure if there's a remedy for this), I am a bit worried about the fact that >>> I had to fix a unit test in the Hibernate testsuite >>> (org.hibernate.test.manytomanyassociationclass.compositeid.ManyToManyAssociationClassCompositeIdTest) >>> to make the 3.6 build succeed. As a rule, that's a bad sign. >>> >>> What happens is that after the patch this test crashes with an NPE in the >>> hashCode() method of MembershipWithCompositeId.Id class. The reason of the >>> NPE is that MembershipWithCompositeId now has a non-null "Id" property, >>> with a null value of the userId and groupId properties. Even though it was >>> easy to fix the test by overriding the deleteMembership()-method in >>> ManyToManyAssociationClassCompositeIdTest by setting the "Id" property of >>> MembershipWithCompositeId to null, this doesn't feel good to me. >>> >>> I'd really like to see a fix for HHH-6361 without these changes in >>> ManyToManyAssociationClassCompositeIdTest, but I couldn't find one. Any >>> help there would be appreciated of course. >>> >>> After an initial review, I'd be more than happy to provide a fix for the >>> Hibernate 4.0.x series, which is also plagued by the same bug. >>> >>> Regards, >>> Erik-Berndt >>> >>> Disclaimer: >>> This message contains information that may be privileged or confidential >>> and is the property of Sogeti Nederland B.V. or its Group members. It is >>> intended only for the person to whom it is addressed. If you are not the >>> intended recipient, you are not authorized to read, print, retain, copy, >>> disseminate, distribute, or use this message or any part thereof. If you >>> receive this message in error, please notify the sender immediately and >>> delete all copies of this message. >>> ___ >>> hibernate-dev mailing list >>> hibernate-dev@lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/hibernate-dev > > -- > st...@hibernate.org > http://hibernate.org > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > > Disclaimer: > This message contains information that may be privileged or confidential and > is the property of Sogeti Nederland B.V. or its Group members. It is intended > only for the person to whom it is addressed. If you are not the intended > recipient, you are not authorized to read, print, retain, copy, disseminate, > distribute, or use this message or any part thereof. If you receive this > message in error, ple
Re: [hibernate-dev] MethodLevel Constraints for method level validations
Hi, > Do you think we should support the following? > > @DateInOrder > void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); IMO that would be a useful feature. The following approaches came up when discussing the idea earlier this year [1]: 1) A generic approach in the spirit of @ScriptAssert which would allow to use script expressions in place like this: @ParameterAssert(script="args[1].before(args[2])", lang="javascript") void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); This could also be combined with a parameter naming approach (see HV-409 [2]): @ParameterAssert(script="from.before(to)", lang="javascript") void bookHotel(@NotNull @Valid Customer customer, @Named("from") Date from, @Named("to") Date to); 2) An approach using dedicated constraints/validators: @DateParameterCheck void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); The validator for the constraint would get passed the parameter values as array/list, maybe one would have a new interface MethodConstraintValidator for this: public class DateParameterCheckValidator implements MethodConstraintValidator { public void initialize(DateParameterCheck constraint) {} public boolean isValid(Object[] parameterValues, ConstraintValidatorContext context) { return ((Date)parameterValues[1]).before((Date)parameterValues[2]); } } As this is pretty un-safe, there also was the idea of supporting typed validators like this: public class DateParameterCheckValidator implements MethodConstraintValidator { public void initialize(DateParameterCheck constraint) {} public boolean isValid(Customer customer, Date from, Date to, ConstraintValidatorContext context) { return from.before(to); } } Such a validator would need an isValid() method for each method annotated with the constraint. You found that "quite interesting" :) Hardy also expressed some constraints that such a parameter constraint couldn't be easily distinguished from a return value constraint. > BTW I forgot but do we support Constructor validation? IMO for the sake of completeness we should. This wouldn't add much conceptual complexity or implementation efforts. Though probably not all integration layers (such as Seam Validation/CDI) would use it depending on their interception mechanism. I guess it's best to continue the discussion at BVAL-232. --Gunnar [1] http://lists.jboss.org/pipermail/hibernate-dev/2011-February/005997.html [2] http://opensource.atlassian.com/projects/hibernate/browse/HV-409 2011/7/8 George Gastaldi : > I think you should. > > 2011/7/8 Emmanuel Bernard > >> BTW I forgot but do we support Constructor validation? >> >> On 8 juil. 2011, at 16:02, Emmanuel Bernard wrote: >> >> > Do you think we should support the following? >> > >> > @DateInOrder >> > void bookHotel(@NotNull @Valid Customer customer, Date from, Date to); >> > >> > ie like we have properly level and class level constraints, we could get >> method level constraints receiving all the parameters and raising >> constraints violations if necessary. >> > >> > Emmanuel >> > ___ >> > hibernate-dev mailing list >> > hibernate-dev@lists.jboss.org >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >> >> >> ___ >> hibernate-dev mailing list >> hibernate-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/hibernate-dev >> > ___ > hibernate-dev mailing list > hibernate-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/hibernate-dev > ___ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev