SONAR-15142 Audit internal properties

server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesDao.java

@@ -36,6 +36,8 @@ import org.sonar.api.utils.System2;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
+import org.sonar.db.audit.AuditPersister;
+import org.sonar.db.audit.model.PropertyNewValue;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
@@ -55,17 +57,23 @@ public class InternalPropertiesDao implements Dao {
   private static final Optional<String> OPTIONAL_OF_EMPTY_STRING = Optional.of("");
 
   private final System2 system2;
+  private AuditPersister auditPersister;
 
   public InternalPropertiesDao(System2 system2) {
     this.system2 = system2;
   }
 
+  public InternalPropertiesDao(System2 system2, AuditPersister auditPersister) {
+    this.system2 = system2;
+    this.auditPersister = auditPersister;
+  }
+
   /**
    * Save a property which value is not empty.
    * <p>Value can't be {@code null} but can have any size except 0.</p>
-   * 
+   *
    * @throws IllegalArgumentException if {@code key} or {@code value} is {@code null} or empty.
-   * 
+   *
    * @see #saveAsEmpty(DbSession, String)
    */
   public void save(DbSession dbSession, String key, String value) {
@@ -73,13 +81,21 @@ public class InternalPropertiesDao implements Dao {
     checkArgument(value != null && !value.isEmpty(), "value can't be null nor empty");
 
     InternalPropertiesMapper mapper = getMapper(dbSession);
-    mapper.deleteByKey(key);
+    int deletedRows = mapper.deleteByKey(key);
     long now = system2.now();
     if (mustsBeStoredInClob(value)) {
       mapper.insertAsClob(key, value, now);
     } else {
       mapper.insertAsText(key, value, now);
     }
+
+    if (auditPersister != null && auditPersister.isTrackedProperty(key)) {
+      if (deletedRows > 0) {
+        auditPersister.updateProperty(dbSession, new PropertyNewValue(key, value), false);
+      } else {
+        auditPersister.addProperty(dbSession, new PropertyNewValue(key, value), false);
+      }
+    }
   }
 
   private static boolean mustsBeStoredInClob(String value) {
@@ -93,12 +109,24 @@ public class InternalPropertiesDao implements Dao {
     checkKey(key);
 
     InternalPropertiesMapper mapper = getMapper(dbSession);
-    mapper.deleteByKey(key);
+    int deletedRows = mapper.deleteByKey(key);
     mapper.insertAsEmpty(key, system2.now());
+
+    if (auditPersister != null && auditPersister.isTrackedProperty(key)) {
+      if (deletedRows > 0) {
+        auditPersister.updateProperty(dbSession, new PropertyNewValue(key, ""), false);
+      } else {
+        auditPersister.addProperty(dbSession, new PropertyNewValue(key, ""), false);
+      }
+    }
   }
 
   public void delete(DbSession dbSession, String key) {
-    getMapper(dbSession).deleteByKey(key);
+    int deletedRows = getMapper(dbSession).deleteByKey(key);
+
+    if (auditPersister != null && deletedRows > 0 && auditPersister.isTrackedProperty(key)) {
+      auditPersister.deleteProperty(dbSession, new PropertyNewValue(key), false);
+    }
   }
 
   /**

server/sonar-db-dao/src/main/java/org/sonar/db/property/InternalPropertiesMapper.java

@@ -33,7 +33,7 @@ public interface InternalPropertiesMapper {
 
   void insertAsClob(@Param("key") String key, @Param("value") String value, @Param("createdAt") long createdAt);
 
-  void deleteByKey(@Param("key") String key);
+  int deleteByKey(@Param("key") String key);
 
   /**
    * Replace the value of the specified key, only if the existing value matches the expected old value.

server/sonar-db-dao/src/main/java/org/sonar/db/property/PropertiesDao.java

@@ -210,6 +210,10 @@ public class PropertiesDao implements Dao {
    *
    * @throws IllegalArgumentException if {@link PropertyDto#getKey()} is {@code null} or empty
    */
+  public void saveProperty(DbSession session, PropertyDto property) {
+    saveProperty(session, property, null, null, null, null);
+  }
+
   public void saveProperty(DbSession session, PropertyDto property, @Nullable String userLogin, @Nullable String projectKey,
     @Nullable String projectName, @Nullable String qualifier) {
     int affectedRows = save(getMapper(session), property.getKey(), property.getUserUuid(), property.getComponentUuid(), property.getValue());
@@ -253,7 +257,7 @@ public class PropertiesDao implements Dao {
 
   public void saveProperty(PropertyDto property) {
     try (DbSession session = mybatis.openSession(false)) {
-      saveProperty(session, property, null, null, null, null);
+      saveProperty(session, property);
       session.commit();
     }
   }

server/sonar-db-dao/src/test/java/org/sonar/db/property/InternalPropertiesDaoTest.java

@@ -34,24 +34,32 @@ import java.util.stream.Stream;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.assertj.core.api.AbstractAssert;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
 import org.sonar.api.utils.System2;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
+import org.sonar.db.audit.AuditPersister;
+import org.sonar.db.audit.model.PropertyNewValue;
 
 import static java.lang.Boolean.FALSE;
 import static java.lang.Boolean.TRUE;
 import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.entry;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
@@ -70,16 +78,23 @@ public class InternalPropertiesDaoTest {
   private static final String VALUE_SIZE_4001 = VALUE_SIZE_4000 + "P";
   private static final String OTHER_VALUE_SIZE_4001 = VALUE_SIZE_4000 + "D";
 
-  private System2 system2 = mock(System2.class);
+  private final System2 system2 = mock(System2.class);
+  private final String DEFAULT_PROJECT_TEMPLATE = "defaultTemplate.prj";
 
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
   @Rule
   public DbTester dbTester = DbTester.create(system2);
 
-  private DbSession dbSession = dbTester.getSession();
+  private final DbSession dbSession = dbTester.getSession();
+  private final AuditPersister auditPersister = mock(AuditPersister.class);
+  private final InternalPropertiesDao underTest = new InternalPropertiesDao(system2, auditPersister);
+  private final ArgumentCaptor<PropertyNewValue> newValueCaptor = ArgumentCaptor.forClass(PropertyNewValue.class);
 
-  private InternalPropertiesDao underTest = new InternalPropertiesDao(system2);
+  @Before
+  public void setUp() {
+    when(auditPersister.isTrackedProperty(DEFAULT_PROJECT_TEMPLATE)).thenReturn(true);
+  }
 
   @Test
   public void save_throws_IAE_if_key_is_null() {
@@ -119,6 +134,54 @@ public class InternalPropertiesDaoTest {
       .hasCreatedAt(DATE_2);
   }
 
+  @Test
+  public void delete_removes_value() {
+    underTest.save(dbSession, A_KEY, VALUE_SMALL);
+    dbSession.commit();
+    assertThat(dbTester.countRowsOfTable("internal_properties")).isOne();
+    clearInvocations(auditPersister);
+    
+    underTest.delete(dbSession, A_KEY);
+    dbSession.commit();
+
+    assertThat(dbTester.countRowsOfTable("internal_properties")).isZero();
+    verify(auditPersister).isTrackedProperty(A_KEY);
+    verifyNoMoreInteractions(auditPersister);
+  }
+
+  @Test
+  public void delete_audits_if_value_was_deleted_and_it_is_tracked_key() {
+    underTest.save(dbSession, DEFAULT_PROJECT_TEMPLATE, VALUE_SMALL);
+    clearInvocations(auditPersister);
+    underTest.delete(dbSession, DEFAULT_PROJECT_TEMPLATE);
+    verify(auditPersister).deleteProperty(any(), newValueCaptor.capture(), eq(false));
+    assertAuditValue(DEFAULT_PROJECT_TEMPLATE, null);
+  }
+
+  @Test
+  public void delete_audits_does_not_audit_if_nothing_was_deleted() {
+    underTest.delete(dbSession, DEFAULT_PROJECT_TEMPLATE);
+    verifyNoInteractions(auditPersister);
+  }
+
+  @Test
+  public void save_audits_if_key_is_tracked() {
+    underTest.save(dbSession, DEFAULT_PROJECT_TEMPLATE, VALUE_SMALL);
+    verify(auditPersister).addProperty(any(), newValueCaptor.capture(), eq(false));
+    assertAuditValue(DEFAULT_PROJECT_TEMPLATE, VALUE_SMALL);
+  }
+
+  @Test
+  public void save_audits_update_if_key_is_tracked_and_updated() {
+    underTest.save(dbSession, DEFAULT_PROJECT_TEMPLATE, "first value");
+
+    Mockito.clearInvocations(auditPersister);
+
+    underTest.save(dbSession, DEFAULT_PROJECT_TEMPLATE, VALUE_SMALL);
+    verify(auditPersister).updateProperty(any(), newValueCaptor.capture(), eq(false));
+    assertAuditValue(DEFAULT_PROJECT_TEMPLATE, VALUE_SMALL);
+  }
+
   @Test
   public void save_persists_value_in_varchar_if_4000() {
     when(system2.now()).thenReturn(DATE_1);
@@ -226,6 +289,22 @@ public class InternalPropertiesDaoTest {
       .hasCreatedAt(DATE_2);
   }
 
+  @Test
+  public void saveAsEmpty_audits_if_key_is_tracked() {
+    underTest.saveAsEmpty(dbSession, DEFAULT_PROJECT_TEMPLATE);
+    verify(auditPersister).addProperty(any(), newValueCaptor.capture(), eq(false));
+    assertAuditValue(DEFAULT_PROJECT_TEMPLATE, "");
+  }
+
+  @Test
+  public void saveAsEmpty_audits_update_if_key_is_tracked_and_updated() {
+    underTest.save(dbSession, DEFAULT_PROJECT_TEMPLATE, "first value");
+    Mockito.clearInvocations(auditPersister);
+    underTest.saveAsEmpty(dbSession, DEFAULT_PROJECT_TEMPLATE);
+    verify(auditPersister).updateProperty(any(), newValueCaptor.capture(), eq(false));
+    assertAuditValue(DEFAULT_PROJECT_TEMPLATE, "");
+  }
+
   @Test
   public void saveAsEmpty_persist_property_without_textvalue_nor_clob_value_when_old_value_was_in_varchar() {
     when(system2.now()).thenReturn(DATE_1, DATE_2);
@@ -500,6 +579,14 @@ public class InternalPropertiesDaoTest {
     underTest.tryLock(dbSession, tooLongName, 60);
   }
 
+  private void assertAuditValue(String key, @Nullable String value) {
+    verify(auditPersister).isTrackedProperty(key);
+    PropertyNewValue newValue = newValueCaptor.getValue();
+    assertThat(newValue)
+      .extracting(PropertyNewValue::getPropertyKey, PropertyNewValue::getPropertyValue, PropertyNewValue::getUserUuid, PropertyNewValue::getUserLogin)
+      .containsExactly(key, value, null, null);
+  }
+
   private static String propertyKeyOf(String lockName) {
     return "lock." + lockName;
   }
@@ -550,10 +637,6 @@ public class InternalPropertiesDaoTest {
       throw new IllegalArgumentException("Unsupported object type returned for column \"isEmpty\": " + flag.getClass());
     }
 
-    public void doesNotExist() {
-      isNull();
-    }
-
     public InternalPropertyAssert isEmpty() {
       isNotNull();
 

server/sonar-webserver-api/src/main/java/org/sonar/server/plugins/PluginConsentVerifier.java

@@ -54,7 +54,7 @@ public class PluginConsentVerifier implements Startable {
       if (hasExternalPlugins && NOT_ACCEPTED == PluginRiskConsent.valueOf(property.getValue())) {
         addWarningInSonarDotLog();
         property.setValue(REQUIRED.name());
-        dbClient.propertiesDao().saveProperty(session, property, null, null, null, null);
+        dbClient.propertiesDao().saveProperty(session, property);
         session.commit();
       } else if (!hasExternalPlugins && REQUIRED == PluginRiskConsent.valueOf(property.getValue())) {
         dbClient.propertiesDao().deleteGlobalProperty(PLUGINS_RISK_CONSENT, session);

server/sonar-webserver-api/src/main/java/org/sonar/server/project/ProjectDefaultVisibility.java

@@ -46,6 +46,6 @@ public class ProjectDefaultVisibility {
   public void set(DbSession dbSession, Visibility visibility) {
     dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto()
       .setKey(PROJECTS_DEFAULT_VISIBILITY_PROPERTY_NAME)
-      .setValue(visibility.getLabel()), null, null, null, null);
+      .setValue(visibility.getLabel()));
   }
 }

server/sonar-webserver-core/src/main/java/org/sonar/server/platform/PersistentSettings.java

@@ -72,7 +72,7 @@ public class PersistentSettings {
     if (value == null) {
       dbClient.propertiesDao().deleteGlobalProperty(key, dbSession);
     } else {
-      dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setKey(key).setValue(value), null, null, null, null);
+      dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setKey(key).setValue(value));
     }
     // refresh the cache of settings
     delegate.setProperty(key, value);

server/sonar-webserver-core/src/main/java/org/sonar/server/platform/serverid/ServerIdManager.java

@@ -139,8 +139,7 @@ public class ServerIdManager implements Startable {
   }
 
   private void persistServerId(DbSession dbSession, ServerId serverId) {
-    dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setKey(SERVER_ID).setValue(serverId.toString()),
-      null, null, null, null);
+    dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto().setKey(SERVER_ID).setValue(serverId.toString()));
   }
 
   private void persistChecksum(DbSession dbSession, String checksump) {