From 244b5d6ef2a53cfbcc0025c97670a28016e83f44 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 23 Jan 2015 07:20:54 +0100 Subject: [PATCH] SONAR-5906 log warning when multiple users share the same SCM account, so that issue can't be assigned to a single person. --- .../issue/ScmAccountCacheLoader.java | 31 +++++++++++++++++-- .../sonar/server/user/index/UserIndex.java | 25 +++++++++++++++ .../issue/ScmAccountCacheLoaderTest.java | 17 ++++++++-- .../server/user/index/UserIndexTest.java | 13 ++++++++ .../{user1.json => charlie.json} | 0 .../charlie_conflict.json | 8 +++++ 6 files changed, 89 insertions(+), 5 deletions(-) rename server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/{user1.json => charlie.json} (100%) create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/charlie_conflict.json diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/ScmAccountCacheLoader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/ScmAccountCacheLoader.java index 6a74a52e670..f4327afa293 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/ScmAccountCacheLoader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/ScmAccountCacheLoader.java @@ -19,28 +19,53 @@ */ package org.sonar.server.computation.issue; +import com.google.common.base.Function; +import com.google.common.base.Joiner; +import com.google.common.collect.Collections2; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.server.user.index.UserDoc; import org.sonar.server.user.index.UserIndex; import org.sonar.server.util.cache.CacheLoader; import java.util.Collection; +import java.util.List; import java.util.Map; /** * Loads the association between a SCM account and a SQ user */ -public class ScmAccountCacheLoader implements CacheLoader { +public class ScmAccountCacheLoader implements CacheLoader { + private final Logger log; private final UserIndex index; public ScmAccountCacheLoader(UserIndex index) { + this(index, LoggerFactory.getLogger(ScmAccountCacheLoader.class)); + } + + public ScmAccountCacheLoader(UserIndex index, Logger log) { + this.log = log; this.index = index; } @Override public String load(String scmAccount) { - UserDoc user = index.getNullableByScmAccount(scmAccount); - return user != null ? user.login() : null; + List users = index.getAtMostThreeUsersForScmAccount(scmAccount); + if (users.isEmpty()) { + return null; + } + if (users.size() == 1) { + return users.get(0).login(); + } + Collection logins = Collections2.transform(users, new Function() { + @Override + public String apply(UserDoc input) { + return input.login(); + } + }); + log.warn(String.format("Multiple users share the SCM account '%s': %s", scmAccount, Joiner.on(", ").join(logins))); + return null; } @Override 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 4aa7abbfa62..2044f29ce8d 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 @@ -33,6 +33,9 @@ import org.sonar.server.exceptions.NotFoundException; import javax.annotation.CheckForNull; +import java.util.ArrayList; +import java.util.List; + public class UserIndex implements ServerComponent { private final EsClient esClient; @@ -80,4 +83,26 @@ public class UserIndex implements ServerComponent { return userDoc; } + /** + * Returns the users (at most 3) who are associated to the given SCM account. This method can be used + * to detect user conflicts. It never returns null. + */ + public List getAtMostThreeUsersForScmAccount(String scmAccount) { + List result = new ArrayList<>(); + 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(3); + for (SearchHit hit : request.get().getHits().getHits()) { + result.add(new UserDoc(hit.sourceAsMap())); + } + } + return result; + } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest.java index 924fa5bc46f..e1f448d8c28 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest.java @@ -21,6 +21,7 @@ package org.sonar.server.computation.issue; import org.junit.Rule; import org.junit.Test; +import org.slf4j.Logger; import org.sonar.api.config.Settings; import org.sonar.server.es.EsTester; import org.sonar.server.user.index.UserIndex; @@ -30,6 +31,8 @@ import java.util.Collections; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; public class ScmAccountCacheLoaderTest { @@ -38,8 +41,7 @@ public class ScmAccountCacheLoaderTest { @Test public void load_login_for_scm_account() throws Exception { - esTester.putDocuments("users", "user", getClass(), "user1.json"); - + esTester.putDocuments("users", "user", getClass(), "charlie.json"); UserIndex index = new UserIndex(esTester.client()); ScmAccountCacheLoader loader = new ScmAccountCacheLoader(index); @@ -47,6 +49,17 @@ public class ScmAccountCacheLoaderTest { assertThat(loader.load("jesuis@charlie.com")).isEqualTo("charlie"); } + @Test + public void warn_if_multiple_users_share_same_scm_account() throws Exception { + esTester.putDocuments("users", "user", getClass(), "charlie.json", "charlie_conflict.json"); + UserIndex index = new UserIndex(esTester.client()); + Logger log = mock(Logger.class); + ScmAccountCacheLoader loader = new ScmAccountCacheLoader(index, log); + + assertThat(loader.load("charlie")).isNull(); + verify(log).warn("Multiple users share the SCM account 'charlie': charlie, another.charlie"); + } + @Test public void load_by_multiple_scm_accounts_is_not_supported_yet() throws Exception { UserIndex index = new UserIndex(esTester.client()); 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 bbecfbc4f50..52c46612f16 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 @@ -112,4 +112,17 @@ public class UserIndexTest { assertThat(index.getNullableByScmAccount("user1@mail.com")).isNull(); } + @Test + public void get_users_for_scm_account() throws Exception { + esTester.putDocuments(UserIndexDefinition.INDEX, UserIndexDefinition.TYPE_USER, this.getClass(), "user1.json", "user3-with-same-email-as-user1.json"); + + assertThat(index.getAtMostThreeUsersForScmAccount("user_1")).extractingResultOf("login").containsOnly("user1"); + assertThat(index.getAtMostThreeUsersForScmAccount("user1")).extractingResultOf("login").containsOnly("user1"); + + // both users share the same email + assertThat(index.getAtMostThreeUsersForScmAccount("user1@mail.com")).extractingResultOf("login").containsOnly("user1", "user3"); + + assertThat(index.getAtMostThreeUsersForScmAccount("")).isEmpty(); + assertThat(index.getAtMostThreeUsersForScmAccount("unknown")).isEmpty(); + } } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/user1.json b/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/charlie.json similarity index 100% rename from server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/user1.json rename to server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/charlie.json diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/charlie_conflict.json b/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/charlie_conflict.json new file mode 100644 index 00000000000..8f5af49f8ae --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/issue/ScmAccountCacheLoaderTest/charlie_conflict.json @@ -0,0 +1,8 @@ +{ + "login": "another.charlie", + "name": "Another Charlie", + "active": true, + "scmAccounts": ["charlie"], + "createdAt": 1500000000000, + "updatedAt": 1500000000000 +} -- 2.39.5