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

fix: do not override existing labels of underlying schemas in alternatives #3001

Conversation

hajekjiri
Copy link

When using labels on underlying schemas of labeled Joi.alternatives, the Joi.alternatives' label overwrites the labels of the underlying schemas. I think this is a bug and therefore propose the changes in this PR.

Based on reading existing tests, I assume passing labels from Joi.alternatives to underlying schemas is wanted, though I do not believe this should be the case when the underlying schemas already have their own labels. The fact that I am not breaking any existing tests with these changes further reinforces my belief that this may be a bug. I only added a few new tests to cover the behavior introduced in my changes.

Background:

In my project, I am using joi-to-swagger to convert Joi schemas to OpenAPI-compatible schemas. In case of Joi.alternatives, this leads to anyof or oneOf (meaning this property can have any/one of the following schemas) , where the schema labels are useful to be able to differentiate the specific alternatives at first glance instead of having a default generic label for each one (object, string, number, ...). When I use nested Joi.alternatives, the nested alternatives all take the parent's label, which results in me being unable to tell the difference at first glance when reading the generated OpenAPI documentation (see screenshots below).

Take the following example:

const Joi = require('./lib/index.js');

const valueSchema = Joi.object().keys({
  value: Joi.string().required(),
});

const notFoundErrorSchema = Joi.object().keys({
  code: Joi.string().required(),
  resource: Joi.string().required(),
});

const forbiddenErrorSchema = Joi.object().keys({
  code: Joi.string().required(),
  user: Joi.string().required(),
});

const errorSchema = Joi.alternatives().try(
  notFoundErrorSchema.label('NOT FOUND ERROR'),
  forbiddenErrorSchema.label('FORBIDDEN ERROR'),
);

const responseSchema = Joi.alternatives().try(
  valueSchema.label('VALUE'),
  errorSchema.label('ERROR'),
);

console.log(JSON.stringify(responseSchema.describe(), null, 2));
Current output
{
  "type": "alternatives",
  "matches": [
    {
      "schema": {
        "type": "object",
        "flags": {
          "label": "VALUE"
        },
        "keys": {
          "value": {
            "type": "string",
            "flags": {
              "presence": "required"
            }
          }
        }
      }
    },
    {
      "schema": {
        "type": "alternatives",
        "flags": {
          "label": "ERROR"
        },
        "matches": [
          {
            "schema": {
              "type": "object",
              "flags": {
                "label": "ERROR"  // <----------------------------------------
              },
              "keys": {
                "code": {
                  "type": "string",
                  "flags": {
                    "presence": "required"
                  }
                },
                "resource": {
                  "type": "string",
                  "flags": {
                    "presence": "required"
                  }
                }
              }
            }
          },
          {
            "schema": {
              "type": "object",
              "flags": {
                "label": "ERROR"  // <----------------------------------------
              },
              "keys": {
                "code": {
                  "type": "string",
                  "flags": {
                    "presence": "required"
                  }
                },
                "user": {
                  "type": "string",
                  "flags": {
                    "presence": "required"
                  }
                }
              }
            }
          }
        ]
      }
    }
  ]
}
Expected output
{
  "type": "alternatives",
  "matches": [
    {
      "schema": {
        "type": "object",
        "flags": {
          "label": "VALUE"
        },
        "keys": {
          "value": {
            "type": "string",
            "flags": {
              "presence": "required"
            }
          }
        }
      }
    },
    {
      "schema": {
        "type": "alternatives",
        "flags": {
          "label": "ERROR"
        },
        "matches": [
          {
            "schema": {
              "type": "object",
              "flags": {
                "label": "NOT FOUND ERROR"  // <----------------------------------------
              },
              "keys": {
                "code": {
                  "type": "string",
                  "flags": {
                    "presence": "required"
                  }
                },
                "resource": {
                  "type": "string",
                  "flags": {
                    "presence": "required"
                  }
                }
              }
            }
          },
          {
            "schema": {
              "type": "object",
              "flags": {
                "label": "FORBIDDEN ERROR"  // <----------------------------------------
              },
              "keys": {
                "code": {
                  "type": "string",
                  "flags": {
                    "presence": "required"
                  }
                },
                "user": {
                  "type": "string",
                  "flags": {
                    "presence": "required"
                  }
                }
              }
            }
          }
        ]
      }
    }
  ]
}
Resulting OpenAPI documentation
Current behavior

image

Expected behavior

image

@Marsup Marsup added this to the 17.11.1 milestone Jan 15, 2024
@Marsup Marsup self-assigned this Jan 15, 2024
@Marsup Marsup added the bug Bug or defect label Jan 15, 2024
@Marsup Marsup force-pushed the do-not-override-existing-labels-of-underlying-schemas-in-alternatives branch from ba603c2 to 3113b72 Compare January 15, 2024 11:28
@Marsup
Copy link
Collaborator

Marsup commented Jan 15, 2024

I agree that it's probably what anyone would expect. Thanks for the PR!

@Marsup Marsup merged commit 3fc290a into hapijs:master Jan 15, 2024
0 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug or defect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants