From 2c7b4a03b685d384d2174e3f821859704b55a825 Mon Sep 17 00:00:00 2001 From: Max Cao Date: Thu, 27 Jul 2023 17:58:26 -0700 Subject: [PATCH] post matchExpression as json --- .../api/beta/MatchExpressionsPostHandler.java | 50 +++++++++++-------- .../rules/MatchExpressionEvaluator.java | 11 ++++ .../beta/MatchExpressionsPostHandlerTest.java | 37 +++++++------- 3 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/main/java/io/cryostat/net/web/http/api/beta/MatchExpressionsPostHandler.java b/src/main/java/io/cryostat/net/web/http/api/beta/MatchExpressionsPostHandler.java index 97d1540dc3..33fb8a81b6 100644 --- a/src/main/java/io/cryostat/net/web/http/api/beta/MatchExpressionsPostHandler.java +++ b/src/main/java/io/cryostat/net/web/http/api/beta/MatchExpressionsPostHandler.java @@ -37,16 +37,15 @@ */ package io.cryostat.net.web.http.api.beta; -import java.lang.reflect.Type; import java.util.EnumSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.Set; import javax.inject.Inject; import javax.persistence.RollbackException; +import javax.script.ScriptException; import io.cryostat.configuration.CredentialsManager; import io.cryostat.messaging.notifications.NotificationFactory; @@ -60,13 +59,13 @@ import io.cryostat.net.web.http.api.v2.RequestParameters; import io.cryostat.platform.ServiceRef; import io.cryostat.rules.MatchExpression; +import io.cryostat.rules.MatchExpressionEvaluator; import io.cryostat.rules.MatchExpressionManager; import io.cryostat.rules.MatchExpressionManager.MatchedMatchExpression; import io.cryostat.rules.MatchExpressionValidationException; import com.google.gson.Gson; import com.google.gson.JsonParseException; -import com.google.gson.reflect.TypeToken; import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; import org.apache.commons.lang3.StringUtils; @@ -78,6 +77,7 @@ public class MatchExpressionsPostHandler extends AbstractV2RequestHandler produces() { @Override public List consumes() { - return List.of(HttpMimeType.MULTIPART_FORM, HttpMimeType.URLENCODED_FORM); + return List.of(HttpMimeType.JSON); } @Override @@ -140,18 +142,18 @@ public boolean isOrdered() { @Override public IntermediateResponse handle(RequestParameters params) throws ApiException { - String matchExpression = params.getFormAttributes().get("matchExpression"); - String targets = params.getFormAttributes().get("targets"); - if (StringUtils.isBlank(matchExpression)) { - throw new ApiException(400, "'matchExpression' is required."); - } try { - if (StringUtils.isNotBlank(targets)) { - Set matched; - List parsedTargets = parseTargets(targets); - matched = + RequestData requestData = gson.fromJson(params.getBody(), RequestData.class); + String matchExpression = requestData.getMatchExpression(); + List targets = requestData.getTargets(); + if (StringUtils.isBlank(matchExpression)) { + throw new ApiException(400, "'matchExpression' is required."); + } + expressionEvaluator.evaluates(matchExpression); + if (targets != null) { + Set matched = expressionManager.resolveMatchingTargets( - matchExpression, (t) -> parsedTargets.contains(t)); + matchExpression, (t) -> targets.contains(t)); return new IntermediateResponse() .statusCode(200) @@ -176,7 +178,7 @@ public IntermediateResponse handle(RequestParameters par .body(new MatchedMatchExpression(expr)); } } catch (JsonParseException e) { - throw new ApiException(400, "JSON formatting error", e); + throw new ApiException(400, "Unable to parse JSON", e); } catch (RollbackException e) { if (ExceptionUtils.indexOfType(e, ConstraintViolationException.class) >= 0) { throw new ApiException(400, "Duplicate matchExpression", e); @@ -184,13 +186,21 @@ public IntermediateResponse handle(RequestParameters par throw new ApiException(500, e); } catch (MatchExpressionValidationException e) { throw new ApiException(400, e); + } catch (ScriptException e) { + throw new ApiException(400, "Invalid matchExpression", e); } } - public List parseTargets(String targets) { - Objects.requireNonNull(targets, "Targets must not be null"); - Type mapType = new TypeToken>() {}.getType(); - List parsedTargets = gson.fromJson(targets, mapType); - return parsedTargets; + static class RequestData { + private String matchExpression; + private List targets; + + String getMatchExpression() { + return matchExpression; + } + + List getTargets() { + return targets; + } } } diff --git a/src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java b/src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java index 9c917a74d4..6e1aee8742 100644 --- a/src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java +++ b/src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java @@ -37,6 +37,8 @@ */ package io.cryostat.rules; +import java.net.URI; +import java.net.URISyntaxException; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -128,6 +130,15 @@ private void invalidate(String matchExpression) { } } + public void evaluates(String matchExpression) throws ScriptException { + try { + ServiceRef dummyRef = new ServiceRef("jvmId", new URI("file:///foo/bar"), "alias"); + compute(matchExpression, dummyRef); + } catch (URISyntaxException e) { + logger.error(e); + } + } + public boolean applies(String matchExpression, ServiceRef serviceRef) throws ScriptException { Pair key = Pair.of(matchExpression, serviceRef); MatchExpressionAppliesEvent evt = new MatchExpressionAppliesEvent(matchExpression); diff --git a/src/test/java/io/cryostat/net/web/http/api/beta/MatchExpressionsPostHandlerTest.java b/src/test/java/io/cryostat/net/web/http/api/beta/MatchExpressionsPostHandlerTest.java index 84ecab4b4d..e0e6ff72f8 100644 --- a/src/test/java/io/cryostat/net/web/http/api/beta/MatchExpressionsPostHandlerTest.java +++ b/src/test/java/io/cryostat/net/web/http/api/beta/MatchExpressionsPostHandlerTest.java @@ -56,12 +56,12 @@ import io.cryostat.net.web.http.api.v2.RequestParameters; import io.cryostat.platform.ServiceRef; import io.cryostat.rules.MatchExpression; +import io.cryostat.rules.MatchExpressionEvaluator; import io.cryostat.rules.MatchExpressionManager; import io.cryostat.rules.MatchExpressionManager.MatchedMatchExpression; import io.cryostat.rules.MatchExpressionValidationException; import com.google.gson.Gson; -import io.vertx.core.MultiMap; import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; import org.hamcrest.MatcherAssert; @@ -84,6 +84,7 @@ class MatchExpressionsPostHandlerTest { @Mock AuthManager auth; @Mock CredentialsManager credentialsManager; @Mock MatchExpressionManager expressionManager; + @Mock MatchExpressionEvaluator expressionEvaluator; @Mock NotificationFactory notificationFactory; @Mock Notification.Builder notificationBuilder; @Mock Notification notification; @@ -111,7 +112,12 @@ void setup() { Mockito.lenient().when(notificationBuilder.build()).thenReturn(notification); this.handler = new MatchExpressionsPostHandler( - auth, credentialsManager, expressionManager, notificationFactory, gson); + auth, + credentialsManager, + expressionManager, + expressionEvaluator, + notificationFactory, + gson); } @Nested @@ -161,10 +167,9 @@ void shouldDelegateToMatchExpressionManager() throws Exception { Optional opt = Mockito.mock(Optional.class); MatchExpression expr = Mockito.mock(MatchExpression.class); int id = 10; - String matchExpression = "target.alias == \"foo\""; - MultiMap form = MultiMap.caseInsensitiveMultiMap(); - form.set("matchExpression", matchExpression); - Mockito.when(requestParams.getFormAttributes()).thenReturn(form); + Mockito.when(requestParams.getBody()) + .thenReturn("{\"matchExpression\": \"target.alias == 'foo'\"}"); + String matchExpression = "target.alias == 'foo'"; Mockito.when(expressionManager.addMatchExpression(Mockito.anyString())).thenReturn(id); Mockito.when(expressionManager.get(id)).thenReturn(opt); Mockito.when(opt.get()).thenReturn(expr); @@ -184,16 +189,15 @@ void shouldDelegateToMatchExpressionManager() throws Exception { @Test void shouldDryRunWithMatchedTargets() throws Exception { - int id = 10; - String matchExpression = "target.alias == \"foo\""; - String stringifiedTargets = - "[{\"alias\":\"foo\",\"connectUrl\":\"service:jmx:rmi:///jndi/rmi://localhost:9091/jmxrmi\"}]"; - MultiMap form = MultiMap.caseInsensitiveMultiMap(); - form.set("matchExpression", matchExpression); - form.set("targets", stringifiedTargets); + String matchExpression = "target.alias == 'foo'"; + + Mockito.when(requestParams.getBody()) + .thenReturn( + "{\"matchExpression\": \"target.alias == 'foo'\", \"targets\":" + + " [{\"alias\":\"foo\",\"connectUrl\":\"service:jmx:rmi:///jndi/rmi://localhost:9091/jmxrmi\"}]}"); + ServiceRef target = Mockito.mock(ServiceRef.class); - Mockito.when(requestParams.getFormAttributes()).thenReturn(form); Mockito.when( expressionManager.resolveMatchingTargets( Mockito.anyString(), Mockito.any())) @@ -215,9 +219,8 @@ void shouldDryRunWithMatchedTargets() throws Exception { @NullAndEmptySource @ValueSource(strings = {"invalid", "==", "", " "}) void shouldRespond400IfMatchExpressionInvalid(String matchExpression) throws Exception { - MultiMap form = MultiMap.caseInsensitiveMultiMap(); - form.set("matchExpression", matchExpression); - Mockito.when(requestParams.getFormAttributes()).thenReturn(form); + Mockito.when(requestParams.getBody()) + .thenReturn(String.format("{\"matchExpression\": %s", matchExpression)); Mockito.lenient() .when(expressionManager.addMatchExpression(Mockito.anyString())) .thenThrow(MatchExpressionValidationException.class);