Skip to content

Commit

Permalink
feat: method validation step in OAS (#6038)
Browse files Browse the repository at this point in the history
* feat: method validation step

* feat: display errors in problem tab from method validation

* fix: test

* chore: set default formatter in vscode and revert format changes

* Update packages/salesforcedx-vscode-apex/src/oas/documentProcessorPipeline/oasProcessor.ts

Co-authored-by: peternhale <[email protected]>

* chore: apply suggestions

* chore: fix merging errors

* chore: refactor errors in processor

* chore: fix

---------

Co-authored-by: peternhale <[email protected]>
  • Loading branch information
mingxuanzhangsfdx and peternhale authored Jan 30, 2025
1 parent 0fd3472 commit 3cc1b89
Show file tree
Hide file tree
Showing 10 changed files with 126 additions and 27 deletions.
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"files.insertFinalNewline": true,
"files.trimFinalNewlines": true,
"editor.formatOnSave": true,
"editor.defaultFormatter": "esbenp.prettier-vscode",
"eslint.enable": true,
"eslint.run": "onSave",
"editor.codeActionsOnSave": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,11 +438,11 @@ describe('Interactive debugger adapter - unit', () => {
});

afterEach(() => {
sessionStartSpy.restore();
sessionConnectedSpy.restore();
streamingSubscribeSpy.restore();
breakpointHasLineNumberMappingSpy.restore();
orgInfoSpy.restore();
sessionStartSpy?.restore();
sessionConnectedSpy?.restore();
streamingSubscribeSpy?.restore();
breakpointHasLineNumberMappingSpy?.restore();
orgInfoSpy?.restore();
});

it('Should save proxy settings', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ import { workspaceContext } from '../context';
import { nls } from '../messages';
import { OasProcessor } from '../oas/documentProcessorPipeline/oasProcessor';
import { BidRule, PromptGenerationOrchestrator } from '../oas/promptGenerationOrchestrator';
import { ApexClassOASGatherContextResponse, ApexOASInfo, ExternalServiceOperation } from '../oas/schemas';
import {
ApexClassOASEligibleResponse,
ApexClassOASGatherContextResponse,
ApexOASInfo,
ExternalServiceOperation
} from '../oas/schemas';
import { getTelemetryService } from '../telemetry/telemetry';
import { MetadataOrchestrator } from './metadataOrchestrator';
export class ApexActionController {
Expand Down Expand Up @@ -83,7 +88,7 @@ export class ApexActionController {
);

// Step 7: Process the OAS document
const processedOasDoc = await this.processOasDocument(openApiDocument, context);
const processedOasDoc = await this.processOasDocument(openApiDocument, context, eligibilityResult);

// Step 8: Write OpenAPI Document to File
progress.report({ message: nls.localize('write_openapi_document') });
Expand Down Expand Up @@ -128,10 +133,11 @@ export class ApexActionController {

private processOasDocument = async (
oasDoc: string,
context: ApexClassOASGatherContextResponse
context: ApexClassOASGatherContextResponse,
eligibleResult: ApexClassOASEligibleResponse
): Promise<OpenAPIV3.Document> => {
const parsed = parse(oasDoc);
const oasProcessor = new OasProcessor(context, parsed);
const oasProcessor = new OasProcessor(context, parsed, eligibleResult);
const processResult = await oasProcessor.process();
return processResult.yaml;
};
Expand Down
7 changes: 6 additions & 1 deletion packages/salesforcedx-vscode-apex/src/messages/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export const messages = {
generate_openapi_document: 'Generating OpenAPI document.',
write_openapi_document: 'Writing OpenAPI Document.',
not_eligible_method:
'Method %s is not eligible for OpenAPI Document creation. It is not annotated with @AuraEnabled or has wrong access modifiers.',
'Method %s is not eligible for OpenAPI Document creation. It is not annotated with an http annotator or has wrong access modifiers.',
apex_test_run_text: 'SFDX: Run Apex Tests',
test_view_loading_message: 'Loading Apex tests ...',
test_view_no_tests_message: 'No Apex Tests Found',
Expand Down Expand Up @@ -131,5 +131,10 @@ export const messages = {
no_oas_generated: 'LLM did not return any content.',
strategy_not_qualified: 'No generation strategy is qualified for the selected class or method.',
invalid_class_annotation_for_generating_oas_doc: 'Invalid class annotation for generating OAS doc',
no_eligible_method: 'No eligible methods found in the class',
failed_to_parse_yaml: 'Failed to parse the document as YAML: %s',
ineligible_method_in_doc: 'Method %s is not eligible for OAS generation, but present in the document',
eligible_method_not_in_doc: 'Methods %s are eligible for OAS generation, but not present in the document',
method_not_found_in_doc_symbols: 'Method %s is not found in the document symbols',
cleanup_yaml_failed: 'Could not find openapi line in document:\n'
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright (c) 2025, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { OpenAPIV3 } from 'openapi-types';
import * as vscode from 'vscode';
import { nls } from '../../messages';
import { ApexClassOASEligibleResponse, OpenAPIDoc } from '../schemas';
import { ProcessorInputOutput, ProcessorStep } from './processorStep';
export class MethodValidationStep implements ProcessorStep {
static diagnosticCollection: vscode.DiagnosticCollection =
vscode.languages.createDiagnosticCollection('OAS Method Validations');
private className: string = '';
private virtualUri: vscode.Uri | null = null; // the url of the virtual YAML file
private diagnostics: vscode.Diagnostic[] = [];
constructor() {}
process(input: ProcessorInputOutput): Promise<ProcessorInputOutput> {
this.className = input.context.classDetail.name as string;
this.virtualUri = vscode.Uri.parse(`untitled:${this.className}_OAS_temp.yaml`);
MethodValidationStep.diagnosticCollection.clear();
const cleanedupYaml = this.validateMethods(input.yaml, input.eligibilityResult);
MethodValidationStep.diagnosticCollection.set(this.virtualUri, this.diagnostics);
input.errors = [...input.errors, ...this.diagnostics];
return new Promise(resolve => {
resolve({ ...input, yaml: cleanedupYaml });
});
}

private validateMethods(
parsed: OpenAPIV3.Document,
eligibilityResult: ApexClassOASEligibleResponse
): OpenAPIV3.Document {
const symbols = eligibilityResult.symbols;
if (!symbols || symbols.length === 0) {
throw new Error(nls.localize('no_eligible_method'));
}
const methodNames = new Set(
symbols.filter(symbol => symbol.isApexOasEligible).map(symbol => symbol.docSymbol.name)
);

for (const [path, methods] of Object.entries(parsed?.paths || {})) {
const methodName = path.split('/').pop();
// make sure all eligible methods are present in the document
if (!methodName || !methodNames.has(methodName)) {
this.diagnostics.push(
new vscode.Diagnostic(
new vscode.Range(0, 0, 0, 0),
nls.localize('ineligible_method_in_doc', methodName),
vscode.DiagnosticSeverity.Error
)
);
} else {
methodNames.delete(methodName);
}
}

if (methodNames.size > 0) {
this.diagnostics.push(
new vscode.Diagnostic(
new vscode.Range(0, 0, 0, 0),
nls.localize('eligible_method_not_in_doc', Array.from(methodNames).join(', ')),
vscode.DiagnosticSeverity.Error
)
);
}
return parsed;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@

import { OpenAPIV3 } from 'openapi-types';
import { nls } from '../../messages';
import { ApexClassOASGatherContextResponse } from '../schemas';
import { ApexClassOASEligibleResponse, ApexClassOASGatherContextResponse } from '../schemas';
import { MethodValidationStep } from './methodValidationStep';
import { MissingPropertiesInjectorStep } from './missingPropertiesInjectorStep';
import { OasValidationStep } from './oasValidationStep';
import { Pipeline } from './pipeline';
Expand All @@ -16,23 +17,33 @@ import { ProcessorInputOutput } from './processorStep';
export class OasProcessor {
private context: ApexClassOASGatherContextResponse;
private document: OpenAPIV3.Document;

constructor(context: ApexClassOASGatherContextResponse, document: OpenAPIV3.Document) {
private eligibilityResult: ApexClassOASEligibleResponse;
constructor(
context: ApexClassOASGatherContextResponse,
document: OpenAPIV3.Document,
eligibilityResult: ApexClassOASEligibleResponse
) {
this.context = context;
this.document = document;
this.eligibilityResult = eligibilityResult;
}

async process(): Promise<ProcessorInputOutput> {
if (this.context.classDetail.annotations.includes('RestResource')) {
// currently only OasValidation exists, in future this would have converters too
const pipeline = new Pipeline(new MissingPropertiesInjectorStep()).addStep(
new OasValidationStep(this.context.classDetail.name)
);
const pipeline = new Pipeline(new MissingPropertiesInjectorStep())
.addStep(new MethodValidationStep())
.addStep(new OasValidationStep(this.context.classDetail.name));

console.log('Executing pipeline with input:');
console.log('context: ', JSON.stringify(this.context));
console.log('document: ', this.document);
const output = await pipeline.execute({ yaml: this.document });
const output = await pipeline.execute({
yaml: this.document,
errors: [],
eligibilityResult: this.eligibilityResult,
context: this.context
});
console.log('Pipeline output:', output);
return output;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class OasValidationStep implements ProcessorStep {

// Add diagnostics to the Problems tab for the virtual document
OasValidationStep.diagnosticCollection.set(virtualUri, diagnostics);
input.errors = results;
input.errors = [...input.errors, ...diagnostics];
});

// Return the input for future processing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { ISpectralDiagnostic } from '@stoplight/spectral-core';
import { OpenAPIV3 } from 'openapi-types';
import * as vscode from 'vscode';
import { ApexClassOASEligibleResponse, ApexClassOASGatherContextResponse } from '../schemas';

export interface ProcessorInputOutput {
yaml: OpenAPIV3.Document;
errors?: ISpectralDiagnostic[];
errors: vscode.Diagnostic[];
readonly eligibilityResult: ApexClassOASEligibleResponse;
readonly context: ApexClassOASGatherContextResponse;
}

export interface ProcessorStep {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
import * as fs from 'fs';
import { OpenAPIV3 } from 'openapi-types';
import { DocumentSymbol } from 'vscode';
import { parse, stringify } from 'yaml';
import * as yaml from 'yaml';
import { nls } from '../../messages';
import {
ApexClassOASEligibleResponse,
ApexClassOASGatherContextResponse,
Expand Down Expand Up @@ -56,10 +57,10 @@ export class MethodByMethodStrategy extends GenerationStrategy {
const combinedText = this.combineYamlByMethod(oas);
return combinedText;
} catch (e) {
throw new Error(`Failed to parse OAS: ${e}`);
throw new Error(nls.localize('failed_to_combine_oas', e));
}
} else {
throw new Error('No OAS generated');
throw new Error(nls.localize('no_oas_generated'));
}
}

Expand All @@ -85,7 +86,7 @@ export class MethodByMethodStrategy extends GenerationStrategy {

for (const doc of docs) {
const yamlCleanDoc = this.cleanYamlString(doc);
const parsed = parse(yamlCleanDoc) as OpenAPIV3.Document;
const parsed = yaml.parse(yamlCleanDoc) as OpenAPIV3.Document;
// Merge paths
if (parsed.paths) {
for (const [path, methods] of Object.entries(parsed.paths)) {
Expand All @@ -104,7 +105,7 @@ export class MethodByMethodStrategy extends GenerationStrategy {
}
}
}
return stringify(combined);
return yaml.stringify(combined);
}

llmResponses: string[];
Expand All @@ -120,6 +121,7 @@ export class MethodByMethodStrategy extends GenerationStrategy {
}
return this.llmResponses;
}

metadata: ApexClassOASEligibleResponse;
context: ApexClassOASGatherContextResponse;
prompts: string[];
Expand Down Expand Up @@ -238,7 +240,7 @@ export class MethodByMethodStrategy extends GenerationStrategy {
const method = lines.slice(startLine - 1, endLine + 1).join('\n');
return method;
} else {
throw new Error(`Method ${methodName} not found in document symbols`);
throw new Error(nls.localize('method_not_found_in_doc_symbols', methodName));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
);
});

Expand Down

0 comments on commit 3cc1b89

Please sign in to comment.