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

Signed-off-by: Thuan Vo <thuan.votann@gmail.com>

Author: tthvo

docs/HTTP_API.md

@@ -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,

src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostHandler.java

@@ -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 =
@@ -201,7 +210,7 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
                                                 eventSpecifier);
                                 IRecordingDescriptor descriptor =
                                         recordingTargetHelper.startRecording(
-                                                replacementPolicy,
+                                                replace,
                                                 connectionDescriptor,
                                                 builder.build(),
                                                 template.getLeft(),

src/main/java/io/cryostat/net/web/http/api/v2/graph/StartRecordingOnTargetMutator.java

@@ -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 =

src/main/resources/types.graphqls

@@ -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
 }
 

src/test/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostHandlerTest.java

@@ -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;
@@ -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));
 
@@ -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");
 
@@ -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()))