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

Generation/Handling of bitwise/flagged enums #3237

Closed
2 tasks done
andrueastman opened this issue Aug 31, 2023 · 2 comments · Fixed by #3240
Closed
2 tasks done

Generation/Handling of bitwise/flagged enums #3237

andrueastman opened this issue Aug 31, 2023 · 2 comments · Fixed by #3240
Assignees
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. WIP
Milestone

Comments

@andrueastman
Copy link
Member

andrueastman commented Aug 31, 2023

Problem 1
OpenApi does not have the notion of bitwise/flagged enums with Kiota in a way supporting this notion via the IsFlags property in CodeEnum that is currently not set. To do this, we would need a signal from the OpenApi metadata to tell us the enum is flagged(bitwise)

Problem 2
Given that the bitwise enum can be technically thought of and represented as a collection of enums, serialization (and model representation) of the bitwise enum could arguably be done as an enum collection that would then be serialized as a json array (e.g. [“value1”,”value2”,”value3”]) however an alternative aproach would be to serialize as a csv (e.g. "value1, value2, value3”) as is done in Graph. From a broader perspective therefore, the fact the an enum is bitwise does not necessarily dictate the format of its serialization and therefore information about the serialization format would be needed to be passed down to the serializer for extensibility.

Looking at the graph metadata, we have both instances of single valued enums represented as collection properties (case 2) as well as bitwise enums represented as singular property(case 3). The case for bitwise enums referenced as collection properties (case 4) can be considered academic for now and not to be supported and labeled as metadata issues here.

  IsFlagged IsCollection OpenAPI representation  
1 False False Enum non collection property Already handled and serialized as “value1”
2 False True Enum collection property Already handled as json representation [“value1”,”value2”,”value3”]. (Collection of case 1)
3 True False Enum non collection property (needs metadata info) currently identical to case 1 in the specification Could be "value1,value2,value3” or "value1” if serialized as csv or [“value1”,”value2”,”value3”] if serialized as a json array.
4 True True Undesired: Flagged Enum collection property. Undesired. Theoretically would be ["value1", "value2,value3"] or [“value1”,”value2”,”value3”] or [”value2”](Collection of case 3)

Proposition

  1. Define an openApi extenstion (maybe x-ms-enum-flags?) with the properties
    1. Boolean property isFlags(to represent whether the enum is bitwise i.e problem 1) - default is false (same as extension not being present). true means that the enum is bitwise.
    2. String property style (to represent the serialization format i.e problem 2)- default is form (meaning csv). More alternatives can then be added later on and possibly borrowed from here.
  2. Implement extension on conversion library
  3. Implement extension on Kiota and use it to set the IsFlags on CodeEnum and create a new Style property
  4. Downstream languages(refiners/writer) would then use the information to represent the bitwise enum appropiately. This could potentially involve the language refiner translating the flagged enum property to enum collection and then set an appropiate serializer for handling the enum. Current examples
    • dotnet - Setting the flags attribute on Enums (Already implemented here)
    • java - using the EnumSet type (Already implemented here)
    • typescript - using spread operators/enum collections (Already implemented here)
    • Golang - generating enum models to have the parse and write function adapt to the flags (this PR)

Breaking Changes concerns
Passing over the style(serialization) format information to the serializers would either mean

  • Adding an extra style parameter to the getCollectionOfEnumValues/writeCollectionOfEnumValues/writeEnumValue/getEnumValue functions used by the serializers
  • Adding new WriteFlaggedEnum/GetFlaggedEnum that takes the style parameter.

Both would be breaking changes as they would introduce a new function to the existing serialization interfaces or change method signatures.
Therefore, for the purposes of avoiding breaking changes and unblocking current SDKs, the scope of this issue will be limited to NOT passing the style information to the serializers and the serializers will by default only support and serialize to csv.

A separate issue to support more serialization formats and styles will be created and then added to the Kiota v3 milestone.

Alternatives Considered

  1. Having the conversion library convert bitwise enums to enum collections.
    Has the downside of more information needed to specify the serialization format without making assumptions. Specific languages could also lose language experiences like bitwise operations on the enums which may be desired.

Todo

  • Create issue for conversion library
  • Create issue for breaking changes
@andrueastman andrueastman self-assigned this Aug 31, 2023
@baywet baywet added enhancement New feature or request generator Issues or improvements relater to generation capabilities. labels Aug 31, 2023
@baywet baywet added this to Kiota Aug 31, 2023
@github-project-automation github-project-automation bot moved this to Todo in Kiota Aug 31, 2023
@baywet baywet added this to the Kiota v1.6 milestone Aug 31, 2023
@baywet
Copy link
Member

baywet commented Aug 31, 2023

Thanks for the great recap of our conversation! I take it that you want to work on it since you've self assigned this issue? I'm happy to take it otherwise.
Also set the 1.6 milestone on this one, even though we're close, it feels the change is small enough that we can deliver it before then. Let me know if that doesn't match your workload.

@andrueastman
Copy link
Member Author

@baywet I'll take this one and resolve this asap if that's okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. WIP
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants