From 162b59156e75f28f55977bd806d58cb67570b726 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20G=C3=B3recki?= <psihae@gmail.com>
Date: Sat, 16 Apr 2016 22:23:59 +0200
Subject: [PATCH] Use a proper domain name in ipapi_com lookup (#1020)

* Add ip-api.com (ip lookup) support

This service returns better coordinates for Europe than other free ip lookup services available in geocoder.

* Clean the results class

Remove default responses for fields.

* Remove checking fields in url query

API already handles unknown fields, so this check is unnecessary.

* Add a test for using parameters directly in search method

* Use proper domain name

Use pro.ip-api.com domain in case when api key is in use, otherwise just ip-api.com.

* Handle invalid api key

* Add forsaken fixture

* Make uniform an invalid key handler with the other geocoder lookups
---
 lib/geocoder/lookups/ipapi_com.rb     | 28 ++++++++++++++++++++++++---
 test/fixtures/ipapi_com_74_200_247_60 |  1 +
 test/unit/lookups/ipapi_com_test.rb   | 24 ++++++++++++++++++++++-
 3 files changed, 49 insertions(+), 4 deletions(-)
 create mode 100644 test/fixtures/ipapi_com_74_200_247_60

diff --git a/lib/geocoder/lookups/ipapi_com.rb b/lib/geocoder/lookups/ipapi_com.rb
index df6f098b..f38a5837 100644
--- a/lib/geocoder/lookups/ipapi_com.rb
+++ b/lib/geocoder/lookups/ipapi_com.rb
@@ -9,7 +9,8 @@ module Geocoder::Lookup
     end
 
     def query_url(query)
-      url_ = "#{protocol}://ip-api.com/json/#{query.sanitized_text}"
+      domain = configuration.api_key ? "pro.ip-api.com" : "ip-api.com"
+      url_ = "#{protocol}://#{domain}/json/#{query.sanitized_text}"
 
       if (params = url_query_string(query)) && !params.empty?
         url_ + "?" + params
@@ -29,17 +30,32 @@ module Geocoder::Lookup
 
     private
 
+    def parse_raw_data(raw_data)
+      if raw_data == "invalid key\n" || raw_data == "invalid key"
+        invalid_key_result
+      else
+        super(raw_data)
+      end
+    end
+
     def results(query)
       return [reserved_result(query.text)] if query.loopback_ip_address?
 
-      (doc = fetch_data(query)) ? [doc] : []
+      return [] unless doc = fetch_data(query)
+
+      if doc["message"] == "invalid key"
+        raise_error(Geocoder::InvalidApiKey) || Geocoder.log(:warn, "Invalid API key.")
+        return []
+      else
+        return [doc]
+      end
     end
 
     def reserved_result(query)
       {
         "message"      => "reserved range",
         "query"        => query,
-        "status"       => fail,
+        "status"       => "fail",
         "ip"           => query,
         "city"         => "",
         "region_code"  => "",
@@ -53,6 +69,12 @@ module Geocoder::Lookup
       }
     end
 
+    def invalid_key_result
+      {
+        "message"      => "invalid key"
+      }
+    end
+
     def query_url_params(query)
       params = {}
       params.merge!(fields: configuration[:fields]) if configuration.has_key?(:fields)
diff --git a/test/fixtures/ipapi_com_74_200_247_60 b/test/fixtures/ipapi_com_74_200_247_60
new file mode 100644
index 00000000..b22bbe1c
--- /dev/null
+++ b/test/fixtures/ipapi_com_74_200_247_60
@@ -0,0 +1 @@
+invalid key
diff --git a/test/unit/lookups/ipapi_com_test.rb b/test/unit/lookups/ipapi_com_test.rb
index 6cb07cb9..ce2fe3bb 100644
--- a/test/unit/lookups/ipapi_com_test.rb
+++ b/test/unit/lookups/ipapi_com_test.rb
@@ -41,6 +41,16 @@ class IpapiComTest < GeocoderTestCase
     assert_equal nil, result.message
   end
 
+  def test_localhost
+    result = Geocoder.search("::1").first
+    assert_equal nil, result.lat
+    assert_equal nil, result.lon
+    assert_equal [nil, nil], result.coordinates
+    assert_equal nil, result.reverse
+    assert_equal "::1", result.query
+    assert_equal "fail", result.status
+  end
+
   def test_api_key
     Geocoder.configure(:api_key => "MY_KEY")
     g = Geocoder::Lookup::IpapiCom.new
@@ -50,7 +60,7 @@ class IpapiComTest < GeocoderTestCase
   def test_url_with_api_key_and_fields
     Geocoder.configure(:api_key => "MY_KEY", :ipapi_com => {:fields => "lat,lon,xyz"})
     g = Geocoder::Lookup::IpapiCom.new
-    assert_equal "http://ip-api.com/json/74.200.247.59?fields=lat%2Clon%2Cxyz&key=MY_KEY", g.query_url(Geocoder::Query.new("74.200.247.59"))
+    assert_equal "http://pro.ip-api.com/json/74.200.247.59?fields=lat%2Clon%2Cxyz&key=MY_KEY", g.query_url(Geocoder::Query.new("74.200.247.59"))
   end
 
   def test_url_with_fields
@@ -70,4 +80,16 @@ class IpapiComTest < GeocoderTestCase
     assert_equal "http://ip-api.com/json/74.200.247.59?fields=lat%2Czip", g.query_url(q)
   end
 
+  def test_use_https_with_api_key
+    Geocoder.configure(:api_key => "MY_KEY", :use_https => true)
+    g = Geocoder::Lookup::IpapiCom.new
+    assert_equal "https://pro.ip-api.com/json/74.200.247.59?key=MY_KEY", g.query_url(Geocoder::Query.new("74.200.247.59"))
+  end
+
+  def test_invalid_api_key
+    Geocoder.configure(:api_key => "MY_KEY")
+    result = Geocoder.search("74.200.247.60").first
+    assert_equal nil, result
+  end
+
 end
-- 
GitLab