simplifies logic and reorganizes methods

most helpful change:
encodes url params before they are put together
which hopefully will mitigate problems we were having
if a param had a "?" character in the string

app/services/api_bridge/query.rb

@@ -6,77 +6,45 @@ module ApiBridge
 
   class Query
     attr_accessor :url
-    attr_accessor :facet_fields
-    attr_accessor :options
-
-    # acceptable for cdrh api
-    # TODO unused currently, use as filter to remove unwanted?
-    @@all_options = %w(
-      f,
-      facet,
-      facet_sort
-      hl,
-      min,
-      num,
-      page,
-      q,
-      sort,
-      start,
-    )
+    attr_accessor :app_facets
+    attr_accessor :app_options
 
     # assumed options upon app startup
-    @@default_options = {
-      "num" => 50,
-      "start" => 0,
-      "facet" => []
-    }
+    @@orchid_default_options = { "num" => 50 }
 
     # create a new query instance with some default settings
     def initialize(url, facets=[], options={})
       if is_url?(url)
         @url = url
-        @facet_fields = facets
         options = remove_rows(options)
-        @options = override_options(@@default_options, options)
-        @options["facet"] = facets
+        # override the orchid defaults with the app query defaults
+        @app_options = @@orchid_default_options.clone.merge(options)
+        # make app default facets available via accessor
+        # and add to the app default request options as well
+        @app_facets = facets
+        @app_options["facet"] = facets
       else
         raise "Provided URL must be valid! #{url}"
       end
     end
 
     # set the start index based on page, start, and num requested parameters
+    # is start is specified, it overrides page
     def calc_start(options)
-      # if start is specified by user then don't override with page
+      # set the page to 1 if the requested parameter is not valid and then remove page
       page = set_page(options["page"])
-      # remove page from options
       options.delete("page")
       if !options.key?("start")
         # use the page and rows to set a start, use default is no num specified
-        num = options.key?("num") ? options["num"].to_i : @options["num"].to_i
+        num = options.key?("num") ? options["num"].to_i : @app_options["num"].to_i
         options["start"] = get_start(page, num)
       end
       options
     end
 
-    # create a version of the request parameters to manipulate
-    # without modifying the original set
-    def clone_and_stringify_options(aOpts)
-      # if params are already a hash, leave them alone,
-      # otherwise assume they are Rails parameters and convert to hash
-      opts_hash = {}
-      if aOpts.class == Hash
-        opts_hash = aOpts.clone
-      elsif aOpts.class == ActionController::Parameters
-        # TODO only allow through "safe" params and convert to hash
-        opts_hash = aOpts.deep_dup.to_unsafe_h
-      end
-      # ensure that there are no symbols hiding in there
-      Hash[opts_hash.map { |k, v| [k.to_s, v] }]
-    end
-
     # url to retrieve a single item
     def create_item_url(id)
-      "#{@url}/item/#{id}"
+      File.join(@url, "item", id)
     end
 
     # url to retrieve multiple items with a query
@@ -87,43 +55,24 @@ def create_items_url(options={})
       query = []
       options.each do |k, v|
         if v.class == Array
-          v.each do |item|
-            query << "#{k}[]=#{item}"
-          end
+          v.each { |item| query << "#{k}[]=#{item}" }
         else
           query << "#{k}=#{v}"
         end
       end
+      query = query.map { |q| encode_param(q) }
       query_string = query.join("&")
       url = "#{@url}/items?#{query_string}"
-      encoded = encode(url)
-      if is_url?(encoded)
-        encoded
+      if is_url?(url)
+        url
       else
-        raise "Invalid URL created for query: #{encoded}"
+        raise "Invalid URL created for query: #{url}"
       end
     end
 
-    # encode the parameters of the URL
-    def 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
-
-    # crudely split query parameters and encode
-    # TODO determine when a parameter includes a ? in the string
-    def encode_query_params(query_string)
-      # puts "QUERY_STRING: #{query_string}"
-      query_string.split(/[&]/).map { |param|
-        name, value = param.split("=")
-
-        "#{name}=#{URI.encode_www_form_component(value)}"
-      }.join("&")
+    def encode_param(param)
+      key, value = param.split("=")
+      "#{key}=#{URI.encode_www_form_component(value)}"
     end
 
     # return a response instance for a particular item id
@@ -147,22 +96,29 @@ def is_url?(url)
       !!result
     end
 
-    # given an existing set of options and a requested set of options
-    # combine them and overwrite any existing with the requested
-    def override_options(existing, requested)
-      existing = clone_and_stringify_options(existing)
-      requested = clone_and_stringify_options(requested)
-      existing.merge(requested)
+    # create a version of the request parameters to manipulate
+    # without modifying the original set
+    # the incoming parameters can either be a hash built in the controller
+    # or parameters passed directly through
+    def params_to_hash(options)
+      opts_hash = {}
+      if options.class == Hash
+        opts_hash = options.clone
+      elsif options.class == ActionController::Parameters
+        # TODO only allow through "safe" params and convert to hash
+        opts_hash = options.deep_dup.to_unsafe_h
+      end
+      # ensure that there are no symbols hiding in there
+      Hash[opts_hash.map { |k, v| [k.to_s, v] }]
     end
 
-    # clone parameters and then remove / manipulate
+    # clone parameters or hash array and then remove / manipulate for api
     def prepare_options(options)
-      opts = clone_and_stringify_options(options)
-      # remove parameters which rails adds
-      opts.delete("controller")
-      opts.delete("action")
-      opts.delete("utf8")
-      opts.delete("commit")
+      # combine the incoming params with the app default options
+      params_hash = params_to_hash(options)
+      opts = @app_options.clone.merge(params_hash)
+      # remove rails specific parameters and reassign "rows"
+      %w[action commit controller utf8].each { |p| opts.delete(p) }
       opts = remove_rows(opts)
       # remove page and replace with start
       opts = calc_start(opts)
@@ -172,15 +128,13 @@ def prepare_options(options)
     end
 
     # pass through parameters to query API and receive response object
-    def query(options={})
-      options = prepare_options(options)
-      # override defaults with requested options
-      req_params = override_options(@options, options)
+    def query(request_options={})
+      options = prepare_options(request_options)
       # create and send the request
-      url = create_items_url(req_params)
+      url = create_items_url(options)
       res = send_request(url)
       # return response format
-      ApiBridge::Response.new(res, url, req_params)
+      ApiBridge::Response.new(res, url, options)
     end
 
     # rows is a deprecated option, please use num instead
@@ -206,14 +160,7 @@ def send_request(url)
 
     # check if the requested page is valid, if not default to 1
     def set_page(page)
-      new_page = 1
-      if !page.nil?
-        is_pos_num = /\A\d+\z/.match(page)
-        if is_pos_num
-          new_page = page.to_i
-        end
-      end
-      new_page
+      page.nil? || !/\A\d+\z/.match(page) ? 1 : page.to_i
     end
 
   end