Choice state detect data type errors

lib/floe.rb

@@ -44,6 +44,7 @@ class Error < StandardError; end
   class InvalidWorkflowError < Error; end
   class InvalidExecutionInput < Error; end
   class PathError < Error; end
+  class ExecutionError < Error; end
 
   def self.logger
     @logger ||= NullLogger.new

lib/floe/validation_mixin.rb

@@ -18,6 +18,10 @@ def invalid_field_error!(field_name, field_value = nil, comment = nil)
       self.class.invalid_field_error!(name, field_name, field_value, comment)
     end
 
+    def runtime_field_error!(field_name, field_value, comment)
+      raise Floe::ExecutionError, self.class.field_error_text(name, field_name, field_value, comment)
+    end
+
     def workflow_state?(field_value, workflow)
       workflow.payload["States"] ? workflow.payload["States"].include?(field_value) : true
     end
@@ -39,10 +43,14 @@ def missing_field_error!(name, field_name)
       end
 
       def invalid_field_error!(name, field_name, field_value, comment)
+        raise Floe::InvalidWorkflowError, field_error_text(name, field_name, field_value, comment)
+      end
+
+      def field_error_text(name, field_name, field_value, comment = nil)
         # instead of displaying a large hash or array, just displaying the word Hash or Array
         field_value = field_value.class if field_value.kind_of?(Hash) || field_value.kind_of?(Array)
 
-        parser_error!(name, "field \"#{field_name}\"#{" value \"#{field_value}\"" unless field_value.nil?} #{comment}")
+        "#{Array(name).join(".")} field \"#{field_name}\"#{" value \"#{field_value}\"" unless field_value.nil?} #{comment}"
       end
     end
   end

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)
@@ -109,11 +109,17 @@ def is_timestamp?(value, ret_value = true)
         # rubocop:enable Style/OptionalBooleanParameter
 
         def variable_value(context, input)
-          variable.value(context, input)
+          fetch_path("Variable", variable, context, input)
         end
 
         def compare_value(context, input)
-          path ? value.value(context, input) : value
+          path ? fetch_path(compare_key, value, context, input) : value
+        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_compare_key
@@ -126,11 +132,29 @@ def parse_compare_key
           # @type.nil? means we won't type check the variable or compare value
         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 parse_path(field_name, payload)
           value = payload[field_name]
           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

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