From 4df8fba3f489286e2956145645c4b5875daa9d33 Mon Sep 17 00:00:00 2001
From: Alex Reisner <alex@alexreisner.com>
Date: Thu, 13 Mar 2014 13:46:13 -0400
Subject: [PATCH] Fix handling of zip-code-only query results.

Also much code cleanup.
---
 lib/geocoder/lookups/smarty_streets.rb   | 11 ++-
 lib/geocoder/results/smarty_streets.rb   | 86 +++++++++++++++---------
 test/fixtures/smarty_streets_11211       |  1 +
 test/unit/lookups/smarty_streets_test.rb | 16 +++--
 4 files changed, 70 insertions(+), 44 deletions(-)
 create mode 100644 test/fixtures/smarty_streets_11211

diff --git a/lib/geocoder/lookups/smarty_streets.rb b/lib/geocoder/lookups/smarty_streets.rb
index 574f8e2e..118a7f0b 100644
--- a/lib/geocoder/lookups/smarty_streets.rb
+++ b/lib/geocoder/lookups/smarty_streets.rb
@@ -12,16 +12,14 @@ module Geocoder::Lookup
     end
 
     def query_url(query)
-      return "#{protocol}://api.smartystreets.com/zipcode?#{url_query_string(query)}" if zipcode_only?(query)
-      "#{protocol}://api.smartystreets.com/street-address?#{url_query_string(query)}"
+      path = zipcode_only?(query) ? "zipcode" : "street-address"
+      "#{protocol}://api.smartystreets.com/#{path}?#{url_query_string(query)}"
     end
 
     private # ---------------------------------------------------------------
 
     def zipcode_only?(query)
-      return false if query.text.is_a?(Array)
-      return true unless (query.to_s.strip =~ /\A[\d]{5}(-[\d]{4})?\Z/).nil?
-      false
+      !query.text.is_a?(Array) and query.to_s.strip =~ /\A\d{5}(-\d{4})?\Z/
     end
 
     def query_url_params(query)
@@ -41,8 +39,7 @@ module Geocoder::Lookup
     end
 
     def results(query)
-      return [] unless doc = fetch_data(query)
-      doc
+      fetch_data(query) || []
     end
   end
 end
diff --git a/lib/geocoder/results/smarty_streets.rb b/lib/geocoder/results/smarty_streets.rb
index 82f7388f..e9a88c52 100644
--- a/lib/geocoder/results/smarty_streets.rb
+++ b/lib/geocoder/results/smarty_streets.rb
@@ -9,78 +9,98 @@ require 'geocoder/lookups/base'
 module Geocoder::Result
   class SmartyStreets < Base
     def coordinates
-      %w(latitude longitude).map{|i| (zipcode_result? && zipcodes.first[i]) ||
-                                      metadata[i] }
+      %w(latitude longitude).map do |i|
+        (zipcode_result? && zipcodes.first[i]) || metadata[i]
+      end
     end
 
     def address
-      begin
-        line_2 = delivery_line_2
-      rescue KeyError
-        line_2 = ''
-      end
-      "#{delivery_line_1} #{line_2} #{last_line}".strip.gsub("  ", " ") || nil
+      [
+        delivery_line_1,
+        delivery_line_2,
+        last_line
+      ].select{ |i| i.to_s != "" }.join(" ")
     end
 
     def state
-      components['state_abbreviation']
-    rescue KeyError
-      city_states.first['state']
+      components['state_abbreviation'] || city_states.first['state']
     end
 
     def state_code
-      city_states.first['state_abbreviation']
-    rescue KeyError
-      state
+      if cs = city_states.first
+        cs['state_abbreviation']
+      else
+        state
+      end
     end
 
     def country
+      # SmartyStreets returns results for USA only
+      "United States"
+    end
+
+    def country_code
       # SmartyStreets returns results for USA only
       "US"
     end
-    alias_method :country_code, :country
 
-## Extra methods not in base.rb ------------------------
+    ## Extra methods not in base.rb ------------------------
 
     def street
       components['street_name']
-    rescue KeyError
-      nil
     end
 
     def city
