cast does not "atomify" keys in maps of enum payloads

[mbuhot]

From #42

enums validation: cast does not "atomify" keys in maps of enum payloads. In other words, the following code does not crash:

schema = %Schema{enum: [%{id: 42}]}
spec = ...
# string keys
{:ok, data} = OpenApiSpex.cast(spec, schema, %{"id" => 42})
{:error, _} = OpenApiSpex.validate(spec, schema, data)
# atom keys
{:ok, data} = OpenApiSpex.cast(spec, schema, %{id: 42})
:ok = OpenApiSpex.validate(spec, schema, data)

I think it would be very dangerous (OOM) to atomify enum keys anyway!
My current workaround is to specify enum schemas with string keys instead, which can be easily missed by anyone.

So how about throwing an error when creating a schema that has atom keys in enum: ...?

[fenollp]

How about using String.to_existing_atom/1?

Doesn’t blow up the atom table & throws. Only requirement is that the schemas modules be loaded before casting.

[hauleth]

Why you cast data before validating it? IMHO it should work other way around.

[mbuhot]

Casting query params from strings to eg integers is required before they can be validated against min/max/multipleOf.

The api was inspired by ecto Changeset cast and validate_* functions.

[hauleth]

@mbuhot why though? You can do something like indifferent access in validator:

def get_indifferent(map, key) when is_atom(key) do
  with :error <- Map.get(map, key),
       :error <- Map.get(map, Atom.to_string(key))
  do
    :error
  else
    return -> return
  end
end

def get_indifferent(map, key) when is_binary(key) do
  with :error <- Map.get(map, key),
       :error <- Map.get(map, String.to_existing_string(key))
  do
    :error
  else
    return -> return
  end
rescue
  ArgumentError -> :error
end

Which will try all possible methods starting with "most safe ones".

[hauleth]

With Ecto 3.0 we can even use Ecto.Changeset to build validator and use it instead of building custom solution.

[moxley]

While not exactly on the topic of this issue, I wanted to address the point about casting and validating. I agree with @hauleth. I don't think there's any value in following Ecto's example of "casting" (actually it only casts primitives) followed by validating. Validations in Open API are defined in the same schema that defines how the data should be casted. This is not the case with Ecto (< 3.0) where the schema and validations are in separate places and incompatible styles (declarative, imperative), and therefore have to be performed as separate steps. In Ecto, I think it's a design side effect rather than a design intention that they're separate.

The following PR combines casting and validating into one step: #69. We're ready to have it reviewed, then merged. It rewrites the casting and validation part of Open API Spex. Once this is in, issues like this that arise from having separate casting and validating will likely go away.

[moxley]

@mbuhot: Is this a bug or an enhancement? Also, is this resolved with the new Cast module we implemented that combines validation with casting?

[mbuhot]

@moxley I think this is an enhancement that is now possible to implement with the combined CastAndValidate process.