From 2082c24dc6c79eaebcca804a2fffb7e02c355479 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Mon, 25 Mar 2024 12:25:36 +0100 Subject: [PATCH 01/13] Skip embedded Node.js runtime deployment if 'sonar.nodejs.executable' is set --- .../plugins/javascript/bridge/EmbeddedNode.java | 10 ++++++++++ .../plugins/javascript/bridge/EmbeddedNodeTest.java | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java index 255f372661a..03cac4926cb 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java @@ -170,6 +170,11 @@ public void deploy() throws IOException { if (platform == UNSUPPORTED) { return; } + if (isNodejsExecutableSet()) { + LOG.info("'sonar.nodejs.executable' is set. Skipping embedded Node.js runtime deployment."); + return; + } + try { var is = getClass().getResourceAsStream(platform.archivePathInJar()); if (is == null) { @@ -197,6 +202,11 @@ public void deploy() throws IOException { } } + private static boolean isNodejsExecutableSet() { + var nodeJsExecutable = System.getProperty("sonar.nodejs.executable"); + return nodeJsExecutable != null; + } + private static boolean isDifferent(InputStream newVersionIs, Path currentVersionPath) throws IOException { var newVersionString = new String(newVersionIs.readAllBytes(), StandardCharsets.UTF_8); diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java index be7ef4d141f..cb00ede646c 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java @@ -93,6 +93,19 @@ void should_extract_if_deployLocation_has_no_version() throws Exception { assertThat(tempDir.resolve(en.binary())).exists(); } + @Test + void should_do_nothing_if_sonar_nodejs_executable_is_set() throws Exception { + var NODEJS_EXECUTABLE_KEY = "sonar.nodejs.executable"; + var tmp = System.getProperty(NODEJS_EXECUTABLE_KEY); + System.setProperty(NODEJS_EXECUTABLE_KEY, "true"); + var en = testEmbeddedNode(); + en.deploy(); + assertThat(en.binary()).doesNotExist(); + if (tmp != null) { + System.setProperty(NODEJS_EXECUTABLE_KEY, tmp); + } + } + @Test void should_detect_platform_for_windows_environment() { var platform = Platform.detect(createWindowsEnvironment()); From be968a1bbb3bd0e70a49bcc07d8860b43eead96c Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Mon, 25 Mar 2024 12:40:03 +0100 Subject: [PATCH 02/13] clear prop in tests --- .../org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java index cb00ede646c..d0bb9e421c1 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java @@ -103,6 +103,8 @@ void should_do_nothing_if_sonar_nodejs_executable_is_set() throws Exception { assertThat(en.binary()).doesNotExist(); if (tmp != null) { System.setProperty(NODEJS_EXECUTABLE_KEY, tmp); + } else { + System.clearProperty(NODEJS_EXECUTABLE_KEY); } } From a833f7b5b71cda27f79f08be4b72fdd5091e41de Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Mon, 25 Mar 2024 13:35:58 +0100 Subject: [PATCH 03/13] refactor to use configuration instead of system.properties --- .../javascript/bridge/BridgeServerImpl.java | 7 ++- .../javascript/bridge/EmbeddedNode.java | 13 +++-- .../nodejs/NodeCommandBuilderImpl.java | 3 +- .../bridge/BridgeServerImplTest.java | 56 +++++++++---------- .../javascript/bridge/EmbeddedNodeTest.java | 31 +++++----- .../javascript/nodejs/NodeCommandTest.java | 4 +- 6 files changed, 62 insertions(+), 52 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java index eb0ed3e0ff0..f4a38ed7553 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java @@ -47,6 +47,7 @@ import javax.annotation.Nullable; import org.sonar.api.SonarProduct; import org.sonar.api.batch.sensor.SensorContext; +import org.sonar.api.config.Configuration; import org.sonar.api.utils.TempFolder; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -162,9 +163,9 @@ int getTimeoutSeconds() { * * @throws IOException */ - void deploy() throws IOException { + void deploy(Configuration configuration) throws IOException { bundle.deploy(temporaryDeployLocation); - embeddedNode.deploy(); + embeddedNode.deploy(configuration); } void startServer(SensorContext context, List deployedBundles) throws IOException { @@ -285,7 +286,7 @@ public void startServerLazily(SensorContext context) throws IOException { status = Status.FAILED; throw new ServerAlreadyFailedException(); } - deploy(); + deploy(context.config()); List deployedBundles = rulesBundles.deploy(temporaryDeployLocation.resolve("package")); rulesBundles .getUcfgRulesBundle() diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java index 03cac4926cb..a8c7ce24649 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java @@ -35,6 +35,8 @@ import java.nio.file.Path; import java.util.Locale; import java.util.Set; +import javax.annotation.Nullable; +import org.sonar.api.config.Configuration; import org.sonar.api.scanner.ScannerSide; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -159,7 +161,7 @@ public boolean isAvailable() { * * @throws IOException */ - public void deploy() throws IOException { + public void deploy(@Nullable Configuration configuration) throws IOException { LOG.info( "Detected os: {} arch: {} alpine: {}. Platform: {}", env.getOsName(), @@ -170,7 +172,7 @@ public void deploy() throws IOException { if (platform == UNSUPPORTED) { return; } - if (isNodejsExecutableSet()) { + if (isNodejsExecutableSet(configuration)) { LOG.info("'sonar.nodejs.executable' is set. Skipping embedded Node.js runtime deployment."); return; } @@ -202,8 +204,11 @@ public void deploy() throws IOException { } } - private static boolean isNodejsExecutableSet() { - var nodeJsExecutable = System.getProperty("sonar.nodejs.executable"); + private static boolean isNodejsExecutableSet(Configuration configuration) { + if (configuration == null) { + return false; + } + var nodeJsExecutable = configuration.get("sonar.nodejs.executable").get(); return nodeJsExecutable != null; } diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java index 41aa2fa0bd7..ee6f8004089 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java @@ -34,6 +34,7 @@ import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.annotation.Nullable; import org.sonar.api.config.Configuration; import org.sonar.api.utils.Version; import org.sonar.api.utils.log.Logger; @@ -220,7 +221,7 @@ static Version nodeVersion(String versionString) throws NodeCommandException { * @throws NodeCommandException * @throws IOException */ - private String retrieveNodeExecutable(Configuration configuration) + private String retrieveNodeExecutable(@Nullable Configuration configuration) throws NodeCommandException, IOException { if (configuration.hasKey(NODE_EXECUTABLE_PROPERTY)) { String nodeExecutable = configuration.get(NODE_EXECUTABLE_PROPERTY).get(); diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java index fab94db679f..626ac135725 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java @@ -126,7 +126,7 @@ public void tearDown() { @Test void should_throw_when_not_existing_script() throws Exception { bridgeServer = createBridgeServer("NOT_EXISTING.js"); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); List deployedBundles = emptyList(); assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles)) @@ -157,7 +157,7 @@ void should_throw_if_failed_to_build_node_command() throws Exception { tempFolder, unsupportedEmbeddedRuntime ); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); List deployedBundles = emptyList(); assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles)) @@ -168,7 +168,7 @@ void should_throw_if_failed_to_build_node_command() throws Exception { @Test void should_forward_process_streams() throws Exception { bridgeServer = createBridgeServer("logging.js"); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThat(logTester.logs(DEBUG)).contains("testing debug log"); @@ -180,7 +180,7 @@ void should_forward_process_streams() throws Exception { @Test void should_get_answer_from_server() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); DefaultInputFile inputFile = TestInputFileBuilder @@ -194,7 +194,7 @@ void should_get_answer_from_server() throws Exception { @Test void test_init() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); List rules = Collections.singletonList( @@ -223,7 +223,7 @@ void test_init() throws Exception { @Test void should_get_answer_from_server_for_ts_request() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); DefaultInputFile inputFile = TestInputFileBuilder @@ -250,7 +250,7 @@ void should_get_answer_from_server_for_ts_request() throws Exception { @Test void should_get_answer_from_server_for_yaml_request() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); DefaultInputFile inputFile = TestInputFileBuilder @@ -278,7 +278,7 @@ private static JsAnalysisRequest createRequest(DefaultInputFile inputFile) { @Test void should_get_answer_from_server_for_program_based_requests() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); TsProgram programCreated = bridgeServer.createProgram( @@ -308,7 +308,7 @@ void should_get_answer_from_server_for_program_based_requests() throws Exception @Test void should_create_tsconfig_files() throws IOException { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); var tsConfig = bridgeServer.createTsConfigFile("{\"include\":[\"/path/to/project/**/*\"]}"); @@ -318,7 +318,7 @@ void should_create_tsconfig_files() throws IOException { @Test void should_not_fail_when_error_during_create_program() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); TsProgram programCreated = bridgeServer.createProgram( @@ -332,7 +332,7 @@ void should_not_fail_when_error_during_create_program() throws Exception { @Test void should_get_answer_from_server_for_css_request() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); DefaultInputFile inputFile = TestInputFileBuilder @@ -350,7 +350,7 @@ void should_get_answer_from_server_for_css_request() throws Exception { @Test void should_throw_if_failed_to_start() throws Exception { bridgeServer = createBridgeServer("throw.js"); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); List deployedBundles = emptyList(); assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles)) @@ -364,7 +364,7 @@ void should_return_command_info() throws Exception { assertThat(bridgeServer.getCommandInfo()) .isEqualTo("Node.js command to start the bridge server was not built yet."); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThat(bridgeServer.getCommandInfo()) @@ -378,7 +378,7 @@ void should_set_max_old_space_size() throws Exception { assertThat(bridgeServer.getCommandInfo()) .isEqualTo("Node.js command to start the bridge server was not built yet."); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); context.setSettings(new MapSettings().setProperty("sonar.javascript.node.maxspace", 2048)); bridgeServer.startServer(context, emptyList()); @@ -388,7 +388,7 @@ void should_set_max_old_space_size() throws Exception { @Test void should_set_allowTsParserJsFiles_to_false() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); context.setSettings( new MapSettings().setProperty("sonar.javascript.allowTsParserJsFiles", "false") ); @@ -401,7 +401,7 @@ void should_set_allowTsParserJsFiles_to_false() throws Exception { @Test void allowTsParserJsFiles_default_value_is_true() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); bridgeServer.stop(); @@ -510,7 +510,7 @@ void should_throw_if_server_not_alive() throws Exception { @Test void should_fail_if_bad_json_response() throws Exception { bridgeServer = createBridgeServer("badResponse.js"); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServerLazily(context); DefaultInputFile inputFile = TestInputFileBuilder @@ -535,7 +535,7 @@ void should_fail_if_bad_json_response() throws Exception { @Test void should_not_search_typescript_when_no_ts_file() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); SensorContextTester ctx = SensorContextTester.create(moduleBase); ctx.fileSystem().setWorkDir(workDir); Path tsDir = moduleBase.resolve("dir/node_modules/typescript"); @@ -547,7 +547,7 @@ void should_not_search_typescript_when_no_ts_file() throws Exception { @Test void should_reload_tsconfig() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThat(bridgeServer.newTsConfig()).isTrue(); } @@ -555,7 +555,7 @@ void should_reload_tsconfig() throws Exception { @Test void should_return_files_for_tsconfig() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); String tsconfig = "path/to/tsconfig.json"; BridgeServerImpl.TsConfigResponse tsConfigResponse = bridgeServer.tsConfigFiles(tsconfig); @@ -571,7 +571,7 @@ void should_return_files_for_tsconfig() throws Exception { @Test void should_return_no_files_for_tsconfig_bad_response() throws Exception { bridgeServer = createBridgeServer("badResponse.js"); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); BridgeServerImpl.TsConfigResponse response = bridgeServer.tsConfigFiles( "path/to/tsconfig.json" @@ -583,7 +583,7 @@ void should_return_no_files_for_tsconfig_bad_response() throws Exception { @Test void should_return_no_files_for_tsconfig_no_response() throws Exception { bridgeServer = createBridgeServer("badResponse.js"); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThat(bridgeServer.tsConfigFiles("path/to/tsconfig.json").files).isEmpty(); TsConfigFile tsConfigFile = bridgeServer.loadTsConfig("path/to/tsconfig.json"); @@ -593,7 +593,7 @@ void should_return_no_files_for_tsconfig_no_response() throws Exception { @Test void should_return_no_files_for_tsconfig_on_error() throws Exception { bridgeServer = createBridgeServer("tsConfigError.js"); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); TsConfigFile tsConfigFile = bridgeServer.loadTsConfig("path/to/tsconfig.json"); @@ -604,7 +604,7 @@ void should_return_no_files_for_tsconfig_on_error() throws Exception { @Test void log_error_when_timeout() throws Exception { bridgeServer = createBridgeServer("timeout.js"); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThatThrownBy(() -> bridgeServer.loadTsConfig("any.ts")) @@ -626,7 +626,7 @@ void test_rule_tostring() { @Test void should_load_custom_rules() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2"))); bridgeServer.stop(); @@ -637,7 +637,7 @@ void should_load_custom_rules() throws Exception { @Test void should_skip_metrics_on_sonarlint() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); context.setRuntime(SonarRuntimeImpl.forSonarLint(Version.create(7, 9))); bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2"))); bridgeServer.stop(); @@ -648,7 +648,7 @@ void should_skip_metrics_on_sonarlint() throws Exception { @Test void should_pass_debug_memory_option() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); context.setSettings(new MapSettings().setProperty("sonar.javascript.node.debugMemory", "true")); bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2"))); bridgeServer.stop(); @@ -715,7 +715,7 @@ void enabled_monitoring() throws Exception { tempFolder, unsupportedEmbeddedRuntime ); - bridgeServer.deploy(); + bridgeServer.deploy(context.config()); bridgeServer.startServerLazily(context); bridgeServer.stop(); assertThat(logTester.logs(INFO).stream().anyMatch(s -> s.startsWith("no-commented-code"))) diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java index d0bb9e421c1..18e747cbeb9 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java @@ -37,7 +37,10 @@ import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; import org.slf4j.event.Level; +import org.sonar.api.config.Configuration; +import org.sonar.api.config.internal.ConfigurationBridge; import org.sonar.api.config.internal.MapSettings; +import org.sonar.api.config.internal.Settings; import org.sonar.api.testfixtures.log.LogTesterJUnit5; import org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform; import org.sonar.plugins.javascript.nodejs.ProcessWrapper; @@ -59,7 +62,7 @@ void should_extract_if_deployLocation_contains_a_different_version() throws Exce var runtimeFolder = en.binary().getParent(); Files.createDirectories(runtimeFolder); Files.write(runtimeFolder.resolve("version.txt"), "a-different-version".getBytes()); - en.deploy(); + en.deploy(null); assertThat(en.binary()).exists(); assertThat(en.isAvailable()).isTrue(); } @@ -68,12 +71,12 @@ void should_extract_if_deployLocation_contains_a_different_version() throws Exce void should_not_extract_if_deployLocation_contains_the_same_version() throws Exception { logTester.setLevel(Level.DEBUG); var en = testEmbeddedNode(); - en.deploy(); + en.deploy(null); assertThat(logTester.logs()).anyMatch(l -> l.startsWith("Extracting embedded node")); logTester.clear(); en = testEmbeddedNode(); assertThat(en.isAvailable()).isFalse(); - en.deploy(); + en.deploy(null); assertThat(logTester.logs()).anyMatch(l -> l.startsWith("Skipping node deploy.")); assertThat(en.isAvailable()).isTrue(); } @@ -81,7 +84,7 @@ void should_not_extract_if_deployLocation_contains_the_same_version() throws Exc @Test void should_not_extract_neither_be_available_if_the_platform_is_unsupported() throws Exception { var en = new EmbeddedNode(mock(ProcessWrapper.class), createUnsupportedEnvironment()); - en.deploy(); + en.deploy(null); assertThat(en.binary()).doesNotExist(); assertThat(en.isAvailable()).isFalse(); } @@ -89,23 +92,19 @@ void should_not_extract_neither_be_available_if_the_platform_is_unsupported() th @Test void should_extract_if_deployLocation_has_no_version() throws Exception { var en = testEmbeddedNode(); - en.deploy(); + en.deploy(null); assertThat(tempDir.resolve(en.binary())).exists(); } @Test void should_do_nothing_if_sonar_nodejs_executable_is_set() throws Exception { var NODEJS_EXECUTABLE_KEY = "sonar.nodejs.executable"; - var tmp = System.getProperty(NODEJS_EXECUTABLE_KEY); - System.setProperty(NODEJS_EXECUTABLE_KEY, "true"); + var settings = new MapSettings(); + settings.setProperty(NODEJS_EXECUTABLE_KEY, "true"); + var config = getConfig(settings); var en = testEmbeddedNode(); - en.deploy(); + en.deploy(config); assertThat(en.binary()).doesNotExist(); - if (tmp != null) { - System.setProperty(NODEJS_EXECUTABLE_KEY, tmp); - } else { - System.clearProperty(NODEJS_EXECUTABLE_KEY); - } } @Test @@ -170,7 +169,7 @@ void should_fail_gracefully() throws Exception { when(processWrapper.waitFor(any(), anyLong(), any())) .thenThrow(new IllegalStateException("My Error")); var en = new EmbeddedNode(processWrapper, createTestEnvironment()); - en.deploy(); + en.deploy(null); assertThat(logTester.logs()) .anyMatch(l -> l.startsWith("Embedded Node.js failed to deploy. Will fallback to host Node.js") @@ -218,4 +217,8 @@ private Environment createUnsupportedEnvironment() { when(mockEnvironment.getOsArch()).thenReturn(""); return mockEnvironment; } + + private Configuration getConfig(Settings settings) { + return new ConfigurationBridge(settings); + } } diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/nodejs/NodeCommandTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/nodejs/NodeCommandTest.java index 919ba8f0f8c..85bc109ee79 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/nodejs/NodeCommandTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/nodejs/NodeCommandTest.java @@ -398,7 +398,7 @@ void test_windows_default_node_not_found() throws Exception { @Test void test_embedded_runtime() throws Exception { var en = new EmbeddedNode(new ProcessWrapperImpl(), createTestEnvironment()); - en.deploy(); + en.deploy(null); NodeCommand nodeCommand = builder() .script(PATH_TO_SCRIPT) .pathResolver(getPathResolver()) @@ -421,7 +421,7 @@ void test_embedded_runtime_with_forceHost_for_macos() throws Exception { Configuration configuration = mapSettings.asConfig(); var en = new EmbeddedNode(mockProcessWrapper, createTestEnvironment()); - en.deploy(); + en.deploy(null); NodeCommand nodeCommand = builder() .script(PATH_TO_SCRIPT) .configuration(configuration) From 3e15b497db6f315e2cca04ab94e9fe7739ec64b5 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Mon, 25 Mar 2024 13:38:23 +0100 Subject: [PATCH 04/13] clenaup --- .../org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java index 18e747cbeb9..47c2c4ffc59 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java @@ -98,9 +98,8 @@ void should_extract_if_deployLocation_has_no_version() throws Exception { @Test void should_do_nothing_if_sonar_nodejs_executable_is_set() throws Exception { - var NODEJS_EXECUTABLE_KEY = "sonar.nodejs.executable"; var settings = new MapSettings(); - settings.setProperty(NODEJS_EXECUTABLE_KEY, "true"); + settings.setProperty("sonar.nodejs.executable", "true"); var config = getConfig(settings); var en = testEmbeddedNode(); en.deploy(config); From e74bfb46a7d7d250415820eab747ffa8b21bc7ba Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Mon, 25 Mar 2024 14:09:07 +0100 Subject: [PATCH 05/13] cleanup --- .../org/sonar/plugins/javascript/bridge/EmbeddedNode.java | 5 ++--- .../plugins/javascript/nodejs/NodeCommandBuilderImpl.java | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java index a8c7ce24649..a8c2caaa36f 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java @@ -204,12 +204,11 @@ public void deploy(@Nullable Configuration configuration) throws IOException { } } - private static boolean isNodejsExecutableSet(Configuration configuration) { + private static boolean isNodejsExecutableSet(@Nullable Configuration configuration) { if (configuration == null) { return false; } - var nodeJsExecutable = configuration.get("sonar.nodejs.executable").get(); - return nodeJsExecutable != null; + return configuration.get("sonar.nodejs.executable").isPresent(); } private static boolean isDifferent(InputStream newVersionIs, Path currentVersionPath) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java index ee6f8004089..41aa2fa0bd7 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java @@ -34,7 +34,6 @@ import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.annotation.Nullable; import org.sonar.api.config.Configuration; import org.sonar.api.utils.Version; import org.sonar.api.utils.log.Logger; @@ -221,7 +220,7 @@ static Version nodeVersion(String versionString) throws NodeCommandException { * @throws NodeCommandException * @throws IOException */ - private String retrieveNodeExecutable(@Nullable Configuration configuration) + private String retrieveNodeExecutable(Configuration configuration) throws NodeCommandException, IOException { if (configuration.hasKey(NODE_EXECUTABLE_PROPERTY)) { String nodeExecutable = configuration.get(NODE_EXECUTABLE_PROPERTY).get(); From ac43fb1e83bcdd7d35c96418a91b94da289ba3c8 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Mon, 25 Mar 2024 14:13:33 +0100 Subject: [PATCH 06/13] cleanup --- .../org/sonar/plugins/javascript/bridge/EmbeddedNode.java | 8 ++++++-- .../plugins/javascript/nodejs/NodeCommandBuilderImpl.java | 2 +- .../sonar/plugins/javascript/bridge/EmbeddedNodeTest.java | 3 ++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java index a8c2caaa36f..73972069475 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java @@ -24,6 +24,7 @@ import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.UNSUPPORTED; +import static org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl.NODE_EXECUTABLE_PROPERTY; import static org.sonarsource.api.sonarlint.SonarLintSide.INSTANCE; import java.io.BufferedInputStream; @@ -173,7 +174,10 @@ public void deploy(@Nullable Configuration configuration) throws IOException { return; } if (isNodejsExecutableSet(configuration)) { - LOG.info("'sonar.nodejs.executable' is set. Skipping embedded Node.js runtime deployment."); + LOG.info( + "'{}' is set. Skipping embedded Node.js runtime deployment.", + NODE_EXECUTABLE_PROPERTY + ); return; } @@ -208,7 +212,7 @@ private static boolean isNodejsExecutableSet(@Nullable Configuration configurati if (configuration == null) { return false; } - return configuration.get("sonar.nodejs.executable").isPresent(); + return configuration.get(NODE_EXECUTABLE_PROPERTY).isPresent(); } private static boolean isDifferent(InputStream newVersionIs, Path currentVersionPath) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java index 41aa2fa0bd7..5679e706855 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/nodejs/NodeCommandBuilderImpl.java @@ -48,7 +48,7 @@ public class NodeCommandBuilderImpl implements NodeCommandBuilder { private static final String NODE_EXECUTABLE_DEFAULT_MACOS = "package/node_modules/run-node/run-node"; - private static final String NODE_EXECUTABLE_PROPERTY = "sonar.nodejs.executable"; + public static final String NODE_EXECUTABLE_PROPERTY = "sonar.nodejs.executable"; private static final String NODE_FORCE_HOST_PROPERTY = "sonar.nodejs.forceHost"; private static final Pattern NODEJS_VERSION_PATTERN = Pattern.compile( diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java index 47c2c4ffc59..16a2ee6bacd 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java @@ -29,6 +29,7 @@ import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.LINUX_X64; import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.UNSUPPORTED; import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.WIN_X64; +import static org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl.NODE_EXECUTABLE_PROPERTY; import java.nio.file.Files; import java.nio.file.Path; @@ -99,7 +100,7 @@ void should_extract_if_deployLocation_has_no_version() throws Exception { @Test void should_do_nothing_if_sonar_nodejs_executable_is_set() throws Exception { var settings = new MapSettings(); - settings.setProperty("sonar.nodejs.executable", "true"); + settings.setProperty(NODE_EXECUTABLE_PROPERTY, "true"); var config = getConfig(settings); var en = testEmbeddedNode(); en.deploy(config); From 531c6dd6617214d8aee11fd4c82775b630558420 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Mon, 25 Mar 2024 17:12:59 +0100 Subject: [PATCH 07/13] cleanup redundant bridgeServer.deploy() calls --- .../bridge/BridgeServerImplTest.java | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java index 626ac135725..473c180ee72 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java @@ -126,7 +126,6 @@ public void tearDown() { @Test void should_throw_when_not_existing_script() throws Exception { bridgeServer = createBridgeServer("NOT_EXISTING.js"); - bridgeServer.deploy(context.config()); List deployedBundles = emptyList(); assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles)) @@ -157,7 +156,6 @@ void should_throw_if_failed_to_build_node_command() throws Exception { tempFolder, unsupportedEmbeddedRuntime ); - bridgeServer.deploy(context.config()); List deployedBundles = emptyList(); assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles)) @@ -168,7 +166,6 @@ void should_throw_if_failed_to_build_node_command() throws Exception { @Test void should_forward_process_streams() throws Exception { bridgeServer = createBridgeServer("logging.js"); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThat(logTester.logs(DEBUG)).contains("testing debug log"); @@ -180,7 +177,6 @@ void should_forward_process_streams() throws Exception { @Test void should_get_answer_from_server() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); DefaultInputFile inputFile = TestInputFileBuilder @@ -194,7 +190,6 @@ void should_get_answer_from_server() throws Exception { @Test void test_init() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); List rules = Collections.singletonList( @@ -223,7 +218,6 @@ void test_init() throws Exception { @Test void should_get_answer_from_server_for_ts_request() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); DefaultInputFile inputFile = TestInputFileBuilder @@ -250,7 +244,6 @@ void should_get_answer_from_server_for_ts_request() throws Exception { @Test void should_get_answer_from_server_for_yaml_request() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); DefaultInputFile inputFile = TestInputFileBuilder @@ -278,7 +271,6 @@ private static JsAnalysisRequest createRequest(DefaultInputFile inputFile) { @Test void should_get_answer_from_server_for_program_based_requests() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); TsProgram programCreated = bridgeServer.createProgram( @@ -308,7 +300,6 @@ void should_get_answer_from_server_for_program_based_requests() throws Exception @Test void should_create_tsconfig_files() throws IOException { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); var tsConfig = bridgeServer.createTsConfigFile("{\"include\":[\"/path/to/project/**/*\"]}"); @@ -318,7 +309,6 @@ void should_create_tsconfig_files() throws IOException { @Test void should_not_fail_when_error_during_create_program() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); TsProgram programCreated = bridgeServer.createProgram( @@ -332,7 +322,6 @@ void should_not_fail_when_error_during_create_program() throws Exception { @Test void should_get_answer_from_server_for_css_request() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); DefaultInputFile inputFile = TestInputFileBuilder @@ -350,7 +339,6 @@ void should_get_answer_from_server_for_css_request() throws Exception { @Test void should_throw_if_failed_to_start() throws Exception { bridgeServer = createBridgeServer("throw.js"); - bridgeServer.deploy(context.config()); List deployedBundles = emptyList(); assertThatThrownBy(() -> bridgeServer.startServer(context, deployedBundles)) @@ -364,7 +352,6 @@ void should_return_command_info() throws Exception { assertThat(bridgeServer.getCommandInfo()) .isEqualTo("Node.js command to start the bridge server was not built yet."); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThat(bridgeServer.getCommandInfo()) @@ -378,7 +365,6 @@ void should_set_max_old_space_size() throws Exception { assertThat(bridgeServer.getCommandInfo()) .isEqualTo("Node.js command to start the bridge server was not built yet."); - bridgeServer.deploy(context.config()); context.setSettings(new MapSettings().setProperty("sonar.javascript.node.maxspace", 2048)); bridgeServer.startServer(context, emptyList()); @@ -388,7 +374,6 @@ void should_set_max_old_space_size() throws Exception { @Test void should_set_allowTsParserJsFiles_to_false() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); context.setSettings( new MapSettings().setProperty("sonar.javascript.allowTsParserJsFiles", "false") ); @@ -401,7 +386,6 @@ void should_set_allowTsParserJsFiles_to_false() throws Exception { @Test void allowTsParserJsFiles_default_value_is_true() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); bridgeServer.stop(); @@ -510,7 +494,6 @@ void should_throw_if_server_not_alive() throws Exception { @Test void should_fail_if_bad_json_response() throws Exception { bridgeServer = createBridgeServer("badResponse.js"); - bridgeServer.deploy(context.config()); bridgeServer.startServerLazily(context); DefaultInputFile inputFile = TestInputFileBuilder @@ -535,7 +518,6 @@ void should_fail_if_bad_json_response() throws Exception { @Test void should_not_search_typescript_when_no_ts_file() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); SensorContextTester ctx = SensorContextTester.create(moduleBase); ctx.fileSystem().setWorkDir(workDir); Path tsDir = moduleBase.resolve("dir/node_modules/typescript"); @@ -547,7 +529,6 @@ void should_not_search_typescript_when_no_ts_file() throws Exception { @Test void should_reload_tsconfig() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThat(bridgeServer.newTsConfig()).isTrue(); } @@ -555,7 +536,6 @@ void should_reload_tsconfig() throws Exception { @Test void should_return_files_for_tsconfig() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); String tsconfig = "path/to/tsconfig.json"; BridgeServerImpl.TsConfigResponse tsConfigResponse = bridgeServer.tsConfigFiles(tsconfig); @@ -571,7 +551,6 @@ void should_return_files_for_tsconfig() throws Exception { @Test void should_return_no_files_for_tsconfig_bad_response() throws Exception { bridgeServer = createBridgeServer("badResponse.js"); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); BridgeServerImpl.TsConfigResponse response = bridgeServer.tsConfigFiles( "path/to/tsconfig.json" @@ -583,7 +562,6 @@ void should_return_no_files_for_tsconfig_bad_response() throws Exception { @Test void should_return_no_files_for_tsconfig_no_response() throws Exception { bridgeServer = createBridgeServer("badResponse.js"); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThat(bridgeServer.tsConfigFiles("path/to/tsconfig.json").files).isEmpty(); TsConfigFile tsConfigFile = bridgeServer.loadTsConfig("path/to/tsconfig.json"); @@ -593,7 +571,6 @@ void should_return_no_files_for_tsconfig_no_response() throws Exception { @Test void should_return_no_files_for_tsconfig_on_error() throws Exception { bridgeServer = createBridgeServer("tsConfigError.js"); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); TsConfigFile tsConfigFile = bridgeServer.loadTsConfig("path/to/tsconfig.json"); @@ -604,7 +581,6 @@ void should_return_no_files_for_tsconfig_on_error() throws Exception { @Test void log_error_when_timeout() throws Exception { bridgeServer = createBridgeServer("timeout.js"); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, emptyList()); assertThatThrownBy(() -> bridgeServer.loadTsConfig("any.ts")) @@ -626,7 +602,6 @@ void test_rule_tostring() { @Test void should_load_custom_rules() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2"))); bridgeServer.stop(); @@ -637,7 +612,6 @@ void should_load_custom_rules() throws Exception { @Test void should_skip_metrics_on_sonarlint() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); context.setRuntime(SonarRuntimeImpl.forSonarLint(Version.create(7, 9))); bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2"))); bridgeServer.stop(); @@ -648,7 +622,6 @@ void should_skip_metrics_on_sonarlint() throws Exception { @Test void should_pass_debug_memory_option() throws Exception { bridgeServer = createBridgeServer(START_SERVER_SCRIPT); - bridgeServer.deploy(context.config()); context.setSettings(new MapSettings().setProperty("sonar.javascript.node.debugMemory", "true")); bridgeServer.startServer(context, Arrays.asList(Paths.get("bundle1"), Paths.get("bundle2"))); bridgeServer.stop(); @@ -715,7 +688,6 @@ void enabled_monitoring() throws Exception { tempFolder, unsupportedEmbeddedRuntime ); - bridgeServer.deploy(context.config()); bridgeServer.startServerLazily(context); bridgeServer.stop(); assertThat(logTester.logs(INFO).stream().anyMatch(s -> s.startsWith("no-commented-code"))) From c848bd252ebd675471b3452c913aa0edaee0fb17 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Mon, 25 Mar 2024 18:33:16 +0100 Subject: [PATCH 08/13] Move logic to bridgeServer --- .../javascript/bridge/BridgeServerImpl.java | 10 +++++++- .../javascript/bridge/EmbeddedNode.java | 19 +-------------- .../bridge/BridgeServerImplTest.java | 21 +++++++++++++++++ .../javascript/bridge/EmbeddedNodeTest.java | 23 +++++-------------- .../javascript/nodejs/NodeCommandTest.java | 4 ++-- 5 files changed, 39 insertions(+), 38 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java index f4a38ed7553..c9864d964d5 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java @@ -21,6 +21,7 @@ import static java.util.Collections.emptyList; import static org.sonar.plugins.javascript.bridge.NetUtils.findOpenPort; +import static org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl.NODE_EXECUTABLE_PROPERTY; import com.google.gson.Gson; import com.google.gson.JsonSyntaxException; @@ -165,7 +166,14 @@ int getTimeoutSeconds() { */ void deploy(Configuration configuration) throws IOException { bundle.deploy(temporaryDeployLocation); - embeddedNode.deploy(configuration); + if (configuration.get(NODE_EXECUTABLE_PROPERTY).isPresent()) { + LOG.info( + "'{}' is set. Skipping embedded Node.js runtime deployment.", + NODE_EXECUTABLE_PROPERTY + ); + return; + } + embeddedNode.deploy(); } void startServer(SensorContext context, List deployedBundles) throws IOException { diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java index 73972069475..4bf847a2133 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java @@ -24,7 +24,6 @@ import static java.nio.file.attribute.PosixFilePermission.OWNER_READ; import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE; import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.UNSUPPORTED; -import static org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl.NODE_EXECUTABLE_PROPERTY; import static org.sonarsource.api.sonarlint.SonarLintSide.INSTANCE; import java.io.BufferedInputStream; @@ -36,8 +35,6 @@ import java.nio.file.Path; import java.util.Locale; import java.util.Set; -import javax.annotation.Nullable; -import org.sonar.api.config.Configuration; import org.sonar.api.scanner.ScannerSide; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -162,7 +159,7 @@ public boolean isAvailable() { * * @throws IOException */ - public void deploy(@Nullable Configuration configuration) throws IOException { + public void deploy() throws IOException { LOG.info( "Detected os: {} arch: {} alpine: {}. Platform: {}", env.getOsName(), @@ -173,13 +170,6 @@ public void deploy(@Nullable Configuration configuration) throws IOException { if (platform == UNSUPPORTED) { return; } - if (isNodejsExecutableSet(configuration)) { - LOG.info( - "'{}' is set. Skipping embedded Node.js runtime deployment.", - NODE_EXECUTABLE_PROPERTY - ); - return; - } try { var is = getClass().getResourceAsStream(platform.archivePathInJar()); @@ -208,13 +198,6 @@ public void deploy(@Nullable Configuration configuration) throws IOException { } } - private static boolean isNodejsExecutableSet(@Nullable Configuration configuration) { - if (configuration == null) { - return false; - } - return configuration.get(NODE_EXECUTABLE_PROPERTY).isPresent(); - } - private static boolean isDifferent(InputStream newVersionIs, Path currentVersionPath) throws IOException { var newVersionString = new String(newVersionIs.readAllBytes(), StandardCharsets.UTF_8); diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java index 473c180ee72..478a27b2c8f 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java @@ -33,6 +33,7 @@ import static org.sonar.api.utils.log.LoggerLevel.INFO; import static org.sonar.api.utils.log.LoggerLevel.WARN; import static org.sonar.plugins.javascript.bridge.AnalysisMode.DEFAULT_LINTER_ID; +import static org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl.NODE_EXECUTABLE_PROPERTY; import java.io.File; import java.io.IOException; @@ -44,6 +45,7 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import org.awaitility.Awaitility; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.AfterEach; @@ -722,6 +724,25 @@ void test_ucfg_bundle_version() throws Exception { .contains("Security Frontend version is available: [some_bundle_version]"); } + @Test + void should_not_deploy_runtime_if_sonar_nodejs_executable_is_set() throws Exception { + var existingDoesntMatterScript = "logging.js"; + bridgeServer = createBridgeServer(existingDoesntMatterScript); + bridgeServer.startServer(context, emptyList()); + var nodeCommand = Arrays + .stream(bridgeServer.getCommandInfo().split(" ")) + .filter(s -> s.contains("node")) + .collect(Collectors.joining()); + bridgeServer.stop(); + context.setSettings(new MapSettings().setProperty(NODE_EXECUTABLE_PROPERTY, nodeCommand)); + bridgeServer.startServerLazily(context); + + assertThat(logTester.logs(INFO)) + .contains( + "'" + NODE_EXECUTABLE_PROPERTY + "' is set. Skipping embedded Node.js runtime deployment." + ); + } + private BridgeServerImpl createBridgeServer(String startServerScript) { return new BridgeServerImpl( builder(), diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java index 16a2ee6bacd..91f944942e2 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java @@ -29,7 +29,6 @@ import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.LINUX_X64; import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.UNSUPPORTED; import static org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform.WIN_X64; -import static org.sonar.plugins.javascript.nodejs.NodeCommandBuilderImpl.NODE_EXECUTABLE_PROPERTY; import java.nio.file.Files; import java.nio.file.Path; @@ -63,7 +62,7 @@ void should_extract_if_deployLocation_contains_a_different_version() throws Exce var runtimeFolder = en.binary().getParent(); Files.createDirectories(runtimeFolder); Files.write(runtimeFolder.resolve("version.txt"), "a-different-version".getBytes()); - en.deploy(null); + en.deploy(); assertThat(en.binary()).exists(); assertThat(en.isAvailable()).isTrue(); } @@ -72,12 +71,12 @@ void should_extract_if_deployLocation_contains_a_different_version() throws Exce void should_not_extract_if_deployLocation_contains_the_same_version() throws Exception { logTester.setLevel(Level.DEBUG); var en = testEmbeddedNode(); - en.deploy(null); + en.deploy(); assertThat(logTester.logs()).anyMatch(l -> l.startsWith("Extracting embedded node")); logTester.clear(); en = testEmbeddedNode(); assertThat(en.isAvailable()).isFalse(); - en.deploy(null); + en.deploy(); assertThat(logTester.logs()).anyMatch(l -> l.startsWith("Skipping node deploy.")); assertThat(en.isAvailable()).isTrue(); } @@ -85,7 +84,7 @@ void should_not_extract_if_deployLocation_contains_the_same_version() throws Exc @Test void should_not_extract_neither_be_available_if_the_platform_is_unsupported() throws Exception { var en = new EmbeddedNode(mock(ProcessWrapper.class), createUnsupportedEnvironment()); - en.deploy(null); + en.deploy(); assertThat(en.binary()).doesNotExist(); assertThat(en.isAvailable()).isFalse(); } @@ -93,20 +92,10 @@ void should_not_extract_neither_be_available_if_the_platform_is_unsupported() th @Test void should_extract_if_deployLocation_has_no_version() throws Exception { var en = testEmbeddedNode(); - en.deploy(null); + en.deploy(); assertThat(tempDir.resolve(en.binary())).exists(); } - @Test - void should_do_nothing_if_sonar_nodejs_executable_is_set() throws Exception { - var settings = new MapSettings(); - settings.setProperty(NODE_EXECUTABLE_PROPERTY, "true"); - var config = getConfig(settings); - var en = testEmbeddedNode(); - en.deploy(config); - assertThat(en.binary()).doesNotExist(); - } - @Test void should_detect_platform_for_windows_environment() { var platform = Platform.detect(createWindowsEnvironment()); @@ -169,7 +158,7 @@ void should_fail_gracefully() throws Exception { when(processWrapper.waitFor(any(), anyLong(), any())) .thenThrow(new IllegalStateException("My Error")); var en = new EmbeddedNode(processWrapper, createTestEnvironment()); - en.deploy(null); + en.deploy(); assertThat(logTester.logs()) .anyMatch(l -> l.startsWith("Embedded Node.js failed to deploy. Will fallback to host Node.js") diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/nodejs/NodeCommandTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/nodejs/NodeCommandTest.java index 85bc109ee79..919ba8f0f8c 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/nodejs/NodeCommandTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/nodejs/NodeCommandTest.java @@ -398,7 +398,7 @@ void test_windows_default_node_not_found() throws Exception { @Test void test_embedded_runtime() throws Exception { var en = new EmbeddedNode(new ProcessWrapperImpl(), createTestEnvironment()); - en.deploy(null); + en.deploy(); NodeCommand nodeCommand = builder() .script(PATH_TO_SCRIPT) .pathResolver(getPathResolver()) @@ -421,7 +421,7 @@ void test_embedded_runtime_with_forceHost_for_macos() throws Exception { Configuration configuration = mapSettings.asConfig(); var en = new EmbeddedNode(mockProcessWrapper, createTestEnvironment()); - en.deploy(null); + en.deploy(); NodeCommand nodeCommand = builder() .script(PATH_TO_SCRIPT) .configuration(configuration) From e005c2d7e25aa1d8d113a7a236a01d861ef49176 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Tue, 26 Mar 2024 07:58:15 +0100 Subject: [PATCH 09/13] use custom method --- .../sonar/plugins/javascript/bridge/BridgeServerImpl.java | 7 +++++++ .../plugins/javascript/bridge/BridgeServerImplTest.java | 7 ++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java index c9864d964d5..9e5b0a619bc 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java @@ -539,6 +539,13 @@ public String getCommandInfo() { } } + /** + * used for tests only + */ + public String getNodeCommandString() { + return nodeCommand.toString(); + } + @Override public void start() { // Server is started lazily from the org.sonar.plugins.javascript.eslint.EslintBasedRulesSensor diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java index 478a27b2c8f..5f83522fe21 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java @@ -45,7 +45,6 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; import org.awaitility.Awaitility; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.AfterEach; @@ -729,11 +728,9 @@ void should_not_deploy_runtime_if_sonar_nodejs_executable_is_set() throws Except var existingDoesntMatterScript = "logging.js"; bridgeServer = createBridgeServer(existingDoesntMatterScript); bridgeServer.startServer(context, emptyList()); - var nodeCommand = Arrays - .stream(bridgeServer.getCommandInfo().split(" ")) - .filter(s -> s.contains("node")) - .collect(Collectors.joining()); + var nodeCommand = bridgeServer.getNodeCommandString().split(" ")[0]; bridgeServer.stop(); + context.setSettings(new MapSettings().setProperty(NODE_EXECUTABLE_PROPERTY, nodeCommand)); bridgeServer.startServerLazily(context); From eafcf0a123042e605551c9a759b354eff2b4c682 Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Tue, 26 Mar 2024 08:32:06 +0100 Subject: [PATCH 10/13] cleanup and run startLazily so we get a runtime on first try --- .../plugins/javascript/bridge/BridgeServerImplTest.java | 2 +- .../sonar/plugins/javascript/bridge/EmbeddedNodeTest.java | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java index 5f83522fe21..e0de2d86fb4 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java @@ -727,7 +727,7 @@ void test_ucfg_bundle_version() throws Exception { void should_not_deploy_runtime_if_sonar_nodejs_executable_is_set() throws Exception { var existingDoesntMatterScript = "logging.js"; bridgeServer = createBridgeServer(existingDoesntMatterScript); - bridgeServer.startServer(context, emptyList()); + bridgeServer.startServerLazily(context); var nodeCommand = bridgeServer.getNodeCommandString().split(" ")[0]; bridgeServer.stop(); diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java index 91f944942e2..be7ef4d141f 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/EmbeddedNodeTest.java @@ -37,10 +37,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.api.io.TempDir; import org.slf4j.event.Level; -import org.sonar.api.config.Configuration; -import org.sonar.api.config.internal.ConfigurationBridge; import org.sonar.api.config.internal.MapSettings; -import org.sonar.api.config.internal.Settings; import org.sonar.api.testfixtures.log.LogTesterJUnit5; import org.sonar.plugins.javascript.bridge.EmbeddedNode.Platform; import org.sonar.plugins.javascript.nodejs.ProcessWrapper; @@ -206,8 +203,4 @@ private Environment createUnsupportedEnvironment() { when(mockEnvironment.getOsArch()).thenReturn(""); return mockEnvironment; } - - private Configuration getConfig(Settings settings) { - return new ConfigurationBridge(settings); - } } From 3400556dd54942561e19a4237d7efda022e1b5ce Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Tue, 26 Mar 2024 08:36:04 +0100 Subject: [PATCH 11/13] simplify test --- .../plugins/javascript/bridge/BridgeServerImplTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java index e0de2d86fb4..a4d3d34be53 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java +++ b/sonar-plugin/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/bridge/BridgeServerImplTest.java @@ -727,12 +727,9 @@ void test_ucfg_bundle_version() throws Exception { void should_not_deploy_runtime_if_sonar_nodejs_executable_is_set() throws Exception { var existingDoesntMatterScript = "logging.js"; bridgeServer = createBridgeServer(existingDoesntMatterScript); - bridgeServer.startServerLazily(context); - var nodeCommand = bridgeServer.getNodeCommandString().split(" ")[0]; - bridgeServer.stop(); - - context.setSettings(new MapSettings().setProperty(NODE_EXECUTABLE_PROPERTY, nodeCommand)); - bridgeServer.startServerLazily(context); + context.setSettings(new MapSettings().setProperty(NODE_EXECUTABLE_PROPERTY, "whatever")); + assertThatThrownBy(() -> bridgeServer.startServerLazily(context)) + .isInstanceOf(NodeCommandException.class); assertThat(logTester.logs(INFO)) .contains( From 5cc7864e684282d52cd11569b64b89e39665131c Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Tue, 26 Mar 2024 09:32:24 +0100 Subject: [PATCH 12/13] cleanup --- .../sonar/plugins/javascript/bridge/BridgeServerImpl.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java index 9e5b0a619bc..c9864d964d5 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/BridgeServerImpl.java @@ -539,13 +539,6 @@ public String getCommandInfo() { } } - /** - * used for tests only - */ - public String getNodeCommandString() { - return nodeCommand.toString(); - } - @Override public void start() { // Server is started lazily from the org.sonar.plugins.javascript.eslint.EslintBasedRulesSensor From 15be19dff05493ff3a64c919ab7718b4a6238c7a Mon Sep 17 00:00:00 2001 From: Ilia Kebets Date: Tue, 26 Mar 2024 14:09:50 +0100 Subject: [PATCH 13/13] remove extra newline --- .../java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java | 1 - 1 file changed, 1 deletion(-) diff --git a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java index 4bf847a2133..255f372661a 100644 --- a/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java +++ b/sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/bridge/EmbeddedNode.java @@ -170,7 +170,6 @@ public void deploy() throws IOException { if (platform == UNSUPPORTED) { return; } - try { var is = getClass().getResourceAsStream(platform.archivePathInJar()); if (is == null) {