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

openapi.spec broken in 2.10 #107

Closed
mohawk2 opened this issue Feb 1, 2019 · 4 comments
Closed

openapi.spec broken in 2.10 #107

mohawk2 opened this issue Feb 1, 2019 · 4 comments
Labels

Comments

@mohawk2
Copy link
Contributor

mohawk2 commented Feb 1, 2019

It seems that 2.09 worked fine, but 2.10 has broken Yancy. Now the openapi.spec helper is returning a Mojo::Collection of two different response specs, which then blows up the plugin on line 282 which wants the $op_spec to be a hash, not that.

Possibly connected is that there are two different string constants used here, for_current vs for_path:

sub _helper_get_spec {
  my $c    = shift;
  my $path = shift // 'for_current';
  my $self = _self($c);

  return $self->validator->get($path) if ref $path or $path =~ m!^/! or !length $path;

  my $jp;
  for my $s (reverse @{$c->match->stack}) {
    $jp ||= [paths => $s->{'openapi.path'}];
  }

  push @$jp, lc $c->req->method if $jp and $path ne 'for_path';    # Internal for now
  return $jp ? $self->validator->get($jp) : undef;
}

See https://travis-ci.org/preaction/Yancy/jobs/487286044#L698 for an example.

@preaction FYI.

@jhthorsen
Copy link
Owner

Can you link to the test that blows up in Yancy? Also, what is the actual JSON pointer? Sure it's valid?

@mohawk2
Copy link
Contributor Author

mohawk2 commented Feb 1, 2019

The test, which is shown in the Travis log I linked, is (even with the line that fails): https://github.com/preaction/Yancy/blob/master/t/plugin/auth/basic.t#L119

The "actual JSON pointer" is set to the array [ 'paths', undef, 'get' ]. It seems the problem is that $s->{'openapi.path'} is undef. The request that fails is for /yancy/api, which I infer leaves that openapi.path as undef. Should the above should actually read:

($s->{'openapi.path'} // '') ? (or possibly openapi.path should be set to '' in that scenario)

You have not addressed my point about for_current vs for_path in your code: are they supposed to be different?

@mohawk2
Copy link
Contributor Author

mohawk2 commented Feb 5, 2019

This is the commit that broke Yancy: b2667ef

mohawk2 added a commit to mohawk2/mojolicious-plugin-openapi that referenced this issue Feb 5, 2019
@mohawk2
Copy link
Contributor Author

mohawk2 commented Feb 5, 2019

@preaction Good news, #108 should sort this out, assuming it is found suitable by @jhthorsen!

mohawk2 added a commit to mohawk2/mojolicious-plugin-openapi that referenced this issue Feb 9, 2019
jhthorsen pushed a commit that referenced this issue Feb 14, 2019
 - Fix HEAD requests #105
 - Fix using /servers/0/url as basePath for OpenAPI v3 #110
   Note: This could be breaking change
 - Fix getting basePath when using under #107
 - Add support for "nullable" in OpenAPI 3.0 #106
 - Improved handling of Accept header in OpenAPI v3 #104
   Can now handle wildcards, such as application/* and */*, even though not
   defined in the specification.
 - Bump JSON::Validator to 3.06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants