Skip to content

Commit

Permalink
refractor reusable notification builder
Browse files Browse the repository at this point in the history
  • Loading branch information
mwangggg committed Oct 19, 2023
1 parent 7f3b29c commit 0013d92
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 105 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/
package io.cryostat.messaging.notifications;

import java.time.Instant;

import io.cryostat.net.web.http.HttpMimeType;

public class Notification<T> {
Expand Down Expand Up @@ -87,7 +85,6 @@ public Notification<T> build() {
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package io.cryostat.messaging.notifications;

import io.cryostat.net.web.http.HttpMimeType;

public class NotificationFactory {

private final NotificationSource source;
Expand All @@ -26,4 +28,10 @@ public class NotificationFactory {
public <T> Notification.Builder<T> createBuilder() {
return new Notification.Builder<T>(source);
}

public <T> Notification.Builder<T> createOwnedResourceBuilder(String notificationCategory) {
return new Notification.Builder<T>(source)
.metaType(HttpMimeType.JSON)
.metaCategory(notificationCategory);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
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;
Expand Down Expand Up @@ -275,9 +274,7 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
try {

notificationFactory
.createBuilder()
.metaCategory(NOTIFICATION_CATEGORY)
.metaType(HttpMimeType.JSON)
.createOwnedResourceBuilder(NOTIFICATION_CATEGORY)
.message(
Map.of(
"recording",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,7 @@ public IntermediateResponse<Void> handle(RequestParameters requestParams) throws
List<Event> events = Arrays.asList(template.getEvents());
try {
notificationFactory
.createBuilder()
.metaCategory(NOTIFICATION_CATEGORY)
.metaType(HttpMimeType.JSON)
.createOwnedResourceBuilder(NOTIFICATION_CATEGORY)
.message(
Map.of(
"targetId",
Expand Down
35 changes: 22 additions & 13 deletions src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,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;
Expand Down Expand Up @@ -385,6 +384,8 @@ public Future<ArchivedRecordingInfo> 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)
Expand All @@ -406,17 +407,15 @@ public Future<ArchivedRecordingInfo> saveRecording(
getArchivedTime(filename));
future.complete(archivedRecordingInfo);
notificationFactory
.createBuilder()
.metaCategory(SAVE_NOTIFICATION_CATEGORY)
.metaType(HttpMimeType.JSON)
.createOwnedResourceBuilder(SAVE_NOTIFICATION_CATEGORY)
.message(
Map.of(
"recording",
archivedRecordingInfo,
"target",
connectionDescriptor.getTargetId(),
"jvmId",
jvmIdHelper.getJvmId(connectionDescriptor.getTargetId())))
jvmId))
.build()
.send();
} catch (Exception e) {
Expand Down Expand Up @@ -453,10 +452,15 @@ public Future<ArchivedRecordingInfo> 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(DELETE_NOTIFICATION_CATEGORY)
.message(
Map.of(
"recording",
archivedRecordingInfo,
"target",
targetId,
"jvmId",
jvmId))
.build()
.send();
fs.deleteIfExists(recordingPath);
Expand Down Expand Up @@ -517,10 +521,15 @@ private CompletableFuture<ArchivedRecordingInfo> 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(DELETE_NOTIFICATION_CATEGORY)
.message(
Map.of(
"recording",
archivedRecordingInfo,
"target",
targetId,
"jvmId",
jvmIdHelper.getJvmId(targetId)))
.build()
.send();
checkEmptySubdirectory(parentPath);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -514,17 +513,15 @@ public Future<Metadata> setRecordingMetadataFromPath(
StandardOpenOption.TRUNCATE_EXISTING);

notificationFactory
.createBuilder()
.metaCategory(RecordingMetadataManager.NOTIFICATION_CATEGORY)
.metaType(HttpMimeType.JSON)
.createOwnedResourceBuilder(NOTIFICATION_CATEGORY)
.message(
Map.of(
"recordingName",
recordingName,
"target",
connectUrl,
"jvmId",
jvmIdHelper.getJvmId(connectUrl),
jvmId,
"metadata",
metadata))
.build()
Expand Down Expand Up @@ -559,17 +556,15 @@ public Future<Metadata> setRecordingMetadata(

if (issueNotification) {
notificationFactory
.createBuilder()
.metaCategory(RecordingMetadataManager.NOTIFICATION_CATEGORY)
.metaType(HttpMimeType.JSON)
.createOwnedResourceBuilder(NOTIFICATION_CATEGORY)
.message(
Map.of(
"recordingName",
recordingName,
"target",
connectionDescriptor.getTargetId(),
"jvmId",
jvmIdHelper.getJvmId(connectionDescriptor),
jvmId,
"metadata",
metadata))
.build()
Expand Down
21 changes: 12 additions & 9 deletions src/main/java/io/cryostat/recordings/RecordingTargetHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -473,13 +471,18 @@ 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(notificationCategory)
.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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ class RecordingsFromIdPostHandlerTest {

@BeforeEach
void setup() {
lenient().when(notificationFactory.createBuilder()).thenReturn(notificationBuilder);
lenient()
.when(notificationFactory.createOwnedResourceBuilder(Mockito.anyString()))
.thenReturn(notificationBuilder);
lenient()
.when(notificationBuilder.metaCategory(Mockito.any()))
.thenReturn(notificationBuilder);
Expand Down Expand Up @@ -323,16 +325,21 @@ public String answer(InvocationOnMock invocation) throws Throwable {
0,
expectedArchivedTime);
ArgumentCaptor<Map<String, Object>> messageCaptor = ArgumentCaptor.forClass(Map.class);
Mockito.verify(notificationFactory).createBuilder();
Mockito.verify(notificationBuilder).metaCategory("ArchivedRecordingCreated");
Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON);
Mockito.verify(notificationFactory).createOwnedResourceBuilder("ArchivedRecordingCreated");
Mockito.verify(notificationBuilder).message(messageCaptor.capture());
Mockito.verify(notificationBuilder).build();
Mockito.verify(notification).send();

MatcherAssert.assertThat(
messageCaptor.getValue(),
Matchers.equalTo(Map.of("recording", recordingInfo, "target", mockConnectUrl, "jvmId", mockJvmId)));
Matchers.equalTo(
Map.of(
"recording",
recordingInfo,
"target",
mockConnectUrl,
"jvmId",
mockJvmId)));
}

@Test
Expand Down Expand Up @@ -482,16 +489,21 @@ public String answer(InvocationOnMock invocation) throws Throwable {
0,
expectedArchivedTime);
ArgumentCaptor<Map<String, Object>> messageCaptor = ArgumentCaptor.forClass(Map.class);
Mockito.verify(notificationFactory).createBuilder();
Mockito.verify(notificationBuilder).metaCategory("ArchivedRecordingCreated");
Mockito.verify(notificationBuilder).metaType(HttpMimeType.JSON);
Mockito.verify(notificationFactory).createOwnedResourceBuilder("ArchivedRecordingCreated");
Mockito.verify(notificationBuilder).message(messageCaptor.capture());
Mockito.verify(notificationBuilder).build();
Mockito.verify(notification).send();

MatcherAssert.assertThat(
messageCaptor.getValue(),
Matchers.equalTo(Map.of("recording", recordingInfo, "target", mockConnectUrl, "jvmId", mockJvmId)));
Matchers.equalTo(
Map.of(
"recording",
recordingInfo,
"target",
mockConnectUrl,
"jvmId",
mockJvmId)));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,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 {
Expand All @@ -78,7 +76,9 @@ public class TargetProbePostHandlerTest {

@BeforeEach
void setup() {
lenient().when(notificationFactory.createBuilder()).thenReturn(notificationBuilder);
lenient()
.when(notificationFactory.createOwnedResourceBuilder(Mockito.anyString()))
.thenReturn(notificationBuilder);
lenient()
.when(notificationBuilder.metaCategory(Mockito.any()))
.thenReturn(notificationBuilder);
Expand Down
Loading

0 comments on commit 0013d92

Please sign in to comment.