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

allOf doesn't work with $ref #51

Closed
yunosh opened this issue Mar 9, 2020 · 9 comments
Closed

allOf doesn't work with $ref #51

yunosh opened this issue Mar 9, 2020 · 9 comments

Comments

@yunosh
Copy link

yunosh commented Mar 9, 2020

Whe using allOf to compose a reference with a single object, the object is ignored. Futhermore, writing a failing unit test for this issue isn't possible either, because the schema validator requires all allOf components to be a cebe\openapi\spec\Schema and doesn't allow cebe\openapi\spec\Reference.

@yunosh
Copy link
Author

yunosh commented Mar 9, 2020

<?php

declare(strict_types=1);

namespace League\OpenAPIValidation\Tests\FromCommunity;

use League\OpenAPIValidation\Schema\Exception\TypeMismatch;
use League\OpenAPIValidation\Schema\SchemaValidator;
use League\OpenAPIValidation\Tests\Schema\SchemaValidatorTest;

final class AllOfTest extends SchemaValidatorTest
{
    public function testItValidatesAllOfGreen() : void
    {
        $spec = <<<SPEC
schema:
  allOf:
    - \$ref: '#/components/schemas/Base'
    - type: object
      properties:
        age:
          type: integer
components:
  schemas:
    Base:
      type: object
      properties:
        name:
          type: string
SPEC;

        $schema = $this->loadRawSchema($spec);
        $data   = ['name' => 'Dima', 'age' => 10];

        (new SchemaValidator())->validate($data, $schema);
        $this->addToAssertionCount(1);
    }

    public function testItValidatesAllOfRed() : void
    {
        $spec = <<<SPEC
schema:
  allOf:
    - \$ref: '#/components/schemas/Base'
    - type: object
      properties:
        age:
          type: integer
components:
  schemas:
    Base:
      type: object
      properties:
        name:
          type: string
SPEC;

        $schema = $this->loadRawSchema($spec);
        $data   = ['name' => 'Dima', 'age' => 10.5];

        $this->expectException(TypeMismatch::class);

        (new SchemaValidator())->validate($data, $schema);
    }
}

@scaytrase
Copy link
Member

Failing test is also test, even if it fails by type error. I'll take a look on this, thanks for reporting

@rejinka
Copy link
Contributor

rejinka commented Apr 23, 2020

Tried to fix this issue (and am doing so further). The problem exists for all *Of keywords, as the code indicates. A possible solution could be to extend League\OpenAPIValidation\Schema\Validator::validate(), so that it also allows references as second parameter and a possibility to resolve this reference to a schema.

@scaytrase
Copy link
Member

I thought that all references are resolved on schema loading...

@rejinka
Copy link
Contributor

rejinka commented Apr 23, 2020

They do, as i found out, a few minutes ago. By the factories, not by SchemaValidatorTest::loadRawSchema()

@rejinka
Copy link
Contributor

rejinka commented Apr 23, 2020

#55 in combination with this tests show, that allOf does work with references. As @scaytrase indicated, the validator isn't responsible for resolving references to schemas. This is done before in the Factories. Then there aren't any references anymore, which the validator has to deal with.

@scaytrase
Copy link
Member

I've merged tests fix, is this still a problem?

@rejinka
Copy link
Contributor

rejinka commented May 5, 2020

I don't see a problem anymore. The issue in our project, which was the source for this issue, is already resolved.

@scaytrase
Copy link
Member

Let me know if problem is still actual

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

No branches or pull requests

3 participants