Merge pull request #1 from CDRH/semicolon

hack to get semicolon through to API

lib/api_bridge/helpers.rb

@@ -2,10 +2,6 @@
 
 module ApiBridge
 
-  def self.encode_html string
-    return URI.encode(string)
-  end
-
   def self.calculate_page count, rows
     if count && rows
       return (count.to_f/rows.to_i).ceil
@@ -14,6 +10,16 @@ def self.calculate_page count, rows
     end
   end
 
+  def self.encode url
+    # Split components of URL so we may encode only the query string
+    request_url, query = url.split("?")
+    if query
+      encoded = encode_query_params(query)
+      request_url << "?#{encoded}"
+    end
+    request_url
+  end
+
   def self.escape_values options
     # TODO what kind of escaping or sanitizing do we
     # want to do here?
@@ -62,4 +68,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

lib/api_bridge/query.rb

@@ -51,9 +51,7 @@ def create_items_url options={}
       end
       query_string = query.join("&")
       url = "#{@url}/items?#{query_string}"
-      # TODO apparently things URI.encode uses are deprecated
-      # even though their docs don't mention it guhhhh
-      encoded = URI.encode(url)
+      encoded = ApiBridge.encode(url)
       if ApiBridge.is_url?(encoded)
         return encoded
       else

lib/api_bridge/version.rb

@@ -1,5 +1,5 @@
 module ApiBridge
-  VERSION = "0.0.2"
+  VERSION = "0.0.3"
 
   def self.version
     VERSION

test/helper_test.rb

@@ -3,20 +3,38 @@
 
 class HelperTest < Minitest::Test
 
-  def test_encode_html
-    # TODO look into why the ampersand and quotation marks are not encoded?
-    unencoded = "this is a 'string' & some stuff"
-    encoded = ApiBridge.encode_html(unencoded)
-    assert_equal encoded, "this%20is%20a%20'string'%20&%20some%20stuff"
-  end
-
   def test_calculate_page
     assert_equal ApiBridge.calculate_page(101, 20), 6
     assert_equal ApiBridge.calculate_page(1, 20), 1
     assert_equal ApiBridge.calculate_page(9, 3), 3
     assert_equal ApiBridge.calculate_page(57, 5), 12
   end
 
+  def test_encode
+    fake_url = "http://something.unl.edu/"
+
+    # semicolons
+    assert_equal "#{fake_url}?param=Cather%3B+Pound",
+      ApiBridge.encode("#{fake_url}?param=Cather; Pound")
+    assert_equal "#{fake_url}?param=Cather%2C+Pound",
+      ApiBridge.encode("#{fake_url}?param=Cather, Pound")
+    # multiple types of characters
+    assert_equal "#{fake_url}?f[]=works%7CTitle+%282012%29+Author",
+      ApiBridge.encode("#{fake_url}?f[]=works|Title (2012) Author")
+    # multiple parameters
+    query_in = "f[]=works|Title (1827)&f[]=works|Title2, Author; Author2&q=*"
+    query_out = "f[]=works%7CTitle+%281827%29&f[]=works%7CTitle2%2C+Author%3B+Author2&q=*"
+    assert_equal "#{fake_url}?#{query_out}",
+      ApiBridge.encode("#{fake_url}?#{query_in}")
+    # no parameters
+    assert_equal fake_url, ApiBridge.encode(fake_url)
+  end
+
+  def test_encode_query_params
+    assert_equal "authors=Cather%3B+Pound&f[]=works%7CSomething",
+      ApiBridge.encode_query_params("authors=Cather; Pound&f[]=works|Something")
+  end
+
   def test_escape_values
     # pending
   end
@@ -37,7 +55,7 @@ def test_is_url?
   end
 
   def test_override_options
-
+    # TODO
   end
 
 end

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