aj-foster/open-api-generator

Nullable fields are not reflected in the module typespec

feynmanliang opened this issue · 4 comments

feynmanliang commented

When I mark a field as non-nullable in my OAI spec

          "stream": {
            "type": "boolean",
            "nullable": false
          },

I expect both the __fields__ and the module typespec to reflect it.

Instead, it seems that the generated module typespec remains nullable while the fields are non-nullable.

  @type t :: %__MODULE__{
          stream: boolean | nil
        }

...

  @doc false
  @spec __fields__(atom) :: keyword
  def __fields__(type \\ :t)

  def __fields__(:t) do
    [
      stream: :boolean
    ]
  end
end
aj-foster commented

Hi @feynmanliang, this occurs because the property is not set as required on the parent schema.

I chose to represent this state (a missing key in the API response) as nil in the typespec, the same as if the key were present but the value were null. There's definitely room for confusion here. It's also possible that I'm misunderstanding JSON Schema:

required

The value of this keyword MUST be an array. Elements of this array, if any, MUST be strings, and MUST be unique. An object instance is valid against this keyword if every item in the array is the name of a property in the instance. Omitting this keyword has the same behavior as an empty array.

I think this means a missing required key should have this behaviour (all properties are optional), but that may be wrong.

feynmanliang commented

That makes sense, thanks for the explanation. Could we do something like optional(:key_name) in the typespec instead for keys that are missing in required (e.g. https://elixirforum.com/t/typespec-for-map-w-both-required-and-optional-keys/21539/6). Currently representing a nullable: false but not required field using a null causes JSONSchema validation to fail:

image

aj-foster commented

I'm open to using optional(:key) if Dialyzer still accepts nil values in that case, however it doesn't look like this syntax is currently supported for structs (see comment).

After typing the above, I tested to see if Dialyzer would accept nil if the key is optional().

defmodule Test do
  @type t :: %{
          required(:a) => String.t() | nil,
          optional(:b) => String.t()
        }

  @spec thing :: t
  def thing do
    %{
      a: "Hello",
      b: nil
    }
  end
end

Unfortunately, Dialyzer catches this with the following error:

Invalid type specification for function 'Elixir.Test':thing/0. The success typing is 
          () -> #{'a' := <<_:40>>, 'b' := 'nil'}

So even if we use optional() for a key, we will still have to give a default type for the value (nil or otherwise).