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

JS-154: Bridge sends AST to plugin as multipart/form-data #4711

Merged
merged 30 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9d9dd8a
return arbitrary string in response body 'ast' prop
kebetsi May 27, 2024
0feab0d
reply with form-data server side
kebetsi May 27, 2024
26b4bd2
adapt other tests
kebetsi May 28, 2024
f8ef04b
java-part first-try
kebetsi May 28, 2024
e64759f
Merge branch 'master' into JS-154-bridge-sends-AST-to-plugin
kebetsi May 28, 2024
5aaa682
parse JSON multipart correctly for /analyze-js
kebetsi May 28, 2024
863f3b6
refactor form data parsing code into its own method
kebetsi May 28, 2024
0e234ea
adapt other bridge calls
kebetsi May 29, 2024
b47c6af
remove separate implementation for tests
kebetsi May 29, 2024
54422f2
generate `ast` prop for other files as well
kebetsi May 29, 2024
b01dd68
fix issues
kebetsi May 29, 2024
1472fee
fix compilation errors
kebetsi May 29, 2024
f35478d
fix analysis errors
kebetsi May 29, 2024
9ff2f4a
add 'form-data' to bundledDependencies
kebetsi May 29, 2024
0208a92
cleanup and fix IT
kebetsi May 29, 2024
1e8ba59
put back check
kebetsi May 29, 2024
a39d70d
put back throws exception
kebetsi May 29, 2024
985a66d
cleanup tests
kebetsi May 30, 2024
0966504
fix test
kebetsi May 30, 2024
fb5476f
improve coverage on parseFormData
kebetsi May 30, 2024
aa21386
fix issues
kebetsi May 30, 2024
0e91b3c
remove package.json and its lockfile
kebetsi May 30, 2024
151e5e9
fix issue
kebetsi May 30, 2024
3da0a20
refactor mock-bridge startServer.js script to not use 'form-data' lib
kebetsi May 30, 2024
012cac2
refactor tests to display newlines clearer
kebetsi May 30, 2024
ff69568
Merge branch 'master' into JS-154-bridge-sends-AST-to-plugin
kebetsi May 30, 2024
d7922c5
add missing file header
kebetsi May 30, 2024
9644ece
add newline
kebetsi May 30, 2024
666b9b1
use content-type header instead of boolean to figure out the response…
kebetsi May 30, 2024
ee1d7bd
fix issue
kebetsi May 31, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,9 @@ private void assertAnalyzeJs(Bridge bridge) throws IOException, InterruptedExcep
r.fileContent =
"function foo() { \n" + " var a; \n" + " var c; // NOSONAR\n" + " var b = 42; \n" + "} \n";
r.filePath = temp.resolve("file.js").toAbsolutePath().toString();
String response = bridge.request(gson.toJson(r), "analyze-js");
JsonObject jsonObject = gson.fromJson(response, JsonObject.class);
HttpResponse<String> response = bridge.request(gson.toJson(r), "analyze-js");
var parsedResponse = parseFormData(response);
JsonObject jsonObject = gson.fromJson(parsedResponse.json(), JsonObject.class);
JsonArray issues = jsonObject.getAsJsonArray("issues");
assertThat(issues).hasSize(3);
assertThat(issues)
Expand All @@ -108,6 +109,34 @@ private void assertAnalyzeJs(Bridge bridge) throws IOException, InterruptedExcep
JsonObject metrics = jsonObject.getAsJsonObject("metrics");
assertThat(metrics.entrySet()).hasSize(1);
assertThat(metrics.get("nosonarLines").getAsJsonArray()).containsExactly(new JsonPrimitive(3));
assertThat(parsedResponse.ast()).contains("plop");
}

private static BridgeResponse parseFormData(HttpResponse<String> response) {
String boundary = "--" + response.headers().firstValue("Content-Type")
.orElseThrow(() -> new IllegalStateException("No Content-Type header"))
.split("boundary=")[1];
String[] parts = response.body().split(boundary);
String json = null;
String ast = null;
for (String part : parts) {
// Split the part into headers and body
String[] splitPart = part.split("\r\n\r\n", 2);
if (splitPart.length < 2) {
// Skip if there's no body
continue;
}

String headers = splitPart[0];
String partBody = splitPart[1];

if (headers.contains("json")) {
json = partBody;
} else if (headers.contains("ast")) {
ast = partBody;
}
}
return new BridgeResponse(json, ast);
ilia-kebets-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
}

