From 7782fc09a6991035ad5cda9f1bd64271304924a7 Mon Sep 17 00:00:00 2001 From: mecampbellsoup Date: Wed, 29 Mar 2017 13:34:42 -0400 Subject: [PATCH 1/3] Handle non-permitted params when comparing rules to request body --- lib/typed_parameters/comparison.rb | 10 ++++++++++ spec/typed_parameters/comparison_spec.rb | 21 ++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/typed_parameters/comparison.rb b/lib/typed_parameters/comparison.rb index ab9c706..9c3f9fd 100644 --- a/lib/typed_parameters/comparison.rb +++ b/lib/typed_parameters/comparison.rb @@ -1,9 +1,19 @@ module TypedParameters class Comparison + # Example usage: + # Expected types for arguments indicated on the next line. + # Comparison.new(rules_format: RulesFormat, request_body: RequestBody) + # def initialize(rules_format:, request_body:) @rules_format, @request_body = rules_format, request_body end + # Example usage: + # comparison.errors => + # Returns nil when there are no errors when comparing the + # rules_format to the request_body. + # Otherwise, returns an object where each key is the path to the + # problematic parameter and whose value describes the problem. def errors @errors = {} diff --git a/spec/typed_parameters/comparison_spec.rb b/spec/typed_parameters/comparison_spec.rb index 477ff22..30e5783 100644 --- a/spec/typed_parameters/comparison_spec.rb +++ b/spec/typed_parameters/comparison_spec.rb @@ -25,10 +25,25 @@ end context "when the request body does not adhere to the rules format" do - let(:name) { 1_000 } + context "when a non-specified parameter is included in the request body" do + let(:params) { { name: "M@", email: "matt@zipmark.com" } } - it "returns an object describing the invalid attribute" do - expect(subject).to eq({ "name" => "param_must_be_string" }) + # NOTE: pick one of the following 2 test cases as desired behavior + it "ignores the non-whitelisted param" do + expect(subject).to eq nil + end + + it "adds an error for the non-whitelisted param" do + expect(subject).to eq({ "email" => "non_permitted_param" }) + end + end + + context "when the require parameter is of the wrong type" do + let(:name) { 1_000 } + + it "returns an object describing the invalid attribute" do + expect(subject).to eq({ "name" => "param_must_be_string" }) + end end end end From e5f2df3ed04f4c6ab343396c507483127ef8c9a1 Mon Sep 17 00:00:00 2001 From: mecampbellsoup Date: Wed, 29 Mar 2017 15:21:21 -0400 Subject: [PATCH 2/3] non_permitted_param error for params that aren't whitelisted in rules --- lib/typed_params/comparison.rb | 20 +++++++++++++++++--- lib/typed_params/request_body.rb | 8 +------- lib/typed_params/rules_format.rb | 14 ++++---------- spec/typed_params/comparison_spec.rb | 5 ----- 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/lib/typed_params/comparison.rb b/lib/typed_params/comparison.rb index 0617047..76cad7d 100644 --- a/lib/typed_params/comparison.rb +++ b/lib/typed_params/comparison.rb @@ -17,6 +17,19 @@ def initialize(rules_format:, request_body:) def errors @errors = {} + # Iterate over each param path and validate the corresponding rule + request_body_paths.each do |path| + param_with_rule = ParameterWithRule.new( + rule: @rules_format.find(*path), + parameter: @request_body.find(*path) + ) + + error = param_with_rule.validate + path_string = path.join("/") + @errors[path_string] = param_with_rule.validate if error + end + + # Iterate over each rule path and validate the corresponding param rule_paths.each do |path| param_with_rule = ParameterWithRule.new( rule: @rules_format.find(*path), @@ -33,9 +46,8 @@ def errors private - # List of parameters corresponding to the rules locations. - def parameters - @parameters ||= paths.map { |path| @request_body.find(*path) } + def request_body_paths + @request_body.paths end def rule_paths @@ -51,6 +63,8 @@ def initialize(parameter:, rule:) # Return nil if the parameter meets the rule's constraint(s). # Otherwise return a tuple conforming to (path_to_param, reason_not_valid). def validate + return "non_permitted_param" if rule.nil? + if !parameter.value.is_a?(rule.condition) "param_must_be_#{rule.condition.name.downcase}" end diff --git a/lib/typed_params/request_body.rb b/lib/typed_params/request_body.rb index 0e0bc04..c9cad3e 100644 --- a/lib/typed_params/request_body.rb +++ b/lib/typed_params/request_body.rb @@ -17,14 +17,8 @@ def paths # Returns a Parameter instance when the path points to an existing param. # Returns nil otherwise. def find(path) - param = parameter_at(path) + param = @parameters.dig(*path) Parameter.new(path, param) if param end - - private - - def parameter_at(chunks) - @parameters.dig(*chunks) - end end end diff --git a/lib/typed_params/rules_format.rb b/lib/typed_params/rules_format.rb index 0c80d92..d076392 100644 --- a/lib/typed_params/rules_format.rb +++ b/lib/typed_params/rules_format.rb @@ -13,17 +13,11 @@ def paths end # find(["data", "attributes", "name"]) - # Raises an error if `rule` not found. - # Returns a Rule instance when the path points to an existing param. + # Returns a Rule instance when the path points to an existing rule. + # Returns nil otherwise. def find(path) - rule = @rules.dig(*path) || raise(RuleNotFound) - Rule.new(path, rule) - end - - private - - def rule_at(chunks) - @rules.dig(*chunks) + rule = @rules.dig(*path) + Rule.new(path, rule) if rule end end end diff --git a/spec/typed_params/comparison_spec.rb b/spec/typed_params/comparison_spec.rb index 04b7488..43e6c43 100644 --- a/spec/typed_params/comparison_spec.rb +++ b/spec/typed_params/comparison_spec.rb @@ -28,11 +28,6 @@ context "when a non-specified parameter is included in the request body" do let(:params) { { name: "M@", email: "matt@zipmark.com" } } - # NOTE: pick one of the following 2 test cases as desired behavior - it "ignores the non-whitelisted param" do - expect(subject).to eq nil - end - it "adds an error for the non-whitelisted param" do expect(subject).to eq({ "email" => "non_permitted_param" }) end From 92ff7339ba81f11d5b105a18ecb666bb54b1c62d Mon Sep 17 00:00:00 2001 From: mecampbellsoup Date: Wed, 29 Mar 2017 15:24:26 -0400 Subject: [PATCH 3/3] Return nil when RulesFormat#find fails --- spec/typed_params/request_body_spec.rb | 16 +++++++++++----- spec/typed_params/rules_format_spec.rb | 4 +--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/spec/typed_params/request_body_spec.rb b/spec/typed_params/request_body_spec.rb index a2b90f0..536879d 100644 --- a/spec/typed_params/request_body_spec.rb +++ b/spec/typed_params/request_body_spec.rb @@ -23,13 +23,19 @@ TypedParams::Parameter.new(path, parameters['data']['attributes']) end - context "when the path argument mixes symbols and strings" do - let(:path) { [ :data, "attributes" ] } + context "when there a rule at the given path" do + context "when the path argument mixes symbols and strings" do + let(:path) { [ :data, "attributes" ] } - it { is_expected.to eq paramter } - end + it { is_expected.to eq paramter } + end + + context "when there is no rule at the given path" do + let(:path) { %w(foo bar) } - it { is_expected.to eq paramter } + it { is_expected.to eq nil } + end + end end describe "#paths" do diff --git a/spec/typed_params/rules_format_spec.rb b/spec/typed_params/rules_format_spec.rb index 7c75da8..11e9df4 100644 --- a/spec/typed_params/rules_format_spec.rb +++ b/spec/typed_params/rules_format_spec.rb @@ -38,9 +38,7 @@ context "when there is no rule at the given path" do let(:path) { %w(foo bar) } - it "raises an error" do - expect { subject }.to raise_error TypedParams::RuleNotFound - end + it { is_expected.to eq nil } end end end