-      components['city_name']
-    rescue KeyError
-      city_states.first['city']
+      components['city_name'] || city_states.first['city']
     end
 
     def zipcode
-      components['zipcode']
-    rescue KeyError
-      zipcodes.first['zipcode']
+      components['zipcode'] || zipcodes.first['zipcode']
     end
     alias_method :postal_code, :zipcode
 
     def zip4
       components['plus4_code']
-    rescue KeyError
-      nil
     end
     alias_method :postal_code_extended, :zip4
 
     def fips
-      metadata['county_fips']
-    rescue KeyError
-      zipcodes.first['county_fips']
+      metadata['county_fips'] || zipcodes.first['county_fips']
     end
 
     def zipcode_result?
-      zipcodes.any? rescue false
+      zipcodes.any?
     end
 
-    # References to `components`, `metadata`, `city_states`, `zipcodes` land here.
-    def method_missing(meth, *args, &block)
-      @data.fetch(meth.to_s)
+    [
+      :delivery_line_1,
+      :delivery_line_2,
+      :last_line,
+      :delivery_point_barcode,
+      :addressee
+    ].each do |m|
+      define_method(m) do
+        @data[m.to_s] || ''
+      end
+    end
+
+    [
+      :components,
+      :metadata,
+      :analysis
+    ].each do |m|
+      define_method(m) do
+        @data[m.to_s] || {}
+      end
+    end
+
+    [
+      :city_states,
+      :zipcodes
+    ].each do |m|
+      define_method(m) do
+        @data[m.to_s] || []
+      end
     end
   end
 end
diff --git a/test/fixtures/smarty_streets_11211 b/test/fixtures/smarty_streets_11211
new file mode 100644
index 00000000..da5745ed
--- /dev/null
+++ b/test/fixtures/smarty_streets_11211
@@ -0,0 +1 @@
+[{"input_index":0,"city_states":[{"city":"Brooklyn","state_abbreviation":"NY","state":"New York"}],"zipcodes":[{"zipcode":"11211","zipcode_type":"S","county_fips":"36047","county_name":"Kings","latitude":40.71184,"longitude":-73.95288}]}]
\ No newline at end of file
diff --git a/test/unit/lookups/smarty_streets_test.rb b/test/unit/lookups/smarty_streets_test.rb
index 5069355f..f797f21f 100644
--- a/test/unit/lookups/smarty_streets_test.rb
+++ b/test/unit/lookups/smarty_streets_test.rb
@@ -1,4 +1,5 @@
 # encoding: utf-8
+$: << File.join(File.dirname(__FILE__), "..", "..")
 require 'test_helper'
 
 class SmartyStreetsTest < GeocoderTestCase
@@ -16,17 +17,17 @@ class SmartyStreetsTest < GeocoderTestCase
 
   def test_query_for_address_geocode
     query = Geocoder::Query.new("42 Wallaby Way Sydney, AU")
-    assert_not_nil query.url =~ /api\.smartystreets\.com\/street-address\?/
+    assert_match /api\.smartystreets\.com\/street-address\?/, query.url
   end
 
   def test_query_for_zipcode_geocode
     query = Geocoder::Query.new("22204")
-    assert_not_nil query.url =~ /api\.smartystreets\.com\/zipcode\?/
+    assert_match /api\.smartystreets\.com\/zipcode\?/, query.url
   end
 
   def test_query_for_zipfour_geocode
-      query = Geocoder::Query.new("22204-1603")
-      assert_not_nil query.url =~ /api\.smartystreets\.com\/zipcode\?/
+    query = Geocoder::Query.new("22204-1603")
+    assert_match /api\.smartystreets\.com\/zipcode\?/, query.url
   end
 
   def test_smarty_streets_result_components
@@ -38,6 +39,13 @@ class SmartyStreetsTest < GeocoderTestCase
     assert_equal "36061", result.fips
   end
 
+  def test_smarty_streets_result_components_with_zipcode_only_query
+    result = Geocoder.search("11211").first
+    assert_equal "Brooklyn", result.city
+    assert_equal "New York", result.state
+    assert_equal "NY", result.state_code
+  end
+
   def test_no_results
     results = Geocoder.search("no results")
     assert_equal 0, results.length
-- 
GitLab