NorfairKing / autodocodec

self(auto)- documenting encoders and decoders
MIT License
121 stars 20 forks source link

Incorrect openapi when using two `parseAlternative`s #10

Closed TheOddler closed 1 year ago

TheOddler commented 2 years ago

It looks like the openapi output is incorrect when using multiple parseAlternatives.

We have the following code:

data UserIDWithExternalIDs = UserIDWithExternalIDs
  { userIDWithExternalIDsUserID :: !PublicID,
    userIDWithExternalIDsExternalIDs :: !ExternalIDs
  }
  deriving stock (Eq, Show, Generic)
  deriving (ToJSON, FromJSON, OpenAPI.ToSchema) via (Autodocodec UserIDWithExternalIDs)

instance HasCodec UserIDWithExternalIDs where
  codec =
    object "UserIDWithExternalIDs" $
      UserIDWithExternalIDs
        <$> parseAlternative (requiredField "userId" "User ID (backwards compatible)") (requiredField "userID" "User ID") .= userIDWithExternalIDsUserID
        <*> parseAlternative (requiredField "externalIds" "All external ids associated with this user (backwards compatible)") (requiredField "externalIDs" "All external ids associated with this user") .= userIDWithExternalIDsExternalIDs

data ExternalIDs = ...

Which produces the following openapi definition:

"UserIDWithExternalIDs": {
    "anyOf": [
        {
            "required": [
                "externalIds"
            ],
            "type": "object",
            "properties": {
                "externalIds": {
                    "$ref": "#/components/schemas/ExternalIDs"
                }
            }
        },
        {
            "required": [
                "externalIDs"
            ],
            "type": "object",
            "properties": {
                "externalIDs": {
                    "$ref": "#/components/schemas/ExternalIDs"
                }
            }
        }
    ],
    "additionalProperties": true
},

This does not include the userId as a field.

When I remove one of the parseAlternatives:

instance HasCodec UserIDWithExternalIDs where
  codec =
    object "UserIDWithExternalIDs" $
      UserIDWithExternalIDs
        <$> requiredField "userId" "User ID (backwards compatible)" .= userIDWithExternalIDsUserID -- no more parseAlternative here
        <*> parseAlternative (requiredField "externalIds" "External IDs (backwards compatible)") (requiredField "externalIDs" "External IDs") .= userIDWithExternalIDsExternalIDs

The openapi definition does include the userId:

"UserIDWithExternalIDs": {
    "anyOf": [
        {
            "required": [
                "externalIds"
            ],
            "type": "object",
            "properties": {
                "externalIds": {
                    "$ref": "#/components/schemas/ExternalIDs"
                }
            }
        },
        {
            "required": [
                "externalIDs"
            ],
            "type": "object",
            "properties": {
                "externalIDs": {
                    "$ref": "#/components/schemas/ExternalIDs"
                }
            }
        }
    ],
    "required": [
        "userId"
    ],
    "additionalProperties": true,
    "type": "object",
    "properties": {
        "userId": {
            "type": "string",
            "description": "User ID (backwards compatible)\n..."
        }
    }
},
NorfairKing commented 2 years ago

That's a very interesting issue. Great digging! PR Welcome

NorfairKing commented 2 years ago

@TheOddler Is this issue still there in the latest release? I would appreciate a PR to fix this.

TheOddler commented 2 years ago

I'll have to check it out. Haven't had time/energy to look at it yet.

NorfairKing commented 1 year ago

PR welcome