From: Nolwenn Cadic <98824442+Nolwenn-cadic-sonarsource@users.noreply.github.com>
Date: Wed, 17 May 2023 12:56:50 +0000 (+0200)
Subject: SONAR-19294 Fail to unset NCD when parent NCD not CaYC compliant (#8314)
X-Git-Tag: 10.1.0.73491~236
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=01363ea39feba1f5bfec0d57ef7ee24d3f3a6d82;p=sonarqube.git
SONAR-19294 Fail to unset NCD when parent NCD not CaYC compliant (#8314)
---
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
index 00000000000..4dccd0a7a1f
--- /dev/null
+++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/CaycUtilsTest.java
@@ -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
diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/ws/UnsetActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/ws/UnsetActionIT.java
index 934d6f2ec0c..543ff39515c 100644
--- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/ws/UnsetActionIT.java
+++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/newcodeperiod/ws/UnsetActionIT.java
@@ -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
index 00000000000..5a371add3be
--- /dev/null
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/CaycUtils.java
@@ -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);
+ }
+ }
+}
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/ws/UnsetAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/ws/UnsetAction.java
index 0b313178a37..16dba5aa6a0 100644
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/ws/UnsetAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/newcodeperiod/ws/UnsetAction.java
@@ -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.
" +
- "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: " +
"" +
"- 'Administer System' to change the global setting
" +
"- 'Administer' rights for a specified component
" +
@@ -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);
+ }
+ }
+
}