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

[W-17756721] fix: add annotation params to prompts #6063

Open
wants to merge 5 commits into
base: feat/apex-oas
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit worried if this can be fully bundled. Will give a try

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw a note on the npm page regarding bundling. It mentioned that bundling for nodejs should be good, but thank you for the "trust but verify" commitment.

"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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this step to capture what placeholders are on the path and make sure those placeholders have their corresponding parameters in path?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If so, I think we also need to do vice versa, make sure those parameters in: path show up in the request path

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mingxuanzhangsfdx please log a ticket for the case to make sure those parameters in: path show up in the request path.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Loading