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

Casting schema with allOf and properties does not work with strings #641

Open
xxdavid opened this issue Oct 31, 2024 · 3 comments
Open

Casting schema with allOf and properties does not work with strings #641

xxdavid opened this issue Oct 31, 2024 · 3 comments

Comments

@xxdavid
Copy link

xxdavid commented Oct 31, 2024

The title may be a bit cryptic but here are a few examples

alias OpenApiSpex.Schema

%Schema{
  type: :object,
  allOf: [
    %Schema{
      type: :object,
      properties: %{a: %Schema{type: :integer}}
    }
  ],
}
|> then(&OpenApiSpex.cast_value(%{"a" => 1}, &1))
# => {:ok, %{a: 1}}

and

%Schema{
  type: :object,
  properties: %{b: %Schema{type: :integer}},
  required: [:b]
}
|> then(&OpenApiSpex.cast_value(%{"b" => 2}, &1))
# => {:ok, %{b: 2}}

but

%Schema{
  allOf: [
    %Schema{
      type: :object,
      properties: %{a: %Schema{type: :integer}}
    }
  ],
  type: :object,
  properties: %{b: %Schema{type: :integer}},
  required: [:b]
}
|> then(&OpenApiSpex.cast_value(%{"a" => 1, "b" => 2}, &1))
# => {:error,
# [
#   %OpenApiSpex.Cast.Error{
#     reason: :missing_field,
#     value: %{"a" => 1, "b" => 2},
#     format: nil,
#     type: nil,
#     name: :b,
#     path: [:b],
#     length: 0,
#     meta: %{}
#   }
# ]}

However, if I use an atom for b, everything works.

%Schema{
  allOf: [
    %Schema{
      type: :object,
      properties: %{a: %Schema{type: :integer}}
    }
  ],
  type: :object,
  properties: %{b: %Schema{type: :integer}},
  required: [:b]
}
|> then(&OpenApiSpex.cast_value(%{"a" => 1, :b => 2}, &1))
# => {:ok, %{a: 1, b: 2}}
@msutkowski
Copy link
Contributor

@xxdavid I wouldn't expect those examples to work. Based on the spec, allOf should really only be evaluated by itself and your b schema should be the 2nd element in the list. E.g.

%Schema{
  allOf: [
    %Schema{
      type: :object,
      properties: %{a: %Schema{type: :integer}}
    },
    %Schema{
      type: :object,
      required: [:b],
      properties: %{b: %Schema{type: :integer}}
    }
  ],
}

References:

The behavior you're showing at the end there with an atom for b seems like a bug to me and I wouldn't expect any variation of that to work 🤔

@xxdavid
Copy link
Author

xxdavid commented Nov 12, 2024

Hi @msutkowski, can you please quote anything in the spec or docs that explicitly says that allOf cannot be combined with properties? I've found the spec and docs quite vague on this. Based on the fact that all examples in the docs used allOf without other keys, we can say that perhaps that is the intended use. However, using it with other keys is not forbidden either.

Nevertheless, I've found an example in the docs for JSON schema (which AFAIK Open API's schema is based on) which uses both allOf and properties: https://json-schema.org/understanding-json-schema/reference/object#extending (section Extending Closed Schemas). But I am not sure what's the exact relation between Open API's schema and JSON Schema and whether we can draw conclusions from this.

@msutkowski
Copy link
Contributor

msutkowski commented Nov 12, 2024

Hi @msutkowski, can you please quote anything in the spec or docs that explicitly says that allOf cannot be combined with properties? I've found the spec and docs quite vague on this. Based on the fact that all examples in the docs used allOf without other keys, we can say that perhaps that is the intended use. However, using it with other keys is not forbidden either.

Nevertheless, I've found an example in the docs for JSON schema (which AFAIK Open API's schema is based on) which uses both allOf and properties: https://json-schema.org/understanding-json-schema/reference/object#extending (section Extending Closed Schemas). But I am not sure what's the exact relation between Open API's schema and JSON Schema and whether we can draw conclusions from this.

Just what I linked already. From a general usability perspective, IMO it doesn't really make sense to merge a base object into a potential union type. But, even though its not explicitly stated that "you can't use base properties + anyOf/oneOf/allOf... it does say:

Use the anyOf keyword to validate the data against any amount of the given subschemas.

It's also not a part of the passing test scenarios, so I wouldn't bother trying to bend things - https://github.com/OAI/OpenAPI-Specification/blob/main/tests/v3.0/pass/petstore-expanded.yaml#L128

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