[API-BOT] Update OpenAPI spec and regenerate SDK - #98
platformsh-devrel wants to merge 1 commit into
Conversation
|
📋 PR Summary Regenerates the PHP SDK from an updated upstream OpenAPI spec. The main functional changes are new egress-allowlist modelling for containers (project-level egress proxy capability plus build/runtime allowed-domain lists on web apps and workers), a new backup Changes
|
![upsun-dispatch[bot]](https://avatars.githubusercontent.com/in/4153575?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 2 warnings · 🔵 1 minor point
🔍 Full review · 30 files reviewed
Verification
- No source or test file still references the deleted RegistryCredentialApi, SbomApi, BasicAuth, RegistryCredential* classes (repo-wide grep, including UpsunClient wiring).
- The new
egress/egressProxyproperties are nullable with matching entries in all three mapper files, so absent fields deserialize to null rather than throwing. - Egress, EgressProxy, BuildPhaseEgress and RuntimePhaseEgress getters/jsonSerialize match their constructor promotions and the spec's build/runtime shape.
- Backup::$sizeObjectStorage is
?int, so responses omitting size_object_storage still deserialize successfully.
No tests were added or changed in this diff; the repository's develop.yml workflow runs composer run lint:all (phpcs, rector, php-cs-fixer) and composer run test:unit, and the existing tests/Core/Tasks/ProjectsTaskTest.php::testGetCapabilities fixture omits the newly required supports_horizontal_scaling_services field (I reproduced the resulting InvalidArgumentException with a standalone script against src/).
Review details
- Commit: 212e866
- Model: claude-opus-5
Review 1 of 10 for this pull request · View the full run
| { | ||
| public function __construct( | ||
| private readonly bool $enabled, | ||
| private readonly bool $supportsHorizontalScalingServices, |
There was a problem hiding this comment.
🟡 Warning — Any capabilities response lacking the new field crashes; existing unit test fails.
Autoscaling::__construct gains a non-nullable, no-default bool $supportsHorizontalScalingServices. ObjectSerializer::deserializeSimplifiedModel() resolves the value via the attribute map key supports_horizontal_scaling_services; when the payload omits it the value is null, $allowsNull is false, and ApiObjectTypesMapper reports 'bool' (no null), so the final guard throws InvalidArgumentException: Required value 'supportsHorizontalScalingServices' missing for class Upsun\Model\Autoscaling. I reproduced this against the repository's own fixtures: deserializing ['autoscaling' => ['enabled' => true]] (the shape used in tests/Core/Tasks/ProjectsTaskTest.php::testGetCapabilities and BaseTestCase) aborts the whole ProjectCapabilities deserialization with that exception, so ProjectsTask::getCapabilities() fails for any API response that does not yet return the new field.
| ], | ||
|
|
||
| 'Upsun\Model\BuildPhaseEgress' => [ | ||
| 'allowed_domains' => '\Upsun\Model\AllowedDomainsInner[]', |
There was a problem hiding this comment.
🟡 Warning — New getters return arrays, not the documented model objects, breaking consumers.
BuildPhaseEgress/RuntimePhaseEgress declare allowed_domains => '\Upsun\Model\AllowedDomainsInner[]', but ObjectSerializer::deserializeSimplifiedModel() looks the item type up with the camelCase constructor parameter name ($types[$paramName], i.e. allowedDomains). The snake_case key never matches, so the fallback path stores the raw payload. Running the deserializer on ['build' => ['allowed_domains' => [['host' => 'a.com','port' => null]]]] yields Egress::getBuild()->getAllowedDomains() === [['host' => 'a.com','port' => null]] — plain arrays, contradicting the @return AllowedDomainsInner[] docblock, so caller code doing ->getHost() fatals. (The camel/snake mismatch is a pre-existing generator/serializer issue; this change adds the first multi-word array-of-model property on the new egress models.)
| private readonly ?int $index, | ||
| private readonly ?int $sizeOfVolumes, | ||
| private readonly ?int $sizeUsed, | ||
| private readonly ?int $sizeObjectStorage, |
There was a problem hiding this comment.
🔵 Minor — Silent positional-argument breakage for library consumers on a patch-looking update.
sizeObjectStorage is inserted into the middle of Backup::__construct, and the same regeneration reorders the constructor parameter lists of Task, WebApplicationsValue and WorkersValue (resources/authorizations moved ahead of type/size/disk/access). Deserialization is by name so it is unaffected, but any downstream code that constructs these readonly value objects positionally now silently binds values to the wrong parameters or fails type checks after a simple composer update of this published library.
2 participants
This PR updates the OpenAPI spec from the upstream source and applies formatting changes.
🧾 API Coverage Report
📋 Full JSON report