elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.48k stars 8.04k forks source link

[OAS] Support recursive and ref in Zod OAS conventor #187335

Open maryam-saeidi opened 1 week ago

maryam-saeidi commented 1 week ago

Summary

While adding support for Zod, in the first iteration, we decided to skip supporting recursive in the first step and cover it in a separate issue.

During adding the test as follows:

const baseCategorySchema = z.object({
  name: z.string(),
});
type ZodRecursiveType = z.infer<typeof baseCategorySchema> & {
  subcategories: ZodRecursiveType[];
};

it('handles recursive schemas', () => {
      const recursiveSchema: z.ZodType<ZodRecursiveType> = baseCategorySchema.extend({
        subcategories: z.lazy(() => recursiveSchema.array()),
      });
      expect(
        generateOpenApiDocument(
          {
            routers: [
              createRouter({
                routes: [
                  {
                    isVersioned: false,
                    path: '/recursive',
                    method: 'get',
                    validationSchemas: {
                      request: {
                        body: recursiveSchema,
                      },
                      response: {
                        [200]: {
                          body: () => schema.string({ maxLength: 10, minLength: 1 }),
                        },
                      },
                    },
                    options: { tags: ['foo'] },
                    handler: jest.fn(),
                  },
                ],
              }),
            ],
            versionedRouters: [],
          },
          {
            title: 'test',
            baseUrl: 'https://test.oas',
            version: '99.99.99',
          }
        )
      ).toMatchSnapshot();
    });
  });

We also noticed the following warning: (comment)

@jloleysens pointed out in this comment that the snapshot should look something like this:

  "components": Object {
    "schemas": Object {
      "recursive": Object {
        "additionalProperties": false,
        "properties": Object {
          "name": Object {
            "type": "string",
          },
          "self": Object {
            "$ref": "#/components/schemas/recursive",
          },
        },
        "required": Array [
          "name",
          "self",
        ],
        "type": "object",
      },
    },    

Comment:

I think we will need to update $refStrategy to something other than none... Probably root? In packages/kbn-router-to-openapispec/src/oas_converter/zod/lib.ts

elasticmachine commented 1 week ago

Pinging @elastic/kibana-core (Team:Core)

jloleysens commented 1 week ago

Thanks for creating this issue @maryam-saeidi

FWIW teams are welcome to create PRs improving OAS support for Zod in Kibana :) I think once o11y starts using it more heavily there will be real use cases driving needs and I don't think we need/should block on Core. Is there a way we can make the code more open/accessible for teams to contribute to?

maryam-saeidi commented 1 week ago

FWIW teams are welcome to create PRs improving OAS support for Zod in Kibana :) I think once o11y starts using it more heavily there will be real use cases driving needs and I don't think we need/should block on Core. Is there a way we can make the code more open/accessible for teams to contribute to?

I agree. I created this ticket and added the Core team by default, but it can also be picked up by any observability team that needs recursive functionality. Do you mean assigning it to the Core team might discourage teams from picking it up, or do you mean not having a ticket until we have a real use case for it?

My main reason for creating this issue was not losing the context of our discussion.