Refactor and allow null branch for fix suggestions

SonarSource · Nov 27, 2024 · c631cdd · c631cdd
1 parent b84191d
commit c631cdd
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 27 deletions.
diff --git a/.../java/org/sonarsource/sonarlint/core/embedded/server/ShowFixSuggestionRequestHandler.java b/.../java/org/sonarsource/sonarlint/core/embedded/server/ShowFixSuggestionRequestHandler.java
@@ -63,6 +63,7 @@
 import org.sonarsource.sonarlint.core.rpc.protocol.client.message.ShowMessageParams;
 import org.sonarsource.sonarlint.core.rpc.protocol.client.telemetry.AiSuggestionSource;
 import org.sonarsource.sonarlint.core.rpc.protocol.client.telemetry.FixSuggestionReceivedParams;
+import org.sonarsource.sonarlint.core.sync.SonarProjectBranchesSynchronizationService;
 import org.sonarsource.sonarlint.core.telemetry.TelemetryService;
 
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
@@ -80,16 +81,19 @@ public class ShowFixSuggestionRequestHandler implements HttpRequestHandler {
   private final RequestHandlerBindingAssistant requestHandlerBindingAssistant;
   private final PathTranslationService pathTranslationService;
   private final String sonarCloudUrl;
+  private final SonarProjectBranchesSynchronizationService sonarProjectBranchesSynchronizationService;
 
   public ShowFixSuggestionRequestHandler(SonarLintRpcClient client, TelemetryService telemetryService,
     InitializeParams params, RequestHandlerBindingAssistant requestHandlerBindingAssistant,
-    PathTranslationService pathTranslationService, SonarCloudActiveEnvironment sonarCloudActiveEnvironment) {
+    PathTranslationService pathTranslationService, SonarCloudActiveEnvironment sonarCloudActiveEnvironment,
+    SonarProjectBranchesSynchronizationService sonarProjectBranchesSynchronizationService) {
     this.client = client;
     this.telemetryService = telemetryService;
     this.canOpenFixSuggestion = params.getFeatureFlags().canOpenFixSuggestion();
     this.requestHandlerBindingAssistant = requestHandlerBindingAssistant;
     this.pathTranslationService = pathTranslationService;
     this.sonarCloudUrl = sonarCloudActiveEnvironment.getUri().toString();
+    this.sonarProjectBranchesSynchronizationService = sonarProjectBranchesSynchronizationService;
   }
 
   @Override
@@ -110,9 +114,13 @@ public void handle(ClassicHttpRequest request, ClassicHttpResponse response, Htt
       showFixSuggestionQuery.projectKey,
       (connectionId, configScopeId, cancelMonitor) -> {
         if (configScopeId != null) {
-          var localBranchMatchesRequesting = client.matchProjectBranch(new MatchProjectBranchParams(configScopeId, showFixSuggestionQuery.branch)).join().isBranchMatched();
+          var branchToMatch = showFixSuggestionQuery.branch;
+          if (branchToMatch == null) {
+            branchToMatch = sonarProjectBranchesSynchronizationService.findMainBranch(connectionId, showFixSuggestionQuery.projectKey, cancelMonitor);
+          }
+          var localBranchMatchesRequesting = client.matchProjectBranch(new MatchProjectBranchParams(configScopeId, branchToMatch)).join().isBranchMatched();
           if (!localBranchMatchesRequesting) {
-            client.showMessage(new ShowMessageParams(MessageType.ERROR, "Attempted to show a fix suggestion from branch '" + showFixSuggestionQuery.branch + "', " +
+            client.showMessage(new ShowMessageParams(MessageType.ERROR, "Attempted to show a fix suggestion from branch '" + branchToMatch + "', " +
               "which is different from the currently checked-out branch.\nPlease switch to the correct branch and try again."));
             return;
           }
@@ -192,6 +200,7 @@ public static class ShowFixSuggestionQuery {
     private final String serverUrl;
     private final String projectKey;
     private final String issueKey;
+    @Nullable
     private final String branch;
     @Nullable
     private final String tokenName;
@@ -202,7 +211,7 @@ public static class ShowFixSuggestionQuery {
     private final boolean isSonarCloud;
     private final FixSuggestionPayload fixSuggestion;
 
-    public ShowFixSuggestionQuery(@Nullable String serverUrl, String projectKey, String issueKey, String branch,
+    public ShowFixSuggestionQuery(@Nullable String serverUrl, String projectKey, String issueKey, @Nullable String branch,
       @Nullable String tokenName, @Nullable String tokenValue, @Nullable String organizationKey, boolean isSonarCloud,
       FixSuggestionPayload fixSuggestion) {
       this.serverUrl = serverUrl;
@@ -217,7 +226,7 @@ public ShowFixSuggestionQuery(@Nullable String serverUrl, String projectKey, Str
     }
 
     public boolean isValid() {
-      return isNotBlank(projectKey) && isNotBlank(issueKey) && isNotBlank(branch)
+      return isNotBlank(projectKey) && isNotBlank(issueKey)
         && (isSonarCloud || isNotBlank(serverUrl))
         && (!isSonarCloud || isNotBlank(organizationKey))
         && fixSuggestion.isValid() && isTokenValid();
@@ -251,6 +260,7 @@ public String getIssueKey() {
       return issueKey;
     }
 
+    @Nullable
     public String getBranch() {
       return branch;
     }

diff --git a/...src/main/java/org/sonarsource/sonarlint/core/embedded/server/ShowIssueRequestHandler.java b/...src/main/java/org/sonarsource/sonarlint/core/embedded/server/ShowIssueRequestHandler.java
@@ -43,7 +43,6 @@
 import org.apache.hc.core5.net.URIBuilder;
 import org.sonarsource.sonarlint.core.ServerApiProvider;
 import org.sonarsource.sonarlint.core.SonarCloudActiveEnvironment;
-import org.sonarsource.sonarlint.core.commons.Binding;
 import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
 import org.sonarsource.sonarlint.core.file.FilePathTranslation;
 import org.sonarsource.sonarlint.core.file.PathTranslationService;
@@ -63,7 +62,6 @@
 import org.sonarsource.sonarlint.core.serverapi.proto.sonarqube.ws.Common;
 import org.sonarsource.sonarlint.core.serverapi.proto.sonarqube.ws.Issues;
 import org.sonarsource.sonarlint.core.serverapi.rules.RulesApi;
-import org.sonarsource.sonarlint.core.storage.StorageService;
 import org.sonarsource.sonarlint.core.sync.SonarProjectBranchesSynchronizationService;
 import org.sonarsource.sonarlint.core.telemetry.TelemetryService;
 
@@ -80,19 +78,17 @@ public class ShowIssueRequestHandler implements HttpRequestHandler {
   private final RequestHandlerBindingAssistant requestHandlerBindingAssistant;
   private final PathTranslationService pathTranslationService;
   private final String sonarCloudUrl;
-  private final StorageService storageService;
   private final SonarProjectBranchesSynchronizationService sonarProjectBranchesSynchronizationService;
 
   public ShowIssueRequestHandler(SonarLintRpcClient client, ServerApiProvider serverApiProvider, TelemetryService telemetryService,
     RequestHandlerBindingAssistant requestHandlerBindingAssistant, PathTranslationService pathTranslationService, SonarCloudActiveEnvironment sonarCloudActiveEnvironment,
-    StorageService storageService, SonarProjectBranchesSynchronizationService sonarProjectBranchesSynchronizationService) {
+    SonarProjectBranchesSynchronizationService sonarProjectBranchesSynchronizationService) {
     this.client = client;
     this.serverApiProvider = serverApiProvider;
     this.telemetryService = telemetryService;
     this.requestHandlerBindingAssistant = requestHandlerBindingAssistant;
     this.pathTranslationService = pathTranslationService;
     this.sonarCloudUrl = sonarCloudActiveEnvironment.getUri().toString();
-    this.storageService = storageService;
     this.sonarProjectBranchesSynchronizationService = sonarProjectBranchesSynchronizationService;
   }
 
@@ -114,15 +110,7 @@ public void handle(ClassicHttpRequest request, ClassicHttpResponse response, Htt
         if (configScopeId != null) {
           var branchToMatch = showIssueQuery.branch;
           if (branchToMatch == null) {
-            var branchesStorage = storageService.binding(new Binding(connectionId, showIssueQuery.projectKey)).branches();
-            if (branchesStorage.exists()) {
-              var storedBranches = branchesStorage.read();
-              branchToMatch = storedBranches.getMainBranchName();
-            } else {
-              var serverApi = serverApiProvider.getServerApiOrThrow(connectionId);
-              var branches = sonarProjectBranchesSynchronizationService.getProjectBranches(serverApi, showIssueQuery.projectKey, cancelMonitor);
-              branchToMatch = branches.getMainBranchName();
-            }
+            branchToMatch = sonarProjectBranchesSynchronizationService.findMainBranch(connectionId, showIssueQuery.projectKey, cancelMonitor);
           }
           var localBranchMatchesRequesting = client.matchProjectBranch(new MatchProjectBranchParams(configScopeId, branchToMatch)).join().isBranchMatched();
           if (!localBranchMatchesRequesting) {

diff --git a/.../java/org/sonarsource/sonarlint/core/sync/SonarProjectBranchesSynchronizationService.java b/.../java/org/sonarsource/sonarlint/core/sync/SonarProjectBranchesSynchronizationService.java
@@ -23,6 +23,7 @@
 import javax.inject.Named;
 import javax.inject.Singleton;
 import org.sonarsource.sonarlint.core.ServerApiProvider;
+import org.sonarsource.sonarlint.core.commons.Binding;
 import org.sonarsource.sonarlint.core.commons.log.SonarLintLogger;
 import org.sonarsource.sonarlint.core.commons.progress.SonarLintCancelMonitor;
 import org.sonarsource.sonarlint.core.serverapi.ServerApi;
@@ -74,4 +75,16 @@ public ProjectBranches getProjectBranches(ServerApi serverApi, String projectKey
     return new ProjectBranches(allBranches.stream().map(ServerBranch::getName).collect(toSet()), mainBranch);
   }
 
+  public String findMainBranch(String connectionId, String projectKey, SonarLintCancelMonitor cancelMonitor) {
+    var branchesStorage = storageService.binding(new Binding(connectionId, projectKey)).branches();
+    if (branchesStorage.exists()) {
+      var storedBranches = branchesStorage.read();
+      return storedBranches.getMainBranchName();
+    } else {
+      var serverApi = serverApiProvider.getServerApiOrThrow(connectionId);
+      var branches = getProjectBranches(serverApi, projectKey, cancelMonitor);
+      return branches.getMainBranchName();
+    }
+  }
+
 }
diff --git a/.../org/sonarsource/sonarlint/core/embedded/server/ShowFixSuggestionRequestHandlerTests.java b/.../org/sonarsource/sonarlint/core/embedded/server/ShowFixSuggestionRequestHandlerTests.java
@@ -23,6 +23,8 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.List;
+import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 import org.apache.hc.core5.http.ClassicHttpRequest;
@@ -42,6 +44,7 @@
 import org.sonarsource.sonarlint.core.SonarCloudActiveEnvironment;
 import org.sonarsource.sonarlint.core.commons.BoundScope;
 import org.sonarsource.sonarlint.core.commons.log.SonarLintLogTester;
+import org.sonarsource.sonarlint.core.file.FilePathTranslation;
 import org.sonarsource.sonarlint.core.file.PathTranslationService;
 import org.sonarsource.sonarlint.core.repository.config.ConfigurationRepository;
 import org.sonarsource.sonarlint.core.repository.connection.ConnectionConfigurationRepository;
@@ -52,6 +55,8 @@
 import org.sonarsource.sonarlint.core.rpc.protocol.client.branch.MatchProjectBranchResponse;
 import org.sonarsource.sonarlint.core.rpc.protocol.client.message.MessageType;
 import org.sonarsource.sonarlint.core.rpc.protocol.client.message.ShowMessageParams;
+import org.sonarsource.sonarlint.core.serverconnection.ProjectBranches;
+import org.sonarsource.sonarlint.core.sync.SonarProjectBranchesSynchronizationService;
 import org.sonarsource.sonarlint.core.telemetry.TelemetryService;
 import org.sonarsource.sonarlint.core.usertoken.UserTokenService;
 
@@ -82,17 +87,22 @@ void setup() {
     var bindingSuggestionProvider = mock(BindingSuggestionProvider.class);
     var bindingCandidatesFinder = mock(BindingCandidatesFinder.class);
     sonarLintRpcClient = mock(SonarLintRpcClient.class);
+    var filePathTranslation = mock(FilePathTranslation.class);
     var pathTranslationService = mock(PathTranslationService.class);
+    when(pathTranslationService.getOrComputePathTranslation(any())).thenReturn(Optional.of(filePathTranslation));
     var userTokenService = mock(UserTokenService.class);
     featureFlagsDto = mock(FeatureFlagsDto.class);
     when(featureFlagsDto.canOpenFixSuggestion()).thenReturn(true);
     initializeParams = mock(InitializeParams.class);
     when(initializeParams.getFeatureFlags()).thenReturn(featureFlagsDto);
     var sonarCloudActiveEnvironment = SonarCloudActiveEnvironment.prod();
     telemetryService = mock(TelemetryService.class);
+    var sonarProjectBranchesSynchronizationService = mock(SonarProjectBranchesSynchronizationService.class);
+    when(sonarProjectBranchesSynchronizationService.getProjectBranches(any(), any(), any())).thenReturn(new ProjectBranches(Set.of(), "main"));
 
     showFixSuggestionRequestHandler = new ShowFixSuggestionRequestHandler(sonarLintRpcClient, telemetryService, initializeParams,
-      new RequestHandlerBindingAssistant(bindingSuggestionProvider, bindingCandidatesFinder, sonarLintRpcClient, connectionConfigurationRepository, configurationRepository, userTokenService, sonarCloudActiveEnvironment), pathTranslationService, sonarCloudActiveEnvironment);
+      new RequestHandlerBindingAssistant(bindingSuggestionProvider, bindingCandidatesFinder, sonarLintRpcClient, connectionConfigurationRepository,
+        configurationRepository, userTokenService, sonarCloudActiveEnvironment), pathTranslationService, sonarCloudActiveEnvironment, sonarProjectBranchesSynchronizationService);
   }
 
   @Test
@@ -259,6 +269,41 @@ void should_cancel_flow_when_branch_does_not_match() throws HttpException, IOExc
     verifyNoMoreInteractions(sonarLintRpcClient);
   }
 
+  @Test
+  void should_find_main_branch_when_not_provided_and_not_stored() throws HttpException, IOException {
+    var request = new BasicClassicHttpRequest("POST", "/sonarlint/api/fix/show" +
+      "?project=org.sonarsource.sonarlint.core%3Asonarlint-core-parent" +
+      "&issue=AX2VL6pgAvx3iwyNtLyr" +
+      "&organizationKey=sample-organization");
+    request.addHeader("Origin", PRODUCTION_URI);
+    request.setEntity(new StringEntity("{\n" +
+      "\"fileEdit\": {\n" +
+      "\"path\": \"src/main/java/Main.java\",\n" +
+      "\"changes\": [{\n" +
+      "\"beforeLineRange\": {\n" +
+      "\"startLine\": 0,\n" +
+      "\"endLine\": 1\n" +
+      "},\n" +
+      "\"before\": \"\",\n" +
+      "\"after\": \"var fix = 1;\"\n" +
+      "}]\n" +
+      "},\n" +
+      "\"suggestionId\": \"eb93b2b4-f7b0-4b5c-9460-50893968c264\",\n" +
+      "\"explanation\": \"Modifying the variable name is good\"\n" +
+      "}\n"));
+    var response = mock(ClassicHttpResponse.class);
+    var context = mock(HttpContext.class);
+
+    when(connectionConfigurationRepository.findByOrganization(any())).thenReturn(List.of(
+      new SonarCloudConnectionConfiguration(PRODUCTION_URI, "name", "organizationKey", false)));
+    when(configurationRepository.getBoundScopesToConnectionAndSonarProject(any(), any())).thenReturn(List.of(new BoundScope("configScope", "connectionId", "projectKey")));
+    when(sonarLintRpcClient.matchProjectBranch(any())).thenReturn(CompletableFuture.completedFuture(new MatchProjectBranchResponse(true)));
+
+    showFixSuggestionRequestHandler.handle(request, response, context);
+
+    await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> verify(sonarLintRpcClient).showFixSuggestion(any()));
+  }
+
   private static ShowFixSuggestionRequestHandler.FixSuggestionPayload generateFixSuggestionPayload() {
     return new ShowFixSuggestionRequestHandler.FixSuggestionPayload(
       new ShowFixSuggestionRequestHandler.FileEditPayload(

diff --git a/...est/java/org/sonarsource/sonarlint/core/embedded/server/ShowIssueRequestHandlerTests.java b/...est/java/org/sonarsource/sonarlint/core/embedded/server/ShowIssueRequestHandlerTests.java
@@ -64,8 +64,6 @@
 import org.sonarsource.sonarlint.core.serverapi.proto.sonarqube.ws.Issues;
 import org.sonarsource.sonarlint.core.serverconnection.ProjectBranches;
 import org.sonarsource.sonarlint.core.serverconnection.ProjectBranchesStorage;
-import org.sonarsource.sonarlint.core.serverconnection.SonarProjectStorage;
-import org.sonarsource.sonarlint.core.storage.StorageService;
 import org.sonarsource.sonarlint.core.sync.SonarProjectBranchesSynchronizationService;
 import org.sonarsource.sonarlint.core.telemetry.TelemetryService;
 import org.sonarsource.sonarlint.core.usertoken.UserTokenService;
@@ -113,17 +111,13 @@ void setup() {
     var serverApiProvider = mock(ServerApiProvider.class);
     when(serverApiProvider.getServerApiOrThrow(any())).thenReturn(serverApi);
     when(serverApiProvider.getServerApi(any())).thenReturn(Optional.of(serverApi));
-    var storageService = mock(StorageService.class);
     branchesStorage = mock(ProjectBranchesStorage.class);
-    var sonarStorage = mock(SonarProjectStorage.class);
-    when(sonarStorage.branches()).thenReturn(branchesStorage);
-    when(storageService.binding(any())).thenReturn(sonarStorage);
     var sonarProjectBranchesSynchronizationService = mock(SonarProjectBranchesSynchronizationService.class);
     when(sonarProjectBranchesSynchronizationService.getProjectBranches(any(), any(), any())).thenReturn(new ProjectBranches(Set.of(), "main"));
 
     showIssueRequestHandler = spy(new ShowIssueRequestHandler(sonarLintRpcClient, serverApiProvider, telemetryService,
       new RequestHandlerBindingAssistant(bindingSuggestionProvider, bindingCandidatesFinder, sonarLintRpcClient, connectionConfigurationRepository, configurationRepository, userTokenService,
-        sonarCloudActiveEnvironment), pathTranslationService, sonarCloudActiveEnvironment, storageService, sonarProjectBranchesSynchronizationService));
+        sonarCloudActiveEnvironment), pathTranslationService, sonarCloudActiveEnvironment, sonarProjectBranchesSynchronizationService));
   }
 
   @Test