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

Validation fails when for array saying it is an object #19

Closed
amplexdenmark opened this issue Feb 3, 2023 · 18 comments
Closed

Validation fails when for array saying it is an object #19

amplexdenmark opened this issue Feb 3, 2023 · 18 comments

Comments

@amplexdenmark
Copy link

amplexdenmark commented Feb 3, 2023

Hi again

Now that the validation is running for my POST request, I have run into the next issue.

When I POST an array as specified, I get:
Failed to validate JSON schema with an error: Type mismatch. Expected Array but got Object.

In line: 66 in Oasis.Validator the value = %{"_json" => [%{"id" => 1, "name" => "SH"}]}
This leads to the call to ExJsonSchema.Validator.validate in line 99 to fail, because the passed in value is not a List
It should only be the value for the key "_json" that should be passed into ExJsonSchema.Validator.validate

My oas definition is:

{
   "openapi": "3.1.0",
   "info": {
      "title": "User Array Example"
   },
   "paths": {
      "/users": {
         "post": {
            "summary": "Adds a new user",
            "requestBody": {
               "content": {
                  "application/json": {
                     "schema": {
                        "$ref": "#/components/schemas/ArrayOfUsers"
                     }
                  }
               }
            },
            "responses": {
               "200": {
                  "description": "OK"
               }
            }
         }
      }
   },
   "components": {
      "schemas": {
         "ArrayOfUsers": {
            "type": "array",
            "items": {
               "type": "object",
               "properties": {
                  "id": {
                     "type": "integer"
                  },
                  "name": {
                     "type": "string"
                  }
               }
            }
         }
      }
   }
}

and I test using this data: [{"id":1,"name":"SH"}]

@xinz
Copy link
Contributor

xinz commented Feb 6, 2023

Hello @amplexdenmark, thanks for your feedback, here is the mentioned case to discuess:

iex(9)> s
%ExJsonSchema.Schema.Root{
  schema: %{
    "items" => %{
      "properties" => %{
        "id" => %{"type" => "integer"},
        "name" => %{"type" => "string"}
      },
      "type" => "object"
    },
    "type" => "array"
  },
  version: 7
}
iex(10)> ExJsonSchema.Validator.validate(s, [%{"id" => 1, "name" => "sh"}])
:ok
iex(11)> ExJsonSchema.Validator.validate(s, %{"id" => 1, "name" => "sh"})
{:error, [{"Type mismatch. Expected Array but got Object.", "#"}]}
iex(12)> ExJsonSchema.Validator.validate(s, %{"_json" => [%{"id" => 1, "name" => "sh"}]})
{:error, [{"Type mismatch. Expected Array but got Object.", "#"}]}

We can see that ExJsonSchema validation expects to input an Array not an Object, your openapi spec defined ArrayOfUsers as an array, but why your test case send request body is not array but an object, be more specific why there is a "_json" wrapper in your request body?

@amplexdenmark
Copy link
Author

Hello @xinz

Exactly that is the question. Where does the "_json" wrapper come from.

The example I'am using is extremely simple, my mix.exs depends is:

defp deps do
    [
      {:plug_cowboy, "~> 2.0"},
      {:oasis, git: "https://github.com/elixir-oasis/oasis.git", tag: "main"},
    ]
 end

You gave seen my oas3 definition above, and the application.ex is just:

defmodule UserArrray.Application do
  @moduledoc false

  use Application

  @impl true
  def start(_type, _args) do
    children = [
      Plug.Adapters.Cowboy.child_spec(
        scheme: :http,
        plug: Oasis.Gen.Router,
        options: [
          port: 8807
        ]
      )
    ]
    opts = [strategy: :one_for_one, name: UserArrray.Supervisor]
    Supervisor.start_link(children, opts)
  end
end

That is all.

I then test using curl like this:
curl -v -H "Content-Type: application/json" -X POST http://localhost:8807/users -d '[{"id":1,"name":"SH"}]'

So to me it looks like Oasis adds that wrapper but it could of course be Plug Cowboy.

@amplexdenmark
Copy link
Author

Yes, it is added by Plug.Parsers.JSON.decode/2 line 96.
Plug.Parsers.JSON.decode/2 is called by oasis as part of the Oasis.Validator.parse_and_validate!/4 lines 20-23

@xinz
Copy link
Contributor

xinz commented Feb 6, 2023

Great, thanks, I will continue to investigate.

@amplexdenmark
Copy link
Author

