From d701a6409d151d45b687d9b6af5c12ef255b2f13 Mon Sep 17 00:00:00 2001 From: Michal Duda Date: Tue, 30 Mar 2021 19:22:52 +0200 Subject: [PATCH] SONAR-14322 api/issues/search returns all issues if provided rule key doesn't exist --- .../sonar/server/issue/index/IssueIndex.java | 4 ++-- .../sonar/server/issue/index/IssueQuery.java | 12 +++++++++++ .../server/issue/index/IssueQueryFactory.java | 12 +++++++++-- .../issue/index/IssueIndexFiltersTest.java | 4 ++-- .../issue/index/IssueQueryFactoryTest.java | 17 +++++++++++++++ .../sonar/server/issue/ws/SearchAction.java | 2 +- .../server/issue/ws/SearchActionTest.java | 21 +++++++++++++++++++ 7 files changed, 65 insertions(+), 7 deletions(-) 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 a948489caab..9f40942a5d6 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 @@ -437,7 +437,7 @@ public class IssueIndex { filters.addFilter( FIELD_ISSUE_RULE_UUID, RULES.getFilterScope(), createTermsFilter( FIELD_ISSUE_RULE_UUID, - query.rules().stream().map(RuleDefinitionDto::getUuid).collect(toList()))); + query.ruleUuids())); filters.addFilter(FIELD_ISSUE_STATUS, STATUSES.getFilterScope(), createTermsFilter(FIELD_ISSUE_STATUS, query.statuses())); // security category @@ -669,7 +669,7 @@ public class IssueIndex { addFacetIfNeeded(options, aggregationHelper, esRequest, FILES, query.files().toArray()); addFacetIfNeeded(options, aggregationHelper, esRequest, SCOPES, query.scopes().toArray()); addFacetIfNeeded(options, aggregationHelper, esRequest, LANGUAGES, query.languages().toArray()); - addFacetIfNeeded(options, aggregationHelper, esRequest, RULES, query.rules().stream().map(RuleDefinitionDto::getUuid).toArray()); + addFacetIfNeeded(options, aggregationHelper, esRequest, RULES, query.ruleUuids().toArray()); addFacetIfNeeded(options, aggregationHelper, esRequest, AUTHORS, query.authors().toArray()); addFacetIfNeeded(options, aggregationHelper, esRequest, AUTHOR, query.authors().toArray()); addFacetIfNeeded(options, aggregationHelper, esRequest, TAGS, query.tags().toArray()); 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 fd017efd005..6c186591c91 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 @@ -74,6 +74,7 @@ public class IssueQuery { private final Collection files; private final Collection views; private final Collection rules; + private final Collection ruleUuids; private final Collection assignees; private final Collection authors; private final Collection scopes; @@ -111,6 +112,7 @@ public class IssueQuery { this.files = defaultCollection(builder.files); this.views = defaultCollection(builder.views); this.rules = defaultCollection(builder.rules); + this.ruleUuids = defaultCollection(builder.ruleUuids); this.assignees = defaultCollection(builder.assigneeUuids); this.authors = defaultCollection(builder.authors); this.scopes = defaultCollection(builder.scopes); @@ -184,6 +186,10 @@ public class IssueQuery { return rules; } + public Collection ruleUuids() { + return ruleUuids; + } + public Collection assignees() { return assignees; } @@ -308,6 +314,7 @@ public class IssueQuery { private Collection files; private Collection views; private Collection rules; + private Collection ruleUuids; private Collection assigneeUuids; private Collection authors; private Collection scopes; @@ -396,6 +403,11 @@ public class IssueQuery { return this; } + public Builder ruleUuids(@Nullable Collection ruleUuids) { + this.ruleUuids = ruleUuids; + return this; + } + public Builder assigneeUuids(@Nullable Collection l) { this.assigneeUuids = l; return this; diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQueryFactory.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQueryFactory.java index bf61e71b655..c238c58a6b5 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQueryFactory.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQueryFactory.java @@ -109,13 +109,22 @@ public class IssueQueryFactory { public IssueQuery create(SearchRequest request) { try (DbSession dbSession = dbClient.openSession(false)) { final ZoneId timeZone = parseTimeZone(request.getTimeZone()).orElse(clock.getZone()); + + Collection ruleDefinitionDtos = ruleKeysToRuleId(dbSession, request.getRules()); + Collection ruleUuids = ruleDefinitionDtos.stream().map(RuleDefinitionDto::getUuid).collect(Collectors.toSet()); + + if (request.getRules() != null && request.getRules().stream().collect(toSet()).size() != ruleDefinitionDtos.size()) { + ruleUuids.add("non-existing-uuid"); + } + IssueQuery.Builder builder = IssueQuery.builder() .issueKeys(request.getIssues()) .severities(request.getSeverities()) .statuses(request.getStatuses()) .resolutions(request.getResolutions()) .resolved(request.getResolved()) - .rules(ruleKeysToRuleId(dbSession, request.getRules())) + .rules(ruleDefinitionDtos) + .ruleUuids(ruleUuids) .assigneeUuids(request.getAssigneeUuids()) .authors(request.getAuthors()) .scopes(request.getScopes()) @@ -367,7 +376,6 @@ public class IssueQueryFactory { return componentDtos; } - @CheckForNull private Collection ruleKeysToRuleId(DbSession dbSession, @Nullable Collection rules) { if (rules != null) { return dbClient.ruleDao().selectDefinitionByKeys(dbSession, transform(rules, RuleKey::parse)); 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 e8e4f5fb17a..a95f32ed55e 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 @@ -569,8 +569,8 @@ public class IssueIndexFiltersTest { indexIssues(newDoc("I1", file).setRuleUuid(ruleDefinitionDto.getUuid())); - assertThatSearchReturnsOnly(IssueQuery.builder().rules(singletonList(ruleDefinitionDto)), "I1"); - assertThatSearchReturnsEmpty(IssueQuery.builder().rules(singletonList(new RuleDefinitionDto().setUuid("uuid-abc")))); + assertThatSearchReturnsOnly(IssueQuery.builder().ruleUuids(singletonList(ruleDefinitionDto.getUuid())), "I1"); + assertThatSearchReturnsEmpty(IssueQuery.builder().ruleUuids(singletonList("uuid-abc"))); } @Test diff --git a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryFactoryTest.java b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryFactoryTest.java index 89d77aed63c..d2057956dd8 100644 --- a/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryFactoryTest.java +++ b/server/sonar-webserver-es/src/test/java/org/sonar/server/issue/index/IssueQueryFactoryTest.java @@ -119,6 +119,7 @@ public class IssueQueryFactoryTest { assertThat(query.onComponentOnly()).isFalse(); assertThat(query.assigned()).isTrue(); assertThat(query.rules()).hasSize(2); + assertThat(query.ruleUuids()).hasSize(2); assertThat(query.directories()).containsOnly("aDirPath"); assertThat(query.createdAfter().date()).isEqualTo(parseDateTime("2013-04-16T09:08:24+0200")); assertThat(query.createdAfter().inclusive()).isTrue(); @@ -127,6 +128,22 @@ public class IssueQueryFactoryTest { assertThat(query.asc()).isTrue(); } + @Test + public void create_with_rule_key_that_does_not_exist_in_the_db() { + db.users().insertUser(u -> u.setLogin("joanna")); + ComponentDto project = db.components().insertPrivateProject(); + db.components().insertComponent(newModuleDto(project)); + db.components().insertComponent(newFileDto(project)); + newRule(RuleKey.of("findbugs", "NullReference")); + SearchRequest request = new SearchRequest() + .setRules(asList("unknown:key1", "unknown:key2")); + + IssueQuery query = underTest.create(request); + + assertThat(query.rules()).isEmpty(); + assertThat(query.ruleUuids()).containsExactly("non-existing-uuid"); + } + @Test public void leak_period_start_date_is_exclusive() { long leakPeriodStart = addDays(new Date(), -14).getTime(); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchAction.java index 6bb64d140a7..303e14077f9 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SearchAction.java @@ -461,7 +461,7 @@ public class SearchAction implements IssuesWsAction { } addMandatoryValuesToFacet(facets, PARAM_ASSIGNEES, assignees); addMandatoryValuesToFacet(facets, FACET_ASSIGNED_TO_ME, singletonList(userSession.getUuid())); - addMandatoryValuesToFacet(facets, PARAM_RULES, query.rules().stream().map(RuleDefinitionDto::getUuid).collect(toList())); + addMandatoryValuesToFacet(facets, PARAM_RULES, query.ruleUuids()); addMandatoryValuesToFacet(facets, PARAM_SCOPES, ISSUE_SCOPES); addMandatoryValuesToFacet(facets, PARAM_LANGUAGES, request.getLanguages()); addMandatoryValuesToFacet(facets, PARAM_TAGS, request.getTags()); diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java index 78d80d7fd67..d4f10af4b72 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java @@ -397,6 +397,27 @@ public class SearchActionTest { execute.assertJson(this.getClass(), "result_for_rule_search.json"); } + @Test + public void search_by_non_existing_rule_key() { + RuleDto rule = newIssueRule(); + ComponentDto project = db.components().insertComponent(ComponentTesting.newPublicProjectDto("PROJECT_ID").setDbKey("PROJECT_KEY").setLanguage("java")); + ComponentDto file = db.components().insertComponent(newFileDto(project, null, "FILE_ID").setDbKey("FILE_KEY").setLanguage("java")); + + db.issues().insertIssue(rule.getDefinition(), project, file); + session.commit(); + indexIssues(); + + userSession.logIn("john") + .addProjectPermission(ISSUE_ADMIN, project); // granted by Anyone + indexPermissions(); + + TestResponse execute = ws.newRequest() + .setParam(PARAM_RULES, "nonexisting:rulekey") + .setParam("additionalFields", "_all") + .execute(); + execute.assertJson(this.getClass(), "no_issue.json"); + } + @Test public void issue_on_removed_file() { RuleDto rule = newIssueRule(); -- 2.39.5