Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sammlungscode auf Doctrine umstellen #131

Open
j3nsch opened this issue Oct 22, 2021 · 6 comments · May be fixed by #216
Open

Sammlungscode auf Doctrine umstellen #131

j3nsch opened this issue Oct 22, 2021 · 6 comments · May be fixed by #216
Assignees

Comments

@j3nsch
Copy link
Member

j3nsch commented Oct 22, 2021

Für die Sammlungen wird in der Datenbank eine Baumstruktur abgebildet. Daran sind mehrere Klassen beteiligt.

Opus\Db\

  • Collections
  • CollectionsRoles
  • NestedSet

Im Zusammenhang mit Sammlungen gibt es weitere Klassen, die dann aber eine Ebene höher liegen und nicht direkt auf der Datenbankanbindung aufsetzen.

  • Opus\Model\Plugin\AbstractCollection
  • Opus\Collection\Plugin\DeleteSubTree
  • Opus\CollectionRole\Plugin\DeleteTree
  • Opus\Db\CollectionsEnrichments

Die Collection-Enrichments werden zur Zeit (noch) nicht genutzt. Es gibt aber geplante Erweiterungen für die sie nützlich sein könnten.

Die komplexeste Klasse ist NestedSet. Dort befindet sich der Code, um die Baumstruktur in der Datenbank abzubilden. Es sollte eine mehr oder weniger direkte Umstellung auf DBAL möglich sein.

Es gibt Erweiterungen für Doctrine 2, die NestedSets implementieren. Für die beiden Lösungen, die in der Doctrine 2 Dokumentation aufgelistet werden gibt es keine Releases. Die DoctrineExtensions sind eine weitere mögliche Lösung für die es Releases gibt und die anscheinend schon seit 10 Jahren gepflegt wird.

https://www.doctrine-project.org/projects/doctrine-orm/en/2.9/reference/limitations-and-known-issues.html
https://github.com/doctrine-extensions/DoctrineExtensions

Ein Umstieg auf DBAL ist der sicherere Weg im Augenblick und sollte mehr oder weniger eine Fleißarbeit sein. Eine externe Lösung hätte den Vorteil den Code im Framework zu reduzieren, aber würde für den aktuellen Umbau eine weitere unbekannte Größe einführen. Wir habe bisher sowieso noch kaum Erfahrungen mit Doctrine-ORM.

@j3nsch j3nsch added this to the Framework 4.8 milestone Oct 28, 2021
@j3nsch j3nsch changed the title Sammlungscode auf Doctrine-DBAL umstellen Sammlungscode auf Doctrine umstellen Nov 24, 2021
@j3nsch
Copy link
Member Author

j3nsch commented Nov 24, 2021

Ich denke, das Ziel ist im Augenblick nur eine Lösung mit Doctrine zu finden, bei der sich die Datenbank nicht verändert. Vermutlich ist das nur mit DBAL möglich bzw. am effizientesten. Das sollte eigentlich auch kein Problem sein. Wenn ein Verknüpftes Dokument gespeichert wird, müssen die Verknüpfungen gespeichert werden, aber nicht die Collection-Objekte. Ich vermute, dass man im Augenblick eine Sammlung verändern und als "Nebeneffekt" beim Speichern eines Dokuments mit speichern kann. Das halte ich aber für ein schlechtes Verhalten. Sammlungen sind Dokumenten nicht untergeordnet, sie sollten nur unabhängig gespeichert werden.

Die Collection-Klasse bildet Baumstrukturen in der Tabelle ab, implementiert in NestedSet. Die CollectionRole-Klasse ist im Prinzip die Konfiguration eines Baums und hat eine Verknüpfung zur Wurzel.

Ich denke im Augenblick ist es nicht sinnvoll auf eine externe Implementation von NestedSet umzusteigen. Ich gehe davon aus, dass der Aufwand unsere eigene Implementation mit DBAL zum Laufen zu bringen überschaubar ist, und das die Verwendung einer anderen externen Klasse, Änderungen an der Datenbank und damit die Migration der existierenden Sammlung notwendig machen würde. Für diesen Aufwand müsste es gute Gründe geben und ich würde das Thema lieber nach hinten schieben, wenn Luft ist für die weitere Optimierung des Frameworks.

