From d859b9ad24aa2d6d457f23c3b685606994021190 Mon Sep 17 00:00:00 2001 From: Derek Stotz Date: Wed, 22 Aug 2018 21:50:41 -0400 Subject: [PATCH] Removed all dependencies on the Addressable gem --- lib/springboard/client.rb | 7 +-- lib/springboard/client/resource.rb | 16 ++++- lib/springboard/client/uri.rb | 77 +++++++++++++---------- spec/springboard/client/resource_spec.rb | 77 +++++++++++++---------- spec/springboard/client/response_spec.rb | 2 +- spec/springboard/client/uri_spec.rb | 78 +++++++++++++++++++----- spec/springboard/client_spec.rb | 22 ++++++- 7 files changed, 190 insertions(+), 89 deletions(-) diff --git a/lib/springboard/client.rb b/lib/springboard/client.rb index 473aeb9..86913ac 100644 --- a/lib/springboard/client.rb +++ b/lib/springboard/client.rb @@ -30,7 +30,7 @@ class Client DEFAULT_CONNECT_TIMEOUT = 10 ## - # @return [Addressable::URI] The client's base URI + # @return [URI] The client's base URI attr_reader :base_uri ## @@ -80,7 +80,7 @@ def auth(opts={}) unless opts[:username] && opts[:password] raise "Must specify :username and :password" end - body = ::Addressable::URI.form_encode \ + body = ::URI.encode_www_form \ :auth_key => opts[:username], :password => opts[:password] response = post '/auth/identity/callback', body, @@ -236,8 +236,7 @@ def raise_on_fail(response) def prepare_uri(uri) uri = URI.parse(uri) - uri.path = uri.path.gsub(/^#{base_uri.path}/, '') - uri + uri.to_s.gsub(/^#{base_uri.to_s}|^#{base_uri.path}/, '') end def new_response(patron_response) diff --git a/lib/springboard/client/resource.rb b/lib/springboard/client/resource.rb index c457e6a..cc33fe4 100644 --- a/lib/springboard/client/resource.rb +++ b/lib/springboard/client/resource.rb @@ -21,7 +21,7 @@ class Resource # # @return [Addressable::URI] attr_reader :uri - + ## # The underlying Springboard Client. # @@ -31,9 +31,9 @@ class Resource ## # @param [Springboard::Client] client # @param [Addressable::URI, #to_s] uri - def initialize(client, uri) + def initialize(client, uri_or_path) @client = client - @uri = URI.join('/', uri.to_s) + @uri = normalize_uri(uri_or_path) end ## @@ -195,6 +195,16 @@ def exists? private + ## + # Normalizes the URI or path given to a URI + def normalize_uri(uri_or_path) + uri = URI.parse(uri_or_path) + return uri if uri.to_s.start_with?(client.base_uri.to_s) + path = uri_or_path.to_s.start_with?('/') ? uri_or_path : "/#{uri_or_path}" + path.to_s.gsub!(/^#{client.base_uri.to_s}|^#{client.base_uri.path}/, '') + URI.parse("#{client.base_uri}#{path}") + end + ## # Calls a client method, passing the URI as the first argument. def call_client(method, *args, &block) diff --git a/lib/springboard/client/uri.rb b/lib/springboard/client/uri.rb index 3a256f7..29db579 100644 --- a/lib/springboard/client/uri.rb +++ b/lib/springboard/client/uri.rb @@ -1,9 +1,9 @@ -require 'addressable/uri' +require 'uri' module Springboard class Client ## - # A wrapper around Addressable::URI + # A wrapper around URI class URI ## # Returns a URI object based on the parsed string. @@ -11,15 +11,7 @@ class URI # @return [URI] def self.parse(value) return value.dup if value.is_a?(self) - new(::Addressable::URI.parse(value)) - end - - ## - # Joins several URIs together. - # - # @return [URI] - def self.join(*args) - new(::Addressable::URI.join(*args)) + new(::URI.parse(value.to_s)) end ## @@ -46,59 +38,80 @@ def dup def subpath(subpath) uri = dup uri.path = "#{path}/" unless path.end_with?('/') - uri.join subpath.to_s.gsub(/^\//, '') + uri.path = uri.path + ::URI.encode(subpath.to_s.gsub(/^\//, '')) + uri end ## # Merges the given hash of query string parameters and values with the URI's # existing query string parameters (if any). def merge_query_values!(values) - self.springboard_query_values = (self.query_values || {}).merge(normalize_query_hash(values)) + old_query_values = self.query_values || {} + self.query_values = old_query_values.merge(normalize_query_hash(values)) end + ## + # Checks if supplied URI matches current URI + # + # @return [boolean] def ==(other_uri) return false unless other_uri.is_a?(self.class) uri == other_uri.__send__(:uri) end + ## + # Overwrites the query using the supplied query values + def query_values=(values) + self.query = ::URI.encode_www_form(normalize_query_hash(values).sort) + end + + ## + # Returns a hash of query string parameters and values + # + # @return [hash] + def query_values + return nil if query.nil? + ::URI.decode_www_form(query).each_with_object({}) do |(k, v), hash| + if k.end_with?('[]') + k.gsub!(/\[\]$/, '') + hash[k] = Array(hash[k]) + [v] + else + hash[k] = v + end + end + end + private attr_reader :uri - def springboard_query_values=(values) - retval = self.query_values = normalize_query_hash(values) - # Hack to strip digits from Addressable::URI's subscript notation - self.query = self.query.gsub(/\[\d+\]=/, '[]=') - retval - end - def self.delegate_and_wrap(*methods) methods.each do |method| define_method(method) do |*args, &block| - result = @uri.__send__(method, *args, &block) - if result.is_a?(Addressable::URI) - self.class.new(result) - else - result - end + @uri.__send__(method, *args, &block) end end end delegate_and_wrap( - :join, :path, :path=, :form_encode, :to_s, - :query_values, :query_values=, :query, :query= + :path, :path=, :to_s, :query, :query= ) def normalize_query_hash(hash) hash.inject({}) do |copy, (k, v)| - copy[k.to_s] = case v - when Hash then normalize_query_hash(v) - when true, false then v.to_s - else v end + k = "#{k}[]" if v.is_a?(Array) && !k.to_s.end_with?('[]') + copy[k.to_s] = normalize_query_hash_value(v) copy end end + + def normalize_query_hash_value(value) + case value + when Hash then normalize_query_hash(value) + when true, false then value.to_s + when Array then value.uniq + else value end + end end end end diff --git a/spec/springboard/client/resource_spec.rb b/spec/springboard/client/resource_spec.rb index afab912..6bd506e 100644 --- a/spec/springboard/client/resource_spec.rb +++ b/spec/springboard/client/resource_spec.rb @@ -13,7 +13,7 @@ end it "should return a resource with the given subpath appended to its URI" do - expect(resource["subpath"].uri.to_s).to eq("/some/path/subpath") + expect(resource["subpath"].uri.to_s).to eq("#{base_url}/some/path/subpath") end it "should return a resource with the same client instance" do @@ -21,15 +21,13 @@ end it "should accept a symbol as a path" do - expect(resource[:subpath].uri.to_s).to eq("/some/path/subpath") + expect(resource[:subpath].uri.to_s).to eq("#{base_url}/some/path/subpath") end - it "should accept a symbol as a path" do - expect(resource[:subpath].uri.to_s).to eq("/some/path/subpath") - end - - it "should not URI encode the given subpath" do - expect(resource["subpath with spaces"].uri.to_s).to eq("/some/path/subpath with spaces") + it "should URI encode the given subpath" do + expect(resource["subpath with spaces"].uri.to_s).to eq( + "#{base_url}/some/path/subpath%20with%20spaces" + ) end end @@ -37,22 +35,25 @@ describe method do describe "when called with a hash" do it "should set the query string parameters" do - expect(resource.__send__(method, :a => 1, :b => 2).uri.to_s).to eq("/some/path?a=1&b=2") + expect(resource.__send__(method, :a => 1, :b => 2).uri.to_s).to eq("#{base_url}/some/path?a=1&b=2") end it "should URL encode the given keys and values" do - expect(resource.__send__(method, "i have spaces" => "so do i: duh").uri.to_s). - to eq("/some/path?i%20have%20spaces=so%20do%20i%3A%20duh") + expect(resource.__send__(method, "i have spaces" => "so do i: duh").uri.to_s).to eq( + "#{base_url}/some/path?i+have+spaces=so+do+i%3A+duh" + ) end it "should add bracket notation for array parameters" do - expect(resource.__send__(method, :somearray => [1, 2, 3]).uri.to_s).to eq("/some/path?somearray[]=1&somearray[]=2&somearray[]=3") + expect(resource.__send__(method, :somearray => [1, 2, 3]).uri.to_s).to eq( + "#{base_url}/some/path?somearray%5B%5D=1&somearray%5B%5D=2&somearray%5B%5D=3" + ) end it "should return a new resource without modifying the existing URI" do new_resource = resource.query(per_page: 1) - expect(new_resource.uri.to_s).to eq("/some/path?per_page=1") - expect(resource.uri.to_s).to eq("/some/path") + expect(new_resource.uri.to_s).to eq("#{base_url}/some/path?per_page=1") + expect(resource.uri.to_s).to eq("#{base_url}/some/path") end end @@ -70,7 +71,7 @@ describe "when given a hash" do it "should add a _filter query string param" do expect(resource.filter(:a => 1, :b => 2).uri).to eq( - '/some/path?_filter={"a":1,"b":2}'.to_uri + "#{base_url}/some/path?_filter=%7B%22a%22%3A1%2C%22b%22%3A2%7D".to_uri ) end end @@ -78,7 +79,7 @@ describe "when called multiple times" do it "should append args to _filter param as JSON array" do expect(resource.filter(:a => 1).filter(:b => 2).filter(:c => 3).uri).to eq( - '/some/path?_filter=[{"a":1},{"b":2},{"c":3}]'.to_uri + "#{base_url}/some/path?_filter=%5B%7B%22a%22%3A1%7D%2C%7B%22b%22%3A2%7D%2C%7B%22c%22%3A3%7D%5D".to_uri ) end end @@ -86,7 +87,15 @@ describe "when given a string" do it "should add a _filter query string param" do expect(resource.filter('{"a":1,"b":2}').uri).to eq( - '/some/path?_filter={"a":1,"b":2}'.to_uri + "#{base_url}/some/path?_filter=%7B%22a%22%3A1%2C%22b%22%3A2%7D".to_uri + ) + end + end + + describe "when called multiple times with other methods" do + it "should append args to _filter param as JSON array" do + expect(resource.filter(:a => 1).embed(:other).only(:field).filter(:b => 2).uri).to eq( + "#{base_url}/some/path?_filter=%5B%7B%22a%22%3A1%7D%2C%7B%22b%22%3A2%7D%5D&_include%5B%5D=other&_only%5B%5D=field".to_uri ) end end @@ -94,22 +103,30 @@ describe "sort" do it "should set the sort parameter based on the given values" do - expect(resource.sort('f1', 'f2,desc').uri.query).to eq('sort[]=f1&sort[]=f2%2Cdesc') + expect(resource.sort('f1', 'f2,desc').uri.to_s).to eq( + "#{base_url}/some/path?sort%5B%5D=f1&sort%5B%5D=f2%2Cdesc" + ) end it "should replace any existing sort parameter" do resource.sort('f1', 'f2,desc') - expect(resource.sort('f3,asc', 'f4').uri.query).to eq('sort[]=f3%2Casc&sort[]=f4') + expect(resource.sort('f3,asc', 'f4').uri.to_s).to eq( + "#{base_url}/some/path?sort%5B%5D=f3%2Casc&sort%5B%5D=f4" + ) end end describe "only" do it "should set the _only parameter based on the given values" do - expect(resource.only('f1', 'f2').uri.query).to eq('_only[]=f1&_only[]=f2') + expect(resource.only('f1', 'f2').uri.to_s).to eq( + "#{base_url}/some/path?_only%5B%5D=f1&_only%5B%5D=f2" + ) end it "should replace the existing _only parameters" do - expect(resource.only('f1').only('f2', 'f3').uri.query).to eq('_only[]=f2&_only[]=f3') + expect(resource.only('f1').only('f2', 'f3').uri.to_s).to eq( + "#{base_url}/some/path?_only%5B%5D=f2&_only%5B%5D=f3" + ) end end @@ -167,7 +184,7 @@ it "should not modify the original resource URI" do request_stub = stub_request(:get, "#{base_url}/some/path?page=1&per_page=1").to_return(response_data) resource.count - expect(resource.uri.to_s).to eq("/some/path") + expect(resource.uri.to_s).to eq("#{base_url}/some/path") end end @@ -193,38 +210,32 @@ it "should not modify the original resource URI" do request_stub = stub_request(:get, "#{base_url}/some/path?page=1&per_page=1").to_return(response_data) resource.first - expect(resource.uri.to_s).to eq("/some/path") + expect(resource.uri.to_s).to eq("#{base_url}/some/path") end end describe "embed" do it "should support a single embed" do expect(resource.embed(:thing1).uri.to_s).to eq( - '/some/path?_include[]=thing1' + "#{base_url}/some/path?_include%5B%5D=thing1" ) end it "should support multiple embeds" do expect(resource.embed(:thing1, :thing2, :thing3).uri.to_s).to eq( - '/some/path?_include[]=thing1&_include[]=thing2&_include[]=thing3' - ) - end - - it "should merge multiple embed calls" do - expect(resource.embed(:thing1, :thing2).embed(:thing3, :thing4).uri.to_s).to eq( - '/some/path?_include[]=thing1&_include[]=thing2&_include[]=thing3&_include[]=thing4' + "#{base_url}/some/path?_include%5B%5D=thing1&_include%5B%5D=thing2&_include%5B%5D=thing3" ) end it "should merge multiple embed calls" do expect(resource.embed(:thing1, :thing2).embed(:thing3, :thing4).uri.to_s).to eq( - '/some/path?_include[]=thing1&_include[]=thing2&_include[]=thing3&_include[]=thing4' + "#{base_url}/some/path?_include%5B%5D=thing1&_include%5B%5D=thing2&_include%5B%5D=thing3&_include%5B%5D=thing4" ) end it "should merge a call to embed with a manually added _include query param" do expect(resource.query('_include[]' => :thing1).embed(:thing2, :thing3).uri.to_s).to eq( - '/some/path?_include[]=thing1&_include[]=thing2&_include[]=thing3' + "#{base_url}/some/path?_include%5B%5D=thing1&_include%5B%5D=thing2&_include%5B%5D=thing3" ) end end diff --git a/spec/springboard/client/response_spec.rb b/spec/springboard/client/response_spec.rb index bca936e..635eafc 100644 --- a/spec/springboard/client/response_spec.rb +++ b/spec/springboard/client/response_spec.rb @@ -61,7 +61,7 @@ end it "should have the Location header value as its URL" do - expect(response.resource.uri.to_s).to eq('/new/path') + expect(response.resource.uri.to_s).to eq("#{base_url}/new/path") end end diff --git a/spec/springboard/client/uri_spec.rb b/spec/springboard/client/uri_spec.rb index 61491ab..8627775 100644 --- a/spec/springboard/client/uri_spec.rb +++ b/spec/springboard/client/uri_spec.rb @@ -70,40 +70,88 @@ end describe "merge_query_values!" do - it "should call springboard_query_values=" do - uri.query_values = {'a' => '1'} - expect(uri).to receive(:springboard_query_values=).with({'a' => '1', 'b' => '2'}) - uri.merge_query_values! 'b' => '2' + it "should merge the given values with the existing query_values" do + uri.query_values = {'a' => '1', 'b' => '2'} + uri.merge_query_values! 'c' => '3' + expect(uri.query_values).to eq( + {'a' => '1', 'b' => '2', 'c' => '3'} + ) end - it "should merge the given values with the existing query_values" do + it "should overwrite the previous values when a new value is given" do uri.query_values = {'a' => '1', 'b' => '2'} - uri.merge_query_values! 'b' => '20', 'c' => '30' - expect(uri.query_values).to eq({'a' => '1', 'b' => '20', 'c' => '30'}) + uri.merge_query_values! 'a' => '3', 'b' => '4' + expect(uri.query_values).to eq( + {'a' => '3', 'b' => '4'} + ) + end + + it "should overwrite the previous values if a new array is given" do + uri.query_values = {'a' => '1', 'b' => ['2', '3']} + uri.merge_query_values! 'b' => ['4', '5'] + expect(uri.query_values).to eq( + {'a' => '1', 'b' => ['4', '5']} + ) end it "should set the given values if there are no existing query_values" do expect(uri.query_values).to be_nil - uri.merge_query_values! 'b' => '20', 'c' => '30' - expect(uri.query_values).to eq({'b' => '20', 'c' => '30'}) + uri.merge_query_values! 'a' => ['10'], 'b' => '20', 'c' => '30' + expect(uri.query_values).to eq({'a' => ['10'], 'b' => '20', 'c' => '30'}) end end - describe "springboard_query_values=" do + describe "query_values=" do + it "should set the string value for the specified key" do + uri.query_values = {'p1' => '1'} + expect(uri.query_values).to eq({'p1' => '1'}) + end + it "should preserve empty bracket notation for array params" do uri.query = 'sort[]=f1&sort[]=f2' - uri.__send__(:springboard_query_values=, uri.query_values) - expect(uri.to_s).to eq('/relative/path?sort[]=f1&sort[]=f2') + uri.query_values = uri.query_values + expect(uri.to_s).to eq('/relative/path?sort%5B%5D=f1&sort%5B%5D=f2') + end + + it "should stringify symbol keys" do + uri.query_values = {:a => '1'} + expect(uri.query_values).to eq({'a' => '1'}) end it "should stringify boolean param values" do - uri.__send__(:springboard_query_values=, {:p1 => true, :p2 => false}) + uri.query_values = {:p1 => true, :p2 => false} expect(uri.to_s).to eq('/relative/path?p1=true&p2=false') end it "should support hash param values" do - uri.__send__(:springboard_query_values=, {:a => {:b => {:c => 123}}}) - expect(uri.to_s).to eq('/relative/path?a[b][c]=123') + uri.query_values = {:a => {:b => {:c => 123}}} + expect(uri.to_s).to eq( + '/relative/path?a=%7B%22b%22%3D%3E%7B%22c%22%3D%3E123%7D%7D' + ) + end + + it "should add [] to the key for array values" do + uri.query_values = {:a => ['1', '2', '3']} + expect(uri.query).to eq('a%5B%5D=1&a%5B%5D=2&a%5B%5D=3') + end + + it "should remove duplicate values for the same key" do + uri.query_values = {:a => ['1', '1', '2']} + expect(uri.query_values).to eq({'a' => ['1', '2']}) + end + end + + describe "query_values" do + it "should return the current query values" do + uri.query = 'sort[]=f1&sort[]=f2&per_page=all' + uri.query_values = uri.query_values + expect(uri.query_values).to eq({'sort' => ['f1', 'f2'], 'per_page' => 'all'}) + end + + it "should remove [] from array keys" do + uri.query = 'sort[]=f1&sort[]=f2' + uri.query_values = uri.query_values + expect(uri.query_values).to eq({'sort' => ['f1', 'f2']}) end end end diff --git a/spec/springboard/client_spec.rb b/spec/springboard/client_spec.rb index 44b9f36..b648911 100644 --- a/spec/springboard/client_spec.rb +++ b/spec/springboard/client_spec.rb @@ -84,8 +84,28 @@ end describe "[]" do - it "should return a resource object with the given path and client" do + it "should return a resource object with the given path string and client" do expect(client["path"]).to be_a Springboard::Client::Resource + expect(client[:path].uri.to_s).to eq("#{base_url}/path") + end + + it "should return a resource object when given a path as a symbol" do + expect(client[:path]).to be_a Springboard::Client::Resource + expect(client[:path].uri.to_s).to eq("#{base_url}/path") + end + + it "should return a resource object when given a path as a URI" do + uri = 'path'.to_uri + expect(client[uri]).to be_a Springboard::Client::Resource + expect(client[uri].uri.to_s).to eq("#{base_url}/path") + end + + it "should not duplicate the base URI path" do + expect(client['api/subpath'].uri.to_s).to eq("#{base_url}/subpath") + end + + it "should not duplicate the base URI" do + expect(client["#{base_url}/subpath"].uri.to_s).to eq("#{base_url}/subpath") end end