Correctly recurse over nested field associations

app/models/dialog_field_association_validator.rb

@@ -1,28 +1,11 @@
 class DialogFieldAssociationValidator
-  def circular_references(associations)
-    return false if associations.empty?
-    association_path_list = initial_paths(associations)
-    association_path_list.each do |path|
-      fieldname_being_triggered = path.last
-      next if associations[fieldname_being_triggered].blank?
-      circular_references = walk_value_path(fieldname_being_triggered, associations, path)
-      return circular_references if circular_references.present?
-    end
-    false
-  end
-
-  private
-
-  def initial_paths(associations)
-    associations.flat_map { |key, values| values.map { |value| [key, value] } }
-  end
-
-  def walk_value_path(fieldname_being_triggered, associations, path)
-    while associations[fieldname_being_triggered].present?
-      return [fieldname_being_triggered, associations[fieldname_being_triggered].first] if path.include?(associations[fieldname_being_triggered].first)
-      path << associations[fieldname_being_triggered]
-      path.flatten!
-      fieldname_being_triggered = path.last
+  class DialogFieldAssociationCircularReferenceError < RuntimeError; end
+  def check_for_circular_references(hash, k, collection = [])
+    raise DialogFieldAssociationCircularReferenceError, "#{k} already exists in #{collection}" if collection.include?(k)
+    collection << k
+    hash[k]&.each do |val|
+      check_for_circular_references(hash, val, collection.dup)
     end
+    nil
   end
 end

app/models/dialog_import_validator.rb

@@ -3,7 +3,6 @@ class ImportNonYamlError < StandardError; end
   class InvalidDialogFieldTypeError < StandardError; end
   class ParsedNonDialogYamlError < StandardError; end
   class ParsedNonDialogError < StandardError; end
-  class DialogFieldAssociationCircularReferenceError < StandardError; end
   class InvalidDialogVersionError < StandardError; end
 
   def initialize(dialog_field_association_validator = DialogFieldAssociationValidator.new)
@@ -67,8 +66,7 @@ def check_dialog_associations_for_validity(dialog_fields)
     associations = {}
     dialog_fields.each { |df| associations.merge!(df["name"] => df["dialog_field_responders"]) if df["dialog_field_responders"].present? }
     unless associations.blank?
-      circular_references = @dialog_field_association_validator.circular_references(associations)
-      raise DialogFieldAssociationCircularReferenceError, circular_references if circular_references
+      associations.each_key { |k|  @dialog_field_association_validator.check_for_circular_references(associations, k) }
     end
   end
 

lib/services/dialog_import_service.rb

@@ -1,7 +1,6 @@
 class DialogImportService
   class ImportNonYamlError < StandardError; end
   class ParsedNonDialogYamlError < StandardError; end
-  class DialogFieldAssociationCircularReferenceError < StandardError; end
 
   DEFAULT_DIALOG_VERSION = '5.10'.freeze # assumed for dialogs without version info
   CURRENT_DIALOG_VERSION = '5.11'.freeze
@@ -96,7 +95,7 @@ def build_resource_actions(dialog)
 
   def check_field_associations(fields)
     associations = fields.each_with_object({}) { |df, hsh| hsh.merge!(df["name"] => df["dialog_field_responders"]) if df["dialog_field_responders"].present? }
-    raise DialogFieldAssociationCircularReferenceError if @dialog_field_association_validator.circular_references(associations)
+    associations.each_key { |k| @dialog_field_association_validator.check_for_circular_references(associations, k) }
   end
 
   def import(dialog)

spec/models/dialog_field_association_validator_spec.rb

@@ -2,34 +2,30 @@
   let(:dialog_field_association_validator) { described_class.new }
 
   describe "circular_references" do
-    context "when the associations are blank" do
-      let(:associations) { {} }
+    context "when there are no circular references" do
+      let(:associations) { {"e" => ["c"], "c" => ["a", "d"], "d" => ["a"]} }
+      let(:trivial_associations) { {"a" => ["b"]} }
 
-      it "returns false" do
-        expect(dialog_field_association_validator.circular_references(associations)).to eq(false)
+      it "doesn't blow up and returns nil" do
+      	expect(dialog_field_association_validator.check_for_circular_references({"a" => []} , [])).to eq(nil)
+        expect(dialog_field_association_validator.check_for_circular_references(trivial_associations, "a")).to eq(nil)
+        expect(dialog_field_association_validator.check_for_circular_references(associations, "e")).to eq(nil)
+        expect(dialog_field_association_validator.check_for_circular_references(associations, "c")).to eq(nil)
+        expect(dialog_field_association_validator.check_for_circular_references(associations, "d")).to eq(nil)
       end
     end
 