@j3nsch
Copy link
Member Author

j3nsch commented Nov 24, 2021

Es gibt die Tabelle collections_enrichmens und die entsprechende Gateway-Klasse, aber im Code scheint das sonst nicht weiter aufzutauchen. Wenn doch, bitte hier im Issue anmerken. Die CollectionEnrichments werden nicht genutzt. Ich denke es gibt Use Cases, aber damit werden wir uns nicht im Rahmen dieses Issues befassen. Das ist etwas für später.

@j3nsch
Copy link
Member Author

j3nsch commented Dec 2, 2021

Hier ist ein Link zur NestedSet Dokumentation in den DoctrineExtensions.

https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/doc/tree.md

Vielleicht lohnt es sich, dass einfach mal auszuprobieren. Also Collection mit ORM zu mappen und dann versuchen mit der Erweiterung die NestedSet Properties zu mappen. Man könnte schauen, ob man damit die Tests ganz ohne Collections und NestedSet zum Laufen bringen kann. Das würde unseren Code reduzieren.

Die Verknüpfung mit Dokumenten ist separat und kann am Anfang ignoriert werden. Erst einmal geht es nur um die Baumstruktur in der Tabelle.

Wenn das nicht klappt oder die Performanz nicht passt, können wir immer noch eine reine DBAL Umsetzung machen. Man könnte auch überlegen, ob es Sinn macht die Tabelle für die Sammlungen von der Tabelle für die Baumstruktur zu trennen und dann separat zu verwalten. Die Entität mit ORM und die Struktur mit DBAL. Wir versuchen nur im Augenblick das Datenbankschema möglichst unverändert zu lassen und ich weiß noch nicht, ob wir ORM und DBAL mixen können, wenn alles in einer Tabelle steht.

TODO Hätte es andere Vorteile, wenn man die Baumstruktur in eine eigene Tabelle auslagert? Nicht die wichtigste Frage.

extracts added a commit that referenced this issue Dec 2, 2021
…Doctrine DBAL

This converts all functions that are required to make the tests in CollectionTest pass again
@extracts extracts linked a pull request Dec 2, 2021 that will close this issue
@extracts
Copy link
Contributor

extracts commented Dec 2, 2021

Mit c5587c5 laufen alle Tests in CollectionTest wieder. Dies liegt allerdings auch daran, dass die in CollectionTest getestete Collection Klasse noch viele Methoden aus Db\NestedSet (anstelle von Db2\NestedSet) nutzt.

Fragen:
Was ist der beste Weg um in der Collection Klasse bestehenden Code wie diesen:

$table = TableGateway::getInstance(self::$tableGatewayClass);

oder diesen:

$table = $this->primaryTableRow->getTable();

so abzuändern, dass die neue Db2\Collections Klasse zurückgegeben wird?

Wenn Collection überall die neue Db2\Collections Klasse verwendet, könnten deutlich mehr Tests in CollectionTest genutzt werden, um die korrekte Funktion von nach DBAL konvertierten Funktionen in NestedSet zu testen.

@j3nsch
Copy link
Member Author

j3nsch commented Dec 9, 2021

Die TableGateway-Klasse fallen mit der neuen Implementation in der Regel weg. Die Funktionen, in denen sie genutzt werden müssen inhaltlich mit Doctrine umgesetzt werden. Die Gateway-Klassen sind ein Artefakt der Implementation mit Zend_Db, sie müssen nicht "ersetzt" werden.

@j3nsch
Copy link
Member Author

j3nsch commented Dec 9, 2021

Da die Verwendung von Doctrine Extensions durch unsere aktuelle Beschränkung auf PHP 7.1 verursachen würden, können wir sie erst mit OPUS 4 v5.1 einsetzen, wenn wir auf eine PHP 7.3+ umsteigen werden. Wir werden also vorerst darauf verzichten müssen. Für einen späteren Umstieg wurde Issue #220 angelegt.

@j3nsch j3nsch removed this from the Framework 4.9 Beta milestone Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants