BARN-77: validate context override keys; update specs and use shared_examples

Author: cgaroutte

lib/parameter_substitution.rb

@@ -3,6 +3,7 @@
 # See lib/parameter_substitution/readme.md
 
 require 'active_support/all'
+require 'set'
 require "parameter_substitution/context"
 require "parameter_substitution/parse_error"
 require "parameter_substitution/parser"
@@ -63,17 +64,17 @@ def configure
       ParameterSubstitution.config = config
     end
 
-    def find_tokens(string_with_tokens, mapping: {}, context_overrides: nil)
+    def find_tokens(string_with_tokens, mapping: {}, context_overrides: {})
       context = build_context(string_with_tokens, mapping, context_overrides)
       parse_expression(context).substitution_parameter_names
     end
 
-    def find_formatters(string_with_tokens, mapping: {}, context_overrides: nil)
+    def find_formatters(string_with_tokens, mapping: {}, context_overrides: {})
       context = build_context(string_with_tokens, mapping, context_overrides)
       parse_expression(context).method_names
     end
 
-    def find_warnings(string_with_tokens, mapping: {}, context_overrides: nil)
+    def find_warnings(string_with_tokens, mapping: {}, context_overrides: {})
       context = build_context(string_with_tokens, mapping, context_overrides)
       parse_expression(context).parameter_and_method_warnings || []
     end
@@ -83,16 +84,42 @@ def find_warnings(string_with_tokens, mapping: {}, context_overrides: nil)
     # Build context with optional overrides
     # @param [String] string_with_tokens The input string containing tokens
     # @param [Hash] mapping The mapping of parameters to values
-    # @param [Hash, nil] context_overrides Optional overrides for context attributes
+    # @param [Hash] context_overrides Optional overrides for context attributes
     # @return [ParameterSubstitution::Context] The constructed context
+    # @raise [ArgumentError] if context_overrides contains invalid keys
     def build_context(string_with_tokens, mapping, context_overrides)
-      override_options = context_overrides || {}
+      validate_context_overrides!(context_overrides)
+
       base_options = {
         input: string_with_tokens,
         mapping: mapping
       }
 
-      ParameterSubstitution::Context.new(**override_options.merge(base_options))
+      ParameterSubstitution::Context.new(**context_overrides.merge(base_options))
+    end
+
+    # @param [Hash] context_overrides The overrides to validate
+    # @raise [ArgumentError] if context_overrides contains invalid keys
+    def validate_context_overrides!(context_overrides)
+      return if context_overrides.empty?
+
+      valid_keys = %i[
+        required_parameters
+        parameter_start
+        parameter_end
+        destination_encoding
+        allow_unknown_replacement_parameters
+        allow_nil
+        allow_unmatched_parameter_end
+      ].to_set
+
+      invalid_keys = context_overrides.keys.reject { |key| valid_keys.include?(key.to_sym) }
+
+      if invalid_keys.any?
+        invalid_keys_list = invalid_keys.join(", ")
+        valid_keys_list = valid_keys.sort.join(", ")
+        raise ArgumentError, "Invalid context_overrides keys: #{invalid_keys_list}. Valid keys are: #{valid_keys_list}"
+      end
     end
 
 

spec/lib/parameter_substitution_spec.rb

@@ -73,6 +73,44 @@ def self.find(_key); end
       let(:expression) { "<call.start_time.blank_if_nil><do_a_barrel_roll.downcase>" }
       let(:mapping) { { 'call.start_time' => 'hello' } }
 
+      shared_examples "passes context_overrides to Context" do
+        it "calls Context.new with the expected overrides" do
+          allow(ParameterSubstitution::Context).to receive(:new).and_call_original
+          subject
+          expect(ParameterSubstitution::Context).to have_received(:new).once.with(
+            hash_including(test_context_overrides.merge(
+              input: test_expression,
+              mapping: test_mapping
+            ))
+          )
+        end
+      end
+
+      shared_examples "validates context_overrides" do
+        context 'when context_overrides attempts to override base options' do
+          context "when input is provided in context_overrides" do
+            let(:test_context_overrides) { { input: "<different>" } }
+            it "raises error" do
+              expect { subject }.to raise_error(ArgumentError, /Invalid context_overrides keys: input/)
+            end
+          end
+
+          context "when mapping is provided in context_overrides" do
+            let(:test_context_overrides) { { mapping: { 'different' => 'value' } } }
+            it "raises error" do
+              expect { subject }.to raise_error(ArgumentError, /Invalid context_overrides keys: mapping/)
+            end
+          end
+        end
+
+        context 'when context_overrides contains invalid keys' do
+          let(:test_context_overrides) { { invalid_key: "value", another_invalid: "value2" } }
+          it "raises error with list of invalid keys" do
+            expect { subject }.to raise_error(ArgumentError, /Invalid context_overrides keys: invalid_key, another_invalid/)
+          end
+        end
+      end
+
       context "#find_tokens" do
         it "returns tokens up to first dot when no mapping is provided" do
           expect(ParameterSubstitution.find_tokens(expression)).to eq(['call', 'do_a_barrel_roll'])
