api icon indicating copy to clipboard operation
api copied to clipboard

[FEATURE]: `allOf` instead of `oneOf` where applicable

Open stnokott opened this issue 1 year ago • 0 comments

(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 avatar May 12 '24 20:05 stnokott