diff --git a/api/src/main/java/org/apache/flink/agents/api/EventType.java b/api/src/main/java/org/apache/flink/agents/api/EventType.java index f486a6d0e..7724d45e8 100644 --- a/api/src/main/java/org/apache/flink/agents/api/EventType.java +++ b/api/src/main/java/org/apache/flink/agents/api/EventType.java @@ -18,6 +18,8 @@ package org.apache.flink.agents.api; +import java.util.Map; + /** * Compile-time constants for built-in event types, sourced from each {@code XxxEvent.EVENT_TYPE}. * @@ -40,5 +42,23 @@ public final class EventType { public static final String ContextRetrievalResponseEvent = org.apache.flink.agents.api.event.ContextRetrievalResponseEvent.EVENT_TYPE; + private static final Map ALL_CONSTANTS = + Map.of( + "InputEvent", InputEvent, + "OutputEvent", OutputEvent, + "ChatRequestEvent", ChatRequestEvent, + "ChatResponseEvent", ChatResponseEvent, + "ToolRequestEvent", ToolRequestEvent, + "ToolResponseEvent", ToolResponseEvent, + "ContextRetrievalRequestEvent", ContextRetrievalRequestEvent, + "ContextRetrievalResponseEvent", ContextRetrievalResponseEvent); + + /** + * Returns the built-in event type constants as a name-to-value map for condition expressions. + */ + public static Map allConstants() { + return ALL_CONSTANTS; + } + private EventType() {} } diff --git a/api/src/main/java/org/apache/flink/agents/api/agents/Agent.java b/api/src/main/java/org/apache/flink/agents/api/agents/Agent.java index 18a7869b7..1f0377c5a 100644 --- a/api/src/main/java/org/apache/flink/agents/api/agents/Agent.java +++ b/api/src/main/java/org/apache/flink/agents/api/agents/Agent.java @@ -29,6 +29,7 @@ import java.lang.reflect.Method; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; /** Base class for defining agent logic. */ @@ -42,7 +43,7 @@ public Agent() { for (ResourceType type : ResourceType.values()) { this.resources.put(type, new HashMap<>()); } - this.actions = new HashMap<>(); + this.actions = new LinkedHashMap<>(); } public Map>> getActions() { @@ -56,8 +57,8 @@ public Map> getResources() { /** * Add action to agent. * - * @param triggerConditions Trigger condition strings — each is either an event-type name or a - * future condition-expression form. + * @param triggerConditions Raw event-type names or Boolean conditions combined with OR + * semantics. Shape and expression validation occur during {@code AgentPlan} construction. * @param method The method of this action, should be static method. * @param config The optional config can be used by this action. */ @@ -70,8 +71,8 @@ public Agent addAction( /** * Add action to agent. * - * @param triggerConditions Trigger condition strings — each is either an event-type name or a - * future condition-expression form. + * @param triggerConditions Raw event-type names or Boolean conditions combined with OR + * semantics. Shape and expression validation occur during {@code AgentPlan} construction. * @param method The method of this action, should be static method. */ public Agent addAction(String[] triggerConditions, Method method) { @@ -82,8 +83,8 @@ public Agent addAction(String[] triggerConditions, Method method) { * Add action to agent. * * @param name The action name. Must be unique within this agent. - * @param triggerConditions Trigger condition strings — each is either an event-type name or a - * future condition-expression form. + * @param triggerConditions Raw event-type names or Boolean conditions combined with OR + * semantics. Shape and expression validation occur during {@code AgentPlan} construction. * @param function The api-layer function descriptor; will be promoted to a plan-layer * executable at {@code AgentPlan} construction. * @param config Optional config for this action. diff --git a/api/src/main/java/org/apache/flink/agents/api/annotation/Action.java b/api/src/main/java/org/apache/flink/agents/api/annotation/Action.java index 49084255b..256879bf5 100644 --- a/api/src/main/java/org/apache/flink/agents/api/annotation/Action.java +++ b/api/src/main/java/org/apache/flink/agents/api/annotation/Action.java @@ -24,46 +24,31 @@ import java.lang.annotation.Target; /** - * Marks a method as an agent action triggered by matching events. + * Marks a Java method as an agent action triggered by event types or condition expressions. * - *

Each {@link #value()} entry is an event type name string. Use the {@code EVENT_TYPE} constants - * on built-in event classes, the {@link org.apache.flink.agents.api.EventType} constants, or plain - * strings for custom events. Multiple entries combine with OR. + *

Each {@link #value()} entry is either an exact event-type name or a condition expression that + * evaluates to boolean. Multiple entries use OR semantics. An entry matching the event-type syntax + * is classified as an event type rather than as a condition expression. To combine an event-type + * restriction and an attribute predicate using AND, place both in a single expression, for example + * {@code type == EventType.InputEvent && score > 5}. * - *

{@code
- * // Built-in event type via the EventType constant
- * @Action(EventType.InputEvent)
+ * 

The event-type syntax is a bare or dotted name, such as {@code order.created} or {@code + * order-created}; each segment may contain hyphens. Boolean attribute conditions must therefore be + * explicit, for example {@code ready == true}. * - * // Equivalent via the legacy class constant - * @Action(InputEvent.EVENT_TYPE) + *

The API preserves entries as raw strings. Entries are classified and condition expressions are + * validated when the agent plan is built. * - * // User-defined event type - * @Action("MyCustomEvent") - * - * // Multiple types (OR semantics) - * @Action({EventType.InputEvent, "MyCustomEvent"}) - * }

- * - *

For a cross-language action, set {@link #target()} to a {@link PythonFunction} with a - * non-empty {@code module}. The annotated Java body is never invoked — throw {@link - * UnsupportedOperationException} so direct calls outside the framework fail loud: - * - *

{@code
- * @Action(
- *     value = EventType.InputEvent,
- *     target = @PythonFunction(module = "my_pkg.handlers", qualname = "handle_input"))
- * public void handleInput(Event event, RunnerContext ctx) {
- *     throw new UnsupportedOperationException("cross-language stub");
- * }
- * }
+ *

Set {@link #target()} to a {@link PythonFunction} configured with a module for a + * cross-language action. The annotated Java method is not invoked for such actions. */ @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) public @interface Action { /** - * Event type name strings; multiple entries have OR semantics. Named {@code value} (not {@code - * triggerConditions}) to enable the {@code @Action({...})} shorthand (JLS §9.7.3); corresponds - * to Python's {@code *trigger_conditions}. + * Raw event-type names or explicit Boolean conditions combined with OR semantics. Named {@code + * value} to enable the {@code @Action({...})} shorthand (JLS §9.7.3); corresponds to Python's + * {@code *trigger_conditions}. */ String[] value(); diff --git a/api/src/main/java/org/apache/flink/agents/api/configuration/AgentConfigOptions.java b/api/src/main/java/org/apache/flink/agents/api/configuration/AgentConfigOptions.java index c39997da1..18ecc170f 100644 --- a/api/src/main/java/org/apache/flink/agents/api/configuration/AgentConfigOptions.java +++ b/api/src/main/java/org/apache/flink/agents/api/configuration/AgentConfigOptions.java @@ -25,6 +25,12 @@ /** The set of configuration options for agents parameters. */ public class AgentConfigOptions { + /** Behaviour when a trigger condition throws or returns a non-Boolean at evaluation time. */ + public enum ConditionEvaluationFailureStrategy { + WARN_AND_SKIP, + FAIL + } + /** * The config parameter specifies which event logger implementation to use. Defaults to {@link * LoggerType#SLF4J}, which surfaces events in Flink's Web UI; setting {@link LoggerType#FILE} @@ -33,6 +39,17 @@ public class AgentConfigOptions { public static final ConfigOption EVENT_LOGGER_TYPE = new ConfigOption<>("eventLoggerType", LoggerType.class, LoggerType.SLF4J); + /** + * Specifies how condition evaluation failures are handled. Defaults to {@code WARN_AND_SKIP}; + * use {@code FAIL} to enforce fail-fast behavior. + */ + public static final ConfigOption + CONDITION_EVALUATION_FAILURE_STRATEGY = + new ConfigOption<>( + "action.trigger-condition.evaluate-failure-strategy", + ConditionEvaluationFailureStrategy.class, + ConditionEvaluationFailureStrategy.WARN_AND_SKIP); + /** The config parameter specifies the directory for the FileEvent file. */ public static final ConfigOption BASE_LOG_DIR = new ConfigOption<>("baseLogDir", String.class, null); diff --git a/api/src/main/java/org/apache/flink/agents/api/yaml/YamlLoader.java b/api/src/main/java/org/apache/flink/agents/api/yaml/YamlLoader.java index 0a33b4e75..d7b670d60 100644 --- a/api/src/main/java/org/apache/flink/agents/api/yaml/YamlLoader.java +++ b/api/src/main/java/org/apache/flink/agents/api/yaml/YamlLoader.java @@ -321,27 +321,31 @@ public static void loadYaml(AgentsExecutionEnvironment env, List paths) { for (Path path : paths) { LoadedFile loaded = buildAgents(path); - // Resolve shared-action string refs first so a bad ref doesn't leave partial state on - // the env. for (Map.Entry entry : loaded.getAgents().entrySet()) { AgentSpec spec = loaded.getAgentSpecs().get(entry.getKey()); + List orderedActions = new ArrayList<>(); for (AgentActionRef ref : spec.getActions()) { - if (!ref.isReference()) { - continue; + ActionSpec actionSpec = ref.getSpec(); + if (ref.isReference()) { + actionSpec = loaded.getSharedActions().get(ref.getReference()); + if (actionSpec == null) { + throw new IllegalArgumentException( + "Agent '" + + entry.getKey() + + "' references shared action '" + + ref.getReference() + + "' in " + + path + + ", but no shared action with that name is defined at" + + " the file level."); + } } - ActionSpec shared = loaded.getSharedActions().get(ref.getReference()); - if (shared == null) { - throw new IllegalArgumentException( - "Agent '" - + entry.getKey() - + "' references shared action '" - + ref.getReference() - + "' in " - + path - + ", but no shared action with that name is defined at" - + " the file level."); - } - addActionToAgent(entry.getValue(), shared); + orderedActions.add(actionSpec); + } + + entry.getValue().getActions().clear(); + for (ActionSpec actionSpec : orderedActions) { + addActionToAgent(entry.getValue(), actionSpec); } } @@ -427,7 +431,10 @@ static Function resolveActionFunction(ActionSpec action) { return resolveFunction(action.getName(), action.getFunction(), language, paramTypes); } - /** Register an action on the agent with event aliases resolved. */ + /** + * Registers an action, replacing trigger conditions that exactly match built-in event aliases + * while leaving expression conditions unchanged. + */ static void addActionToAgent(Agent agent, ActionSpec action) { Function fn = resolveActionFunction(action); String[] triggerConditions = diff --git a/api/src/main/java/org/apache/flink/agents/api/yaml/spec/ActionSpec.java b/api/src/main/java/org/apache/flink/agents/api/yaml/spec/ActionSpec.java index 7a0280be5..e04c539f9 100644 --- a/api/src/main/java/org/apache/flink/agents/api/yaml/spec/ActionSpec.java +++ b/api/src/main/java/org/apache/flink/agents/api/yaml/spec/ActionSpec.java @@ -21,16 +21,18 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.JsonNode; import org.apache.flink.agents.api.yaml.Language; +import java.util.ArrayList; import java.util.List; import java.util.Map; /** * Action referencing a user function plus its trigger conditions. * - *

Each entry in {@code trigger_conditions} is either an event-type name (bare identifier) or a - * future condition-expression form — the runtime classifies the string when it loads the plan. + *

Each entry in {@code trigger_conditions} is either an event-type name or a condition + * expression. Entries are classified and validated during {@code AgentPlan} construction. */ @JsonIgnoreProperties(ignoreUnknown = false) public final class ActionSpec { @@ -41,15 +43,25 @@ public final class ActionSpec { private final Language type; @JsonCreator - public ActionSpec( + private ActionSpec( @JsonProperty(value = "name", required = true) String name, @JsonProperty("function") String function, @JsonProperty(value = "trigger_conditions", required = true) - List triggerConditions, + JsonNode triggerConditionsNode, @JsonProperty("config") Map config, @JsonProperty("type") Language type) { + this(name, function, parseTriggerConditions(name, triggerConditionsNode), config, type); + } + + public ActionSpec( + String name, + String function, + List triggerConditions, + Map config, + Language type) { if (triggerConditions == null || triggerConditions.isEmpty()) { - throw new IllegalArgumentException("trigger_conditions must not be empty"); + throw new IllegalArgumentException( + "'trigger_conditions' is required and must contain at least one entry"); } this.name = name; this.function = function; @@ -58,6 +70,32 @@ public ActionSpec( this.type = type; } + private static List parseTriggerConditions( + String actionName, JsonNode triggerConditionsNode) { + if (triggerConditionsNode == null || triggerConditionsNode.isNull()) { + return null; + } + if (!triggerConditionsNode.isArray()) { + throw new IllegalArgumentException("'trigger_conditions' must be an array"); + } + + List triggerConditions = new ArrayList<>(triggerConditionsNode.size()); + for (int index = 0; index < triggerConditionsNode.size(); index++) { + JsonNode entry = triggerConditionsNode.get(index); + if (entry == null || entry.isNull()) { + triggerConditions.add(null); + } else if (entry.isTextual()) { + triggerConditions.add(entry.textValue()); + } else { + throw new IllegalArgumentException( + String.format( + "'trigger_conditions' entry #%d for action '%s' must be a string, but found %s", + index + 1, actionName, entry.getNodeType())); + } + } + return triggerConditions; + } + public String getName() { return name; } diff --git a/api/src/test/java/org/apache/flink/agents/api/EventTypeTest.java b/api/src/test/java/org/apache/flink/agents/api/EventTypeTest.java index 4ac5aa728..4b7a07b18 100644 --- a/api/src/test/java/org/apache/flink/agents/api/EventTypeTest.java +++ b/api/src/test/java/org/apache/flink/agents/api/EventTypeTest.java @@ -26,22 +26,29 @@ import org.apache.flink.agents.api.event.ToolResponseEvent; import org.junit.jupiter.api.Test; +import java.util.Map; + import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; /** Tests for {@link EventType}. */ class EventTypeTest { @Test - void builtInConstantsMatchEventClassConstants() { - assertEquals(InputEvent.EVENT_TYPE, EventType.InputEvent); - assertEquals(OutputEvent.EVENT_TYPE, EventType.OutputEvent); - assertEquals(ChatRequestEvent.EVENT_TYPE, EventType.ChatRequestEvent); - assertEquals(ChatResponseEvent.EVENT_TYPE, EventType.ChatResponseEvent); - assertEquals(ToolRequestEvent.EVENT_TYPE, EventType.ToolRequestEvent); - assertEquals(ToolResponseEvent.EVENT_TYPE, EventType.ToolResponseEvent); - assertEquals( - ContextRetrievalRequestEvent.EVENT_TYPE, EventType.ContextRetrievalRequestEvent); + void allConstantsProvidesAnUnmodifiableNameToValueMap() { assertEquals( - ContextRetrievalResponseEvent.EVENT_TYPE, EventType.ContextRetrievalResponseEvent); + Map.of( + "InputEvent", InputEvent.EVENT_TYPE, + "OutputEvent", OutputEvent.EVENT_TYPE, + "ChatRequestEvent", ChatRequestEvent.EVENT_TYPE, + "ChatResponseEvent", ChatResponseEvent.EVENT_TYPE, + "ToolRequestEvent", ToolRequestEvent.EVENT_TYPE, + "ToolResponseEvent", ToolResponseEvent.EVENT_TYPE, + "ContextRetrievalRequestEvent", ContextRetrievalRequestEvent.EVENT_TYPE, + "ContextRetrievalResponseEvent", ContextRetrievalResponseEvent.EVENT_TYPE), + EventType.allConstants()); + assertThrows( + UnsupportedOperationException.class, + () -> EventType.allConstants().put("custom", "custom")); } } diff --git a/api/src/test/java/org/apache/flink/agents/api/agents/AgentAddActionTest.java b/api/src/test/java/org/apache/flink/agents/api/agents/AgentAddActionTest.java index 46d7ea0f9..80d728f6b 100644 --- a/api/src/test/java/org/apache/flink/agents/api/agents/AgentAddActionTest.java +++ b/api/src/test/java/org/apache/flink/agents/api/agents/AgentAddActionTest.java @@ -18,10 +18,9 @@ package org.apache.flink.agents.api.agents; -import org.apache.flink.agents.api.function.Function; +import org.apache.flink.agents.api.InputEvent; import org.apache.flink.agents.api.function.JavaFunction; import org.apache.flink.agents.api.function.PythonFunction; -import org.apache.flink.api.java.tuple.Tuple3; import org.junit.jupiter.api.Test; import java.lang.reflect.Method; @@ -35,17 +34,19 @@ class AgentAddActionTest { public static void onInput(Object event, Object ctx) {} @Test - void newFunctionOverloadStoresApiFunction() { + void apiDefersSelectorValidation() { Agent agent = new Agent(); PythonFunction pf = new PythonFunction("pkg", "fn"); - agent.addAction("act", new String[] {"_input_event"}, pf, Map.of("k", "v")); - - Map>> actions = agent.getActions(); - Tuple3> entry = actions.get("act"); - assertThat(entry).isNotNull(); - assertThat(entry.f0).containsExactly("_input_event"); - assertThat(entry.f1).isSameAs(pf); - assertThat(entry.f2).containsEntry("k", "v"); + String[] rawEntries = { + InputEvent.EVENT_TYPE, "\"order-created\"", "ready == true", "type ==" + }; + Map config = Map.of("k", "v"); + agent.addAction("act", rawEntries, pf, config); + + var definition = agent.getActions().get("act"); + assertThat(definition.f0).containsExactly(rawEntries); + assertThat(definition.f1).isSameAs(pf); + assertThat(definition.f2).isEqualTo(config); } @Test @@ -55,9 +56,9 @@ void methodOverloadDelegatesToFunctionAsJavaFunction() throws Exception { Agent agent = new Agent(); agent.addAction(new String[] {"_input_event"}, m); - Tuple3> entry = agent.getActions().get("onInput"); - assertThat(entry.f1).isInstanceOf(JavaFunction.class); - JavaFunction jf = (JavaFunction) entry.f1; + var definition = agent.getActions().get("onInput"); + assertThat(definition.f1).isInstanceOf(JavaFunction.class); + JavaFunction jf = (JavaFunction) definition.f1; assertThat(jf.getQualName()).isEqualTo(AgentAddActionTest.class.getName()); assertThat(jf.getMethodName()).isEqualTo("onInput"); } @@ -90,9 +91,9 @@ void javaFunctionDescriptorStoredAsIs() { agent.addAction("act", new String[] {"_input_event"}, jf, null); - Tuple3> entry = agent.getActions().get("act"); - assertThat(entry).isNotNull(); - assertThat(entry.f1).isSameAs(jf); + var definition = agent.getActions().get("act"); + assertThat(definition).isNotNull(); + assertThat(definition.f1).isSameAs(jf); } @Test diff --git a/api/src/test/java/org/apache/flink/agents/api/yaml/YamlLoaderBuildAgentsTest.java b/api/src/test/java/org/apache/flink/agents/api/yaml/YamlLoaderBuildAgentsTest.java index 95b3509ef..155e986cd 100644 --- a/api/src/test/java/org/apache/flink/agents/api/yaml/YamlLoaderBuildAgentsTest.java +++ b/api/src/test/java/org/apache/flink/agents/api/yaml/YamlLoaderBuildAgentsTest.java @@ -21,7 +21,6 @@ import org.apache.flink.agents.api.InputEvent; import org.apache.flink.agents.api.agents.Agent; import org.apache.flink.agents.api.event.ChatResponseEvent; -import org.apache.flink.agents.api.function.Function; import org.apache.flink.agents.api.function.JavaFunction; import org.apache.flink.agents.api.function.PythonFunction; import org.apache.flink.agents.api.prompt.Prompt; @@ -31,7 +30,6 @@ import org.apache.flink.agents.api.skills.Skills; import org.apache.flink.agents.api.tools.FunctionTool; import org.apache.flink.agents.api.yaml.YamlLoader.LoadedFile; -import org.apache.flink.api.java.tuple.Tuple3; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -53,8 +51,8 @@ void singleAgent() { assertThat(out.getAgents()).containsOnlyKeys("incrementer"); Agent agent = out.getAgents().get("incrementer"); - Map>> actions = agent.getActions(); - Tuple3> entry = actions.get("increment"); + var actions = agent.getActions(); + var entry = actions.get("increment"); assertThat(entry.f0).containsExactly(InputEvent.EVENT_TYPE); assertThat(entry.f1).isInstanceOf(JavaFunction.class); JavaFunction jf = (JavaFunction) entry.f1; @@ -149,11 +147,148 @@ void actionDefaultsToPython(@TempDir Path tmp) throws Exception { + " function: pkg.mod:fn\n" + " trigger_conditions: [input]\n"); LoadedFile out = YamlLoader.buildAgents(file); - Tuple3> entry = - out.getAgents().get("a").getActions().get("act"); + var entry = out.getAgents().get("a").getActions().get("act"); assertThat(entry.f1).isInstanceOf(PythonFunction.class); PythonFunction pf = (PythonFunction) entry.f1; assertThat(pf.getModule()).isEqualTo("pkg.mod"); assertThat(pf.getQualName()).isEqualTo("fn"); } + + @Test + void preservesRawTriggerEntries(@TempDir Path tmp) throws Exception { + Path file = tmp.resolve("raw_trigger_conditions.yaml"); + Files.writeString( + file, + "agents:\n" + + " - name: a\n" + + " actions:\n" + + " - name: act\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions:\n" + + " - input\n" + + " - input\n" + + " - \"type == '_input_event'\"\n" + + " - ' score > 1 '\n" + + " - 'type =='\n"); + + var definition = YamlLoader.buildAgents(file).getAgents().get("a").getActions().get("act"); + assertThat(definition.f0) + .containsExactly( + InputEvent.EVENT_TYPE, + InputEvent.EVENT_TYPE, + "type == '_input_event'", + " score > 1 ", + "type =="); + } + + @Test + void resolvesOnlyCompleteEventAliases(@TempDir Path tmp) throws Exception { + Path file = tmp.resolve("complete_alias_entries.yaml"); + Files.writeString( + file, + "agents:\n" + + " - name: a\n" + + " actions:\n" + + " - name: act\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions:\n" + + " - input\n" + + " - \"attributes.kind == 'input'\"\n" + + " - \"'input'\"\n"); + + var definition = YamlLoader.buildAgents(file).getAgents().get("a").getActions().get("act"); + assertThat(definition.f0) + .containsExactly(InputEvent.EVENT_TYPE, "attributes.kind == 'input'", "'input'"); + } + + @Test + void rejectsMissingOrEmptyConditions(@TempDir Path tmp) throws Exception { + Path file = tmp.resolve("deferred_trigger_validation.yaml"); + Files.writeString( + file, + "agents:\n" + + " - name: a\n" + + " actions:\n" + + " - name: missing\n" + + " function: pkg.mod:fn\n" + + " type: python\n"); + + assertThatThrownBy(() -> YamlLoader.buildAgents(file)) + .rootCause() + .hasMessageContaining("trigger_conditions"); + + Files.writeString( + file, + "agents:\n" + + " - name: a\n" + + " actions:\n" + + " - name: empty\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: []\n"); + + assertThatThrownBy(() -> YamlLoader.buildAgents(file)) + .rootCause() + .hasMessageContaining("trigger_conditions") + .hasMessageContaining("at least one"); + } + + @Test + void defersEntryValidationToPlan(@TempDir Path tmp) throws Exception { + Path file = tmp.resolve("deferred_trigger_validation.yaml"); + Files.writeString( + file, + "agents:\n" + + " - name: a\n" + + " actions:\n" + + " - name: invalid_entries\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [' ', null]\n"); + + Agent agent = YamlLoader.buildAgents(file).getAgents().get("a"); + assertThat(agent.getActions().get("invalid_entries").f0).containsExactly(" ", null); + } + + @Test + void rejectsNonStringCondition(@TempDir Path tmp) throws Exception { + Path file = tmp.resolve("non_string_trigger_condition.yaml"); + Files.writeString( + file, + "agents:\n" + + " - name: a\n" + + " actions:\n" + + " - name: invalid\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [true]\n"); + + assertThatThrownBy(() -> YamlLoader.buildAgents(file)) + .rootCause() + .hasMessageContaining("trigger_conditions") + .hasMessageContaining("entry #1") + .hasMessageContaining("string"); + } + + @Test + void preservesYamlActionOrder(@TempDir Path tmp) throws Exception { + Path file = tmp.resolve("ordered_actions.yaml"); + Files.writeString( + file, + "agents:\n" + + " - name: a\n" + + " actions:\n" + + " - name: first\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [input]\n" + + " - name: second\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [input]\n" + + " - name: third\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [input]\n" + + " - name: fourth\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [input]\n"); + + assertThat(YamlLoader.buildAgents(file).getAgents().get("a").getActions().keySet()) + .containsExactly("first", "second", "third", "fourth"); + } } diff --git a/api/src/test/java/org/apache/flink/agents/api/yaml/YamlLoaderLoadYamlTest.java b/api/src/test/java/org/apache/flink/agents/api/yaml/YamlLoaderLoadYamlTest.java index 0fba37e48..13a5155ba 100644 --- a/api/src/test/java/org/apache/flink/agents/api/yaml/YamlLoaderLoadYamlTest.java +++ b/api/src/test/java/org/apache/flink/agents/api/yaml/YamlLoaderLoadYamlTest.java @@ -22,13 +22,11 @@ import org.apache.flink.agents.api.AgentsExecutionEnvironment; import org.apache.flink.agents.api.agents.Agent; import org.apache.flink.agents.api.configuration.Configuration; -import org.apache.flink.agents.api.function.Function; import org.apache.flink.agents.api.function.JavaFunction; import org.apache.flink.agents.api.resource.ResourceType; import org.apache.flink.agents.api.skills.SkillSourceSpec; import org.apache.flink.agents.api.skills.Skills; import org.apache.flink.api.java.functions.KeySelector; -import org.apache.flink.api.java.tuple.Tuple3; import org.apache.flink.streaming.api.datastream.DataStream; import org.apache.flink.table.api.Table; import org.junit.jupiter.api.Test; @@ -55,16 +53,17 @@ void registersSingleAgentOnEnv() { } @Test - void mergesSharedActionIntoEachReferencingAgent() { + void mergesSharedActionIntoAgents() { AgentsExecutionEnvironment env = new TestEnv(); env.loadYaml(FIXTURES.resolve("with_shared.yaml")); Agent a1 = env.getAgents().get("a1"); Agent a2 = env.getAgents().get("a2"); - Map>> a1Actions = a1.getActions(); - Map>> a2Actions = a2.getActions(); + var a1Actions = a1.getActions(); + var a2Actions = a2.getActions(); assertThat(a1Actions).containsKey("shared_inc").containsKey("own_dec"); + assertThat(a1Actions.keySet()).containsExactly("shared_inc", "own_dec"); assertThat(a2Actions).containsKey("shared_inc"); assertThat(a1Actions.get("shared_inc").f1).isInstanceOf(JavaFunction.class); diff --git a/api/src/test/java/org/apache/flink/agents/api/yaml/spec/ActionSpecTest.java b/api/src/test/java/org/apache/flink/agents/api/yaml/spec/ActionSpecTest.java index 6b7366aec..5af4e618f 100644 --- a/api/src/test/java/org/apache/flink/agents/api/yaml/spec/ActionSpecTest.java +++ b/api/src/test/java/org/apache/flink/agents/api/yaml/spec/ActionSpecTest.java @@ -33,23 +33,74 @@ class ActionSpecTest { void minimal() throws Exception { ActionSpec spec = M.readValue( - "name: a\nfunction: pkg:fn\ntrigger_conditions: [input]\n", + "name: a\nfunction: pkg:fn\n" + + "trigger_conditions: [input, 'score > 1']\n", ActionSpec.class); assertThat(spec.getName()).isEqualTo("a"); assertThat(spec.getFunction()).isEqualTo("pkg:fn"); - assertThat(spec.getTriggerConditions()).containsExactly("input"); + assertThat(spec.getTriggerConditions()).containsExactly("input", "score > 1"); assertThat(spec.getConfig()).isNull(); assertThat(spec.getType()).isNull(); } @Test - void rejectsEmptyTriggerConditions() { + void rejectsMissingNullOrEmptyList() { + assertThatThrownBy(() -> M.readValue("name: a\nfunction: x:y\n", ActionSpec.class)) + .hasMessageContaining("trigger_conditions"); + assertThatThrownBy( + () -> + M.readValue( + "name: a\nfunction: x:y\ntrigger_conditions: null\n", + ActionSpec.class)) + .hasMessageContaining("trigger_conditions") + .hasMessageContaining("at least one"); assertThatThrownBy( () -> M.readValue( "name: a\nfunction: x:y\ntrigger_conditions: []\n", ActionSpec.class)) - .hasMessageContaining("trigger_conditions"); + .hasMessageContaining("trigger_conditions") + .hasMessageContaining("at least one"); + } + + @Test + void preservesInvalidEntriesForPlan() throws Exception { + ActionSpec invalidEntries = + M.readValue( + "name: a\nfunction: x:y\ntrigger_conditions: [' ', null]\n", + ActionSpec.class); + + assertThat(invalidEntries.getTriggerConditions()).containsExactly(" ", null); + } + + @Test + void rejectsNonStringScalars() { + assertThatThrownBy( + () -> + M.readValue( + "name: a\nfunction: x:y\ntrigger_conditions: [true]\n", + ActionSpec.class)) + .hasMessageContaining("trigger_conditions") + .hasMessageContaining("entry #1") + .hasMessageContaining("string"); + assertThatThrownBy( + () -> + M.readValue( + "name: a\nfunction: x:y\ntrigger_conditions: [42]\n", + ActionSpec.class)) + .hasMessageContaining("trigger_conditions") + .hasMessageContaining("entry #1") + .hasMessageContaining("string"); + } + + @Test + void acceptsQuotedStringScalars() throws Exception { + ActionSpec spec = + M.readValue( + "name: a\nfunction: x:y\ntrigger_conditions: ['true', '42']\n", + ActionSpec.class); + + assertThat(spec.getTriggerConditions()).containsExactly("true", "42"); } @Test @@ -61,12 +112,24 @@ void typeJava() throws Exception { assertThat(spec.getType()).isEqualTo(Language.JAVA); } + @Test + void preservesRawValuesAndOrder() throws Exception { + ActionSpec spec = + M.readValue( + "name: a\n" + + "function: x:y\n" + + "trigger_conditions: [input, input, ' score > 1 ']\n", + ActionSpec.class); + assertThat(spec.getTriggerConditions()).containsExactly("input", "input", " score > 1 "); + } + @Test void rejectsUnknownProperty() { assertThatThrownBy( () -> M.readValue( - "name: a\nfunction: x:y\ntrigger_conditions: [input]\nextra: 1\n", + "name: a\nfunction: x:y\n" + + "trigger_conditions: [input]\nextra: 1\n", ActionSpec.class)) .hasMessageContaining("extra"); } diff --git a/api/src/test/java/org/apache/flink/agents/api/yaml/spec/YamlAgentsDocumentTest.java b/api/src/test/java/org/apache/flink/agents/api/yaml/spec/YamlAgentsDocumentTest.java index 67ab6fbf5..243431625 100644 --- a/api/src/test/java/org/apache/flink/agents/api/yaml/spec/YamlAgentsDocumentTest.java +++ b/api/src/test/java/org/apache/flink/agents/api/yaml/spec/YamlAgentsDocumentTest.java @@ -40,7 +40,8 @@ void sharedSectionsAtFileLevel() throws Exception { String yaml = "agents:\n - name: a\n" + "chat_model_connections:\n - name: shared\n clazz: x.Y\n" - + "actions:\n - name: shared_a\n function: pkg:fn\n trigger_conditions: [input]\n"; + + "actions:\n - name: shared_a\n function: pkg:fn\n" + + " trigger_conditions: [input]\n"; YamlAgentsDocument doc = M.readValue(yaml, YamlAgentsDocument.class); assertThat(doc.getChatModelConnections()).hasSize(1); assertThat(doc.getActions()).hasSize(1); diff --git a/api/src/test/resources/yaml/python-parity/yaml_test_agent.yaml b/api/src/test/resources/yaml/python-parity/yaml_test_agent.yaml index aa2e153c1..f4bd357d6 100644 --- a/api/src/test/resources/yaml/python-parity/yaml_test_agent.yaml +++ b/api/src/test/resources/yaml/python-parity/yaml_test_agent.yaml @@ -29,4 +29,4 @@ agents: trigger_conditions: [input] - name: process_chat_response function: flink_agents.e2e_tests.e2e_tests_integration.yaml_test_actions:process_chat_response - trigger_conditions: [chat_response] \ No newline at end of file + trigger_conditions: [chat_response] diff --git a/dist/src/main/resources/META-INF/NOTICE b/dist/src/main/resources/META-INF/NOTICE index afc811a14..2f04cc2ca 100644 --- a/dist/src/main/resources/META-INF/NOTICE +++ b/dist/src/main/resources/META-INF/NOTICE @@ -14,6 +14,12 @@ This project bundles the following dependencies under the Apache Software Licens - com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.18.2 - com.fasterxml.jackson.module:jackson-module-kotlin:2.18.2 - com.fasterxml:classmate:1.7.0 +- dev.cel:cel:0.12.0 +- dev.cel:common:0.12.0 +- dev.cel:compiler:0.12.0 +- dev.cel:protobuf:0.12.0 +- dev.cel:runtime:0.12.0 +- dev.cel:v1alpha1:0.12.0 - org.apache.logging.log4j:log4j-api:2.23.1 - org.apache.logging.log4j:log4j-core:2.23.1 - org.apache.logging.log4j:log4j-slf4j-impl:2.23.1 @@ -24,6 +30,7 @@ This project bundles the following dependencies under the Apache Software Licens - com.openai:openai-java-core:4.8.0 - com.openai:openai-java-client-okhttp:4.8.0 - co.elastic.clients:elasticsearch-java:8.19.0 +- com.google.auto.value:auto-value-annotations:1.11.0 - com.google.guava:guava:33.5.0-jre - com.google.guava:failureaccess:1.0.1 - com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava @@ -181,8 +188,11 @@ See bundled license files for details. This project bundles the following dependencies under the BSD 3-Clause license. See bundled license files for details. -- com.google.protobuf:protobuf-java:3.25.5 +- com.google.protobuf:protobuf-java:4.33.5 +- com.google.re2j:re2j:1.8 +- org.antlr:antlr4-runtime:4.13.2 - org.ow2.asm:asm:9.3 +- org.threeten:threeten-extra:1.8.0 This project bundles the following dependencies under the EPL2 license. See bundled license files for details. diff --git a/dist/src/main/resources/META-INF/licenses/LICENSE.antlr4-runtime b/dist/src/main/resources/META-INF/licenses/LICENSE.antlr4-runtime new file mode 100644 index 000000000..5d2769415 --- /dev/null +++ b/dist/src/main/resources/META-INF/licenses/LICENSE.antlr4-runtime @@ -0,0 +1,28 @@ +Copyright (c) 2012-2022 The ANTLR Project. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions +are met: + +1. Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + +2. Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in the +documentation and/or other materials provided with the distribution. + +3. Neither name of copyright holders nor the names of its contributors +may be used to endorse or promote products derived from this software +without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR +CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/dist/src/main/resources/META-INF/licenses/LICENSE.re2j b/dist/src/main/resources/META-INF/licenses/LICENSE.re2j new file mode 100644 index 000000000..b620ae68f --- /dev/null +++ b/dist/src/main/resources/META-INF/licenses/LICENSE.re2j @@ -0,0 +1,32 @@ +This is a work derived from Russ Cox's RE2 in Go, whose license +http://golang.org/LICENSE is as follows: + +Copyright (c) 2009 The Go Authors. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in + the documentation and/or other materials provided with the + distribution. + + * Neither the name of Google Inc. nor the names of its contributors + may be used to endorse or promote products derived from this + software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/dist/src/main/resources/META-INF/licenses/LICENSE.threeten-extra b/dist/src/main/resources/META-INF/licenses/LICENSE.threeten-extra new file mode 100644 index 000000000..f8f6e594b --- /dev/null +++ b/dist/src/main/resources/META-INF/licenses/LICENSE.threeten-extra @@ -0,0 +1,29 @@ +Copyright (c) 2007-present, Stephen Colebourne & Michael Nascimento Santos. + +All rights reserved. + +* Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +* Neither the name of JSR-310 nor the names of its contributors + may be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR +CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/docs/yaml-schema.json b/docs/yaml-schema.json index 183cc7ac8..dc593377b 100644 --- a/docs/yaml-schema.json +++ b/docs/yaml-schema.json @@ -2,7 +2,7 @@ "$defs": { "ActionSpec": { "additionalProperties": false, - "description": "An action references a user function and its trigger conditions.\n\n``function`` is written as ``:`` \u2014 the\ncolon separates the Python module (or Java class FQN) from the\nattribute path inside it.\n\n``trigger_conditions`` carries one or more strings. Each is either an\nevent-type name (bare identifier) or a future condition-expression\nform \u2014 the runtime classifies the string when it loads the plan.\n\nAction signatures are fixed (``(Event, RunnerContext)``), so there is\nno ``parameter_types`` knob \u2014 Python doesn't need it, and the Java\naction signature is determined by the action contract.", + "description": "An action referencing a user function and its trigger conditions.\n\n``function`` uses ``:``.\n\nEach ``trigger_conditions`` entry is an event-type name or Boolean\ncondition expression. Entries combine with OR semantics. Expression\nvalidation occurs when the plan is applied.", "properties": { "config": { "anyOf": [ @@ -35,6 +35,7 @@ }, "trigger_conditions": { "items": { + "pattern": "\\S", "type": "string" }, "minItems": 1, diff --git a/e2e-test/cross-language-agent-plan-snapshots/java/agent_plan_with_python_action.json b/e2e-test/cross-language-agent-plan-snapshots/java/agent_plan_with_python_action.json index 8b6e28867..3b9a76e32 100644 --- a/e2e-test/cross-language-agent-plan-snapshots/java/agent_plan_with_python_action.json +++ b/e2e-test/cross-language-agent-plan-snapshots/java/agent_plan_with_python_action.json @@ -11,6 +11,17 @@ "trigger_conditions" : [ "_chat_request_event", "_tool_response_event" ], "config" : null }, + "tool_call_action" : { + "name" : "tool_call_action", + "exec" : { + "func_type" : "JavaFunction", + "qualname" : "org.apache.flink.agents.plan.actions.ToolCallAction", + "method_name" : "processToolRequest", + "parameter_types" : [ "org.apache.flink.agents.api.Event", "org.apache.flink.agents.api.context.RunnerContext" ] + }, + "trigger_conditions" : [ "_tool_request_event" ], + "config" : null + }, "context_retrieval_action" : { "name" : "context_retrieval_action", "exec" : { @@ -29,28 +40,10 @@ "module" : "flink_agents.plan.tests.test_agent_plan_cross_language", "qualname" : "_dummy_action" }, - "trigger_conditions" : [ "_input_event" ], - "config" : null - }, - "tool_call_action" : { - "name" : "tool_call_action", - "exec" : { - "func_type" : "JavaFunction", - "qualname" : "org.apache.flink.agents.plan.actions.ToolCallAction", - "method_name" : "processToolRequest", - "parameter_types" : [ "org.apache.flink.agents.api.Event", "org.apache.flink.agents.api.context.RunnerContext" ] - }, - "trigger_conditions" : [ "_tool_request_event" ], + "trigger_conditions" : [ "_input_event", "attributes.ready == true" ], "config" : null } }, - "actions_by_event" : { - "_context_retrieval_request_event" : [ "context_retrieval_action" ], - "_tool_response_event" : [ "chat_model_action" ], - "_chat_request_event" : [ "chat_model_action" ], - "_tool_request_event" : [ "tool_call_action" ], - "_input_event" : [ "handle" ] - }, "resource_providers" : { }, "config" : { "conf_data" : { } diff --git a/e2e-test/cross-language-agent-plan-snapshots/python/agent_plan_with_java_action.json b/e2e-test/cross-language-agent-plan-snapshots/python/agent_plan_with_java_action.json index 19dee444a..97974fac7 100644 --- a/e2e-test/cross-language-agent-plan-snapshots/python/agent_plan_with_java_action.json +++ b/e2e-test/cross-language-agent-plan-snapshots/python/agent_plan_with_java_action.json @@ -12,7 +12,8 @@ ] }, "trigger_conditions": [ - "_input_event" + "_input_event", + "attributes.ready == true" ], "config": null }, @@ -54,25 +55,10 @@ "config": null } }, - "actions_by_event": { - "_input_event": [ - "handle" - ], - "_chat_request_event": [ - "chat_model_action" - ], - "_tool_response_event": [ - "chat_model_action" - ], - "_tool_request_event": [ - "tool_call_action" - ], - "_context_retrieval_request_event": [ - "context_retrieval_action" - ] - }, "resource_providers": {}, "config": { - "conf_data": {} + "conf_data": { + "action.trigger-condition.evaluate-failure-strategy": "FAIL" + } } } diff --git a/e2e-test/cross-language-trigger-condition-fixtures/trigger_condition_conformance.json b/e2e-test/cross-language-trigger-condition-fixtures/trigger_condition_conformance.json new file mode 100644 index 000000000..a0940046c --- /dev/null +++ b/e2e-test/cross-language-trigger-condition-fixtures/trigger_condition_conformance.json @@ -0,0 +1,280 @@ +{ + "cases": [ + { + "name": "bare_event_type", + "entry": "a", + "classification": "event_type", + "plan_validation": "pass", + "runtime_compilation": "not_applicable", + "probe_event_type": "a", + "probe_attributes": {}, + "matches_probe": true + }, + { + "name": "bare_dotted_event_type", + "entry": "a.b.c", + "classification": "event_type", + "plan_validation": "pass", + "runtime_compilation": "not_applicable", + "probe_event_type": "a.b.c", + "probe_attributes": {}, + "matches_probe": true + }, + { + "name": "trimmed_event_type", + "entry": " my_event ", + "classification": "event_type", + "plan_validation": "pass", + "runtime_compilation": "not_applicable", + "probe_event_type": "my_event", + "probe_attributes": {}, + "matches_probe": true + }, + { + "name": "quoted_routable_event_type", + "entry": "'order-created.v1'", + "classification": "event_type", + "plan_validation": "pass", + "runtime_compilation": "not_applicable", + "probe_event_type": "order-created.v1", + "probe_attributes": {}, + "matches_probe": true + }, + { + "name": "quoted_event_type_non_match", + "entry": "\"order-created.v1\"", + "classification": "event_type", + "plan_validation": "pass", + "runtime_compilation": "not_applicable", + "probe_event_type": "other", + "probe_attributes": {}, + "matches_probe": false + }, + { + "name": "quoted_keyword_event_type", + "entry": "'true'", + "classification": "event_type", + "plan_validation": "pass", + "runtime_compilation": "not_applicable", + "probe_event_type": "true", + "probe_attributes": {}, + "matches_probe": true + }, + { + "name": "bare_in_event_type", + "entry": "in", + "classification": "event_type", + "plan_validation": "pass", + "runtime_compilation": "not_applicable", + "probe_event_type": "in", + "probe_attributes": {}, + "matches_probe": true + }, + { + "name": "escaped_quoted_value", + "entry": "\"order\\\"created\"", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "fail", + "error_category": "type_check", + "runtime_error_fragment": "Boolean result" + }, + { + "name": "empty_string_literal", + "entry": "''", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "fail", + "error_category": "type_check", + "runtime_error_fragment": "Boolean result" + }, + { + "name": "boolean_true", + "entry": "true", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "pass", + "probe_event_type": "other", + "probe_attributes": {}, + "matches_probe": true + }, + { + "name": "boolean_false", + "entry": "false", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "pass", + "probe_event_type": "other", + "probe_attributes": {}, + "matches_probe": false + }, + { + "name": "event_type_comparison", + "entry": "type == EventType.InputEvent", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "pass", + "probe_event_type": "_input_event", + "probe_attributes": {}, + "matches_probe": true + }, + { + "name": "explicit_boolean_path", + "entry": "a.b.c == true", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "pass", + "probe_event_type": "other", + "probe_attributes": { + "a": { + "b": { + "c": true + } + } + }, + "matches_probe": true + }, + { + "name": "has_macro", + "entry": "has(score)", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "pass", + "probe_event_type": "other", + "probe_attributes": { + "score": 7 + }, + "matches_probe": true + }, + { + "name": "unknown_function", + "entry": "noSuchFn(1)", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "fail", + "error_category": "type_check", + "runtime_error_fragment": "Runtime type-check" + }, + { + "name": "dynamic_whole_attributes_index", + "entry": "attributes[key_name] == 5", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "fail", + "error_category": "referenced_attributes", + "runtime_error_fragment": "whole 'attributes' map" + }, + { + "name": "standalone_null", + "entry": "null", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "fail", + "error_category": "type_check", + "runtime_error_fragment": "Boolean result" + }, + { + "name": "standalone_number", + "entry": "42", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "fail", + "error_category": "type_check", + "runtime_error_fragment": "Boolean result" + }, + { + "name": "standalone_event_type_constant", + "entry": "EventType.InputEvent", + "classification": "expression", + "plan_validation": "fail", + "runtime_compilation": "not_applicable", + "error_category": "unsupported_construct" + }, + { + "name": "parenthesized_path", + "entry": "(a.b.c)", + "classification": "expression", + "plan_validation": "fail", + "runtime_compilation": "not_applicable", + "error_category": "unsupported_construct" + }, + { + "name": "spaced_path", + "entry": "a . b . c", + "classification": "expression", + "plan_validation": "fail", + "runtime_compilation": "not_applicable", + "error_category": "unsupported_construct" + }, + { + "name": "invalid_cel", + "entry": "type ==", + "classification": "expression", + "plan_validation": "fail", + "runtime_compilation": "not_applicable", + "error_category": "syntax" + }, + { + "name": "unknown_event_type", + "entry": "type == EventType.NotARealEvent", + "classification": "expression", + "plan_validation": "fail", + "runtime_compilation": "not_applicable", + "error_category": "unknown_event_type" + }, + { + "name": "disallowed_macro", + "entry": "[1, 2].all(value, value > 0)", + "classification": "expression", + "plan_validation": "fail", + "runtime_compilation": "not_applicable", + "error_category": "unsupported_construct" + }, + { + "name": "quoted_name_with_whitespace", + "entry": "'order created'", + "classification": "expression", + "plan_validation": "pass", + "runtime_compilation": "fail", + "error_category": "type_check", + "runtime_error_fragment": "Boolean result" + }, + { + "name": "unquoted_extended_name", + "entry": "order-created", + "classification": "event_type", + "plan_validation": "pass", + "runtime_compilation": "not_applicable", + "probe_event_type": "order-created", + "probe_attributes": {}, + "matches_probe": true + } + ], + "ordering_dedupe_type_first": { + "probe_event_type": "_input_event", + "actions": [ + { + "name": "condition_first_in_plan", + "entries": ["type == EventType.InputEvent"] + }, + { + "name": "first_type_hit", + "entries": [ + "_input_event", + "_input_event", + "type == EventType.InputEvent", + "type == EventType.InputEvent" + ] + }, + { + "name": "second_type_hit", + "entries": ["'_input_event'"] + } + ], + "expected_matches": [ + "first_type_hit", + "second_type_hit", + "condition_first_in_plan" + ] + } +} diff --git a/e2e-test/flink-agents-end-to-end-tests-integration/src/test/java/org/apache/flink/agents/integration/test/TriggerConditionIntegrationAgent.java b/e2e-test/flink-agents-end-to-end-tests-integration/src/test/java/org/apache/flink/agents/integration/test/TriggerConditionIntegrationAgent.java new file mode 100644 index 000000000..5553be844 --- /dev/null +++ b/e2e-test/flink-agents-end-to-end-tests-integration/src/test/java/org/apache/flink/agents/integration/test/TriggerConditionIntegrationAgent.java @@ -0,0 +1,154 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.flink.agents.integration.test; + +import org.apache.flink.agents.api.Event; +import org.apache.flink.agents.api.EventType; +import org.apache.flink.agents.api.InputEvent; +import org.apache.flink.agents.api.OutputEvent; +import org.apache.flink.agents.api.agents.Agent; +import org.apache.flink.agents.api.annotation.Action; +import org.apache.flink.agents.api.context.RunnerContext; + +import java.io.Serializable; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** Exercises trigger conditions through the complete action-routing path. */ +public class TriggerConditionIntegrationAgent extends Agent { + + /** Java POJO input used to verify lazy nested-field normalization. */ + public static final class ConditionInput implements Serializable { + public String id; + public String status; + public int value; + + public ConditionInput() {} + + public ConditionInput(String id, String status, int value) { + this.id = id; + this.status = status; + this.value = value; + } + } + + /** Verifies that equivalent POJO and map payloads match the same nested condition. */ + public static final class PojoMapParityAgent extends Agent { + + @Action("type == EventType.InputEvent && input.status == 'ok'") + public static void onMatchingStatus(Event event, RunnerContext ctx) { + Object input = InputEvent.fromEvent(event).getInput(); + ctx.sendEvent(new OutputEvent("parity:" + inputId(input))); + } + } + + /** Verifies whole-value access for scalar and list input payloads. */ + public static final class ScalarListPayloadAgent extends Agent { + + @Action({ + "type == EventType.InputEvent && input == 'ready'", + "type == EventType.InputEvent && input[0] == 'ready'" + }) + public static void onMatchingPayload(Event event, RunnerContext ctx) { + Object input = InputEvent.fromEvent(event).getInput(); + ctx.sendEvent(new OutputEvent(input instanceof List ? "list" : "scalar")); + } + } + + @Action("type == EventType.InputEvent && input.status == 'ok'") + public static void onNestedPojoField(Event event, RunnerContext ctx) { + ctx.sendEvent(new OutputEvent("nested:" + input(event).id)); + } + + @Action("type == EventType.InputEvent && attributes.input.status == 'ok'") + public static void onAttributesNamespace(Event event, RunnerContext ctx) { + ctx.sendEvent(new OutputEvent("envelope:" + input(event).id)); + } + + @Action({ + "type == EventType.InputEvent && input.status == 'ok'", + "type == EventType.InputEvent && input.value > 5" + }) + public static void onStatusOrValue(Event event, RunnerContext ctx) { + ctx.sendEvent(new OutputEvent("or:" + input(event).id)); + } + + @Action(EventType.InputEvent) + public static void emitCustomEvent(Event event, RunnerContext ctx) { + ConditionInput input = input(event); + Map result = new HashMap<>(); + result.put("id", input.id); + result.put("status", input.status); + result.put("value", input.value); + if ("ok".equals(input.status)) { + result.put("details", Map.of("code", "ready")); + } + ctx.sendEvent(new Event("condition.result", Map.of("result", result))); + } + + /** YAML action that emits the bare hyphenated event type used by the routing E2E. */ + public static void emitYamlOrderCreated(Event event, RunnerContext ctx) { + Event orderCreated = new Event("order-created"); + orderCreated.setAttr("id", input(event).id); + ctx.sendEvent(orderCreated); + } + + /** YAML action that confirms a bare hyphenated event type reached the type index. */ + public static void onYamlOrderCreated(Event event, RunnerContext ctx) { + ctx.sendEvent(new OutputEvent("yaml-hyphen:" + event.getAttr("id"))); + } + + @Action("type == 'condition.result' && result.status == 'ok'") + public static void onCustomAttribute(Event event, RunnerContext ctx) { + ctx.sendEvent(new OutputEvent("custom:" + resultId(event))); + } + + @Action( + "type == 'condition.result' " + + "&& has(result.details.code) " + + "&& result.details.code == 'ready'") + public static void onGuardedNestedAttribute(Event event, RunnerContext ctx) { + ctx.sendEvent(new OutputEvent("guarded:" + resultId(event))); + } + + /** Terminal output events must not be routed back into actions. */ + @Action(EventType.OutputEvent) + public static void onTerminalOutput(Event event, RunnerContext ctx) { + ctx.sendEvent(new OutputEvent("unexpected-output-routing")); + } + + private static ConditionInput input(Event event) { + return (ConditionInput) InputEvent.fromEvent(event).getInput(); + } + + private static String inputId(Object input) { + if (input instanceof ConditionInput) { + return ((ConditionInput) input).id; + } + if (input instanceof Map) { + return (String) ((Map) input).get("id"); + } + throw new IllegalArgumentException("Unsupported parity payload: " + input.getClass()); + } + + @SuppressWarnings("unchecked") + private static String resultId(Event event) { + return (String) ((Map) event.getAttr("result")).get("id"); + } +} diff --git a/e2e-test/flink-agents-end-to-end-tests-integration/src/test/java/org/apache/flink/agents/integration/test/TriggerConditionIntegrationTest.java b/e2e-test/flink-agents-end-to-end-tests-integration/src/test/java/org/apache/flink/agents/integration/test/TriggerConditionIntegrationTest.java new file mode 100644 index 000000000..5eec513e9 --- /dev/null +++ b/e2e-test/flink-agents-end-to-end-tests-integration/src/test/java/org/apache/flink/agents/integration/test/TriggerConditionIntegrationTest.java @@ -0,0 +1,192 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.flink.agents.integration.test; + +import org.apache.flink.agents.api.AgentsExecutionEnvironment; +import org.apache.flink.agents.api.agents.Agent; +import org.apache.flink.agents.api.configuration.AgentConfigOptions; +import org.apache.flink.agents.api.configuration.AgentConfigOptions.ConditionEvaluationFailureStrategy; +import org.apache.flink.agents.integration.test.TriggerConditionIntegrationAgent.ConditionInput; +import org.apache.flink.agents.integration.test.TriggerConditionIntegrationAgent.PojoMapParityAgent; +import org.apache.flink.agents.integration.test.TriggerConditionIntegrationAgent.ScalarListPayloadAgent; +import org.apache.flink.api.common.typeinfo.TypeInformation; +import org.apache.flink.api.java.functions.KeySelector; +import org.apache.flink.streaming.api.datastream.DataStream; +import org.apache.flink.streaming.api.environment.StreamExecutionEnvironment; +import org.apache.flink.util.CloseableIterator; +import org.junit.jupiter.api.Test; + +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +import static org.assertj.core.api.Assertions.assertThat; + +/** End-to-end test for trigger conditions in a full Flink pipeline. */ +public class TriggerConditionIntegrationTest { + + @Test + public void testPojoConditionsAndOrRouting() throws Exception { + StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment(); + env.setParallelism(1); + + DataStream inputStream = + env.fromElements( + new ConditionInput("ok-high", "ok", 9), + new ConditionInput("error-high", "error", 9), + new ConditionInput("ok-low", "ok", 2)); + + AgentsExecutionEnvironment agentsEnv = + AgentsExecutionEnvironment.getExecutionEnvironment(env); + agentsEnv + .getConfig() + .set( + AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY, + ConditionEvaluationFailureStrategy.FAIL); + + DataStream outputStream = + agentsEnv + .fromDataStream( + inputStream, + (KeySelector) input -> input.id) + .apply(new TriggerConditionIntegrationAgent()) + .toDataStream(); + + CloseableIterator results = outputStream.collectAsync(); + agentsEnv.execute(); + + List outputs = new ArrayList<>(); + while (results.hasNext()) { + outputs.add((String) results.next()); + } + results.close(); + + assertThat(outputs.stream().filter(output -> output.startsWith("nested:"))) + .containsExactlyInAnyOrder("nested:ok-high", "nested:ok-low"); + assertThat(outputs.stream().filter(output -> output.startsWith("envelope:"))) + .containsExactlyInAnyOrder("envelope:ok-high", "envelope:ok-low"); + assertThat(outputs.stream().filter(output -> output.startsWith("or:"))) + .as("an action runs once even when both OR branches match") + .containsExactlyInAnyOrder("or:ok-high", "or:error-high", "or:ok-low"); + assertThat(outputs.stream().filter(output -> output.startsWith("custom:"))) + .containsExactlyInAnyOrder("custom:ok-high", "custom:ok-low"); + assertThat(outputs.stream().filter(output -> output.startsWith("guarded:"))) + .containsExactlyInAnyOrder("guarded:ok-high", "guarded:ok-low"); + assertThat(outputs).doesNotContain("unexpected-output-routing").hasSize(11); + } + + @Test + public void pojoAndMapHaveSameResult() throws Exception { + List inputs = + List.of( + new ConditionInput("pojo", "ok", 9), + Map.of("id", "map", "status", "ok", "value", 9)); + + assertThat(runPayloads(inputs, new PojoMapParityAgent())) + .containsExactlyInAnyOrder("parity:pojo", "parity:map"); + } + + @Test + public void scalarAndListPayloadsWork() throws Exception { + assertThat(runPayloads(List.of("ready", List.of("ready")), new ScalarListPayloadAgent())) + .containsExactlyInAnyOrder("scalar", "list"); + } + + @Test + public void yamlNestedAndHyphenatedRouting() throws Exception { + StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment(); + env.setParallelism(1); + + DataStream inputStream = + env.fromElements( + new ConditionInput("ok", "ok", 1), new ConditionInput("error", "error", 1)); + + AgentsExecutionEnvironment agentsEnv = + AgentsExecutionEnvironment.getExecutionEnvironment(env); + agentsEnv + .getConfig() + .set( + AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY, + ConditionEvaluationFailureStrategy.FAIL); + agentsEnv.loadYaml(yamlFixture("trigger_condition_agent.yaml")); + + DataStream outputStream = + agentsEnv + .fromDataStream( + inputStream, + (KeySelector) input -> input.id) + .apply("yaml_trigger_condition_agent") + .toDataStream(); + + CloseableIterator results = outputStream.collectAsync(); + agentsEnv.execute(); + + List outputs = new ArrayList<>(); + while (results.hasNext()) { + outputs.add((String) results.next()); + } + results.close(); + + assertThat(outputs).containsExactly("yaml-hyphen:ok"); + } + + private static Path yamlFixture(String name) throws URISyntaxException { + URL resource = + TriggerConditionIntegrationTest.class.getClassLoader().getResource("yaml/" + name); + Objects.requireNonNull(resource, "fixture not found on classpath: yaml/" + name); + return Paths.get(resource.toURI()); + } + + private static List runPayloads(List inputs, Agent agent) throws Exception { + StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment(); + env.setParallelism(1); + + DataStream inputStream = + env.fromCollection(inputs, TypeInformation.of(Object.class)); + AgentsExecutionEnvironment agentsEnv = + AgentsExecutionEnvironment.getExecutionEnvironment(env); + agentsEnv + .getConfig() + .set( + AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY, + ConditionEvaluationFailureStrategy.FAIL); + + DataStream outputStream = + agentsEnv + .fromDataStream( + inputStream, + (KeySelector) ignored -> "payload-shapes") + .apply(agent) + .toDataStream(); + + CloseableIterator results = outputStream.collectAsync(); + agentsEnv.execute(); + + List outputs = new ArrayList<>(); + while (results.hasNext()) { + outputs.add((String) results.next()); + } + results.close(); + return outputs; + } +} diff --git a/e2e-test/flink-agents-end-to-end-tests-integration/src/test/resources/yaml/trigger_condition_agent.yaml b/e2e-test/flink-agents-end-to-end-tests-integration/src/test/resources/yaml/trigger_condition_agent.yaml new file mode 100644 index 000000000..bae9956ca --- /dev/null +++ b/e2e-test/flink-agents-end-to-end-tests-integration/src/test/resources/yaml/trigger_condition_agent.yaml @@ -0,0 +1,30 @@ +################################################################################ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +################################################################################ +agents: + - name: yaml_trigger_condition_agent + actions: + - name: emit_order_created + type: java + function: org.apache.flink.agents.integration.test.TriggerConditionIntegrationAgent:emitYamlOrderCreated + trigger_conditions: + - type == EventType.InputEvent && input.status == 'ok' + - name: handle_order_created + type: java + function: org.apache.flink.agents.integration.test.TriggerConditionIntegrationAgent:onYamlOrderCreated + trigger_conditions: + - order-created diff --git a/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/pom.xml b/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/pom.xml index 9553e3bcb..54a91e04d 100644 --- a/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/pom.xml +++ b/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/pom.xml @@ -71,6 +71,27 @@ flink-python ${flink.version} + + + + org.apache.logging.log4j + log4j-api + ${log4j2.version} + test + + + org.apache.logging.log4j + log4j-core + ${log4j2.version} + test + + + org.apache.logging.log4j + log4j-slf4j-impl + ${log4j2.version} + test + @@ -97,4 +118,4 @@ - \ No newline at end of file + diff --git a/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/src/test/java/org/apache/flink/agents/resource/test/JavaAgentWithPythonActionAgent.java b/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/src/test/java/org/apache/flink/agents/resource/test/JavaAgentWithPythonActionAgent.java index 8be703347..b74ba3f60 100644 --- a/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/src/test/java/org/apache/flink/agents/resource/test/JavaAgentWithPythonActionAgent.java +++ b/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/src/test/java/org/apache/flink/agents/resource/test/JavaAgentWithPythonActionAgent.java @@ -17,7 +17,6 @@ */ package org.apache.flink.agents.resource.test; -import org.apache.flink.agents.api.InputEvent; import org.apache.flink.agents.api.agents.Agent; import org.apache.flink.agents.api.function.PythonFunction; import org.apache.flink.api.java.functions.KeySelector; @@ -31,7 +30,10 @@ public class JavaAgentWithPythonActionAgent extends Agent { public JavaAgentWithPythonActionAgent() { addAction( "multiply_by_two", - new String[] {InputEvent.EVENT_TYPE}, + new String[] { + "type == EventType.InputEvent && input > 1 && input < 7", + "type == EventType.InputEvent && input > 3 && input < 9" + }, new PythonFunction(PYTHON_MODULE, PYTHON_QUALNAME), null); } diff --git a/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/src/test/java/org/apache/flink/agents/resource/test/JavaAgentWithPythonActionTest.java b/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/src/test/java/org/apache/flink/agents/resource/test/JavaAgentWithPythonActionTest.java index f505deffe..0d2f3123a 100644 --- a/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/src/test/java/org/apache/flink/agents/resource/test/JavaAgentWithPythonActionTest.java +++ b/e2e-test/flink-agents-end-to-end-tests-resource-cross-language/src/test/java/org/apache/flink/agents/resource/test/JavaAgentWithPythonActionTest.java @@ -32,11 +32,11 @@ public class JavaAgentWithPythonActionTest { @Test - public void javaAgentDispatchesPythonActionBody() throws Exception { + public void overlappingConditionsRunPythonActionOnce() throws Exception { StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment(); env.setParallelism(1); - DataStream inputStream = env.fromData(1L, 2L, 3L, 4L, 5L); + DataStream inputStream = env.fromData(1L, 3L, 5L, 7L, 9L); AgentsExecutionEnvironment agentsEnv = AgentsExecutionEnvironment.getExecutionEnvironment(env); @@ -57,6 +57,8 @@ public void javaAgentDispatchesPythonActionBody() throws Exception { } Collections.sort(actual); - assertThat(actual).containsExactly(2L, 4L, 6L, 8L, 10L); + // 3 is first-only, 5 overlaps (and executes once), 7 is second-only, + // while 1 and 9 do not match. + assertThat(actual).containsExactly(6L, 10L, 14L); } } diff --git a/plan/pom.xml b/plan/pom.xml index 02df3c2c3..507b628da 100644 --- a/plan/pom.xml +++ b/plan/pom.xml @@ -50,6 +50,11 @@ under the License. com.fasterxml.jackson.core jackson-databind + + dev.cel + cel + ${cel.version} + io.github.bonede @@ -89,11 +94,13 @@ under the License. org.apache.logging.log4j log4j-core ${log4j2.version} + test org.apache.logging.log4j log4j-slf4j-impl ${log4j2.version} + test @@ -145,4 +152,4 @@ under the License. - \ No newline at end of file + diff --git a/plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java b/plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java index 3b08bf2cf..98d967c34 100644 --- a/plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java +++ b/plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java @@ -54,7 +54,6 @@ import org.apache.flink.agents.plan.tools.FunctionTool; import org.apache.flink.agents.plan.tools.ToolMetadataFactory; import org.apache.flink.agents.plan.tools.bash.BashTool; -import org.apache.flink.api.java.tuple.Tuple3; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -91,38 +90,30 @@ public class AgentPlan implements Serializable { /** Mapping from action name to action itself. */ private Map actions; - /** Mapping from event type string to list of actions that should be triggered by the event. */ - private Map> actionsByEvent; - /** Two-level mapping of resource type to resource name to resource provider. */ private Map> resourceProviders; private AgentConfiguration config; - public AgentPlan(Map actions, Map> actionsByEvent) { - this.actions = actions; - this.actionsByEvent = actionsByEvent; + public AgentPlan(Map actions) { + this.actions = Collections.unmodifiableMap(new LinkedHashMap<>(actions)); this.resourceProviders = new HashMap<>(); this.config = new AgentConfiguration(); } public AgentPlan( Map actions, - Map> actionsByEvent, Map> resourceProviders) { - this.actions = actions; - this.actionsByEvent = actionsByEvent; + this.actions = Collections.unmodifiableMap(new LinkedHashMap<>(actions)); this.resourceProviders = resourceProviders; this.config = new AgentConfiguration(); } public AgentPlan( Map actions, - Map> actionsByEvent, Map> resourceProviders, AgentConfiguration config) { - this.actions = actions; - this.actionsByEvent = actionsByEvent; + this.actions = Collections.unmodifiableMap(new LinkedHashMap<>(actions)); this.resourceProviders = resourceProviders; this.config = config; } @@ -139,10 +130,12 @@ public AgentPlan(Agent agent) throws Exception { } public AgentPlan(Agent agent, AgentConfiguration config) throws Exception { - this(new HashMap<>(), new HashMap<>()); + this.actions = new LinkedHashMap<>(); + this.resourceProviders = new HashMap<>(); + this.config = config; extractActionsFromAgent(agent); extractResourceProvidersFromAgent(agent); - this.config = config; + this.actions = Collections.unmodifiableMap(new LinkedHashMap<>(actions)); } public Map getActions() { @@ -157,18 +150,10 @@ public Object getActionConfigValue(String actionName, String key) { return Objects.requireNonNull(actions.get(actionName).getConfig()).get(key); } - public Map> getActionsByEvent() { - return actionsByEvent; - } - public Map> getResourceProviders() { return resourceProviders; } - public List getActionsTriggeredBy(String eventType) { - return actionsByEvent.get(eventType); - } - public AgentConfiguration getConfig() { return config; } @@ -185,47 +170,32 @@ private void writeObject(ObjectOutputStream out) throws IOException { private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { String serializedStr = in.readUTF(); AgentPlan agentPlan = new ObjectMapper().readValue(serializedStr, AgentPlan.class); - this.actions = agentPlan.getActions(); - this.actionsByEvent = agentPlan.getActionsByEvent(); + this.actions = Collections.unmodifiableMap(new LinkedHashMap<>(agentPlan.getActions())); this.resourceProviders = agentPlan.getResourceProviders(); this.config = agentPlan.getConfig(); } private void extractActions( String actionName, - String[] triggerEntries, + String[] triggerConditions, org.apache.flink.agents.plan.Function function, Map config) throws Exception { - List triggerConditions = new ArrayList<>(Arrays.asList(triggerEntries)); - - if (triggerConditions.isEmpty()) { - throw new IllegalArgumentException( - "Action " - + actionName - + " must specify at least one trigger entry via @Action(EventType.x)."); - } - - // Create an Action - Action action = new Action(actionName, function, triggerConditions, config); - - // Add to actions map - actions.put(action.getName(), action); - - // Add to actionsByEvent map - for (String eventTypeName : triggerConditions) { - actionsByEvent.computeIfAbsent(eventTypeName, k -> new ArrayList<>()).add(action); - } + Action action = + new Action( + actionName, + function, + triggerConditions == null ? null : Arrays.asList(triggerConditions), + config); + registerAction(action); } private void addBuiltAction(Action action) { - // Add to actions map - actions.put(action.getName(), action); + registerAction(action); + } - // Add to actionsByEvent map - for (String eventTypeName : action.getListenEventTypes()) { - actionsByEvent.computeIfAbsent(eventTypeName, k -> new ArrayList<>()).add(action); - } + private void registerAction(Action action) { + actions.put(action.getName(), action); } private void extractActionsFromAgent(Agent agent) throws Exception { @@ -260,7 +230,7 @@ private void extractActionsFromAgent(Agent agent) throws Exception { Objects.requireNonNull( method.getAnnotation( org.apache.flink.agents.api.annotation.Action.class)); - String[] triggerEntries = actionAnnotation.value(); + String[] triggerConditions = actionAnnotation.value(); org.apache.flink.agents.api.annotation.PythonFunction target = actionAnnotation.target(); String targetModule = target.module(); @@ -285,20 +255,13 @@ private void extractActionsFromAgent(Agent agent) throws Exception { + method.getName() + "' must set both module and qualname"); } - extractActions(method.getName(), triggerEntries, execFunction, null); + extractActions(method.getName(), triggerConditions, execFunction, null); } - for (Map.Entry< - String, - Tuple3< - String[], - org.apache.flink.agents.api.function.Function, - Map>> - action : agent.getActions().entrySet()) { + for (var action : agent.getActions().entrySet()) { String actionName = action.getKey(); - Tuple3> - tuple = action.getValue(); - extractActions(actionName, tuple.f0, toPlanFunction(tuple.f1), tuple.f2); + var definition = action.getValue(); + extractActions(actionName, definition.f0, toPlanFunction(definition.f1), definition.f2); } } diff --git a/plan/src/main/java/org/apache/flink/agents/plan/AgentPlanPreflight.java b/plan/src/main/java/org/apache/flink/agents/plan/AgentPlanPreflight.java new file mode 100644 index 000000000..88d5b6583 --- /dev/null +++ b/plan/src/main/java/org/apache/flink/agents/plan/AgentPlanPreflight.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.flink.annotation.Internal; + +import javax.annotation.Nullable; + +/** + * Validates Python-produced AgentPlan JSON with the authoritative Java Plan implementation. + * + *

This is an internal Py4J bridge. A valid plan returns {@code null}; an expected user Plan + * error returns one display-ready message. JVM, bridge, and other unexpected failures are allowed + * to propagate to the caller. + */ +@Internal +public final class AgentPlanPreflight { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + /** + * Finds a validation error in a complete serialized AgentPlan. + * + * @return {@code null} when valid, otherwise a display-ready user-error message + */ + @Nullable + public static String findValidationError(String agentPlanJson) { + try { + MAPPER.readValue(agentPlanJson, AgentPlan.class); + return null; + } catch (JsonProcessingException error) { + String message = error.getOriginalMessage(); + return message == null || message.isEmpty() + ? error.getClass().getSimpleName() + : message; + } + } + + private AgentPlanPreflight() {} +} diff --git a/plan/src/main/java/org/apache/flink/agents/plan/actions/Action.java b/plan/src/main/java/org/apache/flink/agents/plan/actions/Action.java index 18e3b83b4..8fc634e0e 100644 --- a/plan/src/main/java/org/apache/flink/agents/plan/actions/Action.java +++ b/plan/src/main/java/org/apache/flink/agents/plan/actions/Action.java @@ -23,27 +23,28 @@ import org.apache.flink.agents.api.Event; import org.apache.flink.agents.api.context.RunnerContext; import org.apache.flink.agents.plan.Function; +import org.apache.flink.agents.plan.condition.ConditionExpressionValidator; +import org.apache.flink.agents.plan.condition.TriggerCondition; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; import org.apache.flink.agents.plan.serializer.ActionJsonDeserializer; import org.apache.flink.agents.plan.serializer.ActionJsonSerializer; import javax.annotation.Nullable; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; -/** - * Representation of an agent action with unified trigger conditions. - * - *

Each entry of {@code triggerConditions} is an event type name string. Multiple entries combine - * with OR. - */ +/** Representation of an agent action with raw and Plan-classified trigger conditions. */ @JsonSerialize(using = ActionJsonSerializer.class) @JsonDeserialize(using = ActionJsonDeserializer.class) public class Action { private final String name; private final Function exec; private final List triggerConditions; + private transient volatile List classifiedTriggerConditions; // TODO: support nested map/list with non primitive type value. @Nullable private final Map config; @@ -56,8 +57,14 @@ public Action( throws Exception { this.name = name; this.exec = exec; - this.triggerConditions = triggerConditions; + if (triggerConditions == null || triggerConditions.isEmpty()) { + throw new IllegalArgumentException( + "Action '" + name + "' must have at least one trigger condition"); + } + this.triggerConditions = new ArrayList<>(triggerConditions); + this.classifiedTriggerConditions = buildClassifiedTriggerConditions(); this.config = config; + exec.checkSignature(new Class[] {Event.class, RunnerContext.class}); } @@ -73,19 +80,17 @@ public Function getExec() { return exec; } - /** Returns the full trigger conditions list. */ + /** Returns raw event-type or condition entries in declaration order. */ public List getTriggerConditions() { - return triggerConditions; + return Collections.unmodifiableList(triggerConditions); } - /** - * Returns event-type names. Kept for callers that still consume the old naming; in this PR all - * trigger entries are plain event-type names so the list is identical to {@link - * #getTriggerConditions()}. A follow-up PR introduces CEL expressions and overrides this to - * filter out non-type entries. - */ - public List getListenEventTypes() { - return triggerConditions; + /** Returns immutable, validated trigger conditions in raw declaration order. */ + public List getClassifiedTriggerConditions() { + if (classifiedTriggerConditions == null) { + classifiedTriggerConditions = buildClassifiedTriggerConditions(); + } + return classifiedTriggerConditions; } @Nullable @@ -107,4 +112,30 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(name, exec, triggerConditions); } + + private List buildClassifiedTriggerConditions() { + List builtConditions = new ArrayList<>(triggerConditions.size()); + for (int index = 0; index < triggerConditions.size(); index++) { + String rawSource = triggerConditions.get(index); + try { + TriggerCondition condition = TriggerCondition.classify(rawSource); + if (condition instanceof ExpressionCondition) { + ConditionExpressionValidator.validate((ExpressionCondition) condition); + } + builtConditions.add(condition); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException( + "Invalid trigger condition #" + + (index + 1) + + " for action '" + + name + + "' from source \"" + + String.valueOf(rawSource) + + "\": " + + e.getMessage(), + e); + } + } + return Collections.unmodifiableList(builtConditions); + } } diff --git a/plan/src/main/java/org/apache/flink/agents/plan/condition/ConditionExpressionDialect.java b/plan/src/main/java/org/apache/flink/agents/plan/condition/ConditionExpressionDialect.java new file mode 100644 index 000000000..2e4893906 --- /dev/null +++ b/plan/src/main/java/org/apache/flink/agents/plan/condition/ConditionExpressionDialect.java @@ -0,0 +1,136 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan.condition; + +import com.google.common.collect.ImmutableList; +import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.CelOptions; +import dev.cel.common.CelValidationException; +import dev.cel.common.ast.CelExpr; +import dev.cel.parser.CelMacro; +import dev.cel.parser.CelMacroExprFactory; +import dev.cel.parser.CelParser; +import dev.cel.parser.CelParserFactory; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; + +/** Shared parsing and evaluation dialect for trigger-condition expressions. */ +public final class ConditionExpressionDialect { + + private static final CelOptions OPTIONS = + CelOptions.current() + .maxParseRecursionDepth(32) + .maxExpressionCodePointSize(8_192) + .comprehensionMaxIterations(1_000) + .build(); + + private static final CelMacro HAS = + CelMacro.newGlobalMacro("has", 1, ConditionExpressionDialect::expandHas); + + private static final CelParser PARSER = + CelParserFactory.standardCelParserBuilder().setOptions(OPTIONS).addMacros(HAS).build(); + + private static final String LOGICAL_AND_FUNCTION = "_&&_"; + private static final Set CEL_STANDARD_MACROS = + Set.of("has", "exists", "exists_one", "all", "filter", "map"); + private static final Set RESERVED_IDENTIFIERS; + + static { + Set reserved = + new HashSet<>( + Set.of( + "type", + "attributes", + "id", + "EventType", + "true", + "false", + "null", + "in", + "int", + "uint", + "double", + "string", + "bool", + "bytes", + "list")); + reserved.addAll(CEL_STANDARD_MACROS); + RESERVED_IDENTIFIERS = Collections.unmodifiableSet(reserved); + } + + /** Returns the shared parse and evaluation limits for this dialect. */ + public static CelOptions options() { + return OPTIONS; + } + + /** Parses one expression with the shared limits and custom presence helper. */ + public static CelAbstractSyntaxTree parse(String source) throws CelValidationException { + return PARSER.parse(source).getAst(); + } + + private static Optional expandHas( + CelMacroExprFactory expressionFactory, + CelExpr target, + ImmutableList arguments) { + CelExpr argument = arguments.get(0); + if (argument.exprKind().getKind() == CelExpr.ExprKind.Kind.SELECT + && !argument.select().testOnly()) { + return Optional.of(expandHasChain(expressionFactory, argument)); + } + if (argument.exprKind().getKind() == CelExpr.ExprKind.Kind.IDENT + && !RESERVED_IDENTIFIERS.contains(argument.ident().name())) { + return Optional.of( + expressionFactory.newSelect( + expressionFactory.newIdentifier("attributes"), + argument.ident().name(), + true)); + } + return Optional.of( + expressionFactory.reportError( + "invalid argument to has(): expected a field selection like" + + " has(a.b) or an attribute name like has(score)")); + } + + private static CelExpr expandHasChain( + CelMacroExprFactory expressionFactory, CelExpr selection) { + CelExpr operand = selection.select().operand(); + CelExpr presence = expressionFactory.newSelect(operand, selection.select().field(), true); + if (operand.exprKind().getKind() == CelExpr.ExprKind.Kind.SELECT + && !operand.select().testOnly()) { + return expressionFactory.newGlobalCall( + LOGICAL_AND_FUNCTION, expandHasChain(expressionFactory, operand), presence); + } + if (operand.exprKind().getKind() == CelExpr.ExprKind.Kind.IDENT + && !RESERVED_IDENTIFIERS.contains(operand.ident().name())) { + return expressionFactory.newGlobalCall( + LOGICAL_AND_FUNCTION, + expressionFactory.newSelect( + expressionFactory.newIdentifier("attributes"), + operand.ident().name(), + true), + presence); + } + return presence; + } + + private ConditionExpressionDialect() {} +} diff --git a/plan/src/main/java/org/apache/flink/agents/plan/condition/ConditionExpressionValidator.java b/plan/src/main/java/org/apache/flink/agents/plan/condition/ConditionExpressionValidator.java new file mode 100644 index 000000000..74f60ee41 --- /dev/null +++ b/plan/src/main/java/org/apache/flink/agents/plan/condition/ConditionExpressionValidator.java @@ -0,0 +1,153 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan.condition; + +import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.CelValidationException; +import dev.cel.common.ast.CelExpr; +import dev.cel.common.navigation.CelNavigableAst; +import org.apache.flink.agents.api.EventType; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; + +import java.util.Set; +import java.util.TreeSet; + +/** Performs Plan-time syntax and static-policy validation for trigger condition expressions. */ +public final class ConditionExpressionValidator { + + private static final Set DISALLOWED_STANDARD_MACROS = + Set.of("exists", "exists_one", "all", "filter", "map"); + private static final Set ALLOWED_MACROS = Set.of("has"); + + public enum ErrorCategory { + SYNTAX, + UNSUPPORTED_CONSTRUCT, + UNKNOWN_EVENT_TYPE + } + + /** A stable Plan-layer validation error with a machine-readable category. */ + public static final class ValidationException extends IllegalArgumentException { + private final ErrorCategory category; + + private ValidationException(ErrorCategory category, String message, Throwable cause) { + super(message, cause); + this.category = category; + } + + public ErrorCategory category() { + return category; + } + } + + public static void validate(ExpressionCondition condition) { + String source = condition.text(); + CelAbstractSyntaxTree parsed; + try { + parsed = ConditionExpressionDialect.parse(source); + } catch (CelValidationException e) { + throw new ValidationException( + ErrorCategory.SYNTAX, + "Invalid trigger condition expression \"" + source + "\": " + e.getMessage(), + e); + } + validateMacros(parsed, source); + validateEventTypeReferences(parsed); + validateStandalonePath(parsed, source); + } + + private static void validateMacros(CelAbstractSyntaxTree parsed, String source) { + String disallowedMacro = + CelNavigableAst.fromAst(parsed) + .getRoot() + .allNodes() + .filter(node -> node.getKind() == CelExpr.ExprKind.Kind.CALL) + .map(node -> node.expr()) + .filter( + expression -> { + CelExpr.CelCall call = expression.call(); + if (!DISALLOWED_STANDARD_MACROS.contains(call.function()) + || call.target().isEmpty() + || call.args().isEmpty() + || call.args().get(0).getKind() + != CelExpr.ExprKind.Kind.IDENT) { + return false; + } + int argumentCount = call.args().size(); + return "map".equals(call.function()) + ? argumentCount == 2 || argumentCount == 3 + : argumentCount == 2; + }) + .map(expression -> expression.call().function()) + .findFirst() + .orElse(null); + if (disallowedMacro != null) { + throw new ValidationException( + ErrorCategory.UNSUPPORTED_CONSTRUCT, + "Invalid trigger condition expression \"" + + source + + "\": unsupported construct '" + + disallowedMacro + + "'. Supported: " + + new TreeSet<>(ALLOWED_MACROS) + + ".", + null); + } + } + + private static void validateEventTypeReferences(CelAbstractSyntaxTree parsed) { + CelNavigableAst.fromAst(parsed) + .getRoot() + .allNodes() + .filter(node -> node.getKind() == CelExpr.ExprKind.Kind.SELECT) + .map(node -> node.expr().select()) + .filter( + selection -> + selection.operand().getKind() == CelExpr.ExprKind.Kind.IDENT + && "EventType".equals(selection.operand().ident().name())) + .forEach( + selection -> { + if (!EventType.allConstants().containsKey(selection.field())) { + throw new ValidationException( + ErrorCategory.UNKNOWN_EVENT_TYPE, + "Unknown EventType constant: EventType." + + selection.field(), + null); + } + }); + } + + private static void validateStandalonePath(CelAbstractSyntaxTree parsed, String source) { + CelExpr root = CelNavigableAst.fromAst(parsed).getRoot().expr(); + boolean standalonePath = + root.getKind() == CelExpr.ExprKind.Kind.IDENT + || (root.getKind() == CelExpr.ExprKind.Kind.SELECT + && !root.select().testOnly()); + if (standalonePath) { + throw new ValidationException( + ErrorCategory.UNSUPPORTED_CONSTRUCT, + "Invalid trigger condition expression \"" + + source + + "\": a standalone path is not a Boolean condition; use an explicit" + + " comparison.", + null); + } + } + + private ConditionExpressionValidator() {} +} diff --git a/plan/src/main/java/org/apache/flink/agents/plan/condition/TriggerCondition.java b/plan/src/main/java/org/apache/flink/agents/plan/condition/TriggerCondition.java new file mode 100644 index 000000000..c3ce41657 --- /dev/null +++ b/plan/src/main/java/org/apache/flink/agents/plan/condition/TriggerCondition.java @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan.condition; + +import java.util.Objects; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** One classified entry from an action's {@code trigger_conditions}. */ +public abstract class TriggerCondition { + + private static final Pattern EVENT_TYPE = + Pattern.compile( + "^(?:" + + "([A-Za-z_][A-Za-z0-9_-]*(?:\\.[A-Za-z_][A-Za-z0-9_-]*)*)" + + "|(['\"])([^\\s'\"\\\\\\p{Cntrl}]+)\\2" + + ")$"); + private static final Set EXPRESSION_LITERALS = Set.of("true", "false", "null"); + + TriggerCondition() {} + + /** Classifies an entry as an event-type condition or an expression condition. */ + public static TriggerCondition classify(String source) { + if (source == null || source.trim().isEmpty()) { + throw new IllegalArgumentException("Trigger condition must be non-null and non-blank"); + } + String entry = source.trim(); + Matcher matcher = EVENT_TYPE.matcher(entry); + if (matcher.matches()) { + String bareType = matcher.group(1); + String eventType = bareType != null ? bareType : matcher.group(3); + boolean reservedExpression = + eventType.startsWith("EventType.") + || (bareType != null && EXPRESSION_LITERALS.contains(bareType)); + if (!reservedExpression) { + return new EventTypeCondition(eventType); + } + } + return new ExpressionCondition(entry); + } + + /** A trigger condition that matches one exact event type. */ + public static final class EventTypeCondition extends TriggerCondition { + + private final String eventType; + + private EventTypeCondition(String eventType) { + this.eventType = eventType; + } + + public String eventType() { + return eventType; + } + + @Override + public boolean equals(Object other) { + return other instanceof EventTypeCondition + && eventType.equals(((EventTypeCondition) other).eventType); + } + + @Override + public int hashCode() { + return Objects.hash(eventType); + } + + @Override + public String toString() { + return "EventTypeCondition{eventType='" + eventType + "'}"; + } + } + + /** A trigger condition expressed as a Boolean expression. */ + public static final class ExpressionCondition extends TriggerCondition { + + private final String text; + + private ExpressionCondition(String text) { + this.text = text; + } + + public String text() { + return text; + } + + @Override + public boolean equals(Object other) { + return other instanceof ExpressionCondition + && text.equals(((ExpressionCondition) other).text); + } + + @Override + public int hashCode() { + return Objects.hash(text); + } + + @Override + public String toString() { + return "ExpressionCondition{text='" + text + "'}"; + } + } +} diff --git a/plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializer.java b/plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializer.java index efdda67fb..51e108c71 100644 --- a/plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializer.java +++ b/plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializer.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.deser.std.StdDeserializer; @@ -38,10 +39,7 @@ import static org.apache.flink.agents.plan.serializer.ActionJsonSerializer.CONFIG_TYPE; -/** - * Custom deserializer for {@link Action} that handles the deserialization of the function and event - * types. - */ +/** Custom deserializer for {@link Action} that handles its function and raw trigger conditions. */ public class ActionJsonDeserializer extends StdDeserializer { public ActionJsonDeserializer() { @@ -66,10 +64,27 @@ public Action deserialize(JsonParser jsonParser, DeserializationContext deserial throw new IOException("Unsupported function type: " + funcType); } - // Deserialize trigger_conditions + JsonNode triggerConditionsNode = node.get("trigger_conditions"); + if (triggerConditionsNode == null + || !triggerConditionsNode.isArray() + || triggerConditionsNode.isEmpty()) { + throw new IOException("Action field 'trigger_conditions' must be a non-empty array"); + } List triggerConditions = new ArrayList<>(); - node.get("trigger_conditions") - .forEach(entryNode -> triggerConditions.add(entryNode.asText())); + for (int index = 0; index < triggerConditionsNode.size(); index++) { + JsonNode entryNode = triggerConditionsNode.get(index); + if (!entryNode.isTextual() && !entryNode.isNull()) { + throw new IOException( + "Invalid trigger condition #" + + (index + 1) + + " for action '" + + name + + "' from source " + + entryNode + + ": entry must be a string"); + } + triggerConditions.add(entryNode.isNull() ? null : entryNode.textValue()); + } // Deserialize params Map config = null; @@ -89,9 +104,10 @@ public Action deserialize(JsonParser jsonParser, DeserializationContext deserial try { return new Action(name, func, triggerConditions, config); + } catch (IllegalArgumentException e) { + throw JsonMappingException.from(jsonParser, e.getMessage(), e); } catch (Exception e) { - throw new RuntimeException( - String.format("Failed to create Action with name \"%s\"", name), e); + throw new IOException("Failed to create Action '" + name + "': " + e.getMessage(), e); } } diff --git a/plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonSerializer.java b/plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonSerializer.java index 85e46823e..0e1285368 100644 --- a/plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonSerializer.java +++ b/plan/src/main/java/org/apache/flink/agents/plan/serializer/ActionJsonSerializer.java @@ -28,10 +28,7 @@ import java.io.IOException; import java.util.Map; -/** - * Custom serializer for {@link Action} that handles the serialization of the function and event - * types. - */ +/** Custom serializer for {@link Action} that handles its function and raw trigger conditions. */ public class ActionJsonSerializer extends StdSerializer { public static final String CONFIG_TYPE = "__config_type__"; private static final String PYTHON_FUNC_TYPE = "PythonFunction"; @@ -62,7 +59,6 @@ public void serialize( "Unsupported function type: " + action.getExec().getClass().getName()); } - // Write trigger_conditions field jsonGenerator.writeFieldName("trigger_conditions"); jsonGenerator.writeStartArray(); for (String entry : action.getTriggerConditions()) { @@ -80,39 +76,34 @@ public void serialize( String configType = (String) config.get(CONFIG_TYPE); if (configType == null) { configType = "java"; - config.put(CONFIG_TYPE, configType); } if (configType.equals("java")) { - action.getConfig() - .forEach( - (name, value) -> { - try { - if (CONFIG_TYPE.equals(name)) { - jsonGenerator.writeStringField(name, (String) value); - } else { - jsonGenerator.writeFieldName(name); - jsonGenerator.writeStartObject(); - jsonGenerator.writeStringField( - "@class", value.getClass().getName()); - jsonGenerator.writeObjectField("value", value); - jsonGenerator.writeEndObject(); - } - } catch (IOException e) { - throw new RuntimeException( - "Error writing action: " + name, e); - } - }); + jsonGenerator.writeStringField(CONFIG_TYPE, configType); + config.forEach( + (name, value) -> { + if (CONFIG_TYPE.equals(name)) { + return; + } + try { + jsonGenerator.writeFieldName(name); + jsonGenerator.writeStartObject(); + jsonGenerator.writeStringField( + "@class", value.getClass().getName()); + jsonGenerator.writeObjectField("value", value); + jsonGenerator.writeEndObject(); + } catch (IOException e) { + throw new RuntimeException("Error writing action: " + name, e); + } + }); } else if (configType.equals("python")) { - action.getConfig() - .forEach( - (name, value) -> { - try { - jsonGenerator.writeObjectField(name, value); - } catch (IOException e) { - throw new RuntimeException( - "Error writing action: " + name, e); - } - }); + config.forEach( + (name, value) -> { + try { + jsonGenerator.writeObjectField(name, value); + } catch (IOException e) { + throw new RuntimeException("Error writing action: " + name, e); + } + }); } else { throw new IllegalArgumentException( String.format("Unknown config type %s", configType)); diff --git a/plan/src/main/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonDeserializer.java b/plan/src/main/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonDeserializer.java index a39503ec7..caaee783a 100644 --- a/plan/src/main/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonDeserializer.java +++ b/plan/src/main/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonDeserializer.java @@ -34,10 +34,9 @@ import org.apache.flink.agents.plan.resourceprovider.ResourceProvider; import java.io.IOException; -import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; -import java.util.List; +import java.util.LinkedHashMap; import java.util.Map; public class AgentPlanJsonDeserializer extends StdDeserializer { @@ -57,39 +56,18 @@ public AgentPlan deserialize(JsonParser parser, DeserializationContext ctx) JavaType actionType = ctx.constructType(Action.class); JsonDeserializer actionDeserializer = ctx.findContextualValueDeserializer(actionType, null); - Map actions = new HashMap<>(); - if (actionsNode != null && actionsNode.isObject()) { - Iterator> iterator = actionsNode.fields(); - while (iterator.hasNext()) { - Map.Entry entry = iterator.next(); - String actionName = entry.getKey(); - JsonNode actionNode = entry.getValue(); - JsonParser actionParser = codec.treeAsTokens(actionNode); - Action action = (Action) actionDeserializer.deserialize(actionParser, ctx); - actions.put(actionName, action); - } + if (actionsNode == null || !actionsNode.isObject()) { + throw new IOException("AgentPlan field 'actions' must be an object"); } - - // Deserialize event trigger actions - JsonNode actionsByEventNode = node.get("actions_by_event"); - Map> actionsByEvent = new HashMap<>(); - if (actionsByEventNode != null && actionsByEventNode.isObject()) { - Iterator> iterator = actionsByEventNode.fields(); - while (iterator.hasNext()) { - Map.Entry entry = iterator.next(); - String eventClassName = entry.getKey(); - JsonNode actionsArrayNode = entry.getValue(); - List actionsTriggeredByEvent = new ArrayList<>(); - for (JsonNode actionNameNode : actionsArrayNode) { - String actionName = actionNameNode.asText(); - Action action = actions.get(actionName); - if (action == null) { - throw new IllegalStateException("Unknown action name: " + actionName); - } - actionsTriggeredByEvent.add(action); - } - actionsByEvent.put(eventClassName, actionsTriggeredByEvent); - } + Map actions = new LinkedHashMap<>(); + Iterator> actionIterator = actionsNode.fields(); + while (actionIterator.hasNext()) { + Map.Entry entry = actionIterator.next(); + String actionName = entry.getKey(); + JsonNode actionNode = entry.getValue(); + JsonParser actionParser = codec.treeAsTokens(actionNode); + Action action = (Action) actionDeserializer.deserialize(actionParser, ctx); + actions.put(actionName, action); } // Deserialize resource providers @@ -133,6 +111,6 @@ public AgentPlan deserialize(JsonParser parser, DeserializationContext ctx) } AgentConfiguration config = new AgentConfiguration(configData); - return new AgentPlan(actions, actionsByEvent, resourceProviders, config); + return new AgentPlan(actions, resourceProviders, config); } } diff --git a/plan/src/main/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonSerializer.java b/plan/src/main/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonSerializer.java index 6b887f0c6..4a1494ff1 100644 --- a/plan/src/main/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonSerializer.java +++ b/plan/src/main/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonSerializer.java @@ -59,28 +59,6 @@ public void serialize( }); jsonGenerator.writeEndObject(); - // Serialize event trigger actions - jsonGenerator.writeFieldName("actions_by_event"); - jsonGenerator.writeStartObject(); - agentPlan - .getActionsByEvent() - .forEach( - (eventClass, actions) -> { - try { - jsonGenerator.writeFieldName(eventClass); - jsonGenerator.writeStartArray(); - for (Action action : actions) { - jsonGenerator.writeString(action.getName()); - } - jsonGenerator.writeEndArray(); - } catch (IOException e) { - throw new RuntimeException( - "Error writing event trigger actions for: " + eventClass, - e); - } - }); - jsonGenerator.writeEndObject(); - // Serialize resource providers jsonGenerator.writeFieldName("resource_providers"); jsonGenerator.writeStartObject(); diff --git a/plan/src/test/java/org/apache/flink/agents/plan/AgentConfigurationTest.java b/plan/src/test/java/org/apache/flink/agents/plan/AgentConfigurationTest.java index 125c737de..43b31eafa 100644 --- a/plan/src/test/java/org/apache/flink/agents/plan/AgentConfigurationTest.java +++ b/plan/src/test/java/org/apache/flink/agents/plan/AgentConfigurationTest.java @@ -18,6 +18,8 @@ package org.apache.flink.agents.plan; +import org.apache.flink.agents.api.configuration.AgentConfigOptions; +import org.apache.flink.agents.api.configuration.AgentConfigOptions.ConditionEvaluationFailureStrategy; import org.apache.flink.agents.api.configuration.ConfigOption; import org.junit.jupiter.api.*; @@ -190,6 +192,19 @@ void testGetWithConfigOption() { assertNull(config.get(missingKey)); } + @Test + void conditionFailureStrategyContract() { + ConfigOption option = + AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY; + + assertEquals("action.trigger-condition.evaluate-failure-strategy", option.getKey()); + assertEquals(ConditionEvaluationFailureStrategy.class, option.getType()); + assertEquals(ConditionEvaluationFailureStrategy.WARN_AND_SKIP, option.getDefaultValue()); + + AgentConfiguration config = new AgentConfiguration(Map.of(option.getKey(), "FAIL")); + assertEquals(ConditionEvaluationFailureStrategy.FAIL, config.get(option)); + } + @Test void testGetWithDefaultValue() { ConfigOption defaultStr = diff --git a/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanCrossLanguageTest.java b/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanCrossLanguageTest.java index 3f55b0915..8a50334dd 100644 --- a/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanCrossLanguageTest.java +++ b/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanCrossLanguageTest.java @@ -23,8 +23,11 @@ import org.apache.flink.agents.api.Event; import org.apache.flink.agents.api.InputEvent; import org.apache.flink.agents.api.agents.Agent; +import org.apache.flink.agents.api.configuration.AgentConfigOptions; +import org.apache.flink.agents.api.configuration.AgentConfigOptions.ConditionEvaluationFailureStrategy; import org.apache.flink.agents.api.context.RunnerContext; import org.apache.flink.agents.plan.actions.Action; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -98,7 +101,10 @@ private static org.apache.flink.agents.api.function.PythonFunction pythonFunctio private static AgentPlan compileWithPythonAction() throws Exception { Agent agent = new Agent(); agent.addAction( - "handle", new String[] {InputEvent.EVENT_TYPE}, pythonFunctionDescriptor(), null); + "handle", + new String[] {InputEvent.EVENT_TYPE, "attributes.ready == true"}, + pythonFunctionDescriptor(), + null); return new AgentPlan(agent); } @@ -137,6 +143,8 @@ void compileAgentWithPythonFunctionDescriptor() throws Exception { assertThat(planFn.getModule()) .isEqualTo("flink_agents.plan.tests.test_agent_plan_cross_language"); assertThat(planFn.getQualName()).isEqualTo("_dummy_action"); + assertThat(action.getTriggerConditions()) + .containsExactly(InputEvent.EVENT_TYPE, "attributes.ready == true"); } @Test @@ -217,6 +225,10 @@ void javaPlanWithPythonActionRoundTripsThroughJson() throws Exception { assertThat(pf.getModule()) .isEqualTo("flink_agents.plan.tests.test_agent_plan_cross_language"); assertThat(pf.getQualName()).isEqualTo("_dummy_action"); + assertThat(action.getTriggerConditions()) + .containsExactly(InputEvent.EVENT_TYPE, "attributes.ready == true"); + assertThat(action.getClassifiedTriggerConditions().get(1)) + .isInstanceOf(ExpressionCondition.class); } // ── Cross-language snapshot (Java writes / Python reads) ─────────────── @@ -265,5 +277,13 @@ void javaCanDeserializePythonPlanWithJavaAction() throws Exception { JavaFunction jf = (JavaFunction) handle.getExec(); assertThat(jf.getQualName()).isEqualTo("com.example.Handlers"); assertThat(jf.getMethodName()).isEqualTo("handle"); + assertThat(handle.getTriggerConditions()) + .containsExactly(InputEvent.EVENT_TYPE, "attributes.ready == true"); + assertThat(handle.getClassifiedTriggerConditions().get(1)) + .isInstanceOf(ExpressionCondition.class); + assertThat( + restored.getConfig() + .get(AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY)) + .isEqualTo(ConditionEvaluationFailureStrategy.FAIL); } } diff --git a/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanPreflightTest.java b/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanPreflightTest.java new file mode 100644 index 000000000..2adce169d --- /dev/null +++ b/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanPreflightTest.java @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +class AgentPlanPreflightTest { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @Test + void returnsNullForValidPlan() throws Exception { + String planJson = + planJson( + List.of( + action( + "valid", + List.of( + "_input_event", + "type == EventType.InputEvent && input > 2")))); + + assertThat(AgentPlanPreflight.findValidationError(planJson)).isNull(); + } + + @Test + void returnsTriggerConditionErrorMessage() throws Exception { + String source = "type =="; + String planJson = + planJson( + List.of(action("failing", List.of("_input_event", source, "other.event")))); + + String errorMessage = AgentPlanPreflight.findValidationError(planJson); + + assertThat(errorMessage) + .contains("trigger condition #2") + .contains("action 'failing'") + .contains(source); + } + + @Test + void returnsInvalidPlanShapeError() throws Exception { + String planJson = planJson(List.of(action("empty", List.of()))); + + String errorMessage = AgentPlanPreflight.findValidationError(planJson); + + assertThat(errorMessage).contains("trigger_conditions"); + } + + private static ObjectNode action(String name, List triggerConditions) { + ObjectNode action = MAPPER.createObjectNode(); + action.put("name", name); + ObjectNode exec = action.putObject("exec"); + exec.put("func_type", "PythonFunction"); + exec.put("module", "example"); + exec.put("qualname", "handle"); + ArrayNode conditions = action.putArray("trigger_conditions"); + triggerConditions.forEach(conditions::add); + action.putNull("config"); + return action; + } + + private static String planJson(List actions) throws Exception { + ObjectNode plan = MAPPER.createObjectNode(); + ObjectNode actionMapping = plan.putObject("actions"); + for (ObjectNode action : actions) { + actionMapping.set(action.get("name").asText(), action); + } + plan.putObject("resource_providers"); + plan.putObject("config").putObject("conf_data"); + return MAPPER.writeValueAsString(plan); + } +} diff --git a/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanTest.java b/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanTest.java index 9e355feb7..3a0a2a3fa 100644 --- a/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanTest.java +++ b/plan/src/test/java/org/apache/flink/agents/plan/AgentPlanTest.java @@ -27,6 +27,7 @@ import org.apache.flink.agents.api.annotation.MCPServer; import org.apache.flink.agents.api.annotation.Tool; import org.apache.flink.agents.api.context.RunnerContext; +import org.apache.flink.agents.api.function.PythonFunction; import org.apache.flink.agents.api.resource.Resource; import org.apache.flink.agents.api.resource.ResourceContext; import org.apache.flink.agents.api.resource.ResourceDescriptor; @@ -35,6 +36,7 @@ import org.apache.flink.agents.api.resource.SerializableResource; import org.apache.flink.agents.api.resource.python.PythonResourceAdapter; import org.apache.flink.agents.api.resource.python.PythonResourceWrapper; +import org.apache.flink.agents.api.yaml.YamlLoader; import org.apache.flink.agents.plan.actions.Action; import org.apache.flink.agents.plan.resourceprovider.JavaResourceProvider; import org.apache.flink.agents.plan.resourceprovider.JavaSerializableResourceProvider; @@ -42,8 +44,11 @@ import org.apache.flink.agents.plan.resourceprovider.ResourceProvider; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import pemja.core.object.PyObject; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import java.util.Map; @@ -198,7 +203,7 @@ public void testConstructorWithAgent() throws Exception { Action inputAction = agentPlan.getActions().get("handleInputEvent"); assertThat(inputAction).isNotNull(); assertThat(inputAction.getName()).isEqualTo("handleInputEvent"); - assertThat(inputAction.getListenEventTypes()).isEqualTo(List.of(InputEvent.EVENT_TYPE)); + assertThat(inputAction.getTriggerConditions()).isEqualTo(List.of(InputEvent.EVENT_TYPE)); assertThat(inputAction.getExec()).isInstanceOf(JavaFunction.class); // Check that the JavaFunction instance has the correct method/class/params @@ -211,30 +216,56 @@ public void testConstructorWithAgent() throws Exception { Action multiAction = agentPlan.getActions().get("handleMultipleEvents"); assertThat(multiAction).isNotNull(); assertThat(multiAction.getName()).isEqualTo("handleMultipleEvents"); - assertThat(multiAction.getListenEventTypes()) + assertThat(multiAction.getTriggerConditions()) .isEqualTo(List.of(TestEvent.EVENT_TYPE, OutputEvent.EVENT_TYPE)); assertThat(multiAction.getExec()).isInstanceOf(JavaFunction.class); + } - // Verify actionsByEvent mapping - assertThat(agentPlan.getActionsByEvent().size()).isEqualTo(7); - - // Check InputEvent mapping - List inputEventActions = agentPlan.getActionsByEvent().get(InputEvent.EVENT_TYPE); - assertThat(inputEventActions).isNotNull(); - assertThat(inputEventActions.size()).isEqualTo(1); - assertThat(inputEventActions.get(0).getName()).isEqualTo("handleInputEvent"); - - // Check TestEvent mapping - List testEventActions = agentPlan.getActionsByEvent().get(TestEvent.EVENT_TYPE); - assertThat(testEventActions).isNotNull(); - assertThat(testEventActions.size()).isEqualTo(1); - assertThat(testEventActions.get(0).getName()).isEqualTo("handleMultipleEvents"); + @Test + public void rejectsInvalidConditionsInPlan() { + for (String[] invalidConditions : + List.of(new String[0], new String[] {" "}, new String[] {null})) { + Agent agent = new Agent(); + agent.addAction( + "invalid", invalidConditions, new PythonFunction("pkg", "function"), null); + + assertThat(agent.getActions()).containsKey("invalid"); + assertThatThrownBy(() -> new AgentPlan(agent)) + .isInstanceOf(IllegalArgumentException.class); + } + } - // Check OutputEvent mapping - List outputEventActions = agentPlan.getActionsByEvent().get(OutputEvent.EVENT_TYPE); - assertThat(outputEventActions).isNotNull(); - assertThat(outputEventActions.size()).isEqualTo(1); - assertThat(outputEventActions.get(0).getName()).isEqualTo("handleMultipleEvents"); + @Test + public void preservesYamlActionOrder(@TempDir Path tmp) throws Exception { + Path file = tmp.resolve("ordered_actions.yaml"); + Files.writeString( + file, + "agents:\n" + + " - name: a\n" + + " actions:\n" + + " - name: first\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [input]\n" + + " - name: second\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [input]\n" + + " - name: third\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [input]\n" + + " - name: fourth\n" + + " function: pkg.mod:fn\n" + + " trigger_conditions: [input]\n"); + Agent agent = YamlLoader.buildAgents(file).getAgents().get("a"); + + assertThat(new AgentPlan(agent).getActions().keySet()) + .containsExactly( + "chat_model_action", + "tool_call_action", + "context_retrieval_action", + "first", + "second", + "third", + "fourth"); } @Test @@ -249,7 +280,6 @@ public void testConstructorWithAgentNoActions() throws Exception { // Verify that no actions were collected assertThat(agentPlan.getActions().size()).isEqualTo(3); - assertThat(agentPlan.getActionsByEvent().size()).isEqualTo(4); } @Test @@ -291,7 +321,7 @@ public void testActionWithPythonTargetCompilesToPythonFunctionExec() throws Exce (org.apache.flink.agents.plan.PythonFunction) action.getExec(); assertThat(exec.getModule()).isEqualTo("my_pkg.handlers"); assertThat(exec.getQualName()).isEqualTo("handle_input"); - assertThat(action.getListenEventTypes()).containsExactly(InputEvent.EVENT_TYPE); + assertThat(action.getTriggerConditions()).containsExactly(InputEvent.EVENT_TYPE); } /** Plain {@code @Action} (no {@code target}) compiles to a native Java exec. */ @@ -412,14 +442,16 @@ public void testAgentAddAction() throws Exception { Assertions.assertEquals(expectedInputAction.getName(), actualInputAction.getName()); Assertions.assertEquals(expectedInputAction.getExec(), actualInputAction.getExec()); Assertions.assertEquals( - expectedInputAction.getListenEventTypes(), actualInputAction.getListenEventTypes()); + expectedInputAction.getTriggerConditions(), + actualInputAction.getTriggerConditions()); expectedInputAction = expectedPlan.getActions().get("handleMultipleEvents"); actualInputAction = actualPlan.getActions().get("handleMultipleEvents"); Assertions.assertEquals(expectedInputAction.getName(), actualInputAction.getName()); Assertions.assertEquals(expectedInputAction.getExec(), actualInputAction.getExec()); Assertions.assertEquals( - expectedInputAction.getListenEventTypes(), actualInputAction.getListenEventTypes()); + expectedInputAction.getTriggerConditions(), + actualInputAction.getTriggerConditions()); Assertions.assertEquals( 123, actualPlan.getActionConfigValue("handleMultipleEvents", "key")); } diff --git a/plan/src/test/java/org/apache/flink/agents/plan/TriggerConditionContractTest.java b/plan/src/test/java/org/apache/flink/agents/plan/TriggerConditionContractTest.java new file mode 100644 index 000000000..5e08d8da3 --- /dev/null +++ b/plan/src/test/java/org/apache/flink/agents/plan/TriggerConditionContractTest.java @@ -0,0 +1,120 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.flink.agents.api.Event; +import org.apache.flink.agents.api.agents.Agent; +import org.apache.flink.agents.api.context.RunnerContext; +import org.apache.flink.agents.plan.actions.Action; +import org.apache.flink.agents.plan.condition.TriggerCondition; +import org.junit.jupiter.api.Test; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class TriggerConditionContractTest { + public static void handler(Event event, RunnerContext context) {} + + static class TriggerConditionAgent extends Agent { + @org.apache.flink.agents.api.annotation.Action(" type == 'x' ") + public void conditionOnly(Event event, RunnerContext context) {} + + @org.apache.flink.agents.api.annotation.Action({"x", " score > 1 ", "x"}) + public void mixedOr(Event event, RunnerContext context) {} + + @org.apache.flink.agents.api.annotation.Action("order-created") + public void hyphenatedEventType(Event event, RunnerContext context) {} + } + + private static JavaFunction function() throws Exception { + return new JavaFunction( + TriggerConditionContractTest.class.getName(), + "handler", + new Class[] {Event.class, RunnerContext.class}); + } + + @Test + void planJsonOmitsDerivedRoutingState() throws Exception { + Action action = new Action("a", function(), List.of("x", "score > 1"), null); + Map source = new LinkedHashMap<>(); + source.put("a", action); + AgentPlan plan = new AgentPlan(source); + source.clear(); + + assertThat(plan.getActions()).containsOnlyKeys("a"); + assertThatThrownBy(() -> plan.getActions().clear()) + .isInstanceOf(UnsupportedOperationException.class); + + String json = new ObjectMapper().writeValueAsString(plan); + assertThat(json).contains("trigger_conditions").doesNotContain("actions_by_event"); + } + + @Test + void javaRoundTripRebuildsConditions() throws Exception { + Action raw = + new Action( + "raw", function(), List.of("x", " score > 1 ", "x", "type == 'x'"), null); + AgentPlan restored = javaRoundTrip(new AgentPlan(Map.of("raw", raw))); + + assertThat(restored.getActions().get("raw").getTriggerConditions()) + .containsExactly("x", " score > 1 ", "x", "type == 'x'"); + assertThat(restored.getActions().get("raw").getClassifiedTriggerConditions()) + .containsExactly( + TriggerCondition.classify("x"), + TriggerCondition.classify("score > 1"), + TriggerCondition.classify("x"), + TriggerCondition.classify("type == 'x'")); + assertThatThrownBy(() -> restored.getActions().clear()) + .isInstanceOf(UnsupportedOperationException.class); + assertThatThrownBy(() -> restored.getActions().get("raw").getTriggerConditions().clear()) + .isInstanceOf(UnsupportedOperationException.class); + } + + @Test + void annotationKeepsConditionOrder() throws Exception { + AgentPlan plan = new AgentPlan(new TriggerConditionAgent()); + + assertThat(plan.getActions().get("conditionOnly").getTriggerConditions()) + .containsExactly(" type == 'x' "); + assertThat(plan.getActions().get("mixedOr").getTriggerConditions()) + .containsExactly("x", " score > 1 ", "x"); + assertThat(plan.getActions().get("hyphenatedEventType").getClassifiedTriggerConditions()) + .containsExactly(TriggerCondition.classify("order-created")); + } + + private static AgentPlan javaRoundTrip(AgentPlan plan) throws Exception { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ObjectOutputStream output = new ObjectOutputStream(bytes)) { + output.writeObject(plan); + } + try (ObjectInputStream input = + new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray()))) { + return (AgentPlan) input.readObject(); + } + } +} diff --git a/plan/src/test/java/org/apache/flink/agents/plan/actions/ActionTriggerConditionsTest.java b/plan/src/test/java/org/apache/flink/agents/plan/actions/ActionTriggerConditionsTest.java new file mode 100644 index 000000000..e57c014a3 --- /dev/null +++ b/plan/src/test/java/org/apache/flink/agents/plan/actions/ActionTriggerConditionsTest.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan.actions; + +import org.apache.flink.agents.api.Event; +import org.apache.flink.agents.api.context.RunnerContext; +import org.apache.flink.agents.plan.JavaFunction; +import org.apache.flink.agents.plan.condition.TriggerCondition; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class ActionTriggerConditionsTest { + + public static void handler(Event event, RunnerContext context) {} + + @Test + void keepsRawAndClassifiedConditions() throws Exception { + List rawEntries = new ArrayList<>(List.of(" a.b.c ", " score > 1 ")); + Action action = new Action("mixed", function(), rawEntries, null); + rawEntries.clear(); + + assertThat(action.getTriggerConditions()).containsExactly(" a.b.c ", " score > 1 "); + assertThatThrownBy(() -> action.getTriggerConditions().clear()) + .isInstanceOf(UnsupportedOperationException.class); + assertThatThrownBy(() -> action.getTriggerConditions().add("late.condition")) + .isInstanceOf(UnsupportedOperationException.class); + assertThat(action.getClassifiedTriggerConditions()) + .containsExactly( + TriggerCondition.classify("a.b.c"), TriggerCondition.classify("score > 1")); + assertThatThrownBy(() -> action.getClassifiedTriggerConditions().clear()) + .isInstanceOf(UnsupportedOperationException.class); + } + + @Test + void reportsInvalidConditionContext() throws Exception { + assertThatThrownBy( + () -> + new Action( + "failing", + function(), + List.of("valid.event", " type == "), + null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("trigger condition #2") + .hasMessageContaining("action 'failing'") + .hasMessageContaining("source \" type == \""); + } + + private static JavaFunction function() throws Exception { + return new JavaFunction( + ActionTriggerConditionsTest.class.getName(), + "handler", + new Class[] {Event.class, RunnerContext.class}); + } +} diff --git a/plan/src/test/java/org/apache/flink/agents/plan/compatibility/CreateJavaAgentPlanFromJson.java b/plan/src/test/java/org/apache/flink/agents/plan/compatibility/CreateJavaAgentPlanFromJson.java index 8df4bace8..3458b0892 100644 --- a/plan/src/test/java/org/apache/flink/agents/plan/compatibility/CreateJavaAgentPlanFromJson.java +++ b/plan/src/test/java/org/apache/flink/agents/plan/compatibility/CreateJavaAgentPlanFromJson.java @@ -69,7 +69,7 @@ public static void main(String[] args) throws IOException { assertEquals( "PythonAgentPlanCompatibilityTestAgent.first_action", firstActionFunction.getQualName()); - assertEquals(List.of(inputEvent), firstAction.getListenEventTypes()); + assertEquals(List.of(inputEvent), firstAction.getTriggerConditions()); // Check the second action assertTrue(agentPlan.getActions().containsKey("second_action")); @@ -80,7 +80,7 @@ public static void main(String[] args) throws IOException { assertEquals( "PythonAgentPlanCompatibilityTestAgent.second_action", secondActionFunc.getQualName()); - assertEquals(List.of(inputEvent, myEvent), secondAction.getListenEventTypes()); + assertEquals(List.of(inputEvent, myEvent), secondAction.getTriggerConditions()); // Check the built-in actions assertTrue(agentPlan.getActions().containsKey("chat_model_action")); @@ -94,7 +94,7 @@ public static void main(String[] args) throws IOException { String toolResponseEvent = "_tool_response_event"; assertEquals( List.of(chatRequestEvent, toolResponseEvent), - chatModelAction.getListenEventTypes()); + chatModelAction.getTriggerConditions()); assertTrue(agentPlan.getActions().containsKey("tool_call_action")); Action toolCallAction = agentPlan.getActions().get("tool_call_action"); @@ -104,7 +104,7 @@ public static void main(String[] args) throws IOException { "flink_agents.plan.actions.tool_call_action", processToolRequestFunc.getModule()); assertEquals("process_tool_request", processToolRequestFunc.getQualName()); String toolRequestEvent = "_tool_request_event"; - assertEquals(List.of(toolRequestEvent), toolCallAction.getListenEventTypes()); + assertEquals(List.of(toolRequestEvent), toolCallAction.getTriggerConditions()); assertTrue(agentPlan.getActions().containsKey("context_retrieval_action")); Action contextRetrievalAction = agentPlan.getActions().get("context_retrieval_action"); @@ -120,25 +120,7 @@ public static void main(String[] args) throws IOException { String contextRetrievalRequestEvent = "_context_retrieval_request_event"; assertEquals( List.of(contextRetrievalRequestEvent), - contextRetrievalAction.getListenEventTypes()); - - // Check event trigger actions - Map> actionsByEvent = agentPlan.getActionsByEvent(); - assertEquals(6, actionsByEvent.size()); - assertTrue(actionsByEvent.containsKey(inputEvent)); - assertTrue(actionsByEvent.containsKey(myEvent)); - assertTrue(actionsByEvent.containsKey(chatRequestEvent)); - assertTrue(actionsByEvent.containsKey(toolRequestEvent)); - assertTrue(actionsByEvent.containsKey(toolResponseEvent)); - assertTrue(actionsByEvent.containsKey(contextRetrievalRequestEvent)); - assertEquals( - List.of(firstAction, secondAction), agentPlan.getActionsByEvent().get(inputEvent)); - assertEquals(List.of(secondAction), agentPlan.getActionsByEvent().get(myEvent)); - assertEquals(List.of(chatModelAction), actionsByEvent.get(chatRequestEvent)); - assertEquals(List.of(toolCallAction), actionsByEvent.get(toolRequestEvent)); - assertEquals(List.of(chatModelAction), actionsByEvent.get(toolResponseEvent)); - assertEquals( - List.of(contextRetrievalAction), actionsByEvent.get(contextRetrievalRequestEvent)); + contextRetrievalAction.getTriggerConditions()); // Check resource providers Map kwargs = new HashMap<>(); diff --git a/plan/src/test/java/org/apache/flink/agents/plan/condition/ConditionExpressionValidatorTest.java b/plan/src/test/java/org/apache/flink/agents/plan/condition/ConditionExpressionValidatorTest.java new file mode 100644 index 000000000..e221805ac --- /dev/null +++ b/plan/src/test/java/org/apache/flink/agents/plan/condition/ConditionExpressionValidatorTest.java @@ -0,0 +1,180 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan.condition; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.io.InputStream; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.catchThrowableOfType; + +class ConditionExpressionValidatorTest { + + private static final String MACRO_FIXTURE_RESOURCE = "/disallowed_macros.yaml"; + + @Test + void rejectsSyntaxErrors() { + ConditionExpressionValidator.ValidationException error = + catchThrowableOfType( + () -> validateExpression("type =="), + ConditionExpressionValidator.ValidationException.class); + + assertThat(error.category()).isEqualTo(ConditionExpressionValidator.ErrorCategory.SYNTAX); + assertThat(error).hasMessageContaining("type =="); + } + + static Stream rejectedMacroFixtureCases() throws IOException { + return loadMacroFixture().get("reject").stream().map(Arguments::of); + } + + static Stream acceptedMacroFixtureCases() throws IOException { + return loadMacroFixture().get("accept").stream().map(Arguments::of); + } + + @ParameterizedTest(name = "reject unsupported macro: {0}") + @MethodSource("rejectedMacroFixtureCases") + void rejectsUnsupportedMacros(String source) { + ConditionExpressionValidator.ValidationException error = + catchThrowableOfType( + () -> validateExpression(source), + ConditionExpressionValidator.ValidationException.class); + + assertThat(error).isNotNull(); + assertThat(error.category()) + .isEqualTo(ConditionExpressionValidator.ErrorCategory.UNSUPPORTED_CONSTRUCT); + } + + @ParameterizedTest(name = "accept supported expression: {0}") + @MethodSource("acceptedMacroFixtureCases") + void acceptsSupportedExpressions(String source) { + assertThatCode(() -> validateExpression(source)).doesNotThrowAnyException(); + } + + @Test + void defersNonMacroCallsToRuntime() { + for (String source : new String[] {"all(1)", "map(1)", "foo.map()"}) { + assertThatCode(() -> validateExpression(source)).doesNotThrowAnyException(); + } + } + + @Test + void validatesEventTypeReferences() { + assertThatCode(() -> validateExpression("type == EventType.InputEvent")) + .doesNotThrowAnyException(); + + ConditionExpressionValidator.ValidationException error = + catchThrowableOfType( + () -> validateExpression("type == EventType.NotARealEvent"), + ConditionExpressionValidator.ValidationException.class); + assertThat(error.category()) + .isEqualTo(ConditionExpressionValidator.ErrorCategory.UNKNOWN_EVENT_TYPE); + assertThat(error) + .hasMessageContaining("Unknown EventType constant") + .hasMessageContaining("EventType.NotARealEvent"); + } + + @Test + void defersResultTypeCheckToRuntime() { + for (String source : + new String[] { + "true", + "false", + "null", + "42", + "'not a routable name'", + "[1, 2]", + "{'key': 1}", + "1 + 2", + "size(attributes)", + "attributes['key']", + "noSuchFn(1)" + }) { + assertThatCode(() -> validateExpression(source)).doesNotThrowAnyException(); + } + } + + @Test + void rejectsStandalonePaths() { + for (String source : + new String[] { + "(a.b.c)", "a . b . c", "a . b. c", "(score)", "EventType.InputEvent" + }) { + ConditionExpressionValidator.ValidationException error = + catchThrowableOfType( + () -> validateExpression(source), + ConditionExpressionValidator.ValidationException.class); + + assertThat(error).as(source).isNotNull(); + assertThat(error.category()) + .as(source) + .isEqualTo(ConditionExpressionValidator.ErrorCategory.UNSUPPORTED_CONSTRUCT); + assertThat(error).as(source).hasMessageContaining("explicit comparison"); + } + } + + @Test + void allowsBooleanPathConditions() { + for (String source : new String[] {"a.b.c == true", "has(a.b.c)"}) { + assertThatCode(() -> validateExpression(source)).as(source).doesNotThrowAnyException(); + } + } + + @Test + void enforcesSyntaxResourceLimits() { + String oversized = "true" + " || true".repeat(2_000); + String tooDeep = "(".repeat(40) + "true" + ")".repeat(40); + + for (String source : new String[] {oversized, tooDeep}) { + ConditionExpressionValidator.ValidationException error = + catchThrowableOfType( + () -> validateExpression(source), + ConditionExpressionValidator.ValidationException.class); + assertThat(error.category()) + .isEqualTo(ConditionExpressionValidator.ErrorCategory.SYNTAX); + } + } + + private static void validateExpression(String source) { + TriggerCondition triggerCondition = TriggerCondition.classify(source); + assertThat(triggerCondition).isInstanceOf(ExpressionCondition.class); + ConditionExpressionValidator.validate((ExpressionCondition) triggerCondition); + } + + @SuppressWarnings("unchecked") + private static Map> loadMacroFixture() throws IOException { + InputStream input = + ConditionExpressionValidatorTest.class.getResourceAsStream(MACRO_FIXTURE_RESOURCE); + assertThat(input) + .as("Shared macro fixture must exist: %s", MACRO_FIXTURE_RESOURCE) + .isNotNull(); + return new ObjectMapper(new YAMLFactory()).readValue(input, Map.class); + } +} diff --git a/plan/src/test/java/org/apache/flink/agents/plan/condition/TriggerConditionConformanceTest.java b/plan/src/test/java/org/apache/flink/agents/plan/condition/TriggerConditionConformanceTest.java new file mode 100644 index 000000000..385e64aa9 --- /dev/null +++ b/plan/src/test/java/org/apache/flink/agents/plan/condition/TriggerConditionConformanceTest.java @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan.condition; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.flink.agents.plan.condition.TriggerCondition.EventTypeCondition; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.catchThrowableOfType; + +/** Plan-layer consumer of the shared trigger-condition behavior matrix. */ +class TriggerConditionConformanceTest { + + private static final String FIXTURE_PATH = + "e2e-test/cross-language-trigger-condition-fixtures/trigger_condition_conformance.json"; + private static final JsonNode FIXTURE = loadFixture(); + + static Stream fixtureCases() { + List cases = new ArrayList<>(); + for (JsonNode testCase : FIXTURE.withArray("cases")) { + cases.add( + Arguments.of( + testCase.get("name").asText(), + testCase.get("entry").asText(), + testCase.get("classification").asText(), + testCase.get("plan_validation").asText(), + testCase.has("error_category") + ? testCase.get("error_category").asText() + : null)); + } + return cases.stream(); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("fixtureCases") + void validatesSharedCases( + String name, + String source, + String expectedClassification, + String expectedValidation, + String expectedErrorCategory) { + TriggerCondition condition = TriggerCondition.classify(source); + if ("event_type".equals(expectedClassification)) { + assertThat(condition).isInstanceOf(EventTypeCondition.class); + assertThat(expectedValidation).isEqualTo("pass"); + return; + } + + assertThat(condition).isInstanceOf(ExpressionCondition.class); + ExpressionCondition expression = (ExpressionCondition) condition; + if ("pass".equals(expectedValidation)) { + assertThatCode(() -> ConditionExpressionValidator.validate(expression)) + .doesNotThrowAnyException(); + return; + } + + ConditionExpressionValidator.ValidationException error = + catchThrowableOfType( + () -> ConditionExpressionValidator.validate(expression), + ConditionExpressionValidator.ValidationException.class); + assertThat(error).isNotNull(); + assertThat(error.category().name().toLowerCase()).isEqualTo(expectedErrorCategory); + } + + private static JsonNode loadFixture() { + Path directory = Paths.get("").toAbsolutePath(); + while (directory != null) { + Path candidate = directory.resolve(FIXTURE_PATH); + if (Files.isRegularFile(candidate)) { + try { + return new ObjectMapper().readTree(candidate.toFile()); + } catch (IOException e) { + throw new IllegalStateException( + "Failed to read trigger-condition fixture " + candidate, e); + } + } + directory = directory.getParent(); + } + throw new IllegalStateException("Could not find trigger-condition fixture " + FIXTURE_PATH); + } +} diff --git a/plan/src/test/java/org/apache/flink/agents/plan/condition/TriggerConditionTest.java b/plan/src/test/java/org/apache/flink/agents/plan/condition/TriggerConditionTest.java new file mode 100644 index 000000000..fd1114367 --- /dev/null +++ b/plan/src/test/java/org/apache/flink/agents/plan/condition/TriggerConditionTest.java @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan.condition; + +import org.apache.flink.agents.plan.condition.TriggerCondition.EventTypeCondition; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class TriggerConditionTest { + + @Test + void classifiesBareEventTypes() { + assertEventType(" a.b.c ", "a.b.c"); + assertEventType("order-created.v1", "order-created.v1"); + } + + @Test + void classifiesQuotedEventTypes() { + TriggerCondition singleQuoted = assertEventType("'order-created.v1'", "order-created.v1"); + TriggerCondition doubleQuoted = assertEventType("\"order-created.v1\"", "order-created.v1"); + + assertThat(singleQuoted).isEqualTo(doubleQuoted); + } + + @Test + void classifiesReservedExpressionCandidates() { + assertConditionExpression("true", "true"); + assertConditionExpression("false", "false"); + assertConditionExpression("null", "null"); + assertConditionExpression("EventType.InputEvent", "EventType.InputEvent"); + + assertEventType("in", "in"); + assertEventType("has", "has"); + assertEventType("size", "size"); + } + + @Test + void classifiesNonEventTypesAsExpressions() { + assertConditionExpression(" a . b . c ", "a . b . c"); + assertConditionExpression("(a.b.c)", "(a.b.c)"); + assertConditionExpression("'has whitespace'", "'has whitespace'"); + assertConditionExpression("'has\\backslash'", "'has\\backslash'"); + assertConditionExpression("'has\"quote'", "'has\"quote'"); + assertConditionExpression("'has\u0001control'", "'has\u0001control'"); + } + + @Test + void rejectsNullOrBlankEntries() { + assertThatThrownBy(() -> TriggerCondition.classify(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("non-null and non-blank"); + assertThatThrownBy(() -> TriggerCondition.classify(" \t\n")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("non-null and non-blank"); + } + + @Test + void trimsConditionExpression() { + assertConditionExpression(" score > 1 ", "score > 1"); + } + + private static TriggerCondition assertEventType(String source, String expectedEventType) { + TriggerCondition condition = TriggerCondition.classify(source); + assertThat(condition).isInstanceOf(EventTypeCondition.class); + assertThat(((EventTypeCondition) condition).eventType()).isEqualTo(expectedEventType); + return condition; + } + + private static TriggerCondition assertConditionExpression(String source, String expectedText) { + TriggerCondition condition = TriggerCondition.classify(source); + assertThat(condition).isInstanceOf(ExpressionCondition.class); + assertThat(((ExpressionCondition) condition).text()).isEqualTo(expectedText); + return condition; + } +} diff --git a/plan/src/test/java/org/apache/flink/agents/plan/condition/YamlTriggerConditionTest.java b/plan/src/test/java/org/apache/flink/agents/plan/condition/YamlTriggerConditionTest.java new file mode 100644 index 000000000..8cb7d9b36 --- /dev/null +++ b/plan/src/test/java/org/apache/flink/agents/plan/condition/YamlTriggerConditionTest.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.plan.condition; + +import org.apache.flink.agents.api.agents.Agent; +import org.apache.flink.agents.api.yaml.YamlLoader; +import org.apache.flink.agents.plan.condition.TriggerCondition.EventTypeCondition; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; + +/** Verifies the YAML loader-to-Plan classification boundary for trigger conditions. */ +class YamlTriggerConditionTest { + + @Test + void classifiesHyphenatedEventType(@TempDir Path tempDir) throws Exception { + Path file = tempDir.resolve("hyphenated-event-type.yaml"); + Files.writeString( + file, + "agents:\n" + + " - name: agent\n" + + " actions:\n" + + " - name: route_order\n" + + " function: pkg.actions:route_order\n" + + " trigger_conditions:\n" + + " - order-created\n"); + + Agent agent = YamlLoader.buildAgents(file).getAgents().get("agent"); + String source = agent.getActions().get("route_order").f0[0]; + TriggerCondition condition = TriggerCondition.classify(source); + + assertThat(source).isEqualTo("order-created"); + assertThat(condition).isInstanceOf(EventTypeCondition.class); + assertThat(((EventTypeCondition) condition).eventType()).isEqualTo("order-created"); + } +} diff --git a/plan/src/test/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializerTest.java b/plan/src/test/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializerTest.java index 8f32d512b..b16a164d4 100644 --- a/plan/src/test/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializerTest.java +++ b/plan/src/test/java/org/apache/flink/agents/plan/serializer/ActionJsonDeserializerTest.java @@ -26,12 +26,14 @@ import org.apache.flink.agents.plan.JavaFunction; import org.apache.flink.agents.plan.PythonFunction; import org.apache.flink.agents.plan.actions.Action; +import org.apache.flink.agents.plan.condition.TriggerCondition; import org.junit.jupiter.api.Test; import java.io.IOException; import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNull; @@ -58,8 +60,10 @@ public void testDeserializeJavaFunction() throws Exception { assertEquals(2, javaFunction.getParameterTypes().length); assertEquals(Event.class, javaFunction.getParameterTypes()[0]); assertEquals(RunnerContext.class, javaFunction.getParameterTypes()[1]); - assertEquals(1, action.getListenEventTypes().size()); - assertEquals(InputEvent.EVENT_TYPE, action.getListenEventTypes().get(0)); + assertEquals(1, action.getTriggerConditions().size()); + assertEquals(InputEvent.EVENT_TYPE, action.getTriggerConditions().get(0)); + assertThat(action.getClassifiedTriggerConditions()) + .containsExactly(TriggerCondition.classify(InputEvent.EVENT_TYPE)); } @Test @@ -77,8 +81,29 @@ public void testDeserializePythonFunction() throws IOException { PythonFunction pythonFunction = (PythonFunction) action.getExec(); assertEquals("test_module", pythonFunction.getModule()); assertEquals("test_function", pythonFunction.getQualName()); - assertEquals(1, action.getListenEventTypes().size()); - assertEquals(InputEvent.EVENT_TYPE, action.getListenEventTypes().get(0)); + assertEquals(1, action.getTriggerConditions().size()); + assertEquals(InputEvent.EVENT_TYPE, action.getTriggerConditions().get(0)); + assertThat(action.getClassifiedTriggerConditions()) + .containsExactly(TriggerCondition.classify(InputEvent.EVENT_TYPE)); + } + + @Test + public void pythonJsonUsesPlanValidation() throws IOException { + String validJson = + "{\"name\":\"pythonCondition\"," + + "\"exec\":{\"func_type\":\"PythonFunction\"," + + "\"module\":\"test_module\",\"qualname\":\"test_function\"}," + + "\"trigger_conditions\":[\" type == '_input_event' \"]," + + "\"config\":null}"; + Action action = new ObjectMapper().readValue(validJson, Action.class); + assertThat(action.getClassifiedTriggerConditions()) + .containsExactly(TriggerCondition.classify("type == '_input_event'")); + + String invalidJson = validJson.replace(" type == '_input_event' ", "type =="); + assertThatThrownBy(() -> new ObjectMapper().readValue(invalidJson, Action.class)) + .hasMessageContaining("action 'pythonCondition'") + .hasMessageContaining("trigger condition #1") + .hasMessageContaining("type =="); } @Test @@ -96,19 +121,55 @@ public void testDeserializeMissingFields() throws IOException { // Read JSON with missing fields from resource file String json = Utils.readJsonFromResource("actions/action_missing_fields.json"); - // Attempt to deserialize the JSON + // Attempt to deserialize the JSON: a JavaFunction exec missing 'qualname' fails fast + // in deserializeJavaFunction before any trigger-condition parsing. ObjectMapper mapper = new ObjectMapper(); - assertThrows(Exception.class, () -> mapper.readValue(json, Action.class)); + assertThrows(NullPointerException.class, () -> mapper.readValue(json, Action.class)); } @Test - public void testDeserializeInvalidEventType() throws IOException { - // Read JSON with an invalid event type from resource file - String json = Utils.readJsonFromResource("actions/action_invalid_event_type.json"); + public void deserializesDottedCondition() throws IOException { + String json = Utils.readJsonFromResource("actions/action_unquoted_dotted_event_type.json"); + + ObjectMapper mapper = new ObjectMapper(); + Action action = mapper.readValue(json, Action.class); + assertThat(action.getTriggerConditions()) + .containsExactly("org.apache.flink.agents.api.NonExistentEvent"); + } + + @Test + public void deserializesQuotedEventType() throws Exception { + String json = + "{\"name\": \"testAction\"," + + " \"exec\": {\"func_type\": \"JavaFunction\"," + + " \"qualname\": \"org.apache.flink.agents.plan.TestAction\"," + + " \"method_name\": \"legal\"," + + " \"parameter_types\": [\"org.apache.flink.agents.api.Event\"," + + " \"org.apache.flink.agents.api.context.RunnerContext\"]}," + + " \"trigger_conditions\": [\"'com.example.order-created'\"]," + + " \"config\": null}"; - // Attempt to deserialize the JSON ObjectMapper mapper = new ObjectMapper(); - assertThrows(RuntimeException.class, () -> mapper.readValue(json, Action.class)); + Action action = mapper.readValue(json, Action.class); + + assertEquals(1, action.getTriggerConditions().size()); + assertEquals("'com.example.order-created'", action.getTriggerConditions().get(0)); + } + + @Test + public void rejectsLegacyListenTypesField() { + String json = + "{\"name\": \"legacyAction\"," + + " \"exec\": {\"func_type\": \"JavaFunction\"," + + " \"qualname\": \"org.apache.flink.agents.plan.TestAction\"," + + " \"method_name\": \"legal\"," + + " \"parameter_types\": [\"org.apache.flink.agents.api.Event\"," + + " \"org.apache.flink.agents.api.context.RunnerContext\"]}," + + " \"listen_event_types\": [\"_input_event\"]," + + " \"config\": null}"; + + assertThatThrownBy(() -> new ObjectMapper().readValue(json, Action.class)) + .hasMessageContaining("trigger_conditions"); } @Test diff --git a/plan/src/test/java/org/apache/flink/agents/plan/serializer/ActionJsonSerializerTest.java b/plan/src/test/java/org/apache/flink/agents/plan/serializer/ActionJsonSerializerTest.java index b6c2170b5..70667c02e 100644 --- a/plan/src/test/java/org/apache/flink/agents/plan/serializer/ActionJsonSerializerTest.java +++ b/plan/src/test/java/org/apache/flink/agents/plan/serializer/ActionJsonSerializerTest.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.flink.agents.api.Event; import org.apache.flink.agents.api.InputEvent; -import org.apache.flink.agents.api.OutputEvent; import org.apache.flink.agents.api.context.RunnerContext; import org.apache.flink.agents.plan.JavaFunction; import org.apache.flink.agents.plan.PythonFunction; @@ -30,7 +29,6 @@ import org.junit.jupiter.api.Test; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -69,7 +67,7 @@ public void testSerializeJavaFunction() throws Exception { json.contains("\"method_name\":\"legal\""), "JSON should contain the method name"); assertTrue( json.contains("\"trigger_conditions\":["), - "JSON should contain the trigger conditions"); + "JSON should contain trigger conditions"); assertTrue( json.contains("\"" + InputEvent.EVENT_TYPE + "\""), "JSON should contain the event type string"); @@ -104,14 +102,14 @@ public void testSerializePythonFunction() throws Exception { "JSON should contain the qualified name"); assertTrue( json.contains("\"trigger_conditions\":["), - "JSON should contain the trigger conditions"); + "JSON should contain trigger conditions"); assertTrue( json.contains("\"" + InputEvent.EVENT_TYPE + "\""), "JSON should contain the event type string"); } @Test - public void testSerializeMultipleEventTypes() throws Exception { + public void testSerializeMultipleTriggerConditions() throws Exception { // Create a JavaFunction JavaFunction function = new JavaFunction( @@ -119,10 +117,11 @@ public void testSerializeMultipleEventTypes() throws Exception { "legal", new Class[] {Event.class, RunnerContext.class}); - // Create an Action with multiple trigger conditions + // Preserve raw type and condition entries, including whitespace and duplicates. List triggerConditions = new ArrayList<>(); triggerConditions.add(InputEvent.EVENT_TYPE); - triggerConditions.add(OutputEvent.EVENT_TYPE); + triggerConditions.add(" score > 1 "); + triggerConditions.add(InputEvent.EVENT_TYPE); Action action = new Action("multiEventAction", function, triggerConditions); // Serialize the action to JSON @@ -134,40 +133,21 @@ public void testSerializeMultipleEventTypes() throws Exception { "JSON should contain the action name"); assertTrue( json.contains("\"trigger_conditions\":["), - "JSON should contain the trigger conditions"); + "JSON should contain trigger conditions"); assertTrue( json.contains("\"" + InputEvent.EVENT_TYPE + "\""), "JSON should contain the InputEvent type string"); assertTrue( - json.contains("\"" + OutputEvent.EVENT_TYPE + "\""), - "JSON should contain the OutputEvent type string"); + json.contains("\" score > 1 \""), "JSON should contain the raw condition string"); assertTrue( json.contains("\"org.apache.flink.agents.api.context.RunnerContext\""), "JSON should contain the runner context class name"); - } - - @Test - public void testSerializeEmptyEventTypes() throws Exception { - // Create a JavaFunction - JavaFunction function = - new JavaFunction( - "org.apache.flink.agents.plan.TestAction", - "legal", - new Class[] {Event.class, RunnerContext.class}); - - // Create an Action with an empty event types list - Action action = new Action("emptyEventsAction", function, Collections.emptyList()); - - // Serialize the action to JSON - String json = new ObjectMapper().writeValueAsString(action); - // Verify the JSON contains the expected fields - assertTrue( - json.contains("\"name\":\"emptyEventsAction\""), - "JSON should contain the action name"); - assertTrue( - json.contains("\"trigger_conditions\":[]"), - "JSON should contain an empty trigger conditions array"); + // Pin the exact serialized trigger_conditions structure and order. + Action restored = new ObjectMapper().readValue(json, Action.class); + assertEquals( + List.of(InputEvent.EVENT_TYPE, " score > 1 ", InputEvent.EVENT_TYPE), + restored.getTriggerConditions()); } @Test @@ -199,8 +179,8 @@ public void testSerializeDeserializeRoundTrip() throws Exception { assertEquals(2, deserializedFunction.getParameterTypes().length); assertEquals(Event.class, deserializedFunction.getParameterTypes()[0]); assertEquals(RunnerContext.class, deserializedFunction.getParameterTypes()[1]); - assertEquals(1, deserializedAction.getListenEventTypes().size()); - assertEquals(InputEvent.EVENT_TYPE, deserializedAction.getListenEventTypes().get(0)); + assertEquals(1, deserializedAction.getTriggerConditions().size()); + assertEquals(InputEvent.EVENT_TYPE, deserializedAction.getTriggerConditions().get(0)); } @Test @@ -234,4 +214,49 @@ public void testSerializeDeserializeConfig() throws Exception { Assertions.assertEquals(arg1, deserializeConfig.get("arg1")); Assertions.assertEquals(arg2, deserializeConfig.get("arg2")); } + + @Test + public void testSerializeDeserializePythonConfig() throws Exception { + JavaFunction function = + new JavaFunction( + "org.apache.flink.agents.plan.TestAction", + "legal", + new Class[] {Event.class, RunnerContext.class}); + + Map config = new HashMap<>(); + config.put(ActionJsonSerializer.CONFIG_TYPE, "python"); + config.put("list", List.of(1, 2, 3)); + config.put("nested", Map.of("k1", 1, "k2", 2)); + + Action action = + new Action("pyConfigAction", function, List.of(InputEvent.EVENT_TYPE), config); + + ObjectMapper mapper = new ObjectMapper(); + String json = mapper.writeValueAsString(action); + Action actual = mapper.readValue(json, Action.class); + + Map restored = actual.getConfig(); + Assertions.assertNotNull(restored); + assertEquals("python", restored.get(ActionJsonSerializer.CONFIG_TYPE)); + assertEquals(List.of(1, 2, 3), restored.get("list")); + assertEquals(Map.of("k1", 1, "k2", 2), restored.get("nested")); + } + + @Test + public void testSerializeDeserializeNullConfig() throws Exception { + JavaFunction function = + new JavaFunction( + "org.apache.flink.agents.plan.TestAction", + "legal", + new Class[] {Event.class, RunnerContext.class}); + + Action action = new Action("nullConfigAction", function, List.of(InputEvent.EVENT_TYPE)); + + ObjectMapper mapper = new ObjectMapper(); + String json = mapper.writeValueAsString(action); + assertTrue(json.contains("\"config\":null"), "JSON should contain a null config field"); + + Action actual = mapper.readValue(json, Action.class); + Assertions.assertNull(actual.getConfig()); + } } diff --git a/plan/src/test/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonDeserializerTest.java b/plan/src/test/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonDeserializerTest.java index 21eb8bfa4..1dbffa558 100644 --- a/plan/src/test/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonDeserializerTest.java +++ b/plan/src/test/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonDeserializerTest.java @@ -18,7 +18,10 @@ package org.apache.flink.agents.plan.serializer; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; import org.apache.flink.agents.api.Event; import org.apache.flink.agents.api.InputEvent; import org.apache.flink.agents.api.context.RunnerContext; @@ -33,14 +36,31 @@ import org.apache.flink.agents.plan.tools.FunctionTool; import org.junit.jupiter.api.Test; +import java.io.IOException; import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class AgentPlanJsonDeserializerTest { + @Test + public void testActionsMustBePresentObjectButMayBeEmpty() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + + for (String json : + List.of("{}", "{\"actions\":null}", "{\"actions\":[]}", "{\"actions\":\"bad\"}")) { + IOException error = + assertThrows(IOException.class, () -> mapper.readValue(json, AgentPlan.class)); + assertTrue(error.getMessage().contains("actions")); + } + + AgentPlan empty = mapper.readValue("{\"actions\":{}}", AgentPlan.class); + assertTrue(empty.getActions().isEmpty()); + } + @Test public void testDeserialize() throws Exception { // Read JSON for an Action with JavaFunction from resource file @@ -52,7 +72,7 @@ public void testDeserialize() throws Exception { assertTrue(agentPlan.getActions().containsKey("first_action")); Action firstAction = agentPlan.getActions().get("first_action"); assertInstanceOf(JavaFunction.class, firstAction.getExec()); - assertEquals(List.of(InputEvent.EVENT_TYPE), firstAction.getListenEventTypes()); + assertEquals(List.of(InputEvent.EVENT_TYPE), firstAction.getTriggerConditions()); // Check the second action assertTrue(agentPlan.getActions().containsKey("second_action")); @@ -60,15 +80,7 @@ public void testDeserialize() throws Exception { assertInstanceOf(JavaFunction.class, secondAction.getExec()); assertEquals( List.of(InputEvent.EVENT_TYPE, MyEvent.EVENT_TYPE), - secondAction.getListenEventTypes()); - - // Check event trigger actions - assertEquals(2, agentPlan.getActionsByEvent().size()); - assertTrue(agentPlan.getActionsByEvent().containsKey(InputEvent.EVENT_TYPE)); - assertEquals( - List.of(firstAction, secondAction), - agentPlan.getActionsByEvent().get(InputEvent.EVENT_TYPE)); - assertEquals(List.of(secondAction), agentPlan.getActionsByEvent().get(MyEvent.EVENT_TYPE)); + secondAction.getTriggerConditions()); // Check the flink agent config Map configData = agentPlan.getConfigData(); @@ -79,6 +91,25 @@ public void testDeserialize() throws Exception { assertEquals("v1", configData.get("key4")); } + @Test + public void testInvalidConditionIsJsonMappingError() throws Exception { + ObjectMapper mapper = new ObjectMapper(); + JsonNode plan = mapper.readTree(Utils.readJsonFromResource("agent_plans/agent_plan.json")); + ((ArrayNode) plan.get("actions").get("first_action").get("trigger_conditions")) + .removeAll() + .add("type =="); + + JsonMappingException error = + assertThrows( + JsonMappingException.class, + () -> mapper.readValue(mapper.writeValueAsString(plan), AgentPlan.class)); + + assertInstanceOf(IllegalArgumentException.class, error.getCause()); + assertTrue(error.getOriginalMessage().contains("trigger condition #1")); + assertTrue(error.getOriginalMessage().contains("action 'first_action'")); + assertTrue(error.getOriginalMessage().contains("\"type ==\"")); + } + @Test public void testDeserializeToolWithInjectedArgs() throws Exception { ObjectMapper mapper = new ObjectMapper(); @@ -98,8 +129,6 @@ public void testDeserializeToolWithInjectedArgs() throws Exception { Map.of( "actions", Map.of(), - "actions_by_event", - Map.of(), "resource_providers", Map.of( "tool", diff --git a/plan/src/test/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonSerializerTest.java b/plan/src/test/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonSerializerTest.java index ef18cdadc..ed316e483 100644 --- a/plan/src/test/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonSerializerTest.java +++ b/plan/src/test/java/org/apache/flink/agents/plan/serializer/AgentPlanJsonSerializerTest.java @@ -119,40 +119,11 @@ public void testSerializeAgentPlanWithActions() throws Exception { assertThat(json).contains("\"method_name\":\"legal\""); assertThat(json).contains("\"trigger_conditions\":["); assertThat(json).contains("\"" + InputEvent.EVENT_TYPE + "\""); - assertThat(json).contains("\"actions_by_event\":{}"); + assertThat(json).doesNotContain("actions_by_event"); } @Test - public void testSerializeAgentPlanWithActionsByEvent() throws Exception { - // Create a JavaFunction - JavaFunction function = - new JavaFunction( - "org.apache.flink.agents.plan.TestAction", - "legal", - new Class[] {Event.class, RunnerContext.class}); - - // Create an Action - Action action = new Action("testAction", function, List.of(InputEvent.EVENT_TYPE)); - - // Create a map of event trigger actions - Map> actionsByEvent = new HashMap<>(); - actionsByEvent.put(InputEvent.EVENT_TYPE, List.of(action)); - - // Create a AgentPlan with event trigger actions but no regular actions - AgentPlan agentPlan = new AgentPlan(new HashMap<>(), actionsByEvent); - - // Serialize the agent plan to JSON - String json = new ObjectMapper().writeValueAsString(agentPlan); - - // Verify the JSON contains the expected fields - assertThat(json).contains("\"actions\":{}"); - assertThat(json).contains("\"actions_by_event\":{"); - assertThat(json).contains("\"" + InputEvent.EVENT_TYPE + "\":["); - assertThat(json).contains("\"testAction\""); - } - - @Test - public void testSerializeAgentPlanWithBothActionsAndActionsByEvent() throws Exception { + public void testSerializeAgentPlanWithMultipleActions() throws Exception { // Create JavaFunctions JavaFunction function1 = new JavaFunction( @@ -174,13 +145,8 @@ public void testSerializeAgentPlanWithBothActionsAndActionsByEvent() throws Exce actions.put(action1.getName(), action1); actions.put(action2.getName(), action2); - // Create a map of event trigger actions - Map> actionsByEvent = new HashMap<>(); - actionsByEvent.put(InputEvent.EVENT_TYPE, List.of(action1)); - actionsByEvent.put(OutputEvent.EVENT_TYPE, List.of(action2)); - - // Create a AgentPlan with both actions and event trigger actions - AgentPlan agentPlan = new AgentPlan(actions, actionsByEvent); + // Create an AgentPlan; event indexes are derived and not serialized. + AgentPlan agentPlan = new AgentPlan(actions); // Serialize the agent plan to JSON String json = new ObjectMapper().writeValueAsString(agentPlan); @@ -189,9 +155,7 @@ public void testSerializeAgentPlanWithBothActionsAndActionsByEvent() throws Exce assertThat(json).contains("\"actions\":{"); assertThat(json).contains("\"action1\":{"); assertThat(json).contains("\"action2\":{"); - assertThat(json).contains("\"actions_by_event\":{"); - assertThat(json).contains("\"" + InputEvent.EVENT_TYPE + "\":["); - assertThat(json).contains("\"" + OutputEvent.EVENT_TYPE + "\":["); + assertThat(json).doesNotContain("actions_by_event"); assertThat(json).contains("\"action1\""); assertThat(json).contains("\"action2\""); } @@ -206,7 +170,7 @@ public void testSerializeEmptyAgentPlan() throws Exception { // Verify the JSON contains the expected fields assertThat(json).contains("\"actions\":{}"); - assertThat(json).contains("\"actions_by_event\":{}"); + assertThat(json).doesNotContain("actions_by_event"); } @Test @@ -226,7 +190,7 @@ public void testSerializeAgentPlanCreatedFromAgent() throws Exception { // Verify the JSON contains the expected fields assertThat(json).contains("\"actions\":{"); - assertThat(json).contains("\"actions_by_event\":{"); + assertThat(json).doesNotContain("actions_by_event"); assertThat(json).contains("\"config\":{"); // Verify that the actions from @Action annotated methods are present diff --git a/plan/src/test/resources/actions/action_invalid_event_type.json b/plan/src/test/resources/actions/action_invalid_event_type.json deleted file mode 100644 index b84d5c508..000000000 --- a/plan/src/test/resources/actions/action_invalid_event_type.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "name": "testAction", - "exec": { - "func_type": "JavaFunction", - "qualname": "org.apache.flink.agents.plan.TestAction", - "method_name": "legal", - "parameter_types": ["org.apache.flink.agents.api.Event"] - }, - "trigger_conditions": ["org.apache.flink.agents.api.NonExistentEvent"] -} \ No newline at end of file diff --git a/plan/src/test/resources/actions/action_invalid_function_type.json b/plan/src/test/resources/actions/action_invalid_function_type.json index 846c76267..4ff504056 100644 --- a/plan/src/test/resources/actions/action_invalid_function_type.json +++ b/plan/src/test/resources/actions/action_invalid_function_type.json @@ -4,7 +4,11 @@ "func_type": "InvalidFunction", "qualname": "org.apache.flink.agents.plan.TestAction", "method_name": "legal", - "parameter_types": ["org.apache.flink.agents.api.Event"] + "parameter_types": [ + "org.apache.flink.agents.api.Event" + ] }, - "trigger_conditions": ["org.apache.flink.agents.api.InputEvent"] -} \ No newline at end of file + "trigger_conditions": [ + "_input_event" + ] +} diff --git a/plan/src/test/resources/actions/action_java_function.json b/plan/src/test/resources/actions/action_java_function.json index 6b73f324a..58e511d05 100644 --- a/plan/src/test/resources/actions/action_java_function.json +++ b/plan/src/test/resources/actions/action_java_function.json @@ -4,7 +4,12 @@ "func_type": "JavaFunction", "qualname": "org.apache.flink.agents.plan.TestAction", "method_name": "legal", - "parameter_types": ["org.apache.flink.agents.api.Event", "org.apache.flink.agents.api.context.RunnerContext"] + "parameter_types": [ + "org.apache.flink.agents.api.Event", + "org.apache.flink.agents.api.context.RunnerContext" + ] }, - "trigger_conditions": ["_input_event"] + "trigger_conditions": [ + "_input_event" + ] } diff --git a/plan/src/test/resources/actions/action_missing_fields.json b/plan/src/test/resources/actions/action_missing_fields.json index 7e9bb0bc0..a6e55b9d7 100644 --- a/plan/src/test/resources/actions/action_missing_fields.json +++ b/plan/src/test/resources/actions/action_missing_fields.json @@ -3,5 +3,7 @@ "exec": { "func_type": "JavaFunction" }, - "trigger_conditions": ["org.apache.flink.agents.api.InputEvent"] -} \ No newline at end of file + "trigger_conditions": [ + "_input_event" + ] +} diff --git a/plan/src/test/resources/actions/action_python_function.json b/plan/src/test/resources/actions/action_python_function.json index 5711b873d..2037c68ea 100644 --- a/plan/src/test/resources/actions/action_python_function.json +++ b/plan/src/test/resources/actions/action_python_function.json @@ -5,5 +5,7 @@ "module": "test_module", "qualname": "test_function" }, - "trigger_conditions": ["_input_event"] + "trigger_conditions": [ + "_input_event" + ] } diff --git a/plan/src/test/resources/actions/action_unquoted_dotted_event_type.json b/plan/src/test/resources/actions/action_unquoted_dotted_event_type.json new file mode 100644 index 000000000..1ba180106 --- /dev/null +++ b/plan/src/test/resources/actions/action_unquoted_dotted_event_type.json @@ -0,0 +1,15 @@ +{ + "name": "testAction", + "exec": { + "func_type": "JavaFunction", + "qualname": "org.apache.flink.agents.plan.TestAction", + "method_name": "legal", + "parameter_types": [ + "org.apache.flink.agents.api.Event", + "org.apache.flink.agents.api.context.RunnerContext" + ] + }, + "trigger_conditions": [ + "org.apache.flink.agents.api.NonExistentEvent" + ] +} diff --git a/plan/src/test/resources/agent_plans/agent_plan.json b/plan/src/test/resources/agent_plans/agent_plan.json index 52a1b5ad8..b236108eb 100644 --- a/plan/src/test/resources/agent_plans/agent_plan.json +++ b/plan/src/test/resources/agent_plans/agent_plan.json @@ -5,7 +5,10 @@ "exec": { "qualname": "org.apache.flink.agents.plan.TestAction", "method_name": "legal", - "parameter_types": ["org.apache.flink.agents.api.Event", "org.apache.flink.agents.api.context.RunnerContext"], + "parameter_types": [ + "org.apache.flink.agents.api.Event", + "org.apache.flink.agents.api.context.RunnerContext" + ], "func_type": "JavaFunction" }, "trigger_conditions": [ @@ -17,7 +20,10 @@ "exec": { "qualname": "org.apache.flink.agents.plan.serializer.AgentPlanJsonDeserializerTest$MyAction", "method_name": "doNothing", - "parameter_types": ["org.apache.flink.agents.api.Event", "org.apache.flink.agents.api.context.RunnerContext"], + "parameter_types": [ + "org.apache.flink.agents.api.Event", + "org.apache.flink.agents.api.context.RunnerContext" + ], "func_type": "JavaFunction" }, "trigger_conditions": [ @@ -26,15 +32,6 @@ ] } }, - "actions_by_event": { - "_input_event": [ - "first_action", - "second_action" - ], - "MyEvent": [ - "second_action" - ] - }, "config": { "conf_data": { "key1": 1, diff --git a/plan/src/test/resources/agent_plans/agent_plan_with_python_resource_providers.json b/plan/src/test/resources/agent_plans/agent_plan_with_python_resource_providers.json index abd08e2d3..6c655a5d2 100644 --- a/plan/src/test/resources/agent_plans/agent_plan_with_python_resource_providers.json +++ b/plan/src/test/resources/agent_plans/agent_plan_with_python_resource_providers.json @@ -47,24 +47,6 @@ ] } }, - "actions_by_event": { - "_input_event": [ - "first_action", - "second_action" - ], - "_my_event": [ - "second_action" - ], - "_chat_request_event": [ - "chat_model_action" - ], - "_tool_response_event": [ - "chat_model_action" - ], - "_tool_request_event": [ - "tool_call_action" - ] - }, "resource_providers": { "chat_model": { "chat_model": { diff --git a/plan/src/test/resources/disallowed_macros.yaml b/plan/src/test/resources/disallowed_macros.yaml new file mode 100644 index 000000000..920e86759 --- /dev/null +++ b/plan/src/test/resources/disallowed_macros.yaml @@ -0,0 +1,22 @@ +# Macro whitelist fixture, owned by Plan; consumed by ConditionExpressionValidatorTest. +# Only `has()` is allowed in trigger conditions; all others are rejected. + +reject: + # Method-style calls (most common CEL usage) + - "attributes.tags.exists(x, x == 'a')" + - "attributes.scores.all(s, s >= 60)" + - "attributes.items.filter(i, i.active == true)" + - "attributes.items.map(i, i.name)" + - "attributes.tags.exists_one(t, t == 'urgent')" + # Nested in logical operators + - "has(attributes.x) && attributes.list.all(t, t > 0)" + - "type == '_input_event' || attributes.items.exists(i, i.price > 100)" + # Nested macros inside macro arguments + - "attributes.groups.exists(g, g.members.all(m, m.active == true))" + +accept: + # One representative for each accepted shape. + - "has(attributes.user_id) && attributes.score > 10" + - "type == '_input_event'" + - "attributes.label == 'has all filter map exists'" + - "attributes.existing == true" diff --git a/pom.xml b/pom.xml index 108841acd..4e4b2ecd2 100644 --- a/pom.xml +++ b/pom.xml @@ -52,6 +52,7 @@ under the License. 3.27.7 5.14.2 1.15.4 + 0.12.0 true diff --git a/python/flink_agents/api/agents/agent.py b/python/flink_agents/api/agents/agent.py index 4e7dba086..5928abae2 100644 --- a/python/flink_agents/api/agents/agent.py +++ b/python/flink_agents/api/agents/agent.py @@ -85,9 +85,7 @@ def my_chat_model() -> ResourceDescriptor: ) """ - _actions: Dict[ - str, Tuple[List[str], Function, Dict[str, Any] | None] - ] + _actions: Dict[str, Tuple[List[str], Function, Dict[str, Any] | None]] _resources: Dict[ResourceType, Dict[str, Any]] def __init__(self) -> None: @@ -123,8 +121,8 @@ def add_action( name : str The name of the action, should be unique in the same Agent. trigger_conditions : list[str] - Trigger condition strings — each is either an event-type name - or a future condition-expression form (see ``@action``). + Raw event-type names or Boolean condition expressions combined with + OR semantics. Expression validation occurs when the plan is applied. func : Callable | Function Either a raw Python callable (it will be wrapped as a ``PythonFunction``) or a pre-built flink-agents ``Function`` diff --git a/python/flink_agents/api/core_options.py b/python/flink_agents/api/core_options.py index d196777c4..d560d8cf9 100644 --- a/python/flink_agents/api/core_options.py +++ b/python/flink_agents/api/core_options.py @@ -58,6 +58,16 @@ class LoggerType(Enum): FILE = "file" +class ConditionEvaluationFailureStrategy(Enum): + """Behavior when evaluating an action trigger condition fails. + + Mirrors Java ``ConditionEvaluationFailureStrategy``. + """ + + WARN_AND_SKIP = "WARN_AND_SKIP" + FAIL = "FAIL" + + class EventLogLevel(Enum): """Log level for event logging. @@ -82,6 +92,12 @@ class AgentConfigOptions: default=LoggerType.SLF4J, ) + CONDITION_EVALUATION_FAILURE_STRATEGY = ConfigOption( + key="action.trigger-condition.evaluate-failure-strategy", + config_type=ConditionEvaluationFailureStrategy, + default=ConditionEvaluationFailureStrategy.WARN_AND_SKIP, + ) + BASE_LOG_DIR = ConfigOption( key="baseLogDir", config_type=str, diff --git a/python/flink_agents/api/decorators.py b/python/flink_agents/api/decorators.py index 722e5868e..14e2227fb 100644 --- a/python/flink_agents/api/decorators.py +++ b/python/flink_agents/api/decorators.py @@ -29,7 +29,9 @@ def _validate_target(target: Function, owner: str) -> None: """Reject targets with empty required identifiers, attributed to ``owner``.""" if isinstance(target, PythonFunction): if not target.module or not target.qualname: - msg = f"PythonFunction target on '{owner}' must set both module and qualname" + msg = ( + f"PythonFunction target on '{owner}' must set both module and qualname" + ) raise ValueError(msg) elif isinstance(target, JavaFunction): if not target.qualname or not target.method_name: @@ -43,14 +45,16 @@ def action( ) -> Callable: """Decorator for marking a function as an agent action. - Each argument is an event-type name string that this action responds to. - Multiple entries combine with OR semantics — the action triggers if any - one matches. + Each trigger condition is an event-type name or Boolean condition + expression. Multiple conditions combine with OR semantics. To combine an + event type and an attribute predicate with AND, place both in one + expression, such as ``type == EventType.InputEvent && score > 5``. + Expression validation occurs when the plan is applied. Parameters ---------- trigger_conditions : str - Event-type name strings that this action responds to. + Raw event-type names or Boolean condition expressions. target : Function, optional Cross-language function descriptor dispatched instead of the decorated body. The body becomes a stub — raise @@ -63,22 +67,14 @@ def action( Raises: ------ - AssertionError - If no conditions are given or any entry is not a non-empty string. TypeError - If ``target`` is provided but is not a :class:`Function` descriptor. + If an entry is not a string, or ``target`` is not a + :class:`Function` descriptor. """ - assert len(trigger_conditions) > 0, ( - "action must have at least one trigger condition (event-type name)" - ) - for entry in trigger_conditions: - assert isinstance(entry, str), ( - f"action trigger condition must be a string, got {entry!r}" - ) - assert entry != "", ( - f"action trigger condition must be non-empty, got {entry!r}" - ) + if not isinstance(entry, str): + msg = f"action trigger condition must be a string, got {entry!r}" + raise TypeError(msg) if target is not None and not isinstance(target, Function): msg = ( diff --git a/python/flink_agents/api/events/event_type.py b/python/flink_agents/api/events/event_type.py index 344d6167c..a244661a7 100644 --- a/python/flink_agents/api/events/event_type.py +++ b/python/flink_agents/api/events/event_type.py @@ -15,7 +15,11 @@ # See the License for the specific language governing permissions and # limitations under the License. ################################################################################# -"""Built-in event-type constants, sourced from each ``XxxEvent.EVENT_TYPE``.""" +"""Built-in event-type constants, sourced from each ``XxxEvent.EVENT_TYPE``. + +Usage: ``@action(EventType.InputEvent)``, or in a trigger condition: +``type == EventType.InputEvent``. +""" from __future__ import annotations diff --git a/python/flink_agents/api/tests/test_agent_add_action.py b/python/flink_agents/api/tests/test_agent_add_action.py index c11c04462..a90bbdbbe 100644 --- a/python/flink_agents/api/tests/test_agent_add_action.py +++ b/python/flink_agents/api/tests/test_agent_add_action.py @@ -142,16 +142,19 @@ def test_add_action_returns_self_for_chaining() -> None: assert result is agent -# ── Multiple trigger conditions ───────────────────────────────────────── - - -def test_add_action_supports_multiple_trigger_conditions() -> None: +@pytest.mark.parametrize( + "conditions", + [ + pytest.param( + ["evt_a", "attributes.ready == true", "type =="], + id="raw-conditions", + ), + pytest.param([], id="empty-deferred-to-plan"), + ], +) +def test_add_action_preserves_raw_conditions(conditions: list[str]) -> None: agent = Agent() - agent.add_action( - name="multi", - trigger_conditions=["evt_a", "evt_b", "evt_c"], - func=_make_java_function(), - ) + agent.add_action(name="act", trigger_conditions=conditions, func=_dummy_action) - trigger_conditions, _, _ = agent.actions["multi"] - assert trigger_conditions == ["evt_a", "evt_b", "evt_c"] + trigger_conditions, _, _ = agent.actions["act"] + assert trigger_conditions == conditions diff --git a/python/flink_agents/api/tests/test_core_options.py b/python/flink_agents/api/tests/test_core_options.py index f1e773a83..5a4e91d26 100644 --- a/python/flink_agents/api/tests/test_core_options.py +++ b/python/flink_agents/api/tests/test_core_options.py @@ -98,13 +98,26 @@ def _collect_config_options(options_class: type) -> dict[str, ConfigOption]: def test_agent_config_options_are_explicitly_declared() -> None: - from flink_agents.api.core_options import AgentConfigOptions, EventLogLevel + from flink_agents.api.core_options import ( + AgentConfigOptions, + ConditionEvaluationFailureStrategy, + EventLogLevel, + ) options = _collect_config_options(AgentConfigOptions) - assert len(options) == 23 assert options["BASE_LOG_DIR"].get_key() == "baseLogDir" assert options["KAFKA_BOOTSTRAP_SERVERS"].get_default_value() == "localhost:9092" assert options["EVENT_LOG_LEVEL"].get_default_value() is EventLogLevel.STANDARD + condition_failure = options["CONDITION_EVALUATION_FAILURE_STRATEGY"] + assert ( + condition_failure.get_key() + == "action.trigger-condition.evaluate-failure-strategy" + ) + assert condition_failure.get_type() is ConditionEvaluationFailureStrategy + assert ( + condition_failure.get_default_value() + is ConditionEvaluationFailureStrategy.WARN_AND_SKIP + ) def test_unknown_agent_config_option_raises_attribute_error() -> None: diff --git a/python/flink_agents/api/tests/test_decorators.py b/python/flink_agents/api/tests/test_decorators.py index 216df45e3..882f5ea2b 100644 --- a/python/flink_agents/api/tests/test_decorators.py +++ b/python/flink_agents/api/tests/test_decorators.py @@ -32,34 +32,58 @@ def forward_action(event: Event, ctx: RunnerContext) -> None: ctx.send_event(OutputEvent(output=input)) assert hasattr(forward_action, "_trigger_conditions") - listen_events = forward_action._trigger_conditions - assert listen_events == (InputEvent.EVENT_TYPE,) + trigger_conditions = forward_action._trigger_conditions + assert trigger_conditions == (InputEvent.EVENT_TYPE,) -def test_action_decorator_listen_multi_events() -> None: - @action(EventType.InputEvent, EventType.OutputEvent) +def test_action_decorator_preserves_mixed_conditions() -> None: + @action( + EventType.InputEvent, + EventType.OutputEvent, + "attributes.ready == true", + ) def forward_action(event: Event, ctx: RunnerContext) -> None: input = InputEvent.from_event(event).input ctx.send_event(OutputEvent(output=input)) assert hasattr(forward_action, "_trigger_conditions") - listen_events = forward_action._trigger_conditions - assert listen_events == (InputEvent.EVENT_TYPE, OutputEvent.EVENT_TYPE) + trigger_conditions = forward_action._trigger_conditions + assert trigger_conditions == ( + InputEvent.EVENT_TYPE, + OutputEvent.EVENT_TYPE, + "attributes.ready == true", + ) + + +@pytest.mark.parametrize( + "conditions", + [ + pytest.param((), id="empty"), + pytest.param((" ",), id="blank"), + ], +) +def test_action_decorator_defers_structural_validation( + conditions: tuple[str, ...], +) -> None: + @action(*conditions) + def forward_action(event: Event, ctx: RunnerContext) -> None: + input = InputEvent.from_event(event).input + ctx.send_event(OutputEvent(output=input)) + assert forward_action._trigger_conditions == conditions -def test_action_decorator_listen_no_event() -> None: - with pytest.raises(AssertionError): - - @action() - def forward_action(event: Event, ctx: RunnerContext) -> None: - input = InputEvent.from_event(event).input - ctx.send_event(OutputEvent(output=input)) +@pytest.mark.parametrize( + "condition", + [ + pytest.param(InputEvent, id="event-class"), + pytest.param(42, id="integer"), + ], +) +def test_action_decorator_rejects_non_string_conditions(condition: object) -> None: + with pytest.raises(TypeError, match="must be a string"): -def test_action_decorator_listen_non_string_type() -> None: - with pytest.raises(AssertionError): - - @action(InputEvent) # type: ignore[arg-type] + @action(condition) # type: ignore[arg-type] def forward_action(event: Event, ctx: RunnerContext) -> None: input = InputEvent.from_event(event).input ctx.send_event(OutputEvent(output=input)) @@ -76,25 +100,6 @@ def my_handler(event: Event, ctx: RunnerContext) -> None: assert my_handler._trigger_conditions == ("MyCustomEvent",) -def test_action_decorator_multiple_strings() -> None: - """Test that @action accepts multiple string identifiers.""" - - @action("_input_event", "AnotherEvent") - def mixed_handler(event: Event, ctx: RunnerContext) -> None: - pass - - assert mixed_handler._trigger_conditions == ("_input_event", "AnotherEvent") - - -def test_action_decorator_rejects_invalid_types() -> None: - """Test that @action rejects non-string arguments.""" - with pytest.raises(AssertionError): - - @action(42) # type: ignore[arg-type] - def bad_handler(event: Event, ctx: RunnerContext) -> None: - pass - - def _java_target() -> JavaFunction: return JavaFunction.for_action("com.example.Handlers", "handle") diff --git a/python/flink_agents/api/yaml/specs.py b/python/flink_agents/api/yaml/specs.py index ecfae4e5f..4a2cb4e43 100644 --- a/python/flink_agents/api/yaml/specs.py +++ b/python/flink_agents/api/yaml/specs.py @@ -25,7 +25,7 @@ import json import sys from enum import Enum -from typing import Any, Dict, List, Literal +from typing import Annotated, Any, Dict, List, Literal from pydantic import BaseModel, ConfigDict, Field, model_validator @@ -186,26 +186,20 @@ def _require_one_source(self) -> "SkillsSpec": class ActionSpec(BaseModel): - """An action references a user function and its trigger conditions. + """An action referencing a user function and its trigger conditions. - ``function`` is written as ``:`` — the - colon separates the Python module (or Java class FQN) from the - attribute path inside it. - - ``trigger_conditions`` carries one or more strings. Each is either an - event-type name (bare identifier) or a future condition-expression - form — the runtime classifies the string when it loads the plan. + ``function`` uses ``:``. - Action signatures are fixed (``(Event, RunnerContext)``), so there is - no ``parameter_types`` knob — Python doesn't need it, and the Java - action signature is determined by the action contract. + Each ``trigger_conditions`` entry is an event-type name or Boolean + condition expression. Entries combine with OR semantics. Expression + validation occurs when the plan is applied. """ model_config = ConfigDict(extra="forbid") name: str function: str | None = None - trigger_conditions: List[str] = Field(..., min_length=1) + trigger_conditions: List[Annotated[str, Field(pattern=r"\S")]] = Field(min_length=1) config: Dict[str, Any] | None = None type: Language | None = None diff --git a/python/flink_agents/api/yaml/tests/test_loader.py b/python/flink_agents/api/yaml/tests/test_loader.py index 54986fbf9..2c9c1ca74 100644 --- a/python/flink_agents/api/yaml/tests/test_loader.py +++ b/python/flink_agents/api/yaml/tests/test_loader.py @@ -159,6 +159,31 @@ def test_build_agents_from_single_agent_yaml() -> None: assert shared_actions == {} +def test_loader_resolves_exact_event_aliases(tmp_path: Path) -> None: + yaml_text = ( + "agents:\n" + " - name: a\n" + " actions:\n" + " - name: mixed\n" + f" function: {_TARGETS_MODULE}:increment\n" + " trigger_conditions:\n" + " - input\n" + " - \"attributes.kind == 'input'\"\n" + " - \"'input'\"\n" + ) + path = tmp_path / "mixed_selectors.yaml" + path.write_text(yaml_text) + + agents, _, _ = build_agents(path) + + trigger_conditions, _, _ = agents["a"].actions["mixed"] + assert trigger_conditions == [ + InputEvent.EVENT_TYPE, + "attributes.kind == 'input'", + "'input'", + ] + + def test_build_agents_resolves_event_alias_and_clazz_alias() -> None: agents, _, _ = build_agents(_FIXTURES / "with_descriptors.yaml") agent = agents["chat_agent"] diff --git a/python/flink_agents/api/yaml/tests/test_specs.py b/python/flink_agents/api/yaml/tests/test_specs.py index 2f658344a..a67869231 100644 --- a/python/flink_agents/api/yaml/tests/test_specs.py +++ b/python/flink_agents/api/yaml/tests/test_specs.py @@ -212,6 +212,22 @@ def test_action_spec_rejects_empty_trigger_conditions() -> None: ActionSpec.model_validate({"name": "a1", "trigger_conditions": []}) +def test_action_spec_rejects_blank_trigger_conditions() -> None: + with pytest.raises(ValidationError): + ActionSpec.model_validate({"name": "a1", "trigger_conditions": [" "]}) + + +def test_action_spec_defers_cel_validation() -> None: + raw_entries = ["input", " attributes.ready == true ", "type =="] + spec = ActionSpec.model_validate({"name": "a1", "trigger_conditions": raw_entries}) + assert spec.trigger_conditions == raw_entries + + +def test_action_spec_rejects_non_string_trigger_condition() -> None: + with pytest.raises(ValidationError): + ActionSpec.model_validate({"name": "a1", "trigger_conditions": ["input", 42]}) + + def test_action_spec_defaults() -> None: spec = ActionSpec.model_validate( {"name": "a1", "trigger_conditions": ["input"]} diff --git a/python/flink_agents/e2e_tests/e2e_tests_resource_cross_language/python_agent_with_java_action.py b/python/flink_agents/e2e_tests/e2e_tests_resource_cross_language/python_agent_with_java_action.py index 1f33d1f6d..936b7ee2e 100644 --- a/python/flink_agents/e2e_tests/e2e_tests_resource_cross_language/python_agent_with_java_action.py +++ b/python/flink_agents/e2e_tests/e2e_tests_resource_cross_language/python_agent_with_java_action.py @@ -20,26 +20,42 @@ from pyflink.datastream import KeySelector from flink_agents.api.agents.agent import Agent -from flink_agents.api.events.event import InputEvent +from flink_agents.api.decorators import action +from flink_agents.api.events.event import Event from flink_agents.api.function import JavaFunction +from flink_agents.api.runner_context import RunnerContext JAVA_HANDLER_QUALNAME = "org.apache.flink.agents.resource.test.JavaActionHandler" JAVA_HANDLER_METHOD = "multiplyByTwo" class PythonAgentWithJavaActionAgent(Agent): - """Python agent that dispatches into ``JavaActionHandler.multiplyByTwo``.""" + """Python agent whose overlapping expressions dispatch one Java action.""" def __init__(self) -> None: """Create a PythonAgentWithJavaActionAgent.""" super().__init__() self.add_action( name="multiply_by_two", - trigger_conditions=[InputEvent.EVENT_TYPE], + trigger_conditions=[ + "type == EventType.InputEvent && input > 1 && input < 7", + "type == EventType.InputEvent && input > 3 && input < 9", + ], func=JavaFunction.for_action(JAVA_HANDLER_QUALNAME, JAVA_HANDLER_METHOD), ) +class InvalidTriggerConditionAgent(Agent): + """Agent used to exercise Java Plan validation through the real gateway.""" + + @action("type ==") + @staticmethod + def invalid_condition(event: Event, ctx: RunnerContext) -> None: + """Provide a serializable action body that preflight must reject.""" + message = "an invalid trigger condition must never execute" + raise AssertionError(message) + + class SingleKeySelector(KeySelector): """Mirror of Java ``JavaAgentWithPythonActionAgent.SingleKeySelector``.""" diff --git a/python/flink_agents/e2e_tests/e2e_tests_resource_cross_language/python_agent_with_java_action_test.py b/python/flink_agents/e2e_tests/e2e_tests_resource_cross_language/python_agent_with_java_action_test.py index 0070675bc..b9e620e4a 100644 --- a/python/flink_agents/e2e_tests/e2e_tests_resource_cross_language/python_agent_with_java_action_test.py +++ b/python/flink_agents/e2e_tests/e2e_tests_resource_cross_language/python_agent_with_java_action_test.py @@ -18,6 +18,7 @@ """E2E mirror of ``JavaAgentWithPythonActionTest``: Python agent + Java action body.""" import os +import sys import sysconfig from pathlib import Path @@ -29,6 +30,7 @@ from flink_agents.api.execution_environment import AgentsExecutionEnvironment from flink_agents.e2e_tests.e2e_tests_resource_cross_language.python_agent_with_java_action import ( + InvalidTriggerConditionAgent, PythonAgentWithJavaActionAgent, SingleKeySelector, ) @@ -46,6 +48,24 @@ os.environ["PYTHONPATH"] = sysconfig.get_paths()["purelib"] +def test_apply_rejects_invalid_condition_via_java() -> None: + env = StreamExecutionEnvironment.get_execution_environment() + input_stream = env.from_collection([1], type_info=Types.LONG()) + agents_env = AgentsExecutionEnvironment.get_execution_environment(env=env) + builder = agents_env.from_datastream( + input=input_stream, + key_selector=SingleKeySelector(), + ) + + with pytest.raises(ValueError) as error: + builder.apply(InvalidTriggerConditionAgent()) + + assert str(error.value).startswith( + "Invalid trigger condition #1 for action 'invalid_condition'" + ) + assert '"type =="' in str(error.value) + + @pytest.mark.skipif( not _TEST_JAR.is_file(), reason=( @@ -54,22 +74,21 @@ "flink-agents-end-to-end-tests-resource-cross-language' first." ), ) -def test_python_agent_dispatches_java_action_body(tmp_path: Path) -> None: +def test_overlapping_conditions_run_java_action_once(tmp_path: Path) -> None: config = Configuration() config.set_string("python.pythonpath", sysconfig.get_paths()["purelib"]) + config.set_string("python.executable", sys.executable) env = StreamExecutionEnvironment.get_execution_environment(config) env.set_parallelism(1) env.add_jars(f"file://{_TEST_JAR}") - input_stream = env.from_collection([1, 2, 3, 4, 5], type_info=Types.LONG()).map( + input_stream = env.from_collection([1, 3, 5, 7, 9], type_info=Types.LONG()).map( lambda x: x ) agents_env = AgentsExecutionEnvironment.get_execution_environment(env=env) output_datastream = ( - agents_env.from_datastream( - input=input_stream, key_selector=SingleKeySelector() - ) + agents_env.from_datastream(input=input_stream, key_selector=SingleKeySelector()) .apply(PythonAgentWithJavaActionAgent()) .to_datastream(Types.LONG()) ) @@ -96,4 +115,6 @@ def test_python_agent_dispatches_java_action_body(tmp_path: Path) -> None: actual.extend(int(line.strip()) for line in f if line.strip()) actual.sort() - assert actual == [2, 4, 6, 8, 10], f"unexpected outputs: {actual}" + # 3 is first-only, 5 overlaps (and executes once), 7 is second-only, + # while 1 and 9 do not match. + assert actual == [6, 10, 14], f"unexpected outputs: {actual}" diff --git a/python/flink_agents/plan/actions/action.py b/python/flink_agents/plan/actions/action.py index 9d57ae7c9..faa52a73a 100644 --- a/python/flink_agents/plan/actions/action.py +++ b/python/flink_agents/plan/actions/action.py @@ -19,11 +19,17 @@ import inspect from typing import Any, Dict, List -from pydantic import BaseModel, ConfigDict, field_serializer, model_validator +from pydantic import ( + BaseModel, + ConfigDict, + StrictStr, + field_serializer, + model_validator, +) from flink_agents.api.events.event import Event from flink_agents.api.runner_context import RunnerContext -from flink_agents.plan.function import Function, JavaFunction, PythonFunction +from flink_agents.plan.function import JavaFunction, PythonFunction _CONFIG_TYPE = "__config_type__" # Tags a config entry that we serialized from a pydantic model, so on the way @@ -32,7 +38,7 @@ class Action(BaseModel): - """Representation of an agent action with unified trigger conditions. + """Representation of an agent action with raw trigger conditions. This class encapsulates a named agent action that triggers on matching events and executes an associated function. @@ -44,8 +50,8 @@ class Action(BaseModel): exec : Function To be executed when the Action is triggered. trigger_conditions : List[str] - Event-type name strings that will trigger this Action. Multiple - entries combine with OR semantics. + Event-type names or Boolean condition expressions. Entries combine with + OR semantics. """ model_config = ConfigDict(arbitrary_types_allowed=True) @@ -53,18 +59,9 @@ class Action(BaseModel): name: str # TODO: Raise a warning when the action has a return value, as it will be ignored. exec: PythonFunction | JavaFunction - trigger_conditions: List[str] + trigger_conditions: List[StrictStr] config: Dict[str, Any] | None = None - @property - def listen_event_types(self) -> List[str]: - """Event-type names. Kept for callers that still consume the old naming; - in this PR all entries are plain event-type names so the list is - identical to ``trigger_conditions``. A follow-up PR introduces CEL - expressions and overrides this to filter out non-type entries. - """ - return self.trigger_conditions - @field_serializer("config") def __serialize_config(self, config: Dict[str, Any]) -> Dict[str, Any] | None: if config is None: @@ -91,11 +88,7 @@ def __custom_deserialize(self) -> "Action": config_type = self["config"].pop(_CONFIG_TYPE) if config_type == "java": for name, value in config.items(): - if ( - isinstance(value, dict) - and "@class" in value - and "value" in value - ): + if isinstance(value, dict) and "@class" in value and "value" in value: self["config"][name] = value["value"] return self for name, value in config.items(): @@ -111,16 +104,20 @@ def __custom_deserialize(self) -> "Action": self["config"][name] = value return self - def __init__( - self, - name: str, - exec: Function, - trigger_conditions: List[str], - config: Dict[str, Any] | None = None, - ) -> None: - """Action will check function signature when init.""" - super().__init__( - name=name, exec=exec, trigger_conditions=trigger_conditions, config=config - ) + def model_post_init(self, __context: Any, /) -> None: + """Validate Python-owned structure and the action function signature.""" + if not self.trigger_conditions: + msg = f"Action '{self.name}' must have at least one trigger condition" + raise ValueError(msg) + + for index, raw_source in enumerate(self.trigger_conditions): + if not raw_source.strip(): + msg = ( + f"Invalid trigger condition #{index + 1} for action " + f"'{self.name}' from source \"{raw_source}\": " + "Trigger condition must be non-blank" + ) + raise ValueError(msg) + # TODO: Update expected signature after import State and Context. self.exec.check_signature(Event, RunnerContext) diff --git a/python/flink_agents/plan/agent_plan.py b/python/flink_agents/plan/agent_plan.py index 747aa8577..27b903f6a 100644 --- a/python/flink_agents/plan/agent_plan.py +++ b/python/flink_agents/plan/agent_plan.py @@ -66,14 +66,11 @@ class AgentPlan(BaseModel): ---------- actions: Dict[str, Action] Mapping of action names to actions - actions_by_event : Dict[Type[Event], str] - Mapping of event types to the list of actions name that listen to them. resource_providers: ResourceProvider Two level mapping of resource type to resource name to resource provider. """ actions: Dict[str, Action] - actions_by_event: Dict[str, List[str]] resource_providers: Dict[ResourceType, Dict[str, ResourceProvider]] | None = None config: AgentConfiguration | None = None @@ -141,14 +138,9 @@ def __custom_deserialize(self) -> "AgentPlan": def from_agent(agent: Agent, config: AgentConfiguration) -> "AgentPlan": """Build a AgentPlan from user defined agent.""" actions = {} - actions_by_event = {} for action in _get_actions(agent) + BUILT_IN_ACTIONS: assert action.name not in actions, f"Duplicate action name: {action.name}" actions[action.name] = action - for event_type in action.trigger_conditions: - if event_type not in actions_by_event: - actions_by_event[event_type] = [] - actions_by_event[event_type].append(action.name) resource_providers = {} for provider in _get_resource_providers(agent, config): @@ -162,26 +154,10 @@ def from_agent(agent: Agent, config: AgentConfiguration) -> "AgentPlan": resource_providers[type][name] = provider return AgentPlan( actions=actions, - actions_by_event=actions_by_event, resource_providers=resource_providers, config=config, ) - def get_actions(self, event_type: str) -> List[Action]: - """Get actions that listen to the specified event type. - - Parameters - ---------- - event_type : Type[Event] - The event type to query. - - Returns: - ------- - list[Action] - List of Actions that will respond to this event type. - """ - return [self.actions[name] for name in self.actions_by_event[event_type]] - def get_action_config(self, action_name: str) -> Dict[str, Any]: """Get config of the action. @@ -215,17 +191,6 @@ def get_action_config_value(self, action_name: str, key: str) -> Any: return self.actions[action_name].config.get(key, None) -def _resolve_event_type(evt: Any) -> str: - """Convert a listen-event entry to a routing-key string. - - Only string type identifiers are accepted. - """ - if isinstance(evt, str): - return evt - msg = f"Event type must be a string identifier, got {evt!r}" - raise ValueError(msg) - - def _action_marker(value: Any) -> tuple | None: """Return ``(inner_callable, trigger_conditions, target)`` if ``value`` is @action. @@ -289,9 +254,7 @@ def _get_actions(agent: Agent) -> List[Action]: Action( name=name, exec=exec_, - trigger_conditions=[ - _resolve_event_type(et) for et in trigger_conditions - ], + trigger_conditions=list(trigger_conditions), ) ) for name, action_tuple in agent.actions.items(): @@ -299,10 +262,7 @@ def _get_actions(agent: Agent) -> List[Action]: Action( name=name, exec=_to_plan_function(action_tuple[1]), - trigger_conditions=[ - _resolve_event_type(et) - for et in action_tuple[0] - ], + trigger_conditions=list(action_tuple[0]), config=action_tuple[2], ) ) diff --git a/python/flink_agents/plan/tests/compatibility/create_python_agent_plan_from_json.py b/python/flink_agents/plan/tests/compatibility/create_python_agent_plan_from_json.py index 6eda7d96e..cb558a2a2 100644 --- a/python/flink_agents/plan/tests/compatibility/create_python_agent_plan_from_json.py +++ b/python/flink_agents/plan/tests/compatibility/create_python_agent_plan_from_json.py @@ -53,8 +53,8 @@ event, runner_context, ] - listen_event_types1 = action1.trigger_conditions - assert listen_event_types1 == [input_event] + trigger_conditions1 = action1.trigger_conditions + assert trigger_conditions1 == [input_event] # check the second action assert "secondAction" in actions @@ -67,21 +67,15 @@ event, runner_context, ] - listen_event_types2 = action2.trigger_conditions - assert sorted(listen_event_types2) == [ + trigger_conditions2 = action2.trigger_conditions + assert sorted(trigger_conditions2) == [ my_event, input_event, ] - # check actions_by_event - actions_by_event = agent_plan.actions_by_event - assert len(actions_by_event) == 6 - - assert input_event in actions_by_event - assert sorted(actions_by_event[input_event]) == ["firstAction", "secondAction"] - - assert my_event in actions_by_event - assert actions_by_event[my_event] == ["secondAction"] + # Runtime indexes are derived after plan loading and are not part of the + # wire format. + assert "actions_by_event" not in raw_agent_plan # check Java tool parameter injection survives agent plan JSON raw_tool_providers = raw_agent_plan["resource_providers"].get( diff --git a/python/flink_agents/plan/tests/resources/agent_plan.json b/python/flink_agents/plan/tests/resources/agent_plan.json index 6e545ab1d..27781c8e4 100644 --- a/python/flink_agents/plan/tests/resources/agent_plan.json +++ b/python/flink_agents/plan/tests/resources/agent_plan.json @@ -63,27 +63,6 @@ "config": null } }, - "actions_by_event": { - "_input_event": [ - "first_action", - "second_action" - ], - "_my_event": [ - "second_action" - ], - "_chat_request_event": [ - "chat_model_action" - ], - "_tool_response_event": [ - "chat_model_action" - ], - "_tool_request_event": [ - "tool_call_action" - ], - "_context_retrieval_request_event": [ - "context_retrieval_action" - ] - }, "resource_providers": { "chat_model": { "mock": { diff --git a/python/flink_agents/plan/tests/test_action.py b/python/flink_agents/plan/tests/test_action.py index 08f0d5dd2..bee7ec841 100644 --- a/python/flink_agents/plan/tests/test_action.py +++ b/python/flink_agents/plan/tests/test_action.py @@ -19,6 +19,7 @@ from pathlib import Path import pytest +from pydantic import ValidationError from pyflink.common.typeinfo import BasicTypeInfo, RowTypeInfo from flink_agents.api.agents.react_agent import OutputSchema @@ -88,7 +89,7 @@ def test_action_deserialize(action: Action) -> None: expected_json = f.read() action = Action.model_validate_json(expected_json) assert action.name == "legal" - assert action.trigger_conditions== ["_input_event"] + assert action.trigger_conditions == ["_input_event"] func = action.exec assert func.module == "flink_agents.plan.tests.test_action" assert func.qualname == "legal_signature" @@ -171,3 +172,32 @@ def test_action_deserialize_python_config_preserves_plain_list() -> None: ) action = Action.model_validate_json(json_str) assert action.config == {"hosts": ["host-a", "host-b", "host-c"]} + + +@pytest.mark.parametrize( + ("trigger_conditions", "message"), + [ + ([], "must have at least one trigger condition"), + ([" "], "Invalid trigger condition #1"), + ], +) +def test_action_rejects_empty_or_blank_trigger_conditions( + trigger_conditions: list[str], message: str +) -> None: + with pytest.raises(ValueError, match=message): + Action( + name="invalid", + exec=PythonFunction.from_callable(legal_signature), + trigger_conditions=trigger_conditions, + ) + + +def test_action_rejects_non_string_trigger_condition() -> None: + with pytest.raises(ValidationError) as exc_info: + Action( + name="invalid", + exec=PythonFunction.from_callable(legal_signature), + trigger_conditions=[42], # type: ignore[list-item] + ) + + assert exc_info.value.errors()[0]["loc"] == ("trigger_conditions", 0) diff --git a/python/flink_agents/plan/tests/test_agent_plan.py b/python/flink_agents/plan/tests/test_agent_plan.py index 04fdb6c79..280145e7d 100644 --- a/python/flink_agents/plan/tests/test_agent_plan.py +++ b/python/flink_agents/plan/tests/test_agent_plan.py @@ -48,6 +48,7 @@ BaseVectorStore, Document, ) +from flink_agents.plan.actions.action import Action from flink_agents.plan.agent_plan import AgentPlan from flink_agents.plan.configuration import AgentConfiguration from flink_agents.plan.function import PythonFunction @@ -68,9 +69,7 @@ def increment(event: Event, ctx: RunnerContext) -> None: def test_from_agent(): agent = AgentForTest() agent_plan = AgentPlan.from_agent(agent, AgentConfiguration()) - actions = agent_plan.get_actions(InputEvent.EVENT_TYPE) - assert len(actions) == 1 - action = actions[0] + action = agent_plan.actions["increment"] assert action.name == "increment" func = action.exec assert isinstance(func, PythonFunction) @@ -79,6 +78,23 @@ def test_from_agent(): assert action.trigger_conditions == [InputEvent.EVENT_TYPE] +class RawConditionAgent(Agent): + @action(EventType.InputEvent, " attributes.ready == true ", "type ==") + @staticmethod + def handle(event: Event, ctx: RunnerContext) -> None: + pass + + +def test_from_agent_preserves_raw_conditions() -> None: + plan = AgentPlan.from_agent(RawConditionAgent(), AgentConfiguration()) + + assert plan.actions["handle"].trigger_conditions == [ + InputEvent.EVENT_TYPE, + " attributes.ready == true ", + "type ==", + ] + + class InvalidAgent(Agent): @action(EventType.InputEvent) @staticmethod @@ -118,13 +134,12 @@ def test_conventional_staticmethod_outer_decorator_order_is_registered() -> None plan = AgentPlan.from_agent( AgentWithConventionalDecoratorOrder(), AgentConfiguration() ) - actions = plan.get_actions(InputEvent.EVENT_TYPE) - assert len(actions) == 1, ( + assert "handle" in plan.actions, ( "Action defined with `@staticmethod` outer / `@action` inner was silently " "dropped — `_get_actions` should unwrap the staticmethod before checking " "for `_trigger_conditions`." ) - assert actions[0].name == "handle" + assert plan.actions["handle"].trigger_conditions == [InputEvent.EVENT_TYPE] class _BaseAgentWithInheritedAction(Agent): @@ -356,6 +371,26 @@ def test_agent_plan_serialize(agent_plan: AgentPlan) -> None: assert actual == expected +def test_plan_serializes_actions_without_legacy_index() -> None: + action = Action( + name="a", + exec=PythonFunction.from_callable(MyAgent.first_action), + trigger_conditions=[InputEvent.EVENT_TYPE, "attributes.ready == true"], + ) + plan = AgentPlan( + actions={"a": action}, + resource_providers={}, + config=AgentConfiguration(), + ) + payload = json.loads(plan.model_dump_json(serialize_as_any=True)) + + assert payload["actions"]["a"]["trigger_conditions"] == [ + InputEvent.EVENT_TYPE, + "attributes.ready == true", + ] + assert "actions_by_event" not in payload + + def test_agent_plan_deserialize(agent_plan: AgentPlan) -> None: with Path.open(Path(f"{current_dir}/resources/agent_plan.json")) as f: expected_json = f.read() @@ -563,32 +598,3 @@ def test_add_action_and_resource_to_agent() -> None: actual = json.loads(json_value) expected = json.loads(expected_json) assert actual == expected - - -# ── String identifier tests ────────────────────────────────────────────── - - -class StringIdAgent(Agent): - """Agent with actions listening to string identifiers.""" - - @action("CustomEvent") - @staticmethod - def handle_custom(event: Event, ctx: RunnerContext) -> None: - ctx.send_event(OutputEvent(output=event.get_attr("msg"))) - - -def test_from_agent_with_string_identifier() -> None: - """Test that AgentPlan correctly handles string identifiers.""" - agent = StringIdAgent() - agent_plan = AgentPlan.from_agent(agent, AgentConfiguration()) - - # The string identifier should be preserved as-is - actions = agent_plan.get_actions("CustomEvent") - assert len(actions) == 1 - assert actions[0].name == "handle_custom" - assert "CustomEvent" in actions[0].trigger_conditions - - # Verify serialization roundtrip preserves the string identifier - json_str = agent_plan.model_dump_json(serialize_as_any=True) - restored = AgentPlan.model_validate_json(json_str) - assert restored.get_actions("CustomEvent")[0].name == "handle_custom" diff --git a/python/flink_agents/plan/tests/test_agent_plan_cross_language.py b/python/flink_agents/plan/tests/test_agent_plan_cross_language.py index 31bacf532..f78b75750 100644 --- a/python/flink_agents/plan/tests/test_agent_plan_cross_language.py +++ b/python/flink_agents/plan/tests/test_agent_plan_cross_language.py @@ -24,6 +24,10 @@ import pytest from flink_agents.api.agents.agent import Agent +from flink_agents.api.core_options import ( + AgentConfigOptions, + ConditionEvaluationFailureStrategy, +) from flink_agents.api.events.event import Event, InputEvent from flink_agents.api.function import ( JavaFunction as ApiJavaFunction, @@ -170,10 +174,15 @@ def _java_action_plan() -> AgentPlan: agent = Agent() agent.add_action( name="handle", - trigger_conditions=[InputEvent.EVENT_TYPE], + trigger_conditions=[InputEvent.EVENT_TYPE, "attributes.ready == true"], func=_make_java_function_descriptor(), ) - return AgentPlan.from_agent(agent, AgentConfiguration()) + config = AgentConfiguration() + config.set( + AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY, + ConditionEvaluationFailureStrategy.FAIL, + ) + return AgentPlan.from_agent(agent, config) def _python_action_plan() -> AgentPlan: @@ -204,6 +213,23 @@ def test_python_plan_with_java_action_has_expected_exec_shape() -> None: } +def test_failure_strategy_round_trips_in_plan_json() -> None: + serialized = _plan_dump_json(_java_action_plan()) + payload = json.loads(serialized) + + assert ( + payload["config"]["conf_data"][ + "action.trigger-condition.evaluate-failure-strategy" + ] + == "FAIL" + ) + restored = AgentPlan.model_validate_json(serialized) + assert ( + restored.config.get(AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY) + is ConditionEvaluationFailureStrategy.FAIL + ) + + def test_python_plan_with_python_action_has_expected_exec_shape() -> None: """Pin the wire shape of a Python-target action's ``exec`` block.""" plan = _python_action_plan() @@ -229,6 +255,10 @@ def test_python_plan_with_java_action_round_trips_through_json() -> None: assert isinstance(action.exec, PlanJavaFunction) assert action.exec.qualname == "com.example.Handlers" assert action.exec.method_name == "handle" + assert action.trigger_conditions == [ + InputEvent.EVENT_TYPE, + "attributes.ready == true", + ] assert list(action.exec.parameter_types) == [ "org.apache.flink.agents.api.Event", "org.apache.flink.agents.api.context.RunnerContext", @@ -290,9 +320,13 @@ def test_python_can_deserialize_java_plan_with_python_action() -> None: ) assert action.exec.module == _dummy_action.__module__ assert action.exec.qualname == _dummy_action.__qualname__ + assert action.trigger_conditions == [ + InputEvent.EVENT_TYPE, + "attributes.ready == true", + ] -def test_python_plan_with_java_action_matches_runtime_operator_wire_shape() -> None: +def test_java_action_plan_matches_runtime_wire_shape() -> None: handler_qualname = ( "org.apache.flink.agents.runtime.operator." "CrossLanguageActionRuntimeTest$Handlers" @@ -326,7 +360,6 @@ def test_python_plan_with_java_action_matches_runtime_operator_wire_shape() -> N "method_name": "handleInput", "parameter_types": expected_parameter_types, } - assert emitted["actions_by_event"][InputEvent.EVENT_TYPE] == ["handle"] def test_python_preserves_conf_data_types_and_event_ordering() -> None: @@ -354,7 +387,6 @@ def test_python_preserves_conf_data_types_and_event_ordering() -> None: "config": None, }, }, - "actions_by_event": {InputEvent.EVENT_TYPE: ["first", "second"]}, "resource_providers": {}, "config": { "conf_data": { @@ -374,4 +406,4 @@ def test_python_preserves_conf_data_types_and_event_ordering() -> None: "k_bool": True, "k_str": "v1", } - assert restored.actions_by_event[InputEvent.EVENT_TYPE] == ["first", "second"] + assert list(restored.actions) == ["first", "second"] diff --git a/python/flink_agents/runtime/remote_execution_environment.py b/python/flink_agents/runtime/remote_execution_environment.py index b14ed0eb3..5071094ee 100644 --- a/python/flink_agents/runtime/remote_execution_environment.py +++ b/python/flink_agents/runtime/remote_execution_environment.py @@ -45,6 +45,8 @@ _CONFIG_FILE_NAME = "config.yaml" _LEGACY_CONFIG_FILE_NAME = "flink-conf.yaml" +_AGENT_PLAN_PREFLIGHT_CLASS = "org.apache.flink.agents.plan.AgentPlanPreflight" +_AGENT_PLAN_PREFLIGHT_METHOD = "findValidationError" class RemoteAgentBuilder(AgentBuilder): @@ -107,10 +109,30 @@ def apply(self, agent: Agent | str) -> "AgentBuilder": for type, name_to_resource in self.__resources.items(): agent.resources[type] = name_to_resource | agent.resources[type] - self.__agent_plan = AgentPlan.from_agent(agent, self.__config) + candidate_plan = AgentPlan.from_agent(agent, self.__config) + candidate_plan_json = candidate_plan.model_dump_json(serialize_as_any=True) + self.__preflight(candidate_plan_json) + self.__agent_plan = candidate_plan return self + @staticmethod + def __preflight(agent_plan_json: str) -> None: + try: + error_message = invoke_method( + None, + _AGENT_PLAN_PREFLIGHT_CLASS, + _AGENT_PLAN_PREFLIGHT_METHOD, + [agent_plan_json], + ["java.lang.String"], + ) + except Exception as error: + message = "Java AgentPlan preflight failed." + raise RuntimeError(message) from error + + if error_message is not None: + raise ValueError(error_message) + def to_datastream(self, output_type: TypeInformation | None = None) -> DataStream: """Get output datastream of agent execution. diff --git a/python/flink_agents/runtime/tests/test_remote_execution_environment.py b/python/flink_agents/runtime/tests/test_remote_execution_environment.py index 3a58cc222..5ec373f22 100644 --- a/python/flink_agents/runtime/tests/test_remote_execution_environment.py +++ b/python/flink_agents/runtime/tests/test_remote_execution_environment.py @@ -15,6 +15,7 @@ # See the License for the specific language governing permissions and # limitations under the License. ################################################################################# +import json import os import tempfile from pathlib import Path @@ -23,8 +24,12 @@ import pytest import yaml +from flink_agents.api.agents.agent import Agent +from flink_agents.api.events.event import Event +from flink_agents.api.runner_context import RunnerContext from flink_agents.plan.configuration import AgentConfiguration from flink_agents.runtime.remote_execution_environment import ( + RemoteAgentBuilder, RemoteExecutionEnvironment, ) @@ -40,6 +45,30 @@ } +def _action(event: Event, ctx: RunnerContext) -> None: + pass + + +def _agent_with_conditions(trigger_conditions: list[str]) -> Agent: + agent = Agent() + agent.add_action( + name="handle", + trigger_conditions=trigger_conditions, + func=_action, + ) + return agent + + +def _remote_builder() -> RemoteAgentBuilder: + with patch( + "flink_agents.runtime.remote_execution_environment.StreamExecutionEnvironment" + ): + remote_env = RemoteExecutionEnvironment(env=MagicMock()) + return remote_env.from_datastream( + input=MagicMock(), key_selector=lambda record: record["key"] + ) + + def test_remote_execution_environment_load_config_file() -> None: """Test RemoteExecutionEnvironment loads config from config.yaml.""" # Create a temporary directory with config.yaml @@ -220,6 +249,81 @@ def test_apply_by_unknown_name_errors() -> None: builder.apply("ghost") +def test_apply_sends_plan_to_java_preflight() -> None: + builder = _remote_builder() + agent = _agent_with_conditions(["type == EventType.InputEvent && input > 2"]) + + with patch( + "flink_agents.runtime.remote_execution_environment.invoke_method", + return_value=None, + ) as invoke: + returned = builder.apply(agent) + + assert returned is builder + invoke.assert_called_once() + _, class_name, method_name, args, parameter_types = invoke.call_args.args + assert class_name == "org.apache.flink.agents.plan.AgentPlanPreflight" + assert method_name == "findValidationError" + assert parameter_types == ["java.lang.String"] + payload = json.loads(args[0]) + assert payload["actions"]["handle"]["trigger_conditions"] == [ + "type == EventType.InputEvent && input > 2" + ] + + +def test_apply_surfaces_java_error_and_allows_retry() -> None: + builder = _remote_builder() + agent = _agent_with_conditions(["type =="]) + error_message = ( + "Invalid trigger condition #1 for action 'handle' from source " + '"type ==": syntax error' + ) + + with patch( + "flink_agents.runtime.remote_execution_environment.invoke_method", + side_effect=[error_message, None], + ) as invoke: + with pytest.raises(ValueError) as error: + builder.apply(agent) + returned = builder.apply(agent) + + assert str(error.value) == error_message + assert returned is builder + assert invoke.call_count == 2 + + +def test_apply_wraps_java_preflight_failure() -> None: + builder = _remote_builder() + agent = _agent_with_conditions(["_input_event"]) + bridge_error = RuntimeError("gateway failed") + + with ( + patch( + "flink_agents.runtime.remote_execution_environment.invoke_method", + side_effect=bridge_error, + ), + pytest.raises(RuntimeError, match="Java AgentPlan preflight failed") as error, + ): + builder.apply(agent) + + assert error.value.__cause__ is bridge_error + + +def test_apply_rejects_structure_before_java_preflight() -> None: + builder = _remote_builder() + agent = _agent_with_conditions([" "]) + + with ( + patch( + "flink_agents.runtime.remote_execution_environment.invoke_method" + ) as invoke, + pytest.raises(ValueError, match="Invalid trigger condition #1"), + ): + builder.apply(agent) + + invoke.assert_not_called() + + def _verify_config(config: AgentConfiguration) -> None: assert config.get_str("database.host") == "localhost" assert config.get_int("database.port") == 5432 diff --git a/runtime/pom.xml b/runtime/pom.xml index 422c5c320..22062ab2e 100644 --- a/runtime/pom.xml +++ b/runtime/pom.xml @@ -193,6 +193,11 @@ under the License. jackson-datatype-jsr310 ${jackson.version} + + dev.cel + cel + ${cel.version} + diff --git a/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ActionMatcher.java b/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ActionMatcher.java new file mode 100644 index 000000000..46225c824 --- /dev/null +++ b/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ActionMatcher.java @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.runtime.condition; + +import org.apache.flink.agents.api.Event; +import org.apache.flink.agents.api.configuration.AgentConfigOptions; +import org.apache.flink.agents.plan.AgentPlan; +import org.apache.flink.agents.plan.actions.Action; +import org.apache.flink.agents.plan.condition.TriggerCondition; +import org.apache.flink.agents.plan.condition.TriggerCondition.EventTypeCondition; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** Matches actions against event types and condition expressions. */ +public final class ActionMatcher { + + private final Map> actionsByEventType; + private final Map> + compiledConditionsByAction; + private final ConditionEvaluator conditionEvaluator; + + public ActionMatcher(AgentPlan agentPlan) { + if (agentPlan == null) { + throw new IllegalArgumentException("ActionMatcher: agentPlan must not be null"); + } + Map> eventTypeIndex = new LinkedHashMap<>(); + Map> conditionIndex = + new LinkedHashMap<>(); + for (Action action : agentPlan.getActions().values()) { + List actionConditions = + new ArrayList<>(); + for (TriggerCondition condition : action.getClassifiedTriggerConditions()) { + if (condition instanceof EventTypeCondition) { + String eventType = ((EventTypeCondition) condition).eventType(); + // Raw trigger conditions preserve duplicates; the runtime index keeps each + // action once. + eventTypeIndex + .computeIfAbsent(eventType, ignored -> new LinkedHashSet<>()) + .add(action); + } else if (condition instanceof ExpressionCondition) { + actionConditions.add( + ConditionExpressionCompiler.compile((ExpressionCondition) condition)); + } else { + throw new IllegalStateException( + "Unsupported trigger condition type: " + + condition.getClass().getName()); + } + } + if (!actionConditions.isEmpty()) { + conditionIndex.put(action, actionConditions); + } + } + this.actionsByEventType = eventTypeIndex; + this.compiledConditionsByAction = conditionIndex; + this.conditionEvaluator = + new ConditionEvaluator( + agentPlan + .getConfig() + .get(AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY)); + } + + /** + * Returns actions whose trigger conditions match the event. + * + *

Exact event-type matches precede expression matches, and each action appears at most once. + */ + public List match(Event event) { + Set eventTypeMatches = + actionsByEventType.getOrDefault(event.getType(), Collections.emptySet()); + List matchingActions = new ArrayList<>(eventTypeMatches); + + for (Map.Entry> entry : + compiledConditionsByAction.entrySet()) { + Action action = entry.getKey(); + if (eventTypeMatches.contains(action)) { + continue; + } + for (ConditionExpressionCompiler.CompiledCondition condition : entry.getValue()) { + if (conditionEvaluator.evaluate(condition, event)) { + matchingActions.add(action); + break; + } + } + } + return matchingActions; + } +} diff --git a/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ConditionEvaluator.java b/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ConditionEvaluator.java new file mode 100644 index 000000000..d2762dec2 --- /dev/null +++ b/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ConditionEvaluator.java @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.runtime.condition; + +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.cel.common.values.NullValue; +import dev.cel.runtime.CelEvaluationException; +import org.apache.flink.agents.api.Event; +import org.apache.flink.agents.api.EventType; +import org.apache.flink.agents.api.configuration.AgentConfigOptions.ConditionEvaluationFailureStrategy; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.math.BigDecimal; +import java.math.BigInteger; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** Evaluates trigger condition expressions against event data. */ +final class ConditionEvaluator { + + private static final Logger LOG = LoggerFactory.getLogger(ConditionEvaluator.class); + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + private final ConditionEvaluationFailureStrategy failureStrategy; + + ConditionEvaluator(ConditionEvaluationFailureStrategy failureStrategy) { + this.failureStrategy = failureStrategy; + } + + /** Builds only the variables needed by one reached condition, then evaluates it. */ + boolean evaluate(ConditionExpressionCompiler.CompiledCondition condition, Event event) { + Map conditionVariables; + try { + conditionVariables = buildConditionVariables(event, condition); + } catch (RuntimeException e) { + return handleConditionFailure( + "Building trigger condition variables failed for event " + event.getId(), e); + } + + String source = condition.source(); + Object result; + try { + result = condition.program().eval(conditionVariables); + } catch (CelEvaluationException e) { + return handleConditionFailure( + "Trigger condition evaluation failed for '" + source + "'", e); + } + if (result instanceof Boolean) { + return (Boolean) result; + } + return handleConditionFailure( + String.format( + "Trigger condition '%s' returned non-boolean type %s", + source, result == null ? "null" : result.getClass().getName()), + null); + } + + /** + * Applies the failure strategy to one condition failure: {@code FAIL} throws, otherwise the + * failure is logged and the condition is treated as false. + */ + private boolean handleConditionFailure(String message, Throwable cause) { + if (failureStrategy == ConditionEvaluationFailureStrategy.FAIL) { + throw new IllegalStateException(message, cause); + } + LOG.warn("{}, treating this condition as false", message, cause); + return false; + } + + /** + * Builds the condition variables required for {@code event} from the referenced keys of {@code + * condition}. + * + *

The {@code attributes} entry remains the explicit root namespace for the event attribute + * map. This is needed for literal keys that cannot be represented as identifiers, such as + * {@code attributes["www.andriod.com"].ip}; simple keys are also promoted for bare-identifier + * access. + */ + Map buildConditionVariables( + Event event, ConditionExpressionCompiler.CompiledCondition condition) { + Set referencedTopLevelAttributeKeys = condition.referencedTopLevelAttributeKeys(); + + Map conditionVariables = new HashMap<>(); + conditionVariables.put("type", event.getType()); + conditionVariables.put("EventType", EventType.allConstants()); + if (event.getId() != null) { + conditionVariables.put("id", event.getId().toString()); + } + + Map attrs = event.getAttributes(); + Map normalizedAttributes = new HashMap<>(); + for (String key : referencedTopLevelAttributeKeys) { + if (attrs.containsKey(key)) { + normalizedAttributes.put(key, normalizeValue(attrs.get(key))); + } + } + + conditionVariables.put("attributes", normalizedAttributes); + // Promote to top level for bare-identifier access; framework keys win on collision. + normalizedAttributes.forEach(conditionVariables::putIfAbsent); + return conditionVariables; + } + + /** + * Normalizes Java values for condition evaluation, converting nulls, numbers, collections, and + * Jackson-serializable objects while preserving evaluator-native values. + */ + @SuppressWarnings("unchecked") + private static Object normalizeValue(Object value) { + if (value == null) { + return NullValue.NULL_VALUE; + } + if (value instanceof Map) { + Map src = (Map) value; + Map dst = new HashMap<>(src.size()); + for (Map.Entry entry : src.entrySet()) { + dst.put(entry.getKey(), normalizeValue(entry.getValue())); + } + return dst; + } + if (value instanceof List) { + List src = (List) value; + List dst = new ArrayList<>(src.size()); + for (Object item : src) { + dst.add(normalizeValue(item)); + } + return dst; + } + if (value instanceof Byte || value instanceof Short || value instanceof Integer) { + return ((Number) value).longValue(); + } + if (value instanceof Float) { + return ((Float) value).doubleValue(); + } + if (value instanceof BigInteger) { + BigInteger bigInt = (BigInteger) value; + if (bigInt.bitLength() < 64) { + return bigInt.longValue(); + } + LOG.debug( + "normalizeValue: BigInteger overflows int64, converting to double (possible precision loss): {}", + bigInt); + return bigInt.doubleValue(); + } + if (value instanceof BigDecimal) { + BigDecimal bigDec = (BigDecimal) value; + LOG.debug( + "normalizeValue: converting BigDecimal to double (possible precision loss): {}", + bigDec); + return bigDec.doubleValue(); + } + if (value == NullValue.NULL_VALUE + || value instanceof String + || value instanceof Boolean + || value instanceof Long + || value instanceof Double + || value instanceof byte[]) { + return value; + } + + Object converted = OBJECT_MAPPER.convertValue(value, Object.class); + if (converted != null && converted.getClass() == value.getClass()) { + throw new IllegalArgumentException( + "Unsupported condition attribute type: " + value.getClass().getName()); + } + return normalizeValue(converted); + } +} diff --git a/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ConditionExpressionCompiler.java b/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ConditionExpressionCompiler.java new file mode 100644 index 000000000..ec10acef4 --- /dev/null +++ b/runtime/src/main/java/org/apache/flink/agents/runtime/condition/ConditionExpressionCompiler.java @@ -0,0 +1,218 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.runtime.condition; + +import dev.cel.common.CelAbstractSyntaxTree; +import dev.cel.common.CelValidationException; +import dev.cel.common.Operator; +import dev.cel.common.ast.CelConstant; +import dev.cel.common.ast.CelExpr; +import dev.cel.common.navigation.CelNavigableAst; +import dev.cel.common.navigation.CelNavigableExpr; +import dev.cel.common.types.CelType; +import dev.cel.common.types.MapType; +import dev.cel.common.types.SimpleType; +import dev.cel.compiler.CelCompilerBuilder; +import dev.cel.compiler.CelCompilerFactory; +import dev.cel.runtime.CelEvaluationException; +import dev.cel.runtime.CelRuntime; +import dev.cel.runtime.CelRuntimeFactory; +import org.apache.flink.agents.plan.condition.ConditionExpressionDialect; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; + +import java.util.HashSet; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; + +/** Compiles Plan-classified expressions into executable programs and referenced attribute keys. */ +final class ConditionExpressionCompiler { + + /** Condition variables and the user-attribute namespace with their types at compile time. */ + private static final Map FRAMEWORK_VARIABLE_TYPES = + Map.of( + "id", + SimpleType.STRING, + "type", + SimpleType.STRING, + "EventType", + MapType.create(SimpleType.STRING, SimpleType.STRING), + "attributes", + MapType.create(SimpleType.STRING, SimpleType.DYN)); + + private static final CelRuntime CEL_RUNTIME = + CelRuntimeFactory.standardCelRuntimeBuilder() + .setOptions(ConditionExpressionDialect.options()) + .build(); + + private ConditionExpressionCompiler() {} + + /** Re-parses and compiles a Plan-classified expression during Runtime initialization. */ + static CompiledCondition compile(ExpressionCondition condition) { + Objects.requireNonNull(condition, "condition"); + String source = condition.text(); + + CelAbstractSyntaxTree parsed; + try { + parsed = ConditionExpressionDialect.parse(source); + } catch (CelValidationException e) { + throw new IllegalArgumentException( + "Plan-validated trigger condition could not be reparsed at Runtime: \"" + + source + + "\" — " + + e.getMessage(), + e); + } + + CelAbstractSyntaxTree checked = typeCheck(source, parsed); + CelType resultType = checked.getResultType(); + if (!resultType.kind().isDyn() && !SimpleType.BOOL.isAssignableFrom(resultType)) { + throw new IllegalArgumentException( + "Trigger condition must have a Boolean result at Runtime: \"" + + source + + "\" has static type " + + resultType.name()); + } + try { + CelRuntime.Program program = CEL_RUNTIME.createProgram(checked); + Set referencedTopLevelAttributeKeys = new HashSet<>(); + CelNavigableAst.fromAst(parsed) + .getRoot() + .allNodes() + .filter(node -> node.getKind() == CelExpr.ExprKind.Kind.IDENT) + .forEach(node -> classifyIdent(node, referencedTopLevelAttributeKeys, source)); + return new CompiledCondition(source, program, referencedTopLevelAttributeKeys); + } catch (CelEvaluationException e) { + throw new IllegalArgumentException( + "Failed to create Runtime program for trigger condition: \"" + + source + + "\" — " + + e.getMessage(), + e); + } + } + + private static CelAbstractSyntaxTree typeCheck(String source, CelAbstractSyntaxTree parsed) { + CelCompilerBuilder builder = + CelCompilerFactory.standardCelCompilerBuilder() + .setOptions(ConditionExpressionDialect.options()); + FRAMEWORK_VARIABLE_TYPES.forEach(builder::addVar); + CelNavigableAst.fromAst(parsed) + .getRoot() + .allNodes() + .filter(node -> node.getKind() == CelExpr.ExprKind.Kind.IDENT) + .map(node -> node.expr().ident().name()) + .filter(ident -> !FRAMEWORK_VARIABLE_TYPES.containsKey(ident)) + .distinct() + .forEach(ident -> builder.addVar(ident, SimpleType.DYN)); + + try { + return builder.build().check(parsed).getAst(); + } catch (CelValidationException e) { + throw new IllegalArgumentException( + "Trigger condition failed Runtime type-check: \"" + + source + + "\" — " + + e.getMessage(), + e); + } + } + + private static void classifyIdent(CelNavigableExpr node, Set keys, String source) { + String name = node.expr().ident().name(); + // id/type/EventType are framework-owned (put()), never attribute keys. Expression + // operators, type conversions and macros never appear as a bare IDENT, so no further skip + // list is needed. + if (name.equals("id") || name.equals("type") || name.equals("EventType")) { + return; + } + if (!name.equals("attributes")) { + // A dynamic index key such as attributes[region_id] also references region_id's value + // — keep it. + keys.add(name); + return; + } + Optional parent = node.parent(); + if (parent.isPresent() && parent.get().getKind() == CelExpr.ExprKind.Kind.SELECT) { + keys.add(parent.get().expr().select().field()); + return; + } + + CelExpr keyExpr = null; + if (parent.isPresent() && parent.get().getKind() == CelExpr.ExprKind.Kind.CALL) { + CelExpr.CelCall call = parent.get().expr().call(); + if (call.args().size() == 2) { + if (Operator.INDEX.getFunction().equals(call.function()) + && call.args().get(0).id() == node.id()) { + keyExpr = call.args().get(1); + } else if (Operator.IN.getFunction().equals(call.function()) + && call.args().get(1).id() == node.id()) { + keyExpr = call.args().get(0); + } + } + } + + if (keyExpr != null + && keyExpr.getKind() == CelExpr.ExprKind.Kind.CONSTANT + && keyExpr.constant().getKind() == CelConstant.Kind.STRING_VALUE) { + keys.add(keyExpr.constant().stringValue()); + return; + } + + // Whole-root-map operations and dynamic root-key access cannot be evaluated against the + // trimmed attributes activation. Static root keys and nested maps below them remain + // supported. + throw new IllegalArgumentException( + "Invalid trigger condition expression: \"" + + source + + "\" — the whole 'attributes' map cannot be used (e.g. dynamic index" + + " attributes[expr], size(attributes), or attributes == ...); reference" + + " attributes by a literal key such as attributes['key'], 'key' in" + + " attributes, or has(attributes.key)."); + } + + /** Immutable result shared by condition evaluation and activation construction. */ + static final class CompiledCondition { + private final String source; + private final CelRuntime.Program program; + private final Set referencedTopLevelAttributeKeys; + + private CompiledCondition( + String source, + CelRuntime.Program program, + Set referencedTopLevelAttributeKeys) { + this.source = source; + this.program = program; + this.referencedTopLevelAttributeKeys = Set.copyOf(referencedTopLevelAttributeKeys); + } + + String source() { + return source; + } + + CelRuntime.Program program() { + return program; + } + + Set referencedTopLevelAttributeKeys() { + return referencedTopLevelAttributeKeys; + } + } +} diff --git a/runtime/src/main/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperator.java b/runtime/src/main/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperator.java index 46d09e0a2..96631776c 100644 --- a/runtime/src/main/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperator.java +++ b/runtime/src/main/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperator.java @@ -266,7 +266,7 @@ private void processEvent(Object key, Event event) throws Exception { } // We then obtain the triggered action and add ActionTasks to the waiting processing // queue. - List triggerActions = eventRouter.getActionsTriggeredBy(event, agentPlan); + List triggerActions = eventRouter.getActionsTriggeredBy(event); if (triggerActions != null && !triggerActions.isEmpty()) { for (Action triggerAction : triggerActions) { stateManager.addActionTask(createActionTask(key, triggerAction, event)); diff --git a/runtime/src/main/java/org/apache/flink/agents/runtime/operator/EventRouter.java b/runtime/src/main/java/org/apache/flink/agents/runtime/operator/EventRouter.java index 7eecc5d26..1ed5025fa 100644 --- a/runtime/src/main/java/org/apache/flink/agents/runtime/operator/EventRouter.java +++ b/runtime/src/main/java/org/apache/flink/agents/runtime/operator/EventRouter.java @@ -30,6 +30,7 @@ import org.apache.flink.agents.api.logger.LoggerType; import org.apache.flink.agents.plan.AgentPlan; import org.apache.flink.agents.plan.actions.Action; +import org.apache.flink.agents.runtime.condition.ActionMatcher; import org.apache.flink.agents.runtime.eventlog.FileEventLogger; import org.apache.flink.agents.runtime.eventlog.Slf4jEventLogger; import org.apache.flink.agents.runtime.metrics.BuiltInMetrics; @@ -87,6 +88,10 @@ class EventRouter implements AutoCloseable { private final EventLogger eventLogger; private final List eventListeners; private final AgentPlan agentPlan; + + /** Handles event-to-action matching, including condition expressions. */ + private final ActionMatcher actionMatcher; + private StreamRecord reusedStreamRecord; private SegmentedQueue keySegmentQueue; private BuiltInMetrics builtInMetrics; @@ -101,6 +106,7 @@ class EventRouter implements AutoCloseable { this.inputIsJava = inputIsJava; this.eventLogger = eventLogger; this.eventListeners = new ArrayList<>(); + this.actionMatcher = new ActionMatcher(agentPlan); } /** @@ -216,8 +222,8 @@ OUT getOutputFromOutputEvent(Event event, PythonActionExecutor pythonActionExecu } } - List getActionsTriggeredBy(Event event, AgentPlan agentPlan) { - return agentPlan.getActionsTriggeredBy(event.getType()); + List getActionsTriggeredBy(Event event) { + return actionMatcher.match(event); } /** diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/actionstate/NoOpAction.java b/runtime/src/test/java/org/apache/flink/agents/runtime/actionstate/NoOpAction.java index cfc1f5d20..9cc000a25 100644 --- a/runtime/src/test/java/org/apache/flink/agents/runtime/actionstate/NoOpAction.java +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/actionstate/NoOpAction.java @@ -39,6 +39,6 @@ public NoOpAction(String name) throws Exception { NoOpAction.class.getName(), "doNothing", new Class[] {Event.class, RunnerContext.class}), - List.of(InputEvent.class.getName())); + List.of(InputEvent.EVENT_TYPE)); } } diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ActionMatcherTest.java b/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ActionMatcherTest.java new file mode 100644 index 000000000..083356087 --- /dev/null +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ActionMatcherTest.java @@ -0,0 +1,416 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.runtime.condition; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.flink.agents.api.Event; +import org.apache.flink.agents.api.InputEvent; +import org.apache.flink.agents.api.OutputEvent; +import org.apache.flink.agents.api.configuration.AgentConfigOptions; +import org.apache.flink.agents.api.configuration.AgentConfigOptions.ConditionEvaluationFailureStrategy; +import org.apache.flink.agents.api.context.RunnerContext; +import org.apache.flink.agents.api.resource.ResourceType; +import org.apache.flink.agents.plan.AgentConfiguration; +import org.apache.flink.agents.plan.AgentPlan; +import org.apache.flink.agents.plan.JavaFunction; +import org.apache.flink.agents.plan.actions.Action; +import org.apache.flink.agents.plan.condition.TriggerCondition; +import org.apache.flink.agents.plan.resourceprovider.ResourceProvider; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.AbstractMap; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** Runtime matching contract for unified trigger-condition entries. */ +class ActionMatcherTest { + + private static final String FIXTURE_PATH = + "e2e-test/cross-language-trigger-condition-fixtures/trigger_condition_conformance.json"; + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private static final JsonNode FIXTURE = loadFixture(); + + public static void handler(Event event, RunnerContext context) {} + + @Test + void constructorRejectsNullPlan() { + assertThatThrownBy(() -> new ActionMatcher(null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("agentPlan"); + } + + @Test + void rejectsUnknownConditionSubtype() { + TriggerCondition unknownCondition = mock(TriggerCondition.class); + Action action = mock(Action.class); + when(action.getClassifiedTriggerConditions()).thenReturn(List.of(unknownCondition)); + AgentPlan agentPlan = mock(AgentPlan.class); + when(agentPlan.getActions()).thenReturn(Map.of("unknown", action)); + + assertThatThrownBy(() -> new ActionMatcher(agentPlan)) + .isInstanceOf(IllegalStateException.class) + .hasMessage( + "Unsupported trigger condition type: " + + unknownCondition.getClass().getName()); + } + + static Stream runtimeSuccessCases() { + return Stream.concat( + fixtureCases("expression", "pass"), + fixtureCases("event_type", "not_applicable")) + .map(ActionMatcherTest::probeArguments); + } + + static Stream runtimeFailureCases() { + return fixtureCases("expression", "fail") + .map( + testCase -> + Arguments.of( + testCase.get("name").asText(), + testCase.get("entry").asText(), + testCase.get("runtime_error_fragment").asText())); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("runtimeSuccessCases") + void matchesSharedSuccessCases( + String name, + String source, + String probeEventType, + Map probeAttributes, + boolean matchesProbe) + throws Exception { + Action action = action(name, source); + List matches = + new ActionMatcher(plan(action)).match(new Event(probeEventType, probeAttributes)); + + if (matchesProbe) { + assertThat(matches).containsExactly(action); + } else { + assertThat(matches).isEmpty(); + } + } + + @ParameterizedTest(name = "{0}") + @MethodSource("runtimeFailureCases") + void rejectsSharedRuntimeFailures(String name, String source, String errorFragment) + throws Exception { + Action action = action(name, source); + + assertThatThrownBy(() -> new ActionMatcher(plan(action))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining(errorFragment); + } + + @Test + void typeAndConditionEntriesUseOr() throws Exception { + Action mixed = action("mixed", InputEvent.EVENT_TYPE, "score > 10"); + ActionMatcher matcher = new ActionMatcher(plan(mixed)); + + assertThat(matcher.match(new InputEvent("without-score"))).containsExactly(mixed); + assertThat(matcher.match(new Event("other", Map.of("score", 20)))).containsExactly(mixed); + assertThat(matcher.match(new Event("other", Map.of("score", 5)))).isEmpty(); + } + + @Test + void typeMatchShortCircuitsFailingCondition() throws Exception { + Action mixed = action("mixed", InputEvent.EVENT_TYPE, "attributes.missing > 0"); + ActionMatcher matcher = + new ActionMatcher(plan(ConditionEvaluationFailureStrategy.FAIL, mixed)); + + assertThat(matcher.match(new InputEvent("x"))).containsExactly(mixed); + } + + @Test + void matchingConditionShortCircuitsLaterConditions() throws Exception { + Action action = action("conditional", "true", "attributes.missing > 0"); + ActionMatcher matcher = + new ActionMatcher(plan(ConditionEvaluationFailureStrategy.FAIL, action)); + + assertThat(matcher.match(new Event("other"))).containsExactly(action); + } + + @Test + void shortCircuitSkipsLaterVariableBuild() throws Exception { + Event event = eventWithFailingAttribute(); + Action warnAction = action("warn", "true", "bad != null"); + Action failAction = action("fail", "true", "bad != null"); + + assertThat( + new ActionMatcher( + plan( + ConditionEvaluationFailureStrategy.WARN_AND_SKIP, + warnAction)) + .match(event)) + .containsExactly(warnAction); + assertThat( + new ActionMatcher(plan(ConditionEvaluationFailureStrategy.FAIL, failAction)) + .match(event)) + .containsExactly(failAction); + } + + @Test + void warnAndSkipContinuesAfterVariableBuildFailure() throws Exception { + Action fallback = action("fallback", "bad != null", "true"); + Action independent = action("independent", "type == 'other'"); + ActionMatcher matcher = + new ActionMatcher( + plan( + ConditionEvaluationFailureStrategy.WARN_AND_SKIP, + fallback, + independent)); + + assertThat(matcher.match(eventWithFailingAttribute())) + .containsExactly(fallback, independent); + } + + @Test + void failPropagatesVariableBuildFailure() throws Exception { + Action broken = action("broken", "bad != null", "true"); + ActionMatcher matcher = + new ActionMatcher(plan(ConditionEvaluationFailureStrategy.FAIL, broken)); + + assertThatThrownBy(() -> matcher.match(eventWithFailingAttribute())) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Building trigger condition variables failed"); + } + + @Test + void laterConditionCanMatch() throws Exception { + Action action = action("conditional", "has(missing) && missing > 0", "score == 7"); + ActionMatcher matcher = new ActionMatcher(plan(action)); + + assertThat(matcher.match(new Event("other", Map.of("score", 7)))).containsExactly(action); + } + + @Test + void deduplicatesMatchesInTypeFirstOrder() throws Exception { + JsonNode scenario = FIXTURE.get("ordering_dedupe_type_first"); + Map actions = new LinkedHashMap<>(); + for (JsonNode actionNode : scenario.withArray("actions")) { + Action action = + action( + actionNode.get("name").asText(), + jsonStringList(actionNode.get("entries"))); + actions.put(action.getName(), action); + } + + assertThat( + new ActionMatcher(new AgentPlan(actions)) + .match(new Event(scenario.get("probe_event_type").asText()))) + .extracting(Action::getName) + .containsExactlyElementsOf(jsonStringList(scenario.get("expected_matches"))); + } + + @Test + void warnAndSkipKeepsTypeHits() throws Exception { + Action type = action("type", InputEvent.EVENT_TYPE); + Action broken = action("broken", "attributes.missing > 0"); + ActionMatcher matcher = new ActionMatcher(plan(type, broken)); + + assertThat(matcher.match(new InputEvent("x"))).containsExactly(type); + } + + @Test + void failPropagatesEvaluationError() throws Exception { + Action broken = action("broken", "attributes.missing > 0"); + ActionMatcher matcher = + new ActionMatcher(plan(ConditionEvaluationFailureStrategy.FAIL, broken)); + + assertThatThrownBy(() -> matcher.match(new Event("other"))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Trigger condition evaluation failed"); + } + + @Test + void matchesOutputEventDirectly() throws Exception { + Action action = action("output", OutputEvent.EVENT_TYPE); + + assertThat(new ActionMatcher(plan(action)).match(new OutputEvent("x"))) + .containsExactly(action); + } + + @Test + void restoredPlanRebuildsConditions() throws Exception { + Action mixed = action("mixed", "'custom.type'", "score == 7"); + AgentPlan restored = javaRoundTrip(plan(mixed)); + Action restoredAction = restored.getActions().get("mixed"); + ActionMatcher matcher = new ActionMatcher(restored); + + assertThat(matcher.match(new Event("custom.type"))).containsExactly(restoredAction); + assertThat(matcher.match(new Event("other", Map.of("score", 7)))) + .containsExactly(restoredAction); + } + + @Test + void pojoAndJsonUseSameCondition() throws Exception { + Action nested = action("nested", "input.status == 'ok'"); + Action bare = action("bare", "has(status) && status == 'ok'"); + ActionMatcher matcher = new ActionMatcher(plan(nested, bare)); + InputEvent typed = new InputEvent(new ConditionInput("id", "ok", 9)); + Event jsonShaped = Event.fromJson(OBJECT_MAPPER.writeValueAsString(typed)); + + assertThat(matcher.match(typed)).containsExactly(nested); + assertThat(matcher.match(jsonShaped)).containsExactly(nested); + } + + private static Action action(String name, String... triggerConditions) throws Exception { + return new Action(name, function(), Arrays.asList(triggerConditions), null); + } + + private static Action action(String name, List triggerConditions) throws Exception { + return new Action(name, function(), triggerConditions, null); + } + + private static Event eventWithFailingAttribute() { + return new Event("other", Map.of("bad", new FailingMap())); + } + + /** Throws only when condition variable building tries to normalize the value. */ + private static final class FailingMap extends AbstractMap { + @Override + public Set> entrySet() { + throw new IllegalStateException("bad attribute must not be normalized"); + } + } + + public static final class ConditionInput { + public String id; + public String status; + public int value; + + public ConditionInput() {} + + public ConditionInput(String id, String status, int value) { + this.id = id; + this.status = status; + this.value = value; + } + } + + private static JavaFunction function() throws Exception { + return new JavaFunction( + ActionMatcherTest.class.getName(), + "handler", + new Class[] {Event.class, RunnerContext.class}); + } + + private static AgentPlan plan(Action... actions) { + Map byName = new LinkedHashMap<>(); + for (Action action : actions) { + byName.put(action.getName(), action); + } + return new AgentPlan(byName); + } + + private static AgentPlan plan( + ConditionEvaluationFailureStrategy failureStrategy, Action... actions) { + Map byName = new LinkedHashMap<>(); + for (Action action : actions) { + byName.put(action.getName(), action); + } + AgentConfiguration config = new AgentConfiguration(); + config.set(AgentConfigOptions.CONDITION_EVALUATION_FAILURE_STRATEGY, failureStrategy); + return new AgentPlan(byName, Map.>of(), config); + } + + private static AgentPlan javaRoundTrip(AgentPlan plan) throws Exception { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ObjectOutputStream output = new ObjectOutputStream(bytes)) { + output.writeObject(plan); + } + try (ObjectInputStream input = + new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray()))) { + return (AgentPlan) input.readObject(); + } + } + + private static Stream fixtureCases(String classification, String runtimeCompilation) { + List matchingCases = new ArrayList<>(); + for (JsonNode testCase : FIXTURE.withArray("cases")) { + if ("pass".equals(testCase.get("plan_validation").asText()) + && classification.equals(testCase.get("classification").asText()) + && runtimeCompilation.equals(testCase.get("runtime_compilation").asText())) { + matchingCases.add(testCase); + } + } + return matchingCases.stream(); + } + + private static Arguments probeArguments(JsonNode testCase) { + return Arguments.of( + testCase.get("name").asText(), + testCase.get("entry").asText(), + testCase.get("probe_event_type").asText(), + jsonMap(testCase.get("probe_attributes")), + testCase.get("matches_probe").asBoolean()); + } + + private static Map jsonMap(JsonNode node) { + return OBJECT_MAPPER.convertValue(node, new TypeReference>() {}); + } + + private static List jsonStringList(JsonNode arrayNode) { + List values = new ArrayList<>(); + for (JsonNode value : arrayNode) { + values.add(value.asText()); + } + return values; + } + + private static JsonNode loadFixture() { + Path directory = Paths.get("").toAbsolutePath(); + while (directory != null) { + Path candidate = directory.resolve(FIXTURE_PATH); + if (Files.isRegularFile(candidate)) { + try { + return OBJECT_MAPPER.readTree(candidate.toFile()); + } catch (IOException e) { + throw new IllegalStateException( + "Failed to read trigger-condition fixture " + candidate, e); + } + } + directory = directory.getParent(); + } + throw new IllegalStateException("Could not find trigger-condition fixture " + FIXTURE_PATH); + } +} diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ConditionEvaluatorTest.java b/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ConditionEvaluatorTest.java new file mode 100644 index 000000000..a89cbe22e --- /dev/null +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ConditionEvaluatorTest.java @@ -0,0 +1,563 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.runtime.condition; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; +import dev.cel.common.values.NullValue; +import org.apache.flink.agents.api.Event; +import org.apache.flink.agents.api.InputEvent; +import org.apache.flink.agents.api.OutputEvent; +import org.apache.flink.agents.api.chat.messages.ChatMessage; +import org.apache.flink.agents.api.configuration.AgentConfigOptions.ConditionEvaluationFailureStrategy; +import org.apache.flink.agents.api.event.ChatResponseEvent; +import org.apache.flink.agents.plan.condition.TriggerCondition; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** Unit tests for {@link ConditionEvaluator}. */ +class ConditionEvaluatorTest { + + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(new YAMLFactory()); + private static final ObjectMapper JSON_MAPPER = new ObjectMapper(); + + private EvaluatorHarness evaluator; + + /** Test case from conformance JSON. */ + static class ConformanceTestCase { + String name; + String condition; + Map event; + boolean expected; + + public String getName() { + return name; + } + + public String getCondition() { + return condition; + } + + public Map getEvent() { + return event; + } + + public boolean isExpected() { + return expected; + } + } + + /** Keeps the existing source-oriented tests focused while production evaluation stays pure. */ + private static final class EvaluatorHarness { + private final ConditionEvaluator delegate; + private final Map conditions; + + private EvaluatorHarness(Collection sources) { + this(sources, ConditionEvaluationFailureStrategy.WARN_AND_SKIP); + } + + private EvaluatorHarness( + Collection sources, ConditionEvaluationFailureStrategy failureStrategy) { + this.delegate = new ConditionEvaluator(failureStrategy); + this.conditions = new HashMap<>(); + for (String source : sources) { + conditions.computeIfAbsent( + source, + ignored -> + ConditionExpressionCompiler.compile(conditionExpression(source))); + } + } + + private Map buildConditionVariables(Event event, String source) { + return delegate.buildConditionVariables(event, compiled(source)); + } + + private boolean evaluate(String source, Event event) { + return delegate.evaluate(compiled(source), event); + } + + private ConditionExpressionCompiler.CompiledCondition compiled(String source) { + ConditionExpressionCompiler.CompiledCondition condition = conditions.get(source); + if (condition == null) { + throw new IllegalArgumentException("Condition was not compiled: " + source); + } + return condition; + } + } + + private static ExpressionCondition conditionExpression(String source) { + TriggerCondition triggerCondition = TriggerCondition.classify(source); + assertThat(triggerCondition).isInstanceOf(ExpressionCondition.class); + return (ExpressionCondition) triggerCondition; + } + + @BeforeEach + void setUp() throws IOException { + // Compile every condition from the conformance JSON. + List testCases = loadConformanceCases(); + List conditionSources = new ArrayList<>(); + for (ConformanceTestCase tc : testCases) { + if (tc.getCondition() != null && !tc.getCondition().isEmpty()) { + conditionSources.add(tc.getCondition()); + } + } + evaluator = new EvaluatorHarness(conditionSources); + } + + private static List loadConformanceCases() throws IOException { + try (InputStream is = + ConditionEvaluatorTest.class.getResourceAsStream("/cel_conformance_cases.yaml")) { + if (is == null) { + throw new IOException("cel_conformance_cases.yaml not found"); + } + return OBJECT_MAPPER.readValue(is, new TypeReference>() {}); + } + } + + private Event buildEvent(Map eventData) { + String id = (String) eventData.get("id"); + String type = (String) eventData.get("type"); + @SuppressWarnings("unchecked") + Map attributes = + (Map) eventData.getOrDefault("attributes", new HashMap<>()); + UUID uuid; + try { + // Reuse the raw id when it's a valid UUID so id-based filters work in conformance + // cases. + uuid = UUID.fromString(id); + } catch (IllegalArgumentException e) { + uuid = UUID.nameUUIDFromBytes(id.getBytes()); + } + Event event = new Event(uuid, type, attributes); + return event; + } + + static Stream conformanceCases() throws IOException { + return loadConformanceCases().stream(); + } + + @Test + void typedEventAttributesMatchJsonRoundTrip() throws Exception { + UUID requestId = UUID.fromString("550e8400-e29b-41d4-a716-446655440000"); + ChatResponseEvent typed = new ChatResponseEvent(requestId, ChatMessage.assistant("hello")); + Event jsonShaped = Event.fromJson(JSON_MAPPER.writeValueAsString(typed)); + List sources = + List.of( + "request_id == '550e8400-e29b-41d4-a716-446655440000'", + "response.content == 'hello'", + "response.role == 'assistant'"); + EvaluatorHarness testEvaluator = + new EvaluatorHarness(sources, ConditionEvaluationFailureStrategy.FAIL); + + for (String source : sources) { + Map typedVariables = + testEvaluator.buildConditionVariables(typed, source); + Map jsonVariables = + testEvaluator.buildConditionVariables(jsonShaped, source); + + assertThat(typedVariables).isEqualTo(jsonVariables); + assertThat(testEvaluator.evaluate(source, typed)).as(source).isTrue(); + assertThat(testEvaluator.evaluate(source, jsonShaped)).as(source).isTrue(); + } + } + + @Test + void missingEventIdDoesNotAbortVariableBuilding() throws Exception { + String source = "score > 80"; + Event event = Event.fromJson("{\"type\":\"test\",\"attributes\":{\"score\":90}}"); + EvaluatorHarness testEvaluator = + new EvaluatorHarness(List.of(source), ConditionEvaluationFailureStrategy.FAIL); + + Map conditionVariables = + testEvaluator.buildConditionVariables(event, source); + + assertThat(conditionVariables).isNotNull().doesNotContainKey("id"); + assertThat(testEvaluator.evaluate(source, event)).isTrue(); + } + + @Test + void eventIdWinsWhileAttributeIdRemainsExplicitlyAccessible() { + UUID eventId = UUID.fromString("550e8400-e29b-41d4-a716-446655440000"); + List sources = + List.of( + "id == '550e8400-e29b-41d4-a716-446655440000'", + "attributes.id == 'tenant-42'"); + Event event = new Event(eventId, "test", Map.of("id", "tenant-42")); + EvaluatorHarness testEvaluator = + new EvaluatorHarness(sources, ConditionEvaluationFailureStrategy.FAIL); + + Map conditionVariables = + testEvaluator.buildConditionVariables(event, "attributes.id == 'tenant-42'"); + + assertThat(conditionVariables).containsEntry("id", eventId.toString()); + assertThat(conditionVariables.get("attributes")).isEqualTo(Map.of("id", "tenant-42")); + assertThat(testEvaluator.evaluate(sources.get(0), event)).isTrue(); + assertThat(testEvaluator.evaluate(sources.get(1), event)).isTrue(); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("conformanceCases") + void conformanceCaseMatchesExpected(ConformanceTestCase testCase) { + Event event = buildEvent(testCase.getEvent()); + boolean result = evaluator.evaluate(testCase.getCondition(), event); + assertThat(result).isEqualTo(testCase.isExpected()); + } + + @Test + void failStrategyThrowsOnEvaluationError() { + EvaluatorHarness failEvaluator = + new EvaluatorHarness( + List.of("attributes.nonexistent > 3"), + ConditionEvaluationFailureStrategy.FAIL); + // Pre-compile a condition that will fail at runtime + String source = "attributes.nonexistent > 3"; + + Event event = new Event("test_type"); + + assertThatThrownBy(() -> failEvaluator.evaluate(source, event)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Trigger condition evaluation failed"); + } + + @Test + void warnAndSkipHandlesVariableBuildFailure() { + String source = "broken == null"; + EvaluatorHarness warnEvaluator = new EvaluatorHarness(List.of(source)); + Event event = new Event("test_type", Map.of("broken", new BrokenAttribute())); + + assertThat(warnEvaluator.evaluate(source, event)).isFalse(); + } + + @Test + void failStrategyThrowsOnVariableBuildFailure() { + String source = "broken == null"; + EvaluatorHarness failEvaluator = + new EvaluatorHarness(List.of(source), ConditionEvaluationFailureStrategy.FAIL); + Event event = new Event("test_type", Map.of("broken", new BrokenAttribute())); + + assertThatThrownBy(() -> failEvaluator.evaluate(source, event)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Building trigger condition variables failed"); + } + + @Test + void nonBooleanWarnStrategyReturnsFalse() { + // A dynamic result can still be non-Boolean when Plan validation was bypassed. + String source = "attributes['value']"; + EvaluatorHarness warnEvaluator = + new EvaluatorHarness( + List.of(source), ConditionEvaluationFailureStrategy.WARN_AND_SKIP); + + Event event = new Event("test_type", Map.of("value", 7)); + assertThat(warnEvaluator.evaluate(source, event)).isFalse(); + } + + @Test + void nonBooleanFailStrategyThrows() { + String source = "attributes['value']"; + EvaluatorHarness failEvaluator = + new EvaluatorHarness(List.of(source), ConditionEvaluationFailureStrategy.FAIL); + + Event event = new Event("test_type", Map.of("value", 7)); + assertThatThrownBy(() -> failEvaluator.evaluate(source, event)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("non-boolean"); + } + + @Test + void normalizeShortToLong() { + String source = "attributes.count + 1 == 8"; + EvaluatorHarness testEvaluator = + new EvaluatorHarness(List.of(source), ConditionEvaluationFailureStrategy.FAIL); + Event event = new Event("test_type"); + event.getAttributes().put("count", Short.valueOf((short) 7)); + + Map conditionVariables = + testEvaluator.buildConditionVariables(event, source); + + assertThat(conditionVariables.get("count")).isEqualTo(7L); + assertThat(((Map) conditionVariables.get("attributes")).get("count")).isEqualTo(7L); + assertThat(testEvaluator.evaluate(source, event)).isTrue(); + } + + @Test + void normalizeFloatToDouble() { + String source = "attributes.score + 0.5 == 5.0"; + EvaluatorHarness testEvaluator = + new EvaluatorHarness(List.of(source), ConditionEvaluationFailureStrategy.FAIL); + Event event = new Event("test_type"); + event.getAttributes().put("score", Float.valueOf(4.5F)); + + Map conditionVariables = + testEvaluator.buildConditionVariables(event, source); + + assertThat(conditionVariables.get("score")).isInstanceOf(Double.class).isEqualTo(4.5D); + assertThat(((Map) conditionVariables.get("attributes")).get("score")) + .isInstanceOf(Double.class) + .isEqualTo(4.5D); + assertThat(testEvaluator.evaluate(source, event)).isTrue(); + } + + @Test + void normalizeBigIntegerInLongRange() { + // BigInteger within Long range should pass through normalizeValue cleanly. + String source = "attributes.amount > 1000"; + EvaluatorHarness testEvaluator = new EvaluatorHarness(List.of(source)); + + Event event = new Event("test_type"); + event.getAttributes().put("amount", java.math.BigInteger.valueOf(9999999999999L)); + + assertThat(testEvaluator.evaluate(source, event)).isTrue(); + } + + @Test + void normalizeBigIntegerOverflowConvertsToDouble() { + // A BigInteger past int64 widens to double so building the variables succeeds and the + // event's other conditions still evaluate. + String source = "amount > 1000"; + EvaluatorHarness testEvaluator = new EvaluatorHarness(List.of(source)); + Event event = new Event("test_type"); + event.getAttributes().put("amount", new java.math.BigInteger("99999999999999999999")); + + Map conditionVariables = + testEvaluator.buildConditionVariables(event, source); + + assertThat(conditionVariables.get("amount")).isInstanceOf(Double.class); + } + + @Test + void normalizeBigDecimalToDouble() { + String source = "attributes.score > 3.14"; + EvaluatorHarness testEvaluator = new EvaluatorHarness(List.of(source)); + + Event event = new Event("test_type"); + event.getAttributes().put("score", new java.math.BigDecimal("99.99")); + + assertThat(testEvaluator.evaluate(source, event)).isTrue(); + } + + // ----- Nested has() short-circuit (has(a.b.c) desugaring) ----- + // These use the FAIL strategy on purpose: under the default WARN_AND_SKIP a thrown + // CelEvaluationException is swallowed and also reported as false, so it cannot tell a + // genuine false apart from a silently-skipped error. FAIL re-throws, so asserting false + // here proves the macro short-circuits a missing level instead of evaluating an operand + // that errors with "key '...' is not present in map". + + @Test + void nestedHasMissingMiddleReturnsFalse() { + EvaluatorHarness failEvaluator = + new EvaluatorHarness( + List.of("has(attributes.meta.source)"), + ConditionEvaluationFailureStrategy.FAIL); + String source = "has(attributes.meta.source)"; + + // No attributes ⇒ intermediate 'meta' is absent; the has() chain desugaring + // short-circuits to false instead of throwing on the deeper select. + Event event = new Event("test_type"); + assertThat(failEvaluator.evaluate(source, event)).isFalse(); + } + + @Test + void nestedHasMissingRootReturnsFalse() { + EvaluatorHarness failEvaluator = + new EvaluatorHarness( + List.of("has(value.user.tier)"), ConditionEvaluationFailureStrategy.FAIL); + String source = "has(value.user.tier)"; + + // Bare root 'value' is wholly absent: the has(value) guard short-circuits before the + // unbound 'value' lookup can throw. + Event event = new Event("test_type"); + assertThat(failEvaluator.evaluate(source, event)).isFalse(); + } + + @Test + void nestedHasFullPathReturnsTrue() { + EvaluatorHarness failEvaluator = + new EvaluatorHarness( + List.of("has(attributes.meta.source)"), + ConditionEvaluationFailureStrategy.FAIL); + String source = "has(attributes.meta.source)"; + + Event event = new Event("test_type"); + Map meta = new HashMap<>(); + meta.put("source", "api"); + event.getAttributes().put("meta", meta); + assertThat(failEvaluator.evaluate(source, event)).isTrue(); + } + + // ---- Trimmed condition variables preserve the Event.attributes envelope ---- + + @Test + void payloadChildrenNeedExplicitPaths() { + List sources = List.of("input.region == 1", "output.region == 2", "has(region)"); + EvaluatorHarness testEvaluator = + new EvaluatorHarness(sources, ConditionEvaluationFailureStrategy.FAIL); + Event inputEvent = new Event(InputEvent.EVENT_TYPE, Map.of("input", Map.of("region", 1))); + Event outputEvent = + new Event(OutputEvent.EVENT_TYPE, Map.of("output", Map.of("region", 2))); + + assertThat(testEvaluator.evaluate("input.region == 1", inputEvent)).isTrue(); + assertThat(testEvaluator.evaluate("output.region == 2", outputEvent)).isTrue(); + assertThat(testEvaluator.evaluate("has(region)", inputEvent)).isFalse(); + assertThat(testEvaluator.evaluate("has(region)", outputEvent)).isFalse(); + } + + @Test + void topLevelNullEvaluatesAsCelNull() { + List sources = + List.of("has(attributes.score)", "attributes.score == null", "score == null"); + EvaluatorHarness testEvaluator = + new EvaluatorHarness(sources, ConditionEvaluationFailureStrategy.FAIL); + Event event = new Event("custom"); + event.getAttributes().put("score", null); + + for (String source : sources) { + Map conditionVariables = + testEvaluator.buildConditionVariables(event, source); + + assertThat(conditionVariables.get("score")).isSameAs(NullValue.NULL_VALUE); + assertThat(((Map) conditionVariables.get("attributes")).get("score")) + .isSameAs(NullValue.NULL_VALUE); + assertThat(testEvaluator.evaluate(source, event)).as(source).isTrue(); + } + } + + @Test + void missingKeyMakesHasReturnFalse() { + String source = "has(score)"; + EvaluatorHarness testEvaluator = + new EvaluatorHarness(List.of(source), ConditionEvaluationFailureStrategy.FAIL); + Event event = new Event(OutputEvent.EVENT_TYPE, Map.of("output", Map.of("score", 99))); + + Map conditionVariables = + testEvaluator.buildConditionVariables(event, source); + + assertThat(conditionVariables).doesNotContainKey("score"); + assertThat(((Map) conditionVariables.get("attributes")).containsKey("score")) + .isFalse(); + assertThat(testEvaluator.evaluate(source, event)).isFalse(); + } + + @Test + void evaluatesDottedLiteralKey() { + List sources = List.of("attributes['a.b.c'] == 7", "'a.b.c' in attributes"); + EvaluatorHarness testEvaluator = + new EvaluatorHarness(sources, ConditionEvaluationFailureStrategy.FAIL); + Event event = new Event("test_type", Map.of("a.b.c", 7)); + + for (String source : sources) { + Map conditionVariables = + testEvaluator.buildConditionVariables(event, source); + + assertThat(conditionVariables).isNotNull(); + assertThat(conditionVariables.get("attributes")).isEqualTo(Map.of("a.b.c", 7L)); + assertThat(testEvaluator.evaluate(source, event)).as(source).isTrue(); + } + } + + @Test + void pojoUsesExplicitAttributePath() { + List sources = + List.of("input.status == 'ok'", "attributes.input.status == 'ok'", "has(status)"); + EvaluatorHarness testEvaluator = + new EvaluatorHarness(sources, ConditionEvaluationFailureStrategy.FAIL); + Event event = new InputEvent(new ConditionInput("ok", 42)); + + Map conditionVariables = + testEvaluator.buildConditionVariables(event, "input.status == 'ok'"); + + assertThat(conditionVariables).containsKey("input").doesNotContainKey("status"); + @SuppressWarnings("unchecked") + Map attributes = (Map) conditionVariables.get("attributes"); + assertThat(attributes).containsOnlyKeys("input").doesNotContainKey("status"); + assertThat(testEvaluator.evaluate("input.status == 'ok'", event)).isTrue(); + assertThat(testEvaluator.evaluate("attributes.input.status == 'ok'", event)).isTrue(); + assertThat(testEvaluator.evaluate("has(status)", event)).isFalse(); + } + + @Test + void scalarAndListAccessibleViaInput() { + List sources = List.of("input == 'ready'", "input[0] == 'ready'"); + EvaluatorHarness testEvaluator = + new EvaluatorHarness(sources, ConditionEvaluationFailureStrategy.FAIL); + + assertThat(testEvaluator.evaluate("input == 'ready'", new InputEvent("ready"))).isTrue(); + assertThat(testEvaluator.evaluate("input[0] == 'ready'", new InputEvent(List.of("ready")))) + .isTrue(); + } + + @Test + void typeOnlySkipsUnneededPayload() { + String source = "type == EventType.InputEvent"; + EvaluatorHarness testEvaluator = + new EvaluatorHarness(List.of(source), ConditionEvaluationFailureStrategy.FAIL); + + assertThat(testEvaluator.evaluate(source, new InputEvent(new BrokenAttribute()))).isTrue(); + } + + @Test + void trimsUnreferencedLargeAttribute() { + String source = "score > 1"; + EvaluatorHarness testEvaluator = new EvaluatorHarness(List.of(source)); + Event event = new Event("t"); + event.getAttributes().put("score", 5); + event.getAttributes().put("big_blob", "{\"a\":1,\"b\":2}"); + + Map conditionVariables = + testEvaluator.buildConditionVariables(event, source); + assertThat(conditionVariables).containsKey("score"); + assertThat(conditionVariables).doesNotContainKey("big_blob"); // never normalized + assertThat(testEvaluator.evaluate(source, event)).isTrue(); + } + + private static final class BrokenAttribute { + public String getValue() { + throw new IllegalStateException("broken attribute"); + } + } + + public static final class ConditionInput { + public String status; + public int value; + + public ConditionInput() {} + + public ConditionInput(String status, int value) { + this.status = status; + this.value = value; + } + } +} diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ConditionExpressionCompilerTest.java b/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ConditionExpressionCompilerTest.java new file mode 100644 index 000000000..49fc4f8eb --- /dev/null +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ConditionExpressionCompilerTest.java @@ -0,0 +1,186 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.runtime.condition; + +import dev.cel.runtime.CelEvaluationException; +import dev.cel.runtime.CelRuntime; +import org.apache.flink.agents.api.EventType; +import org.apache.flink.agents.plan.condition.ConditionExpressionValidator; +import org.apache.flink.agents.plan.condition.TriggerCondition; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** Runtime-only compilation tests for Plan-classified expression conditions. */ +class ConditionExpressionCompilerTest { + + private static ConditionExpressionCompiler.CompiledCondition compileValidated(String source) { + ExpressionCondition condition = conditionExpression(source); + ConditionExpressionValidator.validate(condition); + return ConditionExpressionCompiler.compile(condition); + } + + private static ConditionExpressionCompiler.CompiledCondition compileRuntimeOnly(String source) { + return ConditionExpressionCompiler.compile(conditionExpression(source)); + } + + private static ExpressionCondition conditionExpression(String source) { + TriggerCondition triggerCondition = TriggerCondition.classify(source); + assertThat(triggerCondition).isInstanceOf(ExpressionCondition.class); + return (ExpressionCondition) triggerCondition; + } + + @Test + void createsRunnableBooleanProgram() throws CelEvaluationException { + CelRuntime.Program program = compileValidated("type == 'a'").program(); + Map activation = new HashMap<>(); + activation.put("type", "a"); + activation.put("attributes", new HashMap()); + + assertThat(program.eval(activation)).isEqualTo(true); + } + + @Test + void resolvesKnownEventTypeConstants() throws CelEvaluationException { + CelRuntime.Program program = compileValidated("type == EventType.InputEvent").program(); + Map activation = new HashMap<>(); + activation.put("type", "_input_event"); + activation.put("attributes", new HashMap()); + activation.put("EventType", EventType.allConstants()); + + assertThat(program.eval(activation)).isEqualTo(true); + } + + @Test + void supportsDynamicAttributeTypes() throws CelEvaluationException { + CelRuntime.Program program = compileValidated("score > 0").program(); + + assertThat(program.eval(Map.of("score", 42L))).isEqualTo(true); + } + + @Test + void rejectsUnknownFunctionAtRuntime() { + ConditionExpressionValidator.validate(conditionExpression("noSuchFn(1)")); + + assertThatThrownBy(() -> compileRuntimeOnly("noSuchFn(1)")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Runtime type-check"); + } + + @ParameterizedTest(name = "Runtime rejects statically non-Boolean result: {0}") + @ValueSource( + strings = { + "null", + "42", + "'not a routable name'", + "[1, 2]", + "{'key': 1}", + "1 + 2", + "size(attributes)" + }) + void rejectsStaticNonBooleanResults(String source) { + assertThatThrownBy(() -> compileValidated(source)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Boolean result"); + } + + @Test + void runtimeDefensivelyRejectsStandaloneEventTypeConstant() { + assertThatThrownBy(() -> compileRuntimeOnly("EventType.InputEvent")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Boolean result"); + } + + @Test + void reportsRuntimeReparseInvariantFailure() { + assertThatThrownBy(() -> compileRuntimeOnly("type ==")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Plan-validated trigger condition") + .hasMessageContaining("reparsed at Runtime"); + } + + @Test + void supportsHasDuringRuntimeReparse() throws CelEvaluationException { + CelRuntime.Program program = compileValidated("has(score)").program(); + + assertThat(program.eval(Map.of("attributes", Map.of("score", 1L)))).isEqualTo(true); + assertThat(program.eval(Map.of("attributes", Map.of()))).isEqualTo(false); + } + + static Stream stringFunctionCases() { + return Stream.of( + Arguments.of("name.contains(\"foo\")", "hello_foo_bar", true), + Arguments.of("name.contains(\"xyz\")", "hello_foo_bar", false), + Arguments.of("name.startsWith(\"pre_\")", "pre_order_123", true), + Arguments.of("name.startsWith(\"pre_\")", "order_pre_123", false), + Arguments.of("name.endsWith(\".json\")", "config.json", true), + Arguments.of("name.endsWith(\".json\")", "config.yaml", false), + Arguments.of("name.matches(\"^order_[0-9]+$\")", "order_42", true), + Arguments.of("name.matches(\"^order_[0-9]+$\")", "order_abc", false), + Arguments.of("name.matches(\"[\")", "test", CelEvaluationException.class)); + } + + @ParameterizedTest(name = "{0} with name={1} -> {2}") + @MethodSource("stringFunctionCases") + void stringFunctionsEvaluateOrThrow(String source, String value, Object expected) + throws CelEvaluationException { + CelRuntime.Program program = compileValidated(source).program(); + Map activation = Map.of("attributes", Map.of("name", value), "name", value); + + if (expected instanceof Class) { + @SuppressWarnings("unchecked") + Class exceptionClass = (Class) expected; + assertThatThrownBy(() -> program.eval(activation)).isInstanceOf(exceptionClass); + } else { + assertThat(program.eval(activation)).isEqualTo(expected); + } + } + + @Nested + class SharedParserResourceGuards { + @Test + void rejectsOversizedRuntimeSource() { + String oversized = "true" + " || true".repeat(2_000); + + assertThatThrownBy(() -> compileRuntimeOnly(oversized)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("reparsed at Runtime"); + } + + @Test + void rejectsDeepRuntimeSource() { + String tooDeep = "(".repeat(40) + "true" + ")".repeat(40); + + assertThatThrownBy(() -> compileRuntimeOnly(tooDeep)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("reparsed at Runtime"); + } + } +} diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ReferencedAttributeKeyExtractionTest.java b/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ReferencedAttributeKeyExtractionTest.java new file mode 100644 index 000000000..a02886adc --- /dev/null +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/condition/ReferencedAttributeKeyExtractionTest.java @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.agents.runtime.condition; + +import org.apache.flink.agents.plan.condition.TriggerCondition; +import org.apache.flink.agents.plan.condition.TriggerCondition.ExpressionCondition; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.Set; +import java.util.stream.Stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** Unit tests for referenced-attribute extraction performed during condition compilation. */ +class ReferencedAttributeKeyExtractionTest { + + private static Set referencedAttributeKeys(String source) { + TriggerCondition triggerCondition = TriggerCondition.classify(source); + assertThat(triggerCondition).isInstanceOf(ExpressionCondition.class); + return ConditionExpressionCompiler.compile((ExpressionCondition) triggerCondition) + .referencedTopLevelAttributeKeys(); + } + + static Stream staticKeyCases() { + return Stream.of( + Arguments.of("has(score)", "score"), + Arguments.of("attributes.score > 50", "score"), + Arguments.of("attributes.output.region > 1", "output"), + Arguments.of("region > 10", "region"), + Arguments.of("size(score_name) > 1", "score_name"), + Arguments.of("'k' in score_name", "score_name"), + Arguments.of("score_name == {}", "score_name"), + Arguments.of("score_name['batch-3.12'].region > 10", "score_name"), + Arguments.of("has(a.b)", "a"), + Arguments.of("attributes.id == 'x'", "id"), + Arguments.of("attributes['score'] > 1", "score"), + Arguments.of("attributes['a.b.c'] == 7", "a.b.c"), + Arguments.of("'a.b.c' in attributes", "a.b.c")); + } + + @ParameterizedTest(name = "{0} -> {1}") + @MethodSource("staticKeyCases") + void extractsStaticKey(String source, String expectedKey) { + assertThat(referencedAttributeKeys(source)).containsExactly(expectedKey); + } + + @Test + void frameworkIdIsNotAnAttributeKey() { + assertThat(referencedAttributeKeys("id == 'x'")).isEmpty(); + } + + @Test + void dynamicTopLevelIndexRejected() { + assertThatThrownBy(() -> referencedAttributeKeys("attributes[region_id] > 0")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("whole 'attributes' map"); + } + + @Test + void wholeAttributesUseRejected() { + assertThatThrownBy(() -> referencedAttributeKeys("attributes == {}")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("whole 'attributes' map"); + assertThatThrownBy(() -> referencedAttributeKeys("size(attributes) > 0")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("whole 'attributes' map"); + } + + @Test + void literalKeyNamedAttributesKept() { + // A user attribute that happens to be named "attributes" is a normal key, not the + // whole-map sentinel, and must not be rejected. + assertThat(referencedAttributeKeys("attributes['attributes'] > 1")) + .containsExactly("attributes"); + assertThat(referencedAttributeKeys("attributes.attributes > 1")) + .containsExactly("attributes"); + } + + @Test + void repeatedStaticKeyIsExtractedOnce() { + assertThat(referencedAttributeKeys("attributes['a.b.c'] == 7 && 'a.b.c' in attributes")) + .containsExactly("a.b.c"); + } + + @Test + void dynamicMembershipInAttributesRejected() { + assertThatThrownBy(() -> referencedAttributeKeys("key_name in attributes")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("whole 'attributes' map"); + } +} diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperatorTest.java b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperatorTest.java index 22c0072f8..7f71842b6 100644 --- a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperatorTest.java +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperatorTest.java @@ -270,6 +270,53 @@ void testRestoreOnlyResumesKeysOwnedByCurrentSubtask() throws Exception { } } + @Test + void pendingActionConfigSurvivesRestore() throws Exception { + final long key = 7L; + Action configuredAction = + new Action( + "configuredAction", + new JavaFunction( + TestAgent.class, + "action1", + new Class[] {Event.class, RunnerContext.class}), + Collections.singletonList(InputEvent.EVENT_TYPE), + Collections.singletonMap("mode", "strict")); + AgentPlan agentPlan = + new AgentPlan( + Collections.singletonMap(configuredAction.getName(), configuredAction)); + + OperatorSubtaskState snapshot; + try (KeyedOneInputStreamOperatorTestHarness testHarness = + new KeyedOneInputStreamOperatorTestHarness<>( + new ActionExecutionOperatorFactory(agentPlan, true), + (KeySelector) value -> value, + TypeInformation.of(Long.class))) { + testHarness.open(); + testHarness.processElement(new StreamRecord<>(key)); + assertThat(testHarness.getTaskMailbox().size()).isEqualTo(1); + snapshot = testHarness.snapshot(1L, 1L); + } + + try (KeyedOneInputStreamOperatorTestHarness restoredHarness = + new KeyedOneInputStreamOperatorTestHarness<>( + new ActionExecutionOperatorFactory(agentPlan, true), + (KeySelector) value -> value, + TypeInformation.of(Long.class))) { + restoredHarness.initializeState(snapshot); + restoredHarness.open(); + + ActionExecutionOperator restoredOperator = + (ActionExecutionOperator) restoredHarness.getOperator(); + restoredOperator.setCurrentKey(key); + ActionTask restoredTask = + restoredOperator.getOperatorStateManager().pollNextActionTask(); + + assertThat(restoredTask).isNotNull(); + assertThat(restoredTask.action.getConfig()).containsEntry("mode", "strict"); + } + } + @Test void testMemoryAccessProhibitedOutsideMailboxThread() throws Exception { try (KeyedOneInputStreamOperatorTestHarness testHarness = @@ -1811,7 +1858,7 @@ private static AgentPlan getAgentPlanWithConfig( actions.put(action3.getName(), action3); } - return new AgentPlan(actions, actionsByEvent, new HashMap<>(), config); + return new AgentPlan(actions, new HashMap<>(), config); } catch (Exception e) { ExceptionUtils.rethrow(e); } @@ -1869,7 +1916,7 @@ public static AgentPlan getAsyncAgentPlan(boolean useMultiAsync) { actions.put(action2.getName(), action2); } - return new AgentPlan(actions, actionsByEvent, new HashMap<>()); + return new AgentPlan(actions, new HashMap<>()); } catch (Exception e) { ExceptionUtils.rethrow(e); } @@ -1893,7 +1940,7 @@ public static AgentPlan getDurableSyncAgentPlan() { InputEvent.EVENT_TYPE, Collections.singletonList(durableSyncAction)); actions.put(durableSyncAction.getName(), durableSyncAction); - return new AgentPlan(actions, actionsByEvent, new HashMap<>()); + return new AgentPlan(actions, new HashMap<>()); } catch (Exception e) { ExceptionUtils.rethrow(e); } @@ -1917,7 +1964,7 @@ public static AgentPlan getDurableReconcilableAgentPlan() { InputEvent.EVENT_TYPE, Collections.singletonList(reconcilableAction)); actions.put(reconcilableAction.getName(), reconcilableAction); - return new AgentPlan(actions, actionsByEvent, new HashMap<>()); + return new AgentPlan(actions, new HashMap<>()); } catch (Exception e) { ExceptionUtils.rethrow(e); } @@ -1941,7 +1988,7 @@ public static AgentPlan getDurableMixedRecoveryAgentPlan() { InputEvent.EVENT_TYPE, Collections.singletonList(mixedRecoveryAction)); actions.put(mixedRecoveryAction.getName(), mixedRecoveryAction); - return new AgentPlan(actions, actionsByEvent, new HashMap<>()); + return new AgentPlan(actions, new HashMap<>()); } catch (Exception e) { ExceptionUtils.rethrow(e); } @@ -1965,7 +2012,7 @@ public static AgentPlan getDurableExceptionAgentPlan() { InputEvent.EVENT_TYPE, Collections.singletonList(exceptionAction)); actions.put(exceptionAction.getName(), exceptionAction); - return new AgentPlan(actions, actionsByEvent, new HashMap<>()); + return new AgentPlan(actions, new HashMap<>()); } catch (Exception e) { ExceptionUtils.rethrow(e); } @@ -1974,7 +2021,6 @@ public static AgentPlan getDurableExceptionAgentPlan() { public static AgentPlan getLinkageErrorAgentPlan() { try { - Map> actionsByEvent = new HashMap<>(); Map actions = new HashMap<>(); Action errorAction = @@ -1985,10 +2031,9 @@ public static AgentPlan getLinkageErrorAgentPlan() { "action", new Class[] {Event.class, RunnerContext.class}), Collections.singletonList(InputEvent.EVENT_TYPE)); - actionsByEvent.put(InputEvent.EVENT_TYPE, Collections.singletonList(errorAction)); actions.put(errorAction.getName(), errorAction); - return new AgentPlan(actions, actionsByEvent, new HashMap<>()); + return new AgentPlan(actions); } catch (Exception e) { ExceptionUtils.rethrow(e); } @@ -2068,7 +2113,7 @@ public static AgentPlan getDurableExceptionUncaughtAgentPlan() { InputEvent.EVENT_TYPE, Collections.singletonList(exceptionAction)); actions.put(exceptionAction.getName(), exceptionAction); - return new AgentPlan(actions, actionsByEvent, new HashMap<>()); + return new AgentPlan(actions, new HashMap<>()); } catch (Exception e) { ExceptionUtils.rethrow(e); } @@ -2092,7 +2137,7 @@ public static AgentPlan getDurableAsyncExceptionAgentPlan() { InputEvent.EVENT_TYPE, Collections.singletonList(exceptionAction)); actions.put(exceptionAction.getName(), exceptionAction); - return new AgentPlan(actions, actionsByEvent, new HashMap<>()); + return new AgentPlan(actions, new HashMap<>()); } catch (Exception e) { ExceptionUtils.rethrow(e); } diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/CrossLanguageActionRuntimeTest.java b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/CrossLanguageActionRuntimeTest.java index 08a09b6e3..e80eda6d8 100644 --- a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/CrossLanguageActionRuntimeTest.java +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/CrossLanguageActionRuntimeTest.java @@ -26,19 +26,25 @@ import org.apache.flink.agents.plan.JavaFunction; import org.apache.flink.agents.plan.PythonFunction; import org.apache.flink.agents.plan.actions.Action; +import org.apache.flink.agents.runtime.condition.ActionMatcher; import org.apache.flink.api.common.typeinfo.TypeInformation; import org.apache.flink.api.java.functions.KeySelector; import org.apache.flink.streaming.runtime.streamrecord.StreamRecord; import org.apache.flink.streaming.util.KeyedOneInputStreamOperatorTestHarness; import org.junit.jupiter.api.Test; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; +import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; /** - * Layer F1 — feed a Python-shaped JSON plan into the operator harness and confirm a JavaFunction - * action body runs. Java→Python action dispatch goes through Pemja and is Layer F2 scope. + * Layer F1 — feed Python-shaped JSON plans into the Java runtime and verify trigger-condition + * matching and JavaFunction dispatch. Java→Python action dispatch goes through Pemja and is Layer + * F2 scope. */ public class CrossLanguageActionRuntimeTest { @@ -98,6 +104,24 @@ void pythonFunctionPlanDeserializesAndIsRecognizedByOperatorFactory() throws Exc assertThat(factory).isNotNull(); } + @Test + void pythonPlanUsesRuntimeOrSemantics() throws Exception { + Path repoRoot = Paths.get(System.getProperty("user.dir")).getParent(); + Path snapshot = + repoRoot.resolve( + "e2e-test/cross-language-agent-plan-snapshots/python/agent_plan_with_java_action.json"); + AgentPlan plan = MAPPER.readValue(Files.readString(snapshot), AgentPlan.class); + Action handle = plan.getActions().get("handle"); + ActionMatcher matcher = new ActionMatcher(plan); + + assertThat(handle.getTriggerConditions()) + .containsExactly(InputEvent.EVENT_TYPE, "attributes.ready == true"); + assertThat(matcher.match(new InputEvent("type-hit"))).containsExactly(handle); + assertThat(matcher.match(new Event("other", Map.of("ready", true)))) + .containsExactly(handle); + assertThat(matcher.match(new Event("other", Map.of("ready", false)))).isEmpty(); + } + /** * Wire format Python emits via {@code AgentPlan.model_dump_json}; pinned symmetric in Layer B. */ @@ -122,16 +146,12 @@ private static String pythonShapedPlanJson() { + " \"config\":null" + " }" + "}," - + "\"actions_by_event\":{" - + " \"_input_event\":[\"handle\"]" - + "}," + "\"resource_providers\":{}," + "\"config\":{\"conf_data\":{}}" + "}"; } private static AgentPlan planWithPythonAction() throws Exception { - java.util.Map> actionsByEvent = new java.util.HashMap<>(); java.util.Map actions = new java.util.HashMap<>(); PythonFunction pythonFn = @@ -143,11 +163,8 @@ private static AgentPlan planWithPythonAction() throws Exception { pythonFn, java.util.Collections.singletonList(InputEvent.EVENT_TYPE)); actions.put(act.getName(), act); - actionsByEvent.put(InputEvent.EVENT_TYPE, java.util.Collections.singletonList(act)); - return new AgentPlan( actions, - actionsByEvent, new java.util.HashMap<>(), new org.apache.flink.agents.plan.AgentConfiguration()); } diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/EventRouterTest.java b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/EventRouterTest.java index 97f994b42..dc224ba8c 100644 --- a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/EventRouterTest.java +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/EventRouterTest.java @@ -68,11 +68,11 @@ void getActionsTriggeredByReturnsActionsForJavaEventClass() throws Exception { Action action = TestActions.noopAction(); Map actions = Map.of(action.getName(), action); Map> byEvent = Map.of(InputEvent.EVENT_TYPE, List.of(action)); - AgentPlan plan = new AgentPlan(actions, byEvent); + AgentPlan plan = new AgentPlan(actions); EventRouter router = new EventRouter<>(plan, /* inputIsJava */ true); - List triggered = router.getActionsTriggeredBy(new InputEvent(0L), plan); + List triggered = router.getActionsTriggeredBy(new InputEvent(0L)); assertThat(triggered).containsExactly(action); } diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/PythonBridgeManagerTest.java b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/PythonBridgeManagerTest.java index 9ee685638..17dce19f4 100644 --- a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/PythonBridgeManagerTest.java +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/PythonBridgeManagerTest.java @@ -37,8 +37,8 @@ void openIsNoOpWhenPlanHasNeitherPythonActionsNorResources() throws Exception { // Java-only plan: one Java action, no resources. Action javaAction = TestActions.noopAction(); Map actions = Map.of(javaAction.getName(), javaAction); - Map> byEvent = Map.of(InputEvent.class.getName(), List.of(javaAction)); - AgentPlan plan = new AgentPlan(actions, byEvent); + Map> byEvent = Map.of(InputEvent.EVENT_TYPE, List.of(javaAction)); + AgentPlan plan = new AgentPlan(actions); try (PythonBridgeManager bridge = new PythonBridgeManager()) { bridge.open( diff --git a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/TestActions.java b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/TestActions.java index f427814a0..ec421d683 100644 --- a/runtime/src/test/java/org/apache/flink/agents/runtime/operator/TestActions.java +++ b/runtime/src/test/java/org/apache/flink/agents/runtime/operator/TestActions.java @@ -44,7 +44,7 @@ static Action noopAction() { TestActions.class, "noop", new Class[] {InputEvent.class, RunnerContext.class}), - List.of(InputEvent.class.getName())); + List.of(InputEvent.EVENT_TYPE)); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/runtime/src/test/resources/cel_conformance_cases.yaml b/runtime/src/test/resources/cel_conformance_cases.yaml new file mode 100644 index 000000000..7c6e7caae --- /dev/null +++ b/runtime/src/test/resources/cel_conformance_cases.yaml @@ -0,0 +1,285 @@ +# Trigger-condition conformance corpus, owned by runtime; consumed by ConditionEvaluatorTest. +# Each case is evaluated end-to-end: parse → validate → check → eval (no AST rewriting). +# +# Activation contract: id/type/EventType are framework-owned. Every event promotes only referenced +# top-level entries from Event.attributes; nested values remain under their attribute path, such as +# input.status or output.status. The explicit attributes namespace preserves the same envelope, +# including user attributes whose names collide with framework variables. + +- name: filter_by_type_match + condition: 'type == "_input_event"' + event: { id: abc, type: _input_event, attributes: {} } + expected: true + +- name: filter_by_type_mismatch + condition: 'type == "other"' + event: { id: abc, type: test, attributes: {} } + expected: false + +- name: filter_by_id + condition: 'id == "550e8400-e29b-41d4-a716-446655440000"' + event: { id: '550e8400-e29b-41d4-a716-446655440000', type: test, attributes: {} } + expected: true + +- name: has_macro_on_map + condition: 'has(attributes.score) && attributes.score >= 3' + event: { id: abc, type: test, attributes: { score: 5 } } + expected: true + +- name: has_macro_missing_key + condition: 'has(attributes.missing) && attributes.missing == true' + event: { id: abc, type: test, attributes: {} } + expected: false + +- name: nested_map_access + condition: 'has(attributes.meta) && attributes.meta.source == "api"' + event: { id: abc, type: test, attributes: { meta: { source: api } } } + expected: true + +- name: combined_type_and_attribute + condition: 'type == "review" && has(attributes.score) && attributes.score >= 3' + event: { id: abc, type: review, attributes: { score: 4 } } + expected: true + +- name: boolean_attribute + condition: 'has(attributes.success) && attributes.success == true' + event: { id: abc, type: test, attributes: { success: true } } + expected: true + +- name: string_attribute + condition: 'has(attributes.model) && attributes.model == "gpt-4"' + event: { id: abc, type: _chat_request_event, attributes: { model: gpt-4 } } + expected: true + +- name: numeric_comparison_less_than + condition: 'has(attributes.retry_count) && attributes.retry_count < 3' + event: { id: abc, type: test, attributes: { retry_count: 1 } } + expected: true + +- name: numeric_comparison_fails + condition: 'has(attributes.retry_count) && attributes.retry_count < 3' + event: { id: abc, type: test, attributes: { retry_count: 5 } } + expected: false + +# The framework does not parse JSON-shaped strings; a user supplies structured (map/list) +# attributes when a condition needs to match on nested fields. +- name: nested_object_field_access + condition: 'has(attributes.input) && has(attributes.input.id) && attributes.input.id == "B000YFSR4W"' + event: + id: abc + type: _input_event + attributes: + input: { id: B000YFSR4W, review: comfy fit } + expected: true + +- name: nested_object_field_mismatch + condition: 'has(attributes.input) && has(attributes.input.id) && attributes.input.id == "OTHER"' + event: + id: abc + type: _input_event + attributes: + input: { id: B000YFSR4W, review: comfy fit } + expected: false + +- name: nested_array_element_access + condition: 'has(attributes.tags) && attributes.tags[0] == "sports"' + event: + id: abc + type: test + attributes: + tags: [sports, shoes] + expected: true + +- name: json_shaped_string_not_parsed + condition: "has(attributes.tags) && attributes.tags == '[1, 2, 3]'" + event: { id: abc, type: test, attributes: { tags: '[1, 2, 3]' } } + expected: true + +- name: plain_string_not_parsed + condition: 'has(attributes.name) && attributes.name == "hello world"' + event: { id: abc, type: test, attributes: { name: hello world } } + expected: true + +- name: invalid_json_string_kept_as_string + condition: 'has(attributes.broken) && attributes.broken == "{not json"' + event: { id: abc, type: test, attributes: { broken: '{not json' } } + expected: true + +- name: empty_attributes_has_returns_false + condition: 'has(attributes.key)' + event: { id: abc, type: test, attributes: {} } + expected: false + +- name: deeply_nested_structured_3_levels + condition: 'has(attributes.data) && has(attributes.data.order) && attributes.data.order.id == "O100"' + event: + id: abc + type: test + attributes: + data: { order: { id: O100, amount: 500 } } + expected: true + +- name: numeric_int_equals_int + condition: 'has(attributes.count) && attributes.count == 42' + event: { id: abc, type: test, attributes: { count: 42 } } + expected: true + +- name: or_condition_first_branch_true + condition: 'has(attributes.a) && attributes.a == 1 || has(attributes.b) && attributes.b == 2' + event: { id: abc, type: test, attributes: { a: 1 } } + expected: true + +- name: or_condition_second_branch_true + condition: '(has(attributes.a) && attributes.a == 1) || (has(attributes.b) && attributes.b == 2)' + event: { id: abc, type: test, attributes: { b: 2 } } + expected: true + +- name: dot_chain_deep_nested_access + condition: 'has(region.width) && region.width.score >= 80' + event: { id: abc, type: test, attributes: { region: { width: { score: 95 } } } } + expected: true + +- name: dot_chain_deep_nested_mismatch + condition: 'has(region.width) && region.width.score >= 80' + event: { id: abc, type: test, attributes: { region: { width: { score: 50 } } } } + expected: false + +- name: dot_chain_six_levels + condition: 'has(input.region) && input.region.width.score.month12 == 88' + event: + id: abc + type: test + attributes: + input: { region: { width: { score: { month12: 88 } } } } + expected: true + +- name: or_short_circuit_left_true_skips_right_error + condition: '(type == "hit") || (attributes.nope.deep > 3)' + event: { id: abc, type: hit, attributes: {} } + expected: true + +- name: and_short_circuit_has_guards_missing_field_access + condition: 'has(attributes.missing) && (attributes.missing.deep > 3)' + event: { id: abc, type: test, attributes: {} } + expected: false + +- name: empty_map_missing_key_has_returns_false + condition: 'has(attributes.config.missing_key)' + event: { id: abc, type: test, attributes: { config: {} } } + expected: false + +- name: nested_structured_recursive_access + condition: 'has(attributes.outer.inner) && attributes.outer.inner.id == 1' + event: + id: abc + type: test + attributes: + outer: { inner: { id: 1 } } + expected: true + +- name: event_type_select_expr_folded_to_literal + condition: 'type == EventType.InputEvent' + event: { id: abc, type: _input_event, attributes: {} } + expected: true + +- name: int64_max_in_range + condition: 'attributes.x == 9223372036854775807' + event: { id: abc, type: test, attributes: { x: 9223372036854775807 } } + expected: true + +# ----- Unified top-level attribute activation contract ----- + +- name: bare_root_attribute_access + condition: 'score >= 3' + event: { id: abc, type: test, attributes: { score: 5 } } + expected: true + +- name: has_on_bare_attribute_present + condition: 'has(score) && score >= 3' + event: { id: abc, type: test, attributes: { score: 5 } } + expected: true + +- name: has_on_bare_attribute_missing + condition: 'has(score) && score >= 3' + event: { id: abc, type: test, attributes: {} } + expected: false + +- name: input_payload_access_uses_input_path + condition: 'input.k == "in"' + event: { id: abc, type: _input_event, attributes: { input: { k: in } } } + expected: true + +- name: input_payload_child_is_not_promoted + condition: 'has(k)' + event: { id: abc, type: _input_event, attributes: { input: { k: in } } } + expected: false + +- name: output_payload_access_uses_output_path + condition: 'output.k == "out"' + event: { id: abc, type: _output_event, attributes: { output: { k: out } } } + expected: true + +- name: output_payload_child_is_not_promoted + condition: 'has(k)' + event: { id: abc, type: _output_event, attributes: { output: { k: out } } } + expected: false + +- name: framework_id_wins_over_attribute_id + condition: 'id == "550e8400-e29b-41d4-a716-446655440000"' + event: { id: '550e8400-e29b-41d4-a716-446655440000', type: test, attributes: { id: tenant-42 } } + expected: true + +- name: attribute_id_still_reachable_via_attributes + condition: 'attributes.id == "tenant-42"' + event: { id: '550e8400-e29b-41d4-a716-446655440000', type: test, attributes: { id: tenant-42 } } + expected: true + +- name: framework_type_wins_over_attribute_type + condition: 'type == "real"' + event: { id: abc, type: real, attributes: { type: fake } } + expected: true + +- name: attribute_type_still_reachable_via_attributes + condition: 'attributes.type == "fake"' + event: { id: abc, type: real, attributes: { type: fake } } + expected: true + +- name: flatten_is_single_level_only + condition: 'mylist.name == "x"' + event: { id: abc, type: test, attributes: { mylist: { name: x } } } + expected: true + +- name: event_type_constant_combined_with_bare_attribute + condition: 'type == EventType.InputEvent && score > 0.8' + event: { id: abc, type: _input_event, attributes: { score: 0.9 } } + expected: true + +# ----- Nested has() short-circuit (has(a.b.c) desugaring) ----- +# has(a.b.c) desugars to has(a) && has(a.b) && has(a.b.c): a missing intermediate (or a +# wholly-absent root) must short-circuit to false rather than throwing "no such key". +# Otherwise WARN_AND_SKIP would turn the evaluation error into a silent missed trigger. + +- name: has_nested_intermediate_missing_returns_false + condition: 'has(attributes.meta.source)' + event: { id: abc, type: test, attributes: {} } + expected: false + +- name: has_nested_deep_all_present + condition: 'has(attributes.meta.source)' + event: { id: abc, type: test, attributes: { meta: { source: api } } } + expected: true + +- name: has_nested_guard_then_access + condition: 'has(attributes.meta.source) && attributes.meta.source == "api"' + event: { id: abc, type: test, attributes: { meta: { source: api } } } + expected: true + +- name: has_bare_root_nested_intermediate_missing_returns_false + condition: 'has(value.user.tier)' + event: { id: abc, type: test, attributes: { value: { other: 1 } } } + expected: false + +- name: has_bare_root_nested_wholly_absent_returns_false + condition: 'has(value.user.tier)' + event: { id: abc, type: test, attributes: {} } + expected: false