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

[Enhancement] Improve oneOf schema handling #503

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

kahirokunn
Copy link
Contributor

@kahirokunn kahirokunn commented Oct 25, 2023

This commit introduces an enhancement to better handle the oneOf schema in OpenAPI definitions.

  • Added a new OpenAPI test fixture - oneOfMerge.yaml
  • Updated logic in the codegen script where oneOf schemas are encountered. The properties of the schema are now deeply merged to accommodate different variations.
  • Added a new test case to confirm the improved handling of oneOf schemas.

OpenAPI

openapi: 3.0.3
info:
  title: OneOfMerge
  version: 0.1.0
paths:
  /mix:
    get:
      description: stats
      parameters:
        - name: param1
          in: query
          schema:
            oneOf:
            - required:
              - c
              properties:
                a:
                  type: string
            - required:
              - b
              properties:
                b:
                  type: string
            required:
            - d
            properties:
              c:
                type: string
              d:
                enum:
                - enum1
                - enum2
                type: string
      responses:
        "200":
          description: ok

Before

/** * OneOfMerge * 0.1.0 * DO NOT MODIFY - This file has been generated using oazapfts. * See https://www.npmjs.com/package/oazapfts */ import * as Oazapfts
  from 'oazapfts/lib/runtime';
import * as QS from 'oazapfts/lib/runtime/query';
export const defaults: Oazapfts.RequestOpts = {baseUrl: '/'};
const oazapfts = Oazapfts.runtime (defaults);
export const servers = {};
/** * stats */ export function getMix (
  {param1}: {param1?: {a?: string} | {b: string}} = {},
  opts?: Oazapfts.RequestOpts
) {
  return oazapfts.fetchText (`/mix${QS.query (QS.explode ({param1}))}`, {
    ...opts,
  });
}

After

/** * OneOfMerge * 0.1.0 * DO NOT MODIFY - This file has been generated using oazapfts. * See https://www.npmjs.com/package/oazapfts */ import * as Oazapfts from "oazapfts/lib/runtime";
import * as QS from "oazapfts/lib/runtime/query";
export const defaults: Oazapfts.RequestOpts = { baseUrl: "/" };
const oazapfts = Oazapfts.runtime(defaults);
export const servers = {};
/** * stats */ export function getMix(
  {
    param1,
  }: {
    param1?:
      | { a?: string; c: string; d: "enum1" | "enum2" }
      | { b: string; c?: string; d: "enum1" | "enum2" };
  } = {},
  opts?: Oazapfts.RequestOpts
) {
  return oazapfts.fetchText(`/mix${QS.query(QS.explode({ param1 }))}`, {
    ...opts,
  });
}

@kahirokunn
Copy link
Contributor Author

@jonkoops Hello!
Can you give me a review?
Thx 🙏

This commit introduces an enhancement to better handle the oneOf schema in OpenAPI definitions.

- Added a new OpenAPI test fixture - oneOfMerge.yaml
- Updated logic in the codegen script where oneOf schemas are encountered. The properties of the schema are now deeply merged to accommodate different variations.
- Added a new test case to confirm the improved handling of oneOf schemas.

Signed-off-by: kahiro okina <kahiro.okina@craftsman-software.com>
@Xiphe
Copy link
Collaborator

Xiphe commented Nov 27, 2023

This looks wild 😂 is there open api documentation for this?

The code looks good but I would like to reference why we need this.

@kahirokunn
Copy link
Contributor Author

Hi @Xiphe ,

Thanks for your interest! 😄 Yes, the schema does look a bit complex at first glance, but it's structured according to the OpenAPI 3.0.3 specification. Specifically, the use of oneOf in our API's parameters is integral for allowing multiple types of inputs while ensuring that each input conforms to one of the pre-defined subschemas.

In this case, oneOf is used to validate that the parameter provided in the GET request matches exactly one of the specified subschemas. Each subschema requires different properties (c for the first, b for the second), and the operation ensures that a valid input must conform to one of these, but not both. Additionally, the required property outside the oneOf specifies that the property d is mandatory for all requests, adding an extra layer of validation.

This approach is quite useful for our API as it allows for more flexible and precise parameter handling, ensuring that the inputs are correctly structured and validated against our specific requirements.

For a more detailed understanding, I recommend looking at the OpenAPI 3.0.3 documentation, particularly the sections discussing schema composition keywords like oneOf. This will give you a clearer picture of why and how we're using this schema structure in our API. Here's a link to the relevant section: OpenAPI Specification 3.0.3 - Schema Composition.

Let me know if you need further clarification or have any other questions!

Best,
kahirokunn

@Xiphe Xiphe merged commit d97fc2e into oazapfts:main Nov 28, 2023
1 check passed
@Xiphe
Copy link
Collaborator

Xiphe commented Nov 28, 2023

Thanks for the clarification and for taking the time to contribute ❤️

Copy link

🎉 This PR is included in version 4.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants