refactor: turn EmailAddress into a class, cleanup

10 files changed, 103 insertions, 151 deletions

diff --git a/Gemfile b/Gemfile
index 619272b..5d8013e 100644
--- a/Gemfile
+++ b/Gemfile
@@ -2,3 +2,5 @@ source 'https://rubygems.org'
 
 # Specify your gem's dependencies in keymachine.gemspec
 gemspec
+
+gem 'byebug'
diff --git a/lib/nickserver/email_address.rb b/lib/nickserver/email_address.rb
index 26053a2..2b3f2c2 100644
--- a/lib/nickserver/email_address.rb
+++ b/lib/nickserver/email_address.rb
@@ -1,25 +1,45 @@
-#
-# This rather crazy regexp is from here: http://code.iamcal.com/php/rfc822/
-# Licensed GPLv3
-#
-# It is too liberal, allowing "!@x" as a valid address, for example, but it does
-# follow the specification rather closely.
-#
-
 module Nickserver
-  EmailAddress = begin
-    qtext = '[^\\x0d\\x22\\x5c\\x80-\\xff]'
-    dtext = '[^\\x0d\\x5b-\\x5d\\x80-\\xff]'
-    atom = '[^\\x00-\\x20\\x22\\x28\\x29\\x2c\\x2e\\x3a-\\x3c\\x3e\\x40\\x5b-\\x5d\\x7f-\\xff]+'
-    quoted_pair = '\\x5c[\\x00-\\x7f]'
-    domain_literal = "\\x5b(?:#{dtext}|#{quoted_pair})*\\x5d"
-    quoted_string = "\\x22(?:#{qtext}|#{quoted_pair})*\\x22"
-    domain_ref = atom
-    sub_domain = "(?:#{domain_ref}|#{domain_literal})"
-    word = "(?:#{atom}|#{quoted_string})"
-    domain = "#{sub_domain}(?:\\x2e#{sub_domain})*"
-    local_part = "#{word}(?:\\x2e#{word})*"
-    addr_spec = "#{local_part}\\x40#{domain}"
-    /\A#{addr_spec}\z/n
+  class EmailAddress
+
+    REGEXP = begin
+               qtext = '[^\\x0d\\x22\\x5c\\x80-\\xff]'
+               dtext = '[^\\x0d\\x5b-\\x5d\\x80-\\xff]'
+               atom = '[^\\x00-\\x20\\x22\\x28\\x29\\x2c\\x2e\\x3a-\\x3c\\x3e\\x40\\x5b-\\x5d\\x7f-\\xff]+'
+               quoted_pair = '\\x5c[\\x00-\\x7f]'
+               domain_literal = "\\x5b(?:#{dtext}|#{quoted_pair})*\\x5d"
+               quoted_string = "\\x22(?:#{qtext}|#{quoted_pair})*\\x22"
+               domain_ref = atom
+               sub_domain = "(?:#{domain_ref}|#{domain_literal})"
+               word = "(?:#{atom}|#{quoted_string})"
+               domain = "#{sub_domain}(?:\\x2e#{sub_domain})*"
+               local_part = "#{word}(?:\\x2e#{word})*"
+               addr_spec = "#{local_part}\\x40#{domain}"
+               /\A#{addr_spec}\z/n
+             end
+
+    def initialize(address)
+      @address = address.to_s
+    end
+
+    def valid?
+      address =~ REGEXP
+    end
+
+    def invalid?
+      !valid?
+    end
+
+    def domain?(domain)
+      address.end_with? "@#{domain}"
+    end
+
+    def to_s
+      address
+    end
+
+    protected
+
+    attr_reader :address
+
   end
 end
diff --git a/lib/nickserver/hkp/client.rb b/lib/nickserver/hkp/client.rb
index d9a9b48..1fbe7a2 100644
--- a/lib/nickserver/hkp/client.rb
+++ b/lib/nickserver/hkp/client.rb
@@ -21,7 +21,7 @@ module Nickserver; module Hkp
     # used to fetch an array of KeyInfo objects that match the given email
     #
     def get_key_infos_by_email(email, &block)
-      get op: 'vindex', search: email, fingerprint: 'on'
+      get op: 'vindex', search: email.to_s, fingerprint: 'on'
     end
 
     #
@@ -38,7 +38,8 @@ module Nickserver; module Hkp
     def get(query)
       # in practice, exact=on seems to have no effect
       query = {exact: 'on', options: 'mr'}.merge query
