From f4da3ebac7c3544156995260b90167816d8cc71c Mon Sep 17 00:00:00 2001
From: Peter Backman <peter@iostream.cc>
Date: Thu, 23 Apr 2015 20:52:16 +0200
Subject: [PATCH] Don't treat responses from Bing as valid when the server
 overload header is set.

---
 lib/geocoder/lookups/bing.rb | 20 ++++++++++++++------
 test/unit/cache_test.rb      | 23 +++++++++++++++++++++++
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/lib/geocoder/lookups/bing.rb b/lib/geocoder/lookups/bing.rb
index 40f0d469..53aaadf5 100644
--- a/lib/geocoder/lookups/bing.rb
+++ b/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
diff --git a/test/unit/cache_test.rb b/test/unit/cache_test.rb
index 21954398..0fac933b 100644
--- a/test/unit/cache_test.rb
+++ b/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
-- 
GitLab