Optimize some general things

[corbadoman]

Gedanken

Das SDK besteht zu 90% aus generiertem Code, da hat man wahrscheinlich nicht so viel Handhabe. Solltet schauen ob
euer code-generator auch PSR-18/PSR-17/PSR-7 unterstützt, das würde einiges für eure Nutzer "besser" machen. Mehr dazu
im dependency inversion Abschnitt.

PHP requirement

Laut composer.json unterstützt ihr alle PHP Versionen die eure dependencies unterstützen. Ihr solltet dringend eure
unterstützten und vor allem getesteten PHP versionen eintragen.
Es gibt nichts schlimmeres als in einem Projekt composer update zu machen und dann explodiert beim releasen die
Applikation weil irgendeine library wieder meint, sie müsse entweder keine PHP version oder eben eine ungetestete range
angeben.

Eure integration tests in den github workflows scheinen auch nur mit PHP 8.0, 8.1, 8.2 und 8.3 ausgeführt zu werden.
Würde das in sync halten. Kann euch hier sehr den laminas CI workflow ans Herz legen, nicht weil ich da mit dran
entwickelt habe, sondern weil das Ding anhand der composer.json rausfindet, welche PHP versionen supported werden und
die Matrix dann automatisch generiert wird um dann alle PHP versionen entsprechend zu testen.

Good practice PHP dependency

{
    "require": {
        "php": "~7.2.0 || ~7.3.0 || ~7.4.0 || ~8.0.0 || ~8.1.0 || ~8.2.0 || ~8.3.0"
    }
}

Bad practice PHP dependency

{
    "require": {
        "php": ">=7.2"
    }
}

Worst practice PHP dependency

{
    "require": {}
}

Also einfach gar nix angeben. ;-)

Generated code

Also, der Großteil vom Code ist natürlich generated - da kann man nicht viel machen, daher ist es so wie
es ist. Ich halte nicht viel von generated code, war zuletzt ja auch beim SSO client bei CHECK24 so.
Eins der großen flaws dieser generated Klassen ist einfach immer, dass sie nicht "gut genug" sind.
Warum denke ich das?

Beispiel:

Du hast ne API, die hat nen haufen an requirements, u.A.:

• Path variables (/connectTokens/{connectTokenID})
• Query Parameters (hab gerade kein Beispiel parat)

Man weiß einfach bei diesem generated code nie was jetzt required ist.

Good practice:

final readonly class ConnectTokensRequest
{
    /** @param non-empty-string $connectTokenId */
    public function __construct(
        private string $connectTokenId,
    ) {}
}

Bad practice:

final readonly class ConnectTokensRequest
{
    private string $connectTokenId;
    
    public function setConnectTokenId(string $connectTokenId): void
    {
        $this->connectTokenId = $connectTokenId;
    }    
    
    public function getConnectTokenId(): void
    {
        return $this->connectTokenId;
    }
}

Man sieht beim instanzieren des requests bei "good practice" direkt was phase ist.
Wenn man beim bad practice das Objekt instanziert, sieht man erstmal nicht was man ggf. sonst noch so setzen muss,
damit der request überhaupt durchgeht. Also muss man sich erstmal die Klasse angucken, die ist in den meisten Fällen
aber absurd groß und vielleicht steht irgendwo in einer property sowas wie $required = ['connectTokenId'].
Usability als dev ist halt total räudig, weil man jetzt schon so tief im code drin steckt, das sollte eigentlich
nie nötig sein.

Helpers

Helpers.php scheint nicht (mehr) genutzt zu werden. Kann also weg - davon ab, dass "helpers" auch eher bad practice sind.
Assert solltet ihr mal gegen eine gescheite library austauschen, z.B. webmozart/assert oder beberlei/assert.

Exceptions

Würde hier etwas mehr von RuntimeException, InvalidArgumentException, etc. gebrauch machen statt überall nur
Exception zu extenden.

Dependency inversion

Heutzutage sollte vom dependency inversion Prinzip gebrauch gemacht werden. Was heißt das genau?

{
    "require": {
        "psr/http-client": "^1.0",
        "psr/http-client-implementation": "^1.0",
        "psr/http-factory": "^1.0",
        "psr/http-factory-implementation": "^1.0",
        "psr/http-message": "^1.0",
        "psr/http-message-implementation": "^1.0"
    },
    "require-dev": {
        "guzzlehttp/guzzle": "^7.9"
    }
}

Wie man erkennen kann, geben wir keinem Projekt vor, welchen HTTP client, etc. sie nutzen sollen.
Das hat den Vorteil, dass kein Projekt gezwungen ist, mehr als einen HTTP client zu nutzen.
Ja, viele benutzen Guzzle aber manche nutzen eben auch "http plug", den symfony HTTP client oder was auch immer.
Entwickler wollen volle Kontrolle über ihre dependencies haben und sofern möglich, unnötige vendor packages vermeiden.
Man kann, auch wenn ich da kein Freund von bin, natürlich dann sowas verwenden wie php-http/discovery.

Das bedeutet aber natürlich auch, dass der code anders aussehen muss.
Beispiel anhand von der ConnectTokensApi Klasse wäre das dann quasi wie folgt:

use Http\Discovery\Psr17FactoryDiscovery;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\StreamFactoryInterface;

class ConnectTokensApi
{
    public function __construct(
        ClientInterface $client = null,
        RequestFactoryInterface $requestFactory = null,
        StreamFactoryInterface $streamFactory = null, 
        Configuration $config = null,
        HeaderSelector $selector = null,
        $hostIndex = 0
    ) {
        $this->client = $client ?: Psr18ClientDiscovery::find();
        $this->requestFactory = $requestFactory ?: Psr17FactoryDiscovery::findRequestFactory();
        $this->streamFactory = $requestFactory ?: Psr17FactoryDiscovery::findStreamFactory();
        $this->config = $config ?: new Configuration();
        $this->headerSelector = $selector ?: new HeaderSelector();
        $this->hostIndex = $hostIndex;
    }
    
    
    // [...]
    public function connectTokenListRequest($sort = null, $filter = null, $page = 1, $page_size = 10, string $contentType = self::contentTypes['connectTokenList'][0])
    {
        // [...]
        $request = $this->requestFactory
            ->createRequest(RequestMethodInterface::GET, $operationHost . $resourcePath . ($query ? "?{$query}" : ''))
            ->withBody($this->streamFactory->createStream($httpBody));
        foreach ($headers as $headerName => $headerValue) {
            $request = $request->withHeader($headerName, $headerValue);
        }
       
        return $request;
    }
}

Damit entkoppelt man seinen library code von konkreten dependencies. Klar, für integrationstests im SDK braucht ihr
was konkretes, da kann man sich dann bspw. guzzle als dev-dependency ziehen.

Strictness

Da hier ja auch PHPStan verwendet wird, nehme ich an, ist euch klar, was sonst noch so schönes in der Verwendung von sowas schlummert.
Klassisches Beispiel ist z.B. die non-empty-string annotation, etc. (was euch dann auch den ganzen assert quatsch ersparen würde).

Heutzutage reicht es nicht mehr, nur den Datentyp anzugeben. Es hilft ungemein auch da noch spezifischer zu werden, wie bspw.:
non-empty-string, numeric-string, positive-int, non-negative-int, int<0,100>, usw.

php-cs-fixer

Stellt den mal so ein, dass der redundante doc comments entfernt. Dann wird einiges etwas leserlicher.

autoload-dev

Put integration namespace into composer autoload-dev