Dont save dry-runs in history table

ExpediaGroup · Nov 27, 2024 · e6a5faa · e6a5faa
1 parent c8f3010
commit e6a5faa
Show file tree

Hide file tree

Showing 11 changed files with 35 additions and 41 deletions.
diff --git a/...r-core/src/main/java/com/expediagroup/beekeeper/core/service/BeekeeperHistoryService.java b/...r-core/src/main/java/com/expediagroup/beekeeper/core/service/BeekeeperHistoryService.java
@@ -5,12 +5,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.SerializationFeature;
-import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
-
 import com.expediagroup.beekeeper.core.model.HousekeepingEntity;
+import com.expediagroup.beekeeper.core.model.HousekeepingStatus;
 import com.expediagroup.beekeeper.core.model.history.BeekeeperHistory;
 import com.expediagroup.beekeeper.core.repository.BeekeeperHistoryRepository;
 
@@ -24,7 +20,7 @@ public BeekeeperHistoryService(BeekeeperHistoryRepository beekeeperHistoryReposi
     this.beekeeperHistoryRepository = beekeeperHistoryRepository;
   }
 
-  public void saveHistory(HousekeepingEntity housekeepingEntity, String status) {
+  public void saveHistory(HousekeepingEntity housekeepingEntity, HousekeepingStatus status) {
     String eventDetails = getEventDetails(housekeepingEntity);
 
     BeekeeperHistory event = BeekeeperHistory.builder()
@@ -33,7 +29,7 @@ public void saveHistory(HousekeepingEntity housekeepingEntity, String status) {
         .databaseName(housekeepingEntity.getDatabaseName())
         .tableName(housekeepingEntity.getTableName())
         .lifecycleType(housekeepingEntity.getLifecycleType())
-        .housekeepingStatus(status)
+        .housekeepingStatus(status.name())
         .eventDetails(eventDetails)
         .build();
 

diff --git a/...re/src/test/java/com/expediagroup/beekeeper/core/service/BeekeeperHistoryServiceTest.java b/...re/src/test/java/com/expediagroup/beekeeper/core/service/BeekeeperHistoryServiceTest.java
@@ -2,6 +2,7 @@
 
 import static org.mockito.Mockito.verify;
 
+import static com.expediagroup.beekeeper.core.model.HousekeepingStatus.DELETED;
 import static com.expediagroup.beekeeper.core.model.HousekeepingStatus.SCHEDULED;
 import static com.expediagroup.beekeeper.core.model.LifecycleEventType.EXPIRED;
 import static com.expediagroup.beekeeper.core.model.LifecycleEventType.UNREFERENCED;
@@ -51,7 +52,7 @@ void expiredHistory() throws JsonProcessingException {
     String details = createEventDetails(metadata);
     BeekeeperHistory history = createHistoryEvent(metadata, details, "DELETED");
 
-    beekeeperHistoryService.saveHistory(metadata, "DELETED");
+    beekeeperHistoryService.saveHistory(metadata, DELETED);
     verify(repository).save(history);
   }
 
@@ -61,7 +62,7 @@ void unreferencedHistory() throws JsonProcessingException {
     String details = createEventDetails(path);
     BeekeeperHistory history = createHistoryEvent(path, details, "SCHEDULED");
 
-    beekeeperHistoryService.saveHistory(path, "SCHEDULED");
+    beekeeperHistoryService.saveHistory(path, SCHEDULED);
     verify(repository).save(history);
   }
 

diff --git a/...main/java/com/expediagroup/beekeeper/metadata/cleanup/handler/ExpiredMetadataHandler.java b/...main/java/com/expediagroup/beekeeper/metadata/cleanup/handler/ExpiredMetadataHandler.java
@@ -79,10 +79,8 @@ public Slice<HousekeepingMetadata> findRecordsToClean(LocalDateTime instant, Pag
   public void cleanupMetadata(HousekeepingMetadata housekeepingMetadata, LocalDateTime instant, boolean dryRunEnabled) {
     try (CleanerClient client = cleanerClientFactory.newInstance()) {
       boolean deleted = cleanup(client, housekeepingMetadata, instant, dryRunEnabled);
-      if (deleted) {
-        if (!dryRunEnabled) {
-          updateAttemptsAndStatus(housekeepingMetadata, DELETED);
-        }
+      if (deleted && !dryRunEnabled) {
+        updateAttemptsAndStatus(housekeepingMetadata, DELETED);
         saveHistory(housekeepingMetadata, DELETED, dryRunEnabled);
       }
     } catch (Exception e) {
@@ -189,10 +187,9 @@ private Long countPartitionsForDatabaseAndTable(
 
   private void saveHistory(HousekeepingMetadata metadata, HousekeepingStatus housekeepingStatus,
       boolean dryRunEnabled) {
-    String status = String.valueOf(housekeepingStatus);
     if (dryRunEnabled) {
-      status = "DRY_RUN_" + housekeepingStatus;
+      return;
     }
-    historyService.saveHistory(metadata, status);
+    historyService.saveHistory(metadata, housekeepingStatus);
   }
 }
diff --git a/.../java/com/expediagroup/beekeeper/metadata/cleanup/handler/ExpiredMetadataHandlerTest.java b/.../java/com/expediagroup/beekeeper/metadata/cleanup/handler/ExpiredMetadataHandlerTest.java
@@ -123,7 +123,7 @@ public void typicalRunDroppingTable() {
     verify(housekeepingMetadata).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(DELETED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED));
   }
 
   @Test
@@ -143,7 +143,7 @@ public void typicalDroppingTable_DryRun() {
     verify(housekeepingMetadata, never()).setCleanupAttempts(1);
     verify(housekeepingMetadata, never()).setHousekeepingStatus(DELETED);
     verify(housekeepingMetadataRepository, never()).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq("DRY_RUN_DELETED"));
+    verify(beekeeperHistoryService, never()).saveHistory(any(), any());
   }
 
   @Test
@@ -164,7 +164,7 @@ public void typicalRunDroppingPartition() {
     verify(housekeepingMetadata).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(DELETED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED));
   }
 
   @Test
@@ -185,7 +185,7 @@ public void dontDropTableWithInvalidPath() {
     verify(housekeepingMetadata, never()).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(SKIPPED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(SKIPPED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(SKIPPED));
   }
 
   @Test
@@ -204,7 +204,7 @@ public void dontDropTableWithInvalidPath_DryRun() {
     verify(housekeepingMetadata, never()).setCleanupAttempts(1);
     verify(housekeepingMetadata, never()).setHousekeepingStatus(SKIPPED);
     verify(housekeepingMetadataRepository, never()).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq("DRY_RUN_SKIPPED"));
+    verify(beekeeperHistoryService, never()).saveHistory(any(), any());
   }
 
   @Test
@@ -247,7 +247,7 @@ public void dontDropTableOrPathWhenTableDoesntExist() {
     verify(housekeepingMetadata).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(DELETED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED));
   }
 
   @Test
@@ -263,7 +263,7 @@ public void dontDropPartitionWithInvalidPartitionPath() {
     verify(housekeepingMetadata, never()).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(SKIPPED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(SKIPPED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(SKIPPED));
   }
 
   @Test
@@ -283,7 +283,7 @@ public void dontDropPartitionWhenTableDoesntExist() {
     verify(housekeepingMetadata).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(DELETED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED));
   }
 
   @Test
@@ -302,7 +302,7 @@ public void dontDropPathWhenPartitionDoesntExist() {
     verify(housekeepingMetadata).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(DELETED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED));
   }
 
   @Test
@@ -324,7 +324,7 @@ public void expectedTableDropFailure() {
     verify(housekeepingMetadata).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(FAILED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_DELETE.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_DELETE));
   }
 
   @Test
@@ -342,7 +342,7 @@ public void expectedPathDeleteFailure() {
     verify(housekeepingMetadata).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(FAILED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_DELETE.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_DELETE));
   }
 
   @Test