-      adapter.get Config.hkp_url, query: query
+      response = adapter.get Config.hkp_url, query: query
+      return response
     end
   end
 end; end
diff --git a/lib/nickserver/hkp/parse_key_info.rb b/lib/nickserver/hkp/parse_key_info.rb
index 9d59d6b..2f928a0 100644
--- a/lib/nickserver/hkp/parse_key_info.rb
+++ b/lib/nickserver/hkp/parse_key_info.rb
@@ -19,7 +19,8 @@ module Nickserver; module Hkp
       @vindex_result = vindex_result
     end
 
-    def status_for(uid)
+    def status_for(email)
+      uid = email.to_s
       if hkp_ok? && keys(uid).empty?
         error_status(uid)
       else
@@ -27,7 +28,8 @@ module Nickserver; module Hkp
       end
     end
 
-    def response_for(uid)
+    def response_for(email)
+      uid = email.to_s
       if keys(uid).any?
         keys(uid)
       else
@@ -35,6 +37,8 @@ module Nickserver; module Hkp
       end
     end
 
+    protected
+
     def keys(uid)
       key_infos(uid).reject { |key| error_for_key(key) }
     end
@@ -47,8 +51,6 @@ module Nickserver; module Hkp
       end
     end
 
-    protected
-
     attr_reader :status
     attr_reader :vindex_result
 
diff --git a/lib/nickserver/nickname.rb b/lib/nickserver/nickname.rb
deleted file mode 100644
index 938d4a4..0000000
--- a/lib/nickserver/nickname.rb
+++ /dev/null
@@ -1,51 +0,0 @@
-module Nickserver
-  class Nickname
-
-    EmailAddress = begin
-                     qtext = '[^\\x0d\\x22\\x5c\\x80-\\xff]'
-                     dtext = '[^\\x0d\\x5b-\\x5d\\x80-\\xff]'
-                     atom = '[^\\x00-\\x20\\x22\\x28\\x29\\x2c\\x2e\\x3a-\\x3c\\x3e\\x40\\x5b-\\x5d\\x7f-\\xff]+'
-                     quoted_pair = '\\x5c[\\x00-\\x7f]'
-                     domain_literal = "\\x5b(?:#{dtext}|#{quoted_pair})*\\x5d"
-                     quoted_string = "\\x22(?:#{qtext}|#{quoted_pair})*\\x22"
-                     domain_ref = atom
-                     sub_domain = "(?:#{domain_ref}|#{domain_literal})"
-                     word = "(?:#{atom}|#{quoted_string})"
-                     domain = "#{sub_domain}(?:\\x2e#{sub_domain})*"
-                     local_part = "#{word}(?:\\x2e#{word})*"
-                     addr_spec = "#{local_part}\\x40#{domain}"
-                     /\A#{addr_spec}\z/n
-                   end
-
-    LOCAL_DOMAIN = 'test.me'
-
-    def initialize(address)
-      @address = address.to_s
-    end
-
-    def valid?
-      address =~ EmailAddress
-    end
-
-    def invalid?
-      !valid?
-    end
-
-    def local?
-      address.end_with? LOCAL_DOMAIN
-    end
-
-    def remote?
-      !local?
-    end
-
-    def to_s
-      address
-    end
-
-    protected
-
-    attr_reader :address
-
-  end
-end
diff --git a/lib/nickserver/request_handler.rb b/lib/nickserver/request_handler.rb
index 295d3c0..608db83 100644
--- a/lib/nickserver/request_handler.rb
+++ b/lib/nickserver/request_handler.rb
@@ -10,13 +10,13 @@ module Nickserver
     end
 
     def respond_to(params, headers)
-      uid = get_uid_from_params(params)
-      if uid.nil?
+      email = get_email_from_params(params)
+      if email.nil?
         send_not_found
-      elsif uid !~ EmailAddress
+      elsif email.invalid?
         send_error("Not a valid address")
       else
-        send_key(uid, headers)
+        send_key(email, headers)
       end
     rescue RuntimeError => exc
       puts "Error: #{exc}"
@@ -26,22 +26,22 @@ module Nickserver
 
     protected
 
-    def get_uid_from_params(params)
+    def get_email_from_params(params)
       if params && params["address"] && params["address"].any?
-        return params["address"].first
-      else
-        return nil
+        EmailAddress.new(params["address"].first)
       end
     end
 
-    def send_key(uid, headers)
-      if local_address?(uid, headers)
+    def send_key(email, headers)
+      if local_address?(email, headers)
         source = Nickserver::CouchDB::Source.new(adapter)
       else
         source = Nickserver::Hkp::Source.new(adapter)
       end
