From 964cd0b049e67ca10bd37b67c4b14ccd37064511 Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Fri, 16 Sep 2016 14:32:55 +0200
Subject: deal with network issues in nicknym source

This is a first step. In case the suspected nicknym server cannot be
reached we will now move on and try other sources.

It's robably not what we want in the long run. In order to know wether
no key exists or we just failed to connect to some servers a different
http response code would be nice if network errors occured.

This simplifies testing such scenarios in the unit test and makes the
remote tests skip on network failure.
---
 lib/nickserver/nicknym/source.rb   |  2 ++
 test/remote/nicknym_source_test.rb | 11 +++++---
 test/unit/nicknym/source_test.rb   | 58 +++++++++++++++++++++++++++-----------
 3 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/lib/nickserver/nicknym/source.rb b/lib/nickserver/nicknym/source.rb
index 45daeaf..0638869 100644
--- a/lib/nickserver/nicknym/source.rb
+++ b/lib/nickserver/nicknym/source.rb
@@ -8,6 +8,8 @@ module Nickserver
       def available_for?(domain)
         status, body = adapter.get "https://#{domain}/provider.json"
         status == 200 && provider_with_mx?(body)
+      rescue HTTP::ConnectionError
+        return false
       end
 
       def query(email)
diff --git a/test/remote/nicknym_source_test.rb b/test/remote/nicknym_source_test.rb
index 2be7251..c95c820 100644
--- a/test/remote/nicknym_source_test.rb
+++ b/test/remote/nicknym_source_test.rb
@@ -2,6 +2,9 @@ require 'test_helper'
 require 'nickserver/nicknym/source'
 require 'nickserver/email_address'
 
+#
+# Please note the Readme.md file in this directory
+#
 class RemoteNicknymSourceTest < Minitest::Test
 
   def setup
@@ -15,22 +18,22 @@ class RemoteNicknymSourceTest < Minitest::Test
   end
 
   def test_availablility_check
-    assert source.available_for? 'mail.bitmask.net'
+    skip unless source.available_for? 'mail.bitmask.net'
     refute source.available_for? 'dl.bitmask.net'   # not a provider
-    refute source.available_for? 'demo.bitmask.net' # provider without mx
   end
 
   def test_successful_query
     response = source.query(email_with_key)
+    skip if response.status == 404
     json = JSON.parse response.content
-    assert_equal 200, response.status
     assert_equal email_with_key.to_s, json["address"]
     refute_empty json["openpgp"]
   end
 
   def test_not_found
     response = source.query(email_without_key)
-    assert_equal 404, response.status
+    skip if response.status == 200
+    assert response.status == 404
   end
 
   protected
diff --git a/test/unit/nicknym/source_test.rb b/test/unit/nicknym/source_test.rb
index 76337d4..cddb7c2 100644
--- a/test/unit/nicknym/source_test.rb
+++ b/test/unit/nicknym/source_test.rb
@@ -8,31 +8,55 @@ class NicknymSourceTest < Minitest::Test
     assert source
   end
 
-  def test_available_for_domain
-    adapter.expect :get, [200, '{"services": ["mx"]}'],
-      ['https://leap_powered.tld/provider.json']
-    assert source.available_for?('leap_powered.tld')
-    adapter.verify
+  def test_available_for_domain_with_service_mx
+    assert available_on?(200, '{"services": ["mx"]}')
   end
 
-  def test_not_available_for_domain
-    adapter.expect :get, [404, nil],
-      ['https://remote.tld/provider.json']
-    assert !source.available_for?('remote.tld')
-    adapter.verify
+  def test_no_provider_json_means_no_nicknym
+    refute available_on?(404, 'blablabla')
+  end
+
+  def test_invalid_provider_json_means_no_nicknym
+    refute available_on?(200, 'blablabla')
   end
 
-  def test_successful_query
-    adapter.expect :get, [200, 'dummy body'],
-      ['https://nicknym.leap_powered.tld:6425',
-       {query: {address: email_stub.to_s}}]
+  def test_failing_network_means_no_nicknym
+    failing_network
+    refute source.available_for?('remote.tld')
+  end
+
+  def test_proxy_successful_query
+    assert proxies_query_response?(200, 'dummy body')
+  end
+
+  def test_proxy_query_not_found
+    assert proxies_query_response?(404, 'dummy body')
+  end
+
+  protected
+
+  def proxies_query_response?(status = 0, body = nil)
+    adapter.expect :get, [status, body],
+      ['https://nicknym.leap_powered.tld:6425', query: {address: email_stub.to_s}]
     response = source.query(email_stub)
-    assert_equal 200, response.status
-    assert_equal 'dummy body', response.content
+    assert_equal status, response.status
+    assert_equal body, response.content
     adapter.verify
   end
 
-  protected
+  def available_on?(status = 0, body = nil)
+    adapter.expect :get, [status, body],
+      ['https://remote.tld/provider.json']
+    available = source.available_for?('remote.tld')
+    adapter.verify
+    return available
+  end
+
+  def failing_network
+    def adapter.get(*args)
+      raise HTTP::ConnectionError
+    end
+  end
 
   def source
     Nickserver::Nicknym::Source.new(adapter)
-- 
cgit v1.2.3