[Feature] Custom Resource States

[julian-farbcode]

Hey @RaphaelStoerk,
hier habe ich mal eine Variante für Custom states implementiert, die Devs eigene Resource States registrieren lässt.
Ich konnte nur über ein Interface + Trait eine gewisse type safety herstellen, um sicherzugehen dass ein string-backed Enum registriert wird. Macht das ganze aber auch etwas "verbose":

<?php

namespace App\Enums;

use Farbcode\StatefulResources\Concerns\AsResourceState;
use Farbcode\StatefulResources\Contracts\ResourceState;

enum CustomResourceState: string implements ResourceState
{
    use AsResourceState;

    case Foo = 'foo';
    case Bar = 'bar';
    case Baz = 'c';
}

Wenn du eine andere Möglichkeiten siehst, gerne :)

Und die Registrierung habe ich erstmal manuell über die config gemacht. Eine Überlegung von mir ist aber auch, dass das Package automatisch einen Ordner (z.B. App\Enums\ResourceStates) scannt und keine manuelle Registrierung nötig ist.

Was ich auch noch elegant fände, ist ein make-command für custom resources zum scaffolden von der Klasse, dann auch automatisch im richtigen Ordner. Wollte aber bevor ich das implementiere erstmal generelles Feedback von dir 😄

Doku kommt anschließend natürlich auch noch dazu 😉

[RaphaelStoerk]

Ich hab da jetzt länger drüber nachgedacht, hier mal meine Gedanken:

• Erstes Einschätzung war, dass evtl. einfache strings in der config ausreichend sein könnten 'custom_states' => ['custom_1', 'custom_2']
• Problem ist, dass wenn wir den states später Fähigkeiten / Funktionen geben bricht das gleich das Update
• deswegen sind enums denke ich eine gute Lösung
• Ich glaube nicht, dass das Interface notwendig/möglich ist, zumal die static functions aus enums nicht überschrieben werden dürfen, evtl. sollte ein interface ResourceState extends \BackedEnum ausreichen
• Die StateRegistry finde ich sehr cool, frage mich nur ob ein static für die $stateClasses sinnvoll ist, da es das singleton brechen könnte, da etwa in Jobs oder auch Laravel Octane ein Prozess läuft, in dem statische Werte Request übergreifend genutzt werden.
• Als Beispiel könnte noch eine Custom Zwischenklasse helfen, die zur Code Completion hilfreich sein kann

// App\Http\Resources\CustomStatefulJsonResource.php
/**
 * @method static \Farbcode\StatefulResources\Builder custom()
 *
 * @method MissingValue|mixed whenStateCustom(mixed $value, mixed $default = null)
 * @method MissingValue|mixed unlessStateCustom(mixed $value, mixed $default = null)
 * @method MergeValue|mixed mergeWhenStateCustom(mixed $value, mixed $default = null)
 * @method MergeValue|mixed mergeUnlessStateCustom(mixed $value, mixed $default = null)
 */
abstract class CustomStatefulJsonResource extends StatefulJsonResource
{
}

• Und das auslesen wär sicher mega cool, nur wahrscheinlich etwas overkill, weil ich denke, dass wir je Projekt wahrscheinlich genau ein Enum File haben, theoretisch könnte sogar die config auf ein Enum reduziert werden 🤔

Wir können sonst aber gern auch nochmal gemeinsam drüber sprechen, die Richtung ist aber auf jeden Fall die richtige denke ich

[julian-farbcode]

@RaphaelStoerk

Ich hab da jetzt länger drüber nachgedacht, hier mal meine Gedanken:

• Erstes Einschätzung war, dass evtl. einfache strings in der config ausreichend sein könnten 'custom_states' => ['custom_1', 'custom_2']

Meintest du da strings, die den Enum-Namen entsprechen, oder allgemein strings als States?
Reine strings als State-Values wurden von dir ja in https://github.com/farbcodegmbh/freizeitpass/pull/3/commits/bd72bf1dd651a0a764df8ce0d7373bf189d6abb1 entfernt, deshalb hab ich das auch Enum-basiert gelassen 🤔

• Problem ist, dass wenn wir den states später Fähigkeiten / Funktionen geben bricht das gleich das Update
• deswegen sind enums denke ich eine gute Lösung

