From fb65076b67848376353f2cc995ba5e5eec9d8281 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 16 Mar 2017 13:59:24 +0100 Subject: [PATCH] SONAR-8919 Set default assignee within members of the project's organization --- .../issue/DefaultAssignee.java | 51 +++++++----- .../projectanalysis/issue/IssueAssigner.java | 2 +- .../issue/DefaultAssigneeTest.java | 77 +++++++++++++------ .../issue/IssueAssignerTest.java | 2 +- 4 files changed, 89 insertions(+), 43 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/DefaultAssignee.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/DefaultAssignee.java index 087997ef6c8..eeebe0c86e8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/DefaultAssignee.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/DefaultAssignee.java @@ -23,10 +23,12 @@ import com.google.common.base.Strings; import javax.annotation.CheckForNull; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.user.UserDto; +import org.sonar.server.computation.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.server.computation.task.projectanalysis.component.SettingsRepository; import org.sonar.server.computation.task.projectanalysis.component.TreeRootHolder; -import org.sonar.server.user.index.UserDoc; -import org.sonar.server.user.index.UserIndex; import static org.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE; @@ -38,37 +40,50 @@ public class DefaultAssignee { private static final Logger LOG = Loggers.get(DefaultAssignee.class); + private final DbClient dbClient; private final TreeRootHolder treeRootHolder; - private final UserIndex userIndex; private final SettingsRepository settingsRepository; + private final AnalysisMetadataHolder analysisMetadataHolder; private boolean loaded = false; private String login = null; - public DefaultAssignee(TreeRootHolder treeRootHolder, UserIndex userIndex, SettingsRepository settingsRepository) { + public DefaultAssignee(DbClient dbClient, TreeRootHolder treeRootHolder, SettingsRepository settingsRepository, AnalysisMetadataHolder analysisMetadataHolder) { + this.dbClient = dbClient; this.treeRootHolder = treeRootHolder; - this.userIndex = userIndex; this.settingsRepository = settingsRepository; + this.analysisMetadataHolder = analysisMetadataHolder; } @CheckForNull - public String getLogin() { - if (!loaded) { - String configuredLogin = settingsRepository.getSettings(treeRootHolder.getRoot()).getString(DEFAULT_ISSUE_ASSIGNEE); - if (!Strings.isNullOrEmpty(configuredLogin) && isValidLogin(configuredLogin)) { - this.login = configuredLogin; - } - loaded = true; + public String loadDefaultAssigneeLogin() { + if (loaded) { + return login; + } + String configuredLogin = settingsRepository.getSettings(treeRootHolder.getRoot()).getString(DEFAULT_ISSUE_ASSIGNEE); + if (!Strings.isNullOrEmpty(configuredLogin) && isValidLogin(configuredLogin)) { + this.login = configuredLogin; } + loaded = true; return login; } - private boolean isValidLogin(String s) { - UserDoc user = userIndex.getNullableByLogin(s); - if (user == null || !user.active()) { - LOG.info("Property {} is set with an unknown login: {}", DEFAULT_ISSUE_ASSIGNEE, s); - return false; + private boolean isValidLogin(String login) { + try (DbSession dbSession = dbClient.openSession(false)) { + UserDto user = dbClient.userDao().selectActiveUserByLogin(dbSession, login); + if (user == null) { + LOG.info("Property {} is set with an unknown login: {}", DEFAULT_ISSUE_ASSIGNEE, login); + return false; + } + if (!isUserMemberOfOrganization(dbSession, user)) { + LOG.info("Property {} is set with a user which is not member of the organization of the project : {}", DEFAULT_ISSUE_ASSIGNEE, login); + return false; + } + return true; } - return true; + } + + private boolean isUserMemberOfOrganization(DbSession dbSession, UserDto user) { + return dbClient.organizationMemberDao().select(dbSession, analysisMetadataHolder.getOrganization().getUuid(), user.getId()).isPresent(); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssigner.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssigner.java index 602ca52b894..e776ca2d766 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssigner.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssigner.java @@ -80,7 +80,7 @@ public class IssueAssigner extends IssueVisitor { } } if (authorWasSet && issue.assignee() == null) { - String assigneeLogin = StringUtils.defaultIfEmpty(scmAccountToUser.getNullable(issue.authorLogin()), defaultAssignee.getLogin()); + String assigneeLogin = StringUtils.defaultIfEmpty(scmAccountToUser.getNullable(issue.authorLogin()), defaultAssignee.loadDefaultAssigneeLogin()); issueUpdater.setNewAssignee(issue, assigneeLogin, changeContext); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/DefaultAssigneeTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/DefaultAssigneeTest.java index f122f0b46bb..e3830d1132c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/DefaultAssigneeTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/DefaultAssigneeTest.java @@ -20,64 +20,95 @@ package org.sonar.server.computation.task.projectanalysis.issue; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; -import org.mockito.Mockito; import org.sonar.api.CoreProperties; -import org.sonar.api.config.Settings; import org.sonar.api.config.MapSettings; -import org.sonar.server.computation.task.projectanalysis.component.Component; +import org.sonar.api.config.Settings; +import org.sonar.db.DbTester; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.user.UserDto; +import org.sonar.server.computation.task.projectanalysis.analysis.AnalysisMetadataHolderImpl; +import org.sonar.server.computation.task.projectanalysis.analysis.Organization; import org.sonar.server.computation.task.projectanalysis.component.SettingsRepository; +import org.sonar.server.computation.task.projectanalysis.component.TestSettingsRepository; import org.sonar.server.computation.task.projectanalysis.component.TreeRootHolderRule; -import org.sonar.server.user.index.UserDoc; -import org.sonar.server.user.index.UserIndex; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.sonar.server.computation.task.projectanalysis.component.ReportComponent.DUMB_PROJECT; public class DefaultAssigneeTest { public static final String PROJECT_KEY = "PROJECT_KEY"; + public static final String ORGANIZATION_UUID = "ORGANIZATION_UUID"; + + @Rule + public DbTester db = DbTester.create(); + + @Rule + public TreeRootHolderRule rootHolder = new TreeRootHolderRule().setRoot(DUMB_PROJECT); - TreeRootHolderRule rootHolder = mock(TreeRootHolderRule.class, Mockito.RETURNS_DEEP_STUBS); - UserIndex userIndex = mock(UserIndex.class); - Settings settings = new MapSettings(); - SettingsRepository settingsRepository = mock(SettingsRepository.class); + private Settings settings = new MapSettings(); + private SettingsRepository settingsRepository = new TestSettingsRepository(settings); + private AnalysisMetadataHolderImpl analysisMetadataHolder = new AnalysisMetadataHolderImpl(); + private OrganizationDto organizationDto; - DefaultAssignee underTest = new DefaultAssignee(rootHolder, userIndex, settingsRepository); + private DefaultAssignee underTest = new DefaultAssignee(db.getDbClient(), rootHolder, settingsRepository, analysisMetadataHolder); @Before - public void before() { - when(rootHolder.getRoot()).thenReturn(mock(Component.class)); - when(settingsRepository.getSettings(rootHolder.getRoot())).thenReturn(settings); + public void setUp() throws Exception { + organizationDto = db.organizations().insertForUuid(ORGANIZATION_UUID); + analysisMetadataHolder.setOrganization(Organization.from(new OrganizationDto().setUuid(ORGANIZATION_UUID).setKey("Organization key").setName("Organization name"))); } @Test public void no_default_assignee() { - assertThat(underTest.getLogin()).isNull(); + assertThat(underTest.loadDefaultAssigneeLogin()).isNull(); } @Test - public void default_assignee() { + public void set_default_assignee() { settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); - when(userIndex.getNullableByLogin("erik")).thenReturn(new UserDoc().setLogin("erik").setActive(true)); + UserDto userDto = db.users().insertUser("erik"); + db.organizations().addMember(organizationDto, userDto); - assertThat(underTest.getLogin()).isEqualTo("erik"); + assertThat(underTest.loadDefaultAssigneeLogin()).isEqualTo("erik"); } @Test public void configured_login_does_not_exist() { settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); - when(userIndex.getNullableByLogin("erik")).thenReturn(null); - assertThat(underTest.getLogin()).isNull(); + assertThat(underTest.loadDefaultAssigneeLogin()).isNull(); } @Test public void configured_login_is_disabled() { settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); - when(userIndex.getNullableByLogin("erik")).thenReturn(new UserDoc().setLogin("erik").setActive(false)); + db.users().insertUser(user -> user.setLogin("erik").setActive(false)); + + assertThat(underTest.loadDefaultAssigneeLogin()).isNull(); + } + + @Test + public void configured_login_is_not_member_of_organization() { + settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); + OrganizationDto otherOrganization = db.organizations().insert(); + UserDto userDto = db.users().insertUser("erik"); + db.organizations().addMember(otherOrganization, userDto); + + assertThat(underTest.loadDefaultAssigneeLogin()).isNull(); + } + + @Test + public void default_assignee_is_cached() { + settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); + UserDto userDto = db.users().insertUser("erik"); + db.organizations().addMember(organizationDto, userDto); + assertThat(underTest.loadDefaultAssigneeLogin()).isEqualTo("erik"); - assertThat(underTest.getLogin()).isNull(); + // The setting is updated but the assignee hasn't changed + settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "other"); + assertThat(underTest.loadDefaultAssigneeLogin()).isEqualTo("erik"); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssignerTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssignerTest.java index 0a47ed1430f..a82ece85b5a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssignerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/IssueAssignerTest.java @@ -100,7 +100,7 @@ public class IssueAssignerTest { public void set_default_assignee_if_author_not_found() throws Exception { addScmUser("john", null); setSingleChangeset("john", 123456789L, "rev-1"); - when(defaultAssignee.getLogin()).thenReturn("John C"); + when(defaultAssignee.loadDefaultAssigneeLogin()).thenReturn("John C"); DefaultIssue issue = new DefaultIssue().setLine(1); underTest.onIssue(FILE, issue); -- 2.39.5