EDIT

I do not agree with this solution. The up to now implementation is essential for almost everything we have done until now. If we soften it we do not know which algorithms do not work correct anymore even if no tests fail. Also it makes the correct understanding of equals more difficult.
Also I do not understand exactly why the updated timestamp should make a difference. For volatile objects this timestamp may not change if the object has been changed, so in the next step someone will say that we have to check each single value for equality.

Generally this is a highly sensitive issue that we shouldn't change without further discussion. It is also highly related to a correct transaction model (e.g. optimistic locking uses versioning (@Version, https://www.intertech.com/Blog/versioning-optimistic-locking-in-hibernate/ ) . A change in the equals method should take a correct implementation for a better transaction strategy into account (currently we use "last commit wins" which is definitely not the best). This can't be done in a minute and definitely needs further research.

The current contract is defined as "there is only one instance of the given database object per session and it is not possible to compare objects of 2 different sessions except for the question if they represent the same record in the database.

Certainly more arguing is needed but I commit this for now to stop the preliminary new implementation.

Andreas Kohlbecker wrote:

I changed the method to

/**
* Is true if UUID created and updated timestamp are the same for the passed Object and this one.
* @see eu.etaxonomy.cdm.model.common.CdmBase#equals(java.lang.Object)
* See {@link http://www.hibernate.org/109.html hibernate109}, {@link http://www.geocities.com/technofundo/tech/java/equalhash.html geocities}
* or {@link http://www.ibm.com/developerworks/java/library/j-jtp05273.html ibm}
* for more information about equals and hashcode.
*/
@Override
public boolean equals(Object obj) {

    if(!super.equals(obj)){
        return false;
    }

    if(!VersionableEntity.class.isAssignableFrom(obj.getClass())){
        return false;
    }

    return CdmUtils.nullSafeEqual(((VersionableEntity)obj).getUpdated(), this.getUpdated());
}

all tests in cdmlib are running successful with this. So I will commit this change if no one objects.

By the way, if we implement it finally in this way we should definitely update the javadoc as the links are not related to any updated field or so but only a copy of CdmBase.equals().

Hi Andreas,

I posted this provoking code as a suggestion and as an entry into the discussion. I never would have committed this crucial change without further research on the implications.

Your swift answer, for which I am grateful, contains all the concerns that I actually expected but also the point that drove me in starting this discussion:

The current contract is defined as "there is only one instance of the given database object per session and it is not possible to compare objects of 2 different sessions except for the question if they represent the same record in the database.

In my eyes, the assumption on which the contract is based exactly is the problem. Most client application code runs outside of the hibernate session context. That is, entities will stem from different session and thus are no longer guaranteed to be equal when VersionableEntity.equals() returns true.

On the other hand, using hibernate entities outside the session context, that is in the client application code, is an known antipattern and bad practice. Maybe we rather should guide the discussion in this directions and start thinking about using DTOs in general in clients.

Andreas

There is another possibility to compare instances of a given CdmBase class by using IMatchStrategy (see https://dev.e-taxonomy.eu/redmine/issues/7199#note-1)

Andreas Kohlbecker wrote:

I am re-opening this issue because there is one point raised in here which is still open:

... VersionableEntity.equals() we should remove it, since it does exactly the same as the method it overrides.

Do you agree with removing this method override?

Agreed and done: 51a26cec

great!

AM:

in letzter Zeit wurde von verschiedentlich nachgefragt, ob die equals Methode von CdmBase Klassen überschrieben werden kann.

Diskussion z.B. in https://dev.e-taxonomy.eu/redmine/issues/7155 und related tickets.

Ich habe diese Methode jetzt auf final gesetzt, so dass überschreiben nicht mehr möglich ist, mit entsprechendem javadoc.
In VersionableEntity habe ich die semantisch quasi gleiche override Methode entfernt. (Ohh ich sehe gerade, dass es noch mehr overrides gibt, das kläre ich gleich noch, compile Fehler).

Für Vergleiche stehen die Matching Klassen zur Verfügung oder eigene Implementierungen.

Eine Frage habe ich aber noch: In der derzeitigen Implementierung wird aus irgendeinem Grund noch das created Attribut verglichen.
Ich habe leider überhaupt keine Erinnerung mehr, warum wir das mal eingefügt haben. Falls da jemand eine Idee hat, würde mich das sehr interessieren.
Ansonsten sollten wir mal schauen, ob wir das experimentell mal rausnehmen und ob trotzdem alles noch funktioniert. Möglichst direkt nach einem Release.
Diskussion bitte in https://dev.e-taxonomy.eu/redmine/issues/7201 .

I moved the discussion on making CdmBase.equals final to #7202 as some classes directly inheriting from CdmBase still override equals().