NEW (Extension) @W-16096256@ Send class content as request to ApexGuru and get requestId back #114
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| /* | ||
| * Copyright (c) 2024, Salesforce, Inc. | ||
| * All rights reserved. | ||
| * SPDX-License-Identifier: BSD-3-Clause | ||
| * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||
| */ | ||
|
|
||
| import * as vscode from 'vscode'; | ||
| import * as fspromises from 'fs/promises'; | ||
| import { CoreExtensionService } from '../lib/core-extension-service'; | ||
| import { ApexGuruAuthResponse, ApexGuruInitialResponse } from '../types'; | ||
| import * as Constants from '../lib/constants'; | ||
|
|
||
| export async function isApexGuruEnabledInOrg(outputChannel: vscode.LogOutputChannel): Promise<boolean> { | ||
| try { | ||
| const connection = await CoreExtensionService.getConnection(); | ||
| const response:ApexGuruAuthResponse = await connection.request({ | ||
| method: 'GET', | ||
| url: Constants.APEX_GURU_AUTH_ENDPOINT, | ||
| body: '' | ||
| }); | ||
| return response.status == 'Success'; | ||
| } catch(e) { | ||
| // This could throw an error for a variety of reasons. The API endpoint has not been deployed to the instance, org has no perms, timeouts etc,. | ||
| // In all of these scenarios, we return false. | ||
| const errMsg = e instanceof Error ? e.message : e as string; | ||
| outputChannel.error('***ApexGuru perm check failed with error:***' + errMsg); | ||
| outputChannel.show(); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export async function runApexGuruOnFile(selection: vscode.Uri, outputChannel: vscode.LogOutputChannel) { | ||
| try { | ||
| const requestId = await initiateApexGuruRequest(selection, outputChannel); | ||
| outputChannel.appendLine('***Apex Guru request Id:***' + requestId); | ||
| } catch (e) { | ||
| const errMsg = e instanceof Error ? e.message : e as string; | ||
| outputChannel.appendLine('***Apex Guru initiate request failed***'); | ||
| outputChannel.appendLine(errMsg); | ||
| } | ||
| } | ||
|
|
||
| export async function initiateApexGuruRequest(selection: vscode.Uri, outputChannel: vscode.LogOutputChannel) { | ||
|
There was a problem hiding this comment. Can you add |
||
| const fileContent = await fileSystem.readFile(selection.fsPath); | ||
| const base64EncodedContent = Buffer.from(fileContent).toString('base64'); | ||
| const connection = await CoreExtensionService.getConnection(); | ||
| const response: ApexGuruInitialResponse = await connection.request({ | ||
| method: 'POST', | ||
| url: Constants.APEX_GURU_REQUEST, | ||
| body: JSON.stringify({ | ||
| classContent: base64EncodedContent | ||
| }) | ||
| }); | ||
|
Comment on lines
+48
to
+54
There was a problem hiding this comment. It looks like you are assuming that the response will always be of type ApexGuruInitialResponse. But is it possible that you do not get a response back that has a status field? I suggest you move your ApexGuruInitialResponse and other types into typescript officially (in a .ts - not a .d.ts file) to be compiled, then this will complain unless you cast with "as ApexGuruInitialResponse" to the request output. But do we really want to do this casting? I would guess that you would want to first check the status code of the response. But now I see that the request method is templated so it is inferring the output type. Do we know if the response object universally as something like a response code that can be used? Typically when making http requests, we validate the response to see if it is a 2** or a 4** or a 5** before we try to parse the response body. There was a problem hiding this comment. I moved the ApexGuruInitialResponse out out of the d.ts file. I noticed we don't get 2**/4** responses as part of the response. I'll bring this up with the ApexGuru team tomorrow in the meeting. I hope it should be easy fix for them. Btw, the connection request also does not give the status codes. So, I'll have to leave this as is for now. |
||
|
|
||
| if (response.status != 'new' && response.status != 'success') { | ||
| outputChannel.warn('***Apex Guru returned unexpected response:***' + response.status); | ||
| return ''; | ||
|
There was a problem hiding this comment. I'm guessing we'll be adding in polling soon (in a future PR) in order to wait for the job to finish so we can get the results back and process the output. So my question is, is returning an empty string really what we want when the initiate request doesn't succeed. I would think we would just error and not return in order to prevent polling from even starting. There was a problem hiding this comment. A lot of this is TBD and so I just returned an empty string. Sure, I went ahead and made it return an error instead. |
||
| } | ||
|
|
||
| const requestId = response.requestId; | ||
| return requestId; | ||
| } | ||
|
|
||
| export const fileSystem = { | ||
| readFile: (path: string) => fspromises.readFile(path, 'utf8') | ||
| }; | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| /* | ||
| * Copyright (c) 2023, Salesforce, Inc. | ||
| * All rights reserved. | ||
| * SPDX-License-Identifier: BSD-3-Clause | ||
| * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||
| */ | ||
|
|
||
| import {expect} from 'chai'; | ||
| import Sinon = require('sinon'); | ||
| import {CoreExtensionService} from '../../../lib/core-extension-service'; | ||
| import * as Constants from '../../../lib/constants'; | ||
| import * as ApexGuruFunctions from '../../../apexguru/apex-guru-service' | ||
| import * as fspromises from 'fs/promises'; | ||
|
|
||
| // You can import and use all API from the 'vscode' module | ||
| // as well as import your extension to test it | ||
| import * as vscode from 'vscode'; | ||
|
|
||
| suite('Apex Guru Test Suite', () => { | ||
| let outputChannel = vscode.window.createOutputChannel('Salesforce Code Analyzer', {log: true}); | ||
|
|
||
| suite('#_isApexGuruEnabledInOrg', () => { | ||
| let getConnectionStub: Sinon.SinonStub; | ||
| let requestStub: Sinon.SinonStub; | ||
|
|
||
| setup(() => { | ||
| getConnectionStub = Sinon.stub(CoreExtensionService, 'getConnection'); | ||
| requestStub = Sinon.stub(); | ||
| }); | ||
|
|
||
| teardown(() => { | ||
| Sinon.restore(); | ||
| }); | ||
|
|
||
| test('Returns true if response status is Success', async () => { | ||
| // ===== SETUP ===== | ||
| getConnectionStub.resolves({ | ||
| request: requestStub.resolves({ status: 'Success' }) | ||
| }); | ||
|
|
||
| // ===== TEST ===== | ||
| const result = await ApexGuruFunctions.isApexGuruEnabledInOrg(outputChannel); | ||
|
|
||
| // ===== ASSERTIONS ===== | ||
| expect(result).to.be.true; | ||
| Sinon.assert.calledOnce(getConnectionStub); | ||
| Sinon.assert.calledOnce(requestStub); | ||
| Sinon.assert.calledWith(requestStub, { | ||
| method: 'GET', | ||
| url: Constants.APEX_GURU_AUTH_ENDPOINT, | ||
| body: '' | ||
| }); | ||
| }); | ||
|
|
||
| test('Returns false if response status is not Success', async () => { | ||
| // ===== SETUP ===== | ||
| getConnectionStub.resolves({ | ||
| request: requestStub.resolves({ status: 'Failure' }) | ||
| }); | ||
|
|
||
| // ===== TEST ===== | ||
| const result = await ApexGuruFunctions.isApexGuruEnabledInOrg(outputChannel); | ||
|
|
||
| // ===== ASSERTIONS ===== | ||
| expect(result).to.be.false; | ||
| Sinon.assert.calledOnce(getConnectionStub); | ||
| Sinon.assert.calledOnce(requestStub); | ||
| Sinon.assert.calledWith(requestStub, { | ||
| method: 'GET', | ||
| url: Constants.APEX_GURU_AUTH_ENDPOINT, | ||
| body: '' | ||
| }); | ||
| }); | ||
|
|
||
| test('Returns false if an error is thrown', async () => { | ||
| // ===== SETUP ===== | ||
| getConnectionStub.resolves({ | ||
| request: requestStub.rejects(new Error('Resource not found')) | ||
| }); | ||
|
|
||
| // ===== TEST ===== | ||
| const result = await ApexGuruFunctions.isApexGuruEnabledInOrg(outputChannel); | ||
|
|
||
| // ===== ASSERTIONS ===== | ||
| expect(result).to.be.false; | ||
| Sinon.assert.calledOnce(getConnectionStub); | ||
| Sinon.assert.calledOnce(requestStub); | ||
| Sinon.assert.calledWith(requestStub, { | ||
| method: 'GET', | ||
| url: Constants.APEX_GURU_AUTH_ENDPOINT, | ||
| body: '' | ||
| }); | ||
| }); | ||
| }); | ||
| suite('#initiateApexGuruRequest', () => { | ||
| let getConnectionStub: Sinon.SinonStub; | ||
| let requestStub: Sinon.SinonStub; | ||
| let readFileStub: Sinon.SinonStub; | ||
|
|
||
| setup(() => { | ||
| getConnectionStub = Sinon.stub(CoreExtensionService, 'getConnection'); | ||
| requestStub = Sinon.stub(); | ||
| readFileStub = Sinon.stub(ApexGuruFunctions.fileSystem, 'readFile'); | ||
| }); | ||
|
|
||
| teardown(() => { | ||
| Sinon.restore(); | ||
| }); | ||
|
|
||
| test('Returns requestId if response status is new', async () => { | ||
| // ===== SETUP ===== | ||
| getConnectionStub.resolves({ | ||
| request: requestStub.resolves({ status: 'new', requestId: '12345' }) | ||
| }); | ||
| readFileStub.resolves('console.log("Hello World");'); | ||
|
|
||
| // ===== TEST ===== | ||
| const result = await ApexGuruFunctions.initiateApexGuruRequest(vscode.Uri.file('dummyPath'), outputChannel); | ||
|
|
||
| // ===== ASSERTIONS ===== | ||
| expect(result).to.equal('12345'); | ||
| Sinon.assert.calledOnce(getConnectionStub); | ||
| Sinon.assert.calledOnce(requestStub); | ||
| Sinon.assert.calledOnce(readFileStub); | ||
| Sinon.assert.calledWith(requestStub, Sinon.match({ | ||
| method: 'POST', | ||
| url: Constants.APEX_GURU_REQUEST, | ||
| body: Sinon.match.string | ||
| })); | ||
| }); | ||
|
|
||
| test('Logs warning if response status is not new', async () => { | ||
| // ===== SETUP ===== | ||
| getConnectionStub.resolves({ | ||
| request: requestStub.resolves({ status: 'failed' }) | ||
| }); | ||
| readFileStub.resolves('console.log("Hello World");'); | ||
| const outputChannelSpy = Sinon.spy(outputChannel, 'warn'); | ||
|
|
||
| // ===== TEST ===== | ||
| await ApexGuruFunctions.initiateApexGuruRequest(vscode.Uri.file('dummyPath'), outputChannel); | ||
|
|
||
| // ===== ASSERTIONS ===== | ||
| Sinon.assert.calledOnce(outputChannelSpy); | ||
| Sinon.assert.calledWith(outputChannelSpy, Sinon.match.string); | ||
| }); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we should move all these messages eventually to a message catalog. A super lightweight version that I created for the 5.x world is https://github.com/forcedotcom/code-analyzer-core/blob/dev/packages/code-analyzer-engine-api/src/messages.ts#L18 and an example of how it is used: https://github.com/forcedotcom/code-analyzer-core/blob/dev/packages/code-analyzer-eslint-engine/src/messages.ts and https://github.com/forcedotcom/code-analyzer-core/blob/dev/packages/code-analyzer-eslint-engine/src/engine.ts#L117
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! I would like to follow the pattern that IDX team follows that helps with i18n as well. I am not doing it as part of this. But definitely worth doing it when the 5.x integration work happens in the extension.