Mapping id to uuid breaks openapi schema compliance in EnvironmentResponse.

[fiasco]

Describe the bug See here where the EnvironmentResponse object maps the id field in the response to the uuid field on the object. This prohibits the EnvironmentResponse from being validated successfully against the Cloud Api OpenAPI spec.

I've been using thephpleague/openapi-psr7-validator to validate locally stored and altered objects initially retrieved from Cloud API objects. I want to use this library to homogenise my local data and data that comes via typhonius/acquia-php-sdk-v2 so my functions can use both without excessive conditions to handle the data based on its source. Instead, I can validate the data through the OpenApi Spec, then trust it.

To Reproduce Here is an example:

use cebe\openapi\Reader;
use cebe\openapi\ReferenceContext;
use cebe\openapi\spec\Reference;
use cebe\openapi\SpecObjectInterface;
use League\OpenAPIValidation\Schema\SchemaValidator;

// ...
    const SpecUrl = 'https://cloudapi-docs.acquia.com/acquia-spec.yaml';
    protected SpecObjectInterface $spec;

    public function validate(array|object $response, string $ref) {
        $response = (array) $response;
         // e.g. #/component/schema/Environment.
        $response['$ref'] = $ref;
        $reference = new Reference($response);
        $context = new ReferenceContext($this->spec, self::SpecUrl);
        $schema = $reference->resolve($context);
        $validator = new SchemaValidator();
       // Throw exception because id property is missing on Environment.
        $validator->validate($response, $schema);
        return true;
    }

[typhonius]

I don’t think we can remove uuid because it would break bc. Would this be fixed by adding id as an additional property?

I think I would have originally used uuid here as the docs use the word uuid as the parameter to be passed in to any environment query.

[fiasco]

Turns out there is a lot more wrong with the API response that just the SDK mapping. But I did get it working by adding a number of missing fields to the EnvironmentResponse:

    /**
     * @param object $environment
     */
    public function __construct($environment)
    {
        $this->uuid = $environment->id;
        $this->label = $environment->label;
        $this->name = $environment->name;
        $this->application = $environment->application;
        $this->domains = $environment->domains;
        $this->active_domain = $environment->active_domain;
        $this->default_domain = $environment->default_domain;
        $this->image_url = $environment->image_url;
        if (property_exists($environment, 'ssh_url')) {
            $this->ssh_url = $environment->ssh_url;
        }
        $this->size = $environment->size;
        $this->weight = $environment->weight;
        $this->ips = $environment->ips;
        $this->region = $environment->region;
        $this->balancer = $environment->balancer;
        $this->platform = $environment->platform;
        $this->status = $environment->status;
        $this->type = $environment->type;
        $this->vcs = $environment->vcs;
        $this->vcs->url = $this->vcs->url;
        $this->configuration = $environment->configuration;
        $this->flags = $environment->flags;
        $this->_links = $environment->_links;
    }

I needed to add image_url, active_domain, default_domain, size, weight. I also had to rename links to _links and sshUrl to ssh_url which would break BC. I think artifact also needs to be added to this list.

Those additions and breaking changes aside, there were other things wrong with the API spec that didn't allow environment validation to pass:

• image_url must be nullable
• size must be nullable
• vcs.url format should not be set as the git url is not compliant with the url format
• cloud_actions flag is missing from environment flags
• integers in configuration.php should be nullable
• configuration.php.apcu has a minimum value of 32 but I saw a response with as little as 8.
• artifact must be nullable

For me, this means I have to manipulate the schema before I pass it onto validation. But it would be good to at least have the non-BC additions to the EnvironmentResponse , then perhaps I'll have to do a translation of the EnvironmentResponse into a spec compliant Environment object :(

[danepowell]

Can you submit a PR for this? I'm fine with cutting a new major release if this is a breaking change, but I want to do all of the breaking changes at once. I wish we'd rolled this into 3.0.