Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(recording): backwards compatibility for restart parameter #1672

Merged
merged 2 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/HTTP_API.md
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,10 @@

**The request may include the following fields:**

`restart`: Whether to restart the recording if one already exists with the same name. **DEPRECATED**: See `replace` below.

`replace`: The replacement policy if a recording already exists with the same name. Policies can be `ALWAYS` (i.e. `restart=true`), `NEVER` (i.e.`restart=false`), and `STOPPED` (restart only when the existing one is stopped).

`duration` - The duration of the recording, in seconds.
If this field is not set, or if it is set to zero,
the recording will be continuous,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,18 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
.create(connection.getService())
.name(recordingName);

String replace = attrs.get("replace");
ReplacementPolicy replacementPolicy =
ReplacementPolicy.fromString(replace);
ReplacementPolicy replace = ReplacementPolicy.NEVER;

if (attrs.contains("restart")) {
replace =
Boolean.parseBoolean(attrs.get("restart"))
? ReplacementPolicy.ALWAYS
: ReplacementPolicy.NEVER;
}

if (attrs.contains("replace")) {
replace = ReplacementPolicy.fromString(attrs.get("replace"));
}

if (attrs.contains("duration")) {
builder =
Expand Down Expand Up @@ -201,7 +210,7 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
eventSpecifier);
IRecordingDescriptor descriptor =
recordingTargetHelper.startRecording(
replacementPolicy,
replace,
connectionDescriptor,
builder.build(),
template.getLeft(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,20 @@ public HyperlinkedSerializableRecordingDescriptor getAuthenticated(
return targetConnectionManager.executeConnectedTask(
cd,
conn -> {
ReplacementPolicy replace = ReplacementPolicy.NEVER;
RecordingOptionsBuilder builder =
recordingOptionsBuilderFactory
.create(conn.getService())
.name((String) settings.get("name"));

ReplacementPolicy replace = ReplacementPolicy.NEVER;
if (settings.containsKey("restart")) {
replace =
Boolean.TRUE.equals(settings.get("restart"))
? ReplacementPolicy.ALWAYS
: ReplacementPolicy.NEVER;
}
if (settings.containsKey("replace")) {
String replaceValue = (String) settings.get("replace");
replace = ReplacementPolicy.fromString(replaceValue);
replace = ReplacementPolicy.fromString((String) settings.get("replace"));
}
if (settings.containsKey("duration")) {
builder =
Expand Down
7 changes: 4 additions & 3 deletions src/main/resources/types.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -172,16 +172,17 @@ type RecordingMetadata {
}

input RecordingSettings {
replace: ReplacementPolicy
name: String!
template: String!
templateType: String!
duration: Long
replace: ReplacementPolicy
restart: Boolean
continuous: Boolean
archiveOnStop: Boolean
toDisk: Boolean
duration: Long
maxSize: Long
maxAge: Long
archiveOnStop: Boolean
metadata: Object
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
Expand Down Expand Up @@ -258,8 +259,9 @@ void shouldStartRecording() throws Exception {
"{\"downloadUrl\":\"example-download-url\",\"reportUrl\":\"example-report-url\",\"metadata\":{\"labels\":{}},\"archiveOnStop\":false,\"id\":1,\"name\":\"someRecording\",\"state\":\"STOPPED\",\"startTime\":0,\"duration\":0,\"continuous\":false,\"toDisk\":false,\"maxSize\":0,\"maxAge\":0}");
}

@Test
void shouldRestartRecording() throws Exception {
@ParameterizedTest
@MethodSource("getRestartOptions")
void shouldRestartRecording(String restart, String replace) throws Exception {
Mockito.when(auth.validateHttpHeader(Mockito.any(), Mockito.any()))
.thenReturn(CompletableFuture.completedFuture(true));

Expand Down Expand Up @@ -311,7 +313,12 @@ void shouldRestartRecording() throws Exception {
resp.putHeader(
Mockito.any(CharSequence.class), Mockito.any(CharSequence.class)))
.thenReturn(resp);
attrs.add("replace", "always");
if (restart != null) {
attrs.add("restart", restart);
}
if (replace != null) {
attrs.add("replace", replace);
}
attrs.add("recordingName", "someRecording");
attrs.add("events", "template=Foo");

Expand Down Expand Up @@ -373,6 +380,15 @@ void shouldRestartRecording() throws Exception {
"{\"downloadUrl\":\"example-download-url\",\"reportUrl\":\"example-report-url\",\"metadata\":{\"labels\":{}},\"archiveOnStop\":false,\"id\":1,\"name\":\"someRecording\",\"state\":\"STOPPED\",\"startTime\":0,\"duration\":0,\"continuous\":false,\"toDisk\":false,\"maxSize\":0,\"maxAge\":0}");
}

private static Stream<Arguments> getRestartOptions() {
return Stream.of(
Arguments.of("true", null),
Arguments.of(null, "always"),
Arguments.of("true", "always"),
Arguments.of("false", "always"),
Arguments.of("anything", "always"));
}

@Test
void shouldHandleNameCollision() throws Exception {
Mockito.when(auth.validateHttpHeader(Mockito.any(), Mockito.any()))
Expand Down
Loading
Loading