Merge branch 'bugfix/name-resolution' into 'master'

fix: #3 handle domains without A-record

Closes #3

See merge request !15

7 files changed, 65 insertions, 28 deletions

diff --git a/lib/nickserver/adapters/http.rb b/lib/nickserver/adapters/http.rb
index 636aceb..eb77cc6 100644
--- a/lib/nickserver/adapters/http.rb
+++ b/lib/nickserver/adapters/http.rb
@@ -2,6 +2,19 @@ require 'nickserver/adapters'
 require 'nickserver/config'
 require 'http'
 
+# Nickserver::Adapters::Http
+#
+# Basic http adapter with ssl and minimal error handling.
+# Only implemented get requests so far.
+#
+# Error Handling:
+#
+# Pass a string as the 'rescue' option. If a ConnectionError occures
+# which includes the string passed it will be rescued and the request
+# will return nil. This allows handling the error inside the adapter so
+# that for the derived CelluloidHttp Adapter the actor does not get
+# killed.
+
 module Nickserver::Adapters
   class Http
 
@@ -9,6 +22,8 @@ module Nickserver::Adapters
       url = HTTP::URI.parse url.to_s
       response = get_with_auth url, params: options[:query]
       return response.code, response.to_s
+    rescue HTTP::ConnectionError => e
+      raise unless options[:rescue] && e.to_s.include?(options[:rescue])
     end
 
     protected
diff --git a/lib/nickserver/nicknym/source.rb b/lib/nickserver/nicknym/source.rb
index 96945cb..f49547e 100644
--- a/lib/nickserver/nicknym/source.rb
+++ b/lib/nickserver/nicknym/source.rb
@@ -8,7 +8,8 @@ module Nickserver
       PORT = 6425
 
       def available_for?(domain)
-        status, body = adapter.get "https://#{domain}/provider.json"
+        status, body = adapter.get "https://#{domain}/provider.json",
+          rescue: 'failed to connect: getaddrinfo'
         status == 200 && provider_with_mx?(body)
       end
 
diff --git a/test/functional/sample_test.rb b/test/functional/sample_test.rb
index 68127e1..4be5c26 100644
--- a/test/functional/sample_test.rb
+++ b/test/functional/sample_test.rb
@@ -24,6 +24,11 @@ class SampleTest < FunctionalTest
 
   # Regression Tests
 
+  # #3 handle missing A records
+  def test_nicknym
+    assert_lookup_status 404, 'postmaster@cs.ucl.ac.uk'
+  end
+
   def test_no_file_descriptors_leak
     lookup 'test@mail.bitmask.net'
     before = open_files_count
diff --git a/test/remote/Readme.md b/test/remote/Readme.md
index 957ea12..53f5e65 100644
--- a/test/remote/Readme.md
+++ b/test/remote/Readme.md
@@ -1,15 +1,14 @@
 Integration tests for clients of remote services
 ================================================
 
-The tests in this directory are integration test with remote services.  However
-we aims at testing the client side of the equation as that is what we control
-here.
+The tests in this directory are integration test with remote services.
+However we aims at testing the client side of the equation as that is
+what we control here.
 
-So unexpected server behavious should *crash* the test if we are not dealing
-with it properly yet and have no unit test for it.
+So unexpected server behavious should *crash* the test if we are not
+dealing with it properly yet and have no unit test for it.
 
-Server responses that we do not expect but handle in the code and test in unit
-tests make the test *skip*.
+Server responses that we do not expect but handle in the code and test
+in unit tests make the test *skip*.
 
 The Behaviour we would normally expect should make the test *pass*
-
diff --git a/test/remote/nicknym_source_test.rb b/test/remote/nicknym_source_test.rb
index 4ca3033..b97f2b2 100644
--- a/test/remote/nicknym_source_test.rb
+++ b/test/remote/nicknym_source_test.rb
@@ -10,13 +10,24 @@ require 'nickserver/email_address'
 class RemoteNicknymSourceTest < CelluloidTest
   include HttpAdapterHelper
 
