diff --git a/lib/hanami/middleware/node.rb b/lib/hanami/middleware/node.rb index 568fd026..f3695722 100644 --- a/lib/hanami/middleware/node.rb +++ b/lib/hanami/middleware/node.rb @@ -80,7 +80,7 @@ def leaf? # @api private # @since 2.0.3 def variable?(segment) - Router::ROUTE_VARIABLE_MATCHER.match?(segment) + segment.include?(Router::ROUTE_VARIABLE_INDICATOR) end # @api private diff --git a/lib/hanami/router.rb b/lib/hanami/router.rb index 8a961dcb..44fce312 100644 --- a/lib/hanami/router.rb +++ b/lib/hanami/router.rb @@ -745,13 +745,21 @@ def env_for(env, params = {}, options = {}) # @api private PARAMS = "router.params" - # @since 2.0.0 + # @since 2.2.1 # @api private - ROUTE_VARIABLE_MATCHER = /:/ + ROUTE_OPTIONAL_INDICATOR = "(" - # @since 2.0.0 + # @since 2.2.1 + # @api private + ROUTE_VARIABLE_INDICATOR = ":" + + # @since 2.2.1 + # @api private + ROUTE_GLOBBED_INDICATOR = "*" + + # @since 2.2.1 # @api private - ROUTE_GLOBBED_MATCHER = /\*/ + ROUTE_INNER_PARENTHESES_MATCHER = /\(([^()]*)\)/ # Default response when the route method was not allowed # @@ -793,6 +801,8 @@ def add_route(http_method, path, to, as, constraints, &blk) if globbed?(path) add_globbed_route(http_method, path, endpoint, constraints) + elsif optional?(path) + add_optional_routes(http_method, path, to, constraints, &blk) elsif variable?(path) add_variable_route(http_method, path, endpoint, constraints) else @@ -848,16 +858,54 @@ def add_named_route(path, as, constraints) @url_helpers.add(as, Segment.fabricate(path, **constraints)) end + # @since 2.2.1 + # @api private + def add_optional_routes(http_method, path, to, constraints, &blk) + as = nil # avoid creating named routes for all optional permutations + optional_paths = [path] + + optional_paths.each do |optional_path| + match_data = ROUTE_INNER_PARENTHESES_MATCHER.match(optional_path) + + if match_data.nil? + raise InvalidRouteDefinitionError.new(http_method, path, "unmatched parenthesis in route") + end + + optional_routes_from(match_data).each do |new_path| + if optional?(new_path) + optional_paths << new_path + else + add_route(http_method, new_path, to, as, constraints, &blk) + end + end + end + end + + # @since 2.2.1 + # @api private + def optional_routes_from(match_data) + [ + -"#{match_data.pre_match}#{match_data[1]}#{match_data.post_match}", + -"#{match_data.pre_match}#{match_data.post_match}" + ] + end + # @since 2.0.0 # @api private def variable?(path) - ROUTE_VARIABLE_MATCHER.match?(path) + path.include?(ROUTE_VARIABLE_INDICATOR) + end + + # @since 2.2.1 + # @api private + def optional?(path) + path.include?(ROUTE_OPTIONAL_INDICATOR) end # @since 2.0.0 # @api private def globbed?(path) - ROUTE_GLOBBED_MATCHER.match?(path) + path.include?(ROUTE_GLOBBED_INDICATOR) end # @since 2.0.0 diff --git a/lib/hanami/router/errors.rb b/lib/hanami/router/errors.rb index e71bc79d..68910210 100644 --- a/lib/hanami/router/errors.rb +++ b/lib/hanami/router/errors.rb @@ -38,6 +38,21 @@ def initialize(name) end end + # Error raised when a route definition is invalid + # + # @see Hanami::Router#path + # @see Hanami::Router#url + # + # @since 2.2.1 + # @api public + class InvalidRouteDefinitionError < Error + # @since 2.2.1 + # @api private + def initialize(http_method, path, message) + super("Invalid route definition '#{http_method.downcase} #{path}': #{message}") + end + end + # Error raised when variables given for route cannot be expanded into a full path. # # @see Hanami::Router#path diff --git a/lib/hanami/router/leaf.rb b/lib/hanami/router/leaf.rb index df976137..7e1998c0 100644 --- a/lib/hanami/router/leaf.rb +++ b/lib/hanami/router/leaf.rb @@ -13,8 +13,8 @@ class Leaf # @api private # @since 2.2.0 - def initialize(route, to, constraints) - @route = route + def initialize(param_keys, to, constraints) + @param_keys = param_keys.freeze @to = to @constraints = constraints @params = nil @@ -22,20 +22,20 @@ def initialize(route, to, constraints) # @api private # @since 2.2.0 - def match(path) - match = matcher.match(path) + def match(param_values) + @params = @param_keys.zip(param_values).to_h - @params = match.named_captures if match + return true if @constraints.empty? - match - end + @constraints.all? do |key, constraint| + param_value = @params[key.to_s] - private + next true if param_value.nil? # ignore constraints on nonexistent keys - # @api private - # @since 2.2.0 - def matcher - @matcher ||= Mustermann.new(@route, type: :rails, version: "5.0", capture: @constraints) + match = constraint.match(param_value) + + match && (match.string == match[0]) + end end end end diff --git a/lib/hanami/router/node.rb b/lib/hanami/router/node.rb index 588d21c1..552e1bb4 100644 --- a/lib/hanami/router/node.rb +++ b/lib/hanami/router/node.rb @@ -9,10 +9,6 @@ class Router # @api private # @since 2.0.0 class Node - # @api private - # @since 2.0.0 - attr_reader :to - # @api private # @since 2.0.0 def initialize @@ -23,8 +19,9 @@ def initialize # @api private # @since 2.0.0 - def put(segment) + def put(segment, param_keys) if variable?(segment) + param_keys << segment.delete_prefix(Router::ROUTE_VARIABLE_INDICATOR).freeze @variable ||= self.class.new else @fixed ||= {} @@ -34,21 +31,26 @@ def put(segment) # @api private # @since 2.0.0 - def get(segment) - @fixed&.fetch(segment, nil) || @variable + def get(segment, param_values) + fixed = @fixed&.fetch(segment, nil) + return fixed if fixed + + param_values << segment + + @variable end # @api private # @since 2.0.0 - def leaf!(route, to, constraints) + def leaf!(param_keys, to, constraints) @leaves ||= [] - @leaves << Leaf.new(route, to, constraints) + @leaves << Leaf.new(param_keys, to, constraints) end # @api private # @since 2.2.0 - def match(path) - @leaves&.find { |leaf| leaf.match(path) } + def match(param_values) + @leaves&.find { |leaf| leaf.match(param_values) } end private @@ -56,7 +58,7 @@ def match(path) # @api private # @since 2.0.0 def variable?(segment) - Router::ROUTE_VARIABLE_MATCHER.match?(segment) + segment.include?(Router::ROUTE_VARIABLE_INDICATOR) end end end diff --git a/lib/hanami/router/trie.rb b/lib/hanami/router/trie.rb index d46cc5af..611546a3 100644 --- a/lib/hanami/router/trie.rb +++ b/lib/hanami/router/trie.rb @@ -22,32 +22,36 @@ def initialize # @api private # @since 2.0.0 def add(route, to, constraints) - segments = segments_from(route) node = @root + param_keys = [] - segments.each do |segment| - node = node.put(segment) + segments_from(route).each do |segment| + node = node.put(segment, param_keys) end - node.leaf!(route, to, constraints) + node.leaf!(param_keys, to, constraints) end # @api private # @since 2.0.0 def find(path) - segments = segments_from(path) node = @root + param_values = [] - return unless segments.all? { |segment| node = node.get(segment) } + path.slice(1..).split(SEGMENT_SEPARATOR) do |segment| + node = node.get(segment, param_values) - node.match(path)&.then { |found| [found.to, found.params] } + break if node.nil? + end + + node&.match(param_values)&.then { |found| [found.to, found.params] } end private # @api private # @since 2.0.0 - SEGMENT_SEPARATOR = /\// + SEGMENT_SEPARATOR = "/" private_constant :SEGMENT_SEPARATOR # @api private diff --git a/spec/integration/hanami/router/generation_spec.rb b/spec/integration/hanami/router/generation_spec.rb index e5b66daf..252c07f2 100644 --- a/spec/integration/hanami/router/generation_spec.rb +++ b/spec/integration/hanami/router/generation_spec.rb @@ -148,7 +148,7 @@ end end - xit "generates relative and absolute URLs" do + it "generates relative and absolute URLs" do runner.run!([ [:a, "/foo/bar", {var1: "foo", var2: "bar"}], [:a, "/foo", {var1: "foo"}] @@ -177,7 +177,7 @@ end end - xit "generates relative and absolute URLs" do + it "generates relative and absolute URLs" do runner.run!([ [:a, "/var/fooz/baz", {var1: "var", var2: "fooz", var3: "baz"}], [:a, "/var/fooz", {var1: "var", var2: "fooz"}], diff --git a/spec/integration/hanami/router/recognition_spec.rb b/spec/integration/hanami/router/recognition_spec.rb index a69f5340..f45459a6 100644 --- a/spec/integration/hanami/router/recognition_spec.rb +++ b/spec/integration/hanami/router/recognition_spec.rb @@ -305,7 +305,8 @@ end end - describe "fixed with format" do + # FIXME: not supported in 2.2.1 + xdescribe "fixed with format" do let(:router) do described_class.new do get "/test.:format", as: :fixed, to: RecognitionTestCase.endpoint("fixed") @@ -319,7 +320,8 @@ end end - describe "fixed with optional format" do + # FIXME: not supported in 2.2.1 + xdescribe "fixed with optional format" do let(:router) do described_class.new do get "/test(.:format)", as: :fixed, to: RecognitionTestCase.endpoint("fixed") @@ -350,7 +352,8 @@ end end - describe "variable with format" do + # FIXME: not supported in 2.2.1 + xdescribe "variable with format" do let(:router) do described_class.new do get "/:test.:format", as: :variable, to: RecognitionTestCase.endpoint("variable") @@ -364,7 +367,8 @@ end end - describe "variable with optional format" do + # FIXME: not supported in 2.2.1 + xdescribe "variable with optional format" do let(:router) do described_class.new do get "/:test(.:format)", as: :variable, to: RecognitionTestCase.endpoint("variable") @@ -379,7 +383,8 @@ end end - describe "variable with optional constrainted format" do + # FIXME: not supported in 2.2.1 + xdescribe "variable with optional constrainted format" do let(:router) do described_class.new do get "/:test(.:format)", format: /[^.]+/, as: :variable, to: RecognitionTestCase.endpoint("variable") @@ -591,12 +596,8 @@ it "recognizes route(s)" do runner.run!([ - [:regex, "/123/number", {test: "123"}] - # FIXME: this passes if `:greedy` route has the same constraint of the other (`test: /\d+/`) - # this because the returned segment for the two /:test is different because of the contraint. - # this makes Node `@variable` to set them in two different children where the first shadows the latter - # a potential solution could be to use `Segment.new` and implement `#==` - # [:greedy, "/123/anything", { test: "123" }] + [:regex, "/123/number", {test: "123"}], + [:greedy, "/123/anything", {test: "123"}] ]) end end @@ -630,6 +631,84 @@ end end + describe "optional fixed segment" do + let(:router) do + described_class.new do + get "/one(/two)", as: :nested, to: RecognitionTestCase.endpoint("nested") + end + end + + it "recognizes route(s)" do + runner.run!([ + [:nested, "/one"], + [:nested, "/one/two"] + ]) + end + end + + describe "optional variable segment" do + let(:router) do + described_class.new do + get "/one(/:two)", as: :nested, to: RecognitionTestCase.endpoint("nested") + end + end + + it "recognizes route(s)" do + runner.run!([ + [:nested, "/one", {}], + [:nested, "/one/2", {two: "2"}] + ]) + end + end + + describe "optional variable between fixed segments" do + let(:router) do + described_class.new do + get "/one(/:var)/two", as: :nested, to: RecognitionTestCase.endpoint("nested") + end + end + + it "recognizes route(s)" do + runner.run!([ + [:nested, "/one/two", {}], + [:nested, "/one/abc/two", {var: "abc"}] + ]) + end + end + + describe "consecutive optional variables" do + let(:router) do + described_class.new do + get "/one(/:year)(/:month)/two", as: :nested, to: RecognitionTestCase.endpoint("nested") + end + end + + it "recognizes route(s)" do + runner.run!([ + [:nested, "/one/two", {}], + [:nested, "/one/1970/two", {year: "1970"}], + [:nested, "/one/1970/12/two", {year: "1970", month: "12"}] + ]) + end + end + + describe "consecutive optional variables with constraints" do + let(:router) do + described_class.new do + get "/one(/:year)(/:month)", as: :nested, to: RecognitionTestCase.endpoint("nested"), year: /\d{4}/, month: /\d{2}/ + end + end + + it "recognizes route(s)" do + runner.run!([ + [:nested, "/one", {}], + [:nested, "/one/1970", {year: "1970"}], + [:nested, "/one/12", {month: "12"}], + [:nested, "/one/1970/12", {year: "1970", month: "12"}] + ]) + end + end + describe "multiple nested optional fixed segments" do let(:router) do described_class.new do @@ -637,7 +716,7 @@ end end - xit "recognizes route(s)" do + it "recognizes route(s)" do runner.run!([ [:nested, "/one"], [:nested, "/one/two"], @@ -650,7 +729,20 @@ end end - describe "relative fixed with escaped variable" do + describe "invalid optional definition" do + let(:router) do + described_class.new do + get "/one(/two", as: :broken, to: RecognitionTestCase.endpoint("broken") + end + end + + it "raise InvalidRouteDefinitionError" do + expect { runner }.to raise_error(Hanami::Router::InvalidRouteDefinitionError) + end + end + + # FIXME: not supported in 2.2.1 + xdescribe "relative fixed with escaped variable" do let(:router) do described_class.new do get "test\\:variable", as: :escaped, to: RecognitionTestCase.endpoint("escaped") @@ -664,7 +756,8 @@ end end - describe "relative fixed with escaped optional variable" do + # FIXME: not supported in 2.2.1 + xdescribe "relative fixed with escaped optional variable" do let(:router) do described_class.new do get "test\\(:variable\\)", as: :escaped, to: RecognitionTestCase.endpoint("escaped") @@ -706,7 +799,8 @@ end end - describe "variable sourrounded by fixed tokens in the same segment" do + # FIXME: not supported in 2.2.1 + xdescribe "variable sourrounded by fixed tokens in the same segment" do let(:router) do described_class.new do get "/one-:variable-time", as: :variable, to: RecognitionTestCase.endpoint("variable") @@ -720,7 +814,8 @@ end end - describe "constrainted variable sourrounded by fixed tokens in the same segment" do + # FIXME: not supported in 2.2.1 + xdescribe "constrainted variable sourrounded by fixed tokens in the same segment" do let(:router) do described_class.new do get "/one-:variable-time", as: :variable, variable: /\d+/, to: RecognitionTestCase.endpoint("variable") @@ -735,7 +830,8 @@ end end - describe "variable sourrounded by fixed token and format in the same segment" do + # FIXME: not supported in 2.2.1 + xdescribe "variable sourrounded by fixed token and format in the same segment" do let(:router) do described_class.new do get "hey.:greed.html", as: :variable, to: RecognitionTestCase.endpoint("variable") @@ -749,7 +845,8 @@ end end - describe "multiple routes with variables in the same segment" do + # FIXME: not supported in 2.2.1 + xdescribe "multiple routes with variables in the same segment" do let(:router) do described_class.new do get "/:v1-:v2-:v3-:v4-:v5-:v6", as: :var6, to: RecognitionTestCase.endpoint("var6") @@ -773,7 +870,8 @@ end end - context "variable sourrounded by fixed token and format in the same segment" do + # FIXME: not supported in 2.2.1 + xcontext "variable sourrounded by fixed token and format in the same segment" do let(:router) do described_class.new do get "/:common_variable.:matched", as: :regex, to: RecognitionTestCase.endpoint("regex"), matched: /\d+/ diff --git a/spec/unit/hanami/router/leaf_spec.rb b/spec/unit/hanami/router/leaf_spec.rb index 0a8a81d2..a9e0db0a 100644 --- a/spec/unit/hanami/router/leaf_spec.rb +++ b/spec/unit/hanami/router/leaf_spec.rb @@ -3,60 +3,81 @@ require "hanami/router/leaf" RSpec.describe Hanami::Router::Leaf do - let(:subject) { described_class.new(route, to, constraints) } - let(:route) { "/test/route" } - let(:to) { "test proc" } - let(:constraints) { {} } + let(:subject) { described_class.new(param_keys, to, constraints) } + let(:to) { "test proc" } describe "#initialize" do + let(:param_keys) { [] } + let(:constraints) { {} } + it "returns a #{described_class} instance" do - expect(subject).to be_kind_of(described_class) + result = subject + + expect(result).to be_kind_of(described_class) end - end - describe "#to" do - it "returns the endpoint passed as 'to' when initialized" do - expect(subject.to).to eq(to) + it "sets :to attribute to target action" do + result = subject.to + + expect(result).to eq("test proc") + end + + it "sets :params attribute to nil" do + result = subject.params + + expect(result).to be_nil end end describe "#match" do - context "when path matches route" do - let(:matching_path) { route } + let(:param_keys) { ["variable"] } + let(:param_values) { ["value"] } + + context "with no constraints" do + let(:constraints) { {} } it "returns true" do - expect(subject.match(matching_path)).to be_truthy + result = subject.match(param_values) + + expect(result).to be_truthy + end + + it "sets captured params" do + leaf = subject + leaf.match(param_values) + + result = leaf.params + + expect(result).to eq({"variable" => "value"}) end end - context "when path doesn't match route" do - let(:non_matching_path) { "/bad/path" } + context "with valid constraint" do + let(:constraints) { {variable: /\w+/} } it "returns true" do - expect(subject.match(non_matching_path)).to be_falsey + result = subject.match(param_values) + + expect(result).to be_truthy end - end - end - describe "#params" do - context "without previously calling #match(path)" do - it "returns nil" do - params = subject.params + it "sets captured params" do + leaf = subject + leaf.match(param_values) + + result = leaf.params - expect(params).to be_nil + expect(result).to eq({"variable" => "value"}) end end - context "with variable path" do - let(:route) { "test/:route" } - let(:matching_path) { "test/path" } - let(:matching_params) { {"route" => "path"} } + context "with invalid constraint" do + let(:constraints) { {variable: /\d+/} } - it "returns captured params" do - subject.match(matching_path) - params = subject.params + it "returns false" do + result = subject.match(param_values) - expect(params).to eq(matching_params) + expect(result).to be_falsey end end end diff --git a/spec/unit/hanami/router/node_spec.rb b/spec/unit/hanami/router/node_spec.rb index 2c8afbc6..0eeae72e 100644 --- a/spec/unit/hanami/router/node_spec.rb +++ b/spec/unit/hanami/router/node_spec.rb @@ -6,7 +6,34 @@ RSpec.describe Hanami::Router::Node do describe "#initialize" do it "returns a #{described_class} instance" do - expect(subject).to be_kind_of(described_class) + result = subject + + expect(result).to be_kind_of(described_class) + end + end + + describe "#put" do + context "when segment is fixed" do + it "does not update param_keys" do + segment = "foo" + param_keys = [] + + subject.put(segment, param_keys) + + expect(param_keys).to be_empty + end + end + + context "when segment is variable" do + it "updates param_keys" do + variable_segment = ":foo" + key_for_variable_segment = "foo" + param_keys = [] + + subject.put(variable_segment, param_keys) + + expect(param_keys).to include(key_for_variable_segment) + end end end @@ -15,18 +42,56 @@ context "and segment is fixed" do it "returns a node" do segment = "foo" - subject.put(segment) + param_keys = [] + param_values = [] + + subject.put(segment, param_keys) - expect(subject.get(segment)).to be_kind_of(described_class) + result = subject.get(segment, param_values) + + expect(result).to be_kind_of(described_class) + end + + it "does not update param_values" do + segment = "foo" + param_keys = [] + param_values = [] + + subject.put(segment, param_keys) + subject.get(segment, param_values) + + result = param_values + + expect(result).to be_empty end end context "and segment is variable" do it "returns a node" do - dynamic_segment = ":foo" - subject.put(dynamic_segment) + variable_segment = ":foo" + param_keys = [] + path_segment = "bar" + param_values = [] + + subject.put(variable_segment, param_keys) + + result = subject.get(path_segment, param_values) + + expect(result).to be_kind_of(described_class) + end - expect(subject.get("bar")).to be_kind_of(described_class) + it "update param_values" do + variable_segment = ":foo" + param_keys = [] + path_segment = "bar" + param_values = [] + + subject.put(variable_segment, param_keys) + subject.get(path_segment, param_values) + + result = param_values + + expect(result).to include("bar") end end end @@ -34,66 +99,66 @@ context "when segment is not found" do it "returns nil" do segment = "foo" - subject.put(segment) + param_keys = [] + path_segment = "bar" + param_values = [] + + subject.put(segment, param_keys) - expect(subject.get("bar")).to be_nil + result = subject.get(path_segment, param_values) + + expect(result).to be_nil end end end - describe "#match" do - context "when segment is fixed" do - context "and match not found" do - it "returns nil" do - segment = "foo" - route = "/foo" - to = double("to") - constraints = {} - path = "/bar" - subject.put(segment).leaf!(route, to, constraints) + describe "#leaf!" do + context "when called" do + it "returns a Leaf object" do + segment = "foo" + to = "target action" + constraints = {} + param_keys = [] + param_values = [] - expect(subject.get(segment).match(path)).to be_nil - end - end + subject.put(segment, param_keys).leaf!(param_keys, to, constraints) - context "and match is found" do - it "returns a Leaf object" do - segment = "foo" - route = "/foo" - to = double("to") - constraints = {} - subject.put(segment).leaf!(route, to, constraints) + result = subject.get(segment, param_values).match(param_values) - expect(subject.get(segment).match(route)).to be_kind_of(Hanami::Router::Leaf) - end + expect(result).to be_kind_of(Hanami::Router::Leaf) end end + end - context "when segment is variable" do - context "and match not found" do - it "returns nil" do - segment = ":foo" - route = "/:foo" - to = double("to") - constraints = {foo: :digit} - path = "/bar" - subject.put(segment).leaf!(route, to, constraints) - - expect(subject.get(segment).match(path)).to be_nil - end + describe "#match" do + context "when @leaves collection is empty" do + it "returns nil" do + segment = "foo" + param_keys = [] + param_values = [] + + subject.put(segment, param_keys) + + result = subject.get(segment, param_values).match(param_values) + + expect(result).to be_nil end + end + + context "when @leaves collection contains a match" do + it "returns the matching leaf" do + segment = "foo" + to = "target action" + constraints = {} + param_keys = [] + param_values = [] - context "and match found" do - it "returns Leaf object" do - segment = ":foo" - route = "/:foo" - to = double("to") - constraints = {foo: :digit} - path = "/123" - subject.put(segment).leaf!(route, to, constraints) + subject.put(segment, param_keys).leaf!(param_keys, to, constraints) + leaf = subject.get(segment, param_values).match(param_values) - expect(subject.get(segment).match(path)).to be_kind_of(Hanami::Router::Leaf) - end + result = leaf.to + + expect(result).to be("target action") end end end diff --git a/spec/unit/hanami/router/trie_spec.rb b/spec/unit/hanami/router/trie_spec.rb index 7bd72d49..5739ba0b 100644 --- a/spec/unit/hanami/router/trie_spec.rb +++ b/spec/unit/hanami/router/trie_spec.rb @@ -18,7 +18,7 @@ let(:foo) { double("foo") } let(:bar) { double("bar") } let(:baz) { double("baz") } - let(:foo_constraints) { {foo: :digit} } + let(:foo_constraints) { {foo: /\d+/} } let(:empty_constraints) { {} } it "matches path with variable segment and matching constraints" do