]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-19294 Fail to unset NCD when parent NCD not CaYC compliant (#8314)
authorNolwenn Cadic <98824442+Nolwenn-cadic-sonarsource@users.noreply.github.com>
Wed, 17 May 2023 12:56:50 +0000 (14:56 +0200)
committersonartech <sonartech@sonarsource.com>
Tue, 23 May 2023 20:03:09 +0000 (20:03 +0000)
server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/CaycUtilsTest.java [new file with mode: 0644]
server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/ws/UnsetActionIT.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/CaycUtils.java [new file with mode: 0644]
server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/ws/UnsetAction.java

diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/CaycUtilsTest.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/CaycUtilsTest.java
new file mode 100644 (file)
index 0000000..4dccd0a
--- /dev/null
@@ -0,0 +1,85 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.newcodeperiod;
+
+import org.junit.Test;
+import org.sonar.db.newcodeperiod.NewCodePeriodDto;
+import org.sonar.db.newcodeperiod.NewCodePeriodType;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+public class CaycUtilsTest {
+
+  @Test
+  public void reference_branch_is_compliant() {
+    var newCodePeriod = new NewCodePeriodDto()
+      .setType(NewCodePeriodType.REFERENCE_BRANCH)
+      .setValue("master");
+    assertThat(CaycUtils.isNewCodePeriodCompliant(newCodePeriod.getType(), newCodePeriod.getValue())).isTrue();
+  }
+
+  @Test
+  public void previous_version_is_compliant() {
+    var newCodePeriod = new NewCodePeriodDto()
+      .setType(NewCodePeriodType.PREVIOUS_VERSION)
+      .setValue("1.0");
+    assertThat(CaycUtils.isNewCodePeriodCompliant(newCodePeriod.getType(), newCodePeriod.getValue())).isTrue();
+  }
+
+  @Test
+  public void number_of_days_smaller_than_90_is_compliant() {
+    var newCodePeriod = new NewCodePeriodDto()
+      .setType(NewCodePeriodType.NUMBER_OF_DAYS)
+      .setValue("30");
+    assertThat(CaycUtils.isNewCodePeriodCompliant(newCodePeriod.getType(), newCodePeriod.getValue())).isTrue();
+  }
+
+  @Test
+  public void number_of_days_smaller_than_1_is_not_compliant() {
+    var newCodePeriod = new NewCodePeriodDto()
+      .setType(NewCodePeriodType.NUMBER_OF_DAYS)
+      .setValue("0");
+    assertThat(CaycUtils.isNewCodePeriodCompliant(newCodePeriod.getType(), newCodePeriod.getValue())).isFalse();
+  }
+
+  @Test
+  public void number_of_days_bigger_than_90_is_not_compliant() {
+    var newCodePeriod = new NewCodePeriodDto()
+      .setType(NewCodePeriodType.NUMBER_OF_DAYS)
+      .setValue("91");
+    assertThat(CaycUtils.isNewCodePeriodCompliant(newCodePeriod.getType(), newCodePeriod.getValue())).isFalse();
+  }
+
+  @Test
+  public void specific_analysis_is_compliant() {
+    var newCodePeriod = new NewCodePeriodDto()
+      .setType(NewCodePeriodType.SPECIFIC_ANALYSIS)
+      .setValue("sdfsafsdf");
+    assertThat(CaycUtils.isNewCodePeriodCompliant(newCodePeriod.getType(), newCodePeriod.getValue())).isTrue();
+  }
+
+  @Test
+  public void wrong_number_of_days_format_should_throw_exception() {
+    assertThatThrownBy(() -> CaycUtils.isNewCodePeriodCompliant(NewCodePeriodType.NUMBER_OF_DAYS, "abc"))
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessageContaining("Failed to parse number of days: abc");
+  }
+}
\ No newline at end of file
index 934d6f2ec0c54cd666093860e088851af9a86b02..543ff39515c7e81ec499edbc7424a9f49be11189 100644 (file)
@@ -43,6 +43,7 @@ import org.sonar.server.component.TestComponentFinder;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.tester.UserSessionRule;
+import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -92,9 +93,10 @@ public class UnsetActionIT {
   // validation of project/branch
   @Test
   public void throw_IAE_if_branch_is_specified_without_project() {
-    assertThatThrownBy(() -> ws.newRequest()
-      .setParam("branch", "branch")
-      .execute())
+
+    TestRequest request = ws.newRequest()
+      .setParam("branch", "branch");
+    assertThatThrownBy(() -> request.execute())
       .isInstanceOf(IllegalArgumentException.class)
       .hasMessageContaining("If branch key is specified, project key needs to be specified too");
   }
@@ -191,7 +193,7 @@ public class UnsetActionIT {
     ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
     ComponentDto branch = componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
 
-    db.newCodePeriods().insert(project.uuid(), null, NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid1");
+    db.newCodePeriods().insert(project.uuid(), null, NewCodePeriodType.NUMBER_OF_DAYS, "20");
     db.newCodePeriods().insert(project.uuid(), branch.uuid(), NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid2");
 
     logInAsProjectAdministrator(project);
@@ -201,7 +203,7 @@ public class UnsetActionIT {
       .setParam("branch", "branch")
       .execute();
 
-    assertTableContainsOnly(project.uuid(), null, NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid1");
+    assertTableContainsOnly(project.uuid(), null, NewCodePeriodType.NUMBER_OF_DAYS, "20");
   }
 
   @Test
@@ -222,6 +224,163 @@ public class UnsetActionIT {
     assertTableEmpty();
   }
 
+  @Test
+  public void throw_IAE_if_unset_branch_NCD_and_project_NCD_not_compliant() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    ComponentDto branch = componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
+    db.newCodePeriods().insert(project.uuid(), null, NewCodePeriodType.NUMBER_OF_DAYS, "97");
+    db.newCodePeriods().insert(project.uuid(), branch.uuid(), NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid");
+
+    TestRequest request = ws.newRequest()
+      .setParam("project", project.getKey())
+      .setParam("branch", "branch");
+
+    logInAsProjectAdministrator(project);
+    assertThatThrownBy(() -> request.execute())
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessageContaining("Failed to unset the New Code Definition. Your project " +
+        "New Code Definition is not compatible with the Clean as You Code methodology. Please update your project New Code Definition");
+
+  }
+
+  @Test
+  public void throw_IAE_if_unset_branch_NCD_and_no_project_NCD_and_instance_NCD_not_compliant() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    ComponentDto branch = componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
+    db.newCodePeriods().insert(null, null, NewCodePeriodType.NUMBER_OF_DAYS, "97");
+    db.newCodePeriods().insert(project.uuid(), branch.uuid(), NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid");
+
+    TestRequest request = ws.newRequest()
+      .setParam("project", project.getKey())
+      .setParam("branch", "branch");
+
+    logInAsProjectAdministrator(project);
+    assertThatThrownBy(() -> request.execute())
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessageContaining("Failed to unset the New Code Definition. Your instance " +
+        "New Code Definition is not compatible with the Clean as You Code methodology. Please update your instance New Code Definition");
+  }
+
+  @Test
+  public void throw_IAE_if_unset_project_NCD_and_instance_NCD_not_compliant() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    db.newCodePeriods().insert(null, null, NewCodePeriodType.NUMBER_OF_DAYS, "97");
+    db.newCodePeriods().insert(project.uuid(), null, NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid");
+
+    logInAsProjectAdministrator(project);
+
+    TestRequest request = ws.newRequest()
+      .setParam("project", project.getKey());
+    assertThatThrownBy(() -> request.execute())
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessageContaining("Failed to unset the New Code Definition. Your instance " +
+        "New Code Definition is not compatible with the Clean as You Code methodology. Please update your instance New Code Definition");
+  }
+
+  @Test
+  public void do_not_throw_IAE_if_unset_project_NCD_and_no_instance_NCD() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    db.newCodePeriods().insert(project.uuid(), null, NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid");
+
+    logInAsProjectAdministrator(project);
+    ws.newRequest()
+      .setParam("project", project.getKey())
+      .execute();
+
+    assertTableEmpty();
+  }
+
+  @Test
+  public void do_not_throw_IAE_if_unset_branch_NCD_and_project_NCD_compliant() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    ComponentDto branch = componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
+    db.newCodePeriods().insert(project.uuid(), branch.uuid(), NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid");
+    db.newCodePeriods().insert(project.uuid(), null, NewCodePeriodType.PREVIOUS_VERSION, null);
+
+    logInAsProjectAdministrator(project);
+    ws.newRequest()
+      .setParam("project", project.getKey())
+      .setParam("branch", "branch")
+      .execute();
+
+    assertTableContainsOnly(project.uuid(), null, NewCodePeriodType.PREVIOUS_VERSION, null);
+  }
+
+  @Test
+  public void do_not_throw_IAE_if_unset_branch_NCD_and_project_NCD_not_compliant_and_no_branch_NCD() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
+    db.newCodePeriods().insert(project.uuid(), null, NewCodePeriodType.NUMBER_OF_DAYS, "93");
+
+    logInAsProjectAdministrator(project);
+    ws.newRequest()
+      .setParam("project", project.getKey())
+      .setParam("branch", "branch")
+      .execute();
+
+    assertTableContainsOnly(project.uuid(), null, NewCodePeriodType.NUMBER_OF_DAYS, "93");
+  }
+
+  @Test
+  public void do_not_throw_IAE_if_unset_branch_NCD_and_no_project_NCD_and_instance_NCD_compliant() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    ComponentDto branch = componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
+    db.newCodePeriods().insert(project.uuid(), branch.uuid(), NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid");
+    db.newCodePeriods().insert(null, null, NewCodePeriodType.PREVIOUS_VERSION, null);
+
+    logInAsProjectAdministrator(project);
+    ws.newRequest()
+      .setParam("project", project.getKey())
+      .setParam("branch", "branch")
+      .execute();
+
+    assertTableContainsOnly(null, null, NewCodePeriodType.PREVIOUS_VERSION, null);
+  }
+
+  @Test
+  public void do_not_throw_IAE_if_unset_branch_NCD_and_no_project_NCD_and_no_instance() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    ComponentDto branch = componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
+    db.newCodePeriods().insert(project.uuid(), branch.uuid(), NewCodePeriodType.SPECIFIC_ANALYSIS, "uuid");
+
+    logInAsProjectAdministrator(project);
+    ws.newRequest()
+      .setParam("project", project.getKey())
+      .setParam("branch", "branch")
+      .execute();
+
+    assertTableEmpty();
+  }
+
+  @Test
+  public void do_not_throw_IAE_if_unset_project_NCD_and_instance_NCD_compliant() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    ComponentDto branch = componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
+    db.newCodePeriods().insert(null, null, NewCodePeriodType.PREVIOUS_VERSION, null);
+    db.newCodePeriods().insert(project.uuid(), null, NewCodePeriodType.PREVIOUS_VERSION, null);
+
+    logInAsProjectAdministrator(project);
+    ws.newRequest()
+      .setParam("project", project.getKey())
+      .execute();
+
+    assertTableContainsOnly(null, null, NewCodePeriodType.PREVIOUS_VERSION, null);
+  }
+
+  @Test
+  public void do_not_throw_IAE_if_unset_project_NCD_and_instance_NCD_not_compliant_and_no_project_NCD() {
+    ComponentDto project = componentDb.insertPublicProject().getMainBranchComponent();
+    ComponentDto branch = componentDb.insertProjectBranch(project, b -> b.setKey("branch"));
+    db.newCodePeriods().insert(null, null, NewCodePeriodType.NUMBER_OF_DAYS, "93");
+
+    logInAsProjectAdministrator(project);
+    ws.newRequest()
+      .setParam("project", project.getKey())
+      .execute();
+
+    assertTableContainsOnly(null, null, NewCodePeriodType.NUMBER_OF_DAYS, "93");
+  }
+
   private void assertTableEmpty() {
     assertThat(db.countRowsOfTable(dbSession, "new_code_periods")).isZero();
   }
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/CaycUtils.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/CaycUtils.java
new file mode 100644 (file)
index 0000000..5a371ad
--- /dev/null
@@ -0,0 +1,39 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2023 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.server.newcodeperiod;
+
+import org.sonar.db.newcodeperiod.NewCodePeriodType;
+
+public interface CaycUtils {
+  static boolean isNewCodePeriodCompliant(NewCodePeriodType type, String value) {
+    if (type == NewCodePeriodType.NUMBER_OF_DAYS) {
+      return parseDays(value) > 0 && parseDays(value) <= 90;
+    }
+    return true;
+  }
+
+  static int parseDays(String value) {
+    try {
+      return Integer.parseInt(value);
+    } catch (Exception e) {
+      throw new IllegalArgumentException("Failed to parse number of days: " + value);
+    }
+  }
+}
index 0b313178a374f0e27b630e21c96a3a7ff13294b2..16dba5aa6a07e9574f3047de0622f0c68d3fbbea 100644 (file)
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.newcodeperiod.ws;
 
+import javax.annotation.Nullable;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
@@ -33,14 +34,18 @@ import org.sonar.db.newcodeperiod.NewCodePeriodDao;
 import org.sonar.db.project.ProjectDto;
 import org.sonar.server.component.ComponentFinder;
 import org.sonar.server.exceptions.NotFoundException;
+import org.sonar.server.newcodeperiod.CaycUtils;
 import org.sonar.server.user.UserSession;
 
 import static java.lang.String.format;
 import static org.sonar.server.ws.WsUtils.createHtmlExternalLink;
 
 public class UnsetAction implements NewCodePeriodsWsAction {
-  private static final String PARAM_BRANCH = "branch";
-  private static final String PARAM_PROJECT = "project";
+  private static final String BRANCH = "branch";
+  private static final String PROJECT = "project";
+  private static final String INSTANCE = "instance";
+  private static final String NON_COMPLIANT_CAYC_ERROR_MESSAGE = "Failed to unset the New Code Definition. Your %s " +
+    "New Code Definition is not compatible with the Clean as You Code methodology. Please update your %s New Code Definition";
 
   private final DbClient dbClient;
   private final UserSession userSession;
@@ -64,8 +69,8 @@ public class UnsetAction implements NewCodePeriodsWsAction {
     WebService.NewAction action = context.createAction("unset")
       .setPost(true)
       .setDescription("Unsets the " + createHtmlExternalLink(newCodeDefinitionDocumentationUrl, "new code definition") +
-        " for a branch, project or global.<br>" +
-        "Requires one of the following permissions: " +
+        " for a branch, project or global. It requires the inherited New Code Definition to be compatible with the Clean as You Code methodology, " +
+        "and one of the following permissions: " +
         "<ul>" +
         "<li>'Administer System' to change the global setting</li>" +
         "<li>'Administer' rights for a specified component</li>" +
@@ -73,16 +78,16 @@ public class UnsetAction implements NewCodePeriodsWsAction {
       .setSince("8.0")
       .setHandler(this);
 
-    action.createParam(PARAM_PROJECT)
+    action.createParam(PROJECT)
       .setDescription("Project key");
-    action.createParam(PARAM_BRANCH)
+    action.createParam(BRANCH)
       .setDescription("Branch key");
   }
 
   @Override
   public void handle(Request request, Response response) throws Exception {
-    String projectKey = request.getParam(PARAM_PROJECT).emptyAsNull().or(() -> null);
-    String branchKey = request.getParam(PARAM_BRANCH).emptyAsNull().or(() -> null);
+    String projectKey = request.getParam(PROJECT).emptyAsNull().or(() -> null);
+    String branchKey = request.getParam(BRANCH).emptyAsNull().or(() -> null);
 
     if (projectKey == null && branchKey != null) {
       throw new IllegalArgumentException("If branch key is specified, project key needs to be specified too");
@@ -105,6 +110,9 @@ public class UnsetAction implements NewCodePeriodsWsAction {
         } else if (isCommunityEdition) {
           branchUuid = getMainBranch(dbSession, project).getUuid();
         }
+
+        checkInheritedNcdCompliant(dbSession, projectUuid, branchUuid);
+
       } else {
         userSession.checkIsSystemAdministrator();
       }
@@ -114,11 +122,20 @@ public class UnsetAction implements NewCodePeriodsWsAction {
         // also delete project default in case it was somehow set (downgrade from another edition, for example)
         newCodePeriodDao.delete(dbSession, projectUuid, null);
       }
-
       dbSession.commit();
     }
   }
 
+  private void checkInheritedNcdCompliant(DbSession dbSession, String projectUuid, @Nullable String branchUuid) {
+    if (branchUuid != null) {
+      if (dbClient.newCodePeriodDao().selectByBranch(dbSession, projectUuid, branchUuid).isPresent()) {
+        checkBranchInheritedNcdCompliant(dbSession, projectUuid);
+      }
+    } else if (dbClient.newCodePeriodDao().selectByProject(dbSession, projectUuid).isPresent()) {
+      checkInstanceNcdCompliant(dbSession);
+    }
+  }
+
   private BranchDto getMainBranch(DbSession dbSession, ProjectDto project) {
     return dbClient.branchDao().selectByProject(dbSession, project)
       .stream().filter(BranchDto::isMain)
@@ -134,4 +151,27 @@ public class UnsetAction implements NewCodePeriodsWsAction {
   private ProjectDto getProject(DbSession dbSession, String projectKey) {
     return componentFinder.getProjectByKey(dbSession, projectKey);
   }
+
+  private void checkInstanceNcdCompliant(DbSession dbSession) {
+    var instanceNcd = newCodePeriodDao.selectGlobal(dbSession);
+    if (instanceNcd.isPresent()) {
+      var ncd = instanceNcd.get();
+      if (!CaycUtils.isNewCodePeriodCompliant(ncd.getType(), ncd.getValue())) {
+        throw new IllegalArgumentException(format(NON_COMPLIANT_CAYC_ERROR_MESSAGE, INSTANCE, INSTANCE));
+      }
+    }
+  }
+
+  private void checkBranchInheritedNcdCompliant(DbSession dbSession, String projectUuid) {
+    var projectNcd = newCodePeriodDao.selectByProject(dbSession, projectUuid);
+    if (projectNcd.isPresent()) {
+      var ncd = projectNcd.get();
+      if (!CaycUtils.isNewCodePeriodCompliant(ncd.getType(), ncd.getValue())) {
+        throw new IllegalArgumentException(format(NON_COMPLIANT_CAYC_ERROR_MESSAGE, PROJECT, PROJECT));
+      }
+    } else {
+      checkInstanceNcdCompliant(dbSession);
+    }
+  }
+
 }