Skip to content

Commit

Permalink
use RecordingTargetHelper to clean up
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrewazores
andrewazores committed Oct 6, 2022
1 parent 8307bf4 commit e9e3b11
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 140 deletions.
69 changes: 21 additions & 48 deletions src/main/java/io/cryostat/net/web/http/api/v2/RuleDeleteHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@
import io.cryostat.messaging.notifications.NotificationFactory;
import io.cryostat.net.AuthManager;
import io.cryostat.net.ConnectionDescriptor;
import io.cryostat.net.TargetConnectionManager;
import io.cryostat.net.security.ResourceAction;
import io.cryostat.net.web.http.HttpMimeType;
import io.cryostat.net.web.http.api.ApiVersion;
import io.cryostat.platform.ServiceRef;
import io.cryostat.recordings.RecordingTargetHelper;
import io.cryostat.rules.Rule;
import io.cryostat.rules.RuleRegistry;

Expand All @@ -70,7 +70,7 @@ class RuleDeleteHandler extends AbstractV2RequestHandler<Void> {

private final Vertx vertx;
private final RuleRegistry ruleRegistry;
private final TargetConnectionManager targetConnectionManager;
private final RecordingTargetHelper recordings;
private final DiscoveryStorage storage;
private final CredentialsManager credentialsManager;
private final NotificationFactory notificationFactory;
Expand All @@ -81,7 +81,7 @@ class RuleDeleteHandler extends AbstractV2RequestHandler<Void> {
Vertx vertx,
AuthManager auth,
RuleRegistry ruleRegistry,
TargetConnectionManager targetConnectionManager,
RecordingTargetHelper recordings,
DiscoveryStorage storage,
CredentialsManager credentialsManager,
NotificationFactory notificationFactory,
Expand All @@ -90,7 +90,7 @@ class RuleDeleteHandler extends AbstractV2RequestHandler<Void> {
super(auth, gson);
this.vertx = vertx;
this.ruleRegistry = ruleRegistry;
this.targetConnectionManager = targetConnectionManager;
this.recordings = recordings;
this.storage = storage;
this.credentialsManager = credentialsManager;
this.notificationFactory = notificationFactory;
Expand Down Expand Up @@ -147,61 +147,34 @@ public IntermediateResponse<Void> handle(RequestParameters params) throws ApiExc
.build()
.send();
if (Boolean.valueOf(params.getQueryParams().get(CLEAN_PARAM))) {
vertx.executeBlocking(promise -> {
cleanup(rule);
promise.complete();
});
vertx.executeBlocking(
promise -> {
try {
cleanup(rule);
promise.complete();
} catch (Exception e) {
promise.fail(e);
}
});
}
return new IntermediateResponse<Void>().body(null);
}

private void cleanup(Rule rule) {
for (ServiceRef ref : storage.listDiscoverableServices()) {
vertx.<Boolean>executeBlocking(
vertx.executeBlocking(
promise -> {
try {
promise.complete(ruleRegistry.applies(rule, ref));
if (!ruleRegistry.applies(rule, ref)) {
promise.complete(null);
if (ruleRegistry.applies(rule, ref)) {
ConnectionDescriptor cd =
new ConnectionDescriptor(
ref, credentialsManager.getCredentials(ref));
recordings.stopRecording(cd, rule.getRecordingName());
}
} catch (Exception e) {
promise.fail(e);
}
},
false,
result -> {
if (result.failed()) {
logger.error(new RuntimeException(result.cause()));
return;
}
if (!result.result()) {
return;
}
try {
targetConnectionManager.executeConnectedTaskAsync(
new ConnectionDescriptor(
ref, credentialsManager.getCredentials(ref)),
conn -> {
conn.getService().getAvailableRecordings().stream()
.filter(
rec ->
rec.getName()
.equals(
rule
.getRecordingName()))
.findFirst()
.ifPresent(
r -> {
try {
conn.getService().stop(r);
} catch (Exception e) {
logger.error(e);
}
});
return null;
});
promise.complete();
} catch (Exception e) {
logger.error(e);
promise.fail(e);
}
});
}
Expand Down
120 changes: 28 additions & 92 deletions src/test/java/io/cryostat/net/web/http/api/v2/RuleDeleteHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,20 @@
import java.util.Optional;
import java.util.Set;

import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService;
import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor;

import io.cryostat.MainModule;
import io.cryostat.MockVertx;
import io.cryostat.configuration.CredentialsManager;
import io.cryostat.core.FlightRecorderException;
import io.cryostat.core.log.Logger;
import io.cryostat.core.net.JFRConnection;
import io.cryostat.discovery.DiscoveryStorage;
import io.cryostat.messaging.notifications.Notification;
import io.cryostat.messaging.notifications.NotificationFactory;
import io.cryostat.net.AuthManager;
import io.cryostat.net.TargetConnectionManager;
import io.cryostat.net.security.ResourceAction;
import io.cryostat.net.web.http.HttpMimeType;
import io.cryostat.net.web.http.api.ApiVersion;
import io.cryostat.platform.ServiceRef;
import io.cryostat.recordings.RecordingTargetHelper;
import io.cryostat.rules.Rule;
import io.cryostat.rules.RuleRegistry;

Expand All @@ -86,7 +82,7 @@ class RuleDeleteHandlerTest {
Vertx vertx = MockVertx.vertx();
@Mock AuthManager auth;
@Mock RuleRegistry registry;
@Mock TargetConnectionManager targetConnectionManager;
@Mock RecordingTargetHelper recordingTargetHelper;
@Mock DiscoveryStorage storage;
@Mock CredentialsManager credentialsManager;
@Mock NotificationFactory notificationFactory;
Expand Down Expand Up @@ -116,7 +112,7 @@ void setup() {
vertx,
auth,
registry,
targetConnectionManager,
recordingTargetHelper,
storage,
credentialsManager,
notificationFactory,
Expand Down Expand Up @@ -206,6 +202,13 @@ void shouldRespondWith404ForNonexistentRule() throws Exception {
ApiException ex =
Assertions.assertThrows(ApiException.class, () -> handler.handle(params));
MatcherAssert.assertThat(ex.getStatusCode(), Matchers.equalTo(404));

Mockito.verify(vertx, Mockito.never()).executeBlocking(Mockito.any());
Mockito.verify(registry, Mockito.never()).deleteRule(Mockito.any(Rule.class));
Mockito.verify(registry, Mockito.never()).deleteRule(Mockito.anyString());
Mockito.verify(registry, Mockito.never()).applies(Mockito.any(), Mockito.any());
Mockito.verify(recordingTargetHelper, Mockito.never())
.stopRecording(Mockito.any(), Mockito.any());
}

@Test
Expand Down Expand Up @@ -233,12 +236,17 @@ void shouldRespondWith200ForCleanupFailures() throws Exception {

FlightRecorderException exception =
new FlightRecorderException(new Exception("test message"));
Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any()))
Mockito.when(recordingTargetHelper.stopRecording(Mockito.any(), Mockito.any()))
.thenThrow(exception);

IntermediateResponse<Void> response = handler.handle(params);
MatcherAssert.assertThat(response.getStatusCode(), Matchers.equalTo(200));

Mockito.verify(vertx, Mockito.times(2)).executeBlocking(Mockito.any());
Mockito.verify(registry).deleteRule(rule);
Mockito.verify(registry).applies(rule, serviceRef);
Mockito.verify(recordingTargetHelper)
.stopRecording(Mockito.any(), Mockito.eq(rule.getRecordingName()));
Mockito.verify(logger).error(exception);
}

Expand All @@ -257,78 +265,21 @@ void shouldRespondWith200AfterCleanupNoop() throws Exception {
.build();
Mockito.when(registry.hasRuleByName(testRuleName)).thenReturn(true);
Mockito.when(registry.getRule(testRuleName)).thenReturn(Optional.of(rule));
Mockito.when(registry.applies(Mockito.any(), Mockito.any())).thenReturn(true);

ServiceRef serviceRef =
new ServiceRef(
new URI("service:jmx:rmi:///jndi/rmi://cryostat:9091/jmxrmi"),
"io.cryostat.Cryostat");
Mockito.when(storage.listDiscoverableServices()).thenReturn(List.of(serviceRef));

JFRConnection connection = Mockito.mock(JFRConnection.class);
Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any()))
.thenAnswer(
arg0 ->
((TargetConnectionManager.ConnectedTask<Object>)
arg0.getArgument(1))
.execute(connection));

IFlightRecorderService service = Mockito.mock(IFlightRecorderService.class);
Mockito.when(connection.getService()).thenReturn(service);

Mockito.when(service.getAvailableRecordings()).thenReturn(List.of());
Mockito.when(storage.listDiscoverableServices()).thenReturn(List.of());

IntermediateResponse<Void> response = handler.handle(params);
MatcherAssert.assertThat(response.getStatusCode(), Matchers.equalTo(200));
}

@Test
void shouldRespondWith200AfterSuccessfulCleanup() throws Exception {
Mockito.when(params.getPathParams()).thenReturn(Map.of("name", testRuleName));
MultiMap queryParams = MultiMap.caseInsensitiveMultiMap();
queryParams.set("clean", "true");
Mockito.when(params.getQueryParams()).thenReturn(queryParams);

Rule rule =
new Rule.Builder()
.name(testRuleName)
.matchExpression("true")
.eventSpecifier("template=Continuous")
.build();
Mockito.when(registry.hasRuleByName(testRuleName)).thenReturn(true);
Mockito.when(registry.getRule(testRuleName)).thenReturn(Optional.of(rule));
Mockito.when(registry.applies(Mockito.any(), Mockito.any())).thenReturn(true);

ServiceRef serviceRef =
new ServiceRef(
new URI("service:jmx:rmi:///jndi/rmi://cryostat:9091/jmxrmi"),
"io.cryostat.Cryostat");
Mockito.when(storage.listDiscoverableServices()).thenReturn(List.of(serviceRef));

JFRConnection connection = Mockito.mock(JFRConnection.class);
Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any()))
.thenAnswer(
arg0 ->
((TargetConnectionManager.ConnectedTask<Object>)
arg0.getArgument(1))
.execute(connection));

IFlightRecorderService service = Mockito.mock(IFlightRecorderService.class);
Mockito.when(connection.getService()).thenReturn(service);

IRecordingDescriptor recording = Mockito.mock(IRecordingDescriptor.class);
Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(recording));
Mockito.when(recording.getName()).thenReturn(rule.getRecordingName());

IntermediateResponse<Void> response = handler.handle(params);
MatcherAssert.assertThat(response.getStatusCode(), Matchers.equalTo(200));

Mockito.verify(service).stop(recording);
Mockito.verify(service, Mockito.never()).close(recording);
Mockito.verify(vertx, Mockito.times(1)).executeBlocking(Mockito.any());
Mockito.verify(registry).deleteRule(rule);
Mockito.verify(registry, Mockito.never()).applies(Mockito.any(), Mockito.any());
Mockito.verify(recordingTargetHelper, Mockito.never())
.stopRecording(Mockito.any(), Mockito.eq(rule.getRecordingName()));
}