I'm not sure that last analysis is correct though, but my debugger indicates it.
I have not been able to break on a breakpoint in Oasis.Router, the first break point i stop at is in Plug.Parsers.JSON.decode/2 and stepping ends me in Oasis.Validator.parse_and_validate!/4 at the process() statement.

@xinz
Copy link
Contributor

xinz commented Feb 6, 2023

@amplexdenmark Currently, there uses Plug.Parsers to automatically parse request body, but it will append "_json" into the body when post body is an array, maybe we can deleted the appended "_json" wrapper when post body is an array, but it also maybe hard to prevent the use case which post with the user input "_json" key of value, WDYT?

@amplexdenmark
Copy link
Author

Well, I think that the function defp process({%{"content" => content}, use_in, name, value}) do in Oasis.Validator, should check for receiving a Map with a single key "_json" and unwrap the value.
I realise that this will break Oas definitions actually defining an object with a sole "_json" key, a rare situation though.

Actually it is because Plug.Parsers.JSON.decode/2 returns a Map and therefore wraps arrays and other non object json.
It would be better if it always did the wrapping then users could consistently unwrap the result.
It is however clearly documented here Plug.Parsers.JSON
And I guess that code is used to many places to make them change the behaviour.

@amplexdenmark
Copy link
Author

Not sure the unwrapping should be in Oasis.Validator.process/4 maybe earlier

@xinz
Copy link
Contributor

xinz commented Feb 6, 2023

So you think we need to unwrap "_json" key if existed this issue case, right?

@amplexdenmark
Copy link
Author

Yes, otherwise oasis can only validate json with objects.
I believe arrays are a more common case than an object with a sole "_json" key.

@xinz
Copy link
Contributor

xinz commented Feb 6, 2023

I see, I will try it, thanks again for your help.

@xinz
Copy link
Contributor

xinz commented Feb 7, 2023

hello @amplexdenmark, we have a PR to resolve this issue, could you please check with the latest main branch from your side?

@amplexdenmark
Copy link
Author

amplexdenmark commented Feb 10, 2023

FYI,

I have gotten a PR accepted for Plug.Parsers.JSON

This introduces an option for Plug.Parsers: nest_all_json which will wrap all json parsed into a map with a _json key.
So using this option (when the new Plug is released) the unwrapping in Oasis will always be correct.

@amplexdenmark
Copy link
Author

I have now checked the latest branch and it looks good :)

Only thing I noticed is that a recursive validation is not performed.
That is if the OAS specifies an array of some object, it is correctly checked that the incoming json is indeed an array, but it is not checked if all the items is indeed of that object type.

Likewise for an object, it is validated that an object is received, but not that it is the correct type.

For the above example both these curl commands validates correctly, not just the first one.

curl -v -H "Content-Type: application/json" -X POST http://localhost:8807/users -d '[{"id":1,"name":"SH"}]'

curl -v -H "Content-Type: application/json" -X POST http://localhost:8807/users -d '[{"id2":1,"named":"SH"}]'

@xinz
Copy link
Contributor

xinz commented Feb 13, 2023

Hello,

Since this above spec definition does not define a required field make the command with [{"id2":1,"named":"SH"}] request body passed.

         "ArrayOfUsers": {
            "type": "array",
            "items": {
               "type": "object",
               "properties": {
                  "id": {
                     "type": "integer"
                  },
                  "name": {
                     "type": "string"
                  }
               }
            }
         }

I tested it works as expected, could you double check use the example like this?

        "ArrayOfUsers": {
            "type": "array",
            "items": {
               "type": "object",
               "properties": {
                  "id": {
                     "type": "integer"
                  },
                  "name": {
                     "type": "string"
                  }
               },
               "required": true
            }
         }

@amplexdenmark
Copy link
Author

Hello,

My bad, of course I need the "required", however the correct syntax is "required": ["id","name"]
The example you gave gives: error: Required property true was not present

So to me, this issue is now resolved. Thank you for the collaboration

@xinz
Copy link
Contributor

xinz commented Feb 14, 2023

FYI,

Currently, it heavily leverages ExJsonSchema validation, these raw error messages are from ExJsonSchema:

1, error: Required property true was not present. when set require: true;
2, error: Required properties id, name were not present. when set require: ["id","name"].

Thanks clarify, yes, the correct syntax is "required": ["id","name"], my fault.

xinz added a commit that referenced this issue Feb 14, 2023
@xinz
Copy link
Contributor

xinz commented Feb 14, 2023

Released in 0.5.1

@xinz xinz closed this as completed Feb 14, 2023
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

2 participants