SLCORE-1526, SLCORE-1543 refactor; apply PR review

Author: sophio-japharidze-sonarsource

API_CHANGES.md

@@ -9,7 +9,6 @@
   * Introduce `org.sonarsource.sonarlint.core.rpc.protocol.backend.sca.ScaRpcService.openDependencyRiskInBrowser` that accepts `configScopeId` and `dependencyRiskKey` (UUID) parameters
 * Allow clients to record interactions with dependency risks (SCA issues) in telemetry
   * Introduce `org.sonarsource.sonarlint.core.rpc.protocol.backend.telemetry.TelemetryRpcService.dependencyRiskInvestigatedLocally` method
-  * Introduce `org.sonarsource.sonarlint.core.rpc.protocol.backend.telemetry.TelemetryRpcService.dependencyRiskInvestigatedRemotely` method
 * Add a new `org.sonarsource.sonarlint.core.rpc.protocol.backend.sca.ScaRpcService.getDependencyRiskDetails`.
 
 # 10.26
@@ -18,9 +17,6 @@
 
 * Add a new `SCA_SYNCHRONIZATION` value in `org.sonarsource.sonarlint.core.rpc.protocol.backend.initialize.BackendCapability`. Clients using the feature need to declare it at initialization time.
 * Introduce a new `org.sonarsource.sonarlint.core.rpc.protocol.backend.tracking.ScaIssueTrackingRpcService` service and a `listAll` method