-      response = source.query(uid)
+      response = source.query(email)
       send_response response.status, response.content
+    rescue MissingHostHeader
+      send_error("HTTP request must include a Host header.")
     end
 
     #
@@ -50,21 +50,17 @@ module Nickserver
     #
     # If 'domain' is not configured, we rely on the Host header of the HTTP request.
     #
-    def local_address?(uid, headers)
-      uid_domain = uid.sub(/^.*@(.*)$/, "\\1")
-      if Config.domain
-        return uid_domain == Config.domain
-      else
-        # no domain configured, use Host header
-        host_header = headers['Host']
-        if host_header.nil?
-          send_error("HTTP request must include a Host header.")
-        else
-          host = host_header.split(':')[0].strip.sub(/^nicknym\./, '')
-          return uid_domain == host
-        end
-      end
+    def local_address?(email, headers)
+      email.domain?(Config.domain || domain_from_headers(headers))
     end
+
+    # no domain configured, use Host header
+    def domain_from_headers(headers)
+      host_header = headers['Host']
+      raise MissingHostHeader if host_header.nil?
+      host_header.split(':')[0].strip.sub(/^nicknym\./, '')
+    end
+
     def send_error(msg = "not supported")
       send_response 500, "500 #{msg}\n"
     end
@@ -79,5 +75,7 @@ module Nickserver
 
     attr_reader :responder, :adapter
 
+    class MissingHostHeader < StandardError
+    end
   end
 end
diff --git a/lib/server.rb b/lib/server.rb
deleted file mode 100644
index 4e7cf51..0000000
--- a/lib/server.rb
+++ /dev/null
@@ -1,19 +0,0 @@
-class Server
-
-  def process_http_request
-    lookup.respond_with(Responder)
-  end
-
-  def lookup
-    LookupFactory.lookup_for(nick)
-  end
-
-  def nick
-    Nickname.new(request.address)
-  end
-
-  def request
-    Request.new(params)
-  end
-
-end
diff --git a/test/unit/email_address_test.rb b/test/unit/email_address_test.rb
new file mode 100644
index 0000000..6d57a8c
--- /dev/null
+++ b/test/unit/email_address_test.rb
@@ -0,0 +1,22 @@
+require 'test_helper'
+require 'nickserver/email_address'
+
+class EmailAddressTest < Minitest::Test
+
+  def test_domain
+    nick = Nickserver::EmailAddress.new 'nick@test.me'
+    assert nick.domain?('test.me')
+    assert !nick.domain?('est.me')
+  end
+
+  def test_valid
+    nick = Nickserver::EmailAddress.new 'nick@remote.domain'
+    assert nick.valid?
+  end
+
+  def test_invalid
+    nick = Nickserver::EmailAddress.new 'asdf'
+    assert nick.invalid?
+  end
+
+end
diff --git a/test/unit/nickname_test.rb b/test/unit/nickname_test.rb
deleted file mode 100644
index 8681545..0000000
--- a/test/unit/nickname_test.rb
+++ /dev/null
@@ -1,28 +0,0 @@
-require 'test_helper'
-require 'nickserver/nickname'
-
-class NicknameTest < Minitest::Test
-
-  def test_local
-    nick = Nickserver::Nickname.new 'nick@test.me'
-    assert nick.local?
-    assert !nick.remote?
-  end
-
-  def test_remote
-    nick = Nickserver::Nickname.new 'nick@remote.domain'
-    assert !nick.local?
-    assert nick.remote?
-  end
-
-  def test_valid
-    nick = Nickserver::Nickname.new 'nick@remote.domain'
-    assert nick.valid?
-  end
-
-  def test_invalid
-    nick = Nickserver::Nickname.new 'asdf'
-    assert nick.invalid?
-  end
-
-end
diff --git a/test/unit/request_handler_test.rb b/test/unit/request_handler_test.rb
index 7fe766e..ef656f9 100644
--- a/test/unit/request_handler_test.rb
+++ b/test/unit/request_handler_test.rb
@@ -13,6 +13,11 @@ class Nickserver::RequestHandlerTest < Minitest::Test
     assert_response status: 500, content: "500 Not a valid address\n"
   end
 
+  def test_missing_domain
+    handle address: ['valid@email.tld']
+    assert_response status: 500, content: "500 HTTP request must include a Host header.\n"
+  end
+
   protected
 
   def handle(params = {}, headers = {})