fix(snapshot): Cleanup notification handling for snapshots

src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingDeleteHandler.java

@@ -40,6 +40,8 @@
 import java.util.EnumSet;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.inject.Inject;
 
@@ -59,6 +61,7 @@
 import org.apache.commons.lang3.exception.ExceptionUtils;
 
 class TargetRecordingDeleteHandler extends AbstractAuthenticatedRequestHandler {
+    private static final Pattern SNAPSHOT_NAME_PATTERN = Pattern.compile("^(snapshot\\-)([0-9]+)$");
 
     private final RecordingTargetHelper recordingTargetHelper;
 
@@ -102,7 +105,12 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
         String recordingName = ctx.pathParam("recordingName");
         ConnectionDescriptor connectionDescriptor = getConnectionDescriptorFromContext(ctx);
         try {
-            recordingTargetHelper.deleteRecording(connectionDescriptor, recordingName).get();
+            Matcher m = SNAPSHOT_NAME_PATTERN.matcher(recordingName);
+            if (m.matches()) {
+                recordingTargetHelper.deleteSnapshot(connectionDescriptor, recordingName).get();
+            } else {
+                recordingTargetHelper.deleteRecording(connectionDescriptor, recordingName).get();
+            }
             ctx.response().setStatusCode(200);
             ctx.response().end();
         } catch (ExecutionException e) {

src/main/java/io/cryostat/net/web/http/api/v1/TargetSnapshotPostHandler.java

@@ -113,7 +113,9 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
         boolean verificationSuccessful = false;
         try {
             verificationSuccessful =
-                    recordingTargetHelper.verifySnapshot(connectionDescriptor, snapshotName).get();
+                    recordingTargetHelper
+                            .verifySnapshot(connectionDescriptor, snapshotDescriptor)
+                            .get();
         } catch (ExecutionException e) {
             handleExecutionException(e);
         }

src/main/java/io/cryostat/net/web/http/api/v2/TargetSnapshotPostHandler.java

@@ -121,7 +121,9 @@ public IntermediateResponse<HyperlinkedSerializableRecordingDescriptor> handle(
         boolean verificationSuccessful = false;
         try {
             verificationSuccessful =
-                    recordingTargetHelper.verifySnapshot(connectionDescriptor, snapshotName).get();
+                    recordingTargetHelper
+                            .verifySnapshot(connectionDescriptor, snapshotDescriptor)
+                            .get();
         } catch (ExecutionException e) {
             handleExecutionException(e);
         }

src/main/java/io/cryostat/recordings/RecordingTargetHelper.java

@@ -81,9 +81,12 @@
 
 public class RecordingTargetHelper {
 
-    private static final String CREATE_NOTIFICATION_CATEGORY = "ActiveRecordingCreated";
+    private static final String CREATION_NOTIFICATION_CATEGORY = "ActiveRecordingCreated";
     private static final String STOP_NOTIFICATION_CATEGORY = "ActiveRecordingStopped";
     private static final String DELETION_NOTIFICATION_CATEGORY = "ActiveRecordingDeleted";
+    private static final String SNAPSHOT_CREATION_NOTIFICATION_CATEGORY = "SnapshotCreated";
+    private static final String SNAPSHOT_DELETION_NOTIFICATION_CATEGORY = "SnapshotDeleted";
+
     private static final long TIMESTAMP_DRIFT_SAFEGUARD = 3_000L;
 
     private static final Pattern TEMPLATE_PATTERN =
@@ -181,13 +184,7 @@ public IRecordingDescriptor startRecording(
                                     webServer.get().getDownloadURL(connection, desc.getName()),
                                     webServer.get().getReportURL(connection, desc.getName()),
                                     metadata);
-                    notificationFactory
-                            .createBuilder()
-                            .metaCategory(CREATE_NOTIFICATION_CATEGORY)
-                            .metaType(HttpMimeType.JSON)
-                            .message(Map.of("recording", linkedDesc, "target", targetId))
-                            .build()
-                            .send();
+                    this.issueNotification(targetId, linkedDesc, CREATION_NOTIFICATION_CATEGORY);
 
                     Object fixedDuration =
                             recordingOptions.get(RecordingOptionsBuilder.KEY_DURATION);
@@ -253,7 +250,12 @@ public Future<Optional<InputStream>> getRecording(
 
     public Future<Void> deleteRecording(
             ConnectionDescriptor connectionDescriptor, String recordingName) {
-        return this.deleteRecording(connectionDescriptor, recordingName, true);
+        return this.deleteRecording(connectionDescriptor, recordingName, false, true);
+    }
+
+    public Future<Void> deleteSnapshot(
+            ConnectionDescriptor connectionDescriptor, String recordingName) {
+        return this.deleteRecording(connectionDescriptor, recordingName, true, true);
     }
 
     public IRecordingDescriptor stopRecording(
@@ -284,7 +286,7 @@ public IRecordingDescriptor stopRecording(
                                         d,
                                         webServer.get().getDownloadURL(connection, d.getName()),
                                         webServer.get().getReportURL(connection, d.getName()));
-                        this.notifyRecordingStopped(targetId, linkedDesc);
+                        this.issueNotification(targetId, linkedDesc, STOP_NOTIFICATION_CATEGORY);
                         return getDescriptorByName(connection, recordingName).get();
                     } else {
                         throw new RecordingNotFoundException(targetId, recordingName);
@@ -346,9 +348,18 @@ public Future<HyperlinkedSerializableRecordingDescriptor> createSnapshot(
     }
 
     public Future<Boolean> verifySnapshot(
-            ConnectionDescriptor connectionDescriptor, String snapshotName) {
+            ConnectionDescriptor connectionDescriptor,
+            HyperlinkedSerializableRecordingDescriptor snapshotDescriptor) {
+        return this.verifySnapshot(connectionDescriptor, snapshotDescriptor, true);
+    }
+
+    public Future<Boolean> verifySnapshot(
+            ConnectionDescriptor connectionDescriptor,
+            HyperlinkedSerializableRecordingDescriptor snapshotDescriptor,
+            boolean issueNotification) {
         CompletableFuture<Boolean> future = new CompletableFuture<>();
         try {
+            String snapshotName = snapshotDescriptor.getName();
             Optional<InputStream> snapshotOptional =
                     this.getRecording(connectionDescriptor, snapshotName).get();
             if (snapshotOptional.isEmpty()) {
@@ -357,9 +368,15 @@ public Future<Boolean> verifySnapshot(
             } else {
                 try (InputStream snapshot = snapshotOptional.get()) {
                     if (!snapshotIsReadable(connectionDescriptor, snapshot)) {
-                        this.deleteRecording(connectionDescriptor, snapshotName, false).get();
+                        this.deleteRecording(connectionDescriptor, snapshotName, true, false).get();
                         future.complete(false);
                     } else {
+                        if (issueNotification) {
+                            this.issueNotification(
+                                    connectionDescriptor.getTargetId(),
+                                    snapshotDescriptor,
+                                    SNAPSHOT_CREATION_NOTIFICATION_CATEGORY);
+                        }
                         future.complete(true);
                     }
                 }
@@ -396,6 +413,7 @@ public Optional<IRecordingDescriptor> getDescriptorByName(
     private Future<Void> deleteRecording(
             ConnectionDescriptor connectionDescriptor,
             String recordingName,
+            boolean isSnapshot,
             boolean issueNotification) {
         CompletableFuture<Void> future = new CompletableFuture<>();
         try {
@@ -428,18 +446,17 @@ private Future<Void> deleteRecording(
                                     recordingMetadataManager.deleteRecordingMetadataIfExists(
                                             connectionDescriptor.getTargetId(), recordingName);
                                     if (issueNotification) {
-                                        notificationFactory
-                                                .createBuilder()
-                                                .metaCategory(DELETION_NOTIFICATION_CATEGORY)
-                                                .metaType(HttpMimeType.JSON)
-                                                .message(
-                                                        Map.of(
-                                                                "recording",
-                                                                linkedDesc,
-                                                                "target",
-                                                                connectionDescriptor.getTargetId()))
-                                                .build()
-                                                .send();
+                                        if (isSnapshot) {
+                                            this.issueNotification(
+                                                    connectionDescriptor.getTargetId(),
+                                                    linkedDesc,
+                                                    SNAPSHOT_DELETION_NOTIFICATION_CATEGORY);
+                                        } else {
+                                            this.issueNotification(
+                                                    connectionDescriptor.getTargetId(),
+                                                    linkedDesc,
+                                                    DELETION_NOTIFICATION_CATEGORY);
+                                        }
                                     }
                                 } else {
                                     throw new RecordingNotFoundException(targetId, recordingName);
@@ -453,13 +470,15 @@ private Future<Void> deleteRecording(
         return future;
     }
 
-    private void notifyRecordingStopped(
-            String targetId, HyperlinkedSerializableRecordingDescriptor desc) {
+    private void issueNotification(
+            String targetId,
+            HyperlinkedSerializableRecordingDescriptor linkedDesc,
+            String notificationCategory) {
         notificationFactory
                 .createBuilder()
-                .metaCategory(STOP_NOTIFICATION_CATEGORY)
+                .metaCategory(notificationCategory)
                 .metaType(HttpMimeType.JSON)
-                .message(Map.of("recording", desc, "target", targetId))
+                .message(Map.of("recording", linkedDesc, "target", targetId))
                 .build()
                 .send();
     }
@@ -560,7 +579,10 @@ private void scheduleRecordingStopNotification(
                                                                     .get()
                                                                     .getReportURL(
                                                                             connection, name));
-                                            this.notifyRecordingStopped(targetId, linkedDesc);
+                                            this.issueNotification(
+                                                    targetId,
+                                                    linkedDesc,
+                                                    STOP_NOTIFICATION_CATEGORY);
                                         }
 
                                         return desc;

src/test/java/io/cryostat/net/web/http/api/v1/TargetSnapshotPostHandlerTest.java

@@ -114,7 +114,8 @@ void shouldCreateSnapshot() throws Exception {
         CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
         Mockito.when(
                         recordingTargetHelper.verifySnapshot(
-                                Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
+                                Mockito.any(ConnectionDescriptor.class),
+                                Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
                 .thenReturn(future2);
         Mockito.when(future2.get()).thenReturn(true);
 
@@ -177,7 +178,8 @@ void shouldHandleSnapshotCreationExceptionDuringVerification() throws Exception
         CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
         Mockito.when(
                         recordingTargetHelper.verifySnapshot(
-                                Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
+                                Mockito.any(ConnectionDescriptor.class),
+                                Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
                 .thenReturn(future2);
         Mockito.when(future2.get())
                 .thenThrow(
@@ -215,7 +217,8 @@ void shouldHandleFailedSnapshotVerification() throws Exception {
         CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
         Mockito.when(
                         recordingTargetHelper.verifySnapshot(
-                                Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
+                                Mockito.any(ConnectionDescriptor.class),
+                                Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
                 .thenReturn(future2);
         Mockito.when(future2.get()).thenReturn(false);
 

src/test/java/io/cryostat/net/web/http/api/v2/TargetSnapshotPostHandlerTest.java

@@ -124,7 +124,8 @@ void shouldCreateSnapshot() throws Exception {
         CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
         Mockito.when(
                         recordingTargetHelper.verifySnapshot(
-                                Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
+                                Mockito.any(ConnectionDescriptor.class),
+                                Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
                 .thenReturn(future2);
         Mockito.when(future2.get()).thenReturn(true);
 
@@ -214,7 +215,8 @@ void shouldHandleSnapshotCreationExceptionDuringVerification() throws Exception
         CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
         Mockito.when(
                         recordingTargetHelper.verifySnapshot(
-                                Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
+                                Mockito.any(ConnectionDescriptor.class),
+                                Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
                 .thenReturn(future2);
         Mockito.when(future2.get())
                 .thenThrow(
@@ -254,7 +256,8 @@ void shouldHandleFailedSnapshotVerification() throws Exception {
         CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
         Mockito.when(
                         recordingTargetHelper.verifySnapshot(
-                                Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
+                                Mockito.any(ConnectionDescriptor.class),
+                                Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
                 .thenReturn(future2);
         Mockito.when(future2.get()).thenReturn(false);
 

src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java

@@ -400,8 +400,17 @@ void shouldVerifySnapshot() throws Exception {
         Mockito.when(targetConnectionManager.markConnectionInUse(connectionDescriptor))
                 .thenReturn(true);
 
+        IRecordingDescriptor minimalDescriptor = createDescriptor(snapshotName);
+        HyperlinkedSerializableRecordingDescriptor snapshotDescriptor =
+                new HyperlinkedSerializableRecordingDescriptor(
+                        minimalDescriptor,
+                        "http://example.com/download",
+                        "http://example.com/report");
+
         boolean verified =
-                recordingTargetHelperSpy.verifySnapshot(connectionDescriptor, snapshotName).get();
+                recordingTargetHelperSpy
+                        .verifySnapshot(connectionDescriptor, snapshotDescriptor)
+                        .get();
 
         Assertions.assertTrue(verified);
     }
@@ -419,12 +428,19 @@ void shouldThrowSnapshotCreationExceptionWhenVerificationFails() throws Exceptio
         Optional<InputStream> snapshotOptional = Optional.empty();
         Mockito.when(future.get()).thenReturn(snapshotOptional);
 
+        IRecordingDescriptor minimalDescriptor = createDescriptor(snapshotName);
+        HyperlinkedSerializableRecordingDescriptor snapshotDescriptor =
+                new HyperlinkedSerializableRecordingDescriptor(
+                        minimalDescriptor,
+                        "http://example.com/download",
+                        "http://example.com/report");
+
         Assertions.assertThrows(
                 ExecutionException.class,
                 () -> {
                     try {
                         recordingTargetHelperSpy
-                                .verifySnapshot(connectionDescriptor, snapshotName)
+                                .verifySnapshot(connectionDescriptor, snapshotDescriptor)
                                 .get();
                     } catch (ExecutionException ee) {
                         Assertions.assertTrue(ee.getCause() instanceof SnapshotCreationException);
@@ -475,8 +491,17 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
         Mockito.when(recordingMetadataManager.getMetadata(Mockito.anyString(), Mockito.anyString()))
                 .thenReturn(new Metadata());
 
+        IRecordingDescriptor minimalDescriptor = createDescriptor(snapshotName);
+        HyperlinkedSerializableRecordingDescriptor snapshotDescriptor =
+                new HyperlinkedSerializableRecordingDescriptor(
+                        minimalDescriptor,
+                        "http://example.com/download",
+                        "http://example.com/report");
+
         boolean verified =
-                recordingTargetHelperSpy.verifySnapshot(connectionDescriptor, snapshotName).get();
+                recordingTargetHelperSpy
+                        .verifySnapshot(connectionDescriptor, snapshotDescriptor)
+                        .get();
 
         Assertions.assertFalse(verified);
     }