Skip to content

[elixir] Typespec - allow null on optional struct-attributes#1514

Merged
wing328 merged 5 commits intoOpenAPITools:masterfrom
cyon:elixir/nullable-optionals
Nov 29, 2018
Merged

[elixir] Typespec - allow null on optional struct-attributes#1514
wing328 merged 5 commits intoOpenAPITools:masterfrom
cyon:elixir/nullable-optionals

Conversation

@mrmstn
Copy link
Copy Markdown
Contributor

@mrmstn mrmstn commented Nov 21, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

This PR allows null-values for struct-attributes which are not marked as required. This is very important since the compiler would mark this as a error since a null value in a struct would break the typespec contract.

With this small typespec fix, non-required and nullable attributes will be markes as possible nil values for the given model

image

@mrmstn mrmstn changed the title [ELIXIR] Typespec - allow null on optional parameters [ELIXIR] Typespec - allow null on optional struct-attributes Nov 21, 2018
@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 21, 2018

@mrmstn
Copy link
Copy Markdown
Contributor Author

mrmstn commented Nov 23, 2018

I think this would be a good idea - I kind of missed the existance of nullable all alonge 🙈

@mrmstn mrmstn changed the title [ELIXIR] Typespec - allow null on optional struct-attributes [elixir] Typespec - allow null on optional struct-attributes Nov 23, 2018
@wing328 wing328 added this to the 3.3.4 milestone Nov 28, 2018
@mrmstn
Copy link
Copy Markdown
Contributor Author

mrmstn commented Nov 28, 2018

I also added the nullable type to the musache file.

Based on the following swagger 2.0 defintion

definitions:
  Entity:
    type: object
    properties:
      requiredDate:
        type: string
        format: date
        x-nullable: false
      notRequiredDate:
        type: string
        format: date
        x-nullable: false
      nullableRequiredDate:
        type: string
        format: date
        x-nullable: true
      nullableNotRequiredDate:
        type: string
        format: date
        x-nullable: true
    required:
      - requiredDate
      - nullableRequiredDate

this PR will result to the following results

defmodule SwaggerPetstore.Model.Entity do
  @moduledoc """
  
  """

  @derive [Poison.Encoder]
  defstruct [
    :"requiredDate",
    :"notRequiredDate",
    :"nullableRequiredDate",
    :"nullableNotRequiredDate"
  ]

  @type t :: %__MODULE__{
    :"requiredDate" => Date.t,
    :"notRequiredDate" => Date.t | nil,
    :"nullableRequiredDate" => Date.t | nil,
    :"nullableNotRequiredDate" => Date.t | nil
  }
end

As you can see, even if you didn't allow a non required attribute to be null, the typespec will mark the attribute as possible null. This is given to the fact that elixir will initiate all attributes in a struct as null and therefore a "optional" attribute in a struct can only be null and not "not present"

Copy link
Copy Markdown
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wing328 wing328 merged commit 6930433 into OpenAPITools:master Nov 29, 2018
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
…Tools#1514)

* Typespec - allow null on optional parameters

* Run Petstore for elixir

* considers 'nullable' in model template, fixes 'isRequired'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants