From 1c0dd3e47092235851686a95d32dc64cb3338a9f Mon Sep 17 00:00:00 2001
From: Marcin Olichwirowicz <olichwirowicz@gmail.com>
Date: Wed, 28 Aug 2013 16:42:44 +0200
Subject: [PATCH] Cache only properly geocoded data for google lookup

---
 lib/geocoder/lookups/base.rb    |  6 +++++-
 lib/geocoder/lookups/google.rb  |  4 ++++
 test/cache_test.rb              | 16 ++++++++++++++++
 test/fixtures/google_over_limit |  4 ++++
 4 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 test/fixtures/google_over_limit

diff --git a/lib/geocoder/lookups/base.rb b/lib/geocoder/lookups/base.rb
index 28ad577b..6589345a 100644
--- a/lib/geocoder/lookups/base.rb
+++ b/lib/geocoder/lookups/base.rb
@@ -192,6 +192,10 @@ module Geocoder
         "http" + (configuration.use_https ? "s" : "")
       end
 
+      def valid_response(response)
+        (200..399).include?(response.code.to_i)
+      end
+
       ##
       # Fetch a raw geocoding result (JSON string).
       # The result might or might not be cached.
@@ -204,7 +208,7 @@ module Geocoder
           check_api_key_configuration!(query)
           response = make_api_request(query)
           body = response.body
-          if cache and (200..399).include?(response.code.to_i)
+          if cache and valid_response(response)
             cache[key] = body
           end
           @cache_hit = false
diff --git a/lib/geocoder/lookups/google.rb b/lib/geocoder/lookups/google.rb
index c16bf85f..65308de2 100644
--- a/lib/geocoder/lookups/google.rb
+++ b/lib/geocoder/lookups/google.rb
@@ -16,6 +16,10 @@ module Geocoder::Lookup
       "#{protocol}://maps.googleapis.com/maps/api/geocode/json?" + url_query_string(query)
     end
 
+    def valid_response(response)
+      super(response) && JSON.parse(response.body)["status"] == "OK"
+    end
+
     private # ---------------------------------------------------------------
 
     def results(query)
diff --git a/test/cache_test.rb b/test/cache_test.rb
index edd45bbf..0e45cfcf 100644
--- a/test/cache_test.rb
+++ b/test/cache_test.rb
@@ -16,4 +16,20 @@ class CacheTest < Test::Unit::TestCase
         "Lookup #{l} did not return cached result."
     end
   end
+
+  def test_google_over_query_limit_does_not_hit_cache
+    Geocoder.configure(:cache => {})
+    Geocoder.configure(:lookup => :google)
+    set_api_key!(:google)
+    Geocoder.configure(:always_raise => :all)
+    assert_raises Geocoder::OverQueryLimitError do
+      Geocoder.search("over limit")
+    end
+    lookup = Geocoder::Lookup.get(:google)
+    assert_equal false, lookup.instance_variable_get(:@cache_hit)
+    assert_raises Geocoder::OverQueryLimitError do
+      Geocoder.search("over limit")
+    end
+    assert_equal false, lookup.instance_variable_get(:@cache_hit)
+  end
 end
diff --git a/test/fixtures/google_over_limit b/test/fixtures/google_over_limit
new file mode 100644
index 00000000..353ccf5c
--- /dev/null
+++ b/test/fixtures/google_over_limit
@@ -0,0 +1,4 @@
+{
+  "status": "OVER_QUERY_LIMIT",
+  "results": [ ]
+}
-- 
GitLab