@@ -360,6 +360,6 @@ public void expectedPartitionDropFailure() {
     verify(housekeepingMetadata).setCleanupAttempts(1);
     verify(housekeepingMetadata).setHousekeepingStatus(FAILED);
     verify(housekeepingMetadataRepository).save(housekeepingMetadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_DELETE.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_DELETE));
   }
 }
diff --git a/...nup/src/main/java/com/expediagroup/beekeeper/path/cleanup/handler/GenericPathHandler.java b/...nup/src/main/java/com/expediagroup/beekeeper/path/cleanup/handler/GenericPathHandler.java
@@ -112,6 +112,6 @@ private void updateStatus(HousekeepingPath housekeepingPath, HousekeepingStatus
   }
 
   private void saveHistory(HousekeepingPath housekeepingPath, HousekeepingStatus status) {
-    beekeeperHistoryService.saveHistory(housekeepingPath, status.name());
+    beekeeperHistoryService.saveHistory(housekeepingPath, status);
   }
 }
diff --git a/...src/test/java/com/expediagroup/beekeeper/path/cleanup/handler/GenericPathHandlerTest.java b/...src/test/java/com/expediagroup/beekeeper/path/cleanup/handler/GenericPathHandlerTest.java
@@ -89,7 +89,7 @@ public void typicalProcessPage() {
     verify(mockPath).setCleanupAttempts(1);
     verify(mockPath).setHousekeepingStatus(DELETED);
     verify(housekeepingPathRepository).save(mockPath);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(DELETED));
     assertThat(pageable).isEqualTo(pageable);
   }
 
