Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty additionalProperties object is generated as an array #700

Open
Rid opened this issue Feb 16, 2023 · 9 comments · May be fixed by #704
Open

Empty additionalProperties object is generated as an array #700

Rid opened this issue Feb 16, 2023 · 9 comments · May be fixed by #704
Labels

Comments

@Rid
Copy link

Rid commented Feb 16, 2023

Jane version(s) affected: 7.4.1
Description
When the OpenAPI v3 spec is as so:

          "ExposedPorts": {
            "type": "object",
            "additionalProperties": {
              "type": "object",
              "properties": {}
            },
            "description": "An object mapping ports to an empty object in the form:\n\n`{\"<port>/<tcp|udp|sctp>\": {}}`\n"
          },

Empty additionalProperties are generated as an array rather than an object in the normalizer, whereas the denormalizer correctly creates an object.

How to reproduce
See above.

Possible Solution
additionalProperties normalizer and denormalizer should be the same.

Additional context
Here is the generated code:

Normalizer:
https://github.com/beluga-php/docker-php-api/blob/0082dac1f074aa56abe82d6f18aebb5823559921/src/Normalizer/ContainersCreatePostBodyNormalizer.php#L245-L251

        if (null !== $object->getExposedPorts()) {
            $values = [];
            foreach ($object->getExposedPorts() as $key => $value) {
                $values[$key] = $this->normalizer->normalize($value, 'json', $context);
            }
            $data['ExposedPorts'] = $values;
        }

Denormalizer:
https://github.com/beluga-php/docker-php-api/blob/0082dac1f074aa56abe82d6f18aebb5823559921/src/Normalizer/ContainersCreatePostBodyNormalizer.php#L80-L85

        if (\array_key_exists('ExposedPorts', $data) && null !== $data['ExposedPorts']) {
            $values = new \ArrayObject([], \ArrayObject::ARRAY_AS_PROPS);
            foreach ($data['ExposedPorts'] as $key => $value) {
                $values[$key] = $this->denormalizer->denormalize($value, 'Docker\\API\\Model\\ContainerConfigExposedPortsItem', 'json', $context);
            }
            $object->setExposedPorts($values);
@Rid
Copy link
Author

Rid commented Feb 16, 2023

In janephp 4.2 this is generated as:

        if (null !== $object->getExposedPorts()) {
            $values = new \stdClass();
            foreach ($object->getExposedPorts() as $key => $value) {
                $values->{$key} = $value;
            }
            $data->{'ExposedPorts'} = $values;
        }

Is this a regression?

@Rid
Copy link
Author

Rid commented Feb 16, 2023

So it looks like janephp has moved away from using stdClass() for objects and now uses associative arrays, but how do you differentiate between an empty object and an empty array?

We probably need to check if the object is empty and return new \ArrayObject([], \ArrayObject::ARRAY_AS_PROPS)

@Rid
Copy link
Author

Rid commented Feb 16, 2023

This works, but would mean a check on every object:

        if ($object->isInitialized('exposedPorts') && null !== $object->getExposedPorts()) {
            $values_3 = [];
            foreach ($object->getExposedPorts() as $key => $value_3) {
                $values_4 = [];
                foreach ($value_3 as $key_1 => $value_4) {
                    $values_4[$key_1] = $value_4;
                }
                if (count($values_4) === 0) {
                    $values_4 = new \ArrayObject(array(), \ArrayObject::ARRAY_AS_PROPS);
                }
                $values_3[$key] = $values_4;
            }
            if (count($values_3) === 0) {
                $values_3 = new \ArrayObject(array(), \ArrayObject::ARRAY_AS_PROPS);
            }
            $data['ExposedPorts'] = $values_3;
        }

@Rid
Copy link
Author

Rid commented Feb 16, 2023

It might be cleaner to just set objects as so:

        if ($object->isInitialized('exposedPorts') && null !== $object->getExposedPorts()) {
            $values_3 = [];
            foreach ($object->getExposedPorts() as $key => $value_3) {
                $values_4 = [];
                foreach ($value_3 as $key_1 => $value_4) {
                    $values_4[$key_1] = $value_4;
                }
                $values_3[$key] = new \ArrayObject($values_4, \ArrayObject::ARRAY_AS_PROPS);
            }
            $data['ExposedPorts'] = new \ArrayObject($values_3, \ArrayObject::ARRAY_AS_PROPS);
        }

@Rid
Copy link
Author

Rid commented Feb 16, 2023

Or initialise as an array object:

        if ($object->isInitialized('exposedPorts') && null !== $object->getExposedPorts()) {
            $values_3 = new \ArrayObject(array(), \ArrayObject::ARRAY_AS_PROPS);
            foreach ($object->getExposedPorts() as $key => $value_3) {
                $values_4 = new \ArrayObject(array(), \ArrayObject::ARRAY_AS_PROPS);
                foreach ($value_3 as $key_1 => $value_4) {
                    $values_4[$key_1] = $value_4;
                }
                $values_3[$key] = $values_4;
            }
            $data['ExposedPorts'] = $values_3;
        }

@Rid
Copy link
Author

Rid commented Feb 16, 2023

Just an update, this holds true for any object which is empty regardless if it's an additionalProperties object.

We're going through and manually patching

$values = [];

with

$values = new \ArrayObject([], \ArrayObject::ARRAY_AS_PROPS);

Anywhere in the specification where we are returning an object.

It seems like this is a pretty big bug @Korbeil, I'll have another look at the codebase over the weekend and see if I can find any way to fix it myself.

@Korbeil
Copy link
Member

Korbeil commented Feb 17, 2023

There is a trait CheckArray that has the role to check if the property is an array or not, you should check it and use it for this case. (see src/Component/JsonSchema/Generator/Runtime/data/Normalizer/CheckArray.php)

@achton
Copy link

achton commented Feb 27, 2023

This problem also exists for specs that contain string dictionaries, like this:

        buildingFields:
          type: object
          additionalProperties:
            type: string
          example:
            Carport: "39 m²"
            Outhouse: "8 m²"
          nullable: true

When setting the field to NULL (ie. $model->setBuildingFields(NULL);), the normalized output becomes an empty array:

{
  "buildingFields":[]
}

My expectation (and other consumers of this output) is that this would be an empty object instead (or null since the property is nullabel).

Are there any solutions for this case?

@Rid
Copy link
Author

Rid commented Feb 27, 2023

@achton This PR solves the issue: #704 we're waiting for a review to confirm the fix, then we can fix the failing tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants