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.

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

Merged
merged 12 commits into from
Jan 30, 2025
Merged
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.'
Copy link
Contributor

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.

Copy link
Member Author

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?

);
});

Expand Down
Loading