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.

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

Jump to bottom

fix: Create one endpoint per path, not per method #123

Merged
merged 9 commits into from
Oct 16, 2024
Merged

fix: Create one endpoint per path, not per method #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
16dc8a5
Create one endpoint per path, not per method
andrii-balitskyi Oct 11, 2024
0bce914
ci: Generate code
seambot Oct 11, 2024
45a6264
Remove console log
andrii-balitskyi Oct 11, 2024
6818c2d
Format
andrii-balitskyi Oct 11, 2024
c6a690e
Fix getSemanticMethod
andrii-balitskyi Oct 11, 2024
8f9e78f
Fix semantic method priority order
andrii-balitskyi Oct 14, 2024
7a7bb3a
ci: Generate code
seambot Oct 14, 2024
63f310e
Merge branch 'main' of github.com:seamapi/blueprint into one-endpoint…
andrii-balitskyi Oct 14, 2024
3abefbc
ci: Generate code
seambot Oct 14, 2024
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
65 changes: 41 additions & 24 deletions src/lib/blueprint.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,8 @@ const createRoute = async (
throw new Error(`Could not resolve name for route at ${path}`)
}

const endpoint = await createEndpoint(path, pathItem, context)

return {
path: routePath,
name,
Expand All @@ -398,7 +400,7 @@ const createRoute = async (
isDraft: false,
}
: null,
endpoints: await createEndpoints(path, pathItem, context),
endpoints: endpoint != null ? [endpoint] : [],
subroutes: [],
isUndocumented: false,
isDeprecated: false,
Expand Down Expand Up @@ -446,34 +448,41 @@ const addNamespaceStatusToRoute = (
}
}

const createEndpoints = async (
const createEndpoint = async (
path: string,
pathItem: OpenapiPathItem,
context: Context,
): Promise<Endpoint[]> => {
): Promise<Endpoint | null> => {
const validMethods: Method[] = ['GET', 'POST', 'PUT', 'DELETE', 'PATCH']
const validOperations = Object.entries(pathItem).filter(
([method, operation]) =>
validMethods.includes(method.toUpperCase() as Method) &&
typeof operation === 'object' &&
operation !== null,
)
const supportedMethods = validOperations.map(
([method]) => method.toUpperCase() as Method,
)

return await Promise.all(
Object.entries(pathItem)
.filter(
([method, operation]) =>
validMethods.includes(method.toUpperCase() as Method) &&
typeof operation === 'object' &&
operation !== null,
)
.map(async ([method, operation]) => {
const uppercaseMethod = method.toUpperCase() as Method
return await createEndpoint(
[uppercaseMethod],
operation as OpenapiOperation,
path,
context,
)
}),
const validOperation = validOperations[0]
if (validOperation == null) {
// eslint-disable-next-line no-console
console.warn(`No valid operations found for ${path}`)

return null
}

const [_, operation] = validOperation

return await createEndpointFromOperation(
supportedMethods,
operation as OpenapiOperation,
path,
context,
)
}

const createEndpoint = async (
const createEndpointFromOperation = async (
methods: Method[],
operation: OpenapiOperation,
path: string,
Expand Down Expand Up @@ -505,6 +514,7 @@ const createEndpoint = async (
const draftMessage = parsedOperation['x-draft']

const request = createRequest(methods, operation, path)
const response = createResponse(operation, path)

const endpoint: Omit<Endpoint, 'codeSamples'> = {
title,
Expand All @@ -517,7 +527,7 @@ const createEndpoint = async (
undocumentedMessage,
isDraft,
draftMessage,
response: createResponse(operation, path),
response,
request,
}

Expand Down Expand Up @@ -908,8 +918,15 @@ export const getSemanticMethod = (methods: Method[]): Method => {
return methods[0]!
}

const priorityOrder: Method[] = ['PUT', 'PATCH', 'POST', 'GET', 'DELETE']
return methods.find((m) => priorityOrder.includes(m)) ?? 'POST'
Copy link
Contributor Author

@andrii-balitskyi andrii-balitskyi Oct 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This seems to have been implemented incorrectly because the priority was defined by the passed methods, not by the priorityOrder.
However, now the tests are failing. Am I understanding this incorrectly?
If I'm right that there's a bug here, is the priorityOrder correct so that I could adjust the tests?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This does looks like a mistake 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@razor-x I change the priority method in this commit 8f9e78f to fix the tests, does it look right?

const priorityOrder: Method[] = ['PUT', 'PATCH', 'GET', 'DELETE', 'POST']

for (const method of priorityOrder) {
if (methods.includes(method)) {
return method
}
}

return 'POST'
}

export const getPreferredMethod = (
Expand Down
Loading