-
Notifications
You must be signed in to change notification settings - Fork 405
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
feat: method validation step in OAS #6038
Conversation
|
||
export interface ProcessorInputOutput { | ||
yaml: string; | ||
errors?: ISpectralDiagnostic[]; | ||
eligibilityResult: ApexClassOASEligibleResponse; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found it convenient to pass in the context we need through this structure. But it seems that the data in the structure tends to be writable but the two objects I added are read-only. So open to opinions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mingxuanzhangsfdx you can alter the type with Readonly<T>
packages/salesforcedx-vscode-apex/src/oas/documentProcessorPipeline/oasProcessor.ts
Outdated
Show resolved
Hide resolved
@@ -22,7 +22,7 @@ export class OasValidationStep implements ProcessorStep { | |||
input.errors = result; | |||
}); | |||
|
|||
// Since this step doesn't perform convertions we return the input for future processing | |||
// Since this step doesn't perform conversions we return the input for future processing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any errors found should be added to the input.errors.
|
||
export interface ProcessorInputOutput { | ||
yaml: string; | ||
errors?: ISpectralDiagnostic[]; | ||
eligibilityResult: ApexClassOASEligibleResponse; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mingxuanzhangsfdx you can alter the type with Readonly<T>
@@ -58,7 +58,7 @@ describe('MetadataOrchestrator', () => { | |||
]; | |||
jest.spyOn(orchestrator, 'validateEligibility').mockResolvedValue(mockResponse); | |||
await expect(orchestrator.validateMetadata(editorStub.document.uri, true)).rejects.toThrow( | |||
'Method someMethod is not eligible for OpenAPI Document creation. It is not annotated with @AuraEnabled or has wrong access modifiers.' | |||
'Method someMethod is not eligible for OpenAPI Document creation. It is not annotated with an http annotator or has wrong access modifiers.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mingxuanzhangsfdx Thanks for catching the AuraEnabled message! I believe this message should be more general. Http annotations are the current ones, but I imagine AuraEnabled will eventually return. What do think about referring the user to current documentation regarding what makes an Apex method eligible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I just noticed that the sentence is only used when one method is selected for generation. Since it is not required by TDX, I guess we can leave it for now?
…eline/oasProcessor.ts Co-authored-by: peternhale <[email protected]>
What does this PR do?
This PR adds an extra step in the processor to validate the methods after OAS generation, including
What issues does this PR fix or reference?
@W-17654638@
Functionality Before
methods are not validated in the process
Functionality After
Validation of the methods is implemented.