No worries!
2013/11/22 Roger Kratz <[email protected]> > @Oskar, @Patrick > > > > First – thanks for the feedback. > > > > After reading Patrick’s reply I said to myself “No? The hash code isn’t > changing during the test!”. After looking at it again I saw I was wrong… > Can’t believe I didn’t see the gethashcode impl for the KeyEntity I was > using in the test (that I sort of copied from Envers test base which was > ported from Hibernate Envers code base). Now it makes sense what you were > saying. > > > > To summarize… > > · Really not an issue. When hash code (and/or equals) doesn’t > change, everything works as expected. > > · IF changing the key (and its hash changes), in NH4 it throws > while NH3 it doesn’t. Even though it’s sort of a breaking change, it’s an > illegal op so need to fix it (maybe it would be better with a more explicit > exception though). > > > > > > I’ll rewrite red Envers tests instead. (I’m a bit surprised these ported > tests are there in the first place though – AFAIK also in Java, hash of a > key is not allowed to be changed after put in a dic.) > > > > With great shame I feel sorry wasting your time L, even though I really > appreciate the feedback. Hopefully an Envers version targeting NH 4 now can > be released soon. > > > > Feel free to reject Jira issue and pull request. > > > > All the best, > > Roger > > > > > > *From:* [email protected] [mailto: > [email protected]] *On Behalf Of *Patrick Earl > *Sent:* den 22 november 2013 16:57 > *To:* nhibernate-development > *Subject:* Re: [nhibernate-development] NH-3555 > > > > In the pull request, the hash code is changing while the entry is in the > dictionary. However, I'd agree that aside from the insanity of changing a > key inside of a dictionary, the more expected behaviour would be to only > end up with one entry. While I'm quite unfamiliar with the dictionary side > of NHibernate, I'd suggest a couple minor changes to the code: > > > > 1. Use xmapvalue instead of looking up the value again in the second if. > > 2. In the unit test, assert that the number ends up being 20. > > > > Regards, > > Patrick Earl > > > > > > On Wed, Nov 20, 2013 at 5:54 AM, Roger Kratz <[email protected]> > wrote: > > Yes, forgot to mention… In this case the key instance is the very same > instance, just modify its data so no changes that affects Equals or > GetHashcode. > > > > I just pushed my proposal of fix to the old pull request > > https://github.com/nhibernate/nhibernate-core/pull/229 > > > > If you don’t agree I can fix Envers tests (there’s lot of old ported Java > tests currently failing) instead, but I’ll be happy to get some input first. > > > > *From:* [email protected] [mailto: > [email protected]] *On Behalf Of *Oskar Berggren > *Sent:* den 20 november 2013 13:39 > *To:* [email protected] > *Subject:* Re: [nhibernate-development] NH-3555 > > > > Changing something that affects Equals()/GetHashCode() of an object while > that object is the key of a dictionary isn't allowed by the IDictionary > contract. What behavior should we expect? > > /Oskar > > > > 2013/11/20 Roger Kratz <[email protected]> > > Hi > > > > I reported regression bug 3555 some time ago. This issue currently > prevents releasing an Envers version targeting NH 4. > > > > I’m sitting trying to fixing the issue myself, but run into an issue > whether old behavior is correct or not. > > > > What the issue is all about is… > > > > var key = new KeyEntity(); > > var value = new ValueEntity(); > > var entity = new MapEntity(); > > entity.SomeMapProp[key] = value; (1) > > > > [open session] > > [begin tran] > > [save entity] > > [commit tran] > > [begin tran] > > Key.SomeProperty = newvalue; (2) > > [commit tran] > > [/close session] > > > > ….in other words; changing the key object of an entry in a map/dictionary. > > > > In current NH 4 code base, code above throws. I’ve fixed the problem > locally but noticed that my “fix” got another result than NH 3. In NH 3 > code above resulted in _*two*_ entries in the map – both (1) and (2), > while my local fix resulted in one entry (only 2). I personally think the > behavior in NH 3 was wrong. > > > > I’ll be glad to fix this issue so a new Envers version could be released > together with NH 4 alpha 2 but I need to double check with you first what > the “correct” behavior is in this case. Any opinions? > > > > All the best, > > Roger > > > > > > > > -- > > --- > You received this message because you are subscribed to the Google Groups > "nhibernate-development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. > > > > -- > > --- > You received this message because you are subscribed to the Google Groups > "nhibernate-development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. > > -- > > --- > You received this message because you are subscribed to the Google Groups > "nhibernate-development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. > > > > -- > > --- > You received this message because you are subscribed to the Google Groups > "nhibernate-development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. > > -- > > --- > You received this message because you are subscribed to the Google Groups > "nhibernate-development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/groups/opt_out. > -- --- You received this message because you are subscribed to the Google Groups "nhibernate-development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
