From: Steve Marion <steve.marion@sonarsource.com>
Date: Thu, 28 Sep 2023 16:02:03 +0000 (+0200)
Subject: SONAR-20580 fix gitlab synchro failures from crashing the analysis.
X-Git-Tag: 10.3.0.82913~311
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=08205d2b22b77e57ab378c43313ebec311bba596;p=sonarqube.git

SONAR-20580 fix gitlab synchro failures from crashing the analysis.
---

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationCheck.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationCheck.java
index 909514b9d3f..665d5d306d4 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationCheck.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationCheck.java
@@ -36,9 +36,10 @@ import org.sonar.ce.task.projectanalysis.analysis.Branch;
 public interface PreMeasuresComputationCheck {
 
   /**
-   * This method can make the task fail by throwing a {@link RuntimeException}
+   * Throwing a {@link PreMeasuresComputationCheckException} will only produce an analysis warning.
+   * Any other exception will fail the analysis.
    */
-  void onCheck(Context context);
+  void onCheck(Context context) throws PreMeasuresComputationCheckException;
 
   interface Context {
 
@@ -47,4 +48,14 @@ public interface PreMeasuresComputationCheck {
     Branch getBranch();
 
   }
+
+  class PreMeasuresComputationCheckException extends Exception {
+    public PreMeasuresComputationCheckException(String message) {
+      super(message);
+    }
+
+    public PreMeasuresComputationCheckException(String message, Throwable cause) {
+      super(message, cause);
+    }
+  }
 }
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationChecksStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationChecksStep.java
index 79257403306..1c86e564d87 100644
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationChecksStep.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationChecksStep.java
@@ -19,24 +19,30 @@
  */
 package org.sonar.ce.task.projectanalysis.measure;
 
-import org.sonar.api.ce.ComputeEngineSide;
+import org.sonar.api.utils.System2;
+import org.sonar.ce.task.log.CeTaskMessages;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
 import org.sonar.ce.task.projectanalysis.analysis.Branch;
 import org.sonar.ce.task.step.ComputationStep;
+import org.sonar.db.dismissmessage.MessageType;
+
+import static org.sonar.ce.task.projectanalysis.measure.PreMeasuresComputationCheck.PreMeasuresComputationCheckException;
 
 /**
  * Execute {@link PreMeasuresComputationCheck} instances in no specific order.
  * If an extension fails (throws an exception), consecutive extensions
  * won't be called.
  */
-@ComputeEngineSide
 public class PreMeasuresComputationChecksStep implements ComputationStep {
 
   private final AnalysisMetadataHolder analysisMetadataHolder;
   private final PreMeasuresComputationCheck[] extensions;
+  private final CeTaskMessages ceTaskMessages;
+
 
-  public PreMeasuresComputationChecksStep(AnalysisMetadataHolder analysisMetadataHolder, PreMeasuresComputationCheck... extensions) {
+  public PreMeasuresComputationChecksStep(AnalysisMetadataHolder analysisMetadataHolder, CeTaskMessages ceTaskMessages, PreMeasuresComputationCheck... extensions) {
     this.analysisMetadataHolder = analysisMetadataHolder;
+    this.ceTaskMessages = ceTaskMessages;
     this.extensions = extensions;
   }
 
@@ -44,7 +50,11 @@ public class PreMeasuresComputationChecksStep implements ComputationStep {
   public void execute(Context context) {
     PreMeasuresComputationCheck.Context extensionContext = new ContextImpl();
     for (PreMeasuresComputationCheck extension : extensions) {
-      extension.onCheck(extensionContext);
+      try {
+        extension.onCheck(extensionContext);
+      } catch (PreMeasuresComputationCheckException pmcce) {
+        ceTaskMessages.add(new CeTaskMessages.Message(pmcce.getMessage(), System2.INSTANCE.now(), MessageType.GENERIC));
+      }
     }
   }
 
diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationChecksStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationChecksStepTest.java
index 5f2481025f0..25ee21b4d3b 100644
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationChecksStepTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/measure/PreMeasuresComputationChecksStepTest.java
@@ -22,6 +22,7 @@ package org.sonar.ce.task.projectanalysis.measure;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
+import org.sonar.ce.task.log.CeTaskMessages;
 import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule;
 import org.sonar.ce.task.projectanalysis.analysis.Branch;
 import org.sonar.ce.task.projectanalysis.measure.PreMeasuresComputationCheck.Context;
@@ -31,18 +32,21 @@ import org.sonar.server.project.Project;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.sonar.ce.task.projectanalysis.measure.PreMeasuresComputationCheck.PreMeasuresComputationCheckException;
 import static org.sonar.db.component.ComponentTesting.newPrivateProjectDto;
 
 public class PreMeasuresComputationChecksStepTest {
 
-  public AnalysisMetadataHolderRule analysisMetadataHolder = mock(AnalysisMetadataHolderRule.class);
+  public AnalysisMetadataHolderRule analysisMetadataHolder = mock();
+  public CeTaskMessages ceTaskMessages = mock();
 
   @Test
-  public void execute_extensions() {
+  public void execute_extensions() throws PreMeasuresComputationCheckException {
     PreMeasuresComputationCheck check1 = mock(PreMeasuresComputationCheck.class);
     PreMeasuresComputationCheck check2 = mock(PreMeasuresComputationCheck.class);
 
@@ -54,7 +58,7 @@ public class PreMeasuresComputationChecksStepTest {
   }
 
   @Test
-  public void context_contains_project_uuid_from_analysis_metadata_holder() {
+  public void context_contains_project_uuid_from_analysis_metadata_holder() throws PreMeasuresComputationCheckException {
     Project project = Project.from(newPrivateProjectDto());
     when(analysisMetadataHolder.getProject()).thenReturn(project);
     PreMeasuresComputationCheck check = mock(PreMeasuresComputationCheck.class);
@@ -67,7 +71,7 @@ public class PreMeasuresComputationChecksStepTest {
   }
 
   @Test
-  public void context_contains_pullRequest_key_from_analysis_metadata_holder() {
+  public void context_contains_pullRequest_key_from_analysis_metadata_holder() throws PreMeasuresComputationCheckException {
     mockPr("pr1");
     PreMeasuresComputationCheck check = mock(PreMeasuresComputationCheck.class);
 
@@ -79,7 +83,7 @@ public class PreMeasuresComputationChecksStepTest {
   }
 
   @Test
-  public void context_contains_branch_from_analysis_metadata_holder() {
+  public void context_contains_branch_from_analysis_metadata_holder() throws PreMeasuresComputationCheckException {
     mockBranch("branchName");
     PreMeasuresComputationCheck check = mock(PreMeasuresComputationCheck.class);
 
@@ -90,6 +94,19 @@ public class PreMeasuresComputationChecksStepTest {
     assertThat(contextArgumentCaptor.getValue().getBranch().getName()).isEqualTo("branchName");
   }
 
+  @Test
+  public void whenCheckThrows_thenLogCeMessage() throws PreMeasuresComputationCheckException {
+    PreMeasuresComputationCheck check = mock(PreMeasuresComputationCheck.class);
+    doThrow(new PreMeasuresComputationCheckException("error"))
+      .when(check).onCheck(any());
+
+    newStep(check).execute(new TestComputationStepContext());
+
+    var messageCaptor = ArgumentCaptor.forClass(CeTaskMessages.Message.class);
+    verify(ceTaskMessages).add(messageCaptor.capture());
+    assertThat(messageCaptor.getValue().getText()).isEqualTo("error");
+  }
+
   @Test
   public void test_getDescription() {
     assertThat(newStep().getDescription()).isNotEmpty();
@@ -97,9 +114,9 @@ public class PreMeasuresComputationChecksStepTest {
 
   private PreMeasuresComputationChecksStep newStep(PreMeasuresComputationCheck... preMeasuresComputationChecks) {
     if (preMeasuresComputationChecks.length == 0) {
-      return new PreMeasuresComputationChecksStep(analysisMetadataHolder);
+      return new PreMeasuresComputationChecksStep(analysisMetadataHolder, ceTaskMessages);
     }
-    return new PreMeasuresComputationChecksStep(analysisMetadataHolder, preMeasuresComputationChecks);
+    return new PreMeasuresComputationChecksStep(analysisMetadataHolder, ceTaskMessages, preMeasuresComputationChecks);
   }
 
   private void mockBranch(String branchName) {