Nicht ganz. Bei** instanceof EntityClass** testest Du ja auch nur die eine Richtung (ob Object o die gleiche oder eine Unterklasse ist). Gleiches machst Du mit isAssignablefrom auch. Also nur den einen Zweig Deiner or-Prüfung:
getClass().isAssignableFrom(o.getClass());
(Ich mag das this. nicht :twisted:)
Da funktioniert das ja auch. Bei isAssignableFrom wird ansonsten der equals-Contract verletzt:
Beispiel:
Sei x
eine Instanz von MyEntity
und y
ein dynamisch generierter Proxy vom Typ MyEntityProxy
. MyEntityProxy
ist Unterklasse von MyEntity
.
Daraus folgt MyEntity.class.isAssignableFrom(MyEntityProxy.class) == true
aber MyEntityProxy.class.isAssignableFrom(MyEntity.class) == false
.
Daher kann zwar x.equals(y) == true
gelten, aber es gilt immer y.equals(x) == false
. Die Symmetrieeigenschaft ist verletzt.
Ich habe das nicht 100%ig durchdacht, daher habe ich mein letztes Posting als Frage formuliert.
Dein Einwand mit der Symmetrie ist korrekt. DAS habe ICH nicht bis zu Ende gedacht. Deine Lösungsidee funktioniert aber nicht, weil
(o.getClass().isAssignableFrom(this.getClass()) || this.getClass().isAssignableFrom(o.getClass()))
auch true zurück gibt, wenn der Typ von o eine Superklasse von MyEntity ist (z.B. Object). Der Fall müsste also auch noch ausgeschlossen werden. Damit wird der Code dann aber so hässlich, dass ich inzwischen denke, es ist besser, wenn Du equals in den jeweiligen Klassen ausprogrammierst und dort dann instanceof mit konstanter Klasse prüfst…
nur die bestehende Zeile
if (!(o instanceof AbstractEarlyIdPersistable)) return false;
zu belassen, also eine Zeile mehr statt equals in potentiell 50 Unterklassen, halte ich nicht für häßlich,
falls einem dass zu langsam ist (oder zumindest vorkommt), zuviel Arbeit für equals-Methode, ok,
falls es reale Probleme gibt, vielleicht auch eigene Vererbung zwischen Entity-Klassen?, ok
aber wenn es wirklich so konsequent nach der Id geht und die Chance besteht hier mit einer Methode alle oder viele Klassen abzudecken,
dann sollte dies versucht werden
und in so einer zentralen Methode einer zentralen Klasse wären dann doch noch mehr Code, ruhig auch gleich 20 Zeilen extra für (vermeinliche) Performance, auch ok,
erstmal doch der Class-Vergleich, nur wenn unterschiedliche Klassen dann instanceof & Co. usw.,
[QUOTE=nillehammer]Deine Lösungsidee funktioniert aber nicht, weil
(o.getClass().isAssignableFrom(this.getClass()) || this.getClass().isAssignableFrom(o.getClass()))
auch true zurück gibt, wenn der Typ von o eine Superklasse von MyEntity ist (z.B. Object).[/QUOTE]
Stimmt, den Fall habe ich übersehen. Allerdings gibt equals dann nicht true zurück, sondern es sollte eine ClassCastException fliegen, weil o ja kein AbstractEarlyIdPersistable ist.
Ich finde, dass @SlaterB Recht hat. Es ist nur die eine Zeile vor dem Cast. Der Vorteil, dass man nicht vergessen kann, die Methode zu überschreiben, wiegt schwerer als der zusätzliche Check. Spezialfälle wie tiefere Vererbunghierarchien, bei denen entitypezifische IDs vergeben werden, können dann immer noch separat behandelt werden.
Dann ist das vorerst meine equals
-Methode:
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o.getClass().isAssignableFrom(getClass()) || getClass().isAssignableFrom(o.getClass()))) return false;
if (!(o instanceof AbstractEarlyIdPersistable)) return false;
AbstractEarlyIdPersistable that = (AbstractEarlyIdPersistable) o;
return id.equals(that.id);
}
Soo schlimm finde ich das gar nicht.
Edit: in #14 habe ich übrigens das abstract
bei der Klassendeklaration vergessen.
Da habt Ihr beide Recht. In Kombination sieht das garnicht so schlecht aus. Ich würd nur vielleicht zuerst die instanceof-Prüfung machen oder sie sogar in die ||-Kaskade als ersten Zweig aufnehmen.
Und nur der Vollständigkeit halber:
Ich bezog mich in meinem letzten Post nur auf die Prüfung, die ich hingeschrieben habe, weil das der entscheidende Teil war.
Klar. Ich weiß auch nicht, was ich da in dem Moment gelesen hatte
Mit dem Gesamtproblem bin ich jetzt an einem Punkt angekommen, an dem für jede Entity drei Klassen erstellt werden müssen (weniger geht glaube ich auch gar nicht mehr).
Das sieht dann so aus:
Wie man sieht, ist das einmal der IdGenerator
-Zweig, bestehend aus dem IdGenerator
-Interface:
@FunctionalInterface
public interface IdGenerator {
int nextId();
}
dessen Implementierung LocalIdGenerator
:
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.context.annotation.Scope;
import org.springframework.stereotype.Component;
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
import java.util.concurrent.atomic.AtomicInteger;
@ThreadSafe
@Component
@Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE)
public class LocalIdGenerator implements IdGenerator {
private final AtomicInteger currentId;
public LocalIdGenerator(@Nullable Integer initialId) {
currentId = new AtomicInteger(initialId == null ? 0 : initialId);
}
@Override
public int nextId() {
return currentId.incrementAndGet();
}
}
und einer weiteren Abstraktionsschicht, um die Implementierung des IdGenerator
s später problemlos austauschen zu können:
import org.springframework.stereotype.Component;
import javax.annotation.Nullable;
@Component
public class IdGeneratorFactory {
public IdGenerator createIdGenerator(@Nullable Integer initialId) {
return new LocalIdGenerator(initialId);
}
}
Für den Datenbankzugriff gibt es die Repositories:
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
public interface MyEntityRepository extends JpaRepository<MyEntity, Integer> {
@Query("select max(e.id) from MyEntity e")
Integer getLastId();
}
Und schlussendlich gibt es noch die Entity-Hierarchie, die oben mit AbstractEarlyIdPersistable
beginnt:
import org.springframework.data.domain.Persistable;
import javax.annotation.Nonnull;
import javax.persistence.*;
import java.io.Serializable;
import static com.google.common.base.Preconditions.checkNotNull;
@MappedSuperclass
public abstract class AbstractEarlyIdPersistable<ID extends Serializable> implements Persistable<ID> {
private static final long serialVersionUID = -5020502021329768695L;
@Id
private ID id;
@Transient
private boolean persisted = false;
protected AbstractEarlyIdPersistable() {
}
public AbstractEarlyIdPersistable(@Nonnull ID id) {
this.id = checkNotNull(id);
}
@Override
public ID getId() {
return id;
}
@Override
public boolean isNew() {
return !persisted;
}
@PrePersist
private void checkId() {
if (id == null)
throw new IllegalStateException(String.format("Entity of class %s should get persistet but has no id set.",
this.getClass().getCanonicalName()));
}
@PostPersist
@PostLoad
private void markPersisted() {
persisted = true;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof AbstractEarlyIdPersistable) ||
!(o.getClass().isAssignableFrom(getClass()) || getClass().isAssignableFrom(o.getClass())))
return false;
AbstractEarlyIdPersistable that = (AbstractEarlyIdPersistable) o;
return id.equals(that.id);
}
@Override
public int hashCode() {
return id.hashCode();
}
}
und dann mit der Entity weiter geht:
@Entity
public class MyEntity extends AbstractEarlyIdPersistable<Integer> {
private static final long serialVersionUID = -60470266393164538L;
private String field1;
private String field2;
protected MyEntity() {
}
public MyEntity(int id,
@Nonnull String field1,
@Nonnull String field2) {
super(id);
this.field1 = checkNotNull(field1);
this.field2 = checkNotNull(field2);
}
public String getField1() {
return field1;
}
public String getField2() {
return field2;
}
}
In der Factory laufen die Fäden zusammen:
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import xxx.domain.support.IdGenerator;
import xxx.domain.support.IdGeneratorFactory;
import javax.annotation.Nonnull;
@Component
public class MyEntityFactory {
private final IdGenerator idGenerator;
@Autowired
public MyEntityFactory(MyEntityRepository repository, IdGeneratorFactory idGeneratorFactory) {
idGenerator = idGeneratorFactory.createIdGenerator(repository.getLastId());
}
public MyEntity createEntity(@Nonnull String param1, @Nonnull String param2) {
return new MyEntity(idGenerator.nextId(), param1, param2);
}
}
Die schwierigste Entscheidung war dabei, die Factory sowohl das Repository als auch die IdGeneratorFactory
kennen zu lassen, weil ich sie eigentlich gern vom Repository unabhängig hätte. Ich habe bisher aber noch keinen Weg gefunden (ohne pro Entity noch konkrete IdGenerator
en einzuführen, die dann wiederum das zugehörige Repository kennten), diese Abhängigkeit dort heraus zu halten.
Das ist alles recht Springlastig, das Konzept ist aber problemlos auf andere DI-Frameworks übertragbar. Die Repositories müsste man dann natürlich auch von Hand ausprogrammieren.
Wenn jemand zu den genannten Problemen noch Tipps oder Verbesserungsvorschläge hat, würde ich mich freuen.
Ich habe bis hierher interessiert mitgelesen, und muss jetzt einfach mal fragen, welchen Sinn das alles hat. Ist ja reichlich kompliziert geworden
(vorab: ich hab wirklich keine Ahnung von DDD)
Muss mich einfach wundern als alter Mann: wozu braucht man künstliche Primärschlüssel für Entities BEVOR diese persistiert werden?
Die Identität einer Entity hängt bei Domänengetriebener Entwicklung einzig und allein von der … Identität der Entity ab. Das bedeutet logischerweise, dass in einer equals-Methode einzig und allein diese ID als Vergleichswert zulässig ist. Das wiederum hat zur Folge, dass die ID bereits vor der Persistierung bekannt sein muss.
Doch nur dann, wenn man die Entity vor der Persistierung in ein .equals steckt. Aber wann und warum sollte man das machen, wozu braucht man in der DDD eine Entity in der Schwangerschaft…Kann es sein, dass der herkömmliche Gang der Dinge
- Irgendwoher kommen ein Schwung Daten
- Ein oder mehrere Entities erzeugen mit „LEEREM“ Primärschlüssel
- In einer Transaktion alle in die Datenbank, dabei bekommt jede (von mir aus von der Datenbank per Sequenz, Autoincrement, …) eine ID verpasst
- Fertig
Wie oft - und warum - gibt es bei der DDD überhaupt die Notwendigkeit, mit Entities [die noch nicht persistiert wurden, aber irgendwann werden] großartige Berechnungen oder Aktivitäten durchzuführen? Gerade in hochgradig verteilten System ist ein vom RDBMS generierter Primärschlüssel doch Gold wert und recht einfach zu benutzen…
weil es wieder mehr zur allgemeinen Frage, ob equals nach Id schauen sollte, geht, was quasi ein Vorgänger-Thread war,
und damit dieser Thread hier auch gleich etwas reiner gehalten werden kann,
die letzten Postings nach http://forum.byte-welt.net/threads/11644-JPA-equals()-und-hashcode()?p=97571&viewfull=1#post97571 verschoben
In einem verteilten System funktioniert das aber nicht oder? Weil was machst du, wenn 2 Repositories gleichzeitig die letzte Id aus der Datenbank abgreifen und du dann 2 IDGeneratoren mit gleicher ID hast
Du könntest dir in der DB eine Liste mit IDGeneratoren machen. Jedem IDGenerator wieder eine kurze ID und die dann den generierten IDs vorangestellt wird.
Doch, das ginge sogar ziemlich einfach. Auf jedem Knoten muss natürlich konfiguriert sein, der „wievielte“ er ist. Also bei 3 Knoten müsste vergeben werden, dass ein Knoten Knoten 0, einer Knoten 1 und einer Knoten 2 ist.
import javax.annotation.Nullable;
import javax.annotation.concurrent.ThreadSafe;
import java.util.concurrent.atomic.AtomicInteger;
@ThreadSafe
public class LocalMultiNodeIdGenerator implements IdGenerator {
private final AtomicInteger currentId;
private final int nodeCount;
public LocalMultiNodeIdGenerator(@Nullable Integer initialId, int nodeNumber, int nodeCount) {
currentId = new AtomicInteger(initialId == null
? nodeNumber
: initialId - initialId % nodeCount + nodeCount + nodeNumber);
this.nodeCount = nodeCount;
}
@Override
public int nextId() {
return currentId.addAndGet(nodeCount);
}
}
Der Konstruktor liest sich noch nicht so gut (edit: und überspringt auch nodeCount Knoten, falls der Modulus 0 ist), ist aber nur schnell dahinprogrammiert, weil ich noch kein Multinode-Setup habe.
Außerdem müsste sich natürlich noch die Factory ändern:
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;
import javax.annotation.Nullable;
@Component
public class IdGeneratorFactory {
private final int nodeNumber;
private final int nodeCount;
@Autowired
public IdGeneratorFactory(@Value("0") int nodeNumber, @Value("3") int nodeCount) {
this.nodeNumber = nodeNumber;
this.nodeCount = nodeCount;
}
public IdGenerator createIdGenerator(@Nullable Integer initialId) {
return new LocalMultiNodeIdGenerator(initialId, nodeNumber, nodeCount);
}
}
In den @Value
-Annotationen stünde dann natürlich ein SpEL-Ausdruck, der die passenden Werte aus einer Konfigurationsdatei oder wo auch immer her holt.
Es sind natürlich unendlich viele Variationen möglich. Am interessantesten sind dabei wohl Varianten, bei denen sich die Anzahl der Knoten zur Laufzeit ändern kann.
Und wenn du im Betrieb einen Knoten hinzufügen willst, jedes mal neu compilieren? Ich weis ja jetzt nicht in wiefern diese IdGeneratoren skalierbar sein sollen, es kommt mir nur unnötig kompliziert vor und es gibt bessere best practices (siehe mein link oben).
Ich weiß jetzt wirklich nicht, wo du das hernimmst. Ich habe oben doch geschrieben, dass bei einem Einsatz eines solchen IdGenerators in der Value-Annotation eine Spring-Expression stünde (wodurch die Werte durch eine Konfigurationsdatei oder einen Konfigurationsserver vergeben werden können) und keine fest eingestellten Werte. Und wenn man sich noch ein kleines Bisschen mehr Mühe gibt, kann man die Werte auch zur Laufzeit ändern, muss die Änderung eben nur bis zu den IdGeneratoren propagieren.
Die Id-Vergabe entspricht der eines MySQL Multi-Master-Betriebs.
Und dass die Vorgehensweisen, die in den Folien beschrieben werden, allgemeingültige Best Practices sind, halte ich für ein Gerücht. In einigen Anwendungsfällen funktioniert das sicherlich gut. In einer NoSQL-Datenbank sind UUIDs wahrscheinlich auch super IDs. Das trifft aber nicht auf ein RDBMS zu, bei dem die IDs auch in zig Tabellen als Fremdschlüssel auftauchen.
Die Vorgehensweise taugt sicherlich in vielen Fällen - in diesem aber nicht.
Ich habe mich dabei gar nicht auf UUIDs bezogen sondern z.B auf Twitter Snowflake. Mir kommt aber vor du nimmst das ganze ein wenig zu persönlich, ich will dein Id System ja auch nicht schlechtreden. Ich befürchte nur es könnte dir durch ein klassisches Lost Update beim holen der currentID um die Ohren fliegen.
Die Snowflake wäre in der Tat eine Möglichkeit. Sie hat auch alle Eigenschaften, die man in einem RDBMs benötigt (aufsteigend sortiert, passt in ein BIGINT).
Ich nehme das nicht persönlich. Es tut mir leid, wenn das so rüber gekommen ist. Mir ging es vorerst um die technische Umsetzung (also was kann ich wo injizieren und initialisieren, um möglichst wenig Abhängigkeiten zu erhalten und das System möglichst anpassungsfähig zu bekommen).
Ein System, das ohne Initialisierung auskommt vereinfacht das natürlich. Bei der Snowflake ist aber auch eine Initialisierung notwendig (die Maschinen-ID).
Das einzige was sich dann ändert, ist die Implementierung des LocalMultiNodeIdGenerators (das Interface sollte ich vorher aber schon auf long anpassen). Dort steckt dann kein AtomicLong drin, sondern der „Snowflake-Algorithmus“.
Bei dem geposteten LocalMultiNodeIdGenerator wüsste ich nicht, wo es ein Lost Update geben kann, weil jeder Knoten seine zugewiesenen „Scheibchen“ vom ID-Pool hat.
Wenn man die Knotenzahl im laufenden Betrieb ändert, müsste man sich natürlich eine Synchronisierungsstrategie überlegen, damit es keine Probleme gibt.
Ich befürchte folgendes Szenario:
Knoten A fährt hoch.
Knoten A holt currentID von Repository: currentID = 0
A erstellt 2 IDs ohne Persistierung: IDs= 1, 2
Knoten B fährt hoch.
Knoten B holt currentID von Repository: currentID = 0
B erstellt eine ID: ID = 2
Aber ich kann mich ja auch täuschen.
Der Fall kann nicht eintreten, weil Knoten A (bzw. Knoten 0) alle IDs generiert, die gerade sind. Knoten B generiert nur ungerade IDs. Bei 3 Knoten generiert Knoten 0 die Sequenz 0, 3, 6, 9…, Knoten 1 die Sequenz 1, 4, 7, 10… und Knoten 2 die Sequenz 2, 5, 8, 11… Deshalb habe ich die Initialisierung mit dem Modulus gemacht.
Wenn man einen pragmatischen Ansatz geht, blockt man gleich eine größere Anzahl an Knoten, als man bereits hat. Wenn man diese willkürlich gewählte Knotenzahl auf 1024 setzt, dann ist man schon fast bei der Snowflake. Dazu fehlt dann nur noch die Zeitkomponente.
Ein Snowflakegenerator sähe so aus (habe das IdGenerator-Interface mittlerweile auf long umgestellt):
import javax.annotation.concurrent.ThreadSafe;
import java.util.concurrent.atomic.AtomicInteger;
import static com.google.common.base.Preconditions.checkArgument;
@ThreadSafe
public class SnowflakeIdGenerator implements IdGenerator {
private final int machineId;
private final AtomicInteger sequence = new AtomicInteger();
public SnowflakeIdGenerator(int machineId) {
checkArgument(machineId >= 0 && machineId < 1024, "machineId must be between 0 and 1023.");
this.machineId = machineId << 12;
}
@Override
public long nextId() {
return System.currentTimeMillis() << 22 | machineId | sequence.incrementAndGet() & 0xfff;
}
}
*** Edit ***
Der Snowflakegenerator hat aber einen Nachteil: er kann maximal 4096 IDs in einer Millisekunde auf einem Knoten generieren (das scheint selbst bei Twitter kein reales Problem zu sein). Und irgendwann im Jahre 2039 ist Schluss mit der ID-Generierung.
die naheliegende Frage ist doch, was passiert bzw. ob es möglich ist, einen weiteren, in diesem Fall 4. Knoten aufzunehmen?,
zu „neu compilieren“ wie von bERt0r in #32 genannt, gibt es keinen Anlass, schließlich funktioniert alles dynamisch nach Parametern
aber man muss, sofern so eine Änderung erlaubt sein soll, wohl neustarten oder Methode zur Umkonfiguration anbieten?
mit initialId bestände die Möglichkeit, bereits vorhandene Ids zu berücksichtigen, erst im Bereich darüber neu zu beginnen
Diesen Punkt halte ich auch für die größte Schwierigkeit. Man könnte den pragmatischen Weg gehen, wie Twitter es getan hat, und einfach eine fixe, willkürliche Grenze setzen und pauschal nur jede 1024te ID generieren. Dann kann man problemlos Cluster von 1024 Knoten bilden. Danach ist dann aber Schluss.
Alternativ müssten die Knoten sich „absprechen“. Dabei könnte man eine ID wählen, die gemäß Knotenauslastung „relativ weit“ in der Zukunft liegt. Diese ID wird allen Knoten mitgeteilt. Dazu müsste das IdGenerator-Interface um eine Methode zum rekonfigurieren erweitert werden. Da gibt es dann aber knifflige Randfälle. Was soll z. B. passieren, wenn ein Knoten nicht schnell genug erreicht wird und schon über die vereinbarte ID hinaus ist? Dann gibt es eine Kollision.
Dem kann man natürlich vorbeugen, indem man die neue Start-ID sehr pessimistisch wählt.
als seltenes Ereignis erinnert mich das an etwas aus Studium,
auch wenn für sich als Idee schnell denkbar, nur zusätzlich interessant dass nachlesbar:
‚Zwei-Phasen-Commit-Protokoll‘
hier in etwa:
- Abstimmungsphase: erst alle Knoten abfragen dass die ihre grundsätzliche Erreichbarkeit und Bereitschaft äußern,
nun vorerst Pause machen, keine Ids mehr in ihrem Knoten rausrücken, und die höchste bisher vergebene Id an den Anfrager zurückliefern
(welche vielleicht noch nicht in der DB zu sehen ist) - Commit: wenn alles bereit dann mit höchster Id wieder bei allen melden,
- Rollback: anderenfalls wohl ernste Probleme, vielleicht aber trotzdem die Knoten wie bisher weiterarbeiten lassen, dazu auch Info nötig, die warten ja