@Test
void shouldRespondWith200AfterUnsuccessfulCleanup() throws Exception {
void shouldRespondWith200AfterSuccessfulCleanup() throws Exception {
Mockito.when(params.getPathParams()).thenReturn(Map.of("name", testRuleName));
MultiMap queryParams = MultiMap.caseInsensitiveMultiMap();
queryParams.set("clean", "true");
Expand All @@ -350,29 +301,14 @@ void shouldRespondWith200AfterUnsuccessfulCleanup() throws Exception {
"io.cryostat.Cryostat");
Mockito.when(storage.listDiscoverableServices()).thenReturn(List.of(serviceRef));

JFRConnection connection = Mockito.mock(JFRConnection.class);
Mockito.when(targetConnectionManager.executeConnectedTask(Mockito.any(), Mockito.any()))
.thenAnswer(
arg0 ->
((TargetConnectionManager.ConnectedTask<Object>)
arg0.getArgument(1))
.execute(connection));

IFlightRecorderService service = Mockito.mock(IFlightRecorderService.class);
Mockito.when(connection.getService()).thenReturn(service);

org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException exception =
new org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException("test message");
Mockito.doThrow(exception).when(service).stop(Mockito.any());

IRecordingDescriptor recording = Mockito.mock(IRecordingDescriptor.class);
Mockito.when(service.getAvailableRecordings()).thenReturn(List.of(recording));
Mockito.when(recording.getName()).thenReturn(rule.getRecordingName());

IntermediateResponse<Void> response = handler.handle(params);
MatcherAssert.assertThat(response.getStatusCode(), Matchers.equalTo(200));

Mockito.verify(logger).error(exception);
Mockito.verify(vertx, Mockito.times(2)).executeBlocking(Mockito.any());
Mockito.verify(registry).deleteRule(rule);
Mockito.verify(registry).applies(Mockito.any(), Mockito.any());
Mockito.verify(recordingTargetHelper)
.stopRecording(Mockito.any(), Mockito.eq(rule.getRecordingName()));
}
}
}

0 comments on commit e9e3b11

Please sign in to comment.