fix: adds missing Query and x-metadata to /components/schemas

[NickUfer]

Description

Fixes #16523

Related to #16485

Type of Change

• Bugfix
• Improvement
• New Feature
• Refactor / codestyle updates
• Other, please describe:

Requirements Checklist

• New / updated tests are included
• All tests are passing locally
• Performed a self-review of the submitted code

If adding a new feature:

• Documentation was added/updated. PR:

Spec now includes Query and x-metadata. Swagger editor does not throw errors anymore. Version is 9.21.0 because I just copied the compiled JS file into the podman container as I don't want to fiddle with an extra setup

[cf-ts]

Hello there, I just corrected the yaml files of the base api in my PR and did not change any code of the dynamic spec file generator. I did not see that within the new location (components.schemas) "Query" and "x-metadata" won't be copied into the final file, sorry for that! (see: #16485)

My Problem with this solution here is that it is quite static... So if now the base api would change in a way where a new model is introduced which again is not part of a collection but used at some paths, it must be added here hard coded again. So I'm wondering if it would be better to dynamically add all models to the schema section which don't have a "x-collection" record!? I can't really tell because I'm new to directus... My approach would be something like this: (see: https://github.com/cf-ts/directus/tree/fix-bug-in-oas-gen)

specifications.ts line 324

	components.schemas = {};
	// copy schemas without a collection
	if (openapi.components && openapi.components.schemas) {
		for (const schemaName in openapi.components.schemas) {
			const schema: any = openapi.components!.schemas![schemaName];
			if (!schema['x-collection']) {
				components.schemas[schemaName] = schema;
			}
		}
	}

[NickUfer]

@cf-ts your proposal sounds better, I will add that 👍🏼

And about that:

Hello there, I just corrected the yaml files of the base api in my PR and did not change any code of the dynamic spec file generator. I did not see that within the new location (components.schemas) "Query" and "x-metadata" won't be copied into the final file, sorry for that! (see: #16485)

No worries 😄

[NickUfer]

Okay, I just tried your solution and it introduces some side effects, it adds too much and adds undefined descriptions, however that happens https://github.com/NickUfer/directus/actions/runs/3515780729/jobs/5891478462

So I would stick for now with the static solution until a maintainer decides what to do. And if another schema is added it would be required to add it to the test I edited in my PR anyways, so you have to touch it either way

[cf-ts]

Agree, I don't know where the undefined description comes from too and anyways one schema gets produced which isn't used at all... So it seems that dynamically generating seems to be more complex and someone with deeper knowledge of the generator should look into it! For us it's important that it works, so thank you very much @NickUfer :)

[NickUfer]

Over night I got another idea how its maybe a mix of your and my solution. We define which schemas we want in a string array and search and add them to components.schemas. This way it is only required to add a string rather than copy pasting the same code again

…

On November 22, 2022 7:33:24 AM GMT+01:00, cf-ts ***@***.***> wrote: Agree, I don't know too where the undefined description comes from and anyways one schema gets produced which isn't used at all... So it seems that dynamically generating seems to be more complex and someone with deeper knowledge of the generator should look into it! For us it's important that it works, so thank you very much @NickUfer :) -- Reply to this email directly or view it on GitHub: #16524 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[NickUfer]

directus/api/src/services/specifications.ts

Lines 324 to 335 in 1dc7ce4

	components.schemas = {};
	// Always includes the schemas with these names
	const includeSchemas = ['Query', 'x-metadata'];
	if (openapi.components?.schemas !== null) {
	for (const schemaName of includeSchemas) {
	if (openapi.components!.schemas![schemaName] !== null) {
	components.schemas[schemaName] = cloneDeep(openapi.components!.schemas![schemaName]);
	}
	}
	}

[cf-ts]

I think this one is a good idea... 👍
Maybe the maintainers would like to define that List as a const or as a default config setting somewhere and then this can be changed really easy and even a user could override it if needed...
I hope it will get merged soon, then the OAS client generation would finally work again! :) 🥇 (I think the automated client generation is a key feature if you use TypeScript, otherwise we would have to write and maintain the typings manually...)

[br41nslug]

Looking good and solves the errors thrown by swagger 😄

[NickUfer]

@rijkvanzanten or whoever merges in the end, can this be merged already please? I don't want to resolve conflicts because something else was changed

[br41nslug]

I don't want to resolve conflicts because something else was changed

@NickUfer .Always make sure maintainers have edit right on the PR branch so we can resolve these conflicts when they come up.