cryostatio / cryostatio/cryostat
[Story] Replace LabelSelectorMatcher with MatchExpression
- Dominant language
- Java
- Stars
- 57
- Forks
- 17
- Avg merge
- 15h 39m
- Merged PRs (30d)
- 42
Description
```diff
diff --git a/src/main/java/io/cryostat/graphql/RootNode.java b/src/main/java/io/cryostat/graphql/RootNode.java
index 4af6eef..c2facb6 100644
--- a/src/main/java/io/cryostat/graphql/RootNode.java
+++ b/src/main/java/io/cryostat/graphql/RootNode.java
@@ -21,6 +21,8 @@ import java.util.Set;
import java.util.function.Predicate;
import io.cryostat.discovery.DiscoveryNode;
+import io.cryostat.expressions.MatchExpression;
+import io.cryostat.expressions.MatchExpressionEvaluator;
import io.cryostat.graphql.matchers.LabelSelectorMatcher;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@@ -30,6 +32,7 @@ import org.eclipse.microprofile.graphql.Description;
import org.eclipse.microprofile.graphql.GraphQLApi;
import org.eclipse.microprofile.graphql.Query;
import org.eclipse.microprofile.graphql.Source;
+import org.projectnessie.cel.tools.ScriptException;
@GraphQLApi
public class RootNode {
@@ -77,6 +80,7 @@ public class RootNode {
public @Nullable List nodeTypes;
public @Nullable List labels;
public @Nullable List annotations;
+ public @Nullable List matchExpressions;
@Override
public boolean test(DiscoveryNode t) {
@@ -114,6 +118,24 @@ public class RootNode {
.annotations
.merged())));
+ MatchExpressionEvaluator evaluator = new MatchExpressionEvaluator(); // WRONG
+ Predicate matchesExpression =
+ n ->
+ matchExpressions == null
+ || (n.target != null
+ && matchExpressions.stream()
+ .map(MatchExpression::new)
+ .allMatch(
+ expr -> {
+ try {
+ return evaluator.applies(
+ expr, n.target);
+ } catch (ScriptException e) {
+ // FIXME log this
+ return false;
+ }
+ }));
+
return List.of(
matchesId,
matchesIds,
@@ -122,7 +144,8 @@ public class RootNode {
matchesNames,
matchesNodeTypes,
matchesLabels,
- matchesAnnotations)
+ matchesAnnotations,
+ matchesExpression)
.stream()
.reduce(x -> true, Predicate::and)
.test(t);
diff --git a/src/test/java/itest/GraphQLTest.java b/src/test/java/itest/GraphQLTest.java
index 3d28654..15d1b69 100644
--- a/src/test/java/itest/GraphQLTest.java
+++ b/src/test/java/itest/GraphQLTest.java
@@ -307,8 +307,8 @@ class GraphQLTest extends StandardSelfTest {
JsonObject query2 = new JsonObject();
query2.put(
"query",
- "mutation { createRecording( nodes:{annotations: ["
- + "\"REALM = Custom Targets\""
+ "mutation { createRecording( nodes:{matchExpressions: ["
+ + "\"target.annotations[\"REALM\"] == \"Custom Targets\"\""
+ "]}, recording: { name: \"test\", template:"
+ " \"Profiling\", templateType: \"TARGET\", duration: 30, continuous:"
+ " false, archiveOnStop: true, toDisk: true }) { name state duration"
```
Rough prototype of the idea. The `LabelSelectorMatcher` has a very limited syntax and is only used for matching on maps of labels and annotations. This could easily be a MatchExpression instead and use CEL syntax that is also already used for Automated Rules and Stored Credentials, instead of its own k8s-inspired syntax that only appears in GraphQL query filters.
Contributor guide
Research direction
Start with the GraphQL filtering logic in src/main/java/io/cryostat/graphql/RootNode.java, then inspect LabelSelectorMatcher, MatchExpression, and MatchExpressionEvaluator to understand the existing APIs. Review the related cases in src/test/java/itest/GraphQLTest.java and run the GraphQL tests. Done means GraphQL node filters use the shared CEL-based match-expression behavior with coverage for the shown query.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- graphql, java
- Domain
- api, backend-api-design
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100