[WIP] Model registry

[tristanlins]

Continue of #6131 but against develop branch!

In Bezug auf #6070 wie versprochen ein PR für eine Model Registry.

Checklist

• - (1) Registry zur Database Connection hinzufügen
• - (2) Registry in den Models verwenden
• - (3) Bei Model::save nur die Änderungen speichern
• - (4) Änderungen bei Model::setRow richtig protokollieren
• - (5) Model save darf nur dann ein Insert machen, wenn das Model nicht in der Registry verwaltet wird (siehe A)
• - (6) Model save darf nur dann ein Update machen, wenn das Model in der Registry verwaltet wird (siehe A)
• - (7) Nutzung der Registry in findBy optimieren, wenn dort ausschließlich nach PK gesucht wird
• - (8) Magic Properties entfernen?! (siehe B)
• - (9) Collections iterierbar machen (das war eigentlich die Aufgabe von @discordier ^^)
• - (10) Registry im Debug Tool tracen
• - (11) Feldänderungen nur dann merken, wenn der Wert auch wirklich geändert wurde
• - (12) Einfache Möglichkeit Models freizugeben (z.B. Model::free()) für händische Speicheroptimierung
• - (13) Add Model::refresh() to reload the data from database.
• - (14) Track modifications on the primary key, by keep a copy of the original record.
• - (15) Out of sync check ([WIP] Model registry (obsolete) #6131 (comment))
• - (16) Do not store the database result within the Model (Drop the Database_Result cache #6182 (comment))
• - (17) Do not store the database result within the Collection (Drop the Database_Result cache #6182 (comment))
• - (18) Check if relations are correctly passed through the registry ([WIP] Model registry (obsolete) #6131 (comment))

Es gibt noch ein paar Punkte, an denen ich zu knabbern habe:

---

(A) Das erste ist Model::save(true) was ja bewirkt, dass ein Model als neuer Record gespeichert wird. Das funktioniert aber in Verbindung mit der Registry ganz schlecht. Das Problem ist halt, dass ich quasi das Objekt re-registrieren müsste, für die neu erstellte ID. Das geht aber ja nicht.

$model = \Model::findByPK(1);
// spl_object_hash($model) == 1
$model->save(true);
// spl_object_hash($model) == 1 müsste aber 2 sein, weil neue ID

Normalerweise sollte ein "Klon" so durchgeführt werden:

$modelA = \Model::findByPK(1);
// spl_object_hash($modelA) == 1
$modelB = clone $modelA;
$modelB->save();
// spl_object_hash($modelB) == 2

Ganz davon abgesehen, dass durch die Registry man das hier natürlich gar nicht mehr machen kann:

$model = \Model::findByPK(1);
$model->key = 'new value';
$model->save(true);

Obwohl man save(true) gemacht hat, wird hier natürlich der Originaldatensatz mit der ID 1 gleich mit verändert.

Lösung: das $blnForceInsert wird gedropt. Die __clone löscht bereits den PK von dem neuen Model, d.h. die richtige Variante mit clone ist bereits implementiert.

---

(B) Die Models werden teilweise mit nicht existierenden Werten befüllt. Ein Beispiel ist PageModel::findPublishedSubpagesWithoutGuestsByPid im SELECT werden dort virtuelle Felder erzeugt, die im Model auch verfügbar sind, da dieses ja lediglich auf das Result zugreift. Eigentlich sollte das gar nicht unterstützt werden, um es zu unterstützen mache ich jetzt immer einen Merge mit dem Record, wenn ich ein Model aus der Registry hole. Siehe 35559ab

Lösung: derartige virtuellen Felder dürfen nicht über das Query kommen, sondern müssen über getter des Models kommen.

---

Es gibt sicher noch ein paar Dinge, die ich gerade auf die schnelle nicht berücksichtigt habe.
Außerdem bin ich ja noch nicht fertig wie man an der Checkliste sehen kann ;-)
Trotzdem ist Feedback erwünscht, ich würde mich vor allem über Feedback von @leofeyer freuen, ob das so akzeptabel ist. Ich setze gerne Änderungswünsche um.

[leofeyer]

We definitely have to schedule for a Skype call before I can merge this :)

[tristanlins]

Okay, I removed the multi db support for models to avoid critical BC breaks.
All unit tests pass the new version :-)

@leofeyer please have a look at this commit, it is a proposal for the upgrade notice.
https://github.com/tristanlins/contao-core/commit/f4d6b3769441a69e05b538f99ba54a03d7ffc842

[leofeyer]

Do we really need to trace the registry in the debug bar? It causes nasty line-breaks on screens <= 1024 pixels.

The debug bar has been tracing performance relevant parameters only so far:

• execution time: self-explanatory
• memory usage: self-explanatory
• database queries: relevant if the DB server is on a separate machine
• rows returned: relevant if the DB server is on a separate machine

How is the registry parameter relevant in this context?

[Toflar]

I really like it, Leo :) Do some responsive work and only show the icons and values without label if the window size is not big enough ;) Should be fairly easy to do, shouldn't it?

[leofeyer]

Yes, but I'd also like to see a point in things :) No hard feelings.

[leofeyer]

Also, I don't fully understand why there is a Model\Collection::createFromResult() method. The same code could be in the constructor of the Model\Collection object and take the result object as argument, couldn't it?

[tristanlins]

But now it is possible to use the Collection without a Result object.
But it is to you if you will "open" the collection for the developers.
I'm even fine without Collection::createFromResult(), I will just use another collection implementation :p

[leofeyer]

While reviewing the code I found the answer in the Model class:

public static function findMultipleByIds($arrIds, array $arrOptions=array())
{
    if (empty($arrIds) || !is_array($arrIds))
    {
        return null;
    }

    $arrRegisteredModels = array();
    $arrMissingModelIds  = array();

    …

    return new \Model\Collection(array_filter(array_values($arrRegisteredModels)), static::$strTable);
}

An array with models is passed to the constructor, so Collection::createFromDbResult() is not a choice, but a definite requirement :)

[leofeyer]

Although the note says "do not check $this->arrModified", you are checking it in the following lines. Is this correct?

[tristanlins]

The note is correct, but would still be a little concretized.
The following check is for tracing primary key changes. If the PK has changed, the original value (that still exists in the database) in $this->arrModified[static::$strPk] and not $this->{static::$strPk}.

[leofeyer]

Merged in dcd2cd1. Thank you so much for your time and good work. This was one of the biggest pull requests ever :)

[tristanlins]

You're welcome :-)

[blairwinans]

Thank you as well, @tristanlins... The whole Contao community owes you (and everyone involved) a huge debt of gratitude. Great to see open source in action.