From 9ad210d94393b59ccf5eea015efe4efaa07f9afa Mon Sep 17 00:00:00 2001
From: =?utf8?q?S=C3=A9bastien=20Lesaint?=
 <sebastien.lesaint@sonarsource.com>
Date: Thu, 8 Sep 2016 11:26:17 +0200
Subject: [PATCH] SONAR-7676 move property change listener notification afer
 commit

in PersistentSettings and when possible (ie. not when DbSession is provided by PersistentSettings' caller)
---
 .../server/platform/PersistentSettings.java    | 18 +++++++++++++-----
 .../platform/PersistentSettingsTest.java       |  4 +---
 2 files changed, 14 insertions(+), 8 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 006ca5b8791..7dbbcbc3d2a 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
@@ -49,10 +49,7 @@ public class PersistentSettings {
    * are executed.
    */
   public PersistentSettings saveProperty(DbSession dbSession, String key, @Nullable String value) {
-    dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setKey(key).setValue(value));
-    // refresh the cache of settings
-    delegate.setProperty(key, value);
-
+    savePropertyImpl(dbSession, key, value);
     changeNotifier.onGlobalPropertyChange(key, value);
     return this;
   }
@@ -63,12 +60,23 @@ public class PersistentSettings {
    */
   public PersistentSettings saveProperty(String key, @Nullable String value) {
     try (DbSession dbSession = dbClient.openSession(false)) {
-      saveProperty(dbSession, key, value);
+      savePropertyImpl(dbSession, key, value);
       dbSession.commit();
+      changeNotifier.onGlobalPropertyChange(key, value);
       return this;
     }
   }
 
+  private void savePropertyImpl(DbSession dbSession, String key, @Nullable String value) {
+    if (value == null) {
+      dbClient.propertiesDao().deleteGlobalProperty(key, dbSession);
+    } else {
+      dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setKey(key).setValue(value));
+    }
+    // refresh the cache of settings
+    delegate.setProperty(key, value);
+  }
+
   public Settings getSettings() {
     return delegate;
   }
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 044a1cfeaed..0239f6d8cbf 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
@@ -25,7 +25,6 @@ import org.sonar.api.config.MapSettings;
 import org.sonar.api.config.Settings;
 import org.sonar.api.utils.System2;
 import org.sonar.db.DbTester;
-import org.sonar.db.property.PropertyDto;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
@@ -55,8 +54,7 @@ public class PersistentSettingsTest {
     underTest.saveProperty("foo", null);
 
     assertThat(underTest.getString("foo")).isNull();
-    PropertyDto dto = dbTester.getDbClient().propertiesDao().selectGlobalProperty("foo");
-    assertThat(dto.getValue()).isEmpty();
+    assertThat(dbTester.getDbClient().propertiesDao().selectGlobalProperty("foo")).isNull();
     verify(changeNotifier).onGlobalPropertyChange("foo", null);
   }
 
-- 
2.39.5