Merge pull request #847 from peteb/master

Treat Bing overload responses as invalid

lib/geocoder/lookups/bing.rb

@@ -58,15 +58,23 @@ module Geocoder::Lookup
 
     def check_response_for_errors!(response)
       super
-      if response['x-ms-bm-ws-info'].to_i == 1
-        # Occasionally, the servers processing service requests can be overloaded,
-        # and you may receive some responses that contain no results for queries that
-        # you would normally receive a result. To identify this situation,
-        # check the HTTP headers of the response. If the HTTP header X-MS-BM-WS-INFO is set to 1,
-        # it is best to wait a few seconds and try again.
+      if server_overloaded?(response)
         raise_error(Geocoder::ServiceUnavailable) ||
           Geocoder.log(:warn, "Bing Geocoding API error: Service Unavailable")
       end
     end
+
+    def valid_response?(response)
+      super(response) and not server_overloaded?(response)
+    end
+
+    def server_overloaded?(response)
+      # Occasionally, the servers processing service requests can be overloaded,
+      # and you may receive some responses that contain no results for queries that
+      # you would normally receive a result. To identify this situation,
+      # check the HTTP headers of the response. If the HTTP header X-MS-BM-WS-INFO is set to 1,
+      # it is best to wait a few seconds and try again.
+      response['x-ms-bm-ws-info'].to_i == 1
+    end
   end
 end

test/unit/cache_test.rb

@@ -2,6 +2,17 @@
 require 'test_helper'
 
 class CacheTest < GeocoderTestCase
+  def setup
+    @tempfile = Tempfile.new("log")
+    @logger = Logger.new(@tempfile.path)
+    Geocoder.configure(logger: @logger)
+  end
+
+  def teardown
+    Geocoder.configure(logger: :kernel)
+    @logger.close
+    @tempfile.close
+  end
 
   def test_second_occurrence_of_request_is_cache_hit
     Geocoder.configure(:cache => {})
@@ -33,4 +44,16 @@ class CacheTest < GeocoderTestCase
     end
     assert_equal false, lookup.instance_variable_get(:@cache_hit)
   end
+
+  def test_bing_service_unavailable_without_raising_does_not_hit_cache
+    Geocoder.configure(cache: {}, lookup: :bing, always_raise: [])
+    set_api_key!(:bing)
+    lookup = Geocoder::Lookup.get(:bing)
+
+    Geocoder.search("service unavailable")
+    assert_false lookup.instance_variable_get(:@cache_hit)
+
+    Geocoder.search("service unavailable")
+    assert_false lookup.instance_variable_get(:@cache_hit)
+  end
 end