Merge pull request #6 from luislavena/correctly-split-named-parameters

Correctly split named parameters

CHANGELOG.md

@@ -5,6 +5,12 @@ This project aims to comply with [Semantic Versioning](http://semver.org/),
 so please check *Changed* and *Removed* notes before upgrading.
 
 ## [Unreleased]
+### Fixed
+- No longer split named parameters that share same level (@alsm)
+
+### Changed
+- Attempt to use two named parameters at same level will display a
+  deprecation warning. Future versions will raise `Radix::Tree::SharedKeyError`
 
 ## [0.1.1] - 2016-02-29
 ### Fixed

spec/radix/tree_spec.cr

@@ -1,5 +1,25 @@
 require "../spec_helper"
 
+# Silence deprecation warnings when running specs and allow
+# capture them for inspection.
+module Radix
+  class Tree
+    @show_deprecations = false
+    @stderr : MemoryIO?
+
+    def show_deprecations!
+      @show_deprecations = true
+    end
+
+    private def deprecation(message)
+      if @show_deprecations
+        @stderr ||= MemoryIO.new
+        @stderr.not_nil!.puts message
+      end
+    end
+  end
+end
+
 module Radix
   describe Tree do
     context "a new instance" do
@@ -171,6 +191,63 @@ module Radix
 
           tree.root.children[1].key.should eq("*filepath")
         end
+
+        it "does not split named parameters across shared key" do
+          tree = Tree.new
+          tree.add "/", :root
+          tree.add "/:category", :category
+          tree.add "/:category/:subcategory", :subcategory
+
+          # /                         (:root)
+          # +-:category               (:category)
+          #           \-/:subcategory (:subcategory)
+          tree.root.children.size.should eq(1)
+          tree.root.children[0].key.should eq(":category")
+
+          # inner children
+          tree.root.children[0].children.size.should eq(1)
+          tree.root.children[0].children[0].key.should eq("/:subcategory")
+        end
+
+        it "does not split named parameter marker when only root is shared" do
+          tree = Tree.new
+          tree.add "/", :root
+          tree.add "/:post", :post
+          tree.add "/:category/:post", :category_post
+
+          # /                 (:root)
+          # +-:category/:post (:category_post)
+          # \-:post           (:post)
+          tree.root.children.size.should eq(2)
+          tree.root.children[0].key.should eq(":category/:post")
+          tree.root.children[1].key.should eq(":post")
+        end
+
+        it "displays deprecation warning when two different named parameters share same level" do
+          tree = Tree.new
+          tree.show_deprecations!
+
+          tree.add "/", :root
+          tree.add "/:post", :post
+          tree.add "/:category/:post", :category_post
+
+          stderr = tree.@stderr.not_nil!
+          stderr.rewind
+          message = stderr.gets_to_end
+
+          message.should contain("DEPRECATION WARNING")
+          message.should contain("Tried to place key ':category/:post' at same level as ':post'")
+        end
+
+        pending "does not allow different named parameters sharing same level" do
+          tree = Tree.new
+          tree.add "/", :root
+          tree.add "/:post", :post
+
+          expect_raises Tree::SharedKeyError do
+            tree.add "/:category/:post", :category_post
+          end
+        end
       end
     end
 

src/radix/tree.cr

@@ -53,7 +53,7 @@ module Radix
     # tree.add "/abcxyz", :xyz
     # ```
     #
-    # Nodes inside the tree will be adjusted to accomodate the different
+    # Nodes inside the tree will be adjusted to accommodate the different
     # segments of the given *path*.
     #
     # ```
@@ -73,7 +73,7 @@ module Radix
     # tree.add "/products/featured", :featured
     # ```
     #
-    # Catch all (globbing) and named paramters *path* will be located with
+    # Catch all (globbing) and named parameters *path* will be located with
     # lower priority against other nodes.
     #
     # ```
@@ -125,8 +125,29 @@ module Radix
 
         new_key = path_reader.string.byte_slice(path_reader.pos)
         node.children.each do |child|
-          # compare first character
-          next unless child.key[0]? == new_key[0]?
+          # if child's key starts with named parameter, compare key until
+          # separator (if present).
+          # Otherwise, compare just first character
+          if child.key[0]? == ':' && new_key[0]? == ':'
+            unless _same_key?(new_key, child.key)
+              message = <<-NOTICE
+                DEPRECATION WARNING: Usage of two different named parameters at same level
+                will result in lookup issues and misplaced values.
+
+                Tried to place key '%s' at same level as '%s'.
+
+                Future versions will raise `Radix::Tree::SharedKeyError`.
+
+                See Issue #5 for details:
+                https://github.com/luislavena/radix/issues/5
+                NOTICE
+
+              deprecation message % {new_key, child.key}
+              next
+            end
+          else
+            next unless child.key[0]? == new_key[0]?
+          end
 
           # when found, add to this child
           added = true
@@ -358,5 +379,33 @@ module Radix
 
       count
     end
+
+    # :nodoc:
+    private def _same_key?(path, key)
+      path_reader = Char::Reader.new(path)
+      key_reader = Char::Reader.new(key)
+
+      different = false
+
+      while (path_reader.has_next? && path_reader.current_char != '/') &&
+            (key_reader.has_next? && key_reader.current_char != '/')
+        if path_reader.current_char != key_reader.current_char
+          different = true
+          break
+        end
+
+        path_reader.next_char
+        key_reader.next_char
+      end
+
+      (!key_reader.has_next? && !different) &&
+        (path_reader.current_char == '/' || !path_reader.has_next?)
+    end
+
+    # :nodoc:
+    private def deprecation(message)
+      STDERR.puts message
+      STDERR.flush
+    end
   end
 end