Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Deprecate the string version of the route splat. #181

Merged
merged 1 commit into from

3 participants

@seancribbs
Owner

We've talked about doing this for a long time because '*' is a valid URI path
segment. Replacing it with :* makes the difference more explicit.

Sean Cribbs Deprecate the string version of the route splat.
We've talked about doing this for a long time because '*' is a valid
URI path segment. Replacing it with :* makes the difference more
explicit.
4cd87b5
@samwgoldman
Collaborator

:+1:

@seancribbs
Owner

FYI I'm going to go ahead and merge this, we can remove string-splat in 1.3++

@seancribbs seancribbs merged commit 61a1623 into master
@seancribbs seancribbs deleted the match-all-symbol branch
@lgierth
Collaborator

Cool. For reference, discussion was here: #36

@ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 21, 2014
  1. Deprecate the string version of the route splat.

    Sean Cribbs authored
    We've talked about doing this for a long time because '*' is a valid
    URI path segment. Replacing it with :* makes the difference more
    explicit.
This page is out of date. Refresh to see the latest.
View
9 lib/webmachine/dispatcher/route.rb
@@ -22,7 +22,10 @@ class Route
# When used in a path specification, will match all remaining
# segments
- MATCH_ALL = '*'.freeze
+ MATCH_ALL = :*
+
+ # String version of MATCH_ALL, deprecated. Use the symbol instead.
+ MATCH_ALL_STR = '*'.freeze
# Creates a new Route that will associate a pattern to a
# {Resource}.
@@ -65,6 +68,8 @@ def initialize(path_spec, *args)
guards = args
guards << Proc.new if block_given?
+ warn t('match_all_symbol') if path_spec.include? MATCH_ALL_STR
+
@path_spec = path_spec
@guards = guards
@resource = resource
@@ -107,6 +112,8 @@ def bind(tokens, bindings)
case
when spec.empty? && tokens.empty?
return depth
+ when spec == [MATCH_ALL_STR]
+ return [depth, tokens]
when spec == [MATCH_ALL]
return [depth, tokens]
when tokens.empty?
View
1  lib/webmachine/locale/en.yml
@@ -27,3 +27,4 @@ en:
invalid_media_type: "Invalid media type: %{type}"
not_resource_class: "%{class} is not a subclass of Webmachine::Resource"
process_post_invalid: "process_post returned %{result}"
+ match_all_symbol: '"*" as a path segment is deprecated and will be removed in a future release. Please use :*'
View
37 spec/webmachine/dispatcher/route_spec.rb
@@ -1,5 +1,9 @@
require 'spec_helper'
+Webmachine::Dispatcher::Route.class_eval do
+ def warn(*msgs); end # silence warnings for tests
+end
+
describe Webmachine::Dispatcher::Route do
let(:method) { "GET" }
let(:uri) { URI.parse("http://localhost:8080/") }
@@ -7,7 +11,7 @@
let(:resource){ Class.new(Webmachine::Resource) }
describe '#apply' do
- let(:route) {
+ let(:route) {
Webmachine::Dispatcher::Route.new ['hello', :string], resource, {}
}
@@ -38,11 +42,20 @@
end
end
+ it "warns about the deprecated string splat when initializing" do
+ [["*"],["foo", "*"],["foo", :bar, "*"]].each do |path|
+ route = described_class.allocate
+ expect(route).to receive(:warn)
+ route.send :initialize, path, resource, {}
+ end
+ end
+
context "matching a request" do
context "on the root path" do
subject { "/" }
it { is_expected.to match_route([]) }
it { is_expected.to match_route ['*'] }
+ it { is_expected.to match_route [:*] }
it { is_expected.not_to match_route %w{foo} }
it { is_expected.not_to match_route [:id] }
end
@@ -51,10 +64,10 @@
subject { "/foo/bar/baz" }
it { is_expected.to match_route %w{foo bar baz} }
it { is_expected.to match_route ['foo', :id, "baz"] }
- it { is_expected.to match_route %w{foo *} }
- it { is_expected.to match_route [:id, '*'] }
+ it { is_expected.to match_route ['foo', :*] }
+ it { is_expected.to match_route [:id, :*] }
it { is_expected.not_to match_route [] }
- it { is_expected.not_to match_route %w{bar *} }
+ it { is_expected.not_to match_route ['bar', :*] }
end
context "with a guard on the request method" do
@@ -139,6 +152,14 @@ def call(request)
end
context "with a splat" do
+ subject { described_class.new([:*], resource) }
+
+ it "should assign empty path tokens" do
+ expect(request.path_tokens).to eq([])
+ end
+ end
+
+ context "with a deprecated splat string" do
subject { described_class.new(['*'], resource) }
it "should assign empty path tokens" do
@@ -172,6 +193,14 @@ def call(request)
end
context "with a splat" do
+ subject { described_class.new(['foo', :*], resource) }
+
+ it "should capture the path tokens matched by the splat" do
+ expect(request.path_tokens).to eq(%w{ bar baz })
+ end
+ end
+
+ context "with a deprecated splat string" do
subject { described_class.new(%w{foo *}, resource) }
it "should capture the path tokens matched by the splat" do
Something went wrong with that request. Please try again.