From 9b06b17a237c6086bb9e01da978b9b1637673050 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Mon, 5 Sep 2016 14:37:47 +0200 Subject: [PATCH] SONAR-7970 WS settings/set fix db session management with SettingsChangeNotifier --- .../sonar/server/setting/ws/SetAction.java | 5 ++- .../server/setting/ws/SetActionTest.java | 37 +++++++++++++------ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java index 5f32087103b..f2e5f9a28d9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/setting/ws/SetAction.java @@ -140,7 +140,6 @@ public class SetAction implements SettingsWsAction { DbSession dbSession = dbClient.openSession(false); try { doHandle(dbSession, toWsRequest(request)); - dbSession.commit(); } finally { dbClient.closeSession(dbSession); } @@ -167,8 +166,10 @@ public class SetAction implements SettingsWsAction { dbClient.propertiesDao().insertProperty(dbSession, property); } + dbSession.commit(); + if (!component.isPresent()) { - settingsChangeNotifier.onGlobalPropertyChange(request.getKey(), value); + settingsChangeNotifier.onGlobalPropertyChange(persistedKey(request), value); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java index 3904252ece2..d9d43e6dacf 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java @@ -61,9 +61,6 @@ import static com.google.common.collect.Lists.newArrayList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.groups.Tuple.tuple; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; import static org.sonar.db.component.ComponentTesting.newView; import static org.sonar.db.property.PropertyTesting.newComponentPropertyDto; import static org.sonar.db.property.PropertyTesting.newGlobalPropertyDto; @@ -87,7 +84,7 @@ public class SetActionTest { I18nRule i18n = new I18nRule(); PropertyDefinitions propertyDefinitions = new PropertyDefinitions(); - SettingsChangeNotifier settingsChangeNotifier = mock(SettingsChangeNotifier.class); + FakeSettingsNotifier settingsChangeNotifier = new FakeSettingsNotifier(dbClient); SettingsUpdater settingsUpdater = new SettingsUpdater(dbClient, propertyDefinitions); SetAction underTest = new SetAction(propertyDefinitions, i18n, dbClient, componentFinder, userSession, settingsUpdater, settingsChangeNotifier); @@ -110,7 +107,7 @@ public class SetActionTest { callForGlobalSetting("my.key", "my,value"); assertGlobalSetting("my.key", "my,value"); - verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", "my,value"); + assertThat(settingsChangeNotifier.wasCalled).isTrue(); } @Test @@ -121,7 +118,7 @@ public class SetActionTest { callForGlobalSetting("my.key", "my new value"); assertGlobalSetting("my.key", "my new value"); - verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", "my new value"); + assertThat(settingsChangeNotifier.wasCalled).isTrue(); } @Test @@ -133,7 +130,7 @@ public class SetActionTest { assertGlobalSetting("my.key", "my global value"); assertComponentSetting("my.key", "my project value", project.getId()); - verifyZeroInteractions(settingsChangeNotifier); + assertThat(settingsChangeNotifier.wasCalled).isFalse(); } @Test @@ -164,7 +161,7 @@ public class SetActionTest { String expectedValue = "first%2CValue,second%2CValue,third%2CValue"; assertGlobalSetting("my.key", expectedValue); - verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", expectedValue); + assertThat(settingsChangeNotifier.wasCalled).isTrue(); } @Test @@ -208,7 +205,7 @@ public class SetActionTest { assertGlobalSetting("my.key.2.secondField", "anotherSecondValue"); assertGlobalSetting("my.key.3.firstField", "yetFirstValue"); assertGlobalSetting("my.key.3.secondField", "yetSecondValue"); - verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", "1,2,3"); + assertThat(settingsChangeNotifier.wasCalled).isTrue(); } @Test @@ -255,7 +252,7 @@ public class SetActionTest { assertGlobalSetting("my.key.2.secondField", "anotherSecondValue"); assertGlobalSetting("my.key.3.firstField", "yetFirstValue"); assertGlobalSetting("my.key.3.secondField", "yetSecondValue"); - verify(settingsChangeNotifier).onGlobalPropertyChange("my.key", "1,2,3"); + assertThat(settingsChangeNotifier.wasCalled).isTrue(); } @Test @@ -304,7 +301,7 @@ public class SetActionTest { assertComponentSetting("my.key.1.secondField", "secondValue", projectId); assertComponentSetting("my.key.2.firstField", "anotherFirstValue", projectId); assertComponentSetting("my.key.2.secondField", "anotherSecondValue", projectId); - verifyZeroInteractions(settingsChangeNotifier); + assertThat(settingsChangeNotifier.wasCalled).isFalse(); } @Test @@ -727,4 +724,22 @@ public class SetActionTest { request.execute(); } + + private static class FakeSettingsNotifier extends SettingsChangeNotifier { + private final DbClient dbClient; + + private boolean wasCalled = false; + + private FakeSettingsNotifier(DbClient dbClient) { + this.dbClient = dbClient; + } + + @Override + public void onGlobalPropertyChange(String key, @Nullable String value) { + wasCalled = true; + PropertyDto property = dbClient.propertiesDao().selectGlobalProperty(key); + + assertThat(property.getValue()).isEqualTo(value); + } + } } -- 2.39.5