Add support for 'x-enum-values' extension as used by Bungie

[AlwinGarside]

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 adds support for the 'x-enum-values' extensions as proposed by the Bungie.net API spec. See: https://github.com/Bungie-net/api#extension-properties-on-openapi-specs...

[jmini]

We have introduced x-enum-varnames to control the name associated with the enum value: #893

In my opinion this extension make more sense, because the enum property needs to conform the type defined by the schema. The additional extension x-enum-varnames is about providing a String for the corresponding value.

weather:
type: objectproperties:
type:
type: integerformat: int32enum:
- 1
- 2
- 3x-enum-varnames:
- Sunny
- Cloudy
- Rainy

[AlwinGarside]

We have introduced x-enum-varnames to control the name associated with the enum value: #893

In my opinion this extension make more sense, because the enum property needs to conform the type defined by the schema. The additional extension x-enum-varnames is about providing a String for the corresponding value.

weather:
type: objectproperties:
type:
type: integerformat: int32enum:
- 1
- 2
- 3x-enum-varnames:
- Sunny
- Cloudy
- Rainy

@jmini Both are extensions that solve the same problem.

Unfortunately Bungie will probably never implement the x-enum-varnames extension in their OpenAPI spec. Since OpenAPI Generator is designed for consumers of OpenAPI specs rather than the writers of those specs, I think it is good to support additional custom vendor extensions, even if they are not officially supported. Especially considering it's a small change.

[AlwinGarside]

@wing328 Could you also take a look-see?

[jmini]

Can you post here a small example how the YAML or JSON looks like, so that I can test it?
x-enum-values is supposed to be a List<Map<String, String>>

[AlwinGarside]

Can you post here a small example how the YAML or JSON looks like, so that I can test it?
x-enum-values is supposed to be a List<Map<String, String>>

@jmini This is from the Bungie OpenAPI spec:

"x-enum-values": [
{
"numericValue": "1",
"identifier": "ReadBasicUserProfile",
"description": "Read basic user profile information such as the user's handle, avatar icon, etc."
},
{
"numericValue": "2",
"identifier": "ReadGroups",
"description": "Read Group/Clan Forums, Wall, and Members for groups and clans that the user has joined."
},
{
"numericValue": "4",
"identifier": "WriteGroups",
"description": "Write Group/Clan Forums, Wall, and Members for groups and clans that the user has joined."
},
{
"numericValue": "8",
"identifier": "AdminGroups",
"description": "Administer Group/Clan Forums, Wall, and Members for groups and clans that the user is a founder or an administrator."
},
{
"numericValue": "16",
"identifier": "BnetWrite",
"description": "Create new groups, clans, and forum posts."
},
{
"numericValue": "32",
"identifier": "MoveEquipDestinyItems",
"description": "Move or equip Destiny items"
},
{
"numericValue": "64",
"identifier": "ReadDestinyInventoryAndVault",
"description": "Read Destiny 1 Inventory and Vault contents. For Destiny 2, this scope is needed to read anything regarded as private. This is the only scope a Destiny 2 app needs for read operations against Destiny 2 data such as inventory, vault, currency, vendors, milestones, progression, etc."
},
{
"numericValue": "128",
"identifier": "ReadUserData",
"description": "Read user data such as who they are web notifications, clan/group memberships, recent activity, muted users."
},
{
"numericValue": "256",
"identifier": "EditUserData",
"description": "Edit user data such as preferred language, status, motto, avatar selection and theme."
},
{
"numericValue": "512",
"identifier": "ReadDestinyVendorsAndAdvisors",
"description": "Access vendor and advisor data specific to a user. OBSOLETE. This scope is only used on the Destiny 1 API."
},
{
"numericValue": "1024",
"identifier": "ReadAndApplyTokens",
"description": "Read offer history and claim and apply tokens for the user."
},
{
"numericValue": "2048",
"identifier": "AdvancedWriteActions",
"description": "Can perform actions that will result in a prompt to the user via the Destiny app."
}
]
}}

https://raw.githubusercontent.com/Bungie-net/api/master/openapi.json

[jmini]

@Yogarine thank you a lot, I will try to have a look.

---

As discussed earlier: using the regular enum and the 2 extensions x-enum-varnames and x-enum-descriptions (#1693) is the recommend way because if you do not understand the extension, then the Spec still make sense.

I agree with your comment that we can also support this x-enum-values.

[jmini]

What I do not get:

Is the idea that both enum and x-enum-values are set in the Schema?
Is this numericValue only for schema of type: integer? How is this support to work with String type for example?

[AlwinGarside]

What I do not get:

Is the idea that both enum and x-enum-values are set in the Schema?
Is this numericValue only for schema of type: integer? How is this support to work with String type for example?

@jmini You would always need to declare enum in the Schema, to remain compatible with the OpenAPI spec.

The x-enum-values extension only serves to give additional information about the enum values. This extension is only intended to be used for numeric arrays.

This is how @vthornheart-bng explains it in the Bungie.net API README:

x-enum-values = I really hate that the OpenAPI spec doesn't have a way to tell you both the identifier and the numeric value of an enum, much less documentation for enum values, so I added this extension as a way to return that info. It'll give you way more than the basically useless base "enum" property.

[jmini]

I think there is some consent around the fact that enum as defined in OpenAPI v3 is not enough: What Bungie.net API has proposed is close too what is described in OAI/OpenAPI-Specification#681 (comment). Unless that you need to define the value twice:

"Weather":{
"type":"object",
"properties":{
"type":{
"type":"integer",
"format":"int32",
"enum":[
1,
2,
3
],
"x-enum-values":[
{
"numericValue":"1",
"identifier":"Sunny",
"description":"Blue sky"
},
{
"numericValue":"2",
"identifier":"Cloudy",
"description":"Slightly overcast"
},
{
"numericValue":"3",
"identifier":"Rainy",
"description":"Take an umbrella with you"
}
]
}
}
}

---

When we have solved #1693 we will also have support for descriptions:

"Weather" : {
"type" : "object",
"properties" : {
"type" : {
"type" : "integer",
"format" : "int32",
"enum" : [ 1, 2, 3 ],
"x-enum-descriptions" : [ "Blue sky", "Slightly overcast", "Take an umbrella with you" ],
"x-enum-varnames" : [ "Sunny", "Cloudy", "Rainy" ]
}
}
}

With the advantage that you do not not have to redeclare that enum values twice (in the enum property for OpenAPI compliancy and in the x-enum-values as defined by Bungie.net)

---

Of course as you have suggested earlier, we can decide to support both approaches.
This is a decision that the @OpenAPITools/generator-core-team can discuss.

My opinion is that supporting both constructs is OK with following additions for this PR:

• support for descriptions (probably [core] Support for "x-enum-descriptions"  #1752 need to be merged first)
• handling the case where a value is present in x-enum-values and not in enum (maybe log a warning)
• add some unit tests to catch regressions

What do you think?

[AlwinGarside]

@jmini

What do you think?

Sounds like a good plan.

[jimschubert]

handling the case where a value is present in x-enum-values and not in enum (maybe log a warning)

I think this should throw an error on spec validation by default. We allow for skipping validation, in which case I think a warning should be logged. But, we have no way to know whether the user considers the x-enum-values or the enum array as the source of truth. Most likely, an imbalance between the two is a programming or spec authoring error.

[FearlessHyena]

Would really like to have this too. Any plans when this will get released?