private void assertStatus(Bridge bridge) {
Expand Down Expand Up @@ -186,15 +215,14 @@ void start(Path dest) throws IOException {
process = pb.start();
}

String request(String json, String endpoint) throws IOException, InterruptedException {
HttpResponse<String> request(String json, String endpoint) throws IOException, InterruptedException {
var request = HttpRequest
.newBuilder(url(endpoint))
.header("Content-Type", "application/json")
.POST(HttpRequest.BodyPublishers.ofString(json))
.build();

var response = client.send(request, HttpResponse.BodyHandlers.ofString());
return response.body();
return client.send(request, HttpResponse.BodyHandlers.ofString());
}

String status() throws IOException, InterruptedException {
Expand Down Expand Up @@ -249,4 +277,6 @@ static class Rule {
List<Object> configurations = Collections.emptyList();
String fileTypeTarget = "MAIN";
}

record BridgeResponse(String json, String ast) {}
}
66 changes: 66 additions & 0 deletions package-lock.json

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

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
"eslint-plugin-react-hooks": "4.6.0",
"eslint-plugin-sonarjs": "^1.0.3",
"express": "4.19.2",
"form-data": "4.0.0",
"functional-red-black-tree": "1.0.1",
"htmlparser2": "9.1.0",
"jsx-ast-utils": "3.3.5",
Expand Down Expand Up @@ -136,6 +137,7 @@
"eslint-plugin-react-hooks",
"eslint-plugin-sonarjs",
"express",
"form-data",
"functional-red-black-tree",
"htmlparser2",
"jsx-ast-utils",
Expand Down
1 change: 1 addition & 0 deletions packages/bridge/src/errors/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,5 @@ export const EMPTY_JSTS_ANALYSIS_OUTPUT: JsTsAnalysisOutput = {
cognitiveComplexity: 0,
},
cpdTokens: [],
ast: '',
};
27 changes: 24 additions & 3 deletions packages/bridge/src/worker.js
ilia-kebets-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

require('module-alias/register');

const formData = require('form-data');
const { parentPort, workerData } = require('worker_threads');
const {
analyzeJSTS,
Expand Down Expand Up @@ -47,8 +48,20 @@ exports.delegate = function (worker, type) {
worker.once('message', message => {
switch (message.type) {
case 'success':
response.send(message.result);
if (message.format === 'multipart') {
const fd = new formData();
fd.append('ast', message.result.ast);
delete message.result.ast;
fd.append('json', JSON.stringify(message.result));
// this adds the boundary string that will be used to separate the parts
response.set('Content-Type', fd.getHeaders()['content-type']);
response.set('Content-Length', fd.getLengthSync());
fd.pipe(response);
} else {
response.send(message.result);
}
break;

case 'failure':
next(message.error);
break;
Expand Down Expand Up @@ -92,7 +105,11 @@ if (parentPort) {
await readFileLazily(data);

const output = analyzeJSTS(data, 'js');
parentThread.postMessage({ type: 'success', result: JSON.stringify(output) });
parentThread.postMessage({
type: 'success',
result: output,
format: 'multipart',
});
break;
}

Expand All @@ -107,7 +124,11 @@ if (parentPort) {
await readFileLazily(data);

const output = analyzeJSTS(data, 'ts');
parentThread.postMessage({ type: 'success', result: JSON.stringify(output) });
parentThread.postMessage({
type: 'success',
result: output,
format: 'multipart',
});
break;
}

Expand Down
22 changes: 16 additions & 6 deletions packages/bridge/tests/router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,10 @@ describe('router', () => {
const filePath = path.join(fixtures, 'file.js');
const fileType = 'MAIN';
const data = { filePath, fileType, tsConfigs: [] };
const response = (await request(server, '/analyze-js', 'POST', data)) as string;
const response = (await request(server, '/analyze-js', 'POST', data, 'formdata')) as FormData;
const {
issues: [issue],
} = JSON.parse(response);
} = JSON.parse(response.get('json') as string);
expect(issue).toEqual(
expect.objectContaining({
ruleId: 'prefer-regex-literals',
Expand All @@ -128,6 +128,8 @@ describe('router', () => {
message: `Use a regular expression literal instead of the 'RegExp' constructor.`,
}),
);
const ast = response.get('ast');
expect(ast).toEqual('plop');
});

it('should route /analyze-ts requests', async () => {
Expand All @@ -138,10 +140,10 @@ describe('router', () => {
const fileType = 'MAIN';
const tsConfig = path.join(fixtures, 'tsconfig.json');
const data = { filePath, fileType, tsConfigs: [tsConfig] };
const response = (await request(server, '/analyze-ts', 'POST', data)) as string;
const response = (await request(server, '/analyze-ts', 'POST', data, 'formdata')) as FormData;
const {
issues: [issue],
} = JSON.parse(response);
} = JSON.parse(response.get('json') as string);
expect(issue).toEqual(
expect.objectContaining({
ruleId: 'no-duplicate-in-composite',
Expand All @@ -152,6 +154,8 @@ describe('router', () => {
message: `Remove this duplicated type or replace with another one.`,
}),
);
const ast = response.get('ast');
expect(ast).toEqual('plop');
});

it('should route /analyze-with-program requests', async () => {
Expand All @@ -165,10 +169,16 @@ describe('router', () => {
(await request(server, '/create-program', 'POST', { tsConfig })) as string,
);
const data = { filePath, fileType, programId };
const response = (await request(server, '/analyze-with-program', 'POST', data)) as string;
const response = (await request(
server,
'/analyze-with-program',
'POST',
data,
'formdata',
)) as FormData;
const {
issues: [issue],
} = JSON.parse(response);
} = JSON.parse(response.get('json') as string);
expect(issue).toEqual(
expect.objectContaining({
ruleId: 'no-duplicate-in-composite',
Expand Down
18 changes: 11 additions & 7 deletions packages/bridge/tests/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { start } from '../src/server';
import path from 'path';
import { setContext } from '@sonar/shared';
import { AddressInfo } from 'net';
import { request } from './tools';
import { BridgeResponseType, request } from './tools';
import http from 'http';

describe('server', () => {
Expand Down Expand Up @@ -75,10 +75,10 @@ describe('server', () => {
});

expect(await requestInitLinter(server, fileType, ruleId)).toBe('OK!');

const response = await requestAnalyzeJs(server, fileType, 'formdata');
const {
issues: [issue],
} = JSON.parse(await requestAnalyzeJs(server, fileType));
} = JSON.parse(response.get('json'));
expect(issue).toEqual(
expect.objectContaining({
ruleId,
Expand All @@ -99,11 +99,11 @@ describe('server', () => {
const fileType = 'MAIN';

await requestInitLinter(server, fileType, ruleId);
const response = await requestAnalyzeJs(server, fileType);
const response = await requestAnalyzeJs(server, fileType, 'formdata');

const {
issues: [issue],
} = JSON.parse(response);
} = JSON.parse(response.get('json'));
expect(issue).toEqual(
expect.objectContaining({
ruleId,
Expand Down Expand Up @@ -162,11 +162,15 @@ describe('server', () => {
});
});

async function requestAnalyzeJs(server: http.Server, fileType: string): Promise<any> {
async function requestAnalyzeJs(
server: http.Server,
fileType: string,
format: BridgeResponseType = 'text',
): Promise<any> {
const filePath = path.join(__dirname, 'fixtures', 'routing.js');
const analysisInput = { filePath, fileType };

return await request(server, '/analyze-js', 'POST', analysisInput);
return await request(server, '/analyze-js', 'POST', analysisInput, format);
}

function requestInitLinter(server: http.Server, fileType: string, ruleId: string) {
Expand Down
Loading
Loading