From b60a8667c8634f33a9b3abee49d9796ab4721da9 Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Tue, 20 Aug 2024 10:13:11 -0400
Subject: [PATCH] Choice state detect data type errors

---
 lib/floe/workflow/choice_rule/data.rb | 36 ++++++++++++++++++++++-----
 spec/workflow/choice_rule_spec.rb     | 34 +++++++++++++++++++------
 2 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/lib/floe/workflow/choice_rule/data.rb b/lib/floe/workflow/choice_rule/data.rb
index 1e004191..065c532f 100644
--- a/lib/floe/workflow/choice_rule/data.rb
+++ b/lib/floe/workflow/choice_rule/data.rb
@@ -4,12 +4,12 @@ module Floe
   class Workflow
     class ChoiceRule
       class Data < Floe::Workflow::ChoiceRule
-        TYPES      = ["String", "Numeric", "Boolean", "Timestamp", "Present", "Null"].freeze
+        TYPES      = {"String" => :is_string?, "Numeric" => :is_numeric?, "Boolean" => :is_boolean?, "Timestamp" => :is_timestamp?, "Present" => :is_present?, "Null" => :is_null?}.freeze
         COMPARES   = ["Equals", "LessThan", "GreaterThan", "LessThanEquals", "GreaterThanEquals", "Matches"].freeze
         # e.g.: (Is)(String), (Is)(Present)
-        TYPE_CHECK = /^(Is)(#{TYPES.join("|")})$/.freeze
+        TYPE_CHECK = /^(Is)(#{TYPES.keys.join("|")})$/.freeze
         # e.g.: (String)(LessThan)(Path), (Numeric)(GreaterThanEquals)()
-        OPERATION  = /^(#{(TYPES - %w[Null Present]).join("|")})(#{COMPARES.join("|")})(Path)?$/.freeze
+        OPERATION  = /^(#{(TYPES.keys - %w[Null Present]).join("|")})(#{COMPARES.join("|")})(Path)?$/.freeze
 
         attr_reader :variable, :compare_key, :type, :value, :path
 
@@ -18,7 +18,7 @@ def initialize(_workflow, _name, payload)
 
           @variable = parse_path("Variable", payload)
           parse_compare_key
-          @value = path ? parse_path(compare_key, payload) : payload[compare_key]
+          @value = parse_compare_value
         end
 
         def true?(context, input)
@@ -124,11 +124,29 @@ def parse_compare_key
         end
 
         def compare_value(context, input)
-          path ? value.value(context, input) : value
+          path ? fetch_path(compare_key, value, context, input) : value
         end
 
         def variable_value(context, input)
-          variable.value(context, input)
+          fetch_path("Variable", variable, context, input)
+        end
+
+        def parse_compare_value
+          if @path
+            parse_path(@compare_key, payload)
+          else
+            parse_value(@compare_key, payload)
+          end
+        end
+
+        def correct_type?(val)
+          send(TYPES[type || "Boolean"], val)
+        end
+
+        def fetch_path(field_name, field_path, context, input)
+          ret_value = field_path.value(context, input)
+          runtime_field_error!(field_name, field_path.to_s, "must point to a #{type}") if type && !correct_type?(ret_value)
+          ret_value
         end
 
         def parse_path(field_name, payload)
@@ -136,6 +154,12 @@ def parse_path(field_name, payload)
           missing_field_error!(field_name) unless value
           wrap_parser_error(field_name, value) { Path.new(value) }
         end
+
+        def parse_value(field_name, payload)
+          value = payload[field_name]
+          invalid_field_error!(field_name, value, "required to be a #{type || "Boolean"}") unless correct_type?(value)
+          value
+        end
       end
     end
   end
diff --git a/spec/workflow/choice_rule_spec.rb b/spec/workflow/choice_rule_spec.rb
index 2129e9de..ff49b25d 100644
--- a/spec/workflow/choice_rule_spec.rb
+++ b/spec/workflow/choice_rule_spec.rb
@@ -384,6 +384,16 @@
           end
         end
 
+        context "with wrong match type on the left" do
+          let(:input) { {"foo" => "abc", "bar" => 2} }
+          it { expect { subject }.to raise_exception(Floe::ExecutionError, "States.Choice1.Choices.0.Data field \"Variable\" value \"$.foo\" must point to a Numeric") }
+        end
+
+        context "with wrong match type on the right" do
+          let(:input) { {"foo" => 2, "bar" => "xyz"} }
+          it { expect { subject }.to raise_exception(Floe::ExecutionError, "States.Choice1.Choices.0.Data field \"NumericEqualsPath\" value \"$.bar\" must point to a Numeric") }
+        end
+
         context "with path not found" do
           let(:input) { {"foo" => 2} }
           it { expect { subject }.to raise_error(Floe::PathError, "Path [$.bar] references an invalid value") }
@@ -468,6 +478,12 @@
             expect(subject).to eq(false)
           end
         end
+
+        context "with invalid Path" do
+          let(:choices) { [{"Variable" => "$.foo", "NumericGreaterThanPath" => "bogus", "Next" => "FirstMatchState"}] }
+          let(:input) { {} }
+          it { expect { subject }.to raise_exception(Floe::InvalidWorkflowError, "States.Choice1.Choices.0.Data field \"NumericGreaterThanPath\" value \"bogus\" Path [bogus] must start with \"$\"") }
+        end
       end
 
       context "with a NumericLessThanEquals" do
@@ -555,18 +571,22 @@
 
         context "that is true" do
           let(:input) { {"foo" => "audit.log"} }
-
-          it "returns true" do
-            expect(subject).to eq(true)
-          end
+          it { expect(subject).to eq(true) }
         end
 
         context "that is false" do
           let(:input) { {"foo" => "audit"} }
+          it { expect(subject).to eq(false) }
+        end
 
-          it "returns false" do
-            expect(subject).to eq(false)
-          end
+        context "that does not exist" do
+          let(:input) { {} }
+          it { expect { subject }.to raise_exception(Floe::PathError, "Path [$.foo] references an invalid value") }
+        end
+
+        context "that references a number" do
+          let(:input) { {"foo" => 5} }
+          it { expect { subject }.to raise_exception(Floe::ExecutionError, "States.Choice1.Choices.0.Data field \"Variable\" value \"$.foo\" must point to a String") }
         end
       end
     end