More fixes

Author: adrw

Diff for: service-self-backfill/build.gradle.kts

@@ -61,6 +61,7 @@ dependencies {
   implementation(project(":client-misk-hibernate"))
   implementation(project(":service"))
 
+  testImplementation(testFixtures(libs.miskAuditClient))
   testImplementation(libs.miskTesting)
   testImplementation(libs.miskHibernateTesting)
   testImplementation(libs.junitApi)

Diff for: service-self-backfill/src/test/kotlin/app/cash/backfila/service/selfbackfill/SelfBackfillTestingModule.kt

@@ -17,6 +17,8 @@ import app.cash.backfila.service.runner.BackfillRunnerLoggingSetupProvider
 import app.cash.backfila.service.runner.BackfillRunnerNoLoggingSetupProvider
 import misk.MiskCaller
 import misk.MiskTestingServiceModule
+import misk.audit.FakeAuditClientModule
+import misk.config.AppNameModule
 import misk.environment.DeploymentModule
 import misk.hibernate.HibernateTestingModule
 import misk.inject.KAbstractModule
@@ -50,6 +52,8 @@ class SelfBackfillTestingModule : KAbstractModule() {
     )
 
     install(BackfilaListenerModule())
+    install(AppNameModule("self-backfila"))
+    install(FakeAuditClientModule())
 
     bind<BackfilaConfig>().toInstance(config)
     install(DeploymentModule(TESTING))

Diff for: service/src/main/kotlin/app/cash/backfila/service/listener/AuditClientListener.kt

@@ -10,63 +10,63 @@ import misk.hibernate.Id
 import misk.hibernate.Transacter
 import misk.hibernate.load
 
-class AuditClientListener @Inject constructor(
+internal class AuditClientListener @Inject constructor(
   @BackfilaDb private val transacter: Transacter,
   private val auditClient: AuditClient,
   private val backfilaConfig: BackfilaConfig,
 ) : BackfillRunListener {
   override fun runStarted(id: Id<DbBackfillRun>, user: String) {
-    val (backfillName, run, description) = transacter.transaction { session ->
+    val (backfillName, serviceName, description) = transacter.transaction { session ->
       val run = session.load<DbBackfillRun>(id)
-      AuditEvent(run.registered_backfill.name, run, "Backfill started by $user ${dryRunPrefix(run)}${nameAndId(run)}")
+      AuditEventInputs(run.registered_backfill.name, serviceName(run), "Backfill started by $user ${dryRunPrefix(run)}${nameAndId(run)}")
     }
     auditClient.logEvent(
       target = backfillName,
       description = description,
       requestorLDAP = user,
-      applicationName = serviceName(run),
+      applicationName = serviceName,
       detailURL = idUrl(id),
     )
   }
 
   override fun runPaused(id: Id<DbBackfillRun>, user: String) {
-    val (backfillName, run, description) = transacter.transaction { session ->
+    val (backfillName, serviceName, description) = transacter.transaction { session ->
       val run = session.load<DbBackfillRun>(id)
-      AuditEvent(run.registered_backfill.name, run, "Backfill paused by $user ${dryRunPrefix(run)}${nameAndId(run)}")
+      AuditEventInputs(run.registered_backfill.name, serviceName(run), "Backfill paused by $user ${dryRunPrefix(run)}${nameAndId(run)}")
     }
     auditClient.logEvent(
       target = backfillName,
       description = description,
       requestorLDAP = user,
-      applicationName = serviceName(run),
+      applicationName = serviceName,
       detailURL = idUrl(id),
     )
   }
 
   override fun runErrored(id: Id<DbBackfillRun>) {
-    val (backfillName, run, description) = transacter.transaction { session ->
+    val (backfillName, serviceName, description) = transacter.transaction { session ->
       val run = session.load<DbBackfillRun>(id)
-      AuditEvent(run.registered_backfill.name, run, "Backfill paused due to error ${dryRunPrefix(run)}${nameAndId(run)}")
+      AuditEventInputs(run.registered_backfill.name, serviceName(run), "Backfill paused due to error ${dryRunPrefix(run)}${nameAndId(run)}")
     }
     auditClient.logEvent(
       target = backfillName,
       description = description,
       automatedChange = true,
-      applicationName = serviceName(run),
+      applicationName = serviceName,
       detailURL = idUrl(id),
     )
   }
 
   override fun runCompleted(id: Id<DbBackfillRun>) {
-    val (backfillName, run, description) = transacter.transaction { session ->
+    val (backfillName, serviceName, description) = transacter.transaction { session ->
       val run = session.load<DbBackfillRun>(id)
-      AuditEvent(run.registered_backfill.name, run, "Backfill completed ${dryRunPrefix(run)}${nameAndId(run)}")
+      AuditEventInputs(run.registered_backfill.name, serviceName(run), "Backfill completed ${dryRunPrefix(run)}${nameAndId(run)}")
     }
     auditClient.logEvent(
       target = backfillName,
       description = description,
       automatedChange = true,
-      applicationName = serviceName(run),
+      applicationName = serviceName,
       detailURL = idUrl(id),
     )
   }
@@ -94,9 +94,9 @@ class AuditClientListener @Inject constructor(
 
   private fun idUrl(id: Id<DbBackfillRun>): String = backfilaConfig.web_url_root + BackfillShowAction.path(id.id)
 
-  private data class AuditEvent(
+  private data class AuditEventInputs(
     val backfillName: String,
-    val run: DbBackfillRun,
+    val serviceName: String,
     val description: String,
   )
 }

Diff for: service/src/test/kotlin/app/cash/backfila/service/listener/AuditClientListenerTest.kt

@@ -28,7 +28,7 @@ import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
 
 @MiskTest(startService = true)
-class AuditClientListenerTest {
+internal class AuditClientListenerTest {
   @Suppress("unused")
   @MiskTestModule
   val module: Module = BackfilaTestingModule()
@@ -114,7 +114,7 @@ class AuditClientListenerTest {
           timestampSent = 2147483647,
           applicationName = "deep-fryer",
           // Fix fake client to not provide approver unless explicitly present
-          approverLDAP = "molly",
+          approverLDAP = null,
           automatedChange = false,
           description = "Backfill started by molly [dryRun=true][service=deep-fryer][backfill=ChickenSandwich][id=${response.backfill_run_id}]",
           richDescription = null,
@@ -134,7 +134,7 @@ class AuditClientListenerTest {
           eventTarget = "ChickenSandwich",
           timestampSent = 2147483647,
           applicationName = "deep-fryer",
-          approverLDAP = "molly",
+          approverLDAP = null,
           automatedChange = false,
           description = "Backfill paused by molly [dryRun=true][service=deep-fryer][backfill=ChickenSandwich][id=${response.backfill_run_id}]",
           richDescription = null,
@@ -154,14 +154,14 @@ class AuditClientListenerTest {
           eventTarget = "ChickenSandwich",
           timestampSent = 2147483647,
           applicationName = "deep-fryer",
-          approverLDAP = "molly",
+          approverLDAP = null,
           automatedChange = true,
           description = "Backfill paused due to error [dryRun=true][service=deep-fryer][backfill=ChickenSandwich][id=${response.backfill_run_id}]",
           richDescription = null,
           environment = "testing",
           detailURL = "/backfills/${response.backfill_run_id}",
           region = "us-west-2",
-          requestorLDAP = "molly",
+          requestorLDAP = null,
         ),
         fakeAuditClient.sentEvents.last(),
       )
@@ -174,14 +174,14 @@ class AuditClientListenerTest {
           eventTarget = "ChickenSandwich",
           timestampSent = 2147483647,
           applicationName = "deep-fryer",
-          approverLDAP = "molly",
+          approverLDAP = null,
           automatedChange = true,
           description = "Backfill completed [dryRun=true][service=deep-fryer][backfill=ChickenSandwich][id=${response.backfill_run_id}]",
           richDescription = null,
           environment = "testing",
           detailURL = "/backfills/${response.backfill_run_id}",
           region = "us-west-2",
-          requestorLDAP = "molly",
+          requestorLDAP = null,
         ),
         fakeAuditClient.sentEvents.last(),
       )