-
Notifications
You must be signed in to change notification settings - Fork 224
fix(workflow): use $sourceDescriptions for workflow references #2867
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
base: main
Are you sure you want to change the base?
Changes from all commits
af2e6ab
770b61f
ee91d74
d31dfd1
3e85a8c
69ac9c7
2b962d4
658bf13
ed1b5fb
d692b6c
b43a2b3
2658393
34791a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@redocly/respect-core': patch | ||
| --- | ||
|
|
||
| Added support for the Arazzo spec-compliant workflow reference form `$sourceDescriptions.<name>.<workflowId>`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,7 +143,7 @@ describe('getValueFromContext', () => { | |
| }); | ||
|
|
||
| // $sourceDescriptions.<name>.workflows.<workflowId> | ||
| it('should return workflow from $sourceDescriptions', () => { | ||
| it('should resolve the legacy `$sourceDescriptions.<name>.workflows.<workflowId>` form for backward compatibility', () => { | ||
| const ctx = { | ||
| $sourceDescriptions: { | ||
| test: { | ||
|
|
@@ -168,7 +168,7 @@ describe('getValueFromContext', () => { | |
| }); | ||
| }); | ||
|
|
||
| it('should return undefined from $sourceDescriptions if there is no such sourceDescription', () => { | ||
| it('should return undefined for the legacy form when there is no such sourceDescription', () => { | ||
| const ctx = { | ||
| $sourceDescriptions: { | ||
| test: { | ||
|
|
@@ -190,7 +190,7 @@ describe('getValueFromContext', () => { | |
| ).toEqual(undefined); | ||
| }); | ||
|
|
||
| it('should return undefined from $sourceDescriptions if there is no workflowId provided', () => { | ||
| it('should return undefined for the legacy form when there is no workflowId provided', () => { | ||
| const ctx = { | ||
| $sourceDescriptions: { | ||
| test: { | ||
|
|
@@ -208,6 +208,98 @@ describe('getValueFromContext', () => { | |
| ).toEqual(undefined); | ||
| }); | ||
|
|
||
| it('should return workflow from $sourceDescriptions using the spec form without `workflows` segment', () => { | ||
| const ctx = { | ||
| $sourceDescriptions: { | ||
| test: { | ||
| workflows: [ | ||
| { | ||
| workflowId: 'workflowTestId', | ||
| steps: [], | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| } as unknown as TestContext; | ||
| expect( | ||
| getValueFromContext({ | ||
| value: '$sourceDescriptions.test.workflowTestId', | ||
| ctx, | ||
| logger, | ||
| }) | ||
| ).toEqual({ | ||
| workflowId: 'workflowTestId', | ||
| steps: [], | ||
| }); | ||
| }); | ||
|
|
||
| it('should fall back to field access for the spec form when no workflow matches', () => { | ||
| const ctx = { | ||
| $sourceDescriptions: { | ||
| test: { | ||
| url: './test.yaml', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can there actually be a url on the same level as workflows? Please check create-test-context.ts:44-63.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like synthetic context structure due to not strict typing. |
||
| workflows: [ | ||
| { | ||
| workflowId: 'workflowTestId', | ||
| steps: [], | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| } as unknown as TestContext; | ||
| expect( | ||
| getValueFromContext({ | ||
| value: '$sourceDescriptions.test.url', | ||
| ctx, | ||
| logger, | ||
| }) | ||
| ).toEqual('./test.yaml'); | ||
| }); | ||
|
|
||
| it('should return undefined for the spec form when the source description exists but has no matching workflow or field', () => { | ||
| const ctx = { | ||
| $sourceDescriptions: { | ||
| test: { | ||
| workflows: [ | ||
| { | ||
| workflowId: 'workflowTestId', | ||
| steps: [], | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| } as unknown as TestContext; | ||
| expect( | ||
| getValueFromContext({ | ||
| value: '$sourceDescriptions.test.notExistingWorkflow', | ||
| ctx, | ||
| logger, | ||
| }) | ||
| ).toEqual(undefined); | ||
| }); | ||
|
|
||
| it('should return undefined for the spec form when the source description does not exist', () => { | ||
| const ctx = { | ||
| $sourceDescriptions: { | ||
| test: { | ||
| workflows: [ | ||
| { | ||
| workflowId: 'workflowTestId', | ||
| steps: [], | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| } as unknown as TestContext; | ||
| expect( | ||
| getValueFromContext({ | ||
| value: '$sourceDescriptions.notExistingName.someWorkflow', | ||
| ctx, | ||
| logger, | ||
| }) | ||
| ).toEqual(undefined); | ||
| }); | ||
|
|
||
| it('should return undefined if getFakeData had error resolving value', () => { | ||
| const ctx = { | ||
| $sourceDescriptions: { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -316,26 +316,33 @@ const resolveValue = ( | |||||||
| return path.slice(7, -2); | ||||||||
| } | ||||||||
|
|
||||||||
| // $sourceDescriptions.<name>.<workflowId> | ||||||||
| // $sourceDescriptions.<name>.workflows.<workflowId> | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| if (path.startsWith('$sourceDescriptions.') && path.includes('.workflows.')) { | ||||||||
| if (path.startsWith('$sourceDescriptions.')) { | ||||||||
| const parts = path.split('.'); | ||||||||
|
|
||||||||
| const sourceDescriptionName = parts[1]; | ||||||||
| const workflowId = parts[3]; | ||||||||
|
|
||||||||
| if (!sourceDescriptionName || !workflowId) { | ||||||||
| return undefined; | ||||||||
| const isLegacyForm = parts.length === 4 && parts[2] === 'workflows'; | ||||||||
| const isSpecForm = parts.length === 3; | ||||||||
| const workflowId = isLegacyForm ? parts[3] : isSpecForm ? parts[2] : undefined; | ||||||||
|
harshit078 marked this conversation as resolved.
|
||||||||
|
|
||||||||
| if (sourceDescriptionName && workflowId) { | ||||||||
| const sourceDescriptions = getFrom(ctx)('$sourceDescriptions'); | ||||||||
| const sourceDescription = sourceDescriptions?.[sourceDescriptionName]; | ||||||||
| const workflow = sourceDescription?.workflows?.find( | ||||||||
| (workflow: Workflow) => workflow.workflowId === workflowId | ||||||||
| ); | ||||||||
|
|
||||||||
| if (workflow) { | ||||||||
| return workflow; | ||||||||
| } | ||||||||
| if (isSpecForm) { | ||||||||
| return sourceDescription?.[workflowId]; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| const sourceDescriptions = getFrom(ctx)('$sourceDescriptions'); | ||||||||
|
|
||||||||
| if (!sourceDescriptions[sourceDescriptionName]) { | ||||||||
| if (isLegacyForm) { | ||||||||
|
cursor[bot] marked this conversation as resolved.
|
||||||||
| return undefined; | ||||||||
| } | ||||||||
|
DmitryAnansky marked this conversation as resolved.
|
||||||||
|
|
||||||||
| return sourceDescriptions[sourceDescriptionName].workflows.find( | ||||||||
| (workflow: Workflow) => workflow.workflowId === workflowId | ||||||||
| ); | ||||||||
| } | ||||||||
|
|
||||||||
| if (path && path.trim().startsWith('faker.')) { | ||||||||
|
|
||||||||
Uh oh!
There was an error while loading. Please reload this page.