[Mittel] Harden HTTP/serializer layer: response consumption, injectable client, timeouts (#34)#41
Open
maltehuebner wants to merge 1 commit into
Open
[Mittel] Harden HTTP/serializer layer: response consumption, injectable client, timeouts (#34)#41maltehuebner wants to merge 1 commit into
maltehuebner wants to merge 1 commit into
Conversation
Robustness fixes for the write path and the serializer: - Consume responses. Symfony's HttpClient is lazy; the previous write methods discarded the response, so the request only completed in the response destructor and any HTTP/transport error was thrown from __destruct() with no status check. AbstractApi::assertSuccessful() now forces completion, translating transport failures to LuftApiTransportException and HTTP >= 400 to LuftApiException. All read/write API methods call it and document @throws. - Inject HttpClientInterface. ApiClient no longer calls HttpClient::create() in its constructor; it receives a HttpClientInterface, so it is testable with MockHttpClient and can be swapped for RetryableHttpClient/profiler-aware clients. The extension builds a scoped client (base_uri/verify/timeout) via the HttpClient factory and injects it. - Configurable timeouts. New api.timeout (default 10s) and api.max_duration (default 30s) prevent cron-style provider imports from hanging indefinitely. - Fix serializer context precedence. serialize()/deserialize() merged DEFAULT_CONTEXT *after* the caller context, silently overriding an explicit SKIP_NULL_VALUES => false. Merge defaults first so the caller wins. - Minor contracts: type the $uri parameter as string on ApiClientInterface/ApiClient; centralize the serializer format as AbstractApi::SERIALIZER_FORMAT (protected typed const) and use it in ValueApi instead of the magic 'json' string. BREAKING CHANGE: ApiClient::__construct now takes a HttpClientInterface instead of (hostname, port, verify); wiring is handled by the bundle extension. The undocumented public StationApiInterface::SERIALIZER_FORMAT constant was removed in favour of AbstractApi::SERIALIZER_FORMAT. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the write path fail loudly and predictably, and makes the client testable.
Changes
__destruct()without a status check. NewAbstractApi::assertSuccessful()forces completion, mapping transport failures toLuftApiTransportExceptionand HTTP ≥ 400 toLuftApiException(newsrc/Exception/). All API methods call it and carry@throws.ApiClientnow receives aHttpClientInterfaceinstead of callingHttpClient::create()in its constructor — testable withMockHttpClient, swappable forRetryableHttpClient/profiler clients. The extension builds a private scoped client (base_uri/verify/timeout/max_duration) via theHttpClientfactory and injects it intoApiClient.api.timeout(default 10s) andapi.max_duration(default 30s) config nodes stop cron-style imports from hanging.serialize()/deserialize()mergedDEFAULT_CONTEXTafter the caller context, silently overriding an explicitSKIP_NULL_VALUES => false. Nowarray_merge(self::DEFAULT_CONTEXT, $context)so the caller wins.$urityped asstring; serializer format centralized asAbstractApi::SERIALIZER_FORMAT(protected typed const), replacing the magic'json'inValueApi.Breaking changes
ApiClient::__constructnow takes aHttpClientInterface(washostname, port, verify). The bundle extension performs the wiring, so bundle users are unaffected; anyone constructingApiClientmanually must pass a client.StationApiInterface::SERIALIZER_FORMATin favour ofAbstractApi::SERIALIZER_FORMAT.Overlap / notes
symfony/config/symfony/http-kernel/symfony/property-infoin the vendor tree (declared in [Hoch] Undeklarierte Runtime-Dependencies: symfony/config und symfony/http-kernel fehlen, symfony/property-info nur transitiv #30). This branch does not touchcomposer.json; verified locally against a tree with those installed.main; reconcile on merge. Bearer-auth is intentionally left to [Hoch] TLS-Verifikation standardmäßig aus, fehlende API-Auth, unvollständige Config-Defaults #28.HttpKernelExtension base (the swap is [Mittel] Deprecations auf allen unterstützten Plattformen: implizit nullable Parameter (PHP 8.4) und @internal-Extension-Basisklasse (Symfony 8.1) #32) and the currentservices.phpexcludes (the exclude cleanup is [Niedrig] Altlasten und Paket-Hygiene: tote services.xml, unerreichbarer Normalizer, stale composer.lock, fehlende license/description, Mini-README #35).Verification
Against a full vendor tree (PHP 8.5):
ApiClientarg0 is aReferencetocaldera_luftapi.http_client, whose options arebase_uri=https://api.luft.jetzt:443/,verify_peer=true,timeout=10,max_duration=30.MockHttpClient: HTTP 500 →LuftApiException, HTTP 200 → no throw.SKIP_NULL_VALUES => falsenow keeps null props; default call still skips them.Closes #34
🤖 Generated with Claude Code