From 3d0e8de95253f0d10fb568ba938cfd226c2edc6e Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Sat, 30 Jan 2021 20:15:47 -0300 Subject: [PATCH] Remove Radix::Result#key usage in the codebase No longer expose internal functionality used only in tests. Reduce the exposed elements of `Radix::Result` to focus on payload only. Also remove the associated array used to collect all the traversed nodes when performing the lookup. NOTE: this is a breaking change --- CHANGELOG.md | 3 +++ spec/radix/result_spec.cr | 31 ----------------------------- spec/radix/tree_spec.cr | 28 ++++++++------------------ src/radix/result.cr | 41 +++------------------------------------ 4 files changed, 14 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81ddc75..e929196 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ so please check *Changed* and *Removed* notes before upgrading. ## [Unreleased] +### Removed +- Remove `Radix::Result#key` since exposes internal details about structure (breaking change) + ## [0.3.9] - 2019-01-02 ### Fixed - Correct catch-all issue caused when paths differ [#26](https://github.com/luislavena/radix/pull/26) (@silasb) diff --git a/spec/radix/result_spec.cr b/spec/radix/result_spec.cr index 89ad226..d3fb5ee 100644 --- a/spec/radix/result_spec.cr +++ b/spec/radix/result_spec.cr @@ -21,37 +21,6 @@ module Radix end end - describe "#key" do - context "a new instance" do - it "returns an empty key" do - result = Result(Nil).new - result.key.should eq("") - end - end - - context "given one used node" do - it "returns the node key" do - node = Node(Symbol).new("/", :root) - result = Result(Symbol).new - result.use node - - result.key.should eq("/") - end - end - - context "using multiple nodes" do - it "combines the node keys" do - node1 = Node(Symbol).new("/", :root) - node2 = Node(Symbol).new("about", :about) - result = Result(Symbol).new - result.use node1 - result.use node2 - - result.key.should eq("/about") - end - end - end - describe "#use" do it "uses the node payload" do node = Node(Symbol).new("/", :root) diff --git a/spec/radix/tree_spec.cr b/spec/radix/tree_spec.cr index e9a0d69..d24ac95 100644 --- a/spec/radix/tree_spec.cr +++ b/spec/radix/tree_spec.cr @@ -297,7 +297,6 @@ module Radix result = tree.find "/about" result.found?.should be_true - result.key.should eq("/about") result.payload?.should be_truthy result.payload.should eq(:about) end @@ -308,7 +307,6 @@ module Radix result = tree.find "/about/" result.found?.should be_true - result.key.should eq("/about") end it "finds when key contains trailing slash" do @@ -317,7 +315,6 @@ module Radix result = tree.find "/about" result.found?.should be_true - result.key.should eq("/about/") result.payload.should eq(:about) end end @@ -331,7 +328,6 @@ module Radix result = tree.find("/abc") result.found?.should be_true - result.key.should eq("/abc") result.payload.should eq(:abc) end @@ -342,7 +338,6 @@ module Radix result = tree.find("/products") result.found?.should be_true - result.key.should eq("/products") result.payload.should eq(:products) end @@ -356,7 +351,6 @@ module Radix result = tree.find("/blog/tags/") result.found?.should be_true - result.key.should eq("/blog/tags") result.payload.should eq(:tags) end end @@ -370,7 +364,7 @@ module Radix result = tree.find("/日本日本語は難しい/") result.found?.should be_true - result.key.should eq("/日本日本語は難しい") + result.payload.should eq(:japanese_is_difficult) end end @@ -383,7 +377,6 @@ module Radix result = tree.find("/src/file.png") result.found?.should be_true - result.key.should eq("/*filepath") result.payload.should eq(:all) end @@ -406,7 +399,6 @@ module Radix result = tree.find("/search") result.found?.should be_true - result.key.should eq("/search/*extra") result.params.has_key?("extra").should be_true result.params["extra"].empty?.should be_true end @@ -417,7 +409,6 @@ module Radix result = tree.find("/members") result.found?.should be_true - result.key.should eq("/members*trailing") result.params.has_key?("trailing").should be_true result.params["trailing"].empty?.should be_true end @@ -446,7 +437,7 @@ module Radix result = tree.find("/members") result.found?.should be_true - result.key.should eq("/members") + result.payload.should eq(:members) end it "does prefer catch all over specific key with partially shared key" do @@ -456,7 +447,7 @@ module Radix result = tree.find("/orders/cancelled") result.found?.should be_true - result.key.should eq("/orders/*anything") + result.payload.should eq(:orders_catch_all) result.params.has_key?("anything").should be_true result.params["anything"].should eq("cancelled") end @@ -472,7 +463,6 @@ module Radix result = tree.find("/products/10") result.found?.should be_true - result.key.should eq("/products/:id") result.payload.should eq(:product) end @@ -518,7 +508,7 @@ module Radix result = tree.find("/tag-edit2") result.found?.should be_true - result.key.should eq("/tag-edit2") + result.payload.should eq(:alternate_tag_edit) end it "does prefer named parameter over specific key with partially shared key" do @@ -528,7 +518,7 @@ module Radix result = tree.find("/orders/10") result.found?.should be_true - result.key.should eq("/orders/:id") + result.payload.should eq(:specific_order) result.params.has_key?("id").should be_true result.params["id"].should eq("10") end @@ -542,7 +532,6 @@ module Radix result = tree.find("/about/shipping") result.found?.should be_true - result.key.should eq("/:section/:page") result.payload.should eq(:static_page) end @@ -574,17 +563,16 @@ module Radix result = tree.find("/products/1000") result.found?.should be_true - result.key.should eq("/products/:id") result.payload.should eq(:product) result = tree.find("/admin/articles") result.found?.should be_true - result.key.should eq("/*filepath") + result.payload.should eq(:all) result.params["filepath"].should eq("admin/articles") result = tree.find("/products/featured") result.found?.should be_true - result.key.should eq("/products/featured") + result.payload.should eq(:featured) result.payload.should eq(:featured) end end @@ -597,7 +585,7 @@ module Radix result = tree.find "/one-longer/10" result.found?.should be_true - result.key.should eq("/one-longer/:id") + result.payload.should eq(:two) result.params["id"].should eq("10") end end diff --git a/src/radix/result.cr b/src/radix/result.cr index ad0a0bb..05d2f0b 100644 --- a/src/radix/result.cr +++ b/src/radix/result.cr @@ -1,11 +1,11 @@ require "./node" module Radix - # A Result is the comulative output of walking our [Radix tree](https://en.wikipedia.org/wiki/Radix_tree) + # Result present the output of walking our [Radix tree](https://en.wikipedia.org/wiki/Radix_tree) # `Radix::Tree` implementation. # - # It provides helpers to retrieve the information obtained from walking - # our tree using `Radix::Tree#find` + # It provides helpers to retrieve the success (or failure) and the payload + # obtained from walkin our tree using `Radix::Tree#find` # # This information can be used to perform actions in case of the *path* # that was looked on the Tree was found. @@ -20,7 +20,6 @@ module Radix # :nodoc: def initialize - @nodes = [] of Node(T) @params = {} of String => String end @@ -41,45 +40,11 @@ module Radix payload? ? true : false end - # Returns a String built based on the nodes used in the result - # - # ``` - # node1 = Radix::Node(Symbol).new("/", :root) - # node2 = Radix::Node(Symbol).new("about", :about) - # - # result = Radix::Result(Symbol).new - # result.use node1 - # result.use node2 - # - # result.key - # # => "/about" - # ``` - # - # When no node has been used, returns an empty String. - # - # ``` - # result = Radix::Result(Nil).new - # result.key - # # => "" - # ``` - def key - @key ||= begin - String.build { |io| - @nodes.each do |node| - io << node.key - end - } - end - end - # Adjust result information by using the details of the given `Node`. # # * Collect `Node` for future references. # * Use *payload* if present. def use(node : Node(T), payload = true) - # collect nodes - @nodes << node - if payload && node.payload? @payload = node.payload end