Skip to content

Commit

Permalink
feat(matchExpression): POST matchExpression resource as json (#1603)
Browse files Browse the repository at this point in the history
  • Loading branch information
maxcao13 authored Jul 31, 2023
1 parent bf86535 commit 920c5c2
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -78,17 +77,20 @@ public class MatchExpressionsPostHandler extends AbstractV2RequestHandler<Matche
static final String PATH = "matchExpressions";

private final MatchExpressionManager expressionManager;
private final MatchExpressionEvaluator expressionEvaluator;
private final NotificationFactory notificationFactory;

@Inject
MatchExpressionsPostHandler(
AuthManager auth,
CredentialsManager credentialsManager,
MatchExpressionManager expressionManager,
MatchExpressionEvaluator expressionEvaluator,
NotificationFactory notificationFactory,
Gson gson) {
super(auth, credentialsManager, gson);
this.expressionManager = expressionManager;
this.expressionEvaluator = expressionEvaluator;
this.notificationFactory = notificationFactory;
}

Expand Down Expand Up @@ -124,7 +126,7 @@ public List<HttpMimeType> produces() {

@Override
public List<HttpMimeType> consumes() {
return List.of(HttpMimeType.MULTIPART_FORM, HttpMimeType.URLENCODED_FORM);
return List.of(HttpMimeType.JSON);
}

@Override
Expand All @@ -140,18 +142,18 @@ public boolean isOrdered() {
@Override
public IntermediateResponse<MatchedMatchExpression> 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<ServiceRef> matched;
List<ServiceRef> parsedTargets = parseTargets(targets);
matched =
RequestData requestData = gson.fromJson(params.getBody(), RequestData.class);
String matchExpression = requestData.getMatchExpression();
List<ServiceRef> targets = requestData.getTargets();
if (StringUtils.isBlank(matchExpression)) {
throw new ApiException(400, "'matchExpression' is required.");
}
expressionEvaluator.validate(matchExpression);
if (targets != null) {
Set<ServiceRef> matched =
expressionManager.resolveMatchingTargets(
matchExpression, (t) -> parsedTargets.contains(t));
matchExpression, (t) -> targets.contains(t));

return new IntermediateResponse<MatchedMatchExpression>()
.statusCode(200)
Expand All @@ -176,21 +178,29 @@ public IntermediateResponse<MatchedMatchExpression> 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);
}
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<ServiceRef> parseTargets(String targets) {
Objects.requireNonNull(targets, "Targets must not be null");
Type mapType = new TypeToken<List<ServiceRef>>() {}.getType();
List<ServiceRef> parsedTargets = gson.fromJson(targets, mapType);
return parsedTargets;
static class RequestData {
private String matchExpression;
private List<ServiceRef> targets;

String getMatchExpression() {
return matchExpression;
}

List<ServiceRef> getTargets() {
return targets;
}
}
}
12 changes: 9 additions & 3 deletions src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
*/
package io.cryostat.rules;

import java.net.URI;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -81,7 +82,7 @@ public class MatchExpressionEvaluator {
e -> {
switch (e.getEventType()) {
case REMOVED:
invalidate(e.getPayload());
invalidateCache(e.getPayload());
break;
default:
// ignore
Expand All @@ -92,7 +93,7 @@ public class MatchExpressionEvaluator {
e -> {
switch (e.getEventType()) {
case REMOVED:
invalidate(e.getPayload().getMatchExpression());
invalidateCache(e.getPayload().getMatchExpression());
break;
default:
// ignore
Expand All @@ -118,7 +119,7 @@ private boolean compute(String matchExpression, ServiceRef serviceRef) throws Sc
}
}

private void invalidate(String matchExpression) {
private void invalidateCache(String matchExpression) {
var it = cache.asMap().keySet().iterator();
while (it.hasNext()) {
Pair<String, ServiceRef> entry = it.next();
Expand All @@ -128,6 +129,11 @@ private void invalidate(String matchExpression) {
}
}

public void validate(String matchExpression) throws ScriptException {
ServiceRef dummyRef = new ServiceRef("jvmId", URI.create("file:///foo/bar"), "alias");
compute(matchExpression, dummyRef);
}

public boolean applies(String matchExpression, ServiceRef serviceRef) throws ScriptException {
Pair<String, ServiceRef> key = Pair.of(matchExpression, serviceRef);
MatchExpressionAppliesEvent evt = new MatchExpressionAppliesEvent(matchExpression);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -161,10 +167,9 @@ void shouldDelegateToMatchExpressionManager() throws Exception {
Optional<MatchExpression> 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);
Expand All @@ -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()))
Expand All @@ -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);
Expand Down
22 changes: 16 additions & 6 deletions src/test/java/io/cryostat/rules/MatchExpressionEvaluatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,16 @@ void setup() throws Exception {
this.platformAnnotations = Map.of("annotation1", "someAnnotation");
this.cryostatAnnotations = Map.of(AnnotationKey.JAVA_MAIN, "io.cryostat.Cryostat");

Mockito.when(serviceRef.getServiceUri()).thenReturn(this.serviceUri);
Mockito.when(serviceRef.getJvmId()).thenReturn(this.jvmId);
Mockito.when(serviceRef.getAlias()).thenReturn(Optional.of(this.alias));
Mockito.when(serviceRef.getLabels()).thenReturn(this.labels);
Mockito.when(serviceRef.getPlatformAnnotations()).thenReturn(this.platformAnnotations);
Mockito.when(serviceRef.getCryostatAnnotations()).thenReturn(this.cryostatAnnotations);
Mockito.lenient().when(serviceRef.getServiceUri()).thenReturn(this.serviceUri);
Mockito.lenient().when(serviceRef.getJvmId()).thenReturn(this.jvmId);
Mockito.lenient().when(serviceRef.getAlias()).thenReturn(Optional.of(this.alias));
Mockito.lenient().when(serviceRef.getLabels()).thenReturn(this.labels);
Mockito.lenient()
.when(serviceRef.getPlatformAnnotations())
.thenReturn(this.platformAnnotations);
Mockito.lenient()
.when(serviceRef.getCryostatAnnotations())
.thenReturn(this.cryostatAnnotations);
}

@Nested
Expand Down Expand Up @@ -277,5 +281,11 @@ void shouldThrowExceptionOnNonBooleanExpressionEval(String expr) throws Exceptio
Assertions.assertThrows(
ScriptException.class, () -> ruleMatcher.applies(expr, serviceRef));
}

@ParameterizedTest
@ValueSource(strings = {"1", "null", "target.alias", "\"a string\""})
void shouldThrowExceptionOnInvalidExpressionValidate(String expr) throws Exception {
Assertions.assertThrows(ScriptException.class, () -> ruleMatcher.validate(expr));
}
}
}

0 comments on commit 920c5c2

Please sign in to comment.