Re: equals() method for LoadableDetachableModels

2014-07-14 Thread Martin Grigorov
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 mgrigo...@apache.org
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 s...@meiers.net 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 s...@meiers.net 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

2014-07-12 Thread Martin Grigorov
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 s...@meiers.net 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 s...@meiers.net 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

2014-07-11 Thread Sven Meier

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

2014-07-11 Thread Ernesto Reinaldo Barreiro
Hi,


On Fri, Jul 11, 2014 at 9:51 AM, Sven Meier s...@meiers.net 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

2014-07-11 Thread Martin Grigorov
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 s...@meiers.net 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

2014-07-11 Thread Boris Goldowsky
Thank you all for your replies, very helpful!

Sven Meier s...@meiers.netmailto: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 reier...@gmail.commailto: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 mgrigo...@apache.orgmailto: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

2014-07-11 Thread Sven Meier

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 s...@meiers.net 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