Skip to content

Commit

Permalink
Nested params (e.g., "post[title]=Hello") [sinatra#70]
Browse files Browse the repository at this point in the history
This is based largely on manveru's example implementation:

http://paste.linuxhelp.tv/pastes/view/15309

NOTE: we should plan on ripping this out once nested params
makes it into Rack.
  • Loading branch information
foca authored and rtomayko committed Jan 16, 2009
1 parent 4a75d9e commit 1fa9807
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 4 deletions.
4 changes: 4 additions & 0 deletions CHANGES
Expand Up @@ -8,6 +8,10 @@
Documentation on using these features is forth-coming; the following
provides the basic gist: http://gist.github.com/38605

* Parameters with subscripts are now parsed into a nested/recursive
Hash structure. e.g., "post[title]=Hello&post[body]=World" yields
params: {'post' => {'title' => 'Hello', 'body' => 'World'}}.

* Regular expressions may now be used in route pattens; captures are
available at "params[:captures]".

Expand Down
22 changes: 18 additions & 4 deletions lib/sinatra/base.rb
Expand Up @@ -331,8 +331,7 @@ def dispatch!
self.class.filters.each {|block| instance_eval(&block)}
if routes = self.class.routes[@request.request_method]
path = @request.path_info
original_params = Hash.new{ |hash,k| hash[k.to_s] if Symbol === k }
original_params.merge! @request.params
original_params = nested_params(@request.params)

routes.each do |pattern, keys, conditions, method_name|
if pattern =~ path
Expand All @@ -352,8 +351,7 @@ def dispatch!
else
{}
end
@params = original_params.dup
@params.merge!(params)
@params = original_params.merge(params)

catch(:pass) {
conditions.each { |cond|
Expand All @@ -366,6 +364,22 @@ def dispatch!
raise NotFound
end

def nested_params(params)
return indifferent_hash.merge(params) if !params.keys.join.include?('[')
params.inject indifferent_hash do |res, (key,val)|
if key =~ /\[.*\]/
splat = key.scan(/(^[^\[]+)|\[([^\]]+)\]/).flatten.compact
head, last = splat[0..-2], splat[-1]
head.inject(res){ |s,v| s[v] ||= indifferent_hash }[last] = val
end
res
end
end

def indifferent_hash
Hash.new {|hash,key| hash[key.to_s] if Symbol === key }
end

def invoke(handler)
res = catch(:halt) {
if handler.respond_to?(:call)
Expand Down
57 changes: 57 additions & 0 deletions test/routing_test.rb
Expand Up @@ -119,6 +119,63 @@
assert ok?
end

it "supports basic nested params" do
mock_app {
get '/hi' do
params["person"]["name"]
end
}

get "/hi?person[name]=John+Doe"
assert ok?
assert_equal "John Doe", body
end

it "exposes nested params with indifferent hash" do
mock_app {
get '/testme' do
assert_equal 'baz', params['bar']['foo']
assert_equal 'baz', params['bar'][:foo]
'well, alright'
end
}
get '/testme?bar[foo]=baz'
assert_equal 'well, alright', body
end

it "supports deeply nested params" do
input = {
'browser[chrome][engine][name]' => 'V8',
'browser[chrome][engine][version]' => '1.0',
'browser[firefox][engine][name]' => 'spidermonkey',
'browser[firefox][engine][version]' => '1.7.0',
'emacs[map][goto-line]' => 'M-g g',
'emacs[version]' => '22.3.1',
'paste[name]' => 'hello world',
'paste[syntax]' => 'ruby'
}
expected = {
"emacs" => {
"map" => { "goto-line" => "M-g g" },
"version" => "22.3.1"
},
"browser" => {
"firefox" => {"engine" => {"name"=>"spidermonkey", "version"=>"1.7.0"}},
"chrome" => {"engine" => {"name"=>"V8", "version"=>"1.0"}}
},
"paste" => {"name"=>"hello world", "syntax"=>"ruby"}
}
mock_app {
get '/foo' do
assert_equal expected, params
'looks good'
end
}
get "/foo?#{param_string(input)}"
assert ok?
assert_equal 'looks good', body
end

it "supports paths that include spaces" do
mock_app {
get '/path with spaces' do
Expand Down

8 comments on commit 1fa9807

@tj
Copy link
Contributor

@tj tj commented on 1fa9807 Jan 17, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wahoo! you beat me to it :D I was curious as to why Rack does not facilitate this sort of functionality

@josh
Copy link
Contributor

@josh josh commented on 1fa9807 Jan 17, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make some noise on the Rack ML and maybe we can have it in 1.0 ;)

@tj
Copy link
Contributor

@tj tj commented on 1fa9807 Jan 17, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries this will keep me happy. Does really belong their though obviously.

@rtomayko
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, really. Make some noise. I so want to rip this out of Sinatra.

@josh
Copy link
Contributor

@josh josh commented on 1fa9807 Jan 17, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to remove the extension from Rails too :)

Merb and Ramaze do the same thing.

@sporkmonger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider me as clamoring for this to end up as part of Rack as well. Pretty much every framework is doing this with parameters, so by all means, let’s stop duplicating this code.

@tj
Copy link
Contributor

@tj tj commented on 1fa9807 Jan 17, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah wow, Rack is almost 1.0, totally strange if pretty small chunk of code like tis would not make it in, even weirder that its not already there. I could not care less if it is not the best implementation or something but it should at least be there

@josh
Copy link
Contributor

@josh josh commented on 1fa9807 Jan 26, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please voice your opinion!
http://groups.google.com/group/rack-devel/browse_thread/thread/732e2c1e014bde30

Please sign in to comment.