@@ -83,29 +121,29 @@ def self.find(_key); end
         end
 
         context 'with non-default delimiters' do
-          let(:square_expression) { "[call.start_time.blank_if_nil][do_a_barrel_roll.downcase]" }
-
-          it "returns tokens up to first dot when no mapping is provided" do
-            context_overrides = { parameter_start: "[", parameter_end: "]" }
-            expect(ParameterSubstitution.find_tokens(square_expression, context_overrides: context_overrides)).to eq(['call', 'do_a_barrel_roll'])
-          end
-
-          it "returns tokens that exist in mapping when one is provided" do
-            context_overrides = { parameter_start: "[", parameter_end: "]" }
-            expect(ParameterSubstitution.find_tokens(square_expression, mapping: mapping, context_overrides: context_overrides)).to eq(['call.start_time', 'do_a_barrel_roll'])
+          include_examples "passes context_overrides to Context" do
+            let(:test_expression) { "[call.start_time.blank_if_nil][do_a_barrel_roll.downcase]" }
+            let(:test_mapping) { {} }
+            let(:test_context_overrides) do
+              {
+                required_parameters: ["param1", "param2"],
+                parameter_start: "[",
+                parameter_end: "]",
+                destination_encoding: :json,
+                allow_unknown_replacement_parameters: true,
+                allow_nil: true,
+                allow_unmatched_parameter_end: true
+              }
+            end
+            subject { ParameterSubstitution.find_tokens(test_expression, mapping: test_mapping, context_overrides: test_context_overrides) }
           end
         end
 
-        context 'when context_overrides attempts to override base options' do
-          it "ignores input override and uses method parameter" do
-            context_overrides = { input: "<different>" }
-            expect(ParameterSubstitution.find_tokens(expression, context_overrides: context_overrides)).to eq(['call', 'do_a_barrel_roll'])
-          end
-
-          it "ignores mapping override and uses method parameter" do
-            context_overrides = { mapping: { 'different' => 'value' } }
-            expect(ParameterSubstitution.find_tokens(expression, mapping: mapping, context_overrides: context_overrides)).to eq(['call.start_time', 'do_a_barrel_roll'])
-          end
+        include_examples "validates context_overrides" do
+          let(:test_expression) { expression }
+          let(:test_mapping) { mapping }
+          let(:test_context_overrides) { {} }
+          subject { ParameterSubstitution.find_tokens(test_expression, mapping: test_mapping, context_overrides: test_context_overrides) }
         end
       end
 
@@ -119,29 +157,29 @@ def self.find(_key); end
         end
 
         context 'with non-default delimiters' do
-          let(:square_expression) { "[call.start_time.blank_if_nil][do_a_barrel_roll.downcase]" }
-
-          it "returns all formatters after first dot when no mapping is provided" do
-            context_overrides = { parameter_start: "[", parameter_end: "]" }
-            expect(ParameterSubstitution.find_formatters(square_expression, context_overrides: context_overrides)).to eq(['start_time', 'blank_if_nil', 'downcase'])
-          end
-
-          it "returns formatters after all tokens when mapping is provided" do
-            context_overrides = { parameter_start: "[", parameter_end: "]" }
-            expect(ParameterSubstitution.find_formatters(square_expression, mapping: mapping, context_overrides: context_overrides)).to eq(['blank_if_nil', 'downcase'])
+          include_examples "passes context_overrides to Context" do
+            let(:test_expression) { "[call.start_time.blank_if_nil][do_a_barrel_roll.downcase]" }
+            let(:test_mapping) { {} }
+            let(:test_context_overrides) do
+              {
+                required_parameters: ["param1", "param2"],
+                parameter_start: "[",
+                parameter_end: "]",
+                destination_encoding: :json,
+                allow_unknown_replacement_parameters: true,
+                allow_nil: true,
+                allow_unmatched_parameter_end: true
+              }
+            end
+            subject { ParameterSubstitution.find_formatters(test_expression, mapping: test_mapping, context_overrides: test_context_overrides) }
           end
         end
 
-        context 'when context_overrides attempts to override base options' do
-          it "ignores input override and uses method parameter" do
-            context_overrides = { input: "<different>" }
-            expect(ParameterSubstitution.find_formatters(expression, context_overrides: context_overrides)).to eq(['start_time', 'blank_if_nil', 'downcase'])
-          end
-
-          it "ignores mapping override and uses method parameter" do
-            context_overrides = { mapping: { 'different' => 'value' } }
-            expect(ParameterSubstitution.find_formatters(expression, mapping: mapping, context_overrides: context_overrides)).to eq(['blank_if_nil', 'downcase'])
-          end
+        include_examples "validates context_overrides" do
+          let(:test_expression) { expression }
+          let(:test_mapping) { mapping }
+          let(:test_context_overrides) { {} }
+          subject { ParameterSubstitution.find_formatters(test_expression, mapping: test_mapping, context_overrides: test_context_overrides) }
         end
       end
 
