Skip to content

Commit

Permalink
JS-390 Refactor HTTP layer in BridgeServerImpl (#4899)
Browse files Browse the repository at this point in the history
  • Loading branch information
saberduck authored Nov 13, 2024
1 parent c25f660 commit 7ff98bd
Show file tree
Hide file tree
Showing 7 changed files with 164 additions and 106 deletions.
11 changes: 1 addition & 10 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,7 @@
<artifactId>slf4j-api</artifactId>
<version>${slf4j.version}</version>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
<version>5.4</version>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents.core5</groupId>
<artifactId>httpcore5</artifactId>
<version>5.3</version>
</dependency>


<!-- Test dependencies -->
<dependency>
Expand Down
8 changes: 0 additions & 8 deletions sonar-plugin/bridge/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,6 @@
<groupId>org.tukaani</groupId>
<artifactId>xz</artifactId>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents.client5</groupId>
<artifactId>httpclient5</artifactId>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents.core5</groupId>
<artifactId>httpcore5</artifactId>
</dependency>

<dependency>
<groupId>com.google.protobuf</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,6 @@
import java.util.function.Consumer;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpPost;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.apache.hc.core5.http.io.entity.StringEntity;
import org.apache.hc.core5.util.Timeout;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sonar.api.SonarProduct;
Expand Down Expand Up @@ -99,6 +90,7 @@ private enum Status {
private static final int HEARTBEAT_INTERVAL_SECONDS = 5;
private final ScheduledExecutorService heartbeatService;
private ScheduledFuture<?> heartbeatFuture;
private final Http http;

// Used by pico container for dependency injection
public BridgeServerImpl(
Expand Down Expand Up @@ -128,6 +120,28 @@ public BridgeServerImpl(
NodeDeprecationWarning deprecationWarning,
TempFolder tempFolder,
EmbeddedNode embeddedNode
) {
this(
nodeCommandBuilder,
timeoutSeconds,
bundle,
rulesBundles,
deprecationWarning,
tempFolder,
embeddedNode,
Http.getJdkHttpClient()
);
}

public BridgeServerImpl(
NodeCommandBuilder nodeCommandBuilder,
int timeoutSeconds,
Bundle bundle,
RulesBundles rulesBundles,
NodeDeprecationWarning deprecationWarning,
TempFolder tempFolder,
EmbeddedNode embeddedNode,
Http http
) {
this.nodeCommandBuilder = nodeCommandBuilder;
this.timeoutSeconds = timeoutSeconds;
Expand All @@ -138,32 +152,9 @@ public BridgeServerImpl(
this.temporaryDeployLocation = tempFolder.newDir(BRIDGE_DEPLOY_LOCATION).toPath();
this.heartbeatService = Executors.newSingleThreadScheduledExecutor();
this.embeddedNode = embeddedNode;
silenceHttpClientLogs();
}

private static void silenceHttpClientLogs() {
setLoggerLevelToInfo("org.apache.hc");
}

/**
* This method sets the log level of the logger with the given name to INFO.
* It assumes that SLF4J is used as the logging facade and Logback as the logging implementation.
* Since we don't want to directly depend on logback, we use reflection to set the log level.
* @param loggerName
*/
private static void setLoggerLevelToInfo(String loggerName) {
try {
ClassLoader cl = BridgeServerImpl.class.getClassLoader();
Class<?> level = cl.loadClass("ch.qos.logback.classic.Level");
Logger logger = LoggerFactory.getLogger(loggerName);
Method setLevel = logger.getClass().getMethod("setLevel", level);
setLevel.invoke(logger, level.getField("INFO").get(null));
} catch (NoSuchFieldException | IllegalAccessException | ClassNotFoundException | NoSuchMethodException | InvocationTargetException e) {
LOG.info("Failed to set logger level to INFO for " + loggerName, e);
}
this.http = http;
}


void heartbeat() {
LOG.trace("Pinging the bridge server");
isAlive();
Expand Down Expand Up @@ -373,7 +364,7 @@ private void initLinter(
List<String> globals,
String baseDir,
List<String> exclusions
) throws IOException {
) {
InitLinterRequest initLinterRequest = new InitLinterRequest(
linterId,
rules,
Expand All @@ -397,59 +388,44 @@ public AnalysisResponse analyzeJavaScript(JsAnalysisRequest request) throws IOEx
}

@Override
public AnalysisResponse analyzeTypeScript(JsAnalysisRequest request) throws IOException {
public AnalysisResponse analyzeTypeScript(JsAnalysisRequest request) {
String json = GSON.toJson(request);
return response(request(json, "analyze-ts"), request.filePath());
}

@Override
public AnalysisResponse analyzeCss(CssAnalysisRequest request) throws IOException {
public AnalysisResponse analyzeCss(CssAnalysisRequest request) {
String json = GSON.toJson(request);
return response(request(json, "analyze-css"), request.filePath());
}

@Override
public AnalysisResponse analyzeYaml(JsAnalysisRequest request) throws IOException {
public AnalysisResponse analyzeYaml(JsAnalysisRequest request) {
String json = GSON.toJson(request);
return response(request(json, "analyze-yaml"), request.filePath());
}

@Override
public AnalysisResponse analyzeHtml(JsAnalysisRequest request) throws IOException {
public AnalysisResponse analyzeHtml(JsAnalysisRequest request) {
var json = GSON.toJson(request);
return response(request(json, "analyze-html"), request.filePath());
}

private BridgeResponse request(String json, String endpoint) throws IOException {
try (var httpclient = HttpClients.createDefault()) {

var config = RequestConfig.custom()
.setResponseTimeout(Timeout.ofSeconds(timeoutSeconds))
.build();

HttpPost httpPost = new HttpPost(url(endpoint));
httpPost.setEntity(new StringEntity(json, ContentType.APPLICATION_JSON));
httpPost.setConfig(config);

return httpclient.execute(httpPost, response -> {
var contentTypeHeader = response.getHeader("Content-Type");
var responseBody = EntityUtils.toByteArray(response.getEntity());
if (isFormData(contentTypeHeader)) {
return FormDataUtils.parseFormData(contentTypeHeader.toString(), responseBody);
} else {
return new BridgeResponse(new String(responseBody, StandardCharsets.UTF_8));
}
});
private BridgeResponse request(String json, String endpoint) {
try {
var response = http.post(json, url(endpoint), timeoutSeconds);
if (isFormData(response.contentType())) {
return FormDataUtils.parseFormData(response.contentType(), response.body());
} else {
return new BridgeServer.BridgeResponse(new String(response.body(), StandardCharsets.UTF_8));
}
} catch (IOException e) {
throw new IllegalStateException("The bridge server is unresponsive", e);
}
}

private static boolean isFormData(@Nullable Header contentTypeHeader) {
if (contentTypeHeader == null) {
return false;
}
return contentTypeHeader.toString().contains("multipart/form-data");
private static boolean isFormData(@Nullable String contentTypeHeader) {
return contentTypeHeader != null && contentTypeHeader.contains("multipart/form-data");
}

private static AnalysisResponse response(BridgeResponse result, String filePath) {
Expand All @@ -470,9 +446,8 @@ public boolean isAlive() {
if (nodeCommand == null && status != Status.STARTED) {
return false;
}
try (var client = HttpClients.custom().build()) {
var get = new HttpGet(url("status"));
var res = client.execute(get, response -> EntityUtils.toString(response.getEntity()));
try {
String res = http.get(url("status"));
return "OK!".equals(res);
} catch (IOException e) {
return false;
Expand All @@ -481,13 +456,8 @@ public boolean isAlive() {

@Override
public boolean newTsConfig() {
try {
var response = request("", "new-tsconfig").json();
return "OK!".equals(response);
} catch (IOException e) {
LOG.error("Failed to post new-tsconfig", e);
}
return false;
var response = request("", "new-tsconfig").json();
return "OK!".equals(response);
}

TsConfigResponse tsConfigFiles(String tsconfigAbsolutePath) {
Expand All @@ -496,8 +466,6 @@ TsConfigResponse tsConfigFiles(String tsconfigAbsolutePath) {
TsConfigRequest tsConfigRequest = new TsConfigRequest(tsconfigAbsolutePath);
result = request(GSON.toJson(tsConfigRequest), "tsconfig-files").json();
return GSON.fromJson(result, TsConfigResponse.class);
} catch (IOException e) {
LOG.error("Failed to request files for tsconfig: " + tsconfigAbsolutePath, e);
} catch (JsonSyntaxException e) {
LOG.error(
"Failed to parse response when requesting files for tsconfig: {}: \n-----\n{}\n-----\n{}", tsconfigAbsolutePath, result, e.getMessage()
Expand All @@ -521,20 +489,20 @@ public TsConfigFile loadTsConfig(String filename) {
}

@Override
public TsProgram createProgram(TsProgramRequest tsProgramRequest) throws IOException {
public TsProgram createProgram(TsProgramRequest tsProgramRequest) {
var response = request(GSON.toJson(tsProgramRequest), "create-program");
return GSON.fromJson(response.json(), TsProgram.class);
}

@Override
public boolean deleteProgram(TsProgram tsProgram) throws IOException {
public boolean deleteProgram(TsProgram tsProgram) {
var programToDelete = new TsProgram(tsProgram.programId(), null, null);
var response = request(GSON.toJson(programToDelete), "delete-program").json();
return "OK!".equals(response);
}

@Override
public TsConfigFile createTsConfigFile(String content) throws IOException {
public TsConfigFile createTsConfigFile(String content) {
var response = request(content, "create-tsconfig-file");
return GSON.fromJson(response.json(), TsConfigFile.class);
}
Expand All @@ -548,11 +516,7 @@ public void clean() {
LOG.trace("Closing heartbeat service");
heartbeatService.shutdownNow();
if (nodeCommand != null && isAlive()) {
try {
request("", "close");
} catch (IOException e) {
LOG.warn("Failed to close the bridge server", e);
}
request("", "close");
nodeCommand.waitFor();
nodeCommand = null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2024 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.plugins.javascript.bridge;

import java.io.IOException;
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.time.Duration;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public interface Http {

static Http getJdkHttpClient() {
return new JdkHttp();
}

Response post(String json, URI uri, long timeoutSeconds) throws IOException;

String get(URI uri) throws IOException;

record Response(@Nullable String contentType, byte[] body) {}

class JdkHttp implements Http {

private static final Logger LOG = LoggerFactory.getLogger(JdkHttp.class);
private final HttpClient client;

JdkHttp() {
this.client = HttpClient.newBuilder().build();
}

@Override
public Response post(String json, URI uri, long timeoutSeconds) throws IOException {
var request = HttpRequest
.newBuilder()
.uri(uri)
.timeout(Duration.ofSeconds(timeoutSeconds))
.header("Content-Type", "application/json")
.POST(HttpRequest.BodyPublishers.ofString(json))
.build();

try {
var response = client.send(request, HttpResponse.BodyHandlers.ofByteArray());
var contentType = response.headers().firstValue("Content-Type")
.orElse(null);
return new Response(contentType, response.body());
} catch (InterruptedException e) {
throw handleInterruptedException(e, "Request " + uri + " was interrupted.");
}
}

@Override
public String get(URI uri) throws IOException{
var request = HttpRequest.newBuilder(uri).GET().build();
try {
var response = client.send(request, HttpResponse.BodyHandlers.ofString());
return response.body();
} catch (InterruptedException e) {
throw handleInterruptedException(e, "isAlive was interrupted");
}
}

private static IllegalStateException handleInterruptedException(
InterruptedException e,
String msg
) {
LOG.error(msg, e);
Thread.currentThread().interrupt();
return new IllegalStateException(msg, e);
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ protected void analyzeFile(InputFile file, @Nullable List<String> tsConfigs, @Nu
file
);
acceptAstResponse(response, file);
} catch (IOException e) {
} catch (Exception e) {
LOG.error("Failed to get response while analyzing " + file.uri(), e);
throw e;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ void should_analyse() throws Exception {
@Test
void should_explode_if_no_response() throws Exception {
createVueInputFile();
when(bridgeServerMock.analyzeTypeScript(any())).thenThrow(new IOException("error"));
when(bridgeServerMock.analyzeTypeScript(any())).thenThrow(new IllegalStateException("error"));

var tsProgram = new TsProgram("1", List.of(), List.of());
when(bridgeServerMock.createProgram(any())).thenReturn(tsProgram);
Expand Down Expand Up @@ -778,7 +778,7 @@ void should_stop_when_no_input_files() throws Exception {
@Test
void should_fail_fast() throws Exception {
createTsConfigFile();
when(bridgeServerMock.analyzeTypeScript(any())).thenThrow(new IOException("error"));
when(bridgeServerMock.analyzeTypeScript(any())).thenThrow(new IllegalStateException("error"));
JsTsSensor sensor = createSensor();
createInputFile(context);

Expand Down
Loading

0 comments on commit 7ff98bd

Please sign in to comment.