diff --git a/HTTP_API.md b/HTTP_API.md index 1ab04b391c..7ff51d662b 100644 --- a/HTTP_API.md +++ b/HTTP_API.md @@ -686,15 +686,15 @@ **The request must include the following fields:** `toDisk` - Whether a recording is stored to disk; - either `true` or `false`. + either `true` or `false`, or `unset` to restore the JVM default. **The request may include the following fields:** - `maxAge` - The maximum event age of a recording, in seconds. - A value of zero means there is no maximum event age. + `maxAge` - The maximum event age of a recording, in seconds as a positive + integer, or `unset` to restore the JVM default. - `maxSize` - The maximum size of a recording, in bytes. - A value of zero means there is no maximum recording size. + `maxSize` - The maximum size of a recording, in bytes as a positive integer, + or `unset` to restore the JVM default. ###### response `200` - The body is the updated default recording options of the @@ -721,8 +721,8 @@ ###### example ``` - $ curl -X PATCH --data "toDisk=true&maxAge=0" localhost:8181/api/v1/targets/localhost/recordingOptions - {"maxAge":0,"toDisk":true,"maxSize":0} + $ curl -X PATCH --data "toDisk=unset&maxAge=60&maxSize=1024" localhost:8181/api/v1/targets/localhost/recordingOptions + {"maxAge":60,"toDisk":"unset","maxSize":1024} ``` diff --git a/pom.xml b/pom.xml index e424e74d09..414fd39113 100644 --- a/pom.xml +++ b/pom.xml @@ -484,6 +484,7 @@ ${org.apache.maven.plugins.surefire.version} ${skipUTs} + false diff --git a/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchHandler.java b/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchHandler.java index 6ba1c0d8fb..cd261fbc56 100644 --- a/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchHandler.java @@ -68,6 +68,7 @@ class TargetRecordingOptionsPatchHandler extends AbstractAuthenticatedRequestHandler { static final String PATH = TargetRecordingOptionsGetHandler.PATH; + private static final String UNSET_KEYWORD = "unset"; private final RecordingOptionsCustomizer customizer; private final TargetConnectionManager connectionManager; private final RecordingOptionsBuilderFactory recordingOptionsBuilderFactory; @@ -114,7 +115,7 @@ public boolean isAsync() { @Override public void handleAuthenticated(RoutingContext ctx) throws Exception { - Pattern bool = Pattern.compile("true|false"); + Pattern bool = Pattern.compile("true|false|" + UNSET_KEYWORD); MultiMap attrs = ctx.request().formAttributes(); if (attrs.contains("toDisk")) { Matcher m = bool.matcher(attrs.get("toDisk")); @@ -125,8 +126,12 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { key -> { if (attrs.contains(key)) { try { - Long.parseLong(attrs.get(key)); - } catch (Exception e) { + String v = attrs.get(key); + if (UNSET_KEYWORD.equals(v)) { + return; + } + Long.parseLong(v); + } catch (NumberFormatException e) { throw new HttpStatusException(400, "Invalid options"); } } @@ -139,13 +144,14 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception { .forEach( key -> { if (attrs.contains(key)) { - OptionKey.fromOptionName(key) - .ifPresent( - optionKey -> - customizer.set( - optionKey, - attrs.get( - key))); + String v = attrs.get(key); + OptionKey optionKey = + OptionKey.fromOptionName(key).get(); + if (UNSET_KEYWORD.equals(v)) { + customizer.unset(optionKey); + } else { + customizer.set(optionKey, v); + } } }); diff --git a/src/test/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchHandlerTest.java index 2d0bcf61d0..801ab98b26 100644 --- a/src/test/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/v1/TargetRecordingOptionsPatchHandlerTest.java @@ -115,16 +115,16 @@ void shouldHaveExpectedRequiredPermissions() { @Test void shouldSetRecordingOptions() throws Exception { - Map defaultValues = + Map originalValues = Map.of("toDisk", "true", "maxAge", "50", "maxSize", "32"); Mockito.when(recordingOptionsBuilderFactory.create(Mockito.any())).thenReturn(builder); Mockito.when(builder.build()).thenReturn(recordingOptions); - Mockito.when(recordingOptions.get("toDisk")).thenReturn(defaultValues.get("toDisk")); - Mockito.when(recordingOptions.get("maxAge")).thenReturn(defaultValues.get("maxAge")); - Mockito.when(recordingOptions.get("maxSize")).thenReturn(defaultValues.get("maxSize")); + Mockito.when(recordingOptions.get("toDisk")).thenReturn(originalValues.get("toDisk")); + Mockito.when(recordingOptions.get("maxAge")).thenReturn(originalValues.get("maxAge")); + Mockito.when(recordingOptions.get("maxSize")).thenReturn(originalValues.get("maxSize")); MultiMap requestAttrs = MultiMap.caseInsensitiveMultiMap(); - requestAttrs.addAll(defaultValues); + requestAttrs.addAll(originalValues); Mockito.when( connectionManager.executeConnectedTask( @@ -159,11 +159,57 @@ public Map answer(InvocationOnMock args) throws Throwable { } } + @Test + void shouldUnsetRecordingOptions() throws Exception { + Map originalValues = + Map.of("toDisk", "true", "maxAge", "50", "maxSize", "32"); + Mockito.when(recordingOptionsBuilderFactory.create(Mockito.any())).thenReturn(builder); + Mockito.when(builder.build()).thenReturn(recordingOptions); + Mockito.when(recordingOptions.get("toDisk")).thenReturn(originalValues.get("toDisk")); + Mockito.when(recordingOptions.get("maxAge")).thenReturn(originalValues.get("maxAge")); + Mockito.when(recordingOptions.get("maxSize")).thenReturn(originalValues.get("maxSize")); + + MultiMap requestAttrs = MultiMap.caseInsensitiveMultiMap(); + requestAttrs.addAll(Map.of("toDisk", "unset", "maxAge", "unset", "maxSize", "unset")); + + Mockito.when( + connectionManager.executeConnectedTask( + Mockito.any(ConnectionDescriptor.class), Mockito.any())) + .thenAnswer( + new Answer<>() { + @Override + public Map answer(InvocationOnMock args) throws Throwable { + TargetConnectionManager.ConnectedTask ct = + (TargetConnectionManager.ConnectedTask) + args.getArguments()[1]; + return (Map) ct.execute(jfrConnection); + } + }); + + RoutingContext ctx = Mockito.mock(RoutingContext.class); + Mockito.when(ctx.pathParam("targetId")).thenReturn("foo:9091"); + HttpServerRequest req = Mockito.mock(HttpServerRequest.class); + Mockito.when(ctx.request()).thenReturn(req); + Mockito.when(req.headers()).thenReturn(MultiMap.caseInsensitiveMultiMap()); + Mockito.when(req.formAttributes()).thenReturn(requestAttrs); + HttpServerResponse resp = Mockito.mock(HttpServerResponse.class); + Mockito.when(ctx.response()).thenReturn(resp); + IFlightRecorderService service = Mockito.mock(IFlightRecorderService.class); + Mockito.when(jfrConnection.getService()).thenReturn(service); + + handler.handleAuthenticated(ctx); + + for (var entry : requestAttrs.entries()) { + var key = OptionKey.fromOptionName(entry.getKey()); + Mockito.verify(customizer).unset(key.get()); + } + } + @ParameterizedTest @MethodSource("getRequestMaps") - void shouldThrowInvalidOptionException(Map defaultValues) throws Exception { + void shouldThrowInvalidOptionException(Map values) throws Exception { MultiMap requestAttrs = MultiMap.caseInsensitiveMultiMap(); - requestAttrs.addAll(defaultValues); + requestAttrs.addAll(values); RoutingContext ctx = Mockito.mock(RoutingContext.class); HttpServerRequest req = Mockito.mock(HttpServerRequest.class); diff --git a/src/test/java/itest/TargetRecordingOptionsIT.java b/src/test/java/itest/TargetRecordingOptionsIT.java index a5848fd4f5..218b78d6d7 100644 --- a/src/test/java/itest/TargetRecordingOptionsIT.java +++ b/src/test/java/itest/TargetRecordingOptionsIT.java @@ -69,9 +69,9 @@ public class TargetRecordingOptionsIT extends StandardSelfTest { static void resetDefaultRecordingOptions() throws Exception { CompletableFuture dumpResponse = new CompletableFuture<>(); MultiMap form = MultiMap.caseInsensitiveMultiMap(); - form.add("maxAge", "0"); - form.add("toDisk", "false"); - form.add("maxSize", "0"); + form.add("maxAge", "unset"); + form.add("toDisk", "unset"); + form.add("maxSize", "unset"); webClient .patch(OPTIONS_REQ_URL)