Skip to content

Commit

Permalink
fix(snapshot): do not issue a Recording Deleted notification when emp…
Browse files Browse the repository at this point in the history 
…ty snapshots are deleted (#912)

* Only issue a Recording Deleted notification if the recording in question is not an unreadable (i.e. likely empty) snapshot

* Encapsulate decision to issue Recording Deletion notification inside private method
  • Loading branch information
Hareet Dhillon authored Apr 27, 2022
1 parent 3037175 commit 00fc838
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 58 deletions.
115 changes: 62 additions & 53 deletions src/main/java/io/cryostat/recordings/RecordingTargetHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,58 +253,7 @@ public Future<Optional<InputStream>> getRecording(

public Future<Void> deleteRecording(
ConnectionDescriptor connectionDescriptor, String recordingName) {
CompletableFuture<Void> future = new CompletableFuture<>();
try {
String targetId = connectionDescriptor.getTargetId();
Void v =
targetConnectionManager.executeConnectedTask(
connectionDescriptor,
connection -> {
Optional<IRecordingDescriptor> descriptor =
getDescriptorByName(connection, recordingName);
if (descriptor.isPresent()) {
IRecordingDescriptor d = descriptor.get();
connection.getService().close(d);
reportService.delete(connectionDescriptor, recordingName);
this.cancelScheduledNotificationIfExists(
targetId, recordingName);
HyperlinkedSerializableRecordingDescriptor linkedDesc =
new HyperlinkedSerializableRecordingDescriptor(
d,
webServer
.get()
.getDownloadURL(
connection, d.getName()),
webServer
.get()
.getReportURL(connection, d.getName()),
recordingMetadataManager.getMetadata(
connectionDescriptor.getTargetId(),
recordingName));
recordingMetadataManager.deleteRecordingMetadataIfExists(
connectionDescriptor.getTargetId(), recordingName);
notificationFactory
.createBuilder()
.metaCategory(DELETION_NOTIFICATION_CATEGORY)
.metaType(HttpMimeType.JSON)
.message(
Map.of(
"recording",
linkedDesc,
"target",
connectionDescriptor.getTargetId()))
.build()
.send();
} else {
throw new RecordingNotFoundException(targetId, recordingName);
}
return null;
});
future.complete(v);
} catch (Exception e) {
future.completeExceptionally(e);
}
return future;
return this.deleteRecording(connectionDescriptor, recordingName, true);
}

public IRecordingDescriptor stopRecording(
Expand Down Expand Up @@ -408,7 +357,7 @@ public Future<Boolean> verifySnapshot(
} else {
try (InputStream snapshot = snapshotOptional.get()) {
if (!snapshotIsReadable(connectionDescriptor, snapshot)) {
this.deleteRecording(connectionDescriptor, snapshotName).get();
this.deleteRecording(connectionDescriptor, snapshotName, false).get();
future.complete(false);
} else {
future.complete(true);
Expand Down Expand Up @@ -444,6 +393,66 @@ public Optional<IRecordingDescriptor> getDescriptorByName(
.findFirst();
}

private Future<Void> deleteRecording(
ConnectionDescriptor connectionDescriptor,
String recordingName,
boolean issueNotification) {
CompletableFuture<Void> future = new CompletableFuture<>();
try {
String targetId = connectionDescriptor.getTargetId();
Void v =
targetConnectionManager.executeConnectedTask(
connectionDescriptor,
connection -> {
Optional<IRecordingDescriptor> descriptor =
getDescriptorByName(connection, recordingName);
if (descriptor.isPresent()) {
IRecordingDescriptor d = descriptor.get();
connection.getService().close(d);
reportService.delete(connectionDescriptor, recordingName);
this.cancelScheduledNotificationIfExists(
targetId, recordingName);
HyperlinkedSerializableRecordingDescriptor linkedDesc =
new HyperlinkedSerializableRecordingDescriptor(
d,
webServer
.get()
.getDownloadURL(
connection, d.getName()),
webServer
.get()
.getReportURL(connection, d.getName()),
recordingMetadataManager.getMetadata(
connectionDescriptor.getTargetId(),
recordingName));
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();
}
} else {
throw new RecordingNotFoundException(targetId, recordingName);
}
return null;
});
future.complete(v);
} catch (Exception e) {
future.completeExceptionally(e);
}
return future;
}

private void notifyRecordingStopped(
String targetId, HyperlinkedSerializableRecordingDescriptor desc) {
notificationFactory
Expand Down
24 changes: 19 additions & 5 deletions src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ void shouldHandleVerificationWhenSnapshotNotReadable() throws Exception {
RecordingTargetHelper recordingTargetHelperSpy = Mockito.spy(recordingTargetHelper);
ConnectionDescriptor connectionDescriptor = new ConnectionDescriptor("fooTarget");
String snapshotName = "snapshot-1";

Future<Optional<InputStream>> getFuture = Mockito.mock(Future.class);
Mockito.doReturn(getFuture)
.when(recordingTargetHelperSpy)
Expand All @@ -455,11 +456,24 @@ void shouldHandleVerificationWhenSnapshotNotReadable() throws Exception {
.thenReturn(true);
Mockito.doThrow(IOException.class).when(snapshot).read();

Future<Void> deleteFuture = Mockito.mock(Future.class);
Mockito.doReturn(deleteFuture)
.when(recordingTargetHelperSpy)
.deleteRecording(connectionDescriptor, snapshotName);
Mockito.when(deleteFuture.get()).thenReturn(null);
Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any()))
.thenAnswer(
new Answer() {
@Override
public Object answer(InvocationOnMock invocation) throws Throwable {
TargetConnectionManager.ConnectedTask task =
(TargetConnectionManager.ConnectedTask)
invocation.getArgument(1);
return task.execute(connection);
}
});

Mockito.when(connection.getService()).thenReturn(service);
IRecordingDescriptor descriptor = createDescriptor(snapshotName);
Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor));

Mockito.when(recordingMetadataManager.getMetadata(Mockito.anyString(), Mockito.anyString()))
.thenReturn(new Metadata());

boolean verified =
recordingTargetHelperSpy.verifySnapshot(connectionDescriptor, snapshotName).get();
Expand Down

0 comments on commit 00fc838

Please sign in to comment.