jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
53 stars 41 forks source link

V3: writeOnly Attributes Marked as required Fails Response Validation #191

Closed atancasis closed 3 years ago

atancasis commented 3 years ago

Attribute marked as writeOnly can't be included in the required list as response validation would otherwise fail with a Missing property error.

According to OAS3, the writeOnly attribute, as excerpted from here:

Declares the property as "write only". Therefore, it MAY be sent as part of a request but SHOULD NOT be sent as part of the response. If the property is marked as writeOnly being true and is in the required list, the required will take effect on the request only.

However, as mentioned, response validation appears to still be expecting these writeOnly attributes to be returned as part of the response.

Outlined below is a test suite illustrating this scenario:

use Mojo::Base -strict;
use Test::Mojo;
use Test::More;

use Mojolicious::Lite;

post '/base'     => \&post_test, 'writeonly';
post '/required' => \&post_test, 'with_required';

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

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

$t->post_ok('/base'     => json => {app_id => 1})->status_is(201);
$t->post_ok('/required' => json => {app_id => 1})->status_is(201);

done_testing;

sub post_test {
  my $c = shift->openapi->valid_input or return;
  $c->render(openapi => {id => 1}, status => 201);
}

__DATA__
@@ writeonly.json
{
  "openapi": "3.0.3",
  "info": {
    "title": "Test",
    "version": "0.0.0"
  },
  "paths": {
    "/base": {
      "post": {
        "operationId": "writeonly",
        "requestBody": {
          "content": {
            "application/json": {
              "schema": { "$ref": "#/components/schemas/Base" }
            }
          }
        },
        "responses": {
          "201": {
            "description": "ok",
            "content": {
              "application/json": {
                "schema": { "$ref": "#/components/schemas/Base" }
              }
            }
          }
        }
      }
    },
    "/required": {
      "post": {
        "operationId": "with_required",
        "requestBody": {
          "content": {
            "application/json": { "schema": { "$ref": "#/components/schemas/Required" } }
          }
        },
        "responses": {
          "201": {
            "description": "ok",
            "content": {
              "application/json": { "schema": { "$ref": "#/components/schemas/Required" } }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Base" : {
        "required": ["id"],
        "properties": {
          "app_id": {
            "type": "integer",
            "writeOnly": true
          },
          "id": {
            "type": "integer",
            "readOnly": true
          }
        }
      },
      "Required": {
        "required": ["app_id", "id"],
        "properties": {
          "app_id": {
            "type": "integer",
            "writeOnly": true
          },
          "id": {
            "type": "integer",
            "readOnly": true
          }
        }
      }
    }
  }
}

This test suite illustrates 2 endpoints (POST /base and POST /required) which takes in an app_id and returns an id.

Note that the only difference between the Base and the Required schema is marking the writeOnly property, app_id, as required in the latter.

Running the outlined/attached test suite with MOJO_CLIENT_DEBUG=1 will illustrate that POST /required fails with a 500 with the following response:

HTTP/1.1 500 Internal Server Error\x0d
Content-Type: application/json;charset=UTF-8\x0d
Date: Mon, 21 Sep 2020 09:23:16 GMT\x0d
Server: Mojolicious (Perl)\x0d
Content-Length: 75\x0d
\x0d
{"errors":[{"message":"Missing property.","path":"\/app_id"}],"status":201}

We have been using this excellent plugin extensively for over a year now in our core API (thanks for the excellent, excellent work!) and we have been working around this issue by creating 3 schemas (1 for the request and 1 for the response, each composed from a base) but it would be really nice if writeOnly and required can be supported from this library.

Please don't hesitate to let me know if there's any additional details that I can provide. Thanks!

jhthorsen commented 3 years ago

Thanks for the kind words and the test case ❤️

This should be fixed in the upcoming release.

atancasis commented 3 years ago

thank you so much @jhthorsen, much much appreciated! 🙏

jhthorsen commented 3 years ago

It was very easy to fix when you submitted such a detailed bug report 👍