Skip to content

Commit

Permalink
[W-17756721] fix: add annotation params to prompts (#6063)
Browse files Browse the repository at this point in the history
* fix: add annotation params to prompts

@W-17756721@ add annotation params from context to prompts
add process step to resolve semantically duplicate paths

* Update packages/salesforcedx-utils-vscode/src/helpers/utils.ts

Co-authored-by: Mingxuan Zhang <[email protected]>

* chore: apply review suggestions

modified annotation prompt builder to exclude Parameters text when no params

---------

Co-authored-by: mingxuanzhang <[email protected]>
Co-authored-by: Mingxuan Zhang <[email protected]>
Co-authored-by: Daphne Yang <[email protected]>
  • Loading branch information
4 people authored Feb 7, 2025
1 parent 0817e16 commit 43cac02
Show file tree
Hide file tree
Showing 13 changed files with 239 additions and 118 deletions.
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/salesforcedx-utils-vscode/src/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export {
export * from './extensionUris';
export { TraceFlags } from './traceFlags';
export { TraceFlagsRemover } from './traceFlagsRemover';
export { asyncFilter, extractJsonObject, getMessageFromError, isNullOrUndefined, fileUtils } from './utils';
export { asyncFilter, difference, extractJsonObject, getMessageFromError, isNullOrUndefined, fileUtils } from './utils';
export { isAlphaNumSpaceString, isAlphaNumString, isInteger, isIntegerInRange, isRecordIdFormat } from './validations';
export { isSFContainerMode } from './env';
export { ActivationTracker } from './activationTracker';
10 changes: 10 additions & 0 deletions packages/salesforcedx-utils-vscode/src/helpers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,13 @@ export const ansiRegex = ({ onlyFirst = false } = {}): RegExp => {

return new RegExp(pattern, onlyFirst ? undefined : 'g');
};

/**
* Returns elements that are in setA but not in setB.
* @param setA
* @param setB
* @returns
*/
export const difference = <T>(setA: Set<T>, setB: Set<T>): Set<T> => {
return new Set([...setA].filter(x => !setB.has(x)));
};
Binary file modified packages/salesforcedx-vscode-apex/out/apex-jorje-lsp.jar
Binary file not shown.
1 change: 1 addition & 0 deletions packages/salesforcedx-vscode-apex/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@stoplight/spectral-rulesets": "1.21.3",
"expand-home-dir": "0.0.3",
"find-java-home": "0.2.0",
"jsonpath-plus": "10.2.0",
"shelljs": "0.8.5",
"vscode-extension-telemetry": "0.0.17",
"vscode-languageclient": "8.1.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import { difference } from '@salesforce/salesforcedx-utils-vscode';
import { JSONPath } from 'jsonpath-plus';
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');
Expand All @@ -30,42 +33,40 @@ export class MethodValidationStep implements ProcessorStep {
}

private validateMethods(
parsed: OpenAPIV3.Document,
oasYaml: 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(
const methodNames = new Set<string>(
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);
}
}
// Use JSONPath to find all operationIds in the OAS document
const operationIds = new Set<string>(JSONPath({ path: '$..operationId', json: oasYaml }));

if (methodNames.size > 0) {
difference(methodNames, operationIds).forEach(methodName => {
this.diagnostics.push(
new vscode.Diagnostic(
new vscode.Range(0, 0, 0, 0),
nls.localize('eligible_method_not_in_doc', Array.from(methodNames).join(', ')),
nls.localize('eligible_method_not_in_doc', methodName),
vscode.DiagnosticSeverity.Error
)
);
}
return parsed;
});

difference(operationIds, methodNames).forEach(methodName => {
this.diagnostics.push(
new vscode.Diagnostic(
new vscode.Range(0, 0, 0, 0),
nls.localize('ineligible_method_in_doc', methodName),
vscode.DiagnosticSeverity.Error
)
);
});

return oasYaml;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import { OpenAPIV3 } from 'openapi-types';
import { nls } from '../../messages';
import { ApexClassOASEligibleResponse, ApexClassOASGatherContextResponse } from '../schemas';
import { MethodValidationStep } from './methodValidationStep';
import { MissingPropertiesInjectorStep } from './missingPropertiesInjectorStep';
import { OasValidationStep } from './oasValidationStep';
import { Pipeline } from './pipeline';
import { ProcessorInputOutput } from './processorStep';
import { PropertyCorrectionStep } from './propertyCorrectionStep';
import { ReconcileDuplicateSemanticPathsStep } from './reconcileDuplicateSemanticPathsStep';

export class OasProcessor {
private context: ApexClassOASGatherContextResponse;
Expand All @@ -29,9 +30,10 @@ export class OasProcessor {
}

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

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* 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 { ProcessorInputOutput, ProcessorStep } from './processorStep';

export class PropertyCorrectionStep implements ProcessorStep {
process(input: ProcessorInputOutput): Promise<ProcessorInputOutput> {
let fixedYaml = this.ensureServersIsPresent(input.yaml);
fixedYaml = this.ensureInfoVersionIsPresent(fixedYaml);

return new Promise(resolve => {
resolve({ ...input, yaml: fixedYaml });
});
}

private ensureInfoVersionIsPresent(yaml: OpenAPIV3.Document<{}>): OpenAPIV3.Document<{}> {
return { ...yaml, ...{ info: { ...yaml.info, ...{ version: '1.0.0' } } } };
}

private ensureServersIsPresent(yaml: OpenAPIV3.Document<{}>): OpenAPIV3.Document<{}> {
return { ...yaml, ...{ servers: [{ url: '/servers/apexrest' }] } };
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* 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 { JSONPath } from 'jsonpath-plus';
import { OpenAPIV3 } from 'openapi-types';
import { ProcessorInputOutput, ProcessorStep } from './processorStep';

export class ReconcileDuplicateSemanticPathsStep implements ProcessorStep {
process(input: ProcessorInputOutput): Promise<ProcessorInputOutput> {
const fixedYaml = this.resolvePathsThatAreSemanticallyEqual(input.yaml);

return new Promise(resolve => {
resolve({ ...input, yaml: fixedYaml });
});
}

private resolvePathsThatAreSemanticallyEqual(yaml: OpenAPIV3.Document): OpenAPIV3.Document {
const newPaths: Record<string, OpenAPIV3.PathItemObject> = {};
const paramNames: Record<string, string> = {};

const pathsToFix = this.getPathsToFix(yaml);

JSONPath({
path: '$.paths',
json: yaml,
resultType: 'all',
callback: ({ value }: { value: Record<string, OpenAPIV3.PathItemObject> }) => {
Object.entries(value).forEach(([methodPath, methodValues]) => {
const fromName = this.getNameFromPath(methodPath);
const toName = this.getNameFromPath(pathsToFix[methodPath] ?? methodPath);
const paramName = toName ?? fromName ?? 'param';
const newPath = pathsToFix[methodPath] ?? methodPath;

newPaths[newPath] = { ...(newPaths[newPath] ?? {}), ...methodValues };

// Store the parameter name for the new path
if (!paramNames[newPath]) {
paramNames[newPath] = paramName;
}

// Update parameter names to match the new path
JSONPath({
path: "$..parameters[?(@.in=='path')]",
json: methodValues,
resultType: 'all',
callback: ({ value: paramValue }: { value: OpenAPIV3.ParameterObject }) => {
paramValue.name = paramName;
}
});
});
}
});

yaml.paths = newPaths;
return yaml;
}

private getPathsToFix(yaml: OpenAPIV3.Document): Record<string, string> {
const paths = JSONPath({
path: '$.paths',
json: yaml,
resultType: 'value'
})[0] as Record<string, OpenAPIV3.PathItemObject>;

return Object.keys(paths)
.filter(path => path.match(/\{[^}]+}/))
.reduce(
(acc, path, index, thePaths) => {
const toPath = thePaths[0];
acc[path] = toPath;
return acc;
},
{} as Record<string, string>
);
}

private getNameFromPath(path: string): string | undefined {
const match = path.match(/\{([^}]+)}/);
return match ? match[1] : undefined;
}
}
Loading

0 comments on commit 43cac02

Please sign in to comment.