From ce7d0571ec1110fe11af3b957afae8602e4367d2 Mon Sep 17 00:00:00 2001
From: Carlos Villela <cv@lixo.org>
Date: Fri, 17 Nov 2017 03:18:53 -0300
Subject: [PATCH] [db-ip] Handle Geocoder::OverQueryLimitError gracefully
 (#1230)

* Handle Geocoder::OverQueryLimitError gracefully.

Geocoder::OverQueryLimitError errors were not being generated correctly, and the response was coming back empty but with no indication.

This changes the behavior slightly so that unknown errors are also returned.

* Add error handling tests

* Remove unnecessary config in tests
---
 lib/geocoder/lookups/db_ip_com.rb             | 22 +++++++++++--------
 ..._240_0 => db_ip_com_madison_square_garden} |  0
 test/fixtures/db_ip_com_quota_exceeded        |  1 +
 test/fixtures/db_ip_com_unknown_error         |  1 +
 test/test_helper.rb                           |  4 ++--
 test/unit/lookups/db_ip_com_test.rb           | 20 +++++++++++++----
 6 files changed, 33 insertions(+), 15 deletions(-)
 rename test/fixtures/{db_ip_com_23_255_240_0 => db_ip_com_madison_square_garden} (100%)
 create mode 100644 test/fixtures/db_ip_com_quota_exceeded
 create mode 100644 test/fixtures/db_ip_com_unknown_error

diff --git a/lib/geocoder/lookups/db_ip_com.rb b/lib/geocoder/lookups/db_ip_com.rb
index 8a032157..bd7144d6 100644
--- a/lib/geocoder/lookups/db_ip_com.rb
+++ b/lib/geocoder/lookups/db_ip_com.rb
@@ -27,18 +27,22 @@ module Geocoder::Lookup
     private
 
     def results(query)
-      return [] unless doc = fetch_data(query)
+      return [] unless (doc = fetch_data(query))
 
-      if doc['error']
-        if doc['error'] == 'invalid API key'
-          raise_error(Geocoder::InvalidApiKey) || Geocoder.log(:warn, 'Invalid DB-IP API key.')
-        else
-          Geocoder.log(:warn, "DB-IP Geocoding API error: #{doc['error']}.")
-        end
+      case doc['error']
+      when 'maximum number of queries per day exceeded'
+        raise_error Geocoder::OverQueryLimitError ||
+                    Geocoder.log(:warn, 'DB-API query limit exceeded.')
+
+      when 'invalid API key'
+        raise_error Geocoder::InvalidApiKey ||
+                    Geocoder.log(:warn, 'Invalid DB-IP API key.')
+      when nil
+        [doc]
 
-        return []
       else
-        return [ doc ]
+        raise_error Geocoder::Error ||
+                    Geocoder.log(:warn, "Request failed: #{doc['error']}")
       end
     end
   end
diff --git a/test/fixtures/db_ip_com_23_255_240_0 b/test/fixtures/db_ip_com_madison_square_garden
similarity index 100%
rename from test/fixtures/db_ip_com_23_255_240_0
rename to test/fixtures/db_ip_com_madison_square_garden
diff --git a/test/fixtures/db_ip_com_quota_exceeded b/test/fixtures/db_ip_com_quota_exceeded
new file mode 100644
index 00000000..f5cbef52
--- /dev/null
+++ b/test/fixtures/db_ip_com_quota_exceeded
@@ -0,0 +1 @@
+{"error":"maximum number of queries per day exceeded"}
\ No newline at end of file
diff --git a/test/fixtures/db_ip_com_unknown_error b/test/fixtures/db_ip_com_unknown_error
new file mode 100644
index 00000000..7d77aa90
--- /dev/null
+++ b/test/fixtures/db_ip_com_unknown_error
@@ -0,0 +1 @@
+{"error":"unknown error"}
\ No newline at end of file
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 55978055..87e11da4 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -164,8 +164,8 @@ module Geocoder
     require 'geocoder/lookups/db_ip_com'
     class DbIpCom
       private
-      def default_fixture_filename
-        "db_ip_com_23_255_240_0"
+      def fixture_prefix
+        "db_ip_com"
       end
     end
 
diff --git a/test/unit/lookups/db_ip_com_test.rb b/test/unit/lookups/db_ip_com_test.rb
index 47fc26e6..5425fa4d 100644
--- a/test/unit/lookups/db_ip_com_test.rb
+++ b/test/unit/lookups/db_ip_com_test.rb
@@ -1,8 +1,6 @@
-# encoding: utf-8
 require 'test_helper'
 
 class DbIpComTest < GeocoderTestCase
-
   def configure_for_free_api_access
     Geocoder.configure(ip_lookup: :db_ip_com, db_ip_com: { api_key: 'MY_API_KEY' })
     set_api_key!(:db_ip_com)
@@ -51,14 +49,28 @@ class DbIpComTest < GeocoderTestCase
   def test_free_host_config
     configure_for_free_api_access
     lookup = Geocoder::Lookup::DbIpCom.new
-    query = Geocoder::Query.new("23.255.240.0")
+    query = Geocoder::Query.new('23.255.240.0')
     assert_match 'http://api.db-ip.com/v2/MY_API_KEY/23.255.240.0', lookup.query_url(query)
   end
 
   def test_paid_host_config
     configure_for_paid_api_access
     lookup = Geocoder::Lookup::DbIpCom.new
-    query = Geocoder::Query.new("23.255.240.0")
+    query = Geocoder::Query.new('23.255.240.0')
     assert_match 'https://api.db-ip.com/v2/MY_API_KEY/23.255.240.0', lookup.query_url(query)
   end
+
+  def test_raises_over_limit_exception
+    Geocoder.configure always_raise: :all
+    assert_raises Geocoder::OverQueryLimitError do
+      Geocoder::Lookup::DbIpCom.new.send(:results, Geocoder::Query.new('quota exceeded'))
+    end
+  end
+
+  def test_raises_unknown_error
+    Geocoder.configure always_raise: :all
+    assert_raises Geocoder::Error do
+      Geocoder::Lookup::DbIpCom.new.send(:results, Geocoder::Query.new('unknown error'))
+    end
+  end
 end
-- 
GitLab