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

[go-experimental] Ensure enum deserialization checks enum values #6315

Merged
merged 1 commit into from
May 18, 2020

Conversation

bkabrda
Copy link
Contributor

@bkabrda bkabrda commented May 15, 2020

This new unmarshalling method for enums ensures that unmarshalling fails if the value is not in the enumeration choices. As the enum type is just a type alias in go, technically any value of the base type can actually be assigned to it - this patch prevents that at least on unmarshalling.

This allows to properly deserialize oneOf members that have a common field that has specific distinct choices for each of the members. For example:

  • A and B are members of oneOf and both have a foo
  • For A, foo is an enum with allowed choices a, A
  • For B, foo is an enum with allowed choices b, B
  • The rest of the fields is potentially the same.

Having this new unmarshalling method ensures that entity with foo: b can't get deserialized as A, but only as B, hence the oneOf deserialization works properly.

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

@antihax (2017/11) @bvwells (2017/12) @grokify (2018/07) @kemokemo (2018/09) @bkabrda (2019/07)

import (
"fmt"
)

Copy link
Member

Choose a reason for hiding this comment

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

The change looks good to me. I'll try to consolidate the multiple import later with another PR.

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