-  def test_availablility_check
+  def test_available_for_mail
     source.available_for? 'mail.bitmask.net'
-    refute source.available_for? 'dl.bitmask.net'   # not a provider
   rescue HTTP::ConnectionError => e
     skip e.to_s
   end
 
+  # not a provider
+  def test_not_available
+    refute source.available_for? 'dl.bitmask.net'
+  rescue HTTP::ConnectionError => e
+    skip e.to_s
+  end
+
+  # cs.ucl.ac.uk only has an MX not an A-record
+  def test_not_available_without_a_record
+    refute source.available_for? 'cs.ucl.ac.uk'
+  end
+
   def test_successful_query
     response = source.query(email_with_key)
     skip if response.status == 404
diff --git a/test/support/http_stub_helper.rb b/test/support/http_stub_helper.rb
index c9f2bfa..4e3d89b 100644
--- a/test/support/http_stub_helper.rb
+++ b/test/support/http_stub_helper.rb
@@ -10,32 +10,31 @@ module HttpStubHelper
   end
 
   def stub_nicknym_available_response(domain, response = {})
-    stub_http_request :get, "https://#{domain}/provider.json",
-      response: response
+    stub_http_get "https://#{domain}/provider.json",
+      response,
+      Hash
   end
 
   def stub_sks_vindex_reponse(uid, response = {})
-    stub_http_request :get, config.hkp_url,
-      query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'},
-      response: response
+    stub_http_get config.hkp_url, response,
+      query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'}
   end
 
   def stub_sks_get_reponse(key_id, response = {})
-    stub_http_request :get, config.hkp_url,
-      query: {op: 'get', search: "0x"+key_id, exact: 'on', options: 'mr'},
-      response: response
+    stub_http_get config.hkp_url, response,
+      query: {op: 'get', search: "0x"+key_id, exact: 'on', options: 'mr'}
   end
 
   def stub_couch_response(uid, response = {})
     query = "\?key=#{"%22#{uid}%22"}&reduce=false"
-    stub_http_request :get,
-      /#{Regexp.escape(config.couch_url)}.*#{query}/,
-      response: response
+    stub_http_get /#{Regexp.escape(config.couch_url)}.*#{query}/,
+      response
   end
 
-  def stub_http_request(verb, url, options = {})
-    response = {status: 200, body: ""}.merge(options.delete(:response) || {})
-    options = nil if options == {}
+  private
+
+  def stub_http_get(url, response, options = nil)
+    response = {status: 200, body: ""}.merge(response || {})
     adapter.expect :get, [response[:status], response[:body]],
       [url, options].compact
   end
diff --git a/test/unit/nicknym/source_test.rb b/test/unit/nicknym/source_test.rb
index f8c9b60..b17f22b 100644
--- a/test/unit/nicknym/source_test.rb
+++ b/test/unit/nicknym/source_test.rb
@@ -1,4 +1,6 @@
 require 'test_helper'
+require 'http'
+require 'json'
 require 'nickserver/nicknym/source'
 require 'nickserver/email_address'
 
@@ -20,6 +22,11 @@ class NicknymSourceTest < Minitest::Test
     refute available_on?(200, 'blablabla')
   end
 
+  # adapter rescues name resolution errors and returns nothing
+  def test_not_available_without_response
+    refute available_on?
+  end
+
   def test_proxy_successful_query
     assert proxies_query_response?(200, 'dummy body')
   end
@@ -39,9 +46,9 @@ class NicknymSourceTest < Minitest::Test
     adapter.verify
   end
 
-  def available_on?(status = 0, body = nil)
-    adapter.expect :get, [status, body],
-      ['https://remote.tld/provider.json']
+  def available_on?(*args)
+    adapter.expect :get, args,
+      ['https://remote.tld/provider.json', Hash]
     available = source.available_for?('remote.tld')
     adapter.verify
     return available