diff options
25 files changed, 479 insertions, 100 deletions
diff --git a/server/sonar-db-dao/src/schema/schema-sq.ddl b/server/sonar-db-dao/src/schema/schema-sq.ddl index c6929b10b95..bb7e4166b03 100644 --- a/server/sonar-db-dao/src/schema/schema-sq.ddl +++ b/server/sonar-db-dao/src/schema/schema-sq.ddl @@ -1165,6 +1165,7 @@ CREATE TABLE "SCA_RELEASES"( ); ALTER TABLE "SCA_RELEASES" ADD CONSTRAINT "PK_SCA_RELEASES" PRIMARY KEY("UUID"); CREATE INDEX "SCA_RELEASES_COMP_UUID_UUID" ON "SCA_RELEASES"("COMPONENT_UUID" NULLS FIRST, "UUID" NULLS FIRST); +CREATE UNIQUE NULLS NOT DISTINCT INDEX "SCA_RELEASES_PACKAGE_URL_UNIQ" ON "SCA_RELEASES"("PACKAGE_URL" NULLS FIRST, "COMPONENT_UUID" NULLS FIRST); CREATE TABLE "SCA_VULNERABILITY_ISSUES"( "UUID" CHARACTER VARYING(40) NOT NULL, diff --git a/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v202503/CreateUniqueIndexOnScaReleasesIT.java b/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v202503/CreateUniqueIndexOnScaReleasesIT.java new file mode 100644 index 00000000000..9588ff8fe88 --- /dev/null +++ b/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v202503/CreateUniqueIndexOnScaReleasesIT.java @@ -0,0 +1,53 @@ +/* + * SonarQube + * Copyright (C) 2009-2025 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.platform.db.migration.version.v202503; + +import java.sql.SQLException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.sonar.db.MigrationDbTester; +import org.sonar.server.platform.db.migration.step.DdlChange; + +import static org.sonar.db.MigrationDbTester.createForMigrationStep; +import static org.sonar.server.platform.db.migration.version.v202503.CreateUniqueIndexOnScaReleases.COLUMN_NAME_COMPONENT_UUID; +import static org.sonar.server.platform.db.migration.version.v202503.CreateUniqueIndexOnScaReleases.COLUMN_NAME_PACKAGE_URL; +import static org.sonar.server.platform.db.migration.version.v202503.CreateUniqueIndexOnScaReleases.INDEX_NAME; +import static org.sonar.server.platform.db.migration.version.v202503.CreateUniqueIndexOnScaReleases.TABLE_NAME; + +class CreateUniqueIndexOnScaReleasesIT { + @RegisterExtension + public final MigrationDbTester db = createForMigrationStep(CreateUniqueIndexOnScaReleases.class); + private final DdlChange underTest = new CreateUniqueIndexOnScaReleases(db.database()); + + @Test + void execute_shouldCreateIndex() throws SQLException { + db.assertIndexDoesNotExist(TABLE_NAME, INDEX_NAME); + underTest.execute(); + db.assertUniqueIndex(TABLE_NAME, INDEX_NAME, COLUMN_NAME_PACKAGE_URL, COLUMN_NAME_COMPONENT_UUID); + } + + @Test + void execute_shouldBeReentrant() throws SQLException { + db.assertIndexDoesNotExist(TABLE_NAME, INDEX_NAME); + underTest.execute(); + underTest.execute(); + db.assertUniqueIndex(TABLE_NAME, INDEX_NAME, COLUMN_NAME_PACKAGE_URL, COLUMN_NAME_COMPONENT_UUID); + } +} diff --git a/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v202503/MigrateRemoveDuplicateScaReleasesIT.java b/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v202503/MigrateRemoveDuplicateScaReleasesIT.java new file mode 100644 index 00000000000..0daf88d66d8 --- /dev/null +++ b/server/sonar-db-migration/src/it/java/org/sonar/server/platform/db/migration/version/v202503/MigrateRemoveDuplicateScaReleasesIT.java @@ -0,0 +1,138 @@ +/* + * SonarQube + * Copyright (C) 2009-2025 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.platform.db.migration.version.v202503; + +import java.sql.SQLException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.sonar.db.MigrationDbTester; +import org.sonar.server.platform.db.migration.step.MigrationStep; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.db.MigrationDbTester.createForMigrationStep; + +class MigrateRemoveDuplicateScaReleasesIT { + @RegisterExtension + public final MigrationDbTester db = createForMigrationStep(MigrateRemoveDuplicateScaReleases.class); + private final MigrationStep underTest = new MigrateRemoveDuplicateScaReleases(db.database()); + + @Test + void test_removesDuplicates() throws SQLException { + // we should keep this one + insertRelease("0", "componentUuid1", "packageUrlNotDuplicated", 1L); + // we should keep these rows associated with release 0 + insertDependency("0", "scaReleaseUuid0"); + insertIssueRelease("0", "scaReleaseUuid0"); + insertIssueReleaseChange("0"); + // we should keep the first (oldest) packageUrl1 entry on componentUuid1 + insertRelease("1", "componentUuid1", "packageUrl1", 2L); + insertRelease("2", "componentUuid1", "packageUrl1", 3L); + insertRelease("3", "componentUuid1", "packageUrl1", 4L); + // we should delete these rows associated with release 3 that we delete + insertDependency("3", "scaReleaseUuid3"); + insertIssueRelease("3", "scaReleaseUuid3"); + insertIssueReleaseChange("3"); + // we should keep the first (oldest) packageUrl2 entry on componentUuid1 + insertRelease("4", "componentUuid1", "packageUrl2", 5L); + insertRelease("5", "componentUuid1", "packageUrl2", 6L); + // we should keep the first (oldest) packageUrl1 entry on componentUuid2 + insertRelease("6", "componentUuid2", "packageUrl1", 7L); + insertRelease("7", "componentUuid2", "packageUrl1", 8L); + // we should keep these rows associated with release 6 + insertDependency("6", "scaReleaseUuid6"); + insertIssueRelease("6", "scaReleaseUuid6"); + insertIssueReleaseChange("6"); + // we should delete these rows associated with release 7 that we delete + insertDependency("7", "scaReleaseUuid7"); + insertIssueRelease("7", "scaReleaseUuid7"); + insertIssueReleaseChange("7"); + + assertThat(db.countSql("select count(*) from sca_releases")).isEqualTo(8); + assertThat(db.countSql("select count(*) from sca_dependencies")).isEqualTo(4); + assertThat(db.countSql("select count(*) from sca_issues_releases")).isEqualTo(4); + assertThat(db.countSql("select count(*) from sca_issue_rels_changes")).isEqualTo(4); + underTest.execute(); + + assertThat(db.select("select uuid from sca_releases")).map(row -> row.get("uuid")) + .containsExactlyInAnyOrder("scaReleaseUuid0", "scaReleaseUuid1", "scaReleaseUuid4", "scaReleaseUuid6"); + assertThat(db.select("select uuid from sca_dependencies")).map(row -> row.get("uuid")) + .containsExactlyInAnyOrder("scaDependencyUuid0", "scaDependencyUuid6"); + assertThat(db.select("select uuid from sca_issues_releases")).map(row -> row.get("uuid")) + .containsExactlyInAnyOrder("scaIssueReleaseUuid0", "scaIssueReleaseUuid6"); + assertThat(db.select("select uuid from sca_issue_rels_changes")).map(row -> row.get("uuid")) + .containsExactlyInAnyOrder("scaIssueReleaseChangeUuid0", "scaIssueReleaseChangeUuid6"); + } + + @Test + void test_canRunMultipleTimesOnEmptyTable() throws SQLException { + assertThat(db.countSql("select count(*) from sca_releases")).isZero(); + underTest.execute(); + underTest.execute(); + assertThat(db.countSql("select count(*) from sca_releases")).isZero(); + } + + private void insertRelease(String suffix, String componentUuid, String packageUrl, long createdAt) { + db.executeInsert("sca_releases", + "uuid", "scaReleaseUuid" + suffix, + "component_uuid", componentUuid, + "package_url", packageUrl, + "package_manager", "MAVEN", + "package_name", "packageName", + "version", "1.0.0", + "license_expression", "MIT", + "declared_license_expression", "MIT", + "is_new", false, + "known", true, + "known_package", true, + "updated_at", 1L, + "created_at", createdAt); + } + + private void insertDependency(String suffix, String releaseUuid) { + db.executeInsert("sca_dependencies", + "uuid", "scaDependencyUuid" + suffix, + "sca_release_uuid", releaseUuid, + "direct", true, + "scope", "compile", + "is_new", false, + "updated_at", 1L, + "created_at", 2L); + } + + private void insertIssueRelease(String suffix, String releaseUuid) { + db.executeInsert("sca_issues_releases", + "uuid", "scaIssueReleaseUuid" + suffix, + "sca_release_uuid", releaseUuid, + "sca_issue_uuid", "scaIssueUuid" + suffix, + "severity", "LOW", + "severity_sort_key", 10, + "status", "OPEN", + "updated_at", 1L, + "created_at", 2L); + } + + private void insertIssueReleaseChange(String suffix) { + db.executeInsert("sca_issue_rels_changes", + "uuid", "scaIssueReleaseChangeUuid" + suffix, + "sca_issues_releases_uuid", "scaIssueReleaseUuid" + suffix, + "updated_at", 1L, + "created_at", 2L); + } +} diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/CreateUniqueIndexOnScaReleases.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/CreateUniqueIndexOnScaReleases.java new file mode 100644 index 00000000000..08afc724ab8 --- /dev/null +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/CreateUniqueIndexOnScaReleases.java @@ -0,0 +1,57 @@ +/* + * SonarQube + * Copyright (C) 2009-2025 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.platform.db.migration.version.v202503; + +import java.sql.Connection; +import java.sql.SQLException; +import org.sonar.db.Database; +import org.sonar.db.DatabaseUtils; +import org.sonar.server.platform.db.migration.sql.CreateIndexBuilder; +import org.sonar.server.platform.db.migration.step.DdlChange; + +public class CreateUniqueIndexOnScaReleases extends DdlChange { + static final String TABLE_NAME = "sca_releases"; + static final String INDEX_NAME = "sca_releases_package_url_uniq"; + static final String COLUMN_NAME_PACKAGE_URL = "package_url"; + static final String COLUMN_NAME_COMPONENT_UUID = "component_uuid"; + + public CreateUniqueIndexOnScaReleases(Database db) { + super(db); + } + + @Override + public void execute(Context context) throws SQLException { + try (Connection connection = getDatabase().getDataSource().getConnection()) { + createIndex(context, connection); + } + } + + private void createIndex(Context context, Connection connection) { + if (!DatabaseUtils.indexExistsIgnoreCase(TABLE_NAME, INDEX_NAME, connection)) { + context.execute(new CreateIndexBuilder(getDialect()) + .setTable(TABLE_NAME) + .setName(INDEX_NAME) + .setUnique(true) + .addColumn(COLUMN_NAME_PACKAGE_URL, false) + .addColumn(COLUMN_NAME_COMPONENT_UUID, false) + .build()); + } + } +} diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/DbVersion202503.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/DbVersion202503.java index a66a1884bce..1d95a39afb6 100644 --- a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/DbVersion202503.java +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/DbVersion202503.java @@ -64,6 +64,8 @@ public class DbVersion202503 implements DbVersion { .add(2025_03_025, "Create SCA encountered licenses unique index", CreateUniqueIndexOnScaEncounteredLicenses.class) .add(2025_03_026, "Add change_comment to SCA issues releases changes", AddCommentToScaIssuesReleasesChangesTable.class) .add(2025_03_027, "Drop change_type from SCA issues releases changes", DropChangeTypeFromScaIssuesReleasesChangesTable.class) + .add(2025_03_028, "Remove duplicates from SCA releases table", MigrateRemoveDuplicateScaReleases.class) + .add(2025_03_029, "Create unique index on SCA releases table", CreateUniqueIndexOnScaReleases.class) ; } diff --git a/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/MigrateRemoveDuplicateScaReleases.java b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/MigrateRemoveDuplicateScaReleases.java new file mode 100644 index 00000000000..de27c26d2c7 --- /dev/null +++ b/server/sonar-db-migration/src/main/java/org/sonar/server/platform/db/migration/version/v202503/MigrateRemoveDuplicateScaReleases.java @@ -0,0 +1,113 @@ +/* + * SonarQube + * Copyright (C) 2009-2025 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.platform.db.migration.version.v202503; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import org.sonar.db.Database; +import org.sonar.server.platform.db.migration.step.MigrationStep; + +public class MigrateRemoveDuplicateScaReleases implements MigrationStep { + static final String SELECT_BATCH_QUERY = """ + WITH duplicate_releases AS ( + SELECT + uuid, + ROW_NUMBER() OVER ( + PARTITION BY component_uuid, package_url + ORDER BY created_at ASC + ) AS row_num + FROM sca_releases + ) + SELECT + uuid + FROM duplicate_releases + WHERE row_num > 1 + """; + + static final String DELETE_BATCH_DEPENDENCIES_QUERY = """ + DELETE FROM sca_dependencies WHERE sca_release_uuid IN (?) + """; + + static final String DELETE_BATCH_ISSUES_RELEASES_CHANGES_QUERY = """ + DELETE FROM sca_issue_rels_changes WHERE sca_issues_releases_uuid IN (SELECT uuid FROM sca_issues_releases WHERE sca_release_uuid IN (?)) + """; + + static final String DELETE_BATCH_ISSUES_RELEASES_QUERY = """ + DELETE FROM sca_issues_releases WHERE sca_release_uuid IN (?) + """; + + static final String DELETE_BATCH_RELEASES_QUERY = """ + DELETE FROM sca_releases WHERE uuid IN (?) + """; + + private final Database db; + + public MigrateRemoveDuplicateScaReleases(Database db) { + this.db = db; + } + + private static List<String> findBatchOfDuplicates(Connection connection) throws SQLException { + List<String> results = new ArrayList<>(); + + try (PreparedStatement preparedStatement = connection.prepareStatement(SELECT_BATCH_QUERY)) { + preparedStatement.setMaxRows(999); + try (ResultSet resultSet = preparedStatement.executeQuery()) { + while (resultSet.next()) { + results.add(resultSet.getString(1)); + } + } + } + + return results; + } + + private static void deleteBatch(Connection connection, String batchSql, List<String> duplicateReleaseUuids) throws SQLException { + try (PreparedStatement preparedStatement = connection.prepareStatement(batchSql)) { + for (String uuid : duplicateReleaseUuids) { + preparedStatement.setString(1, uuid); + preparedStatement.addBatch(); + } + preparedStatement.executeBatch(); + } + } + + private static void deleteBatchOfDuplicates(Connection connection, List<String> duplicateRowUuids) throws SQLException { + deleteBatch(connection, DELETE_BATCH_DEPENDENCIES_QUERY, duplicateRowUuids); + deleteBatch(connection, DELETE_BATCH_ISSUES_RELEASES_CHANGES_QUERY, duplicateRowUuids); + deleteBatch(connection, DELETE_BATCH_ISSUES_RELEASES_QUERY, duplicateRowUuids); + deleteBatch(connection, DELETE_BATCH_RELEASES_QUERY, duplicateRowUuids); + } + + @Override + public void execute() throws SQLException { + try (var connection = db.getDataSource().getConnection()) { + List<String> duplicateRowUuids = findBatchOfDuplicates(connection); + while (!duplicateRowUuids.isEmpty()) { + deleteBatchOfDuplicates(connection, duplicateRowUuids); + connection.commit(); + duplicateRowUuids = findBatchOfDuplicates(connection); + } + } + } +} diff --git a/server/sonar-server-common/build.gradle b/server/sonar-server-common/build.gradle index 2aefc235c70..5533700fdac 100644 --- a/server/sonar-server-common/build.gradle +++ b/server/sonar-server-common/build.gradle @@ -29,7 +29,10 @@ dependencies { api project(':sonar-markdown') api project(':sonar-ws') + implementation project(':server:sonar-statemachine') + compileOnlyApi 'com.github.spotbugs:spotbugs-annotations' + testImplementation 'org.elasticsearch.plugin:transport-netty4-client' testImplementation 'ch.qos.logback:logback-core' testImplementation 'com.github.spotbugs:spotbugs-annotations' diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java index abda9fc29d8..82da993207e 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/IssueWorkflow.java @@ -27,7 +27,6 @@ import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.rule.RuleType; import org.sonar.server.issue.workflow.codequalityissue.CodeQualityIssueWorkflow; import org.sonar.server.issue.workflow.securityhotspot.SecurityHotspotWorkflow; -import org.sonar.server.issue.workflow.statemachine.Transition; /** * Common entry point for both issues and security hotspots, because some features are not making the difference. @@ -57,11 +56,11 @@ public class IssueWorkflow { } } - public List<Transition> outTransitions(DefaultIssue issue) { + public List<String> outTransitionsKeys(DefaultIssue issue) { if (isSecurityHotspot(issue)) { - return securityHotspotWorkflow.outTransitions(issue); + return securityHotspotWorkflow.outTransitionsKeys(issue); } else { - return codeQualityIssueWorkflow.outTransitions(issue); + return codeQualityIssueWorkflow.outTransitionsKeys(issue); } } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/codequalityissue/CodeQualityIssueWorkflow.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/codequalityissue/CodeQualityIssueWorkflow.java index 1325e685b97..750fcc81107 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/codequalityissue/CodeQualityIssueWorkflow.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/codequalityissue/CodeQualityIssueWorkflow.java @@ -29,10 +29,10 @@ import org.sonar.api.issue.IssueStatus; import org.sonar.api.server.ServerSide; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; +import org.sonar.issue.workflow.statemachine.State; +import org.sonar.issue.workflow.statemachine.Transition; import org.sonar.server.issue.TaintChecker; import org.sonar.server.issue.workflow.issue.IssueWorkflowEntityAdapter; -import org.sonar.server.issue.workflow.statemachine.State; -import org.sonar.server.issue.workflow.statemachine.Transition; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; @@ -70,17 +70,19 @@ public class CodeQualityIssueWorkflow { } public Set<CodeQualityIssueWorkflowTransition> outTransitionsEnums(DefaultIssue issue) { - return outTransitions(issue).stream().map(Transition::key) + return outTransitionsKeys(issue).stream() .map(CodeQualityIssueWorkflowTransition::fromKey) .flatMap(Optional::stream) .collect(Collectors.toSet()); } - public List<Transition> outTransitions(DefaultIssue issue) { + public List<String> outTransitionsKeys(DefaultIssue issue) { String status = issue.status(); State<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions> state = workflowDefinition.getMachine().state(status); checkArgument(state != null, "Unknown status: %s", status); - return state.outManualTransitions(adapt(issue)).stream().map(t -> (Transition) t).toList(); + return state.outManualTransitions(adapt(issue)).stream() + .map(Transition::key) + .toList(); } public void doAutomaticTransition(DefaultIssue issue, IssueChangeContext issueChangeContext) { diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/codequalityissue/CodeQualityIssueWorkflowDefinition.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/codequalityissue/CodeQualityIssueWorkflowDefinition.java index a190ca1b144..389d1697c5f 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/codequalityissue/CodeQualityIssueWorkflowDefinition.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/codequalityissue/CodeQualityIssueWorkflowDefinition.java @@ -23,9 +23,8 @@ import java.util.function.Consumer; import java.util.function.Predicate; import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.server.ServerSide; -import org.sonar.db.permission.ProjectPermission; -import org.sonar.server.issue.workflow.statemachine.StateMachine; -import org.sonar.server.issue.workflow.statemachine.Transition; +import org.sonar.issue.workflow.statemachine.StateMachine; +import org.sonar.issue.workflow.statemachine.Transition; import static org.sonar.api.issue.Issue.RESOLUTION_FALSE_POSITIVE; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; @@ -78,34 +77,28 @@ public class CodeQualityIssueWorkflowDefinition { .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(ACCEPT.getKey()) .from(STATUS_OPEN).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_WONT_FIX), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(ACCEPT.getKey()) .from(STATUS_REOPENED).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_WONT_FIX), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(ACCEPT.getKey()) .from(STATUS_CONFIRMED).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_WONT_FIX), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) // resolve as false-positive .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(FALSE_POSITIVE.getKey()) .from(STATUS_OPEN).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_FALSE_POSITIVE), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(FALSE_POSITIVE.getKey()) .from(STATUS_REOPENED).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_FALSE_POSITIVE), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(FALSE_POSITIVE.getKey()) .from(STATUS_CONFIRMED).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_FALSE_POSITIVE), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) // reopen @@ -132,34 +125,28 @@ public class CodeQualityIssueWorkflowDefinition { .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(RESOLVE.getKey()) .from(STATUS_OPEN).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_FIXED)) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(RESOLVE.getKey()) .from(STATUS_REOPENED).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_FIXED)) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(RESOLVE.getKey()) .from(STATUS_CONFIRMED).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_FIXED)) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) // resolve as won't fix, deprecated .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(WONT_FIX.getKey()) .from(STATUS_OPEN).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_WONT_FIX), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(WONT_FIX.getKey()) .from(STATUS_REOPENED).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_WONT_FIX), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()) .transition(Transition.<CodeQualityIssueWorkflowEntity, CodeQualityIssueWorkflowActions>builder(WONT_FIX.getKey()) .from(STATUS_CONFIRMED).to(STATUS_RESOLVED) .actions(a -> a.setResolution(RESOLUTION_WONT_FIX), UNSET_ASSIGNEE) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build()); } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/securityhotspot/SecurityHotspotWorkflow.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/securityhotspot/SecurityHotspotWorkflow.java index 93bbf8c5906..cffefe28741 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/securityhotspot/SecurityHotspotWorkflow.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/securityhotspot/SecurityHotspotWorkflow.java @@ -26,9 +26,9 @@ import org.sonar.api.issue.IssueStatus; import org.sonar.api.server.ServerSide; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; +import org.sonar.issue.workflow.statemachine.State; +import org.sonar.issue.workflow.statemachine.Transition; import org.sonar.server.issue.workflow.issue.IssueWorkflowEntityAdapter; -import org.sonar.server.issue.workflow.statemachine.State; -import org.sonar.server.issue.workflow.statemachine.Transition; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; @@ -58,11 +58,14 @@ public class SecurityHotspotWorkflow { return false; } - public List<Transition> outTransitions(DefaultIssue issue) { + public List<String> outTransitionsKeys(DefaultIssue issue) { String status = issue.status(); State<SecurityHotspotWorkflowEntity, SecurityHotspotWorkflowActions> state = workflowDefinition.getMachine().state(status); checkArgument(state != null, "Unknown status: %s", status); - return state.outManualTransitions(adapt(issue)).stream().map(t -> (Transition) t).toList(); + return state.outManualTransitions(adapt(issue)) + .stream() + .map(Transition::key) + .toList(); } public void doAutomaticTransition(DefaultIssue issue, IssueChangeContext issueChangeContext) { diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/securityhotspot/SecurityHotspotWorkflowDefinition.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/securityhotspot/SecurityHotspotWorkflowDefinition.java index c40a955df85..8c6793221af 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/securityhotspot/SecurityHotspotWorkflowDefinition.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/securityhotspot/SecurityHotspotWorkflowDefinition.java @@ -22,9 +22,8 @@ package org.sonar.server.issue.workflow.securityhotspot; import java.util.function.Consumer; import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.server.ServerSide; -import org.sonar.db.permission.ProjectPermission; -import org.sonar.server.issue.workflow.statemachine.StateMachine; -import org.sonar.server.issue.workflow.statemachine.Transition; +import org.sonar.issue.workflow.statemachine.StateMachine; +import org.sonar.issue.workflow.statemachine.Transition; import static org.sonar.api.issue.Issue.RESOLUTION_ACKNOWLEDGED; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; @@ -71,8 +70,7 @@ public class SecurityHotspotWorkflowDefinition { Transition.TransitionBuilder<SecurityHotspotWorkflowEntity, SecurityHotspotWorkflowActions> reviewedAsFixedBuilder = Transition .<SecurityHotspotWorkflowEntity, SecurityHotspotWorkflowActions>builder(RESOLVE_AS_REVIEWED.getKey()) .to(STATUS_REVIEWED) - .actions(a -> a.setResolution(RESOLUTION_FIXED)) - .requiredProjectPermission(ProjectPermission.SECURITYHOTSPOT_ADMIN); + .actions(a -> a.setResolution(RESOLUTION_FIXED)); builder .transition(reviewedAsFixedBuilder .from(STATUS_TO_REVIEW) @@ -86,8 +84,7 @@ public class SecurityHotspotWorkflowDefinition { Transition.TransitionBuilder<SecurityHotspotWorkflowEntity, SecurityHotspotWorkflowActions> resolveAsSafeTransitionBuilder = Transition .<SecurityHotspotWorkflowEntity, SecurityHotspotWorkflowActions>builder(RESOLVE_AS_SAFE.getKey()) .to(STATUS_REVIEWED) - .actions(a -> a.setResolution(RESOLUTION_SAFE)) - .requiredProjectPermission(ProjectPermission.SECURITYHOTSPOT_ADMIN); + .actions(a -> a.setResolution(RESOLUTION_SAFE)); builder .transition(resolveAsSafeTransitionBuilder .from(STATUS_TO_REVIEW) @@ -101,8 +98,7 @@ public class SecurityHotspotWorkflowDefinition { Transition.TransitionBuilder<SecurityHotspotWorkflowEntity, SecurityHotspotWorkflowActions> resolveAsAcknowledgedTransitionBuilder = Transition .<SecurityHotspotWorkflowEntity, SecurityHotspotWorkflowActions>builder(RESOLVE_AS_ACKNOWLEDGED.getKey()) .to(STATUS_REVIEWED) - .actions(a -> a.setResolution(RESOLUTION_ACKNOWLEDGED)) - .requiredProjectPermission(ProjectPermission.SECURITYHOTSPOT_ADMIN); + .actions(a -> a.setResolution(RESOLUTION_ACKNOWLEDGED)); builder .transition(resolveAsAcknowledgedTransitionBuilder .from(STATUS_TO_REVIEW) @@ -118,7 +114,6 @@ public class SecurityHotspotWorkflowDefinition { .from(STATUS_REVIEWED).to(STATUS_TO_REVIEW) .conditions(sh -> sh.hasAnyResolution(RESOLUTION_FIXED, RESOLUTION_SAFE, RESOLUTION_ACKNOWLEDGED)) .actions(UNSET_RESOLUTION) - .requiredProjectPermission(ProjectPermission.SECURITYHOTSPOT_ADMIN) .build()); } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForCodeQualityIssuesTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForCodeQualityIssuesTest.java index b49f7e03d1e..386b3666c26 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForCodeQualityIssuesTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForCodeQualityIssuesTest.java @@ -19,10 +19,8 @@ */ package org.sonar.server.issue.workflow; -import com.google.common.collect.Collections2; import java.util.Arrays; import java.util.Calendar; -import java.util.Collection; import java.util.Date; import java.util.List; import java.util.Random; @@ -44,7 +42,6 @@ import org.sonar.server.issue.workflow.codequalityissue.CodeQualityIssueWorkflow import org.sonar.server.issue.workflow.codequalityissue.CodeQualityIssueWorkflowActionsFactory; import org.sonar.server.issue.workflow.codequalityissue.CodeQualityIssueWorkflowDefinition; import org.sonar.server.issue.workflow.codequalityissue.CodeQualityIssueWorkflowTransition; -import org.sonar.server.issue.workflow.statemachine.Transition; import static com.google.common.base.Preconditions.checkArgument; import static org.apache.commons.lang3.time.DateUtils.addDays; @@ -85,35 +82,35 @@ class IssueWorkflowForCodeQualityIssuesTest { @Test void list_out_transitions_from_status_open() { DefaultIssue issue = new DefaultIssue().setStatus(STATUS_OPEN); - List<Transition> transitions = underTest.outTransitions(issue); - assertThat(keys(transitions)).containsOnly("confirm", "falsepositive", "resolve", "wontfix", "accept"); + List<String> transitions = underTest.outTransitionsKeys(issue); + assertThat(transitions).containsOnly("confirm", "falsepositive", "resolve", "wontfix", "accept"); } @Test void list_out_transitions_from_status_confirmed() { DefaultIssue issue = new DefaultIssue().setStatus(STATUS_CONFIRMED); - List<Transition> transitions = underTest.outTransitions(issue); - assertThat(keys(transitions)).containsOnly("unconfirm", "falsepositive", "resolve", "wontfix", "accept"); + List<String> transitions = underTest.outTransitionsKeys(issue); + assertThat(transitions).containsOnly("unconfirm", "falsepositive", "resolve", "wontfix", "accept"); } @Test void list_out_transitions_from_status_resolved() { DefaultIssue issue = new DefaultIssue().setStatus(STATUS_RESOLVED); - List<Transition> transitions = underTest.outTransitions(issue); - assertThat(keys(transitions)).containsOnly("reopen"); + List<String> transitions = underTest.outTransitionsKeys(issue); + assertThat(transitions).containsOnly("reopen"); } @Test void list_out_transitions_from_status_reopen() { DefaultIssue issue = new DefaultIssue().setStatus(STATUS_REOPENED); - List<Transition> transitions = underTest.outTransitions(issue); - assertThat(keys(transitions)).containsOnly("confirm", "resolve", "falsepositive", "wontfix", "accept"); + List<String> transitions = underTest.outTransitionsKeys(issue); + assertThat(transitions).containsOnly("confirm", "resolve", "falsepositive", "wontfix", "accept"); } @Test void list_no_out_transition_from_status_closed() { DefaultIssue issue = new DefaultIssue().setStatus(STATUS_CLOSED).setRuleKey(RuleKey.of("java", "R1 ")); - List<Transition> transitions = underTest.outTransitions(issue); + List<String> transitions = underTest.outTransitionsKeys(issue); assertThat(transitions).isEmpty(); } @@ -121,7 +118,7 @@ class IssueWorkflowForCodeQualityIssuesTest { void fail_if_unknown_status_when_listing_transitions() { DefaultIssue issue = new DefaultIssue().setStatus("xxx"); try { - underTest.outTransitions(issue); + underTest.outTransitionsKeys(issue); fail(); } catch (IllegalArgumentException e) { assertThat(e).hasMessage("Unknown status: xxx"); @@ -500,7 +497,4 @@ class IssueWorkflowForCodeQualityIssuesTest { return newResolution == null ? "" : newResolution; } - private Collection<String> keys(List<Transition> transitions) { - return Collections2.transform(transitions, Transition::key); - } } diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java index 9c7b4b6346c..7f5e8691158 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/IssueWorkflowForSecurityHotspotsTest.java @@ -42,7 +42,6 @@ import org.sonar.server.issue.workflow.securityhotspot.SecurityHotspotWorkflow; import org.sonar.server.issue.workflow.securityhotspot.SecurityHotspotWorkflowActionsFactory; import org.sonar.server.issue.workflow.securityhotspot.SecurityHotspotWorkflowDefinition; import org.sonar.server.issue.workflow.securityhotspot.SecurityHotspotWorkflowTransition; -import org.sonar.server.issue.workflow.statemachine.Transition; import static org.apache.commons.lang3.RandomStringUtils.secure; import static org.assertj.core.api.Assertions.assertThat; @@ -77,9 +76,9 @@ public class IssueWorkflowForSecurityHotspotsTest { public void to_review_hotspot_with_any_resolution_can_be_resolved_as_safe_or_fixed(@Nullable String resolution) { DefaultIssue hotspot = newHotspot(STATUS_TO_REVIEW, resolution); - List<Transition> transitions = underTest.outTransitions(hotspot); + List<String> transitions = underTest.outTransitionsKeys(hotspot); - assertThat(keys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_REVIEWED, RESOLVE_AS_SAFE, RESOLVE_AS_ACKNOWLEDGED); + assertThat(fromKeys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_REVIEWED, RESOLVE_AS_SAFE, RESOLVE_AS_ACKNOWLEDGED); } @DataProvider @@ -97,18 +96,18 @@ public class IssueWorkflowForSecurityHotspotsTest { public void reviewed_as_fixed_hotspot_can_be_resolved_as_safe_or_put_back_to_review() { DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_FIXED); - List<Transition> transitions = underTest.outTransitions(hotspot); + List<String> transitions = underTest.outTransitionsKeys(hotspot); - assertThat(keys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_SAFE, RESET_AS_TO_REVIEW, RESOLVE_AS_ACKNOWLEDGED); + assertThat(fromKeys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_SAFE, RESET_AS_TO_REVIEW, RESOLVE_AS_ACKNOWLEDGED); } @Test public void reviewed_as_safe_hotspot_can_be_resolved_as_fixed_or_put_back_to_review() { DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, RESOLUTION_SAFE); - List<Transition> transitions = underTest.outTransitions(hotspot); + List<String> transitions = underTest.outTransitionsKeys(hotspot); - assertThat(keys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_REVIEWED, RESET_AS_TO_REVIEW, RESOLVE_AS_ACKNOWLEDGED); + assertThat(fromKeys(transitions)).containsExactlyInAnyOrder(RESOLVE_AS_REVIEWED, RESET_AS_TO_REVIEW, RESOLVE_AS_ACKNOWLEDGED); } @Test @@ -116,7 +115,7 @@ public class IssueWorkflowForSecurityHotspotsTest { public void reviewed_with_any_resolution_but_safe_or_fixed_can_not_be_changed(String resolution) { DefaultIssue hotspot = newHotspot(STATUS_REVIEWED, resolution); - List<Transition> transitions = underTest.outTransitions(hotspot); + List<String> transitions = underTest.outTransitionsKeys(hotspot); assertThat(transitions).isEmpty(); } @@ -283,8 +282,8 @@ public class IssueWorkflowForSecurityHotspotsTest { assertThat(hotspot.resolution()).isNull(); } - private Collection<SecurityHotspotWorkflowTransition> keys(List<Transition> transitions) { - return transitions.stream().map(Transition::key).map(SecurityHotspotWorkflowTransition::fromKey).flatMap(Optional::stream).toList(); + private Collection<SecurityHotspotWorkflowTransition> fromKeys(List<String> transitionKeys) { + return transitionKeys.stream().map(SecurityHotspotWorkflowTransition::fromKey).flatMap(Optional::stream).toList(); } private static void setStatusPreviousToClosed(DefaultIssue hotspot, String previousStatus, @Nullable String previousResolution, @Nullable String newResolution) { diff --git a/server/sonar-statemachine/build.gradle b/server/sonar-statemachine/build.gradle new file mode 100644 index 00000000000..d15abf7c90d --- /dev/null +++ b/server/sonar-statemachine/build.gradle @@ -0,0 +1,29 @@ +description = 'State machine used for issue workflow, can be later extracted and shared with other products' + +sonar { + properties { + property 'sonar.projectName', "${projectTitle} :: Server :: State Machine" + } +} + +dependencies { + // Please don't add dependency on other SonarQube modules, as this library might be extracted in its own repo + implementation 'com.google.guava:guava' + implementation 'org.apache.commons:commons-lang3' + + compileOnlyApi 'com.github.spotbugs:spotbugs-annotations' + + testImplementation 'org.assertj:assertj-core' + testImplementation 'org.junit.jupiter:junit-jupiter-api' + testImplementation 'org.junit.jupiter:junit-jupiter-params' + testImplementation 'org.mockito:mockito-core' + testImplementation 'org.mockito:mockito-junit-jupiter' + + testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine' + testRuntimeOnly 'org.junit.vintage:junit-vintage-engine' +} + +test { + // Enabling the JUnit Platform (see https://github.com/junit-team/junit5-samples/tree/master/junit5-migration-gradle) + useJUnitPlatform() +} diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/statemachine/State.java b/server/sonar-statemachine/src/main/java/org/sonar/issue/workflow/statemachine/State.java index 135bde69311..db02ed2a457 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/statemachine/State.java +++ b/server/sonar-statemachine/src/main/java/org/sonar/issue/workflow/statemachine/State.java @@ -17,7 +17,7 @@ * 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.issue.workflow.statemachine; +package org.sonar.issue.workflow.statemachine; import com.google.common.base.Preconditions; import com.google.common.base.Strings; diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/statemachine/StateMachine.java b/server/sonar-statemachine/src/main/java/org/sonar/issue/workflow/statemachine/StateMachine.java index c719c64580b..ba7824d46dc 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/statemachine/StateMachine.java +++ b/server/sonar-statemachine/src/main/java/org/sonar/issue/workflow/statemachine/StateMachine.java @@ -17,7 +17,7 @@ * 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.issue.workflow.statemachine; +package org.sonar.issue.workflow.statemachine; import com.google.common.base.Preconditions; import com.google.common.collect.ArrayListMultimap; diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/statemachine/Transition.java b/server/sonar-statemachine/src/main/java/org/sonar/issue/workflow/statemachine/Transition.java index 36603adfee1..389983d1145 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/statemachine/Transition.java +++ b/server/sonar-statemachine/src/main/java/org/sonar/issue/workflow/statemachine/Transition.java @@ -17,16 +17,14 @@ * 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.issue.workflow.statemachine; +package org.sonar.issue.workflow.statemachine; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.function.Consumer; import java.util.function.Predicate; -import javax.annotation.CheckForNull; import org.apache.commons.lang3.StringUtils; -import org.sonar.db.permission.ProjectPermission; import static com.google.common.base.Preconditions.checkArgument; @@ -42,7 +40,6 @@ public class Transition<E, A> { private final List<Predicate<E>> conditions; private final List<Consumer<A>> actions; private final boolean automatic; - private final ProjectPermission requiredProjectPermission; private Transition(TransitionBuilder<E, A> builder) { key = builder.key; @@ -51,7 +48,6 @@ public class Transition<E, A> { conditions = List.copyOf(builder.conditions); actions = List.copyOf(builder.actions); automatic = builder.automatic; - requiredProjectPermission = builder.requiredProjectPermission; } public String key() { @@ -87,11 +83,6 @@ public class Transition<E, A> { return true; } - @CheckForNull - public ProjectPermission requiredProjectPermission() { - return requiredProjectPermission; - } - @Override public String toString() { return String.format("%s->%s->%s", from, key, to); @@ -112,7 +103,6 @@ public class Transition<E, A> { private final List<Predicate<E>> conditions = new ArrayList<>(); private final List<Consumer<A>> actions = new ArrayList<>(); private boolean automatic = false; - private ProjectPermission requiredProjectPermission; private TransitionBuilder(String key) { this.key = key; @@ -145,11 +135,6 @@ public class Transition<E, A> { return this; } - public TransitionBuilder<E, A> requiredProjectPermission(ProjectPermission requiredProjectPermission) { - this.requiredProjectPermission = requiredProjectPermission; - return this; - } - public Transition<E, A> build() { checkArgument(StringUtils.isNotEmpty(key), "Transition key must be set"); checkArgument(StringUtils.isAllLowerCase(key), "Transition key must be lower-case"); diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/statemachine/package-info.java b/server/sonar-statemachine/src/main/java/org/sonar/issue/workflow/statemachine/package-info.java index d288e191fed..8f09e812c72 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/statemachine/package-info.java +++ b/server/sonar-statemachine/src/main/java/org/sonar/issue/workflow/statemachine/package-info.java @@ -18,6 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ @ParametersAreNonnullByDefault -package org.sonar.server.issue.workflow.statemachine; +package org.sonar.issue.workflow.statemachine; import javax.annotation.ParametersAreNonnullByDefault; diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/statemachine/StateMachineTest.java b/server/sonar-statemachine/src/test/java/org/sonar/issue/workflow/statemachine/StateMachineTest.java index e1a417b597b..d52b59d910e 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/statemachine/StateMachineTest.java +++ b/server/sonar-statemachine/src/test/java/org/sonar/issue/workflow/statemachine/StateMachineTest.java @@ -17,7 +17,7 @@ * 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.issue.workflow.statemachine; +package org.sonar.issue.workflow.statemachine; import org.junit.jupiter.api.Test; diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/statemachine/StateTest.java b/server/sonar-statemachine/src/test/java/org/sonar/issue/workflow/statemachine/StateTest.java index a9600a7ce16..7feaa50f1f7 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/statemachine/StateTest.java +++ b/server/sonar-statemachine/src/test/java/org/sonar/issue/workflow/statemachine/StateTest.java @@ -17,7 +17,7 @@ * 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.issue.workflow.statemachine; +package org.sonar.issue.workflow.statemachine; import java.util.List; import org.junit.jupiter.api.Test; diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/statemachine/TransitionTest.java b/server/sonar-statemachine/src/test/java/org/sonar/issue/workflow/statemachine/TransitionTest.java index 9219688961e..6654c2f9c18 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/workflow/statemachine/TransitionTest.java +++ b/server/sonar-statemachine/src/test/java/org/sonar/issue/workflow/statemachine/TransitionTest.java @@ -17,11 +17,10 @@ * 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.issue.workflow.statemachine; +package org.sonar.issue.workflow.statemachine; import java.util.function.Predicate; import org.junit.jupiter.api.Test; -import org.sonar.db.permission.ProjectPermission; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -50,7 +49,6 @@ class TransitionTest { .from("OPEN").to("CLOSED") .conditions(condition1, condition2) .actions(WfEntityActions::action1, WfEntityActions::action2) - .requiredProjectPermission(ProjectPermission.ISSUE_ADMIN) .build(); assertThat(transition.key()).isEqualTo("close"); assertThat(transition.from()).isEqualTo("OPEN"); @@ -58,7 +56,6 @@ class TransitionTest { assertThat(transition.conditions()).containsOnly(condition1, condition2); assertThat(transition.actions()).hasSize(2); assertThat(transition.automatic()).isFalse(); - assertThat(transition.requiredProjectPermission()).isEqualTo(ProjectPermission.ISSUE_ADMIN); } @Test @@ -71,7 +68,6 @@ class TransitionTest { assertThat(transition.to()).isEqualTo("CLOSED"); assertThat(transition.conditions()).isEmpty(); assertThat(transition.actions()).isEmpty(); - assertThat(transition.requiredProjectPermission()).isNull(); } @Test diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/issue/TransitionServiceIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/issue/TransitionServiceIT.java index c9745bd28d4..12740b6e639 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/issue/TransitionServiceIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/issue/TransitionServiceIT.java @@ -23,7 +23,6 @@ import java.util.Date; import java.util.List; import org.junit.Rule; import org.junit.Test; -import org.sonar.api.issue.Issue; import org.sonar.core.issue.DefaultIssue; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; @@ -37,7 +36,6 @@ import org.sonar.server.issue.workflow.codequalityissue.CodeQualityIssueWorkflow import org.sonar.server.issue.workflow.securityhotspot.SecurityHotspotWorkflow; import org.sonar.server.issue.workflow.securityhotspot.SecurityHotspotWorkflowActionsFactory; import org.sonar.server.issue.workflow.securityhotspot.SecurityHotspotWorkflowDefinition; -import org.sonar.server.issue.workflow.statemachine.Transition; import org.sonar.server.tester.UserSessionRule; import static org.assertj.core.api.Assertions.assertThat; diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/TransitionService.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/TransitionService.java index 0e84d93e923..4da0c931330 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/TransitionService.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/TransitionService.java @@ -20,20 +20,25 @@ package org.sonar.server.issue; import java.util.List; +import java.util.Set; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; +import org.sonar.core.rule.RuleType; +import org.sonar.db.permission.ProjectPermission; import org.sonar.server.issue.workflow.IssueWorkflow; import org.sonar.server.issue.workflow.WorkflowTransition; -import org.sonar.server.issue.workflow.statemachine.Transition; import org.sonar.server.user.UserSession; import static java.util.Objects.requireNonNull; +import static org.sonar.server.issue.workflow.codequalityissue.CodeQualityIssueWorkflowTransition.CONFIRM; +import static org.sonar.server.issue.workflow.codequalityissue.CodeQualityIssueWorkflowTransition.UNCONFIRM; /** * This service is a kind of overlay of {@link IssueWorkflow} that also deals with permission checking */ public class TransitionService { + public static final Set<String> CONFIRM_TRANSITION_KEYS = Set.of(UNCONFIRM.getKey(), CONFIRM.getKey()); private final UserSession userSession; private final IssueWorkflow workflow; @@ -44,11 +49,16 @@ public class TransitionService { public List<String> listTransitionKeys(DefaultIssue issue) { String projectUuid = requireNonNull(issue.projectUuid()); - return workflow.outTransitions(issue) + return workflow.outTransitionsKeys(issue) .stream() - .filter(transition -> (userSession.isLoggedIn() && transition.requiredProjectPermission() == null) - || (transition.requiredProjectPermission() != null && userSession.hasComponentUuidPermission(transition.requiredProjectPermission(), projectUuid))) - .map(Transition::key) + .filter(key -> { + // Confirm is an exception and is accessible to any logged-in user + if (CONFIRM_TRANSITION_KEYS.contains(key)) { + return userSession.isLoggedIn(); + } else { + return userSession.hasComponentUuidPermission(getProjectPermissionForIssueType(issue), projectUuid); + } + }) .toList(); } @@ -66,10 +76,24 @@ public class TransitionService { public void checkTransitionPermission(String transitionKey, DefaultIssue defaultIssue) { String projectUuid = requireNonNull(defaultIssue.projectUuid()); - workflow.outTransitions(defaultIssue) + workflow.outTransitionsKeys(defaultIssue) .stream() - .filter(transition -> transition.key().equals(transitionKey) && transition.requiredProjectPermission() != null) - .forEach(transition -> userSession.checkComponentUuidPermission(transition.requiredProjectPermission(), projectUuid)); + .filter(key -> key.equals(transitionKey)) + .forEach(transition -> { + // Confirm is an exception and is accessible to any logged-in user + if (CONFIRM_TRANSITION_KEYS.contains(transitionKey)) { + return; + } + userSession.checkComponentUuidPermission(getProjectPermissionForIssueType(defaultIssue), projectUuid); + }); + } + + private static ProjectPermission getProjectPermissionForIssueType(DefaultIssue defaultIssue) { + return isSecurityHotspot(defaultIssue) ? ProjectPermission.SECURITYHOTSPOT_ADMIN : ProjectPermission.ISSUE_ADMIN; + } + + private static boolean isSecurityHotspot(DefaultIssue issue) { + return issue.type() == RuleType.SECURITY_HOTSPOT; } } diff --git a/settings.gradle b/settings.gradle index d36d04c6cce..bd7c6754f48 100644 --- a/settings.gradle +++ b/settings.gradle @@ -44,6 +44,7 @@ include 'server:sonar-db-migration' include 'server:sonar-main' include 'server:sonar-process' include 'server:sonar-server-common' +include 'server:sonar-statemachine' include 'server:sonar-telemetry' include 'server:sonar-telemetry-core' include 'server:sonar-webserver' |