From 32fce03ccd2032023fa2ca88699666b002ef1b33 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 21 Jan 2015 15:56:52 +0100 Subject: [PATCH] SONAR-5960 ES request fails if author is empty ("") - do not allow blank author on issues - fix ES search --- .../computation/issue/SourceLinesCache.java | 4 ++- .../sonar/server/user/index/UserIndex.java | 25 +++++++++++-------- .../issue/SourceLinesCacheTest.java | 10 +++++++- .../server/user/index/UserIndexTest.java | 2 ++ .../issue/SourceLinesCacheTest/load_data.xml | 2 +- 5 files changed, 29 insertions(+), 14 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/SourceLinesCache.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/SourceLinesCache.java index 09bcbcb516e..155cf700151 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/SourceLinesCache.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/SourceLinesCache.java @@ -20,6 +20,7 @@ package org.sonar.server.computation.issue; import com.google.common.base.Function; +import com.google.common.base.Strings; import org.apache.commons.csv.CSVFormat; import org.apache.commons.csv.CSVParser; import org.apache.commons.csv.CSVRecord; @@ -69,7 +70,8 @@ public class SourceLinesCache { public String lineAuthor(int lineId) { loadIfNeeded(); if (lineId <= authors.size()) { - return authors.get(lineId - 1); + String author = authors.get(lineId - 1); + return Strings.emptyToNull(author); } return null; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java b/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java index 0354da4952c..4aa7abbfa62 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/index/UserIndex.java @@ -20,6 +20,7 @@ package org.sonar.server.user.index; +import org.apache.commons.lang.StringUtils; import org.elasticsearch.action.get.GetRequestBuilder; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.search.SearchRequestBuilder; @@ -54,17 +55,19 @@ public class UserIndex implements ServerComponent { @CheckForNull public UserDoc getNullableByScmAccount(String scmAccount) { - SearchRequestBuilder request = esClient.prepareSearch(UserIndexDefinition.INDEX) - .setTypes(UserIndexDefinition.TYPE_USER) - .setQuery(QueryBuilders.filteredQuery(QueryBuilders.matchAllQuery(), - FilterBuilders.boolFilter() - .should(FilterBuilders.termFilter(UserIndexDefinition.FIELD_LOGIN, scmAccount)) - .should(FilterBuilders.termFilter(UserIndexDefinition.FIELD_EMAIL, scmAccount)) - .should(FilterBuilders.termFilter(UserIndexDefinition.FIELD_SCM_ACCOUNTS, scmAccount)))) - .setSize(2); - SearchHit[] result = request.get().getHits().getHits(); - if (result.length == 1) { - return new UserDoc(result[0].sourceAsMap()); + if (!StringUtils.isEmpty(scmAccount)) { + SearchRequestBuilder request = esClient.prepareSearch(UserIndexDefinition.INDEX) + .setTypes(UserIndexDefinition.TYPE_USER) + .setQuery(QueryBuilders.filteredQuery(QueryBuilders.matchAllQuery(), + FilterBuilders.boolFilter() + .should(FilterBuilders.termFilter(UserIndexDefinition.FIELD_LOGIN, scmAccount)) + .should(FilterBuilders.termFilter(UserIndexDefinition.FIELD_EMAIL, scmAccount)) + .should(FilterBuilders.termFilter(UserIndexDefinition.FIELD_SCM_ACCOUNTS, scmAccount)))) + .setSize(2); + SearchHit[] result = request.get().getHits().getHits(); + if (result.length == 1) { + return new UserDoc(result[0].sourceAsMap()); + } } return null; } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/SourceLinesCacheTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/SourceLinesCacheTest.java index 718c4520b38..bbb3b5b1dc8 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/SourceLinesCacheTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/SourceLinesCacheTest.java @@ -47,10 +47,18 @@ public class SourceLinesCacheTest { assertThat(cache.lineAuthor(1)).isEqualTo("charlie"); assertThat(cache.lineAuthor(2)).isEqualTo("cabu"); + + // blank author -> return null assertThat(cache.lineAuthor(3)).isNull(); - assertThat(cache.countLines()).isEqualTo(2); + + // only 3 lines in the file + assertThat(cache.lineAuthor(100)).isNull(); + + assertThat(cache.countLines()).isEqualTo(3); cache.clear(); assertThat(cache.countLines()).isEqualTo(0); } + + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java index 61be62bdbfd..bbecfbc4f50 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/user/index/UserIndexTest.java @@ -56,6 +56,7 @@ public class UserIndexTest { assertThat(userDoc.createdAt()).isEqualTo(1500000000000L); assertThat(userDoc.updatedAt()).isEqualTo(1500000000000L); + assertThat(index.getNullableByLogin("")).isNull(); assertThat(index.getNullableByLogin("unknown")).isNull(); } @@ -100,6 +101,7 @@ public class UserIndexTest { assertThat(index.getNullableByScmAccount("user1@mail.com").login()).isEqualTo("user1"); assertThat(index.getNullableByScmAccount("user1").login()).isEqualTo("user1"); + assertThat(index.getNullableByScmAccount("")).isNull(); assertThat(index.getNullableByScmAccount("unknown")).isNull(); } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/SourceLinesCacheTest/load_data.xml b/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/SourceLinesCacheTest/load_data.xml index 4079fb9d58d..907296082d8 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/SourceLinesCacheTest/load_data.xml +++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/SourceLinesCacheTest/load_data.xml @@ -1,6 +1,6 @@