-    context "when the associations are not blank" do
-      context "when there are no circular references" do
-        it "returns false" do
-          expect(dialog_field_association_validator.circular_references("foo" => ["baz"])).to eq(false)
-          expect(dialog_field_association_validator.circular_references("foo" => %w(foo2 foo4), "foo2" => ["foo3"], "foo3" => ["foo4"])).to eq(false)
-        end
-      end
-
-      context "when there are circular references" do
-        it "returns true on the trivial case" do
-          expect(dialog_field_association_validator.circular_references("foo" => ["baz"], "baz" => ["foo"])).to eq(%w(baz foo))
-        end
-
-        it "returns true on the non-trivial case" do
-          expect(dialog_field_association_validator.circular_references("foo" => %w(foo2 foo4), "foo2" => ["foo3"], "foo3" => %w(foo5 foo4), "foo5" => ["foo2"])).to eq(%w(foo2 foo3))
-        end
+    context "when there are circular references" do
+      let(:trivial_associations) { {"a" => ["b"], "b" => ["a"]} }
+      let(:associations) { {"a" => %w(b d), "b" => ["c"], "c" => %w(e d), "e" => ["b"]} }
+      let(:associations1) { {"a" => %w(b d), "b" => ["c"], "d" => ["a"]} }
 
-        it "returns true on the non-trivial case" do
-          expect(dialog_field_association_validator.circular_references("foo" => %w(foo2 foo4), "foo2" => ["foo3"], "foo4" => ["foo"])).to eq(%w(foo4 foo))
-        end
+      it "raises circular reference error and returns problematic fields" do
+        expect { dialog_field_association_validator.check_for_circular_references(trivial_associations, "a") }.to raise_error(DialogFieldAssociationValidator::DialogFieldAssociationCircularReferenceError, 'a already exists in ["a", "b"]')
+        expect { dialog_field_association_validator.check_for_circular_references(trivial_associations, "b") }.to raise_error(DialogFieldAssociationValidator::DialogFieldAssociationCircularReferenceError, 'b already exists in ["b", "a"]')
+        expect { dialog_field_association_validator.check_for_circular_references(associations, 'a') }.to raise_error(DialogFieldAssociationValidator::DialogFieldAssociationCircularReferenceError, 'b already exists in ["a", "b", "c", "e"]')
+        expect { dialog_field_association_validator.check_for_circular_references(associations1, "a") }.to raise_error(DialogFieldAssociationValidator::DialogFieldAssociationCircularReferenceError, 'a already exists in ["a", "d"]')
+        expect(dialog_field_association_validator.check_for_circular_references(associations1, "b")).to eq(nil)
       end
     end
   end

spec/models/dialog_import_validator_spec.rb

@@ -34,30 +34,19 @@
 
       it "does not raise an error" do
         expect { dialog_import_validator.determine_validity(import_file_upload) }.to_not raise_error
+        expect(dialog_field_association_validator).not_to receive(:check_for_circular_references)
       end
     end
 
-    context "when associations are not blank" do
-      context "when associations are not circular" do
-        let(:uploaded_content) do
-          [{"dialog_tabs" => [{"dialog_groups" => [{"dialog_fields" => [{"name" => "df1", "dialog_field_responders" => "foo"}]}]}]}].to_yaml
-        end
-
-        it "does not raise an error" do
-          allow(dialog_field_association_validator).to receive(:circular_references).and_return(false)
-          expect { dialog_import_validator.determine_validity(import_file_upload) }.to_not raise_error
-        end
-      end
-    end
-
-    context "when associations are circular" do
+    context "when associations present" do
       let(:uploaded_content) do
-        [{"dialog_tabs" => [{"dialog_groups" => [{"dialog_fields" => [{"name" => "df1", "dialog_field_responders" => "foo"}]}]}]}].to_yaml
+        [{"dialog_tabs" => [{"dialog_groups" => [{"dialog_fields" => [{"name" => "df1", "dialog_field_responders" => "foo"}, {"name" => "foo", "dialog_field_responders" => "df1"}]}]}]}].to_yaml
       end
 
-      it "does raise an error" do
-        allow(dialog_field_association_validator).to receive(:circular_references).and_return(%w(df1 df2))
-        expect { dialog_import_validator.determine_validity(import_file_upload) }.to raise_error(DialogImportValidator::DialogFieldAssociationCircularReferenceError)
+      it "calls the circular ref checker" do
+        expect(dialog_field_association_validator).to receive(:check_for_circular_references).at_least(:twice)
+
+        dialog_import_validator.determine_validity(import_file_upload)
       end
     end