-* Allow changing status of SCA issues via `org.sonarsource.sonarlint.core.rpc.protocol.backend.sca.ScaRpcService.changeStatus`.
-  * Required parameters are `configScopeId`, `issueId` and `transition`.
-  * If transition is `ACCEPT`, `FIXED, or `SAFE`, a `comment` field is mandatory
 
 # 10.25
 

backend/core/src/main/java/org/sonarsource/sonarlint/core/sca/ScaService.java

@@ -32,14 +32,14 @@
 import org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcClient;
 import org.sonarsource.sonarlint.core.rpc.protocol.SonarLintRpcErrorCode;
 import org.sonarsource.sonarlint.core.rpc.protocol.backend.sca.DependencyRiskTransition;
-import org.sonarsource.sonarlint.core.rpc.protocol.backend.tracking.AffectedPackageDto;
 import org.sonarsource.sonarlint.core.rpc.protocol.backend.sca.GetDependencyRiskDetailsResponse;
+import org.sonarsource.sonarlint.core.rpc.protocol.backend.tracking.AffectedPackageDto;
 import org.sonarsource.sonarlint.core.rpc.protocol.backend.tracking.ScaIssueDto;
-import org.sonarsource.sonarlint.core.serverapi.sca.GetIssueReleaseResponse;
 import org.sonarsource.sonarlint.core.rpc.protocol.client.OpenUrlInBrowserParams;
 import org.sonarsource.sonarlint.core.serverapi.EndpointParams;
 import org.sonarsource.sonarlint.core.serverapi.ServerApiHelper;
 import org.sonarsource.sonarlint.core.serverapi.UrlUtils;
+import org.sonarsource.sonarlint.core.serverapi.sca.GetIssueReleaseResponse;
 import org.sonarsource.sonarlint.core.serverconnection.issues.ServerScaIssue;
 import org.sonarsource.sonarlint.core.storage.StorageService;
 import org.sonarsource.sonarlint.core.telemetry.TelemetryService;
@@ -152,17 +152,15 @@ private static GetDependencyRiskDetailsResponse convertToRpcResponse(GetIssueRel
       serverResponse.vulnerability().description(), affectedPackages);
   }
 
-  public void openDependencyRiskInBrowser(String configurationScopeId, String dependencyKey) {
+  public void openDependencyRiskInBrowser(String configurationScopeId, UUID dependencyKey) {
     var effectiveBinding = configurationRepository.getEffectiveBinding(configurationScopeId);
     var endpointParams = effectiveBinding.flatMap(binding -> connectionRepository.getEndpointParams(binding.connectionId()));
     if (effectiveBinding.isEmpty() || endpointParams.isEmpty()) {
-      LOG.warn("Configuration scope {} is not bound properly, unable to open dependency risk", configurationScopeId);
-      return;
+      throw new IllegalArgumentException(String.format("Configuration scope '%s' is not bound properly, unable to open dependency risk", configurationScopeId));
     }
     var branchName = branchTrackingService.awaitEffectiveSonarProjectBranch(configurationScopeId);
     if (branchName.isEmpty()) {
-      LOG.warn("Configuration scope {} has no matching branch, unable to open dependency risk", configurationScopeId);
-      return;
+      throw new IllegalArgumentException(String.format("Configuration scope %s has no matching branch, unable to open dependency risk", configurationScopeId));
     }
 
     var url = buildScaBrowseUrl(effectiveBinding.get().sonarProjectKey(), branchName.get(), dependencyKey, endpointParams.get());

backend/rpc-impl/src/main/java/org/sonarsource/sonarlint/core/rpc/impl/ScaRpcServiceDelegate.java

@@ -64,7 +64,16 @@ public CompletableFuture<GetDependencyRiskDetailsResponse> getDependencyRiskDeta
   }
 
   @Override
-  public void openDependencyRiskInBrowser(OpenDependencyRiskInBrowserParams params) {
-    notify(() -> getBean(ScaService.class).openDependencyRiskInBrowser(params.getConfigScopeId(), params.getDependencyKey()), params.getConfigScopeId());
+  public CompletableFuture<Void> openDependencyRiskInBrowser(OpenDependencyRiskInBrowserParams params) {
+    return runAsync(cancelMonitor -> {
+      try {
+        getBean(ScaService.class).openDependencyRiskInBrowser(
+          params.getConfigScopeId(),
+          params.getDependencyKey());
+      } catch (IllegalArgumentException e) {
+        var error = new ResponseError(SonarLintRpcErrorCode.INVALID_ARGUMENT, e.getMessage(), null);
+        throw new ResponseErrorException(error);
+      }
+    }, params.getConfigScopeId());
   }
 }

backend/rpc-impl/src/main/java/org/sonarsource/sonarlint/core/rpc/impl/TelemetryRpcServiceDelegate.java

@@ -154,11 +154,6 @@ public void dependencyRiskInvestigatedLocally() {
     notify(() -> getBean(TelemetryService.class).dependencyRiskInvestigatedLocally());
   }
 
-  @Override
-  public void dependencyRiskInvestigatedRemotely() {
-    notify(() -> getBean(TelemetryService.class).dependencyRiskInvestigatedRemotely());
-  }
-
   @Override
   public void findingsFiltered(FindingsFilteredParams params) {
     notify(() -> getBean(TelemetryService.class).findingsFiltered(params.getFilterType()));

medium-tests/src/test/java/mediumtest/TelemetryMediumTests.java

@@ -342,20 +342,14 @@ void it_should_accumulate_investigated_taint_vulnerabilities(SonarLintTestHarnes
   void it_should_accumulate_investigated_dependency_risks(SonarLintTestHarness harness) {
     var backend = setupClientAndBackend(harness);
 
-    backend.getTelemetryService().dependencyRiskInvestigatedRemotely();
     backend.getTelemetryService().dependencyRiskInvestigatedLocally();
 
-    await().untilAsserted(() -> assertThat(backend.telemetryFileContent())
-      .extracting(TelemetryLocalStorage::getDependencyRiskInvestigatedRemotelyCount, TelemetryLocalStorage::getDependencyRiskInvestigatedLocallyCount)
-        .containsExactly(1, 1));
+    await().untilAsserted(() -> assertThat(backend.telemetryFileContent().getDependencyRiskInvestigatedLocallyCount()).isEqualTo(1));
 
-    backend.getTelemetryService().dependencyRiskInvestigatedRemotely();
     backend.getTelemetryService().dependencyRiskInvestigatedLocally();
     backend.getTelemetryService().dependencyRiskInvestigatedLocally();
 
-    await().untilAsserted(() -> assertThat(backend.telemetryFileContent())
-      .extracting(TelemetryLocalStorage::getDependencyRiskInvestigatedRemotelyCount, TelemetryLocalStorage::getDependencyRiskInvestigatedLocallyCount)
-      .containsExactly(2, 3));
+    await().untilAsserted(() -> assertThat(backend.telemetryFileContent().getDependencyRiskInvestigatedLocallyCount()).isEqualTo(3));
   }
 
   @SonarLintTest

medium-tests/src/test/java/mediumtest/sca/OpenDependencyRiskInBrowserMediumTests.java

@@ -21,7 +21,9 @@
 
 import java.io.IOException;
 import java.net.URL;
+import java.time.Duration;
 import java.util.UUID;
+import java.util.concurrent.ExecutionException;
 import org.sonarsource.sonarlint.core.rpc.protocol.backend.sca.OpenDependencyRiskInBrowserParams;
 import org.sonarsource.sonarlint.core.test.utils.junit5.SonarLintTest;
 import org.sonarsource.sonarlint.core.test.utils.junit5.SonarLintTestHarness;
@@ -50,7 +52,7 @@ void it_should_open_dependency_risk_in_sonarqube(SonarLintTestHarness harness) t
       .start(fakeClient);
 
     backend.getScaService().openDependencyRiskInBrowser(new OpenDependencyRiskInBrowserParams(
-      SCOPE_ID, DEPENDENCY_KEY));
+      SCOPE_ID, DEPENDENCY_KEY)).join();
 
     var expectedUrl = String.format("http://localhost:12345/dependency-risks/%s/what?id=%s&branch=%s",
       urlEncode(DEPENDENCY_KEY.toString()), urlEncode(PROJECT_KEY), urlEncode(BRANCH_NAME));
@@ -66,9 +68,11 @@ void it_should_not_open_dependency_risk_if_unbound(SonarLintTestHarness harness)
       .withUnboundConfigScope(SCOPE_ID)
       .start(fakeClient);
 
-    backend.getScaService().openDependencyRiskInBrowser(new OpenDependencyRiskInBrowserParams(
+    var result = backend.getScaService().openDependencyRiskInBrowser(new OpenDependencyRiskInBrowserParams(
       SCOPE_ID, DEPENDENCY_KEY));
 
+    assertThat(result).failsWithin(Duration.ofSeconds(2)).withThrowableOfType(ExecutionException.class)
+      .withMessage("org.eclipse.lsp4j.jsonrpc.ResponseErrorException: Configuration scope 'scopeId' is not bound properly, unable to open dependency risk");
     verify(fakeClient, timeout(5000).times(0)).openUrlInBrowser(any(URL.class));
   }
 }

rpc-protocol/src/main/java/org/sonarsource/sonarlint/core/rpc/protocol/backend/sca/ScaRpcService.java

@@ -20,7 +20,6 @@
 package org.sonarsource.sonarlint.core.rpc.protocol.backend.sca;
 
 import java.util.concurrent.CompletableFuture;
-import org.eclipse.lsp4j.jsonrpc.services.JsonNotification;
 import org.eclipse.lsp4j.jsonrpc.services.JsonRequest;
 import org.eclipse.lsp4j.jsonrpc.services.JsonSegment;
 
@@ -54,6 +53,6 @@ public interface ScaRpcService {
   @JsonRequest
   CompletableFuture<GetDependencyRiskDetailsResponse> getDependencyRiskDetails(GetDependencyRiskDetailsParams params);
 
-  @JsonNotification
-  void openDependencyRiskInBrowser(OpenDependencyRiskInBrowserParams params);
+  @JsonRequest
+  CompletableFuture<Void> openDependencyRiskInBrowser(OpenDependencyRiskInBrowserParams params);
 }

rpc-protocol/src/main/java/org/sonarsource/sonarlint/core/rpc/protocol/backend/telemetry/TelemetryRpcService.java

@@ -125,9 +125,6 @@ public interface TelemetryRpcService {
   @JsonNotification
   void dependencyRiskInvestigatedLocally();
 
-  @JsonNotification
-  void dependencyRiskInvestigatedRemotely();
-
   @JsonNotification
   void findingsFiltered(FindingsFilteredParams params);
 }