SONAR-15142 Logs for add/update/delete are inconsistent for webhooks and devops

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java
index b946a7c87972351e2acd48ce7b4b133e1c11d05b..35f8ae7fa2e5db73b9f94a95a02c461a5381a7d3 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDao.java
@@ -58,8 +58,7 @@ public class AlmSettingDao implements Dao {
     getMapper(dbSession).insert(almSettingDto);
 
     if (auditPersister != null) {
-      auditPersister.addDevOpsPlatformSetting(dbSession, new DevOpsPlatformSettingNewValue(almSettingDto.getUuid(),
-        almSettingDto.getKey()));
+      auditPersister.addDevOpsPlatformSetting(dbSession, new DevOpsPlatformSettingNewValue(almSettingDto));
     }
   }
 
@@ -83,8 +82,7 @@ public class AlmSettingDao implements Dao {
     getMapper(dbSession).deleteByKey(almSettingDto.getKey());
 
     if (auditPersister != null) {
-      auditPersister.deleteDevOpsPlatformSetting(dbSession, new DevOpsPlatformSettingNewValue(almSettingDto.getUuid(),
-        almSettingDto.getKey()));
+      auditPersister.deleteDevOpsPlatformSetting(dbSession, new DevOpsPlatformSettingNewValue(almSettingDto.getUuid(), almSettingDto.getKey()));
     }
   }
 

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDto.java
index 4f56d4b3989872270542fac1c45212d22dc59dc9..94a74ff68ed2308f4e801c198c95eee723404616 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDto.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/AlmSettingDto.java
@@ -89,8 +89,9 @@ public class AlmSettingDto {
     return uuid;
   }
 
