ChoiceRule validates next and data validates variable

Variable was only for Data and not for Not, Or, or And, so moved it to Data.

Since we want to add type checking, and that is Data specific,
it works better variable is over there.

lib/floe/workflow/choice_rule.rb

@@ -3,6 +3,8 @@
 module Floe
   class Workflow
     class ChoiceRule
+      include ValidationMixin
+
       class << self
         def build(workflow, name, payload)
           if (sub_payloads = payload["Not"])
@@ -27,18 +29,47 @@ def build_children(workflow, name, sub_payloads)
 
       attr_reader :next, :payload, :children, :name
 
-      def initialize(_workflow, name, payload, children = nil)
+      def initialize(workflow, name, payload, children = nil)
         @name      = name
         @payload   = payload
         @children  = children
         @next      = payload["Next"]
+
+        validate_next!(workflow)
       end
 
       def true?(*)
         raise NotImplementedError, "Must be implemented in a subclass"
       end
 
       private
+
+      def validate_next!(workflow)
+        if is_child?
+          # non-top level nodes don't allow a next
+          invalid_field_error!("Next", @next, "not allowed in a child rule") if @next
+        elsif !@next
+          # top level nodes require a next
+          missing_field_error!("Next")
+        elsif !workflow_state?(@next, workflow)
+          # top level nodes require a next field that is found
+          invalid_field_error!("Next", @next, "is not found in \"States\"")
+        end
+      end
+
+      # returns true if this is a child rule underneath an And/Or/Not
+      # {
+      #   "Or": [
+      #     {"Variable": "$.foo", "IsString": true},
+      #     {"Variable": "$.foo", "IsBoolean": true}
+      #   ], "Next": "Finished"
+      # }
+      #
+      # The Or node, has no conjunction parent, so it is not a child (requires a Next)
+      # The 2 Data nodes have a conjunction parent, so each one is a child (do not allow a Next)
+      def is_child? # rubocop:disable Naming/PredicateName
+        !(%w[And Or Not] & name[0..-2]).empty?
+      end
     end
   end
 end

lib/floe/workflow/choice_rule/data.rb

@@ -8,13 +8,12 @@ class Data < Floe::Workflow::ChoiceRule
 
         attr_reader :variable, :compare_key
 
-        def initialize(*)
+        def initialize(_workflow, _name, payload)
           super
 
-          @variable = payload["Variable"]
+          @variable = parse_path("Variable", payload)
           @compare_key = payload.keys.detect { |key| key.match?(/^(#{COMPARE_KEYS.join("|")})/) }
-
-          raise Floe::InvalidWorkflowError, "Data-test Expression Choice Rule must have a compare key" if @compare_key.nil?
+          parser_error!("requires a compare key") unless compare_key
         end
 
         def true?(context, input)
@@ -106,12 +105,18 @@ def is_timestamp?(value) # rubocop:disable Naming/PredicateName
         end
 
         def variable_value(context, input)
-          Path.value(variable, context, input)
+          variable.value(context, input)
         end
 
         def compare_value(context, input)
           compare_key.end_with?("Path") ? Path.value(payload[compare_key], context, input) : payload[compare_key]
         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
       end
     end
   end

spec/workflow/choice_rule_spec.rb

@@ -16,6 +16,34 @@
     it "works with valid next" do
       workflow
     end
+
+    context "with unknown Next" do
+      let(:choices) { [{"Variable" => "$.foo", "StringEquals" => "bar", "Next" => "Missing"}] }
+      it { expect { workflow }.to raise_exception(Floe::InvalidWorkflowError, "States.Choice1.Choices.0.Data field \"Next\" value \"Missing\" is not found in \"States\"") }
+    end
+
+    context "with Variable missing" do
+      let(:choices) { [{"StringEquals" => "bar", "Next" => "FirstMatchState"}] }
+
+      it { expect { workflow }.to raise_exception(Floe::InvalidWorkflowError, "States.Choice1.Choices.0.Data does not have required field \"Variable\"") }
+    end
+
+    context "with non-path Variable" do
+      let(:choices) { [{"Variable" => "wrong", "Next" => "FirstMatchState"}] }
+      it { expect { workflow }.to raise_exception(Floe::InvalidWorkflowError, "States.Choice1.Choices.0.Data field \"Variable\" value \"wrong\" Path [wrong] must start with \"$\"") }
+    end
+
+    context "with second level Next (Not)" do
+      let(:choices) { [{"Not" => {"Variable" => "$.foo", "StringEquals" => "bar", "Next" => "FirstMatchState"}, "Next" => "FirstMatchState"}] }
+
+      it { expect { workflow }.to raise_exception(Floe::InvalidWorkflowError, "States.Choice1.Choices.0.Not.0.Data field \"Next\" value \"FirstMatchState\" not allowed in a child rule") }
+    end
+
+    context "with second level Next (And)" do
+      let(:choices) { [{"And" => [{"Variable" => "$.foo", "StringEquals" => "bar", "Next" => "FirstMatchState"}], "Next" => "FirstMatchState"}] }
+
+      it { expect { workflow }.to raise_exception(Floe::InvalidWorkflowError, "States.Choice1.Choices.0.And.0.Data field \"Next\" value \"FirstMatchState\" not allowed in a child rule") }
+    end
   end
 
   describe "#true?" do
@@ -25,7 +53,7 @@
 
     context "with abstract top level class" do
       let(:input) { {} }
-      let(:subject) { described_class.new(workflow, ["Choice1", "Choices", 1], choices.first).true?(context, input) }
+      let(:subject) { described_class.new(workflow, ["Choice1", "Choices", 1, "Data"], choices.first).true?(context, input) }
 
       it "is not implemented" do
         expect { subject }.to raise_exception(NotImplementedError)
@@ -109,7 +137,7 @@
         let(:input) { {"foo" => "bar"} }
 
         it "raises an exception" do
-          expect { subject }.to raise_exception(Floe::InvalidWorkflowError, "Data-test Expression Choice Rule must have a compare key")
+          expect { subject }.to raise_exception(Floe::InvalidWorkflowError, "States.Choice1.Choices.0.Data requires a compare key")
         end
       end
 
@@ -118,7 +146,7 @@
         let(:input)   { {"foo" => 0, "bar" => 1} }
 
         it "fails" do
-          expect { subject }.to raise_exception(Floe::InvalidWorkflowError, "Data-test Expression Choice Rule must have a compare key")
+          expect { subject }.to raise_exception(Floe::InvalidWorkflowError, "States.Choice1.Choices.0.Data requires a compare key")
         end
       end