@@ -103,7 +103,7 @@ public void processPageFails() {
     verify(mockPath).setCleanupAttempts(1);
     verify(mockPath).setHousekeepingStatus(FAILED);
     verify(housekeepingPathRepository).save(mockPath);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_DELETE.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_DELETE));
     assertThat(pageable).isEqualTo(pageable);
   }
 
@@ -117,7 +117,7 @@ public void processPageInvalidPath() {
     verify(mockPath, never()).setCleanupAttempts(1);
     verify(mockPath).setHousekeepingStatus(SKIPPED);
     verify(housekeepingPathRepository).save(mockPath);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(SKIPPED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(SKIPPED));
     assertThat(pageable).isEqualTo(pageable);
   }
 }
diff --git a/.../migration/V3_1__Create_history_table.sql → .../migration/V2_4__Create_history_table.sql b/.../migration/V3_1__Create_history_table.sql → .../migration/V2_4__Create_history_table.sql
diff --git a/...expediagroup/beekeeper/scheduler/service/ExpiredHousekeepingMetadataSchedulerService.java b/...expediagroup/beekeeper/scheduler/service/ExpiredHousekeepingMetadataSchedulerService.java
@@ -151,6 +151,6 @@ private boolean isPartitionedTable(HousekeepingMetadata housekeepingMetadata) {
   }
 
   private void saveHistory(HousekeepingMetadata housekeepingMetadata, HousekeepingStatus status) {
-    beekeeperHistoryService.saveHistory(housekeepingMetadata, status.name());
+    beekeeperHistoryService.saveHistory(housekeepingMetadata, status);
   }
 }
diff --git a/...xpediagroup/beekeeper/scheduler/service/UnreferencedHousekeepingPathSchedulerService.java b/...xpediagroup/beekeeper/scheduler/service/UnreferencedHousekeepingPathSchedulerService.java
@@ -71,6 +71,6 @@ public void scheduleForHousekeeping(HousekeepingEntity housekeepingEntity) {
   }
 
   private void saveHistory(HousekeepingPath housekeepingPath, HousekeepingStatus status) {
-    beekeeperHistoryService.saveHistory(housekeepingPath, status.name());
+    beekeeperHistoryService.saveHistory(housekeepingPath, status);
   }
 }