Verstehe ich nicht ganz, wo sollten States später Funktionen haben und welche Funktionen sollten da brechen können?

• Ich glaube nicht, dass das Interface notwendig/möglich ist, zumal die static functions aus enums nicht überschrieben werden dürfen, evtl. sollte ein interface ResourceState extends \BackedEnum ausreichen

Super, danke für's Feedback! Hab das jetzt deutlich vereinfacht in 6d3a16b.

• Die StateRegistry finde ich sehr cool, frage mich nur ob ein static für die $stateClasses sinnvoll ist, da es das singleton brechen könnte, da etwa in Jobs oder auch Laravel Octane ein Prozess läuft, in dem statische Werte Request übergreifend genutzt werden.

Hast du vollkommen Recht, habe ich in ac92c76 gefixt 👍🏻

• Als Beispiel könnte noch eine Custom Zwischenklasse helfen, die zur Code Completion hilfreich sein kann

// App\Http\Resources\CustomStatefulJsonResource.php
/**
 * @method static \Farbcode\StatefulResources\Builder custom()
 *
 * @method MissingValue|mixed whenStateCustom(mixed $value, mixed $default = null)
 * @method MissingValue|mixed unlessStateCustom(mixed $value, mixed $default = null)
 * @method MergeValue|mixed mergeWhenStateCustom(mixed $value, mixed $default = null)
 * @method MergeValue|mixed mergeUnlessStateCustom(mixed $value, mixed $default = null)
 */
abstract class CustomStatefulJsonResource extends StatefulJsonResource
{
}

Klar, kann man dann projektspezifisch machen, bringt im Endeffekt aber ja nur was für die autocompletion von den magic names, whenState(CustomState::Custom, ...) etc. wird unabhängig davon ja trotzdem completed 🙂

• Und das auslesen wär sicher mega cool, nur wahrscheinlich etwas overkill, weil ich denke, dass wir je Projekt wahrscheinlich genau ein Enum File haben, theoretisch könnte sogar die config auf ein Enum reduziert werden 🤔

Hast du Recht, auto-discovery ist für den use-case definitiv overkill. Aber ich würde hier nur ungerne von der Laravel-Konvention abweichen und die Config über etwas anderes als mit einem Config-File machen.

[RaphaelStoerk]

@RaphaelStoerk

Ich hab da jetzt länger drüber nachgedacht, hier mal meine Gedanken:

• Erstes Einschätzung war, dass evtl. einfache strings in der config ausreichend sein könnten 'custom_states' => ['custom_1', 'custom_2']

Meintest du da strings, die den Enum-Namen entsprechen, oder allgemein strings als States? Reine strings als State-Values wurden von dir ja in farbcodegmbh/freizeitpass@bd72bf1 entfernt, deshalb hab ich das auch Enum-basiert gelassen 🤔

• Problem ist, dass wenn wir den states später Fähigkeiten / Funktionen geben bricht das gleich das Update
• deswegen sind enums denke ich eine gute Lösung

Verstehe ich nicht ganz, wo sollten States später Funktionen haben und welche Funktionen sollten da brechen können?

Vielleicht nochmal klarer:
Wenn wir die States fest vorgeben sind Enums eindeutig die beste Wahl, da nicht erweiterbar und leicht anzuwenden.
Wenn wir aber sagen die States sind beliebig definierbar ist das schon nicht mehr so eindeutig, zumal ich ja als Entwickler dann plötzlich mehrere Enum-Types habe und wissen muss aus welchem der gerade relevante State kommt.

Das finde ich erst mal schwierig, deswegen könnten Strings hier mehr Flexibilität rein bringen. Die hätte ich dann als Konstanten definiert, etwa in der StatefulJsonResource und kann dann von dieser Klasse die eigene CustomStatefulJsonResource kreieren, mit eigenen Konstanten (static::FULL und static::CUSTOM dann kein Unterschied) und mit den unten genannten PHP Docs.

Jetzt könnte es aber sein, dass wir den Enums später noch Funktionen hinzufügen, die mit den Resourcen interagieren um die Funktionalität deutlich zu erweitern. Etwa um defaults zu schaffen, welche Parameter oder Relations niemals angezeigt werden, o.ä. Das ist mit Strings eben nicht mehr möglich und würde diesen Weg versperren.

