Skip to content

Commit

Permalink
fix: adjust for new eligibility properties (#5972)
Browse files Browse the repository at this point in the history
@W-17249057@
  • Loading branch information
peternhale authored Dec 9, 2024
1 parent ecaf0d6 commit 90b83ba
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 21 deletions.
Binary file modified packages/salesforcedx-vscode-apex/out/apex-jorje-lsp.jar
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as fs from 'fs';
import * as path from 'path';
import { URL } from 'url';
import * as vscode from 'vscode';
import { stringify } from 'yaml';
import { parse, stringify } from 'yaml';
import { nls } from '../messages';
import { ApexClassOASEligibleResponse, SymbolEligibility } from '../openApiUtilities/schemas';
import { getTelemetryService } from '../telemetry/telemetry';
Expand Down Expand Up @@ -91,13 +91,13 @@ export class ApexActionController {
const documentText = fs.readFileSync(new URL(metadata.resourceUri.toString()), 'utf8');
const className = path.basename(metadata.resourceUri, '.cls');
const methodNames = (metadata.symbols || [])
.filter((symbol: SymbolEligibility) => symbol.isEligible)
.filter((symbol: SymbolEligibility) => symbol.isApexOasEligible)
.map((symbol: SymbolEligibility) => symbol.docSymbol?.name)
.filter((name: string | undefined) => name);
const openAPIdocument = await this.metadataOrchestrator.sendPromptToLLM(documentText, methodNames, className);

// Convert the OpenAPI document to YAML
return stringify(openAPIdocument);
return this.cleanupYaml(openAPIdocument);
};

/**
Expand All @@ -111,4 +111,18 @@ export class ApexActionController {
notificationService.showErrorMessage(`${nls.localize('create_apex_action_failed')}: ${errorMessage}`);
telemetryService.sendException(telemetryEvent, errorMessage);
};

private cleanupYaml(doc: string): string {
// Remove the first line of the document
const openApiIndex = doc.indexOf('openapi');
if (openApiIndex === -1) {
throw new Error('Could not find openapi line in document:\n' + doc);
}
const theDoc = doc
.substring(openApiIndex)
.split('\n')
.filter((line: string) => !line.includes('{AUTHOR_PLACEHOLDER}'))
.join('\n');
return stringify(parse(theDoc));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,18 @@ export class MetadataOrchestrator {
if (!isEligibleResponses || isEligibleResponses.length === 0) {
throw new Error(nls.localize('validation_failed'));
}
if (!isEligibleResponses[0].isEligible) {
if (!isEligibleResponses[0].isApexOasEligible) {
if (isMethodSelected) {
const name = isEligibleResponses?.[0]?.symbols?.[0]?.docSymbol.name;
throw new Error(nls.localize('not_eligible_method', name));
}
throw new Error(nls.localize('apex_class_not_valid', path.basename(isEligibleResponses[0].resourceUri, '.cls')));
}
const symbols = isEligibleResponses[0].symbols ?? [];
const eligibleSymbols = symbols.filter(s => s.isApexOasEligible);
if (eligibleSymbols.length === 0) {
throw new Error(nls.localize('apex_class_not_valid', path.basename(isEligibleResponses[0].resourceUri, '.cls')));
}
return isEligibleResponses[0];
};

Expand Down Expand Up @@ -107,7 +112,7 @@ export class MetadataOrchestrator {
includeAllMethods: true,
includeAllProperties: true,
methodNames: [],
positions: null,
position: null,
propertyNames: []
} as ApexClassOASEligibleRequest;
requests.push(request);
Expand All @@ -128,7 +133,7 @@ export class MetadataOrchestrator {
resourceUri: sourceUri ? sourceUri.toString() : vscode.window.activeTextEditor?.document.uri.toString() || '',
includeAllMethods: !isMethodSelected,
includeAllProperties: !isMethodSelected,
positions: cursorPosition ? [cursorPosition] : null,
position: cursorPosition ?? null,
methodNames: [],
propertyNames: []
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,20 @@ export type ApexClassOASEligibleRequest = {
resourceUri: string;
includeAllMethods: boolean;
includeAllProperties: boolean;
positions: vscode.Position[] | null;
position: vscode.Position | null;
methodNames: string[] | null;
propertyNames: string[] | null;
};

export interface SymbolEligibility {
isEligible: boolean;
isApexOasEligible: boolean;
docSymbol: DocumentSymbol;
}

export type ApexClassOASEligibleResponse = {
isEligible: boolean;
isApexOasEligible: boolean;
resourceUri: string;
symbols?: SymbolEligibility[];
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,31 @@ describe('MetadataOrchestrator', () => {
});

it('should throw an error if the first eligible response is not eligible and method is selected', async () => {
const mockResponse: any = [{ isEligible: false, symbols: [{ docSymbol: { name: 'someMethod' } }] }];
const mockResponse: any = [
{ isApexOasEligible: false, isEligible: false, symbols: [{ docSymbol: { name: 'someMethod' } }] }
];
jest.spyOn(orchestrator, 'validateEligibility').mockResolvedValue(mockResponse);
await expect(orchestrator.extractMetadata(editorStub.document.uri, true)).rejects.toThrow(
'Method someMethod is not eligible for Apex Action creation. It is not annotated with @AuraEnabled or has wrong access modifiers.'
);
});

it('should throw an error if the first eligible response is not eligible and method is not selected', async () => {
const mockResponse: any = [{ isEligible: false, resourceUri: '/hello/world.cls' }];
const mockResponse: any = [{ isApexOasEligible: false, isEligible: false, resourceUri: '/hello/world.cls' }];
jest.spyOn(orchestrator, 'validateEligibility').mockResolvedValue(mockResponse);
await expect(orchestrator.extractMetadata(editorStub.document.uri)).rejects.toThrow(
'The Apex Class world is not valid for Open AI document generation.'
);
});

it('should return the first eligible response if it is eligible', async () => {
const mockResponse: any = [{ isEligible: true }];
const mockResponse: any = [
{
isApexOasEligible: true,
isEligible: true,
symbols: [{ isApexOasEligible: true, docSymbol: { name: 'someMethod' } }]
}
];
jest.spyOn(orchestrator, 'validateEligibility').mockResolvedValue(mockResponse);
const result = await orchestrator.extractMetadata(editorStub.document.uri);
expect(result).toEqual(mockResponse[0]);
Expand All @@ -81,23 +89,23 @@ describe('MetadataOrchestrator', () => {
(getTelemetryService as jest.Mock).mockResolvedValue(new MockTelemetryService());
});
it('should call eligibilityDelegate with expected parameter when there are multiple uris (requests)', async () => {
const responses = [{ isEligible: true, resourceUri: 'file.cls' }];
const responses = [{ isApexOasEligible: true, isEligible: true, resourceUri: 'file.cls' }];
const uris = [{ path: '/hello/world.cls' } as vscode.Uri, { path: 'hola/world.cls' } as vscode.Uri];
const expectedRequest = {
payload: [
{
resourceUri: uris[0].toString(),
includeAllMethods: true,
includeAllProperties: true,
positions: null,
position: null,
methodNames: [],
propertyNames: []
},
{
resourceUri: uris[1].toString(),
includeAllMethods: true,
includeAllProperties: true,
positions: null,
position: null,
methodNames: [],
propertyNames: []
}
Expand Down Expand Up @@ -126,7 +134,7 @@ describe('MetadataOrchestrator', () => {
});

it('should call eligibilityDelegate with expected parameter when there is single request', async () => {
const responses = [{ isEligible: true, resourceUri: 'file.cls' }];
const responses = [{ isApexOasEligible: true, isEligible: true, resourceUri: 'file.cls' }];
const uri = { path: '/file.cls' } as vscode.Uri;
eligibilityDelegateSpy = jest.spyOn(orchestrator, 'eligibilityDelegate').mockResolvedValue(responses);
const mockEditor = {
Expand All @@ -137,7 +145,7 @@ describe('MetadataOrchestrator', () => {
resourceUri: uri.toString(),
includeAllMethods: true,
includeAllProperties: true,
positions: null,
position: null,
methodNames: [],
propertyNames: []
};
Expand Down Expand Up @@ -165,7 +173,7 @@ describe('MetadataOrchestrator', () => {
resourceUri: 'file:///Users/peter.hale/git/apex-perf-project/force-app/main/default/classes',
includeAllMethods: true,
includeAllProperties: true,
positions: [],
position: null,
methodNames: [],
propertyNames: []
}
Expand All @@ -185,7 +193,7 @@ describe('MetadataOrchestrator', () => {
resourceUri: 'file:///Users/peter.hale/git/apex-perf-project/force-app/main/default/classes',
includeAllMethods: true,
includeAllProperties: true,
positions: [],
position: null,
methodNames: [],
propertyNames: []
}
Expand All @@ -201,7 +209,7 @@ describe('MetadataOrchestrator', () => {
resourceUri: 'file:///Users/peter.hale/git/apex-perf-project/force-app/main/default/classes/file.cls',
includeAllMethods: true,
includeAllProperties: true,
positions: [],
position: null,
methodNames: [],
propertyNames: []
}
Expand All @@ -217,7 +225,7 @@ describe('MetadataOrchestrator', () => {
resourceUri: 'file:///Users/peter.hale/git/apex-perf-project/force-app/main/default/classes/file.cls',
includeAllMethods: false,
includeAllProperties: false,
positions: [new vscode.Position(3, 5)],
position: new vscode.Position(3, 5),
methodNames: [],
propertyNames: []
}
Expand All @@ -233,15 +241,15 @@ describe('MetadataOrchestrator', () => {
resourceUri: 'file:///Users/peter.hale/git/apex-perf-project/force-app/main/default/classes/file1.cls',
includeAllMethods: true,
includeAllProperties: true,
positions: null,
position: null,
methodNames: [],
propertyNames: []
},
{
resourceUri: 'file:///Users/peter.hale/git/apex-perf-project/force-app/main/default/classes/file2.cls',
includeAllMethods: true,
includeAllProperties: true,
positions: null,
position: null,
methodNames: [],
propertyNames: []
}
Expand Down

0 comments on commit 90b83ba

Please sign in to comment.