@@ -152,6 +190,26 @@ def self.find(_key); end
         let(:expression_with_bad_params_and_methods) { "<bobby.test1.test2><bobby2.test3.test4>" }
         let(:expression_with_mixed_bad_params_and_methods) { "<bobby.test1.test2><foo.test3.test4>" }
 
+        shared_examples "validates context_overrides for find_warnings" do
+          context 'when context_overrides attempts to override base options' do
+            context "when input is provided in context_overrides" do
+              let(:test_context_overrides) { { input: "<different>" } }
+              it "raises error" do
+                expect { subject }.to raise_error(ArgumentError, /Invalid context_overrides keys: input/)
+              end
+            end
+
+            context "when mapping is provided in context_overrides" do
+              let(:test_expression) { expression_with_bad_params }
+              let(:test_mapping) { {} }
+              let(:test_context_overrides) { { mapping: { 'bobby' => 'value', 'bobby2' => 'value' } } }
+              it "raises error" do
+                expect { subject }.to raise_error(ArgumentError, /Invalid context_overrides keys: mapping/)
+              end
+            end
+          end
+        end
+
         context "when parameters are valid" do
           it "returns empty array" do
             expect(ParameterSubstitution.find_warnings(expression_with_valid_params, mapping: default_mapping))
@@ -181,42 +239,29 @@ def self.find(_key); end
         end
 
         context "with non-default delimiters" do
-          let(:square_expression_with_valid_params) { "[foo]" }
-          let(:square_expression_with_bad_params) { "[bobby][bobby2]" }
-          let(:square_expression_with_bad_methods) { "[foo.test1.test2][foo.test3.test4][black.test1.test2]" }
-
-          it "returns empty array for valid parameters" do
-            context_overrides = { parameter_start: "[", parameter_end: "]" }
-            expect(ParameterSubstitution.find_warnings(square_expression_with_valid_params, mapping: default_mapping, context_overrides: context_overrides))
-              .to eq([])
-          end
-
-          it "returns warnings for invalid parameters" do
-            context_overrides = { parameter_start: "[", parameter_end: "]" }
-            expect(ParameterSubstitution.find_warnings(square_expression_with_bad_params, context_overrides: context_overrides))
-              .to eq(["Unknown param 'bobby'", "Unknown param 'bobby2'"])
-          end
-
-          it "returns warnings for invalid methods" do
-            context_overrides = { parameter_start: "[", parameter_end: "]" }
-            expect(ParameterSubstitution.find_warnings(square_expression_with_bad_methods, mapping: default_mapping, context_overrides: context_overrides))
-              .to eq(["Unknown methods 'test1', 'test2', 'test3', 'test4' used on parameter 'foo'",
-                      "Unknown methods 'test1', 'test2' used on parameter 'black'"])
+          include_examples "passes context_overrides to Context" do
+            let(:test_expression) { "[foo]" }
+            let(:test_mapping) { default_mapping }
+            let(:test_context_overrides) do
+              {
+                required_parameters: ["param1", "param2"],
+                parameter_start: "[",
+                parameter_end: "]",
+                destination_encoding: :json,
+                allow_unknown_replacement_parameters: true,
+                allow_nil: true,
+                allow_unmatched_parameter_end: true
+              }
+            end
+            subject { ParameterSubstitution.find_warnings(test_expression, mapping: test_mapping, context_overrides: test_context_overrides) }
           end
         end
 
-        context 'when context_overrides attempts to override base options' do
-          it "ignores input override and uses method parameter" do
-            context_overrides = { input: "<different>" }
-            expect(ParameterSubstitution.find_warnings(expression_with_valid_params, mapping: default_mapping, context_overrides: context_overrides))
-              .to eq([])
-          end
-
-          it "ignores mapping override and uses method parameter" do
-            context_overrides = { mapping: { 'bobby' => 'value', 'bobby2' => 'value' } }
-            expect(ParameterSubstitution.find_warnings(expression_with_bad_params, mapping: {}, context_overrides: context_overrides))
-              .to eq(["Unknown param 'bobby'", "Unknown param 'bobby2'"])
-          end
+        include_examples "validates context_overrides for find_warnings" do
+          let(:test_expression) { expression_with_valid_params }
+          let(:test_mapping) { default_mapping }
+          let(:test_context_overrides) { {} }
+          subject { ParameterSubstitution.find_warnings(test_expression, mapping: test_mapping, context_overrides: test_context_overrides) }
         end
       end
     end