From b611dcb418c8650ada7790363505196c69d06ba6 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Fri, 29 Sep 2023 15:26:28 -0400 Subject: [PATCH 1/8] include jvmId in issueNotifications for recordings --- .../recordings/RecordingTargetHelper.java | 24 +++++++++++++------ .../cryostat/recordings/RecordingsModule.java | 2 ++ .../recordings/RecordingTargetHelperTest.java | 3 +++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/cryostat/recordings/RecordingTargetHelper.java b/src/main/java/io/cryostat/recordings/RecordingTargetHelper.java index 949f75e728..8582396757 100644 --- a/src/main/java/io/cryostat/recordings/RecordingTargetHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingTargetHelper.java @@ -45,6 +45,8 @@ import io.cryostat.net.reports.ReportService; import io.cryostat.net.web.WebServer; import io.cryostat.net.web.http.HttpMimeType; +import io.cryostat.recordings.JvmIdHelper; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import io.cryostat.recordings.RecordingMetadataManager.Metadata; import dagger.Lazy; @@ -72,6 +74,7 @@ public class RecordingTargetHelper { private final Vertx vertx; private final TargetConnectionManager targetConnectionManager; private final Lazy webServer; + private final Lazy jvmIdHelper; private final EventOptionsBuilder.Factory eventOptionsBuilderFactory; private final NotificationFactory notificationFactory; private final RecordingOptionsBuilderFactory recordingOptionsBuilderFactory; @@ -85,6 +88,7 @@ public class RecordingTargetHelper { Vertx vertx, TargetConnectionManager targetConnectionManager, Lazy webServer, + Lazy jvmIdHelper, EventOptionsBuilder.Factory eventOptionsBuilderFactory, NotificationFactory notificationFactory, RecordingOptionsBuilderFactory recordingOptionsBuilderFactory, @@ -95,6 +99,7 @@ public class RecordingTargetHelper { this.vertx = vertx; this.targetConnectionManager = targetConnectionManager; this.webServer = webServer; + this.jvmIdHelper = jvmIdHelper; this.eventOptionsBuilderFactory = eventOptionsBuilderFactory; this.notificationFactory = notificationFactory; this.recordingOptionsBuilderFactory = recordingOptionsBuilderFactory; @@ -467,13 +472,18 @@ private void issueNotification( String targetId, HyperlinkedSerializableRecordingDescriptor linkedDesc, String notificationCategory) { - notificationFactory - .createBuilder() - .metaCategory(notificationCategory) - .metaType(HttpMimeType.JSON) - .message(Map.of("recording", linkedDesc, "target", targetId)) - .build() - .send(); + try { + notificationFactory + .createBuilder() + .metaCategory(notificationCategory) + .metaType(HttpMimeType.JSON) + .message(Map.of("recording", linkedDesc, "target", targetId, "jvmId", jvmIdHelper.get().getJvmId(targetId))) + .build() + .send(); + } catch (JvmIdGetException e) { + logger.info("Retain null jvmId for target [{}]", targetId); + logger.info(e); + } } private void cancelScheduledTasksIfExists(String targetId, String stoppedRecordingName) diff --git a/src/main/java/io/cryostat/recordings/RecordingsModule.java b/src/main/java/io/cryostat/recordings/RecordingsModule.java index 1ab3e655ea..210b41129a 100644 --- a/src/main/java/io/cryostat/recordings/RecordingsModule.java +++ b/src/main/java/io/cryostat/recordings/RecordingsModule.java @@ -79,6 +79,7 @@ static RecordingTargetHelper provideRecordingTargetHelper( Vertx vertx, TargetConnectionManager targetConnectionManager, Lazy webServer, + Lazy jvmIdHelper, EventOptionsBuilder.Factory eventOptionsBuilderFactory, NotificationFactory notificationFactory, RecordingOptionsBuilderFactory recordingOptionsBuilderFactory, @@ -90,6 +91,7 @@ static RecordingTargetHelper provideRecordingTargetHelper( vertx, targetConnectionManager, webServer, + jvmIdHelper, eventOptionsBuilderFactory, notificationFactory, recordingOptionsBuilderFactory, diff --git a/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java b/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java index ee6c77f426..124b3883a7 100644 --- a/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java +++ b/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java @@ -53,6 +53,7 @@ import io.cryostat.net.reports.ReportService; import io.cryostat.net.web.WebServer; import io.cryostat.net.web.http.HttpMimeType; +import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.RecordingMetadataManager.Metadata; import io.cryostat.recordings.RecordingTargetHelper.ReplacementPolicy; import io.cryostat.recordings.RecordingTargetHelper.SnapshotCreationException; @@ -80,6 +81,7 @@ public class RecordingTargetHelperTest { @Mock AuthManager auth; @Mock TargetConnectionManager targetConnectionManager; @Mock WebServer webServer; + @Mock JvmIdHelper jvmIdHelper; @Mock EventOptionsBuilder.Factory eventOptionsBuilderFactory; @Mock NotificationFactory notificationFactory; @Mock RecordingOptionsBuilderFactory recordingOptionsBuilderFactory; @@ -113,6 +115,7 @@ void setup() { vertx, targetConnectionManager, () -> webServer, + () -> jvmIdHelper, eventOptionsBuilderFactory, notificationFactory, recordingOptionsBuilderFactory, From ef707e729da09572eb7989b309989153b29d668c Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Mon, 2 Oct 2023 12:35:20 -0400 Subject: [PATCH 2/8] fix recording tests --- .../recordings/RecordingTargetHelperTest.java | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java b/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java index 124b3883a7..d625903332 100644 --- a/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java +++ b/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java @@ -110,6 +110,16 @@ void setup() { lenient().when(notificationBuilder.message(Mockito.any())).thenReturn(notificationBuilder); lenient().when(notificationBuilder.build()).thenReturn(notification); lenient().when(vertx.setTimer(Mockito.anyLong(), Mockito.any())).thenReturn(1234L); + lenient() + .when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) + .thenAnswer( + new Answer() { + @Override + public String answer(InvocationOnMock invocation) throws Throwable { + return invocation.getArgument(0); + } + }); + this.recordingTargetHelper = new RecordingTargetHelper( vertx, @@ -199,6 +209,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } }); + Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor(recordingName); Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor)); @@ -227,7 +238,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingDeleted"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); Mockito.verify(notificationBuilder) - .message(Map.of("recording", linkedDesc, "target", "fooTarget")); + .message(Map.of("recording", linkedDesc, "target", "fooTarget", "jvmId", "id")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } @@ -249,6 +260,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } }); + Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor(recordingName); Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor)); @@ -277,7 +289,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.verify(notificationBuilder).metaCategory("SnapshotDeleted"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); Mockito.verify(notificationBuilder) - .message(Map.of("recording", linkedDesc, "target", "fooTarget")); + .message(Map.of("recording", linkedDesc, "target", "fooTarget", "jvmId", "id")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } @@ -314,6 +326,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } }); + Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor(recordingName); Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor)); @@ -351,6 +364,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } }); + Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor(recordingName); Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor)); @@ -517,6 +531,7 @@ void shouldVerifySnapshotWithNotification() throws Exception { new Random(123456).nextBytes(src); InputStream snapshot = new ByteArrayInputStream(src); Mockito.when(snapshotOptional.get()).thenReturn(snapshot); + Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(targetConnectionManager.markConnectionInUse(connectionDescriptor)) .thenReturn(true); @@ -537,7 +552,7 @@ void shouldVerifySnapshotWithNotification() throws Exception { Mockito.verify(notificationBuilder).metaCategory("SnapshotCreated"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); Mockito.verify(notificationBuilder) - .message(Map.of("recording", snapshotDescriptor, "target", "fooTarget")); + .message(Map.of("recording", snapshotDescriptor, "target", "fooTarget", "jvmId", "id")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); @@ -704,6 +719,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.when(recordingOptions.get(Mockito.any())).thenReturn(recordingName, duration); + Mockito.when(jvmIdHelper.getJvmId(targetId)).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); Mockito.when(service.getAvailableRecordings()) .thenReturn(Collections.emptyList(), List.of(recordingDescriptor)); @@ -856,6 +872,7 @@ void shouldCloseAndRecreateIfRecordingExistsAndIsRunning() throws Exception { Metadata metadata = new Metadata(Map.of("template.name", "Profiling", "template.type", "TARGET")); + Mockito.when(jvmIdHelper.getJvmId(targetId)).thenReturn("id"); Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any())) .thenAnswer( invocation -> { @@ -910,6 +927,7 @@ void shouldRestartRecordingWhenRecordingExistsAndIsStopped() throws Exception { Metadata metadata = new Metadata(Map.of("template.name", "Profiling", "template.type", "TARGET")); + Mockito.when(jvmIdHelper.getJvmId(targetId)).thenReturn("id"); Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any())) .thenAnswer( invocation -> { @@ -966,6 +984,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { return task.execute(connection); } }); + Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor("someRecording"); Mockito.when(descriptor.getName()).thenReturn("someRecording"); @@ -984,7 +1003,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingStopped"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); Mockito.verify(notificationBuilder) - .message(Map.of("recording", linkedDesc, "target", "fooTarget")); + .message(Map.of("recording", linkedDesc, "target", "fooTarget", "jvmId", "id")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } From e5ce1b284045118d59dd9223b3ab199a0f8e9d74 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Tue, 3 Oct 2023 12:45:56 -0400 Subject: [PATCH 3/8] add JvmId to notifications --- .../api/beta/RecordingsFromIdPostHandler.java | 11 ++++- .../http/api/v2/TargetProbePostHandler.java | 40 ++++++++++++------- .../recordings/RecordingArchiveHelper.java | 8 ++-- .../recordings/RecordingMetadataManager.java | 4 ++ 4 files changed, 44 insertions(+), 19 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandler.java index ba3f21eef0..bbfbf226b7 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandler.java @@ -47,6 +47,7 @@ import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.JvmIdHelper.JvmIdDoesNotExistException; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingMetadataManager; import io.cryostat.recordings.RecordingMetadataManager.Metadata; @@ -69,6 +70,7 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan private final FileSystem fs; private final JvmIdHelper idHelper; private final NotificationFactory notificationFactory; + private final JvmIdHelper jvmIdHelper; private final RecordingArchiveHelper recordingArchiveHelper; private final RecordingMetadataManager recordingMetadataManager; private final Path savedRecordingsPath; @@ -85,6 +87,7 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan FileSystem fs, JvmIdHelper idHelper, NotificationFactory notificationFactory, + JvmIdHelper jvmIdHelper, RecordingArchiveHelper recordingArchiveHelper, RecordingMetadataManager recordingMetadataManager, @Named(MainModule.RECORDINGS_PATH) Path savedRecordingsPath, @@ -95,6 +98,7 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan this.fs = fs; this.idHelper = idHelper; this.notificationFactory = notificationFactory; + this.jvmIdHelper = jvmIdHelper; this.recordingArchiveHelper = recordingArchiveHelper; this.recordingMetadataManager = recordingMetadataManager; this.savedRecordingsPath = savedRecordingsPath; @@ -297,12 +301,15 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { size, archivedTime), "target", - connectUrl)) + connectUrl, + "jvmId", + jvmIdHelper.getJvmId(connectUrl))) .build() .send(); } catch (URISyntaxException | UnknownHostException - | SocketException e) { + | SocketException + | JvmIdGetException e) { logger.error(e); throw new ApiException(500, e); } diff --git a/src/main/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandler.java b/src/main/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandler.java index 9caacbca60..50c6bd0b7f 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandler.java @@ -38,6 +38,8 @@ import io.cryostat.net.security.ResourceAction; import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.net.web.http.api.ApiVersion; +import io.cryostat.recordings.JvmIdHelper; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import com.google.gson.Gson; import io.vertx.core.http.HttpMethod; @@ -76,6 +78,7 @@ class TargetProbePostHandler extends AbstractV2RequestHandler { private final NotificationFactory notificationFactory; private final LocalProbeTemplateService probeTemplateService; private final FileSystem fs; + private final JvmIdHelper jvmIdHelper; private final TargetConnectionManager connectionManager; private final Environment env; private static final String NOTIFICATION_CATEGORY = "ProbeTemplateApplied"; @@ -88,6 +91,7 @@ class TargetProbePostHandler extends AbstractV2RequestHandler { FileSystem fs, AuthManager auth, CredentialsManager credentialsManager, + JvmIdHelper jvmIdHelper, TargetConnectionManager connectionManager, Environment env, Gson gson) { @@ -95,6 +99,7 @@ class TargetProbePostHandler extends AbstractV2RequestHandler { this.logger = logger; this.notificationFactory = notificationFactory; this.probeTemplateService = service; + this.jvmIdHelper = jvmIdHelper; this.connectionManager = connectionManager; this.env = env; this.fs = fs; @@ -151,20 +156,27 @@ public IntermediateResponse handle(RequestParameters requestParams) throws new ByteArrayInputStream( templateContent.getBytes(StandardCharsets.UTF_8))); List events = Arrays.asList(template.getEvents()); - notificationFactory - .createBuilder() - .metaCategory(NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message( - Map.of( - "targetId", - targetId, - "probeTemplate", - probeTemplate, - "events", - events)) - .build() - .send(); + try { + notificationFactory + .createBuilder() + .metaCategory(NOTIFICATION_CATEGORY) + .metaType(HttpMimeType.JSON) + .message( + Map.of( + "targetId", + targetId, + "probeTemplate", + probeTemplate, + "events", + events, + "jvmId", + jvmIdHelper.getJvmId(targetId))) + .build() + .send(); + } catch (JvmIdGetException e) { + logger.info("Retain null jvmId for target [{}]", targetId); + logger.info(e); + } return new IntermediateResponse().body(null); }); } diff --git a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java index 2d0b1f88fb..c0719c2e1c 100644 --- a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java @@ -414,7 +414,9 @@ public Future saveRecording( "recording", archivedRecordingInfo, "target", - connectionDescriptor.getTargetId())) + connectionDescriptor.getTargetId(), + "jvmId", + jvmIdHelper.getJvmId(connectionDescriptor))) .build() .send(); } catch (Exception e) { @@ -454,7 +456,7 @@ public Future deleteRecordingFromPath( .createBuilder() .metaCategory(DELETE_NOTIFICATION_CATEGORY) .metaType(HttpMimeType.JSON) - .message(Map.of("recording", archivedRecordingInfo, "target", targetId)) + .message(Map.of("recording", archivedRecordingInfo, "target", targetId, "jvmId", jvmIdHelper.getJvmId(targetId))) .build() .send(); fs.deleteIfExists(recordingPath); @@ -518,7 +520,7 @@ private CompletableFuture handleDeleteRecordingRequest( .createBuilder() .metaCategory(DELETE_NOTIFICATION_CATEGORY) .metaType(HttpMimeType.JSON) - .message(Map.of("recording", archivedRecordingInfo, "target", targetId)) + .message(Map.of("recording", archivedRecordingInfo, "target", targetId, "jvmId", jvmIdHelper.getJvmId(targetId))) .build() .send(); checkEmptySubdirectory(parentPath); diff --git a/src/main/java/io/cryostat/recordings/RecordingMetadataManager.java b/src/main/java/io/cryostat/recordings/RecordingMetadataManager.java index db88573327..baf35e6b15 100644 --- a/src/main/java/io/cryostat/recordings/RecordingMetadataManager.java +++ b/src/main/java/io/cryostat/recordings/RecordingMetadataManager.java @@ -523,6 +523,8 @@ public Future setRecordingMetadataFromPath( recordingName, "target", connectUrl, + "jvmId", + jvmIdHelper.getJvmId(connectUrl), "metadata", metadata)) .build() @@ -566,6 +568,8 @@ public Future setRecordingMetadata( recordingName, "target", connectionDescriptor.getTargetId(), + "jvmId", + jvmIdHelper.getJvmId(connectionDescriptor), "metadata", metadata)) .build() From ded46c046c9eb42b0063b0ddc66cadf3ce717868 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Tue, 3 Oct 2023 14:57:26 -0400 Subject: [PATCH 4/8] send jvmId through notifications to -web --- .../api/beta/RecordingsFromIdPostHandler.java | 8 ++------ .../recordings/RecordingArchiveHelper.java | 2 +- .../beta/RecordingsFromIdPostHandlerTest.java | 4 ++-- .../http/api/v2/TargetProbePostHandlerTest.java | 10 +++++++++- .../recordings/RecordingArchiveHelperTest.java | 16 +++++++++------- .../recordings/RecordingMetadataManagerTest.java | 9 +++++++++ .../recordings/RecordingTargetHelperTest.java | 9 --------- 7 files changed, 32 insertions(+), 26 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandler.java index bbfbf226b7..e6e76611a8 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandler.java @@ -70,7 +70,6 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan private final FileSystem fs; private final JvmIdHelper idHelper; private final NotificationFactory notificationFactory; - private final JvmIdHelper jvmIdHelper; private final RecordingArchiveHelper recordingArchiveHelper; private final RecordingMetadataManager recordingMetadataManager; private final Path savedRecordingsPath; @@ -87,7 +86,6 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan FileSystem fs, JvmIdHelper idHelper, NotificationFactory notificationFactory, - JvmIdHelper jvmIdHelper, RecordingArchiveHelper recordingArchiveHelper, RecordingMetadataManager recordingMetadataManager, @Named(MainModule.RECORDINGS_PATH) Path savedRecordingsPath, @@ -98,7 +96,6 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan this.fs = fs; this.idHelper = idHelper; this.notificationFactory = notificationFactory; - this.jvmIdHelper = jvmIdHelper; this.recordingArchiveHelper = recordingArchiveHelper; this.recordingMetadataManager = recordingMetadataManager; this.savedRecordingsPath = savedRecordingsPath; @@ -303,13 +300,12 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { "target", connectUrl, "jvmId", - jvmIdHelper.getJvmId(connectUrl))) + jvmId)) .build() .send(); } catch (URISyntaxException | UnknownHostException - | SocketException - | JvmIdGetException e) { + | SocketException e) { logger.error(e); throw new ApiException(500, e); } diff --git a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java index c0719c2e1c..55d390cab7 100644 --- a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java @@ -416,7 +416,7 @@ public Future saveRecording( "target", connectionDescriptor.getTargetId(), "jvmId", - jvmIdHelper.getJvmId(connectionDescriptor))) + jvmIdHelper.getJvmId(connectionDescriptor.getTargetId()))) .build() .send(); } catch (Exception e) { diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandlerTest.java index 8f21a0fe1b..54e4a6309c 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandlerTest.java @@ -332,7 +332,7 @@ public String answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat( messageCaptor.getValue(), - Matchers.equalTo(Map.of("recording", recordingInfo, "target", mockConnectUrl))); + Matchers.equalTo(Map.of("recording", recordingInfo, "target", mockConnectUrl, "jvmId", mockJvmId))); } @Test @@ -491,7 +491,7 @@ public String answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat( messageCaptor.getValue(), - Matchers.equalTo(Map.of("recording", recordingInfo, "target", mockConnectUrl))); + Matchers.equalTo(Map.of("recording", recordingInfo, "target", mockConnectUrl, "jvmId", mockJvmId))); } @Test diff --git a/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java index 5f27937c22..077b43cc63 100644 --- a/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java @@ -41,6 +41,7 @@ import io.cryostat.net.security.ResourceAction; import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.net.web.http.api.ApiVersion; +import io.cryostat.recordings.JvmIdHelper; import com.google.gson.Gson; import io.vertx.core.MultiMap; @@ -54,7 +55,9 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.stubbing.Answer; @ExtendWith(MockitoExtension.class) public class TargetProbePostHandlerTest { @@ -68,6 +71,7 @@ public class TargetProbePostHandlerTest { @Mock NotificationFactory notificationFactory; @Mock Notification notification; @Mock Notification.Builder notificationBuilder; + @Mock JvmIdHelper jvmIdHelper; @Mock TargetConnectionManager targetConnectionManager; @Mock Environment env; Gson gson = MainModule.provideGson(logger); @@ -86,6 +90,7 @@ void setup() { .thenReturn(notificationBuilder); lenient().when(notificationBuilder.message(Mockito.any())).thenReturn(notificationBuilder); lenient().when(notificationBuilder.build()).thenReturn(notification); + this.handler = new TargetProbePostHandler( logger, @@ -94,6 +99,7 @@ void setup() { fs, auth, credentialsManager, + jvmIdHelper, targetConnectionManager, env, gson); @@ -144,7 +150,7 @@ class Requests { @Test public void shouldRespondOK() throws Exception { Mockito.when(requestParams.getPathParams()) - .thenReturn(Map.of("targetId", "foo", "probeTemplate", "bar")); + .thenReturn(Map.of("targetId", "foo", "probeTemplate", "bar", "jvmId", "id")); Mockito.when(requestParams.getHeaders()).thenReturn(MultiMap.caseInsensitiveMultiMap()); JFRConnection connection = Mockito.mock(JFRConnection.class); IConnectionHandle handle = Mockito.mock(IConnectionHandle.class); @@ -170,6 +176,7 @@ public void shouldRespondOK() throws Exception { + " "; Mockito.when(templateService.getTemplateContent(Mockito.anyString())) .thenReturn(templateContent); + Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getHandle()).thenReturn(handle); Mockito.when(handle.getServiceOrDummy(MBeanServerConnection.class)).thenReturn(mbsc); Object result = Mockito.mock(Object.class); @@ -188,6 +195,7 @@ public void shouldRespondOK() throws Exception { MatcherAssert.assertThat(s.get("probeTemplate"), Matchers.equalTo("bar")); MatcherAssert.assertThat(s.get("targetId"), Matchers.equalTo("foo")); MatcherAssert.assertThat(s.get("events"), Matchers.instanceOf(List.class)); + MatcherAssert.assertThat(s.get("jvmId"), Matchers.equalTo("id")); MatcherAssert.assertThat(response.getStatusCode(), Matchers.equalTo(200)); } diff --git a/src/test/java/io/cryostat/recordings/RecordingArchiveHelperTest.java b/src/test/java/io/cryostat/recordings/RecordingArchiveHelperTest.java index 3d694c8619..5e3a1d8b46 100644 --- a/src/test/java/io/cryostat/recordings/RecordingArchiveHelperTest.java +++ b/src/test/java/io/cryostat/recordings/RecordingArchiveHelperTest.java @@ -267,7 +267,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.verify(notificationFactory).createBuilder(); Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId)); + Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } @@ -355,7 +355,9 @@ public Object answer(InvocationOnMock invocation) throws Throwable { "recording", info, "target", - serviceRef1.getServiceUri().toString())); + serviceRef1.getServiceUri().toString(), + "jvmId", + "mockId")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } @@ -443,7 +445,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.verify(notificationFactory).createBuilder(); Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId)); + Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } @@ -524,7 +526,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.verify(notificationFactory).createBuilder(); Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId)); + Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } @@ -612,7 +614,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.verify(notificationFactory).createBuilder(); Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId)); + Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } @@ -753,7 +755,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.verify(notificationFactory).createBuilder(); Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId)); + Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); Mockito.verify(notificationBuilder).build(); Mockito.verify(notification).send(); } @@ -856,7 +858,7 @@ public String answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat( messageCaptor.getValue(), - Matchers.equalTo(Map.of("recording", matcher, "target", "uploads"))); + Matchers.equalTo(Map.of("recording", matcher, "target", "uploads", "jvmId", "mockId"))); } @Test diff --git a/src/test/java/io/cryostat/recordings/RecordingMetadataManagerTest.java b/src/test/java/io/cryostat/recordings/RecordingMetadataManagerTest.java index 6dc2c83a9a..00629e038e 100644 --- a/src/test/java/io/cryostat/recordings/RecordingMetadataManagerTest.java +++ b/src/test/java/io/cryostat/recordings/RecordingMetadataManagerTest.java @@ -110,6 +110,15 @@ public String answer(InvocationOnMock invocation) throws Throwable { return invocation.getArgument(0); } }); + lenient() + .when(jvmIdHelper.subdirectoryNameToJvmId(Mockito.anyString())) + .thenAnswer( + new Answer() { + @Override + public String answer(InvocationOnMock invocation) throws Throwable { + return invocation.getArgument(0); + } + }); this.recordingMetadataManager = new RecordingMetadataManager( diff --git a/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java b/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java index d625903332..73b20aea78 100644 --- a/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java +++ b/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java @@ -110,15 +110,6 @@ void setup() { lenient().when(notificationBuilder.message(Mockito.any())).thenReturn(notificationBuilder); lenient().when(notificationBuilder.build()).thenReturn(notification); lenient().when(vertx.setTimer(Mockito.anyLong(), Mockito.any())).thenReturn(1234L); - lenient() - .when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) - .thenAnswer( - new Answer() { - @Override - public String answer(InvocationOnMock invocation) throws Throwable { - return invocation.getArgument(0); - } - }); this.recordingTargetHelper = new RecordingTargetHelper( From 77ce9cd9bf6e4e9b0913348f4053d38c01e31068 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 19 Oct 2023 14:32:39 -0400 Subject: [PATCH 5/8] refractor reusable notification builder --- .../messaging/notifications/Notification.java | 25 ++- .../notifications/NotificationFactory.java | 19 ++- .../notifications/NotificationsModule.java | 7 +- .../api/beta/RecordingsFromIdPostHandler.java | 45 +++--- .../http/api/v2/TargetProbePostHandler.java | 16 +- .../recordings/RecordingArchiveHelper.java | 30 ++-- .../recordings/RecordingMetadataManager.java | 34 +--- .../recordings/RecordingTargetHelper.java | 14 +- .../beta/RecordingsFromIdPostHandlerTest.java | 62 ++++---- .../api/v2/TargetProbePostHandlerTest.java | 58 ++++--- .../RecordingArchiveHelperTest.java | 117 +++++++------- .../RecordingMetadataManagerTest.java | 39 +++-- .../recordings/RecordingTargetHelperTest.java | 149 +++++++----------- 13 files changed, 291 insertions(+), 324 deletions(-) diff --git a/src/main/java/io/cryostat/messaging/notifications/Notification.java b/src/main/java/io/cryostat/messaging/notifications/Notification.java index a4c414826c..a1d8981c1f 100644 --- a/src/main/java/io/cryostat/messaging/notifications/Notification.java +++ b/src/main/java/io/cryostat/messaging/notifications/Notification.java @@ -15,7 +15,8 @@ */ package io.cryostat.messaging.notifications; -import java.time.Instant; +import java.util.HashMap; +import java.util.Map; import io.cryostat.net.web.http.HttpMimeType; @@ -84,10 +85,30 @@ public Notification build() { } } + public static class OwnedResourceBuilder extends Builder> { + + private final Map map = new HashMap<>(); + + OwnedResourceBuilder(NotificationSource source, String category) { + super(source); + metaType(HttpMimeType.JSON); + metaCategory(category); + message(map); + } + + public OwnedResourceBuilder messageEntry(String key, Object value) { + if (value == null) { + this.map.remove(key); + } else { + this.map.put(key, value); + } + return this; + } + } + public static class Meta { private final String category; private final MetaType type; - private final long serverTime = Instant.now().getEpochSecond(); public Meta(String category, MetaType type) { this.category = category; diff --git a/src/main/java/io/cryostat/messaging/notifications/NotificationFactory.java b/src/main/java/io/cryostat/messaging/notifications/NotificationFactory.java index 74477a8435..d198811024 100644 --- a/src/main/java/io/cryostat/messaging/notifications/NotificationFactory.java +++ b/src/main/java/io/cryostat/messaging/notifications/NotificationFactory.java @@ -15,15 +15,32 @@ */ package io.cryostat.messaging.notifications; +import io.cryostat.recordings.JvmIdHelper; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; + public class NotificationFactory { private final NotificationSource source; + private final JvmIdHelper jvmIdHelper; - NotificationFactory(NotificationSource source) { + NotificationFactory(NotificationSource source, JvmIdHelper jvmIdHelper) { this.source = source; + this.jvmIdHelper = jvmIdHelper; } public Notification.Builder createBuilder() { return new Notification.Builder(source); } + + public Notification.OwnedResourceBuilder createOwnedResourceBuilder( + String notificationCategory) { + return new Notification.OwnedResourceBuilder(source, notificationCategory); + } + + public Notification.OwnedResourceBuilder createOwnedResourceBuilder( + String targetId, String notificationCategory) throws JvmIdGetException { + return new Notification.OwnedResourceBuilder(source, notificationCategory) + .messageEntry("target", targetId) + .messageEntry("jvmId", jvmIdHelper.getJvmId(targetId)); + } } diff --git a/src/main/java/io/cryostat/messaging/notifications/NotificationsModule.java b/src/main/java/io/cryostat/messaging/notifications/NotificationsModule.java index 9777bfffd6..120f72c3e3 100644 --- a/src/main/java/io/cryostat/messaging/notifications/NotificationsModule.java +++ b/src/main/java/io/cryostat/messaging/notifications/NotificationsModule.java @@ -19,6 +19,8 @@ import javax.inject.Singleton; +import io.cryostat.recordings.JvmIdHelper; + import dagger.Lazy; import dagger.Module; import dagger.Provides; @@ -34,7 +36,8 @@ static NotificationSource provideNotificationSource(Lazy handle(RequestParameters requestParams) throws List events = Arrays.asList(template.getEvents()); try { notificationFactory - .createBuilder() - .metaCategory(NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message( - Map.of( - "targetId", - targetId, - "probeTemplate", - probeTemplate, - "events", - events, - "jvmId", - jvmIdHelper.getJvmId(targetId))) + .createOwnedResourceBuilder(targetId, NOTIFICATION_CATEGORY) + .messageEntry("probeTemplate", probeTemplate) + .messageEntry("events", events) .build() .send(); } catch (JvmIdGetException e) { diff --git a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java index 55d390cab7..7fc8137865 100644 --- a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java @@ -34,7 +34,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.concurrent.CancellationException; @@ -66,7 +65,6 @@ import io.cryostat.net.TargetConnectionManager; import io.cryostat.net.web.WebModule; import io.cryostat.net.web.WebServer; -import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.net.web.http.api.v2.ApiException; import io.cryostat.platform.PlatformClient; import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; @@ -385,6 +383,8 @@ public Future saveRecording( validateSavePath(recordingName, savePath); Path filenamePath = savePath.getFileName(); String filename = filenamePath.toString(); + String targetId = connectionDescriptor.getTargetId(); + String jvmId = jvmIdHelper.getJvmId(targetId); Metadata metadata = recordingMetadataManager .copyMetadataToArchives(connectionDescriptor, recordingName, filename) @@ -406,17 +406,9 @@ public Future saveRecording( getArchivedTime(filename)); future.complete(archivedRecordingInfo); notificationFactory - .createBuilder() - .metaCategory(SAVE_NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message( - Map.of( - "recording", - archivedRecordingInfo, - "target", - connectionDescriptor.getTargetId(), - "jvmId", - jvmIdHelper.getJvmId(connectionDescriptor.getTargetId()))) + .createOwnedResourceBuilder( + connectionDescriptor.getTargetId(), SAVE_NOTIFICATION_CATEGORY) + .messageEntry("recording", archivedRecordingInfo) .build() .send(); } catch (Exception e) { @@ -453,10 +445,8 @@ public Future deleteRecordingFromPath( getFileSize(filename), getArchivedTime(filename)); notificationFactory - .createBuilder() - .metaCategory(DELETE_NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message(Map.of("recording", archivedRecordingInfo, "target", targetId, "jvmId", jvmIdHelper.getJvmId(targetId))) + .createOwnedResourceBuilder(targetId, DELETE_NOTIFICATION_CATEGORY) + .messageEntry("recording", archivedRecordingInfo) .build() .send(); fs.deleteIfExists(recordingPath); @@ -517,10 +507,8 @@ private CompletableFuture handleDeleteRecordingRequest( getFileSize(filename), getArchivedTime(filename)); notificationFactory - .createBuilder() - .metaCategory(DELETE_NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message(Map.of("recording", archivedRecordingInfo, "target", targetId, "jvmId", jvmIdHelper.getJvmId(targetId))) + .createOwnedResourceBuilder(targetId, DELETE_NOTIFICATION_CATEGORY) + .messageEntry("recording", archivedRecordingInfo) .build() .send(); checkEmptySubdirectory(parentPath); diff --git a/src/main/java/io/cryostat/recordings/RecordingMetadataManager.java b/src/main/java/io/cryostat/recordings/RecordingMetadataManager.java index baf35e6b15..ccbe13d3b1 100644 --- a/src/main/java/io/cryostat/recordings/RecordingMetadataManager.java +++ b/src/main/java/io/cryostat/recordings/RecordingMetadataManager.java @@ -47,7 +47,6 @@ import io.cryostat.messaging.notifications.NotificationFactory; import io.cryostat.net.ConnectionDescriptor; import io.cryostat.net.TargetConnectionManager; -import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.platform.PlatformClient; import io.cryostat.platform.ServiceRef; import io.cryostat.platform.TargetDiscoveryEvent; @@ -514,19 +513,9 @@ public Future setRecordingMetadataFromPath( StandardOpenOption.TRUNCATE_EXISTING); notificationFactory - .createBuilder() - .metaCategory(RecordingMetadataManager.NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message( - Map.of( - "recordingName", - recordingName, - "target", - connectUrl, - "jvmId", - jvmIdHelper.getJvmId(connectUrl), - "metadata", - metadata)) + .createOwnedResourceBuilder(connectUrl, NOTIFICATION_CATEGORY) + .messageEntry("recordingName", recordingName) + .messageEntry("metadata", metadata) .build() .send(); @@ -559,19 +548,10 @@ public Future setRecordingMetadata( if (issueNotification) { notificationFactory - .createBuilder() - .metaCategory(RecordingMetadataManager.NOTIFICATION_CATEGORY) - .metaType(HttpMimeType.JSON) - .message( - Map.of( - "recordingName", - recordingName, - "target", - connectionDescriptor.getTargetId(), - "jvmId", - jvmIdHelper.getJvmId(connectionDescriptor), - "metadata", - metadata)) + .createOwnedResourceBuilder( + connectionDescriptor.getTargetId(), NOTIFICATION_CATEGORY) + .messageEntry("recordingName", recordingName) + .messageEntry("metadata", metadata) .build() .send(); } diff --git a/src/main/java/io/cryostat/recordings/RecordingTargetHelper.java b/src/main/java/io/cryostat/recordings/RecordingTargetHelper.java index 8582396757..b42479dc30 100644 --- a/src/main/java/io/cryostat/recordings/RecordingTargetHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingTargetHelper.java @@ -44,8 +44,6 @@ import io.cryostat.net.TargetConnectionManager; import io.cryostat.net.reports.ReportService; import io.cryostat.net.web.WebServer; -import io.cryostat.net.web.http.HttpMimeType; -import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import io.cryostat.recordings.RecordingMetadataManager.Metadata; @@ -473,13 +471,11 @@ private void issueNotification( HyperlinkedSerializableRecordingDescriptor linkedDesc, String notificationCategory) { try { - notificationFactory - .createBuilder() - .metaCategory(notificationCategory) - .metaType(HttpMimeType.JSON) - .message(Map.of("recording", linkedDesc, "target", targetId, "jvmId", jvmIdHelper.get().getJvmId(targetId))) - .build() - .send(); + notificationFactory + .createOwnedResourceBuilder(targetId, notificationCategory) + .messageEntry("recording", linkedDesc) + .build() + .send(); } catch (JvmIdGetException e) { logger.info("Retain null jvmId for target [{}]", targetId); logger.info(e); diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandlerTest.java index 54e4a6309c..fd6874f4d6 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/RecordingsFromIdPostHandlerTest.java @@ -45,6 +45,7 @@ import io.cryostat.platform.ServiceRef; import io.cryostat.recordings.JvmIdHelper; import io.cryostat.recordings.JvmIdHelper.JvmIdDoesNotExistException; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import io.cryostat.recordings.RecordingArchiveHelper; import io.cryostat.recordings.RecordingMetadataManager; import io.cryostat.recordings.RecordingMetadataManager.Metadata; @@ -69,7 +70,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.Mockito; @@ -89,7 +89,7 @@ class RecordingsFromIdPostHandlerTest { @Mock WebServer webServer; @Mock NotificationFactory notificationFactory; @Mock Notification notification; - @Mock Notification.Builder notificationBuilder; + @Mock Notification.OwnedResourceBuilder notificationOwnedResourceBuilder; @Mock JvmIdHelper jvmIdHelper; @Mock RecordingArchiveHelper recordingArchiveHelper; @Mock RecordingMetadataManager recordingMetadataManager; @@ -104,19 +104,29 @@ class RecordingsFromIdPostHandlerTest { String mockConnectUrl = "someConnectUrl"; @BeforeEach - void setup() { - lenient().when(notificationFactory.createBuilder()).thenReturn(notificationBuilder); + void setup() throws JvmIdGetException { lenient() - .when(notificationBuilder.metaCategory(Mockito.any())) - .thenReturn(notificationBuilder); + .when( + notificationFactory.createOwnedResourceBuilder( + Mockito.anyString(), Mockito.anyString())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(Notification.MetaType.class))) - .thenReturn(notificationBuilder); + .when(notificationOwnedResourceBuilder.metaCategory(Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(HttpMimeType.class))) - .thenReturn(notificationBuilder); - lenient().when(notificationBuilder.message(Mockito.any())).thenReturn(notificationBuilder); - lenient().when(notificationBuilder.build()).thenReturn(notification); + .when( + notificationOwnedResourceBuilder.metaType( + Mockito.any(Notification.MetaType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when(notificationOwnedResourceBuilder.metaType(Mockito.any(HttpMimeType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when( + notificationOwnedResourceBuilder.messageEntry( + Mockito.anyString(), Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); + lenient().when(notificationOwnedResourceBuilder.build()).thenReturn(notification); lenient().when(mockServiceRef.getServiceUri()).thenReturn(mockConnectUri); lenient().when(mockConnectUri.toString()).thenReturn(mockConnectUrl); @@ -322,17 +332,11 @@ public String answer(InvocationOnMock invocation) throws Throwable { new Metadata(), 0, expectedArchivedTime); - ArgumentCaptor> messageCaptor = ArgumentCaptor.forClass(Map.class); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ArchivedRecordingCreated"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(messageCaptor.capture()); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(mockConnectUrl, "ArchivedRecordingCreated"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", recordingInfo); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); - - MatcherAssert.assertThat( - messageCaptor.getValue(), - Matchers.equalTo(Map.of("recording", recordingInfo, "target", mockConnectUrl, "jvmId", mockJvmId))); } @Test @@ -481,17 +485,11 @@ public String answer(InvocationOnMock invocation) throws Throwable { metadata, 0, expectedArchivedTime); - ArgumentCaptor> messageCaptor = ArgumentCaptor.forClass(Map.class); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ArchivedRecordingCreated"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(messageCaptor.capture()); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(mockConnectUrl, "ArchivedRecordingCreated"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", recordingInfo); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); - - MatcherAssert.assertThat( - messageCaptor.getValue(), - Matchers.equalTo(Map.of("recording", recordingInfo, "target", mockConnectUrl, "jvmId", mockJvmId))); } @Test diff --git a/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java index 077b43cc63..113e160c4e 100644 --- a/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java @@ -16,6 +16,7 @@ package io.cryostat.net.web.http.api.v2; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.startsWith; import static org.mockito.Mockito.lenient; import java.util.List; @@ -42,6 +43,7 @@ import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.net.web.http.api.ApiVersion; import io.cryostat.recordings.JvmIdHelper; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import com.google.gson.Gson; import io.vertx.core.MultiMap; @@ -55,9 +57,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; -import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoExtension; -import org.mockito.stubbing.Answer; @ExtendWith(MockitoExtension.class) public class TargetProbePostHandlerTest { @@ -71,25 +71,36 @@ public class TargetProbePostHandlerTest { @Mock NotificationFactory notificationFactory; @Mock Notification notification; @Mock Notification.Builder notificationBuilder; + @Mock Notification.OwnedResourceBuilder notificationOwnedResourceBuilder; @Mock JvmIdHelper jvmIdHelper; @Mock TargetConnectionManager targetConnectionManager; @Mock Environment env; Gson gson = MainModule.provideGson(logger); @BeforeEach - void setup() { - lenient().when(notificationFactory.createBuilder()).thenReturn(notificationBuilder); + void setup() throws JvmIdGetException { lenient() - .when(notificationBuilder.metaCategory(Mockito.any())) - .thenReturn(notificationBuilder); + .when( + notificationFactory.createOwnedResourceBuilder( + Mockito.anyString(), Mockito.anyString())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(Notification.MetaType.class))) - .thenReturn(notificationBuilder); + .when(notificationOwnedResourceBuilder.metaCategory(Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(HttpMimeType.class))) - .thenReturn(notificationBuilder); - lenient().when(notificationBuilder.message(Mockito.any())).thenReturn(notificationBuilder); - lenient().when(notificationBuilder.build()).thenReturn(notification); + .when( + notificationOwnedResourceBuilder.metaType( + Mockito.any(Notification.MetaType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when(notificationOwnedResourceBuilder.metaType(Mockito.any(HttpMimeType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when( + notificationOwnedResourceBuilder.messageEntry( + Mockito.anyString(), Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); + lenient().when(notificationOwnedResourceBuilder.build()).thenReturn(notification); this.handler = new TargetProbePostHandler( @@ -176,7 +187,6 @@ public void shouldRespondOK() throws Exception { + " "; Mockito.when(templateService.getTemplateContent(Mockito.anyString())) .thenReturn(templateContent); - Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getHandle()).thenReturn(handle); Mockito.when(handle.getServiceOrDummy(MBeanServerConnection.class)).thenReturn(mbsc); Object result = Mockito.mock(Object.class); @@ -187,15 +197,21 @@ public void shouldRespondOK() throws Exception { any(Object[].class), any(String[].class))) .thenReturn(result); + IntermediateResponse response = handler.handle(requestParams); - ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Map.class); - - Mockito.verify(notificationBuilder).message(argumentCaptor.capture()); - Map s = argumentCaptor.getValue(); - MatcherAssert.assertThat(s.get("probeTemplate"), Matchers.equalTo("bar")); - MatcherAssert.assertThat(s.get("targetId"), Matchers.equalTo("foo")); - MatcherAssert.assertThat(s.get("events"), Matchers.instanceOf(List.class)); - MatcherAssert.assertThat(s.get("jvmId"), Matchers.equalTo("id")); + + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder("foo", "ProbeTemplateApplied"); + Mockito.verify(notificationOwnedResourceBuilder) + .messageEntry("probeTemplate", "bar"); + Mockito.verify(notificationOwnedResourceBuilder).build(); + Mockito.verify(notification).send(); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Object.class); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry(Mockito.matches("events"), argumentCaptor.capture()); + Object s = argumentCaptor.getValue(); + MatcherAssert.assertThat(s, Matchers.instanceOf(List.class)); + MatcherAssert.assertThat(response.getStatusCode(), Matchers.equalTo(200)); } diff --git a/src/test/java/io/cryostat/recordings/RecordingArchiveHelperTest.java b/src/test/java/io/cryostat/recordings/RecordingArchiveHelperTest.java index 5e3a1d8b46..918d11adb0 100644 --- a/src/test/java/io/cryostat/recordings/RecordingArchiveHelperTest.java +++ b/src/test/java/io/cryostat/recordings/RecordingArchiveHelperTest.java @@ -28,7 +28,6 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.List; -import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -49,6 +48,7 @@ import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.platform.PlatformClient; import io.cryostat.platform.ServiceRef; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import io.cryostat.recordings.RecordingArchiveHelper.ArchiveDirectory; import io.cryostat.recordings.RecordingMetadataManager.Metadata; import io.cryostat.rules.ArchivedRecordingInfo; @@ -64,7 +64,6 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; -import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; @@ -89,7 +88,7 @@ class RecordingArchiveHelperTest { @Mock JvmIdHelper jvmIdHelper; @Mock Base32 base32; @Mock Notification notification; - @Mock Notification.Builder notificationBuilder; + @Mock Notification.OwnedResourceBuilder notificationOwnedResourceBuilder; @Mock JFRConnection connection; @Mock CryostatFlightRecorderService service; @Mock Vertx vertx; @@ -99,19 +98,29 @@ class RecordingArchiveHelperTest { String recordingName = "someRecording"; @BeforeEach - void setup() { - lenient().when(notificationFactory.createBuilder()).thenReturn(notificationBuilder); + void setup() throws JvmIdGetException { lenient() - .when(notificationBuilder.metaCategory(Mockito.any())) - .thenReturn(notificationBuilder); + .when( + notificationFactory.createOwnedResourceBuilder( + Mockito.anyString(), Mockito.anyString())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(Notification.MetaType.class))) - .thenReturn(notificationBuilder); + .when(notificationOwnedResourceBuilder.metaCategory(Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(HttpMimeType.class))) - .thenReturn(notificationBuilder); - lenient().when(notificationBuilder.message(Mockito.any())).thenReturn(notificationBuilder); - lenient().when(notificationBuilder.build()).thenReturn(notification); + .when( + notificationOwnedResourceBuilder.metaType( + Mockito.any(Notification.MetaType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when(notificationOwnedResourceBuilder.metaType(Mockito.any(HttpMimeType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when( + notificationOwnedResourceBuilder.messageEntry( + Mockito.anyString(), Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); + lenient().when(notificationOwnedResourceBuilder.build()).thenReturn(notification); /* subdirectoryName is actually the base32 encoded jvmId, but for testing purposes they are equal */ lenient() @@ -264,11 +273,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat(info.getName(), Matchers.equalTo(savedName)); Mockito.verify(fs).copy(Mockito.isA(BufferedInputStream.class), Mockito.eq(destination)); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(targetId, "ActiveRecordingSaved"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", info); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } @@ -346,19 +354,11 @@ public Object answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat(info.getName(), Matchers.equalTo(savedName)); Mockito.verify(fs).copy(Mockito.isA(BufferedInputStream.class), Mockito.eq(destination)); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder) - .message( - Map.of( - "recording", - info, - "target", - serviceRef1.getServiceUri().toString(), - "jvmId", - "mockId")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder( + serviceRef1.getServiceUri().toString(), "ActiveRecordingSaved"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", info); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } @@ -442,11 +442,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat(info.getName(), Matchers.equalTo(savedName)); Mockito.verify(fs).copy(Mockito.isA(BufferedInputStream.class), Mockito.eq(destination)); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(targetId, "ActiveRecordingSaved"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", info); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } @@ -523,11 +522,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat(info.getName(), Matchers.equalTo(savedName)); Mockito.verify(fs).copy(Mockito.isA(BufferedInputStream.class), Mockito.eq(destination)); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(targetId, "ActiveRecordingSaved"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", info); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } @@ -611,11 +609,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat(info.getName(), Matchers.equalTo(savedName)); Mockito.verify(fs).copy(Mockito.isA(BufferedInputStream.class), Mockito.eq(destination)); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(targetId, "ActiveRecordingSaved"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", info); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } @@ -752,11 +749,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat(info.getName(), Matchers.equalTo(savedName)); Mockito.verify(fs).copy(Mockito.isA(BufferedInputStream.class), Mockito.eq(destination)); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingSaved"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(Map.of("recording", info, "target", targetId, "jvmId", "mockId")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(targetId, "ActiveRecordingSaved"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", info); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } @@ -827,8 +823,6 @@ public String answer(InvocationOnMock invocation) throws Throwable { ArchivedRecordingInfo deleted = recordingArchiveHelper.deleteRecording(recordingName).get(); - ArgumentCaptor> messageCaptor = ArgumentCaptor.forClass(Map.class); - Mockito.verify(fs) .deleteIfExists( archivedRecordingsPath @@ -836,14 +830,6 @@ public String answer(InvocationOnMock invocation) throws Throwable { .resolve(recordingName) .toAbsolutePath()); - Mockito.verify(fs).deleteIfExists(destinationFile); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ArchivedRecordingDeleted"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(messageCaptor.capture()); - Mockito.verify(notificationBuilder).build(); - Mockito.verify(notification).send(); - ArchivedRecordingInfo matcher = new ArchivedRecordingInfo( "uploads", @@ -856,9 +842,12 @@ public String answer(InvocationOnMock invocation) throws Throwable { MatcherAssert.assertThat(deleted, Matchers.equalTo(matcher)); - MatcherAssert.assertThat( - messageCaptor.getValue(), - Matchers.equalTo(Map.of("recording", matcher, "target", "uploads", "jvmId", "mockId"))); + Mockito.verify(fs).deleteIfExists(destinationFile); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder("uploads", "ArchivedRecordingDeleted"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", matcher); + Mockito.verify(notificationOwnedResourceBuilder).build(); + Mockito.verify(notification).send(); } @Test diff --git a/src/test/java/io/cryostat/recordings/RecordingMetadataManagerTest.java b/src/test/java/io/cryostat/recordings/RecordingMetadataManagerTest.java index 00629e038e..d63c0bef77 100644 --- a/src/test/java/io/cryostat/recordings/RecordingMetadataManagerTest.java +++ b/src/test/java/io/cryostat/recordings/RecordingMetadataManagerTest.java @@ -41,6 +41,7 @@ import io.cryostat.net.TargetConnectionManager; import io.cryostat.net.web.http.HttpMimeType; import io.cryostat.platform.PlatformClient; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import io.cryostat.recordings.RecordingMetadataManager.Metadata; import io.cryostat.recordings.RecordingMetadataManager.StoredRecordingMetadata; @@ -79,28 +80,38 @@ public class RecordingMetadataManagerTest { @Mock NotificationFactory notificationFactory; @Mock JvmIdHelper jvmIdHelper; @Mock Notification notification; - @Mock Notification.Builder notificationBuilder; + @Mock Notification.OwnedResourceBuilder notificationOwnedResourceBuilder; @Mock JFRConnection connection; @Mock ConnectionDescriptor connectionDescriptor; Gson gson = MainModule.provideGson(logger); @BeforeEach - void setup() { + void setup() throws JvmIdGetException { Gson gson = new Gson(); Base32 base32 = new Base32(); - lenient().when(notificationFactory.createBuilder()).thenReturn(notificationBuilder); lenient() - .when(notificationBuilder.metaCategory(Mockito.any())) - .thenReturn(notificationBuilder); + .when( + notificationFactory.createOwnedResourceBuilder( + Mockito.anyString(), Mockito.anyString())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(Notification.MetaType.class))) - .thenReturn(notificationBuilder); + .when(notificationOwnedResourceBuilder.metaCategory(Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(HttpMimeType.class))) - .thenReturn(notificationBuilder); - lenient().when(notificationBuilder.message(Mockito.any())).thenReturn(notificationBuilder); - lenient().when(notificationBuilder.build()).thenReturn(notification); + .when( + notificationOwnedResourceBuilder.metaType( + Mockito.any(Notification.MetaType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when(notificationOwnedResourceBuilder.metaType(Mockito.any(HttpMimeType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when( + notificationOwnedResourceBuilder.messageEntry( + Mockito.anyString(), Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); + lenient().when(notificationOwnedResourceBuilder.build()).thenReturn(notification); lenient() .when(jvmIdHelper.jvmIdToSubdirectoryName(Mockito.anyString())) .thenAnswer( @@ -114,10 +125,10 @@ public String answer(InvocationOnMock invocation) throws Throwable { .when(jvmIdHelper.subdirectoryNameToJvmId(Mockito.anyString())) .thenAnswer( new Answer() { - @Override - public String answer(InvocationOnMock invocation) throws Throwable { + @Override + public String answer(InvocationOnMock invocation) throws Throwable { return invocation.getArgument(0); - } + } }); this.recordingMetadataManager = diff --git a/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java b/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java index 73b20aea78..9164a5a967 100644 --- a/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java +++ b/src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java @@ -53,7 +53,7 @@ import io.cryostat.net.reports.ReportService; import io.cryostat.net.web.WebServer; import io.cryostat.net.web.http.HttpMimeType; -import io.cryostat.recordings.JvmIdHelper; +import io.cryostat.recordings.JvmIdHelper.JvmIdGetException; import io.cryostat.recordings.RecordingMetadataManager.Metadata; import io.cryostat.recordings.RecordingTargetHelper.ReplacementPolicy; import io.cryostat.recordings.RecordingTargetHelper.SnapshotCreationException; @@ -86,7 +86,7 @@ public class RecordingTargetHelperTest { @Mock NotificationFactory notificationFactory; @Mock RecordingOptionsBuilderFactory recordingOptionsBuilderFactory; @Mock Notification notification; - @Mock Notification.Builder notificationBuilder; + @Mock Notification.OwnedResourceBuilder notificationOwnedResourceBuilder; @Mock ReportService reportService; @Mock RecordingMetadataManager recordingMetadataManager; @Mock RecordingArchiveHelper recordingArchiveHelper; @@ -96,19 +96,29 @@ public class RecordingTargetHelperTest { @Mock CryostatFlightRecorderService service; @BeforeEach - void setup() { - lenient().when(notificationFactory.createBuilder()).thenReturn(notificationBuilder); + void setup() throws JvmIdGetException { lenient() - .when(notificationBuilder.metaCategory(Mockito.any())) - .thenReturn(notificationBuilder); + .when( + notificationFactory.createOwnedResourceBuilder( + Mockito.anyString(), Mockito.anyString())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(Notification.MetaType.class))) - .thenReturn(notificationBuilder); + .when(notificationOwnedResourceBuilder.metaCategory(Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); lenient() - .when(notificationBuilder.metaType(Mockito.any(HttpMimeType.class))) - .thenReturn(notificationBuilder); - lenient().when(notificationBuilder.message(Mockito.any())).thenReturn(notificationBuilder); - lenient().when(notificationBuilder.build()).thenReturn(notification); + .when( + notificationOwnedResourceBuilder.metaType( + Mockito.any(Notification.MetaType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when(notificationOwnedResourceBuilder.metaType(Mockito.any(HttpMimeType.class))) + .thenReturn(notificationOwnedResourceBuilder); + lenient() + .when( + notificationOwnedResourceBuilder.messageEntry( + Mockito.anyString(), Mockito.any())) + .thenReturn(notificationOwnedResourceBuilder); + lenient().when(notificationOwnedResourceBuilder.build()).thenReturn(notification); lenient().when(vertx.setTimer(Mockito.anyLong(), Mockito.any())).thenReturn(1234L); this.recordingTargetHelper = @@ -200,7 +210,6 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } }); - Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor(recordingName); Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor)); @@ -225,12 +234,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { HyperlinkedSerializableRecordingDescriptor linkedDesc = new HyperlinkedSerializableRecordingDescriptor(descriptor, null, null, metadata); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingDeleted"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder) - .message(Map.of("recording", linkedDesc, "target", "fooTarget", "jvmId", "id")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder("fooTarget", "ActiveRecordingDeleted"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", linkedDesc); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } @@ -251,7 +258,6 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } }); - Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor(recordingName); Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor)); @@ -276,12 +282,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { HyperlinkedSerializableRecordingDescriptor linkedDesc = new HyperlinkedSerializableRecordingDescriptor(descriptor, null, null, metadata); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("SnapshotDeleted"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder) - .message(Map.of("recording", linkedDesc, "target", "fooTarget", "jvmId", "id")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder("fooTarget", "SnapshotDeleted"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", linkedDesc); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } @@ -317,7 +321,6 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } }); - Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor(recordingName); Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor)); @@ -329,7 +332,9 @@ public Object answer(InvocationOnMock invocation) throws Throwable { recordingTargetHelper.deleteRecording(connectionDescriptor, recordingName).get(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingDeleted"); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder( + connectionDescriptor.getTargetId(), "ActiveRecordingDeleted"); } @ParameterizedTest @@ -355,7 +360,6 @@ public Object answer(InvocationOnMock invocation) throws Throwable { } }); - Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor(recordingName); Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(descriptor)); @@ -367,7 +371,8 @@ public Object answer(InvocationOnMock invocation) throws Throwable { recordingTargetHelper.deleteRecording(connectionDescriptor, recordingName).get(); - Mockito.verify(notificationBuilder).metaCategory("SnapshotDeleted"); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(connectionDescriptor.getTargetId(), "SnapshotDeleted"); } @Test @@ -522,7 +527,6 @@ void shouldVerifySnapshotWithNotification() throws Exception { new Random(123456).nextBytes(src); InputStream snapshot = new ByteArrayInputStream(src); Mockito.when(snapshotOptional.get()).thenReturn(snapshot); - Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(targetConnectionManager.markConnectionInUse(connectionDescriptor)) .thenReturn(true); @@ -539,12 +543,11 @@ void shouldVerifySnapshotWithNotification() throws Exception { .verifySnapshot(connectionDescriptor, snapshotDescriptor) .get(); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("SnapshotCreated"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder) - .message(Map.of("recording", snapshotDescriptor, "target", "fooTarget", "jvmId", "id")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder("fooTarget", "SnapshotCreated"); + Mockito.verify(notificationOwnedResourceBuilder) + .messageEntry("recording", snapshotDescriptor); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); Assertions.assertTrue(verified); @@ -585,12 +588,11 @@ void shouldVerifySnapshotWithoutNotification() throws Exception { .verifySnapshot(connectionDescriptor, snapshotDescriptor, false) .get(); - Mockito.verify(notificationFactory, Mockito.never()).createBuilder(); - Mockito.verify(notificationBuilder, Mockito.never()).metaCategory("SnapshotCreated"); - Mockito.verify(notificationBuilder, Mockito.never()).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder, Mockito.never()) - .message(Map.of("recording", snapshotDescriptor, "target", "fooTarget")); - Mockito.verify(notificationBuilder, Mockito.never()).build(); + Mockito.verify(notificationFactory, Mockito.never()) + .createOwnedResourceBuilder("fooTarget", "SnapshotCreated"); + Mockito.verify(notificationOwnedResourceBuilder, Mockito.never()) + .messageEntry("recording", snapshotDescriptor); + Mockito.verify(notificationOwnedResourceBuilder, Mockito.never()).build(); Mockito.verify(notification, Mockito.never()).send(); Assertions.assertTrue(verified); @@ -710,7 +712,6 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Mockito.when(recordingOptions.get(Mockito.any())).thenReturn(recordingName, duration); - Mockito.when(jvmIdHelper.getJvmId(targetId)).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); Mockito.when(service.getAvailableRecordings()) .thenReturn(Collections.emptyList(), List.of(recordingDescriptor)); @@ -744,48 +745,17 @@ public Future answer(InvocationOnMock invocation) Mockito.verify(service) .start(Mockito.any(), Mockito.eq(templateName), Mockito.eq(templateType)); + Metadata descMetadata = + new Metadata(Map.of("template.name", "Profiling", "template.type", "TARGET")); HyperlinkedSerializableRecordingDescriptor linkedDesc = - new HyperlinkedSerializableRecordingDescriptor(recordingDescriptor, null, null); - - ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(Map.class); + new HyperlinkedSerializableRecordingDescriptor( + recordingDescriptor, null, null, descMetadata); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingCreated"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder).message(messageCaptor.capture()); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder(targetId, "ActiveRecordingCreated"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", linkedDesc); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); - - Map message = messageCaptor.getValue(); - MatcherAssert.assertThat(message.get("target"), Matchers.equalTo(targetId)); - - HyperlinkedSerializableRecordingDescriptor capturedDescriptor = - (HyperlinkedSerializableRecordingDescriptor) message.get("recording"); - MatcherAssert.assertThat( - capturedDescriptor.getName(), Matchers.equalTo(linkedDesc.getName())); - MatcherAssert.assertThat( - capturedDescriptor.getReportUrl(), Matchers.equalTo(linkedDesc.getReportUrl())); - MatcherAssert.assertThat( - capturedDescriptor.getDownloadUrl(), Matchers.equalTo(linkedDesc.getDownloadUrl())); - MatcherAssert.assertThat( - capturedDescriptor.getState(), Matchers.equalTo(linkedDesc.getState())); - MatcherAssert.assertThat( - capturedDescriptor.getDuration(), Matchers.equalTo(linkedDesc.getDuration())); - MatcherAssert.assertThat(capturedDescriptor.getId(), Matchers.equalTo(linkedDesc.getId())); - MatcherAssert.assertThat( - capturedDescriptor.getMaxAge(), Matchers.equalTo(linkedDesc.getMaxAge())); - MatcherAssert.assertThat( - capturedDescriptor.getMaxSize(), Matchers.equalTo(linkedDesc.getMaxSize())); - MatcherAssert.assertThat( - capturedDescriptor.getStartTime(), Matchers.equalTo(linkedDesc.getStartTime())); - MatcherAssert.assertThat( - capturedDescriptor.getToDisk(), Matchers.equalTo(linkedDesc.getToDisk())); - - MatcherAssert.assertThat( - capturedDescriptor.getMetadata(), - Matchers.equalTo( - new Metadata( - Map.of("template.name", "Profiling", "template.type", "TARGET")))); } void shouldReplaceExistingRecording() throws Exception { @@ -863,7 +833,6 @@ void shouldCloseAndRecreateIfRecordingExistsAndIsRunning() throws Exception { Metadata metadata = new Metadata(Map.of("template.name", "Profiling", "template.type", "TARGET")); - Mockito.when(jvmIdHelper.getJvmId(targetId)).thenReturn("id"); Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any())) .thenAnswer( invocation -> { @@ -918,7 +887,6 @@ void shouldRestartRecordingWhenRecordingExistsAndIsStopped() throws Exception { Metadata metadata = new Metadata(Map.of("template.name", "Profiling", "template.type", "TARGET")); - Mockito.when(jvmIdHelper.getJvmId(targetId)).thenReturn("id"); Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any())) .thenAnswer( invocation -> { @@ -975,7 +943,6 @@ public Object answer(InvocationOnMock invocation) throws Throwable { return task.execute(connection); } }); - Mockito.when(jvmIdHelper.getJvmId(Mockito.anyString())).thenReturn("id"); Mockito.when(connection.getService()).thenReturn(service); IRecordingDescriptor descriptor = createDescriptor("someRecording"); Mockito.when(descriptor.getName()).thenReturn("someRecording"); @@ -990,12 +957,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { new HyperlinkedSerializableRecordingDescriptor( descriptor, null, null, RecordingState.STOPPED); - Mockito.verify(notificationFactory).createBuilder(); - Mockito.verify(notificationBuilder).metaCategory("ActiveRecordingStopped"); - Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON); - Mockito.verify(notificationBuilder) - .message(Map.of("recording", linkedDesc, "target", "fooTarget", "jvmId", "id")); - Mockito.verify(notificationBuilder).build(); + Mockito.verify(notificationFactory) + .createOwnedResourceBuilder("fooTarget", "ActiveRecordingStopped"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("recording", linkedDesc); + Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); } From 8f4f04ffa99ee75e9a886609dbb583769a8aceaf Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Fri, 20 Oct 2023 11:49:49 -0400 Subject: [PATCH 6/8] spotless --- .../net/web/http/api/v2/TargetProbePostHandlerTest.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java index 113e160c4e..789ea41e1b 100644 --- a/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/v2/TargetProbePostHandlerTest.java @@ -16,7 +16,6 @@ package io.cryostat.net.web.http.api.v2; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.startsWith; import static org.mockito.Mockito.lenient; import java.util.List; @@ -202,13 +201,13 @@ public void shouldRespondOK() throws Exception { Mockito.verify(notificationFactory) .createOwnedResourceBuilder("foo", "ProbeTemplateApplied"); - Mockito.verify(notificationOwnedResourceBuilder) - .messageEntry("probeTemplate", "bar"); + Mockito.verify(notificationOwnedResourceBuilder).messageEntry("probeTemplate", "bar"); Mockito.verify(notificationOwnedResourceBuilder).build(); Mockito.verify(notification).send(); - + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Object.class); - Mockito.verify(notificationOwnedResourceBuilder).messageEntry(Mockito.matches("events"), argumentCaptor.capture()); + Mockito.verify(notificationOwnedResourceBuilder) + .messageEntry(Mockito.matches("events"), argumentCaptor.capture()); Object s = argumentCaptor.getValue(); MatcherAssert.assertThat(s, Matchers.instanceOf(List.class)); From 1f63b8b6390c72148c08f57198644be7f13caa50 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Fri, 20 Oct 2023 12:03:25 -0400 Subject: [PATCH 7/8] spotbugs --- .../AbstractReportGeneratorService.java | 27 +++++++++---------- .../recordings/RecordingArchiveHelper.java | 9 +++---- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/main/java/io/cryostat/net/reports/AbstractReportGeneratorService.java b/src/main/java/io/cryostat/net/reports/AbstractReportGeneratorService.java index f0d19a52f7..03541bb000 100644 --- a/src/main/java/io/cryostat/net/reports/AbstractReportGeneratorService.java +++ b/src/main/java/io/cryostat/net/reports/AbstractReportGeneratorService.java @@ -84,22 +84,21 @@ Path copyRecordingToFile( if (!Objects.equals(rec.getName(), recordingName)) { continue; } - try (OutputStream out = new BufferedOutputStream(new FileOutputStream(path.toFile()))) { - try (conn; - InputStream in = conn.getService().openStream(rec, false)) { - byte[] buff = new byte[READ_BUFFER_SIZE]; - int n = 0; - while ((n = in.read(buff)) != -1) { - out.write(buff, 0, n); - if (!targetConnectionManager.markConnectionInUse(cd)) { - throw new IOException( - "Target connection unexpectedly closed while streaming" - + " recording"); - } + try (conn; + OutputStream out = new BufferedOutputStream(new FileOutputStream(path.toFile())); + InputStream in = conn.getService().openStream(rec, false)) { + byte[] buff = new byte[READ_BUFFER_SIZE]; + int n = 0; + while ((n = in.read(buff)) != -1) { + out.write(buff, 0, n); + if (!targetConnectionManager.markConnectionInUse(cd)) { + throw new IOException( + "Target connection unexpectedly closed while streaming" + + " recording"); } - out.flush(); - return path; } + out.flush(); + return path; } } throw new RecordingNotFoundException(cd.getTargetId(), recordingName); diff --git a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java index 7fc8137865..d203e8b90f 100644 --- a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java @@ -384,30 +384,29 @@ public Future saveRecording( Path filenamePath = savePath.getFileName(); String filename = filenamePath.toString(); String targetId = connectionDescriptor.getTargetId(); - String jvmId = jvmIdHelper.getJvmId(targetId); Metadata metadata = recordingMetadataManager .copyMetadataToArchives(connectionDescriptor, recordingName, filename) .get(); ArchivedRecordingInfo archivedRecordingInfo = new ArchivedRecordingInfo( - connectionDescriptor.getTargetId(), + targetId, filename, webServerProvider .get() .getArchivedDownloadURL( - connectionDescriptor.getTargetId(), filename), + targetId, filename), webServerProvider .get() .getArchivedReportURL( - connectionDescriptor.getTargetId(), filename), + targetId, filename), metadata, getFileSize(filename), getArchivedTime(filename)); future.complete(archivedRecordingInfo); notificationFactory .createOwnedResourceBuilder( - connectionDescriptor.getTargetId(), SAVE_NOTIFICATION_CATEGORY) + targetId, SAVE_NOTIFICATION_CATEGORY) .messageEntry("recording", archivedRecordingInfo) .build() .send(); From 17e55b05fba2b4e1cf8cb5079fbf656a362004e1 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Fri, 20 Oct 2023 12:05:11 -0400 Subject: [PATCH 8/8] spotless --- .../net/reports/AbstractReportGeneratorService.java | 3 ++- .../cryostat/recordings/RecordingArchiveHelper.java | 13 +++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/cryostat/net/reports/AbstractReportGeneratorService.java b/src/main/java/io/cryostat/net/reports/AbstractReportGeneratorService.java index 03541bb000..c97335a6aa 100644 --- a/src/main/java/io/cryostat/net/reports/AbstractReportGeneratorService.java +++ b/src/main/java/io/cryostat/net/reports/AbstractReportGeneratorService.java @@ -85,7 +85,8 @@ Path copyRecordingToFile( continue; } try (conn; - OutputStream out = new BufferedOutputStream(new FileOutputStream(path.toFile())); + OutputStream out = + new BufferedOutputStream(new FileOutputStream(path.toFile())); InputStream in = conn.getService().openStream(rec, false)) { byte[] buff = new byte[READ_BUFFER_SIZE]; int n = 0; diff --git a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java index d203e8b90f..7243c29c86 100644 --- a/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java @@ -392,21 +392,14 @@ public Future saveRecording( new ArchivedRecordingInfo( targetId, filename, - webServerProvider - .get() - .getArchivedDownloadURL( - targetId, filename), - webServerProvider - .get() - .getArchivedReportURL( - targetId, filename), + webServerProvider.get().getArchivedDownloadURL(targetId, filename), + webServerProvider.get().getArchivedReportURL(targetId, filename), metadata, getFileSize(filename), getArchivedTime(filename)); future.complete(archivedRecordingInfo); notificationFactory - .createOwnedResourceBuilder( - targetId, SAVE_NOTIFICATION_CATEGORY) + .createOwnedResourceBuilder(targetId, SAVE_NOTIFICATION_CATEGORY) .messageEntry("recording", archivedRecordingInfo) .build() .send();