From 1284a2d9a48da4a8b912efb1ec2d362b35b8352b Mon Sep 17 00:00:00 2001 From: Mathieu Le Tiec Date: Thu, 3 Mar 2022 14:10:16 +0100 Subject: [PATCH 1/3] Raise an InvalidKeyErrors on substring of valid keys Fixes #705 I'm not completely sure of the fix, but it's probably good enough as a starting point --- .../validation/contract/class_interface.rb | 4 ++- .../contract/class_interface/rule_spec.rb | 34 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/dry/validation/contract/class_interface.rb b/lib/dry/validation/contract/class_interface.rb index b4992e3c..39426031 100644 --- a/lib/dry/validation/contract/class_interface.rb +++ b/lib/dry/validation/contract/class_interface.rb @@ -159,7 +159,9 @@ def ensure_valid_keys(*keys) key_paths = key_paths(keys) invalid_keys = key_paths.map { |(key, path)| - unless valid_paths.any? { |vp| vp.include?(path) || vp.include?("#{path}[]") } + unless valid_paths.any? { |vp| + vp == path || vp.include?("#{path}.") || vp.include?("#{path}[]") + } key end }.compact.uniq diff --git a/spec/integration/contract/class_interface/rule_spec.rb b/spec/integration/contract/class_interface/rule_spec.rb index 4910c204..2d1866db 100644 --- a/spec/integration/contract/class_interface/rule_spec.rb +++ b/spec/integration/contract/class_interface/rule_spec.rb @@ -167,6 +167,40 @@ def self.name end end + context "when keys are substring of valid keys" do + it "raises error with a list of symbol keys" do + expect { contract_class.rule(:details, :addres) } + .to raise_error( + Dry::Validation::InvalidKeysError, + "TestContract.rule specifies keys that are not defined by the schema: [:addres]" + ) + end + + it "raises error with a hash path" do + expect { contract_class.rule(details: :addres) } + .to raise_error( + Dry::Validation::InvalidKeysError, + "TestContract.rule specifies keys that are not defined by the schema: [{:details=>:addres}]" + ) + end + + it "raises error with a dot notation" do + expect { contract_class.rule("details.addres") } + .to raise_error( + Dry::Validation::InvalidKeysError, + 'TestContract.rule specifies keys that are not defined by the schema: ["details.addres"]' + ) + end + + it "raises error with a hash path with multiple nested keys" do + expect { contract_class.rule(details: %i[addres]) } + .to raise_error( + Dry::Validation::InvalidKeysError, + "TestContract.rule specifies keys that are not defined by the schema: [{:details=>[:addres]}]" + ) + end + end + describe "abstract contract" do let(:abstract_contract) do Class.new(Dry::Validation::Contract) do From 852317b562ba532ffe8dbff60876cea7561714f8 Mon Sep 17 00:00:00 2001 From: Mathieu Le Tiec Date: Mon, 14 Mar 2022 14:55:53 +0100 Subject: [PATCH 2/3] Raise InvalidKeysError on suffixes of valid keys --- .../validation/contract/class_interface.rb | 6 ++-- .../contract/class_interface/rule_spec.rb | 36 ++++++++++++++++++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/dry/validation/contract/class_interface.rb b/lib/dry/validation/contract/class_interface.rb index 39426031..a3d4f193 100644 --- a/lib/dry/validation/contract/class_interface.rb +++ b/lib/dry/validation/contract/class_interface.rb @@ -158,13 +158,13 @@ def ensure_valid_keys(*keys) valid_paths = key_map.to_dot_notation key_paths = key_paths(keys) - invalid_keys = key_paths.map { |(key, path)| + invalid_keys = key_paths.filter_map { |(key, path)| unless valid_paths.any? { |vp| - vp == path || vp.include?("#{path}.") || vp.include?("#{path}[]") + vp == path || vp.start_with?("#{path}.", "#{path}[]") } key end - }.compact.uniq + }.uniq return if invalid_keys.empty? diff --git a/spec/integration/contract/class_interface/rule_spec.rb b/spec/integration/contract/class_interface/rule_spec.rb index 2d1866db..81d8a3c3 100644 --- a/spec/integration/contract/class_interface/rule_spec.rb +++ b/spec/integration/contract/class_interface/rule_spec.rb @@ -167,7 +167,7 @@ def self.name end end - context "when keys are substring of valid keys" do + context "when keys are prefixes of valid keys" do it "raises error with a list of symbol keys" do expect { contract_class.rule(:details, :addres) } .to raise_error( @@ -201,6 +201,40 @@ def self.name end end + context "when keys are suffixes of valid keys" do + it "raises error with a list of symbol keys" do + expect { contract_class.rule(:etails, :address) } + .to raise_error( + Dry::Validation::InvalidKeysError, + "TestContract.rule specifies keys that are not defined by the schema: [:etails, :address]" + ) + end + + it "raises error with a hash path" do + expect { contract_class.rule(etails: :address) } + .to raise_error( + Dry::Validation::InvalidKeysError, + "TestContract.rule specifies keys that are not defined by the schema: [{:etails=>:address}]" + ) + end + + it "raises error with a dot notation" do + expect { contract_class.rule("etails.address") } + .to raise_error( + Dry::Validation::InvalidKeysError, + 'TestContract.rule specifies keys that are not defined by the schema: ["etails.address"]' + ) + end + + it "raises error with a hash path with multiple nested keys" do + expect { contract_class.rule(etails: %i[address]) } + .to raise_error( + Dry::Validation::InvalidKeysError, + "TestContract.rule specifies keys that are not defined by the schema: [{:etails=>[:address]}]" + ) + end + end + describe "abstract contract" do let(:abstract_contract) do Class.new(Dry::Validation::Contract) do From 5ede7ce0b2484d1091a746ea4ec6f08da9168e74 Mon Sep 17 00:00:00 2001 From: Mathieu Le Tiec Date: Mon, 14 Mar 2022 15:06:05 +0100 Subject: [PATCH 3/3] Replace double negation --- lib/dry/validation/contract/class_interface.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dry/validation/contract/class_interface.rb b/lib/dry/validation/contract/class_interface.rb index a3d4f193..6c035487 100644 --- a/lib/dry/validation/contract/class_interface.rb +++ b/lib/dry/validation/contract/class_interface.rb @@ -159,9 +159,9 @@ def ensure_valid_keys(*keys) key_paths = key_paths(keys) invalid_keys = key_paths.filter_map { |(key, path)| - unless valid_paths.any? { |vp| - vp == path || vp.start_with?("#{path}.", "#{path}[]") - } + if valid_paths.none? { |vp| + vp == path || vp.start_with?("#{path}.", "#{path}[]") + } key end }.uniq