Merge d3f82e0 into 61a8a68

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)

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)

lib/springboard/client/uri.rb

@@ -1,25 +1,17 @@
-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.
       #
       # @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_value(v)
           copy
         end
       end
+
+      def normalize_query_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

spec/springboard/client/resource_spec.rb

@@ -13,46 +13,47 @@
     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
       expect(resource["subpath"].client).to be === resource.client
     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
 
   %w{query params}.each do |method|
     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,46 +71,62 @@
     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
 
     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
 
     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
   end
 
   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

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