Re: equals() method for LoadableDetachableModels
https://issues.apache.org/jira/browse/WICKET-5642 Martin Grigorov Wicket Training and Consulting https://twitter.com/mtgrigorov On Sat, Jul 12, 2014 at 7:22 PM, Martin Grigorov wrote: > I'll test with IdentityHashMap with our main app soon. > > Martin Grigorov > Wicket Training and Consulting > https://twitter.com/mtgrigorov > > > On Fri, Jul 11, 2014 at 3:06 PM, Sven Meier wrote: > >> Hi, >> >> > really need to use #equals() ... >> >> > checking for identity equality should be OK too >> >> I've just tried with an IdentityHashMap and all tests passed (except >> SerializableCheckerTest#runtimeExceptionTolerance() of course). >> >> Sven >> >> >> >> On 07/11/2014 11:39 AM, Martin Grigorov wrote: >> >>> Hi, >>> >>> I agree that using the primary key should be enough for #equals() but >>> does org.apache.wicket.core.util.objects.checker. >>> CheckingObjectOutputStream#check() >>> really need to use #equals() (via Stack#contains()) ? >>> I think checking for identity equality should be OK too. Java >>> Serialization >>> is not the simplest part of Java and I may miss something... >>> >>> On Fri, Jul 11, 2014 at 10:51 AM, Sven Meier wrote: >>> >>> Hi, detachable models should never use getObject() in their implementation > of > equals()? generally this is a good advice: there are several places in Wicket checking for model equality (e.g. ReuseIfModelsEqualStrategy). A detachable model should have enough information to decide equality without loading the model object. Regards Sven On 07/11/2014 01:52 AM, Boris Goldowsky wrote: I’ve started using CheckingObjectOutputStream to test for models of > database objects that are not properly detached, and find it very > useful. > > However I just diagnosed (after many hours of frustration) that it can > also cause problems rather than solve them. > CheckingObjectOutputStream > causes the equals() method of models to be called, and in the case of > our > models the equals() implementation loads the database object (models > of the > same object being considered equal to each other). Since it’s doing > this > during serialization and thus after the detach process, the checker > actually causes the exact problem that it’s supposed to prevent (and > doesn’t generate any warnings in this case). > > Is it a general rule, that I was just not aware of, that detachable > models should never use getObject() in their implementation of > equals()? > Or should CheckingObjectOutputStream be changed to avoid calling > equals() ? > > Bng > > > - > To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org > For additional commands, e-mail: users-h...@wicket.apache.org > > > - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org >> >> - >> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org >> For additional commands, e-mail: users-h...@wicket.apache.org >> >> >
Re: equals() method for LoadableDetachableModels
I'll test with IdentityHashMap with our main app soon. Martin Grigorov Wicket Training and Consulting https://twitter.com/mtgrigorov On Fri, Jul 11, 2014 at 3:06 PM, Sven Meier wrote: > Hi, > > > really need to use #equals() ... > > > checking for identity equality should be OK too > > I've just tried with an IdentityHashMap and all tests passed (except > SerializableCheckerTest#runtimeExceptionTolerance() of course). > > Sven > > > > On 07/11/2014 11:39 AM, Martin Grigorov wrote: > >> Hi, >> >> I agree that using the primary key should be enough for #equals() but >> does org.apache.wicket.core.util.objects.checker. >> CheckingObjectOutputStream#check() >> really need to use #equals() (via Stack#contains()) ? >> I think checking for identity equality should be OK too. Java >> Serialization >> is not the simplest part of Java and I may miss something... >> >> On Fri, Jul 11, 2014 at 10:51 AM, Sven Meier wrote: >> >> Hi, >>> >>> >>> detachable models should never use getObject() in their implementation of >>> equals()? >>> >>> generally this is a good advice: there are several places in Wicket >>> checking for model equality (e.g. ReuseIfModelsEqualStrategy). >>> A detachable model should have enough information to decide equality >>> without loading the model object. >>> >>> Regards >>> Sven >>> >>> >>> >>> On 07/11/2014 01:52 AM, Boris Goldowsky wrote: >>> >>> I’ve started using CheckingObjectOutputStream to test for models of database objects that are not properly detached, and find it very useful. However I just diagnosed (after many hours of frustration) that it can also cause problems rather than solve them. CheckingObjectOutputStream causes the equals() method of models to be called, and in the case of our models the equals() implementation loads the database object (models of the same object being considered equal to each other). Since it’s doing this during serialization and thus after the detach process, the checker actually causes the exact problem that it’s supposed to prevent (and doesn’t generate any warnings in this case). Is it a general rule, that I was just not aware of, that detachable models should never use getObject() in their implementation of equals()? Or should CheckingObjectOutputStream be changed to avoid calling equals() ? Bng - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org - >>> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org >>> For additional commands, e-mail: users-h...@wicket.apache.org >>> >>> >>> > > - > To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org > For additional commands, e-mail: users-h...@wicket.apache.org > >
Re: equals() method for LoadableDetachableModels
Hi, > really need to use #equals() ... > checking for identity equality should be OK too I've just tried with an IdentityHashMap and all tests passed (except SerializableCheckerTest#runtimeExceptionTolerance() of course). Sven On 07/11/2014 11:39 AM, Martin Grigorov wrote: Hi, I agree that using the primary key should be enough for #equals() but does org.apache.wicket.core.util.objects.checker.CheckingObjectOutputStream#check() really need to use #equals() (via Stack#contains()) ? I think checking for identity equality should be OK too. Java Serialization is not the simplest part of Java and I may miss something... On Fri, Jul 11, 2014 at 10:51 AM, Sven Meier wrote: Hi, detachable models should never use getObject() in their implementation of equals()? generally this is a good advice: there are several places in Wicket checking for model equality (e.g. ReuseIfModelsEqualStrategy). A detachable model should have enough information to decide equality without loading the model object. Regards Sven On 07/11/2014 01:52 AM, Boris Goldowsky wrote: I’ve started using CheckingObjectOutputStream to test for models of database objects that are not properly detached, and find it very useful. However I just diagnosed (after many hours of frustration) that it can also cause problems rather than solve them. CheckingObjectOutputStream causes the equals() method of models to be called, and in the case of our models the equals() implementation loads the database object (models of the same object being considered equal to each other). Since it’s doing this during serialization and thus after the detach process, the checker actually causes the exact problem that it’s supposed to prevent (and doesn’t generate any warnings in this case). Is it a general rule, that I was just not aware of, that detachable models should never use getObject() in their implementation of equals()? Or should CheckingObjectOutputStream be changed to avoid calling equals() ? Bng - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org
Re: equals() method for LoadableDetachableModels
Thank you all for your replies, very helpful! Sven Meier mailto:s...@meiers.net>> wrote: >detachable models should never use getObject() in their implementation of >equals()? generally this is a good advice: there are several places in Wicket checking for model equality (e.g. ReuseIfModelsEqualStrategy). A detachable model should have enough information to decide equality without loading the model object. It would be good to note this somewhere in the documentation where model implementers will find it. Ernesto Reinaldo Barreiro mailto:reier...@gmail.com>> wrote: Shouldn't the serialize "primary key"=the bit of info needed to recover data on attach stored on LDM be the criteria for equality? Yes, and I will look into changing our implementation to do this. However it is more complicated, since our LDM [*] also supports constructing a model based on a query rather than an ID, or storing a not-persisted object directly, so we’d have to do considerably more than just compare IDs. And you could have a query-based model and an ID-based model that resolve to the same object — it’s perhaps a philosophical question whether those ought to compare equal or not. The practical question, of course, is whether changing the definition of equality will break any existing code. [*] we are using a version of the HibernateObjectModel from the no-longer-maintained Databinder (see https://github.com/n8han/Databinder-for-Wicket/ ) updated for wicket 6 and recent Hibernate versions. BTW if anyone is interested in collaborating on maintaining and improving this library that mediates between Wicket & Hibernate, please get in touch. Martin Grigorov mailto:mgrigo...@apache.org>> wrote: does org.apache.wicket.core.util.objects.checker.CheckingObjectOutputStream#check() really need to use #equals() (via Stack#contains()) ? I think checking for identity equality should be OK too I was wondering the same thing; using identity would certainly be safer for issues like these. Boris
Re: equals() method for LoadableDetachableModels
Hi, I agree that using the primary key should be enough for #equals() but does org.apache.wicket.core.util.objects.checker.CheckingObjectOutputStream#check() really need to use #equals() (via Stack#contains()) ? I think checking for identity equality should be OK too. Java Serialization is not the simplest part of Java and I may miss something... On Fri, Jul 11, 2014 at 10:51 AM, Sven Meier wrote: > Hi, > > > >detachable models should never use getObject() in their implementation of > equals()? > > generally this is a good advice: there are several places in Wicket > checking for model equality (e.g. ReuseIfModelsEqualStrategy). > A detachable model should have enough information to decide equality > without loading the model object. > > Regards > Sven > > > > On 07/11/2014 01:52 AM, Boris Goldowsky wrote: > >> I’ve started using CheckingObjectOutputStream to test for models of >> database objects that are not properly detached, and find it very useful. >> >> However I just diagnosed (after many hours of frustration) that it can >> also cause problems rather than solve them. CheckingObjectOutputStream >> causes the equals() method of models to be called, and in the case of our >> models the equals() implementation loads the database object (models of the >> same object being considered equal to each other). Since it’s doing this >> during serialization and thus after the detach process, the checker >> actually causes the exact problem that it’s supposed to prevent (and >> doesn’t generate any warnings in this case). >> >> Is it a general rule, that I was just not aware of, that detachable >> models should never use getObject() in their implementation of equals()? >> Or should CheckingObjectOutputStream be changed to avoid calling equals() ? >> >> Bng >> >> >> - >> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org >> For additional commands, e-mail: users-h...@wicket.apache.org >> >> > > - > To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org > For additional commands, e-mail: users-h...@wicket.apache.org > >
Re: equals() method for LoadableDetachableModels
Hi, On Fri, Jul 11, 2014 at 9:51 AM, Sven Meier wrote: > Hi, > > > >detachable models should never use getObject() in their implementation of > equals()? > > generally this is a good advice: there are several places in Wicket > checking for model equality (e.g. ReuseIfModelsEqualStrategy). > A detachable model should have enough information to decide equality > without loading the model object. > Shouldn't the serialize "primary key"=the bit of info needed to recover data on attach stored on LDM be the criteria for equality? > Regards > Sven > > > > On 07/11/2014 01:52 AM, Boris Goldowsky wrote: > >> I’ve started using CheckingObjectOutputStream to test for models of >> database objects that are not properly detached, and find it very useful. >> >> However I just diagnosed (after many hours of frustration) that it can >> also cause problems rather than solve them. CheckingObjectOutputStream >> causes the equals() method of models to be called, and in the case of our >> models the equals() implementation loads the database object (models of the >> same object being considered equal to each other). Since it’s doing this >> during serialization and thus after the detach process, the checker >> actually causes the exact problem that it’s supposed to prevent (and >> doesn’t generate any warnings in this case). >> >> Is it a general rule, that I was just not aware of, that detachable >> models should never use getObject() in their implementation of equals()? >> Or should CheckingObjectOutputStream be changed to avoid calling equals() ? >> >> Bng >> >> >> - >> To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org >> For additional commands, e-mail: users-h...@wicket.apache.org >> >> > > - > To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org > For additional commands, e-mail: users-h...@wicket.apache.org > > -- Regards - Ernesto Reinaldo Barreiro
Re: equals() method for LoadableDetachableModels
Hi, >detachable models should never use getObject() in their implementation of equals()? generally this is a good advice: there are several places in Wicket checking for model equality (e.g. ReuseIfModelsEqualStrategy). A detachable model should have enough information to decide equality without loading the model object. Regards Sven On 07/11/2014 01:52 AM, Boris Goldowsky wrote: I’ve started using CheckingObjectOutputStream to test for models of database objects that are not properly detached, and find it very useful. However I just diagnosed (after many hours of frustration) that it can also cause problems rather than solve them. CheckingObjectOutputStream causes the equals() method of models to be called, and in the case of our models the equals() implementation loads the database object (models of the same object being considered equal to each other). Since it’s doing this during serialization and thus after the detach process, the checker actually causes the exact problem that it’s supposed to prevent (and doesn’t generate any warnings in this case). Is it a general rule, that I was just not aware of, that detachable models should never use getObject() in their implementation of equals()? Or should CheckingObjectOutputStream be changed to avoid calling equals() ? Bng - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org
equals() method for LoadableDetachableModels
I’ve started using CheckingObjectOutputStream to test for models of database objects that are not properly detached, and find it very useful. However I just diagnosed (after many hours of frustration) that it can also cause problems rather than solve them. CheckingObjectOutputStream causes the equals() method of models to be called, and in the case of our models the equals() implementation loads the database object (models of the same object being considered equal to each other). Since it’s doing this during serialization and thus after the detach process, the checker actually causes the exact problem that it’s supposed to prevent (and doesn’t generate any warnings in this case). Is it a general rule, that I was just not aware of, that detachable models should never use getObject() in their implementation of equals()? Or should CheckingObjectOutputStream be changed to avoid calling equals() ? Bng - To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org