Eine andere Alternative wären Klassen und Instanzen dieser Klasse anstatt Enums. Das wäre für Entwickler vermutlich nochmal leichter anzuwenden bietet aber mehr overhead, auch was korrekte Serialisierung angeht. Ich würde das nicht komplett ausschließen, denke aber dass der Enum Weg der sinnvollste ist.

• Als Beispiel könnte noch eine Custom Zwischenklasse helfen, die zur Code Completion hilfreich sein kann

// App\Http\Resources\CustomStatefulJsonResource.php
/**
 * @method static \Farbcode\StatefulResources\Builder custom()
 *
 * @method MissingValue|mixed whenStateCustom(mixed $value, mixed $default = null)
 * @method MissingValue|mixed unlessStateCustom(mixed $value, mixed $default = null)
 * @method MergeValue|mixed mergeWhenStateCustom(mixed $value, mixed $default = null)
 * @method MergeValue|mixed mergeUnlessStateCustom(mixed $value, mixed $default = null)
 */
abstract class CustomStatefulJsonResource extends StatefulJsonResource
{
}

Klar, kann man dann projektspezifisch machen, bringt im Endeffekt aber ja nur was für die autocompletion von den magic names, whenState(CustomState::Custom, ...) etc. wird unabhängig davon ja trotzdem completed 🙂

Plus eventuell tatsächlich einfacheres entwickeln, je nach state definition (siehe oben), aber eher nebensächlich 👍

• Und das auslesen wär sicher mega cool, nur wahrscheinlich etwas overkill, weil ich denke, dass wir je Projekt wahrscheinlich genau ein Enum File haben, theoretisch könnte sogar die config auf ein Enum reduziert werden 🤔

Hast du Recht, auto-discovery ist für den use-case definitiv overkill. Aber ich würde hier nur ungerne von der Laravel-Konvention abweichen und die Config über etwas anderes als mit einem Config-File machen.

Das ist klar, wir sollten hier auf jeden Fall im Standard bleiben, aber das lässt sich denke ich auch gut in diesem Fall machen

[julian-farbcode]

@RaphaelStoerk
So, ich habe wie besprochen noch einiges angepasst.
Das ResourceState Interface hab ich doch dringelassen, hauptsächlich zur Ästhetik.
Auch die StateRegistry hab ich doch weiterhin als hilfreich gesehen, deshalb ist die auch noch drin.

Wie besprochen habe ich die State-Logik zu strings refactored, aber weiterhin die Möglichkeit zugelassen, Enum/ResourceState-Instanzen zu verwenden.

Die config hab ich auch noch erweitert, dass ein expliziter default state für Ressourcen angepasst werden kann.
Wir haben innerhalb des Packages jetzt auch keine Abhängigkeiten mehr vom Variant Enum, States können also vollkommen frei angepasst werden.

[RaphaelStoerk]

Find ich mega, ist genauso wie besprochen und ich mi das vorgestellt habe. Also für eine Version 0.1.0 ist das in meinen Augen gut.

Das ResourceState Interface hab ich doch dringelassen, hauptsächlich zur Ästhetik.
Auch die StateRegistry hab ich doch weiterhin als hilfreich gesehen, deshalb ist die auch
noch drin.

Find ich beides korrekt, mir gefällt die StateRegistry auch, vor allem könnte man als Entwickler das Singleton evtl. mal überschreiben, wenn es eine eigene Logik braucht.
Und ich denke die ResourceState kann nicht schaden, dann könnte ein späterer stärkerer Fokus auf Enums leichter zu migrieren sein.

Die config hab ich auch noch erweitert, dass ein expliziter default state für Ressourcen
angepasst werden kann.
Wir haben innerhalb des Packages jetzt auch keine Abhängigkeiten mehr vom Variant
Enum, States können also vollkommen frei angepasst werden.

Auch die Config sieht jetzt extrem sauber aus, und verständlich, ich denke das ist dann auch gut nutzbar / erweiterbar.

Also passt denke ich alles sehr gut 👍