Skip to content

Commit

Permalink
resolved issues: restart paramter reverted
Browse files Browse the repository at this point in the history
  • Loading branch information
@aali309
aali309 committed Jul 19, 2023
1 parent 0b79434 commit 380d15d
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 35 deletions.
4 changes: 2 additions & 2 deletions src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,10 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
recordingOptionsBuilderFactory
.create(connection.getService())
.name(recordingName);
String restart = "false";
Boolean restart = false;
String replace = "never";
if (attrs.contains("restart")) {
restart = attrs.get("restart");
restart = Boolean.parseBoolean(attrs.get("restart"));
}
if (attrs.contains("replace")) {
replace = attrs.get("replace");
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/io/cryostat/net/web/http/api/v2/graph/StartRecordingOnTargetMutator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,17 @@ public HyperlinkedSerializableRecordingDescriptor getAuthenticated(
return targetConnectionManager.executeConnectedTask(
cd,
conn -> {
String restart = "false";
Boolean restart = false;
String replace = "never";
RecordingOptionsBuilder builder =
recordingOptionsBuilderFactory
.create(conn.getService())
.name((String) settings.get("name"));
if (settings.containsKey("restart")) {
restart = (String) settings.get("restart");
restart = Boolean.TRUE.equals(settings.get("restart"));
}
if (settings.containsKey("replace")) {
restart = (String) settings.get("replace");
replace = (String) settings.get("replace");
}
if (settings.containsKey("duration")) {
builder =
Expand Down
20 changes: 8 additions & 12 deletions src/main/java/io/cryostat/recordings/RecordingTargetHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public List<IRecordingDescriptor> getRecordings(ConnectionDescriptor connectionD
}

public IRecordingDescriptor startRecording(
String restart, // Deprecated: Use replace parameter instead
Boolean restart, // Deprecated: Use replace parameter instead
String replace,
ConnectionDescriptor connectionDescriptor,
IConstrainedMap<String> recordingOptions,
Expand All @@ -147,7 +147,7 @@ public IRecordingDescriptor startRecording(
boolean archiveOnStop)
throws Exception {
String recordingName = (String) recordingOptions.get(RecordingOptionsBuilder.KEY_NAME);
boolean restartRecording = shouldRestartRecording(replace, restart);
boolean restartRecording = shouldRestartRecording(restart, replace);

return targetConnectionManager.executeConnectedTask(
connectionDescriptor,
Expand All @@ -162,7 +162,7 @@ public IRecordingDescriptor startRecording(
String.format(
"Recording with name \"%s\" already exists",
recordingName));
} else {
} else if (isRecordingStopped(previous.get())) {
connection.getService().close(previous.get());
}
}
Expand Down Expand Up @@ -207,31 +207,27 @@ public IRecordingDescriptor startRecording(
});
}

private boolean shouldRestartRecording(String replace, String restart) {
private boolean shouldRestartRecording(Boolean restart, String replace) {
if (replace != null) {
switch (replace) {
case "always":
return true;
case "stopped":
// Check if the recording is stopped
boolean isStopped = isRecordingStopped(replace);
return isStopped;
return restart != null && restart.equals(true);
case "never":
return false;
}
}
// Handle restart parameter (deprecated)
if (restart != null) {
boolean isStopped = isRecordingStopped(restart);
return isStopped;
return restart.equals(true);
}
// if neither restart nor replace is specified, default to never
return false;
}

private boolean isRecordingStopped(String restart) {
// If restart is null or "true", consider the recording as stopped
return restart != null && restart.equals("true");
private boolean isRecordingStopped(IRecordingDescriptor recording) {
return recording.getState() == IRecordingDescriptor.RecordingState.STOPPED;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/io/cryostat/rules/RuleProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ private void startRuleRecording(ConnectionDescriptor connectionDescriptor, Rule
RecordingTargetHelper.parseEventSpecifierToTemplate(
rule.getEventSpecifier());
return recordingTargetHelper.startRecording(
"true",
true,
"always",
connectionDescriptor,
builder.build(),
Expand Down
20 changes: 10 additions & 10 deletions src/test/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ void shouldStartRecording() throws Exception {
IRecordingDescriptor descriptor = createDescriptor("someRecording");
Mockito.when(
recordingTargetHelper.startRecording(
Mockito.any(),
Mockito.anyBoolean(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Expand All @@ -225,7 +225,7 @@ void shouldStartRecording() throws Exception {
Mockito.verify(recordingOptionsBuilder).maxAge(50L);
Mockito.verify(recordingOptionsBuilder).maxSize(64L);

ArgumentCaptor<String> restartCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);

ArgumentCaptor<String> replaceCaptor = ArgumentCaptor.forClass(String.class);

Expand Down Expand Up @@ -255,7 +255,7 @@ void shouldStartRecording() throws Exception {
metadataCaptor.capture(),
archiveOnStopCaptor.capture());

MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo("false"));
MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo(false));

MatcherAssert.assertThat(replaceCaptor.getValue(), Matchers.equalTo("never"));

Expand Down Expand Up @@ -314,7 +314,7 @@ void shouldRestartRecording() throws Exception {
IRecordingDescriptor descriptor = createDescriptor("someRecording");
Mockito.when(
recordingTargetHelper.startRecording(
Mockito.any(),
Mockito.anyBoolean(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Expand Down Expand Up @@ -346,7 +346,7 @@ void shouldRestartRecording() throws Exception {

Mockito.verify(recordingOptionsBuilder).name("someRecording");

ArgumentCaptor<String> restartCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);

ArgumentCaptor<String> replaceCaptor = ArgumentCaptor.forClass(String.class);

Expand Down Expand Up @@ -376,7 +376,7 @@ void shouldRestartRecording() throws Exception {
metadataCaptor.capture(),
archiveOnStopCaptor.capture());

MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo("true"));
MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo(true));

MatcherAssert.assertThat(replaceCaptor.getValue(), Matchers.equalTo("always"));

Expand Down Expand Up @@ -425,7 +425,7 @@ void shouldHandleNameCollision() throws Exception {
Mockito.when(recordingOptionsBuilder.build()).thenReturn(recordingOptions);
Mockito.when(
recordingTargetHelper.startRecording(
Mockito.any(),
Mockito.anyBoolean(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Expand Down Expand Up @@ -589,7 +589,7 @@ void shouldStartRecordingAndArchiveOnStop() throws Exception {
IRecordingDescriptor descriptor = createDescriptor("someRecording");
Mockito.when(
recordingTargetHelper.startRecording(
Mockito.any(),
Mockito.anyBoolean(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Expand All @@ -610,7 +610,7 @@ void shouldStartRecordingAndArchiveOnStop() throws Exception {
Mockito.verify(recordingOptionsBuilder).maxAge(50L);
Mockito.verify(recordingOptionsBuilder).maxSize(64L);

ArgumentCaptor<String> restartCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);

ArgumentCaptor<String> replaceCaptor = ArgumentCaptor.forClass(String.class);

Expand Down Expand Up @@ -640,7 +640,7 @@ void shouldStartRecordingAndArchiveOnStop() throws Exception {
metadataCaptor.capture(),
archiveOnStopCaptor.capture());

MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo("false"));
MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo(false));

MatcherAssert.assertThat(replaceCaptor.getValue(), Matchers.equalTo("never"));

Expand Down
2 changes: 1 addition & 1 deletion src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ public Future<Metadata> answer(InvocationOnMock invocation)
});

recordingTargetHelper.startRecording(
"false",
false,
"never",
connectionDescriptor,
recordingOptions,
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/io/cryostat/rules/RuleProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
import org.apache.commons.lang3.tuple.Pair;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -209,7 +210,7 @@ void testSuccessfulRuleActivationWithCredentials() throws Exception {
Mockito.verify(recordingOptionsBuilder).maxAge(30);
Mockito.verify(recordingOptionsBuilder).maxSize(1234);

ArgumentCaptor<String> restartCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);

ArgumentCaptor<String> replaceCaptor = ArgumentCaptor.forClass(String.class);

Expand Down Expand Up @@ -239,7 +240,7 @@ void testSuccessfulRuleActivationWithCredentials() throws Exception {
metadataCaptor.capture(),
archiveOnStopCaptor.capture());

assertEquals("true", restartCaptor.getValue());
Assertions.assertTrue(restartCaptor.getValue());

ConnectionDescriptor connectionDescriptor = connectionDescriptorCaptor.getValue();
MatcherAssert.assertThat(
Expand Down Expand Up @@ -465,7 +466,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
Mockito.verify(recordingOptionsBuilder).maxAge(30);
Mockito.verify(recordingOptionsBuilder).maxSize(1234);

ArgumentCaptor<String> restartCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);

ArgumentCaptor<String> replaceCaptor = ArgumentCaptor.forClass(String.class);

Expand Down Expand Up @@ -495,8 +496,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable {
metadataCaptor.capture(),
archiveOnStopCaptor.capture());

assertEquals("true", restartCaptor.getValue());

Assertions.assertTrue(restartCaptor.getValue());
IConstrainedMap<String> actualRecordingOptions = recordingOptionsCaptor.getValue();
MatcherAssert.assertThat(actualRecordingOptions, Matchers.sameInstance(recordingOptions));

Expand Down Expand Up @@ -536,7 +536,7 @@ void testSuccessfulRuleNonActivationWithCredentials() throws Exception {

Mockito.verify(recordingOptionsBuilder, never()).name("auto_Test_Rule");

ArgumentCaptor<String> restartCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);

ArgumentCaptor<String> replaceCaptor = ArgumentCaptor.forClass(String.class);

Expand Down

0 comments on commit 380d15d

Please sign in to comment.