Cannot use `id` as a property in response examples

mschout commented 4 years ago

Seems that we cannot use id as a property in a response example.

E.g.: consider this response definition:

responses:
  200:
    description: foo
    schema:
      type: object
      properties:
        id:
          type: integer
    examples:
      application/json:
        id: 12345

This fails in load_and_validate_schema because, if I undesrtand the problem correctly, JSON::Validator is trying to resolve id as a URL.

Digging a bit into this, it seems that the JSON schema version is set to 2 or 3 because the OpenAPI version is set as version(), and this puts JSON::Validator into the mode where id is considered a reference, whereas if JSON schema version 7 or higher is used, it would have used $id instead.

If I manually change sub _id_key in JSON::Validator to just return $id instead, then the problem goes away. Maybe JSON::Validator::OpenAPI::Mojolicious should not define a version attribute, or it should be renamed to openapi_version or something else so it does not conflict with the JSON schema version in JSON::Validator?

Not sure whats the best solution here as several modules are involved.

kiwiroy commented 4 years ago

IIRC this is similar to mojolicious/json-validator#172

mschout commented 4 years ago

Yes, same issue.

mschout commented 4 years ago

The more I think about this, I think the version attribute should be renamed to avoid unintended consequences.

jhthorsen commented 3 years ago

I don't know how to reproduce what you are trying to describe. Please provide a more complete test case. Also have a look at the test I just committed (which works)

kiwiroy commented 3 years ago

@jhthorsen please consider https://github.com/jhthorsen/mojolicious-plugin-openapi/compare/master...kiwiroy:failing-test-173 as a failing test case, although the failure appears to be JSON::Validator's (172 referenced above) rather than this plugin. https://github.com/mojolicious/json-validator/compare/master...kiwiroy:file-schema-id isn't necessarily the solution there if I read @karenetheridge's comment correctly.

mschout commented 3 years ago

Ok, yeah, I remember now. @kiwiroy's test is a good example of the problem, as is the one I have in my repo (in commit cb90086)

Somewhat interesting, if run as part of the entire test suite, it passes, but if you just run the individual test itself, it blows up:

$ perl -Ilib t/v3-example-with-id.t
Can't locate object method "scheme" via package "Mojo::File" at /home/mschout/.plenv/versions/5.30.3/lib/perl5/site_perl/5.30.3/Mojo/URL.pm line 129.

This is after cherry-picking cb90086 into your master branch, with latest Mojolicious and JSON::Validator installed.

Adding a Carp::cluck there in Mojo::URL, the path to get to this exception is:

Trace at /home/mschout/.plenv/versions/5.30.3/lib/perl5/site_perl/5.30.3/Mojo/URL.pm line 128.
    Mojo::URL::to_abs(Mojo::URL=HASH(0x55c4a6786b80), Mojo::File=SCALAR(0x55c4a6a45488)) called at /home/mschout/.plenv/versions/5.30.3/lib/perl5/site_perl/5.30.3/JSON/Validator.pm line 479
    JSON::Validator::_resolve(JSON::Validator::OpenAPI::Mojolicious=HASH(0x55c4a673d8c0), Mojo::Home=SCALAR(0x55c4a674bea0)) called at lib/JSON/Validator/OpenAPI/Mojolicious.pm line 38
    JSON::Validator::OpenAPI::Mojolicious::load_and_validate_schema(JSON::Validator::OpenAPI::Mojolicious=HASH(0x55c4a673d8c0), Mojo::Home=SCALAR(0x55c4a674bea0), HASH(0x55c4a69f5bc8)) called at lib/Mojolicious/Plugin/OpenAPI.pm line 35
    Mojolicious::Plugin::OpenAPI::register(Mojolicious::Plugin::OpenAPI=HASH(0x55c4a69f5910), Mojolicious::Lite=HASH(0x55c4a63c5450), HASH(0x55c4a674c218)) called at /home/mschout/.plenv/versions/5.30.3/lib/perl5/site_perl/5.30.3/Mojolicious/Plugins.pm line 45
    Mojolicious::Plugins::register_plugin(Mojolicious::Plugins=HASH(0x55c4a63ce7e8), "OpenAPI", Mojolicious::Lite=HASH(0x55c4a63c5450), HASH(0x55c4a674c218)) called at /home/mschout/.plenv/versions/5.30.3/lib/perl5/site_perl/5.30.3/Mojolicious.pm line 178
    Mojolicious::plugin(Mojolicious::Lite=HASH(0x55c4a63c5450), "OpenAPI", HASH(0x55c4a674c218)) called at /home/mschout/.plenv/versions/5.30.3/lib/perl5/site_perl/5.30.3/Mojolicious/Lite.pm line 45
    main::plugin("OpenAPI", HASH(0x55c4a674c218)) called at t/v3-example-with-id.t line 13
Can't locate object method "scheme" via package "Mojo::File" at /home/mschout/.plenv/versions/5.30.3/lib/perl5/site_perl/5.30.3/Mojo/URL.pm line 129.

So it seems the plugin wound up calling JSON::Validator::_resolve(), which seems to be (per code comment):

  # _resolve() method is used to convert all "id" into absolute URLs and
  # resolve all the $ref's that we find inside JSON Schema specification.

Specifically, its blowing up in the to_abs call here:

        if ($topic->{$id_key} and !ref $topic->{$id_key}) {
          my $fqn = Mojo::URL->new($topic->{$id_key});
          $fqn = $fqn->to_abs($base) unless $fqn->is_abs;
          $self->_register_schema($topic, $fqn->to_string);
        }

and running through the debugger, $id_key is the string id here.

If I remove id from the example block of the response section of t/spec/v3-example_with_id.yaml then it does NOT blow up at all.

So TL;DR here:

The problem still seems to exist
If the test is run as part of the entire test suite, the problem does not manifest (I have no idea why offhand)
The problem DOES happen if running only the individual test from cb90086 by itself (also same with @kiwiroy's test)
The end result is a cryptic Can't locate object method "scheme" via package "Mojo::File", but I do not think this code pathway should have been entered at all in the first place since the id in example of the spec is not a reference at all.

Hopefully that makes it clear. If not let me know :).

jhthorsen commented 2 years ago

Pretty sure this is fixed in https://github.com/jhthorsen/json-validator/pull/251. Please reopen if it still does not work.

jhthorsen / mojolicious-plugin-openapi

Cannot use `id` as a property in response examples #173