diff --git a/...diagroup/beekeeper/scheduler/service/ExpiredHousekeepingMetadataSchedulerServiceTest.java b/...diagroup/beekeeper/scheduler/service/ExpiredHousekeepingMetadataSchedulerServiceTest.java
@@ -74,7 +74,7 @@ public void typicalCreateScheduleForHousekeeping() {
     expiredHousekeepingMetadataSchedulerService.scheduleForHousekeeping(metadata);
 
     verify(housekeepingMetadataRepository).save(metadata);
-    verify(beekeeperHistoryService).saveHistory(metadata, SCHEDULED.name());
+    verify(beekeeperHistoryService).saveHistory(metadata, SCHEDULED);
   }
 
   @Test
@@ -91,7 +91,7 @@ public void typicalCreatePartitionScheduleForHousekeeping() {
     expiredHousekeepingMetadataSchedulerService.scheduleForHousekeeping(metadata);
 
     verify(housekeepingMetadataRepository).save(metadata);
-    verify(beekeeperHistoryService).saveHistory(metadata, SCHEDULED.name());
+    verify(beekeeperHistoryService).saveHistory(metadata, SCHEDULED);
   }
 
   @Test
@@ -110,7 +110,7 @@ public void typicalUpdateScheduleForHousekeepingWhenChangingCleanupDelay() {
     verify(existingTable).setClientId(metadata.getClientId());
     verify(existingTable).setCleanupDelay(metadata.getCleanupDelay());
     verify(housekeepingMetadataRepository).save(existingTable);
-    verify(beekeeperHistoryService).saveHistory(metadata, SCHEDULED.name());
+    verify(beekeeperHistoryService).saveHistory(metadata, SCHEDULED);
   }
 
   @Test
@@ -136,7 +136,7 @@ public void typicalUpdatePartitionedTableWithShorterCleanupDelay() {
     verify(existingTable).setCleanupTimestamp(CREATION_TIMESTAMP.plus(Duration.parse("P30D")));
 
     verify(housekeepingMetadataRepository).save(existingTable);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(SCHEDULED.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(SCHEDULED));
   }
 
   @Test
@@ -154,7 +154,7 @@ public void scheduleFails() {
         .isThrownBy(() -> expiredHousekeepingMetadataSchedulerService.scheduleForHousekeeping(metadata))
         .withMessage(format("Unable to schedule %s", metadata));
     verify(housekeepingMetadataRepository).save(metadata);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_SCHEDULE.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_SCHEDULE));
   }
 
   private HousekeepingMetadata createHousekeepingMetadataPartition() {

diff --git a/...iagroup/beekeeper/scheduler/service/UnreferencedHousekeepingPathSchedulerServiceTest.java b/...iagroup/beekeeper/scheduler/service/UnreferencedHousekeepingPathSchedulerServiceTest.java
@@ -65,7 +65,7 @@ public void typicalScheduleForHousekeeping() {
 
     verify(housekeepingPathRepository).save(path);
 
-    verify(beekeeperHistoryService).saveHistory(path, SCHEDULED.name());
+    verify(beekeeperHistoryService).saveHistory(path, SCHEDULED);
   }
 
   @Test
@@ -88,6 +88,6 @@ public void scheduleFails() {
         .isThrownBy(() -> unreferencedHousekeepingPathSchedulerService.scheduleForHousekeeping(path))
         .withMessage(format("Unable to schedule %s", path));
     verify(housekeepingPathRepository).save(path);
-    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_SCHEDULE.name()));
+    verify(beekeeperHistoryService).saveHistory(any(), eq(FAILED_TO_SCHEDULE));
   }
 }