Feature/embedded entities

[pmattmann]

#1579 Embedded Entities Beispiel aufbauen

Idee:
Es werden pro Entität jeweils folgende Serialization-Groups angelegt:

• read
  Ist bei allen Lese-Operationen aktiv.
  Properties markiert mit read werden immer ausgegeben
• write
  Ist bei allen Schreib-Operationen aktiv.
  Properties markiert mit write sind immer schreibbar (post & patch)
• create
  ist bei Post-Operation aktiv
  Properties markeirt mit create sind nur beim erstellen schreibbar (post)
• update
  Ist bei Patch-Operation aktiv
  Properties markiert mit update sind änderbar (patch)

Zusätzlich werden Serialization-Groups pro Relation Quelle:Ziel erstellt.
Diese erlauben selektives embedden von solchen Relationen.

Damit die Relation selektiv embedded werden kann, wird jeweils das Property der Relation mit #[ApiProperty(readableLink:false)] markiert.
Zusätzlich wird eine Getter-Methode getEmbeddedXYZ() erstellt. Diese wird mit #[ApiProperty(readableLink:true)] und #[Groups(Quelle:Ziel)] markiert.

Siehe Beispiele:

• read: Day.dayOffset
• write: Camp.title
• create: Camp.name
• linked Entity: Period.Camp
• embedded Entity: Period.Camp
• linked Collection: Period.Days
• embedded Collection: Period.Days

Durch angeben von Relationen Quelle:Ziel im normalizationContext kann bestimmt werden, ob eine Relation embedded werden soll, oder nicht.

[carlobeltrame]

Ich finde den Vorschlag grundsätzlich gut. Statt Properties:read / Properties:write könnte ich mir auch Any:read / Any:write vorstellen, da man es dann immer als eine Einschränkung nach Root Entity Klasse lesen kann. Aber zugegebenermassen wird das wohl fast immer nur für primitive Properties verwendet, daher wäre auch dein Vorschlag okay für mich.

Eine Limitierung dieses Systems mit den Groups tritt auf, wenn man rekursive Datenstrukturen hat. Z.B. wenn wir die children eines ContentNodes mit writableLink: true markieren würden, also nested POST / nested PATCH erlauben würden, dann könnte man nicht separate Regeln für Payload-Root und nested Children festlegen.
Ein anderes Beispiel wo das vielleicht schlimmer sein könnte, wäre wenn wir z.B. MaterialItem und MaterialList gegenseitig nested schreibbar machen. Wenn ich dann ein MaterialItem patche ist die Group MaterialItem:write aktiv. Im Payload könnte ich also nested die MaterialList und darin nested ein beliebiges anderes MaterialItem mitsenden und so mitbearbeiten.
Aber ich glaube solange wir writableLink: true nur sparsam und bedacht einsetzen, sollte das in der Praxis kein grosses Problem sein.

[pmattmann]

Allgemeine Notiz:
Mit der vorgeschlagenen Groups-Logik funktionieren die UnitTests aktuell nicht.
Grund ist die Idee ECampApiTestCase::getExamplePayload.

Diese versucht anhand einer API-Resource einen Beispiel-Payload automatisch zu erzeugen. Die Schema-Namen der Api-Resourcen ändern jedoch zusammen mit den Groups. Das führt dazu, dass diese Schema-Namen je nach refactoring immer wieder ändern können / nicht stabil sind.
Evtl. sind wir gut beraten selber Biespiel-Payloads pro HTTP-Verb und Entität zu erstellen.

-> Bitte diskutieren.

[carlobeltrame]

Bei getEmbeddedDays hast du ein Example drin, hier nicht. Müsste man das ergänzen oder fällt er zurück auf das Example der Property?

[carlobeltrame]

Bei den getEmbedded* Methoden brauchts kein Example, das wird sowieso nicht in Swagger angezeigt. Stattdessen werden, wenn man den Return Type korrekt annotiert, die eingebetteten Felder perfekt automatisch angezeigt, inklusive den korrekten Normalisierungs-Gruppen 🥳

    /**
     * @return Day[]
     */
    #[ApiProperty(readableLink: true)]
    #[SerializedName('days')]
    #[Groups('Period:Days')]
    public function getEmbeddedDays(): array {
        return $this->days->getValues();
    }

    #[ApiProperty(readableLink: true)]
    #[SerializedName('camp')]
    #[Groups(['Period:Camp'])]
    public function getEmbeddedCamp(): ?Camp {
        return $this->camp;
    }

führt zu folgendem Output in Swagger:

[carlobeltrame]

Ich habe die Tests gefixt und die vorgeschlagenen Gruppen bei allen Entities eingefügt.
Danach hat API Platform automatisch mal sämtliche Relationen embedded, auch alle die wir nicht so markiert haben. Bei der Fehlersuche bin ich auf ein verstecktes Stück Dokumentation gestossen, und mir ist einiges in Bezug auf Embedding in API Platform klarer geworden.

Automatisch embedded (d.h. automatisch readableLink=true) wird eine Relation dann, wenn 1. die Relation selber durch die aktiven Gruppen lesbar ist und 2. zumindest ein Feld in der related Entity durch die aktiven Gruppen lesbar ist. Normalerweise schlägt API Platform vor (implizit in den Docs und explizit in den SymfonyCasts), Serialisierungsgruppen ala camp:update, period:read zu verwenden. Bei uns ist aber der Vorschlag, update, read etc. zu verwenden. Dadurch sind beide obigen Konditionen gegeben, und alles wird überall automatisch embedded.

Ich finde unseren Ansatz trotzdem noch spannend wegen der Eleganz mit der Camp:Periods Config die er für unseren Fall bietet. Um das so zum fliegen zu bringen, musste ich daher ein paar Anpassungen an API Platform vornehmen. Eine davon ist in api-platform/core#4377 bei API Platform angebracht (aber bei uns vorgeholt), die anderen (CircularReferenceDetectingHalItemNormalizer, PreventAutomaticEmbeddingPropertyMetadataFactory) sind spezifisch für unseren Fall, die habe ich nur bei uns implementiert.

To discuss: Halten wir an unserem Vorschlag für Serializer Groups fest und unterhalten diese Abweichungen selber, oder gehen wir zurück zum inoffiziellen Standard?

[carlobeltrame]

Wäre jetzt nie darauf gekommen, aber b0cb899 hat tatsächlich die Tests gefixt. Merci @pmattmann

[pmattmann]

Wäre jetzt nie darauf gekommen, aber b0cb899 hat tatsächlich die Tests gefixt. Merci @pmattmann

Jooo... war auch nicht einfach, den zu finden... musste ich ordentlich tief debuggen gehen...

[BacLuc]

Die Coverage geht leider um 1.1 Prozent runter für api.
Wir haben ein threshold von 1% für den decrease in einem PR.
Gibt es noch Code, den man sinnvoll testen könnte, oder soll jemand (z.b. ich) den threshold auf 1.1 Prozent decrease stellen?

[carlobeltrame]

Die eagerloadingextension ist wohl dafür verantwortlich. Die ist eigentlich nur temporär bei uns drin, bis api-platform/core#4377 gemerged und released ist. Wir könnten höchstens tests dafür noch bei uns reinmachen.

[BacLuc]

minor:
Hätte ich jetzt eher positiv formuliert.
Falls möglich möchte ich wissen, was zurückgegeben wird. Nicht dass das $result nicht diesem Array entspricht.

[BacLuc]

minor:
Hätte ich jetzt eher positiv formuliert.
Falls möglich möchte ich wissen, was zurückgegeben wird. Nicht dass das $result nicht diesem Array entspricht.