jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
54 stars 44 forks source link

OpenAPI v3: Support for nullable fields #106

Closed NGEfreak closed 5 years ago

NGEfreak commented 5 years ago

OpenAPI v3 features a new option to declare fields as nullable. Reference: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#fixed-fields-20

This doesn't appear to work yet.

#!/usr/bin/env perl

use 5.010;
use strict;
use warnings;
use utf8;

use Test::More;
use Test::Mojo;

use Mojolicious::Lite;

get '/test' => sub {
    shift->render( openapi => { foo => undef }, status => 200 );
}, 'test';

plugin 'OpenAPI' => {
    url    => 'data:///api.yml',
    schema => 'v3',
};

my $t = Test::Mojo->new();

$t->get_ok( '/test', { Accept => 'application/json' } )
    ->status_is(200)
    ->json_is( { foo => undef } );

done_testing;

__DATA__

@@ api.yml
openapi: 3.0.0
info:
  title: Test
  version: 0.0.0
paths:
  /test:
    get:
      x-mojo-name: test
      responses:
        '200':
          description: Test
          content:
            application/json:
              schema:
                properties:
                  foo:
                    type: string
                    nullable: true
jhthorsen commented 5 years ago

This whole issue is very depressing to me. Do you know why they don't encourage "type":[..., "null"] instead?

NGEfreak commented 5 years ago

According to https://github.com/OAI/OpenAPI-Specification/issues/229#issuecomment-164592837 they overlooked the fact that JSON Schema allows null and array values in type. I guess they introduced nullable to keep it consistent with v2.

In v2 null or nullable was not supported at all officially. However, many validators ignored this fact and applied the logic of JSON Schema or introduced keywords like x-nullable instead.

See also: https://github.com/OAI/OpenAPI-Specification/issues/1389 https://github.com/OAI/OpenAPI-Specification/issues/1443 https://github.com/OAI/OpenAPI-Specification/issues/397

jhthorsen commented 5 years ago

That's so sad 😥I was hoping there was a better reason, but I couldn't find one.

jhthorsen commented 5 years ago

Seems like there's an implementation more or less ready at https://github.com/mojolicious/json-validator/pull/146

augensalat commented 5 years ago

Hi, I understand why my change from mojolicious/json-validator#146 is better done in the OpenAPI plugin.

In order to implement the nullable check in M:P::OpenAPI I thougt to override _validate(), but then I'd have to copy all that dereferencing and anti-recursion logic from the base class _validate() and this doesn't feel right. It seems a "correct" solution would require a deeper intervention in the code, but I'm not familiar enough with it to do that.