From eb647d194313e80e70701a5081fd8f5d615b7a00 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Mon, 2 May 2016 16:06:13 +0200 Subject: [PATCH] SONAR-7517 Rename default group property when renaming group --- .../server/platform/PersistentSettings.java | 28 +++++-- .../sonar/server/platform/ServerSettings.java | 3 +- .../server/usergroups/ws/UpdateAction.java | 21 ++++- .../SetDefaultTemplateActionTest.java | 2 +- .../platform/PersistentSettingsTest.java | 84 +++++++++++-------- .../usergroups/ws/UpdateActionTest.java | 45 +++++++--- 6 files changed, 122 insertions(+), 61 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/PersistentSettings.java b/server/sonar-server/src/main/java/org/sonar/server/platform/PersistentSettings.java index bf344e1788c..852970b9739 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/PersistentSettings.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/PersistentSettings.java @@ -20,24 +20,27 @@ package org.sonar.server.platform; import com.google.common.collect.Maps; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; import org.picocontainer.Startable; import org.sonar.api.config.Settings; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; import org.sonar.db.property.PropertiesDao; import org.sonar.db.property.PropertyDto; -import javax.annotation.Nullable; -import java.util.List; -import java.util.Map; - /** * @since 3.2 */ public class PersistentSettings implements Startable { + private final DbClient dbClient; private final PropertiesDao propertiesDao; private final ServerSettings serverSettings; - public PersistentSettings(PropertiesDao propertiesDao, ServerSettings serverSettings) { - this.propertiesDao = propertiesDao; + public PersistentSettings(DbClient dbClient, ServerSettings serverSettings) { + this.dbClient = dbClient; + this.propertiesDao = dbClient.propertiesDao(); this.serverSettings = serverSettings; } @@ -56,8 +59,19 @@ public class PersistentSettings implements Startable { } public PersistentSettings saveProperty(String key, @Nullable String value) { + DbSession dbSession = dbClient.openSession(false); + try { + saveProperty(dbSession, key, value); + dbSession.commit(); + } finally { + dbClient.closeSession(dbSession); + } + return this; + } + + public PersistentSettings saveProperty(DbSession dbSession, String key, @Nullable String value) { serverSettings.setProperty(key, value); - propertiesDao.insertProperty(new PropertyDto().setKey(key).setValue(value)); + propertiesDao.insertProperty(dbSession, new PropertyDto().setKey(key).setValue(value)); return this; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/ServerSettings.java b/server/sonar-server/src/main/java/org/sonar/server/platform/ServerSettings.java index 36ea1473763..8b03af4de32 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/ServerSettings.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/ServerSettings.java @@ -20,6 +20,7 @@ package org.sonar.server.platform; import java.util.Map; +import javax.annotation.Nullable; import org.sonar.api.config.Settings; /** @@ -49,7 +50,7 @@ public interface ServerSettings { /** * @see Settings#setProperty(String, String) */ - Settings setProperty(String key, String value); + Settings setProperty(String key, @Nullable String value); /** * @see Settings#removeProperty(String) diff --git a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java index d427ec3cded..e0923bf8c20 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/usergroups/ws/UpdateAction.java @@ -19,7 +19,6 @@ */ package org.sonar.server.usergroups.ws; -import java.util.Arrays; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService.NewAction; @@ -30,8 +29,11 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.user.GroupDto; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.platform.PersistentSettings; import org.sonar.server.user.UserSession; +import static java.util.Collections.singletonList; +import static org.sonar.api.CoreProperties.CORE_DEFAULT_GROUP; import static org.sonar.api.user.UserGroupValidation.GROUP_NAME_MAX_LENGTH; import static org.sonar.db.MyBatis.closeQuietly; import static org.sonar.server.usergroups.ws.UserGroupUpdater.DESCRIPTION_MAX_LENGTH; @@ -43,12 +45,14 @@ public class UpdateAction implements UserGroupsWsAction { private final DbClient dbClient; private final UserSession userSession; + private final PersistentSettings persistentSettings; private final UserGroupUpdater groupUpdater; - public UpdateAction(DbClient dbClient, UserSession userSession, UserGroupUpdater groupUpdater) { + public UpdateAction(DbClient dbClient, UserSession userSession, UserGroupUpdater groupUpdater, PersistentSettings persistentSettings) { this.dbClient = dbClient; this.groupUpdater = groupUpdater; this.userSession = userSession; + this.persistentSettings = persistentSettings; } @Override @@ -85,14 +89,16 @@ public class UpdateAction implements UserGroupsWsAction { DbSession dbSession = dbClient.openSession(false); try { - groupUpdater.checkNameIsUnique(name, dbSession); GroupDto group = dbClient.groupDao().selectById(dbSession, groupId); if (group == null) { throw new NotFoundException(String.format("Could not find a user group with id '%s'.", groupId)); } + String oldName = group.getName(); if (name != null) { + groupUpdater.checkNameIsUnique(name, dbSession); groupUpdater.validateName(name); group.setName(name); + updateDefaultGroupIfNeeded(dbSession, oldName, name); } if (description != null) { groupUpdater.validateDescription(description); @@ -102,10 +108,17 @@ public class UpdateAction implements UserGroupsWsAction { dbSession.commit(); JsonWriter json = response.newJsonWriter().beginObject(); - groupUpdater.writeGroup(json, group, dbClient.groupMembershipDao().countUsersByGroups(dbSession, Arrays.asList(groupId)).get(group.getName())); + groupUpdater.writeGroup(json, group, dbClient.groupMembershipDao().countUsersByGroups(dbSession, singletonList(groupId)).get(group.getName())); json.endObject().close(); } finally { closeQuietly(dbSession); } } + + private void updateDefaultGroupIfNeeded(DbSession dbSession, String oldName, String newName) { + String defaultGroupName = persistentSettings.getString(CORE_DEFAULT_GROUP); + if (defaultGroupName.equals(oldName)) { + persistentSettings.saveProperty(dbSession, CORE_DEFAULT_GROUP, newName); + } + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/SetDefaultTemplateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/SetDefaultTemplateActionTest.java index b9440a08816..972c71dc186 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/SetDefaultTemplateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/SetDefaultTemplateActionTest.java @@ -82,7 +82,7 @@ public class SetDefaultTemplateActionTest { @Before public void setUp() { DbClient dbClient = db.getDbClient(); - persistentSettings = new PersistentSettings(dbClient.propertiesDao(), new WebServerSettings(new PropertyDefinitions(), new Properties())); + persistentSettings = new PersistentSettings(dbClient, new WebServerSettings(new PropertyDefinitions(), new Properties())); persistentSettings.saveProperty(DEFAULT_TEMPLATE_PROPERTY, "any-template-uuid"); persistentSettings.saveProperty(defaultRootQualifierTemplateProperty(PROJECT), "any-template-uuid"); persistentSettings.saveProperty(defaultRootQualifierTemplateProperty(VIEW), "any-view-template-uuid"); diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/PersistentSettingsTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/PersistentSettingsTest.java index 0dc3f8c1d4e..5a9bfaa1544 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/PersistentSettingsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/PersistentSettingsTest.java @@ -20,43 +20,37 @@ package org.sonar.server.platform; import com.google.common.collect.ImmutableMap; -import org.junit.Before; +import java.util.Properties; +import org.junit.Rule; import org.junit.Test; -import org.mockito.ArgumentMatcher; import org.sonar.api.config.PropertyDefinitions; +import org.sonar.api.utils.System2; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.DbTester; import org.sonar.db.property.PropertiesDao; import org.sonar.db.property.PropertyDto; -import java.util.Arrays; -import java.util.Properties; - import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Matchers.argThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; public class PersistentSettingsTest { - private PropertiesDao dao; - private ServerSettings settings; + @Rule + public DbTester db = DbTester.create(System2.INSTANCE); - @Before - public void init() { - dao = mock(PropertiesDao.class); + DbClient dbClient = db.getDbClient(); + DbSession dbSession = db.getSession(); - settings = new WebServerSettings( - new PropertyDefinitions(), - new Properties()); - } + private PropertiesDao dao = dbClient.propertiesDao(); + private ServerSettings settings = new WebServerSettings( + new PropertyDefinitions(), + new Properties()); @Test public void load_database_properties_at_startup() { - when(dao.selectGlobalProperties()).thenReturn(Arrays.asList( - new PropertyDto().setKey("in_db").setValue("bar") - )); + newGlobalProperty("in_db", "bar"); - PersistentSettings persistentSettings = new PersistentSettings(dao, settings); + PersistentSettings persistentSettings = new PersistentSettings(dbClient, settings); persistentSettings.start(); assertThat(settings.getString("in_db")).isEqualTo("bar"); @@ -64,42 +58,39 @@ public class PersistentSettingsTest { @Test public void saveProperty() { - PersistentSettings persistentSettings = new PersistentSettings(dao, settings); + PersistentSettings persistentSettings = new PersistentSettings(dbClient, settings); persistentSettings.saveProperty("foo", "bar"); // kept in memory cache and persisted in db assertThat(settings.getString("foo")).isEqualTo("bar"); - verify(dao).insertProperty(argThat(new ArgumentMatcher() { - @Override - public boolean matches(Object o) { - PropertyDto dto = (PropertyDto) o; - return dto.getKey().equals("foo"); - } - })); + verifyGlobalPropertyExists("foo", "bar"); } @Test public void deleteProperty() { + newGlobalProperty("foo", "bar_in_db"); settings.setProperty("foo", "bar"); assertThat(settings.hasKey("foo")).isTrue(); - PersistentSettings persistentSettings = new PersistentSettings(dao, settings); + PersistentSettings persistentSettings = new PersistentSettings(dbClient, settings); persistentSettings.deleteProperty("foo"); assertThat(settings.hasKey("foo")).isFalse(); - verify(dao).deleteGlobalProperty("foo"); + verifyGlobalPropertyDoesNotExist("foo"); } @Test public void deleteProperties() { + newGlobalProperty("in_db1", "foo"); + newGlobalProperty("in_db2", "bar"); settings.setProperty("foo", "bar"); assertThat(settings.hasKey("foo")).isTrue(); - PersistentSettings persistentSettings = new PersistentSettings(dao, settings); + PersistentSettings persistentSettings = new PersistentSettings(dbClient, settings); persistentSettings.deleteProperties(); assertThat(settings.getProperties()).isEmpty(); - verify(dao).deleteGlobalProperties(); + assertThat(dao.selectGlobalProperties()).isEmpty(); } @Test @@ -107,7 +98,7 @@ public class PersistentSettingsTest { settings.setProperty("foo", "bar"); assertThat(settings.hasKey("foo")).isTrue(); - PersistentSettings persistentSettings = new PersistentSettings(dao, settings); + PersistentSettings persistentSettings = new PersistentSettings(dbClient, settings); assertThat(persistentSettings.getProperties()).isEqualTo(settings.getProperties()); assertThat(persistentSettings.getString("foo")).isEqualTo("bar"); @@ -116,12 +107,31 @@ public class PersistentSettingsTest { @Test public void saveProperties() { - PersistentSettings persistentSettings = new PersistentSettings(dao, settings); + PersistentSettings persistentSettings = new PersistentSettings(dbClient, settings); ImmutableMap props = ImmutableMap.of("foo", "bar"); persistentSettings.saveProperties(props); assertThat(settings.getString("foo")).isEqualTo("bar"); - verify(dao).insertGlobalProperties(props); + verifyGlobalPropertyExists("foo", "bar"); + } + + private PropertyDto newGlobalProperty(String key, String value) { + PropertyDto propertyDto = new PropertyDto().setKey(key).setValue(value); + dao.insertProperty(dbSession, propertyDto); + dbSession.commit(); + return propertyDto; + } + + private void verifyGlobalPropertyExists(String key, String value){ + PropertyDto propertyDto = dao.selectGlobalProperty(dbSession, key); + assertThat(propertyDto).isNotNull(); + assertThat(propertyDto.getValue()).isEqualTo(value); + assertThat(propertyDto.getUserId()).isNull(); + assertThat(propertyDto.getResourceId()).isNull(); + } + + private void verifyGlobalPropertyDoesNotExist(String key){ + assertThat(dao.selectGlobalProperty(dbSession, key)).isNull(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java index f0aa9741138..a14cab2d373 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/usergroups/ws/UpdateActionTest.java @@ -37,32 +37,39 @@ import org.sonar.db.user.UserGroupDto; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.exceptions.ServerException; +import org.sonar.server.platform.PersistentSettings; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsTester; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class UpdateActionTest { + static final String DEFAULT_GROUP_NAME_KEY = "sonar.defaultGroup"; + static final String DEFAULT_GROUP_NAME_VALUE = "DEFAULT_GROUP_NAME_VALUE"; + @Rule public DbTester db = DbTester.create(System2.INSTANCE); @Rule public UserSessionRule userSession = UserSessionRule.standalone(); + @Rule public ExpectedException expectedException = ExpectedException.none(); + DbClient dbClient = db.getDbClient(); + DbSession dbSession = db.getSession(); + GroupDao groupDao = dbClient.groupDao(); + UserGroupDao userGroupDao = dbClient.userGroupDao(); - private WsTester ws; - private DbSession dbSession; - private GroupDao groupDao; - private UserGroupDao userGroupDao; + PersistentSettings settings = mock(PersistentSettings.class); + WsTester ws = new WsTester(new UserGroupsWs(new UpdateAction(dbClient, userSession, new UserGroupUpdater(dbClient), settings))); @Before - public void setUp() { - DbClient dbClient = db.getDbClient(); - dbSession = db.getSession(); - groupDao = dbClient.groupDao(); - userGroupDao = dbClient.userGroupDao(); - - ws = new WsTester(new UserGroupsWs(new UpdateAction(dbClient, userSession, new UserGroupUpdater(dbClient)))); + public void setUp() throws Exception { + when(settings.getString(DEFAULT_GROUP_NAME_KEY)).thenReturn(DEFAULT_GROUP_NAME_VALUE); } @Test @@ -122,6 +129,20 @@ public class UpdateActionTest { "}"); } + @Test + public void update_default_group_name_also_update_default_group_property() throws Exception { + GroupDto existingGroup = groupDao.insert(dbSession, new GroupDto().setName(DEFAULT_GROUP_NAME_VALUE).setDescription("Default group name")); + dbSession.commit(); + + loginAsAdmin(); + newRequest() + .setParam("id", existingGroup.getId().toString()) + .setParam("name", "new-name") + .execute(); + + verify(settings).saveProperty(any(DbSession.class), eq(DEFAULT_GROUP_NAME_KEY), eq("new-name")); + } + @Test public void require_admin_permission() throws Exception { expectedException.expect(ForbiddenException.class); @@ -226,4 +247,6 @@ public class UpdateActionTest { private void loginAsAdmin() { userSession.login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); } + + } -- 2.39.5