Skip to content

Commit ba68395

Browse files
committed
Report all missing path parameters, not just the first
This makes it possible to take remedial action such as copying the parameters from the parent. Even without this change, it is no longer necessary to symbolize the keys or duplicate the attributes in request_path.
1 parent f6ea2bc commit ba68395

File tree

4 files changed

+22
-13
lines changed

4 files changed

+22
-13
lines changed

lib/her/errors.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
module Her
22
module Errors
33
class PathError < StandardError
4-
attr_reader :missing_parameter
4+
attr_reader :missing_parameters
55

6-
def initialize(message, missing_parameter=nil)
6+
def initialize(message, missing_parameters=nil)
77
super(message)
8-
@missing_parameter = missing_parameter
8+
@missing_parameters = missing_parameters
99
end
1010
end
1111

lib/her/model/introspection.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ def inspect
1515
resource_path = begin
1616
request_path
1717
rescue Her::Errors::PathError => e
18-
"<unknown path, missing `#{e.missing_parameter}`>"
18+
joined = e.missing_parameters.map { |m| "`#{m}`" }.join(", ")
19+
"<unknown path, missing #{joined}>"
1920
end
2021

2122
"#<#{self.class}(#{resource_path}) #{attributes.keys.map { |k| "#{k}=#{attribute_for_inspect(send(k))}" }.join(" ")}>"

lib/her/model/paths.rb

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ module Paths
1515
# @param [Hash] params An optional set of additional parameters for
1616
# path construction. These will not override attributes of the resource.
1717
def request_path(params = {})
18-
self.class.build_request_path(params.merge(attributes.dup))
18+
self.class.build_request_path(params.merge(attributes))
1919
end
2020

2121
module ClassMethods
@@ -104,10 +104,18 @@ def build_request_path(path=nil, parameters={})
104104
path.gsub!(/(\A|\/):id(\Z|\/)/, "\\1:#{primary_key}\\2")
105105
end
106106

107-
path.gsub(/:([\w_]+)/) do
108-
# Look for :key or :_key, otherwise raise an exception
109-
value = $1.to_sym
110-
parameters.delete(value) || parameters.delete(:"_#{value}") || raise(Her::Errors::PathError.new("Missing :_#{$1} parameter to build the request path. Path is `#{path}`. Parameters are `#{parameters.symbolize_keys.inspect}`.", $1))
107+
missing = []
108+
109+
result = path.gsub(/:([\w_]+)/) do
110+
# Look for "key" or "_key", otherwise add to the missing list and raise below
111+
parameters[$1] || parameters["_#{$1}"] || (missing.push($1) && $1)
112+
end
113+
114+
if missing.empty?
115+
result
116+
else
117+
joined = missing.map { |m| ":_#{m}" }.join(", ")
118+
raise Her::Errors::PathError.new("Missing #{joined} parameters to build the request path. Path is `#{path}`. Parameters are `#{parameters.symbolize_keys.inspect}`.", missing)
111119
end
112120
end
113121

spec/model/paths_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@
6060
end
6161

6262
it "raises exceptions when building a path without required custom variables" do
63-
Foo::User.collection_path "/organizations/:organization_id/utilisateurs"
64-
expect { Foo::User.build_request_path(:id => "foo") }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameter to build the request path. Path is `/organizations/:organization_id/utilisateurs/:id`. Parameters are `{:id=>\"foo\"}`.")
63+
Foo::User.collection_path "/organizations/:organization_id/departments/:department_id/utilisateurs"
64+
expect { Foo::User.build_request_path(:id => "foo") }.to raise_error(Her::Errors::PathError, "Missing :_organization_id, :_department_id parameters to build the request path. Path is `/organizations/:organization_id/departments/:department_id/utilisateurs/:id`. Parameters are `{:id=>\"foo\"}`.")
6565
end
6666
end
6767
end
@@ -115,12 +115,12 @@
115115

116116
it "raises exceptions when building a path without required custom variables" do
117117
Foo::AdminUser.collection_path "/organizations/:organization_id/users"
118-
expect { Foo::AdminUser.build_request_path(:id => "foo") }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameter to build the request path. Path is `/organizations/:organization_id/users/:id`. Parameters are `{:id=>\"foo\"}`.")
118+
expect { Foo::AdminUser.build_request_path(:id => "foo") }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameters to build the request path. Path is `/organizations/:organization_id/users/:id`. Parameters are `{:id=>\"foo\"}`.")
119119
end
120120

121121
it "raises exceptions when building a relative path without required custom variables" do
122122
Foo::AdminUser.collection_path "organizations/:organization_id/users"
123-
expect { Foo::AdminUser.build_request_path(:id => "foo") }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameter to build the request path. Path is `organizations/:organization_id/users/:id`. Parameters are `{:id=>\"foo\"}`.")
123+
expect { Foo::AdminUser.build_request_path(:id => "foo") }.to raise_error(Her::Errors::PathError, "Missing :_organization_id parameters to build the request path. Path is `organizations/:organization_id/users/:id`. Parameters are `{:id=>\"foo\"}`.")
124124
end
125125
end
126126
end

0 commit comments

Comments
 (0)