-  void setUuid(String uuid) {
+  AlmSettingDto setUuid(String uuid) {
     this.uuid = uuid;
+    return this;
   }
 
   public String getKey() {

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/ProjectAlmSettingDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/ProjectAlmSettingDao.java
index caa11bea5b550f3b00f43766eeece2f7303f89c0..6333987cbbd4b0e769bab4e8f0e93b33c76aac41 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/ProjectAlmSettingDao.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/alm/setting/ProjectAlmSettingDao.java
@@ -48,8 +48,7 @@ public class ProjectAlmSettingDao implements Dao {
     this.auditPersister = auditPersister;
   }
 
-  public void insertOrUpdate(DbSession dbSession, ProjectAlmSettingDto projectAlmSettingDto,
-    String key, String projectName) {
+  public void insertOrUpdate(DbSession dbSession, ProjectAlmSettingDto projectAlmSettingDto, String key, String projectName) {
     String uuid = uuidFactory.create();
     long now = system2.now();
     ProjectAlmSettingMapper mapper = getMapper(dbSession);
@@ -64,11 +63,11 @@ public class ProjectAlmSettingDao implements Dao {
     projectAlmSettingDto.setUpdatedAt(now);
 
     if (auditPersister != null) {
+      DevOpsPlatformSettingNewValue value = new DevOpsPlatformSettingNewValue(projectAlmSettingDto, key, projectName);
       if (isUpdate) {
-        auditPersister.updateDevOpsPlatformSetting(dbSession, new DevOpsPlatformSettingNewValue(projectAlmSettingDto, key, projectName));
+        auditPersister.updateDevOpsPlatformSetting(dbSession, value);
       } else {
-        auditPersister.addDevOpsPlatformSetting(dbSession, new DevOpsPlatformSettingNewValue(projectAlmSettingDto.getAlmSettingUuid(),
-          projectAlmSettingDto.getProjectUuid(), key, projectName));
+        auditPersister.addDevOpsPlatformSetting(dbSession, value);
       }
     }
   }
@@ -82,16 +81,7 @@ public class ProjectAlmSettingDao implements Dao {
   }
 
   public void deleteByAlmSetting(DbSession dbSession, AlmSettingDto almSetting) {
-    deleteByAlmSetting(dbSession, almSetting, false);
-  }
-
-  public void deleteByAlmSetting(DbSession dbSession, AlmSettingDto almSetting, boolean track) {
     getMapper(dbSession).deleteByAlmSettingUuid(almSetting.getUuid());
-
-    if (track && auditPersister != null) {
-      auditPersister.deleteDevOpsPlatformSetting(dbSession, new DevOpsPlatformSettingNewValue(almSetting.getUuid(),
-        almSetting.getKey()));
-    }
   }
 
   public int countByAlmSetting(DbSession dbSession, AlmSettingDto almSetting) {

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/DevOpsPlatformSettingNewValue.java b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/DevOpsPlatformSettingNewValue.java
index bfef1eac78b9431729cfa81736a6c893bf66996f..71876aa96c05a0c5648cfa611cb31781d8ed3b80 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/DevOpsPlatformSettingNewValue.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/DevOpsPlatformSettingNewValue.java
@@ -88,13 +88,6 @@ public class DevOpsPlatformSettingNewValue implements NewValue {
     this.isMonorepo = dto.getMonorepo();
   }
 
-  public DevOpsPlatformSettingNewValue(String devOpsPlatformSettingUuid, String projectUuid, String key, String projectName) {
-    this.devOpsPlatformSettingUuid = devOpsPlatformSettingUuid;
-    this.key = key;
-    this.projectUuid = projectUuid;
-    this.projectName = projectName;
-  }
-
   public DevOpsPlatformSettingNewValue(ProjectDto projectDto) {
     this.projectUuid = projectDto.getUuid();
     this.projectName = projectDto.getName();

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/WebhookNewValue.java b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/WebhookNewValue.java
index cdf2be4463c207c84bfc77fee16ce217a6bfe12b..c5a0c49043ddcf62b5f9b6d91e9d4ee23eb257d3 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/WebhookNewValue.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/audit/model/WebhookNewValue.java
@@ -41,20 +41,19 @@ public class WebhookNewValue implements NewValue {
   private String projectName;
 
   public WebhookNewValue(WebhookDto dto, @Nullable String projectName) {
-    this.webhookUuid = dto.getUuid();
-    this.name = dto.getName();
-    this.url = dto.getUrl();
-    this.projectUuid = dto.getProjectUuid();
-    this.projectName = projectName;
+    this(dto.getUuid(), dto.getName(), dto.getProjectUuid(), projectName, dto.getUrl());
   }
 
   public WebhookNewValue(String webhookUuid, String webhookName) {
+
     this.webhookUuid = webhookUuid;
     this.name = webhookName;
   }
 
-  public WebhookNewValue(String webhookUuid, String webhookName, @Nullable String projectUuid, @Nullable String projectName) {
-    this(webhookUuid, webhookName);
+  public WebhookNewValue(String webhookUuid, String webhookName, @Nullable String projectUuid, @Nullable String projectName, @Nullable String url) {
+    this.webhookUuid = webhookUuid;
+    this.name = webhookName;
+    this.url = url;
     this.projectUuid = projectUuid;
     this.projectName = projectName;
   }

diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java
index 246c3e89f9162f7a517a39a5dba05ab5c964a589..7d906aec2facb86bcdfe84f09dee335855bbb13d 100644 (file)
--- a/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java
+++ b/server/sonar-db-dao/src/main/java/org/sonar/db/webhook/WebhookDao.java
@@ -60,8 +60,7 @@ public class WebhookDao implements Dao {
     mapper(dbSession).insert(dto.setCreatedAt(system2.now()).setUpdatedAt(system2.now()));
 
     if (auditPersister != null) {
-      auditPersister.addWebhook(dbSession, new WebhookNewValue(dto.getUuid(), dto.getName(),
-        dto.getProjectUuid(), projectName));
+      auditPersister.addWebhook(dbSession, new WebhookNewValue(dto, projectName));
     }
   }
 

diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java
index fbde7b47b9b8002b0ef4895ed5f625e921c8daeb..9d734652e28770681a9cf136f9bfc0b13a4feea2 100644 (file)
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/AlmSettingDaoWithPersisterTest.java
@@ -55,34 +55,36 @@ public class AlmSettingDaoWithPersisterTest {
   @Test
   public void insertAndUpdateArePersisted() {
     when(uuidFactory.create()).thenReturn(A_UUID);
-    AlmSettingDto almSettingDto = newGithubAlmSettingDto();
+    AlmSettingDto almSettingDto = newGithubAlmSettingDto()
+      .setKey("key")
+      .setAppId("id1")
+      .setClientId("cid1")
+      .setUrl("url");
 
     underTest.insert(dbSession, almSettingDto);
 
     verify(auditPersister).addDevOpsPlatformSetting(eq(dbSession), newValueCaptor.capture());
     DevOpsPlatformSettingNewValue newValue = newValueCaptor.getValue();
     assertThat(newValue)
-      .extracting(DevOpsPlatformSettingNewValue::getDevOpsPlatformSettingUuid, DevOpsPlatformSettingNewValue::getKey)
+      .extracting("devOpsPlatformSettingUuid", "key")
       .containsExactly(almSettingDto.getUuid(), almSettingDto.getKey());
-    assertThat(newValue.toString()).doesNotContain("url");
+    assertThat(newValue).hasToString("{\"devOpsPlatformSettingUuid\": \"1\", \"key\": \"key\", \"devOpsPlatformName\": \"id1\", \"url\": \"url\", \"appId\": \"id1\", \"clientId\": \"cid1\" }");
 
     almSettingDto.setPrivateKey("updated private key");
     almSettingDto.setAppId("updated app id");
     almSettingDto.setUrl("updated url");
     almSettingDto.setPersonalAccessToken("updated pat");
     almSettingDto.setKey("updated key");
-    system2.setNow(NOW + 1);
 
     underTest.update(dbSession, almSettingDto);
 
     verify(auditPersister).updateDevOpsPlatformSetting(eq(dbSession), newValueCaptor.capture());
     newValue = newValueCaptor.getValue();
     assertThat(newValue)
-      .extracting(DevOpsPlatformSettingNewValue::getDevOpsPlatformSettingUuid, DevOpsPlatformSettingNewValue::getKey,
-        DevOpsPlatformSettingNewValue::getAppId, DevOpsPlatformSettingNewValue::getDevOpsPlatformName,
-        DevOpsPlatformSettingNewValue::getUrl, DevOpsPlatformSettingNewValue::getClientId)
-      .containsExactly(almSettingDto.getUuid(), almSettingDto.getKey(), almSettingDto.getAppId(), almSettingDto.getAppId(),
-        almSettingDto.getUrl(), almSettingDto.getClientId());
+      .extracting("devOpsPlatformSettingUuid", "key","appId", "devOpsPlatformName", "url", "clientId")
+      .containsExactly(almSettingDto.getUuid(), almSettingDto.getKey(), almSettingDto.getAppId(), almSettingDto.getAppId(), almSettingDto.getUrl(), almSettingDto.getClientId());
+    assertThat(newValue).hasToString("{\"devOpsPlatformSettingUuid\": \"1\", \"key\": \"updated key\", \"devOpsPlatformName\": \"updated app id\", "
+        + "\"url\": \"updated url\", \"appId\": \"updated app id\", \"clientId\": \"cid1\" }");
   }
 
   @Test
@@ -96,7 +98,7 @@ public class AlmSettingDaoWithPersisterTest {
     verify(auditPersister).deleteDevOpsPlatformSetting(eq(dbSession), newValueCaptor.capture());
     DevOpsPlatformSettingNewValue newValue = newValueCaptor.getValue();
     assertThat(newValue)
-      .extracting(DevOpsPlatformSettingNewValue::getDevOpsPlatformSettingUuid, DevOpsPlatformSettingNewValue::getKey)
+      .extracting("devOpsPlatformSettingUuid", "key")
       .containsExactly(almSettingDto.getUuid(), almSettingDto.getKey());
     assertThat(newValue.toString()).doesNotContain("url");
   }

diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/ProjectAlmSettingDaoWithPersisterTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/ProjectAlmSettingDaoWithPersisterTest.java
index 25d3ce811ea9f0ab4af493b35f7f3403cd6f1568..1d33930bd2e6e4109836b14955a16d9b67e3901a 100644 (file)
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/ProjectAlmSettingDaoWithPersisterTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/alm/setting/ProjectAlmSettingDaoWithPersisterTest.java
@@ -109,24 +109,10 @@ public class ProjectAlmSettingDaoWithPersisterTest {
   }
 
   @Test
-  public void deleteByAlmSettingWhenNotTracked() {
+  public void deleteByAlmSettingNotTracked() {
     AlmSettingDto githubAlmSetting = db.almSettings().insertGitHubAlmSetting();
     underTest.deleteByAlmSetting(dbSession, githubAlmSetting);
 
     verifyNoInteractions(auditPersister);
   }
-
-  @Test
-  public void deleteByAlmSettingWhenTracked() {
-    AlmSettingDto githubAlmSetting = db.almSettings().insertGitHubAlmSetting();
-    underTest.deleteByAlmSetting(dbSession, githubAlmSetting, true);
-
-    verify(auditPersister).deleteDevOpsPlatformSetting(eq(dbSession), newValueCaptor.capture());
-    DevOpsPlatformSettingNewValue newValue = newValueCaptor.getValue();
-    assertThat(newValue)
-      .extracting(DevOpsPlatformSettingNewValue::getDevOpsPlatformSettingUuid, DevOpsPlatformSettingNewValue::getKey)
-      .containsExactly(githubAlmSetting.getUuid(), githubAlmSetting.getKey());
-    assertThat(newValue.toString()).doesNotContain("url");
-  }
-
 }

diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java
index edb6f4073684e9d49d808c7c76e14eeb1230e49f..7dd4c49e67445f527536030b9d287bcee74bcb44 100644 (file)
--- a/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java
+++ b/server/sonar-db-dao/src/test/java/org/sonar/db/webhook/WebhookDaoWithPersisterTest.java
@@ -37,7 +37,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
 public class WebhookDaoWithPersisterTest {
-  private AuditPersister auditPersister = mock(AuditPersister.class);
+  private final AuditPersister auditPersister = mock(AuditPersister.class);
 
   @Rule
   public final DbTester dbTester = DbTester.create(System2.INSTANCE, auditPersister);
@@ -65,7 +65,7 @@ public class WebhookDaoWithPersisterTest {
     assertThat(newValue)
       .extracting(WebhookNewValue::getWebhookUuid, WebhookNewValue::getName)
       .containsExactly(dto.getUuid(), dto.getName());
-    assertThat(newValue.toString()).doesNotContain("url");
+    assertThat(newValue).hasToString("{\"webhookUuid\": \"UUID_1\", \"name\": \"NAME_1\", \"url\": \"URL_1\" }");
   }
 
   @Test
@@ -82,14 +82,13 @@ public class WebhookDaoWithPersisterTest {
     verify(auditPersister).addWebhook(eq(dbSession), newValueCaptor.capture());
     WebhookNewValue newValue = newValueCaptor.getValue();
     assertThat(newValue)
-      .extracting(WebhookNewValue::getWebhookUuid, WebhookNewValue::getName, WebhookNewValue::getProjectUuid,
-        WebhookNewValue::getProjectName)
+      .extracting(WebhookNewValue::getWebhookUuid, WebhookNewValue::getName, WebhookNewValue::getProjectUuid, WebhookNewValue::getProjectName)
       .containsExactly(dto.getUuid(), dto.getName(), dto.getProjectUuid(), "project_name");
-    assertThat(newValue.toString()).doesNotContain("url");
+    assertThat(newValue).hasToString("{\"webhookUuid\": \"UUID_1\", \"name\": \"NAME_1\", \"url\": \"URL_1\", \"projectUuid\": \"UUID_2\", \"projectName\": \"project_name\" }");
   }
 
   @Test
-  public void updateWebhookIsPersisted() {
+  public void updateGlobalWebhookIsPersisted() {
     WebhookDto dto = webhookDbTester.insertGlobalWebhook();
     dto = dto
       .setName("a-fancy-webhook")
@@ -103,12 +102,30 @@ public class WebhookDaoWithPersisterTest {
     assertThat(newValue)
       .extracting(WebhookNewValue::getWebhookUuid, WebhookNewValue::getName, WebhookNewValue::getUrl)
       .containsExactly(dto.getUuid(), dto.getName(), dto.getUrl());
-    assertThat(newValue.toString()).doesNotContain("projectUuid");
+    assertThat(newValue).hasToString("{\"webhookUuid\": \"" + dto.getUuid() + "\", \"name\": \"a-fancy-webhook\", \"url\": \"http://www.fancy-webhook.io\" }");
+  }
+
+  @Test
+  public void updateProjectWebhookIsPersisted() {
+    WebhookDto dto = webhookDbTester.insertGlobalWebhook();
+    dto = dto
+      .setName("a-fancy-webhook")
+      .setUrl("http://www.fancy-webhook.io")
+      .setSecret(null);
+
+    underTest.update(dbSession, dto, "project");
+
+    verify(auditPersister).updateWebhook(eq(dbSession), newValueCaptor.capture());
+    WebhookNewValue newValue = newValueCaptor.getValue();
+    assertThat(newValue)
+      .extracting(WebhookNewValue::getWebhookUuid, WebhookNewValue::getName, WebhookNewValue::getUrl)
+      .containsExactly(dto.getUuid(), dto.getName(), dto.getUrl());
+    assertThat(newValue).hasToString("{\"webhookUuid\": \"" + dto.getUuid() +"\", \"name\": \"a-fancy-webhook\", \"url\": \"http://www.fancy-webhook.io\", \"projectName\": \"project\" }");
   }
 
   @Test
   public void deleteProjectWebhooksIsPersisted() {
-    ProjectDto projectDto = componentDbTester.insertPrivateProjectDto();
+    ProjectDto projectDto = componentDbTester.insertPrivateProjectDto(p -> p.setUuid("puuid").setName("pname"));
     webhookDbTester.insertWebhook(projectDto);
 
     underTest.deleteByProject(dbSession, projectDto);
@@ -118,7 +135,7 @@ public class WebhookDaoWithPersisterTest {
     assertThat(newValue)
       .extracting(WebhookNewValue::getProjectUuid, WebhookNewValue::getProjectName)
       .containsExactly(projectDto.getUuid(), projectDto.getName());
-    assertThat(newValue.toString()).doesNotContain("webhookUuid");
+    assertThat(newValue).hasToString("{\"projectUuid\": \"puuid\", \"projectName\": \"pname\" }");
   }
 
   @Test
@@ -132,6 +149,6 @@ public class WebhookDaoWithPersisterTest {
     assertThat(newValue)
       .extracting(WebhookNewValue::getWebhookUuid, WebhookNewValue::getName)
       .containsExactly(dto.getUuid(), dto.getName());
-    assertThat(newValue.toString()).doesNotContain("url");
+    assertThat(newValue).hasToString("{\"webhookUuid\": \"" + dto.getUuid() + "\", \"name\": \"" + dto.getName() + "\" }");
   }
 }