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

[FEATURE]: allOf instead of oneOf where applicable #103

Open
stnokott opened this issue May 12, 2024 · 0 comments
Open

[FEATURE]: allOf instead of oneOf where applicable #103

stnokott opened this issue May 12, 2024 · 0 comments
Labels
feature request This issue requests a feature that doesn't exist (yet) mapping This issue or it's solution is related to how information is mapped PR ready This issue is approved and ready to be worked on.
Milestone

Comments

@stnokott
Copy link
Contributor

(copy-paste from #102)

The OpenAPI spec currently contains oneOf lists for many schema object fields.

In most cases, these fields are mandatory. See for example PlanetInfo.position:

"PlanetInfo": {
  "type": "object",
  "description": "Represents information of a planet from the 'WarInfo' endpoint returned by ArrowHead's API.",
  "additionalProperties": false,
  "properties": {
    "index": {
      "type": "integer",
      "description": "The numerical identifier for this planet, used as reference by other properties throughout the API (like Waypoints).",
      "format": "int32"
    },
    "settingsHash": {
      "type": "integer",
      "description": "Purpose unknown at this time.",
      "format": "int64"
    },
    "position": {
      "description": "A set of X/Y coordinates specifying the position of this planet on the galaxy map.",
      "oneOf": [
        {
          "$ref": "#/components/schemas/PlanetCoordinates"  // <- here
        }
      ]
    },
    // ...

Now, from a OpenAPI perspective, this is perfectly fine.

The problem I have is the following: I currently use a client code generator in my project which reads through your OpenAPI spec and produces client code for the requests in the spec.
It also produces model structs for each response schema (analogue to your server-side models).

This is where the problem with oneOf occurs: all code generators I tested thus far handle oneOf as an ambiguous type for which the actual type is unknown at runtime. This results in the generator producing intermediate models with multiple types and thus requiring additional client-side steps before the actual model instance is available (resulting in a lot of boilerplate code).


Sample client code (Go)
// MustPlanetPosition implements a converter for a planet position (i.e. coordinate).
func MustPlanetPosition(source *api.Planet_Position) ([]float64, error) {
	if source == nil {
		return nil, errors.New("Planet position is nil")
	}
	parsed, err := source.AsPosition() // convert ambigous type into actual type
	if err != nil {
		return nil, err
	}
	if parsed.X == nil || parsed.Y == nil {
		return nil, errors.New("Planet position coordinates are nil")
	}
	return []float64{*parsed.X, *parsed.Y}, nil
}

This method receives the immediate (generated) API response struct, but needs to perform additional processing in AsPosition to get the actual (usable) type.


While I can see the usecase of this for actual ambigous types such as your recently-added localized strings, the example above and almost all other occurences of oneOf are different: they occur in places where only one model is possible (i.e. the oneOf field only has one item).

This is where my request comes in: is it possible to switch to allOf for these cases? Semantically, this shouldn't make a difference for API specification.
It does make a difference for client-side processing though, especially when talking about generated code.

I am aware that the OpenAPI specs are automatically generated from your C# source code, but perhaps there is a way of annotating such fields so that allOf instead of oneOf is generated?

Thanks for reading this far, I hope this wall of text was half understandable.

And just to be clear: it works the way it currently is, it just produces a lot of boilerplate code on my end, so I understand if this problem is too niche and thus not worth the effort.
Please do let me know in that case and I'll happily settle with the current implementation.

@stnokott stnokott added the feature request This issue requests a feature that doesn't exist (yet) label May 12, 2024
@dealloc dealloc added PR ready This issue is approved and ready to be worked on. mapping This issue or it's solution is related to how information is mapped labels May 12, 2024
@dealloc dealloc added this to the V1 milestone May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request This issue requests a feature that doesn't exist (yet) mapping This issue or it's solution is related to how information is mapped PR ready This issue is approved and ready to be worked on.
Projects
None yet
Development

No branches or pull requests

2 participants