Skip to content

Sort routes by path before writing the output JSON in openapi-generator #925

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Sort routes by path before writing the output JSON in openapi-generator #925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/openapi-generator/src/openapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,21 @@ export function convertRoutesToOpenAPI(
{} as Record<string, OpenAPIV3.SchemaObject | OpenAPIV3.ReferenceObject>,
);

const sortedPaths = Object.keys(paths)
.sort((a, b) => a.localeCompare(b))
.reduce(
(acc, key) => {
acc[key] = paths[key]!;
Copy link
Contributor Author

@bitgopatmcl bitgopatmcl Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works because insertion order is preserved for string keys

Copy link
Contributor

@ericcrosson-bitgo ericcrosson-bitgo Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yes I see this in the comment you linked:

entries come in the order in which the properties were added

Followed immediately by:

Relying on object property ordering in JavaScript is a really bad idea, because it makes code extremely fragile. If you need an ordering, create an array with the properties in it in the order that works for your application.

😅

How do you feel about using a Map instead of an object? The overview of Map on MDN reads:

The Map object holds key-value pairs and remembers the original insertion order of the keys.

Copy link
Contributor Author

@bitgopatmcl bitgopatmcl Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try and see how it serializes.

Copy link
Contributor Author

@bitgopatmcl bitgopatmcl Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No luck:

YAMLException: unacceptable kind of an object to dump [object Map]

Copy link
Contributor Author

@bitgopatmcl bitgopatmcl Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

js-yaml has a sortKeys option but that sorts all the keys and not just the ones under paths.

Copy link
Contributor

@ericcrosson-bitgo ericcrosson-bitgo Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All righty, well, so long as it's tested and deterministic!

return acc;
},
{} as Record<string, OpenAPIV3.PathItemObject>,
);

return {
openapi: '3.0.3',
info,
...(servers.length > 0 ? { servers } : {}),
paths,
paths: sortedPaths,
components: {
schemas: openapiSchemas,
},
Expand Down
146 changes: 141 additions & 5 deletions packages/openapi-generator/test/openapi/base.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,8 @@ export const route = h.httpRoute({
method: 'GET',
request: h.httpRequest({
query: {
/**
* This is a foo description.
/**
* This is a foo description.
* @example abc
* @pattern ^[a-z]+$
*/
Expand Down Expand Up @@ -642,8 +642,8 @@ export const route = h.httpRoute({
method: 'GET',
request: h.httpRequest({
query: {
/**
* This is a foo description.
/**
* This is a foo description.
* @example abc
* @pattern ^[a-z]+$
*/
Expand Down Expand Up @@ -714,4 +714,140 @@ testCase('route with array union of null and undefined', ROUTE_WITH_ARRAY_UNION_
components: {
schemas: {}
}
});
});

const MULTIPLE_ROUTES = `
import * as t from 'io-ts';
import * as h from '@api-ts/io-ts-http';

// Purposefully out of order to test sorting
export const route1 = h.httpRoute({
path: '/foo',
method: 'GET',
request: h.httpRequest({
query: {
foo: t.string,
},
}),
response: {
200: t.string
},
});

export const route2 = h.httpRoute({
path: '/bar',
method: 'GET',
request: h.httpRequest({
query: {
bar: t.string,
},
}),
response: {
200: t.string
},
});

export const route3 = h.httpRoute({
path: '/baz',
method: 'GET',
request: h.httpRequest({
query: {
baz: t.string,
},
}),
response: {
200: t.string
},
});
`;

testCase('multiple routes', MULTIPLE_ROUTES, {
openapi: '3.0.3',
info: {
title: 'Test',
version: '1.0.0',
},
paths: {
'/bar': {
get: {
parameters: [
{
in: 'query',
name: 'bar',
required: true,
schema: {
type: 'string',
},
},
],
responses: {
200: {
description: 'OK',
content: {
'application/json': {
schema: {
type: 'string',
},
},
},
},
},
},
},
'/baz': {
get: {
parameters: [
{
in: 'query',
name: 'baz',
required: true,
schema: {
type: 'string',
},
},
],
responses: {
200: {
description: 'OK',
content: {
'application/json': {
schema: {
type: 'string',
},
},
},
},
},
},
},
'/foo': {
get: {
parameters: [
{
in: 'query',
name: 'foo',
required: true,
schema: {
type: 'string',
},
},
],
responses: {
200: {
description: 'OK',
content: {
'application/json': {
schema: {
type: 'string',
},
},
},
},
},
},
},
},
components: {
schemas: {},
},
});
Loading