From aa64c3ecf4c1b0f97889dd6fe05cada155ed5266 Mon Sep 17 00:00:00 2001
From: Max Cao <macao@redhat.com>
Date: Fri, 28 Jul 2023 12:51:55 -0700
Subject: [PATCH] rename method and refactor URI creation

---
 .../api/beta/MatchExpressionsPostHandler.java |  2 +-
 .../rules/MatchExpressionEvaluator.java       | 17 +++++---------
 .../rules/MatchExpressionEvaluatorTest.java   | 22 ++++++++++++++-----
 3 files changed, 23 insertions(+), 18 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 33fb8a81b6..e85a976efd 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
@@ -149,7 +149,7 @@ public IntermediateResponse<MatchedMatchExpression> handle(RequestParameters par
             if (StringUtils.isBlank(matchExpression)) {
                 throw new ApiException(400, "'matchExpression' is required.");
             }
-            expressionEvaluator.evaluates(matchExpression);
+            expressionEvaluator.validate(matchExpression);
             if (targets != null) {
                 Set<ServiceRef> matched =
                         expressionManager.resolveMatchingTargets(
diff --git a/src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java b/src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java
index 6e1aee8742..7f48596c57 100644
--- a/src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java
+++ b/src/main/java/io/cryostat/rules/MatchExpressionEvaluator.java
@@ -38,7 +38,6 @@
 package io.cryostat.rules;
 
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -83,7 +82,7 @@ public class MatchExpressionEvaluator {
                 e -> {
                     switch (e.getEventType()) {
                         case REMOVED:
-                            invalidate(e.getPayload());
+                            invalidateCache(e.getPayload());
                             break;
                         default:
                             // ignore
@@ -94,7 +93,7 @@ public class MatchExpressionEvaluator {
                 e -> {
                     switch (e.getEventType()) {
                         case REMOVED:
-                            invalidate(e.getPayload().getMatchExpression());
+                            invalidateCache(e.getPayload().getMatchExpression());
                             break;
                         default:
                             // ignore
@@ -120,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();
@@ -130,13 +129,9 @@ 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 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 {
diff --git a/src/test/java/io/cryostat/rules/MatchExpressionEvaluatorTest.java b/src/test/java/io/cryostat/rules/MatchExpressionEvaluatorTest.java
index 94aaedefef..4e8767bb7f 100644
--- a/src/test/java/io/cryostat/rules/MatchExpressionEvaluatorTest.java
+++ b/src/test/java/io/cryostat/rules/MatchExpressionEvaluatorTest.java
@@ -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
@@ -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));
+        }
     }
 }