Replace obsolete URI.encode; Update tests

Supported encoding methods encode far more characters
Split URL passed into individual components
Encode only param values
Skip encoding param names because the [] encoding breaks API requests

Must pass a full URL in tests to use method which splits components
Update tests to reflect different encoding output
- Spaces encode as '+', commas as '%2C'
Remove test for pre-encoded semi-colons
Update assert_equal argument order for modified tests

lib/api_bridge/helpers.rb

@@ -11,11 +11,17 @@ def self.calculate_page count, rows
   end
 
   def self.encode url
-    encoded = URI.encode(url)
-    # semicolons are not caught in the above step
-    encoded = encoded.gsub(";", "%3B")
-    # encoded semicolons may accidentally have had the % double encoded
-    encoded.gsub("%253B", "%3B")
+    # Split components of URL so we may encode only the query string, index 7
+    components = URI.split(url).map.with_index { |component, i|
+      i != 7 || component == "" ? component : encode_query_params(component)
+    }
+
+    # Assume URLs are hierarchical, i.e. (scheme)://... not opaque (scheme):...
+    url = "#{components[0]}://" << components[1..5].join("")
+    url << "?#{components[7]}" if components[7] != ""
+    url << components[8] if components[8]
+
+    url
   end
 
   def self.escape_values options
@@ -66,4 +72,14 @@ def self.set_page page
     end
     return new_page
   end
+
+  private
+
+  def self.encode_query_params(query_string)
+    query_string.split(/[&]/).map { |param|
+      name, value = param.split("=")
+
+      "#{name}=#{URI.encode_www_form_component(value)}"
+    }.join("&")
+  end
 end

test/helper_test.rb

@@ -11,9 +11,12 @@ def test_calculate_page
   end
 
   def test_encode
-    assert_equal "Cather%3B%20Pound", ApiBridge.encode("Cather; Pound")
-    assert_equal "Cather,%20Pound", ApiBridge.encode("Cather, Pound")
-    assert_equal "Cather%3B%20Pound", ApiBridge.encode("Cather%3B Pound")
+    fake_url = "http://something.unl.edu/?param="
+
+    assert_equal "#{fake_url}Cather%3B+Pound",
+      ApiBridge.encode("#{fake_url}Cather; Pound")
+    assert_equal "#{fake_url}Cather%2C+Pound",
+      ApiBridge.encode("#{fake_url}Cather, Pound")
   end
 
   def test_escape_values

test/query_test.rb

@@ -16,12 +16,12 @@ def test_create_item_url
   def test_create_items_url
     # basic
     url = @api.create_items_url()
-    assert_equal url, "#{@fake_url}/items?"
+    assert_equal url, "#{@fake_url}/items"
 
     # with f[] and q
     opts = { "f" => ["thing1", "thing2"], "q" => "water" }
     url = @api.create_items_url(opts)
-    assert_equal url, "#{@fake_url}/items?f[]=thing1&f[]=thing2&q=water"
+    assert_equal "#{@fake_url}/items?f[]=thing1&f[]=thing2&q=water", url
 
     # with f[], facet[], start, and num
     opts = {
@@ -30,17 +30,17 @@ def test_create_items_url
       "start" => 21, "num" => 20
     }
     url = @api.create_items_url(opts)
-    assert_equal url, "#{@fake_url}/items?f[]=subcategory%7Cworks&facet[]=title&facet[]=name&start=21&num=20"
+    assert_equal "#{@fake_url}/items?f[]=subcategory%7Cworks&facet[]=title&facet[]=name&start=21&num=20", url
 
     # with nested f[]
     opts = { "f" => ["creator.name|Willa, Cather"] }
     url = @api.create_items_url(opts)
-    assert_equal url, "#{@fake_url}/items?f[]=creator.name%7CWilla,%20Cather"
+    assert_equal "#{@fake_url}/items?f[]=creator.name%7CWilla%2C+Cather", url
 
     # with multiple sorts
     opts = { "sort" => ["title|asc", "name|desc"] }
     url = @api.create_items_url(opts)
-    assert_equal url, "#{@fake_url}/items?sort[]=title%7Casc&sort[]=name%7Cdesc"
+    assert_equal "#{@fake_url}/items?sort[]=title%7Casc&sort[]=name%7Cdesc", url
   end
 
   def test_reset_options