allOf is not properly merged

[shadowhand]

Given a schema that uses allOf to combine two different object schemas, the properties of the two schemas are not combined:

{
  "components": {
    "schemas": {
      "identifier": {
        "type": "object",
        "properties": {
           "id": {"type": "string"}
        }
      },
      "person": {
        "allOf": [
          {"$ref": "#/components/schemas/identifier"},
          {
            "type": "object",
            "properties": {
              "name": {
                "type": "string"
              }
            }
          }
        ]
      }
    }
  }
}

Not 100% sure that schema validates, but it should be a good start.

[cebe]

Hi, thanks for the report, could you elaborate on how you read the schema and what you expect in the output. I added a test for your Schema (b9eddbd) and as far as I see everything is loaded correctly.

[shadowhand]

I believe the resolved reference should include both the id and name properties; something like:

assert($resolvedRef->type === 'object');
assert($resolvedRef->properties->id->type === 'string');
assert($resolvedRef->properties->name->type === 'string');

When I tried to use https://github.com/lezhnev74/openapi-psr7-validator it gave me errors with any schema that used allOf, stating that it couldn't find the (combined) properties.

[cebe]

The purpose of php-openapi is to read and write OpenAPI description files as they are, so you get the raw allOf element with its sub-schemas only. For schema validation you need to use other tools (at least now this is not implemented). The validation of the request schema against the API description has to be made by lezhnev74/openapi-psr7-validator which should validate the request based on allOf semantics.

[cebe]

ping @lezhnev74

[lezhnev74]

Hey! Ok, this looks like a problem with resolving references. Let me try to write a failing test first.

[lezhnev74]

I've added this test which validates a response body against a schema with a reference:

• a test
• a schema and a reference

The validator builds a schema in the memory and resolves all the references automatically. You can access resolved schema like this:

$schema = $validator->getSchema();
$allOf = $schema->paths['/ref']->post->responses['200']->content['application/json']->schema->allOf;
var_dump($allOf[0]->properties['age']->type); // "integer"
var_dump($allOf[1]->properties['name']->type); // "string"

[lezhnev74]

@cebe as I understand, one can resolve references manually like this:

use cebe\openapi\Reader;
use cebe\openapi\ReferenceContext;
use function realpath;


$filePath = "./schema.yaml";
$schema = Reader::readFromYamlFile($filePath);
$schema->resolveReferences(new ReferenceContext($schema, realpath($filePath)));

[cebe]

resolveReferences() is called automatically by default, so the reference should not be a problem:

$resolveReferences parameter defaults to true:

https://github.com/cebe/php-openapi/blob/13b91759f9daccd90729e1c246bab791f13e59c8/src/Reader.php#L93-L100

[cebe]

Hi, version 1.3.0 fixes a lot of issues with references, please try if this is solved by the changes.

[cebe]

Closing this issue for now, please comment if the issue persists. I'll reopen it then.

[hotmeteor]

@cebe We're still seeing this issue.

allOf components are not resolving correctly.

You can see the attached issue here: https://github.com/hotmeteor/spectator/issues/38

[cebe]

@hotmeteor I see https://github.com/hotmeteor/spectator/issues/38#issuecomment-824249505 is closed now, is there still something to fix in cebe/php-openapi or was this only a problem in your tool?

[hotmeteor]

No, there's still an issue. I created a workaround in my tool, but I'd prefer to see things merging as expected. This is the example schema that we're testing with: https://github.com/hotmeteor/spectator/blob/master/tests/Fixtures/Components.v1.json

[hotmeteor]

@cebe I forgot I had reported this issue, so I had created another ticket here: https://github.com/cebe/php-openapi/issues/120

It can be closed, but it does contain some useful information for testing.

[cebe]

still not really sure what this is about. Could someone come up with a failing test case for this?