From bde46f38a61a418cbe88adf46c8c5b2de4fea8a2 Mon Sep 17 00:00:00 2001 From: Zipeng WU Date: Thu, 4 Nov 2021 16:58:20 +0100 Subject: [PATCH] SONAR-15397 Fix Issue tags aren't filtered by branch --- server/sonar-web/src/main/js/api/issues.ts | 2 + .../main/js/apps/issues/sidebar/Sidebar.tsx | 16 ++++- .../main/js/apps/issues/sidebar/TagFacet.tsx | 4 +- .../issue/popups/SetIssueTagsPopup.tsx | 1 + .../sonar/server/issue/index/IssueIndex.java | 8 ++- .../sonar/server/issue/index/IssueQuery.java | 8 +-- .../issue/index/IssueIndexFiltersTest.java | 1 + .../org/sonar/server/issue/ws/TagsAction.java | 57 +++++++++++++---- .../sonar/server/issue/ws/TagsActionTest.java | 64 ++++++++++++++++++- 9 files changed, 137 insertions(+), 24 deletions(-) diff --git a/server/sonar-web/src/main/js/api/issues.ts b/server/sonar-web/src/main/js/api/issues.ts index f1c5ff3b74b..47f94bf9fd4 100644 --- a/server/sonar-web/src/main/js/api/issues.ts +++ b/server/sonar-web/src/main/js/api/issues.ts @@ -75,6 +75,8 @@ export function getFacet( export function searchIssueTags(data: { project?: string; + branch?: string; + all?: boolean; ps?: number; q?: string; }): Promise { diff --git a/server/sonar-web/src/main/js/apps/issues/sidebar/Sidebar.tsx b/server/sonar-web/src/main/js/apps/issues/sidebar/Sidebar.tsx index 51cfb8fea75..67666f9bc77 100644 --- a/server/sonar-web/src/main/js/apps/issues/sidebar/Sidebar.tsx +++ b/server/sonar-web/src/main/js/apps/issues/sidebar/Sidebar.tsx @@ -21,6 +21,7 @@ import * as React from 'react'; import { connect } from 'react-redux'; import { getGlobalSettingValue, Store } from '../../../store/rootReducer'; import { BranchLike } from '../../../types/branch-like'; +import { isBranch, isPullRequest } from '../../../helpers/branch-like'; import { ComponentQualifier, isApplication, isPortfolioLike } from '../../../types/component'; import { Facet, @@ -105,7 +106,19 @@ export class Sidebar extends React.PureComponent { } render() { - const { component, createdAfterIncludesTime, facets, openFacets, query } = this.props; + const { + component, + createdAfterIncludesTime, + facets, + openFacets, + query, + branchLike + } = this.props; + + const branch = + (isBranch(branchLike) && branchLike.name) || + (isPullRequest(branchLike) && branchLike.branch) || + undefined; const displayProjectsFacet = !component || !['TRK', 'BRC', 'DIR', 'DEV_PRJ'].includes(component.qualifier); @@ -216,6 +229,7 @@ export class Sidebar extends React.PureComponent { /> ) => Promise; onChange: (changes: Partial) => void; @@ -44,11 +45,12 @@ const SEARCH_SIZE = 100; export default class TagFacet extends React.PureComponent { handleSearch = (query: string) => { - const { component } = this.props; + const { component, branch } = this.props; const project = component && ['TRK', 'VW', 'APP'].includes(component.qualifier) ? component.key : undefined; return searchIssueTags({ project, + branch, ps: SEARCH_SIZE, q: query }).then(tags => ({ maxResults: tags.length === SEARCH_SIZE, results: tags })); diff --git a/server/sonar-web/src/main/js/components/issue/popups/SetIssueTagsPopup.tsx b/server/sonar-web/src/main/js/components/issue/popups/SetIssueTagsPopup.tsx index b163ab90472..79f79be5d49 100644 --- a/server/sonar-web/src/main/js/components/issue/popups/SetIssueTagsPopup.tsx +++ b/server/sonar-web/src/main/js/components/issue/popups/SetIssueTagsPopup.tsx @@ -50,6 +50,7 @@ export default class SetIssueTagsPopup extends React.PureComponent onSearch = (query: string) => { return searchIssueTags({ + all: true, q: query, ps: Math.min(this.props.selectedTags.length - 1 + LIST_SIZE, MAX_LIST_SIZE) }).then( diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java index ed80458dd0e..9b9b0e49fd7 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java @@ -516,9 +516,11 @@ public class IssueIndex { if (Boolean.TRUE.equals(query.onComponentOnly())) { return; } - allFilters.addFilter( - "__is_main_branch", new SimpleFieldFilterScope(FIELD_ISSUE_IS_MAIN_BRANCH), - createTermFilter(FIELD_ISSUE_IS_MAIN_BRANCH, Boolean.toString(query.isMainBranch()))); + if (query.isMainBranch() != null) { + allFilters.addFilter( + "__is_main_branch", new SimpleFieldFilterScope(FIELD_ISSUE_IS_MAIN_BRANCH), + createTermFilter(FIELD_ISSUE_IS_MAIN_BRANCH, query.isMainBranch().toString())); + } allFilters.addFilter( FIELD_ISSUE_BRANCH_UUID, new SimpleFieldFilterScope(FIELD_ISSUE_BRANCH_UUID), createTermFilter(FIELD_ISSUE_BRANCH_UUID, query.branchUuid())); diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java index 6c186591c91..16e0cafe927 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java @@ -96,7 +96,7 @@ public class IssueQuery { private final Boolean asc; private final String facetMode; private final String branchUuid; - private final boolean mainBranch; + private final Boolean mainBranch; private final ZoneId timeZone; private IssueQuery(Builder builder) { @@ -279,7 +279,7 @@ public class IssueQuery { return branchUuid; } - public boolean isMainBranch() { + public Boolean isMainBranch() { return mainBranch; } @@ -336,7 +336,7 @@ public class IssueQuery { private Boolean asc = false; private String facetMode; private String branchUuid; - private boolean mainBranch = true; + private Boolean mainBranch = true; private ZoneId timeZone; private Builder() { @@ -540,7 +540,7 @@ public class IssueQuery { return this; } - public Builder mainBranch(boolean mainBranch) { + public Builder mainBranch(@Nullable Boolean mainBranch) { this.mainBranch = mainBranch; return this; } diff --git a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java index a95f32ed55e..26b2f8f1faa 100644 --- a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java +++ b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueIndexFiltersTest.java @@ -276,6 +276,7 @@ public class IssueIndexFiltersTest { assertThatSearchReturnsOnly( IssueQuery.builder().componentUuids(singletonList(branch.uuid())).projectUuids(singletonList(project.uuid())).branchUuid(branch.uuid()).mainBranch(false), issueOnBranch.key()); + assertThatSearchReturnsOnly(IssueQuery.builder().projectUuids(singletonList(project.uuid())).mainBranch(null), issueOnProject.key(), issueOnBranch.key(), issueOnAnotherBranch.key()); assertThatSearchReturnsEmpty(IssueQuery.builder().branchUuid("unknown")); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/TagsAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/TagsAction.java index ffbe70ea0c9..364f796fe46 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/TagsAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/TagsAction.java @@ -19,10 +19,10 @@ */ package org.sonar.server.issue.ws; -import com.google.common.collect.ImmutableSet; import com.google.common.io.Resources; import java.util.List; import java.util.Optional; +import java.util.Set; import javax.annotation.Nullable; import org.sonar.api.resources.Qualifiers; import org.sonar.api.resources.Scopes; @@ -33,6 +33,7 @@ import org.sonar.api.server.ws.WebService; import org.sonar.api.server.ws.WebService.NewAction; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.component.BranchDto; import org.sonar.db.component.ComponentDto; import org.sonar.server.component.ComponentFinder; import org.sonar.server.issue.index.IssueIndex; @@ -41,19 +42,22 @@ import org.sonar.server.issue.index.IssueQuery; import org.sonarqube.ws.Issues; import static com.google.common.base.Preconditions.checkArgument; -import static java.util.Optional.ofNullable; import static org.sonar.api.server.ws.WebService.Param.PAGE_SIZE; import static org.sonar.api.server.ws.WebService.Param.TEXT_QUERY; import static org.sonar.server.issue.index.IssueQueryFactory.ISSUE_TYPE_NAMES; +import static org.sonar.server.ws.KeyExamples.KEY_BRANCH_EXAMPLE_001; import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001; import static org.sonar.server.ws.WsUtils.writeProtobuf; /** * List issue tags matching a given query. + * * @since 5.1 */ public class TagsAction implements IssuesWsAction { private static final String PARAM_PROJECT = "project"; + private static final String PARAM_BRANCH = "branch"; + private static final String PARAM_ALL = "all"; private final IssueIndex issueIndex; private final IssueIndexSyncProgressChecker issueIndexSyncProgressChecker; @@ -61,8 +65,8 @@ public class TagsAction implements IssuesWsAction { private final ComponentFinder componentFinder; public TagsAction(IssueIndex issueIndex, - IssueIndexSyncProgressChecker issueIndexSyncProgressChecker, DbClient dbClient, - ComponentFinder componentFinder) { + IssueIndexSyncProgressChecker issueIndexSyncProgressChecker, DbClient dbClient, + ComponentFinder componentFinder) { this.issueIndex = issueIndex; this.issueIndexSyncProgressChecker = issueIndexSyncProgressChecker; this.dbClient = dbClient; @@ -84,15 +88,31 @@ public class TagsAction implements IssuesWsAction { .setRequired(false) .setExampleValue(KEY_PROJECT_EXAMPLE_001) .setSince("7.4"); + action.createParam(PARAM_BRANCH) + .setDescription("Branch key") + .setRequired(false) + .setExampleValue(KEY_BRANCH_EXAMPLE_001) + .setSince("9.2"); + action.createParam(PARAM_ALL) + .setDescription("Indicator to search for all tags or only for tags in the main branch of a project") + .setRequired(false) + .setDefaultValue(Boolean.FALSE) + .setPossibleValues(Boolean.TRUE, Boolean.FALSE) + .setSince("9.2"); } @Override public void handle(Request request, Response response) throws Exception { try (DbSession dbSession = dbClient.openSession(false)) { String projectKey = request.param(PARAM_PROJECT); + String branchKey = request.param(PARAM_BRANCH); + boolean all = request.mandatoryParamAsBoolean(PARAM_ALL); checkIfAnyComponentsNeedIssueSync(dbSession, projectKey); + Optional project = getProject(dbSession, projectKey); - List tags = searchTags(project.orElse(null), request); + Optional branch = project.flatMap(p -> dbClient.branchDao().selectByBranchKey(dbSession, p.uuid(), branchKey)); + List tags = searchTags(project.orElse(null), branch.orElse(null), request, all); + Issues.TagsResponse.Builder tagsResponseBuilder = Issues.TagsResponse.newBuilder(); tags.forEach(tagsResponseBuilder::addTags); writeProtobuf(tagsResponseBuilder.build(), request, response); @@ -116,22 +136,31 @@ public class TagsAction implements IssuesWsAction { } } - private List searchTags(@Nullable ComponentDto project, Request request) { + private List searchTags(@Nullable ComponentDto project, @Nullable BranchDto branch, Request request, boolean all) { IssueQuery.Builder issueQueryBuilder = IssueQuery.builder() .types(ISSUE_TYPE_NAMES); - ofNullable(project).ifPresent(p -> { - switch (p.qualifier()) { + if (project != null) { + switch (project.qualifier()) { case Qualifiers.PROJECT: - issueQueryBuilder.projectUuids(ImmutableSet.of(p.uuid())); - return; + issueQueryBuilder.projectUuids(Set.of(project.uuid())); + break; case Qualifiers.APP: case Qualifiers.VIEW: - issueQueryBuilder.viewUuids(ImmutableSet.of(p.uuid())); - return; + issueQueryBuilder.viewUuids(Set.of(project.uuid())); + break; default: - throw new IllegalArgumentException(String.format("Component of type '%s' is not supported", p.qualifier())); + throw new IllegalArgumentException(String.format("Component of type '%s' is not supported", project.qualifier())); } - }); + + if (branch != null && !project.uuid().equals(branch.getUuid())) { + issueQueryBuilder.branchUuid(branch.getUuid()); + issueQueryBuilder.mainBranch(false); + } + } + if (all) { + issueQueryBuilder.mainBranch(null); + } + return issueIndex.searchTags( issueQueryBuilder.build(), request.param(TEXT_QUERY), diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/TagsActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/TagsActionTest.java index 1ce5af692ad..7d86aa5c8d8 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/TagsActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/TagsActionTest.java @@ -156,6 +156,66 @@ public class TagsActionTest { verify(issueIndexSyncProgressChecker).checkIfComponentNeedIssueSync(any(), eq(project1.getKey())); } + @Test + public void search_tags_by_branch_equals_main_branch() { + RuleDefinitionDto rule = db.rules().insertIssueRule(); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey("my_branch")); + db.issues().insertIssue(rule, project, project, issue -> issue.setTags(asList("tag1", "tag2"))); + db.issues().insertIssue(rule, branch, branch, issue -> issue.setTags(asList("tag12", "tag4", "tag5"))); + indexIssues(); + permissionIndexer.allowOnlyAnyone(project, branch); + + assertThat(tagListOf(ws.newRequest() + .setParam("project", project.getKey()) + .setParam("branch", project.uuid()))).containsExactly("tag1", "tag2"); + } + + @Test + public void search_tags_by_branch() { + RuleDefinitionDto rule = db.rules().insertIssueRule(); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey("my_branch")); + db.issues().insertIssue(rule, project, project, issue -> issue.setTags(asList("tag1", "tag2"))); + db.issues().insertIssue(rule, branch, branch, issue -> issue.setTags(asList("tag12", "tag4", "tag5"))); + indexIssues(); + permissionIndexer.allowOnlyAnyone(project, branch); + + assertThat(tagListOf(ws.newRequest() + .setParam("project", project.getKey()) + .setParam("branch", "my_branch"))).containsExactly("tag12", "tag4", "tag5"); + } + + @Test + public void search_tags_by_branch_not_exist_fall_back_to_main_branch() { + RuleDefinitionDto rule = db.rules().insertIssueRule(); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey("my_branch")); + db.issues().insertIssue(rule, project, project, issue -> issue.setTags(asList("tag1", "tag2"))); + db.issues().insertIssue(rule, branch, branch, issue -> issue.setTags(asList("tag12", "tag4", "tag5"))); + indexIssues(); + permissionIndexer.allowOnlyAnyone(project, branch); + + assertThat(tagListOf(ws.newRequest() + .setParam("project", project.getKey()) + .setParam("branch", "not_exist"))).containsExactly("tag1", "tag2"); + } + + @Test + public void search_all_tags_by_query() { + RuleDefinitionDto rule = db.rules().insertIssueRule(); + ComponentDto project = db.components().insertPrivateProject(); + ComponentDto branch = db.components().insertProjectBranch(project, b -> b.setKey("my_branch")); + db.issues().insertIssue(rule, project, project, issue -> issue.setTags(asList("tag1", "tag2"))); + db.issues().insertIssue(rule, branch, branch, issue -> issue.setTags(asList("tag12", "tag4", "tag5"))); + indexIssues(); + permissionIndexer.allowOnlyAnyone(project, branch); + + assertThat(tagListOf(ws.newRequest() + .setParam("q", "tag1") + .setParam("all", "true"))).containsExactly("tag1", "tag12"); + } + @Test public void search_tags_by_project_ignores_hotspots() { RuleDefinitionDto issueRule = db.rules().insertIssueRule(); @@ -319,7 +379,9 @@ public class TagsActionTest { .containsExactlyInAnyOrder( tuple("q", null, null, false, false), tuple("ps", "10", null, false, false), - tuple("project", null, "7.4", false, false)); + tuple("project", null, "7.4", false, false), + tuple("branch", null, "9.2", false, false), + tuple("all", "false", "9.2", false, false)); } private static ProtocolStringList tagListOf(TestRequest testRequest) { -- 2.39.5