From 2b530aad5ce09e760d02ec3d16c5779375bebbbc Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 26 Aug 2016 19:39:26 +0200 Subject: refactor: Request helper class in RequestHandler We have a specific way of encoding the thing we are looking for in the request. I added a small helper class to access the fingerprint and email. This also cleans up the RequestHandler code to some extend. --- lib/nickserver/request_handler.rb | 63 ++++++++++++++++++++++++++------------- 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/lib/nickserver/request_handler.rb b/lib/nickserver/request_handler.rb index 856ec5d..8841b43 100644 --- a/lib/nickserver/request_handler.rb +++ b/lib/nickserver/request_handler.rb @@ -4,16 +4,46 @@ require 'nickserver/couch_db/source' module Nickserver class RequestHandler + class Request + def initialize(params, headers) + @params = params || {} + @headers = headers + end + + def email + param("address") + end + + def fingerprint + param("fingerprint") + end + + def domain + host_header = headers['Host'] + raise MissingHostHeader if host_header.nil? + host_header.split(':')[0].strip.sub(/^nicknym\./, '') + end + + protected + + def param(key) + params[key] && params[key].first + end + + attr_reader :params, :headers + end + def initialize(responder, adapter) @responder = responder @adapter = adapter end def respond_to(params, headers) - if params && params["address"] && params["address"].any? - by_email(params, headers) - elsif params && params["fingerprint"] && params["fingerprint"].any? - by_fingerprint(params) + request = Request.new params, headers + if request.email + by_email(request) + elsif request.fingerprint + by_fingerprint(request) else send_not_found end @@ -26,17 +56,17 @@ module Nickserver protected - def by_email(params, headers) - email = EmailAddress.new(params["address"].first) + def by_email(request) + email = EmailAddress.new(request.email) if email.invalid? send_error("Not a valid address") else - send_key(email, headers) + send_key(email, request) end end - def by_fingerprint(params) - fingerprint = params["fingerprint"].first + def by_fingerprint(request) + fingerprint = request.fingerprint if fingerprint.length == 40 && !fingerprint[/\H/] source = Nickserver::Hkp::Source.new(adapter) key_response = source.get_key_by_fingerprint(fingerprint) @@ -46,8 +76,8 @@ module Nickserver end end - def send_key(email, headers) - if local_address?(email, headers) + def send_key(email, request) + if local_address?(email, request) source = Nickserver::CouchDB::Source.new(adapter) else source = Nickserver::Hkp::Source.new(adapter) @@ -64,15 +94,8 @@ module Nickserver # # If 'domain' is not configured, we rely on the Host header of the HTTP request. # - 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\./, '') + def local_address?(email, request) + email.domain?(Config.domain || request.domain) end def send_error(msg = "not supported") -- cgit v1.2.3 From decdf85a8b2713fddf13888b48810d59b0ec2189 Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 26 Aug 2016 23:15:34 +0200 Subject: refactor: split up RequestHandler even more --- lib/nickserver/request_handler.rb | 120 +++++++++++++++++++++++--------------- 1 file changed, 74 insertions(+), 46 deletions(-) diff --git a/lib/nickserver/request_handler.rb b/lib/nickserver/request_handler.rb index 8841b43..da9bcb3 100644 --- a/lib/nickserver/request_handler.rb +++ b/lib/nickserver/request_handler.rb @@ -40,70 +40,98 @@ module Nickserver def respond_to(params, headers) request = Request.new params, headers - if request.email - by_email(request) - elsif request.fingerprint - by_fingerprint(request) - else - send_not_found - end + response = handle request + send_response response.status, response.content + end + + protected + def handle(request) + handler = handler_for_request request + handler.call request rescue RuntimeError => exc puts "Error: #{exc}" puts exc.backtrace - send_error(exc.to_s) + ErrorResponse.new(exc.to_s) end - protected - - def by_email(request) - email = EmailAddress.new(request.email) - if email.invalid? - send_error("Not a valid address") + def handler_for_request(request) + if request.email + EmailHandler.new adapter + elsif request.fingerprint + FingerprintHandler.new adapter else - send_key(email, request) + Proc.new { Nickserver::Response.new(404, "Not Found\n") } end end - def by_fingerprint(request) - fingerprint = request.fingerprint - if fingerprint.length == 40 && !fingerprint[/\H/] - source = Nickserver::Hkp::Source.new(adapter) - key_response = source.get_key_by_fingerprint(fingerprint) - send_response key_response.status, key_response.content - else - send_error('Fingerprint invalid: ' + fingerprint) + class EmailHandler + + def initialize(adapter) + @adapter = adapter end - end - def send_key(email, request) - if local_address?(email, request) - source = Nickserver::CouchDB::Source.new(adapter) - else - source = Nickserver::Hkp::Source.new(adapter) + def call(request) + email = EmailAddress.new(request.email) + if email.invalid? + ErrorResponse.new("Not a valid address") + else + send_key(email, request) + end + end + + protected + + def send_key(email, request) + if local_address?(email, request) + source = Nickserver::CouchDB::Source.new(adapter) + else + source = Nickserver::Hkp::Source.new(adapter) + end + source.query(email) + rescue MissingHostHeader + ErrorResponse.new("HTTP request must include a Host header.") + end + + # + # Return true if the user address is for a user of this service provider. + # e.g. if the provider is example.org, then alice@example.org returns true. + # + # If 'domain' is not configured, we rely on the Host header of the HTTP request. + # + def local_address?(email, request) + email.domain?(Config.domain || request.domain) end - response = source.query(email) - send_response response.status, response.content - rescue MissingHostHeader - send_error("HTTP request must include a Host header.") - end - # - # Return true if the user address is for a user of this service provider. - # e.g. if the provider is example.org, then alice@example.org returns true. - # - # If 'domain' is not configured, we rely on the Host header of the HTTP request. - # - def local_address?(email, request) - email.domain?(Config.domain || request.domain) + attr_reader :adapter end - def send_error(msg = "not supported") - send_response 500, "500 #{msg}\n" + class FingerprintHandler + + def initialize(adapter) + @adapter = adapter + end + + def call(request) + fingerprint = request.fingerprint + if fingerprint.length == 40 && !fingerprint[/\H/] + source = Nickserver::Hkp::Source.new(adapter) + source.get_key_by_fingerprint(fingerprint) + else + ErrorResponse.new('Fingerprint invalid: ' + fingerprint) + end + end + + protected + + attr_reader :adapter end - def send_not_found(msg = "Not Found") - send_response 404, "404 #{msg}\n" + class ErrorResponse < Nickserver::Response + def initialize(message) + @status = 500 + @message = message + "\n" + end end def send_response(status = 200, content = '') -- cgit v1.2.3 From 47343fad827ac2bef2ed7bc08768e4e58ac9a95f Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 27 Aug 2016 09:09:07 +0200 Subject: refactor: push down adapter dependency injection to sources The original idea was that we would have all the celluloid related things injected in reel_server. However it seems unlikely that we will use a different http system anytime soon. Removing some layers of dependency injection to reduce complexity. We can easily bring this back if we want. --- lib/nickserver/reel_server.rb | 2 +- lib/nickserver/request_handler.rb | 28 +++++++--------------------- lib/nickserver/source.rb | 3 +-- test/unit/request_handler_test.rb | 5 +---- 4 files changed, 10 insertions(+), 28 deletions(-) diff --git a/lib/nickserver/reel_server.rb b/lib/nickserver/reel_server.rb index 6f05e0b..549b738 100644 --- a/lib/nickserver/reel_server.rb +++ b/lib/nickserver/reel_server.rb @@ -32,7 +32,7 @@ module Nickserver protected def handler_for(request) - RequestHandler.new(request, Nickserver::Adapters::CelluloidHttp.new) + RequestHandler.new(request) end def params(request) diff --git a/lib/nickserver/request_handler.rb b/lib/nickserver/request_handler.rb index da9bcb3..239ac5f 100644 --- a/lib/nickserver/request_handler.rb +++ b/lib/nickserver/request_handler.rb @@ -33,9 +33,8 @@ module Nickserver attr_reader :params, :headers end - def initialize(responder, adapter) + def initialize(responder) @responder = responder - @adapter = adapter end def respond_to(params, headers) @@ -57,9 +56,9 @@ module Nickserver def handler_for_request(request) if request.email - EmailHandler.new adapter + EmailHandler.new elsif request.fingerprint - FingerprintHandler.new adapter + FingerprintHandler.new else Proc.new { Nickserver::Response.new(404, "Not Found\n") } end @@ -67,10 +66,6 @@ module Nickserver class EmailHandler - def initialize(adapter) - @adapter = adapter - end - def call(request) email = EmailAddress.new(request.email) if email.invalid? @@ -84,9 +79,9 @@ module Nickserver def send_key(email, request) if local_address?(email, request) - source = Nickserver::CouchDB::Source.new(adapter) + source = Nickserver::CouchDB::Source.new else - source = Nickserver::Hkp::Source.new(adapter) + source = Nickserver::Hkp::Source.new end source.query(email) rescue MissingHostHeader @@ -102,29 +97,20 @@ module Nickserver def local_address?(email, request) email.domain?(Config.domain || request.domain) end - - attr_reader :adapter end class FingerprintHandler - def initialize(adapter) - @adapter = adapter - end - def call(request) fingerprint = request.fingerprint if fingerprint.length == 40 && !fingerprint[/\H/] - source = Nickserver::Hkp::Source.new(adapter) + source = Nickserver::Hkp::Source.new source.get_key_by_fingerprint(fingerprint) else ErrorResponse.new('Fingerprint invalid: ' + fingerprint) end end - protected - - attr_reader :adapter end class ErrorResponse < Nickserver::Response @@ -138,7 +124,7 @@ module Nickserver responder.respond status, content end - attr_reader :responder, :adapter + attr_reader :responder class MissingHostHeader < StandardError end diff --git a/lib/nickserver/source.rb b/lib/nickserver/source.rb index b8135da..78f245b 100644 --- a/lib/nickserver/source.rb +++ b/lib/nickserver/source.rb @@ -1,13 +1,12 @@ module Nickserver class Source - def initialize(adapter) + def initialize(adapter = Nickserver::Adapters::CelluloidHttp.new) @adapter = adapter end protected attr_reader :adapter - end end diff --git a/test/unit/request_handler_test.rb b/test/unit/request_handler_test.rb index 6c7a036..d938706 100644 --- a/test/unit/request_handler_test.rb +++ b/test/unit/request_handler_test.rb @@ -63,14 +63,11 @@ class Nickserver::RequestHandlerTest < Minitest::Test end def handler - Nickserver::RequestHandler.new responder, adapter + Nickserver::RequestHandler.new responder end def responder @responder ||= Minitest::Mock.new end - def adapter - @adapter ||= Minitest::Mock.new - end end -- cgit v1.2.3 From 16d77cc03068b19791e704abfc2eb14f8cd141f0 Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 27 Aug 2016 11:25:30 +0200 Subject: expose Request class from RequestHandler This way we can separate the EmailHandler and the FingerprintHandler as well. --- lib/nickserver/request.rb | 30 ++++++++++++++++++++++++++ lib/nickserver/request_handler.rb | 36 +++++--------------------------- test/unit/request_test.rb | 44 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 31 deletions(-) create mode 100644 lib/nickserver/request.rb create mode 100644 test/unit/request_test.rb diff --git a/lib/nickserver/request.rb b/lib/nickserver/request.rb new file mode 100644 index 0000000..c21c280 --- /dev/null +++ b/lib/nickserver/request.rb @@ -0,0 +1,30 @@ +module Nickserver + class Request + def initialize(params, headers) + @params = params || {} + @headers = headers + end + + def email + param("address") + end + + def fingerprint + param("fingerprint") + end + + def domain + host_header = headers['Host'] || '' + domain_part = host_header.split(':')[0] || '' + domain_part.strip.sub(/^nicknym\./, '') + end + + protected + + def param(key) + params[key] && params[key].first + end + + attr_reader :params, :headers + end +end diff --git a/lib/nickserver/request_handler.rb b/lib/nickserver/request_handler.rb index 239ac5f..1239c44 100644 --- a/lib/nickserver/request_handler.rb +++ b/lib/nickserver/request_handler.rb @@ -1,44 +1,16 @@ require 'nickserver/hkp/source' require 'nickserver/couch_db/source' +require 'nickserver/request' module Nickserver class RequestHandler - class Request - def initialize(params, headers) - @params = params || {} - @headers = headers - end - - def email - param("address") - end - - def fingerprint - param("fingerprint") - end - - def domain - host_header = headers['Host'] - raise MissingHostHeader if host_header.nil? - host_header.split(':')[0].strip.sub(/^nicknym\./, '') - end - - protected - - def param(key) - params[key] && params[key].first - end - - attr_reader :params, :headers - end - def initialize(responder) @responder = responder end def respond_to(params, headers) - request = Request.new params, headers + request = Nickserver::Request.new params, headers response = handle request send_response response.status, response.content end @@ -95,7 +67,9 @@ module Nickserver # If 'domain' is not configured, we rely on the Host header of the HTTP request. # def local_address?(email, request) - email.domain?(Config.domain || request.domain) + domain = Config.domain || request.domain + raise MissingHostHeader if domain == '' + email.domain? domain end end diff --git a/test/unit/request_test.rb b/test/unit/request_test.rb new file mode 100644 index 0000000..698a275 --- /dev/null +++ b/test/unit/request_test.rb @@ -0,0 +1,44 @@ +require 'test_helper' +require 'nickserver/request' + +class Nickserver::RequestTest < Minitest::Test + + def test_email + request = request_with_params address: fake_email + assert_equal fake_email, request.email + end + + def test_blank_email + request = request_with_params + assert_equal nil, request.email + end + + def test_fingerprint + request = request_with_params fingerprint: fake_fingerprint + assert_equal fake_fingerprint, request.fingerprint + end + + def test_domain + request = Nickserver::Request.new Hash.new, + 'Host' => ' nicknym.my.domain.tld:123' + assert_equal 'my.domain.tld', request.domain + end + + protected + + # params are encoded with strings as keys and arrays with the + # given value(s) + def request_with_params(params = {}) + params = params.collect{|k,v| [k.to_s, Array(v)]}.to_h + Nickserver::Request.new params, {} + end + + def fake_email + 'test@domain.tld' + end + + def fake_fingerprint + 'F' * 40 + end + +end -- cgit v1.2.3 From 58d687f927aabc8aa2fdfc46132cb71706bdde46 Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 29 Aug 2016 09:41:12 +0200 Subject: refactor: split up ResponseHandler Now we have a Dispatcher and two ResponseHandlers that have the same interface. Moving towards a Chain of Responsibility pattern. --- lib/nickserver/dispatcher.rb | 46 +++++++++ lib/nickserver/error_response.rb | 11 +++ lib/nickserver/reel_server.rb | 4 +- lib/nickserver/request_handler.rb | 106 --------------------- lib/nickserver/request_handlers/email_handler.rb | 49 ++++++++++ .../request_handlers/fingerprint_handler.rb | 20 ++++ test/unit/dispatcher_test.rb | 73 ++++++++++++++ test/unit/request_handler_test.rb | 73 -------------- 8 files changed, 201 insertions(+), 181 deletions(-) create mode 100644 lib/nickserver/dispatcher.rb create mode 100644 lib/nickserver/error_response.rb delete mode 100644 lib/nickserver/request_handler.rb create mode 100644 lib/nickserver/request_handlers/email_handler.rb create mode 100644 lib/nickserver/request_handlers/fingerprint_handler.rb create mode 100644 test/unit/dispatcher_test.rb delete mode 100644 test/unit/request_handler_test.rb diff --git a/lib/nickserver/dispatcher.rb b/lib/nickserver/dispatcher.rb new file mode 100644 index 0000000..b818b98 --- /dev/null +++ b/lib/nickserver/dispatcher.rb @@ -0,0 +1,46 @@ +require 'nickserver/request' +require 'nickserver/request_handlers/email_handler' +require 'nickserver/request_handlers/fingerprint_handler' + +module Nickserver + class Dispatcher + + def initialize(responder) + @responder = responder + end + + def respond_to(params, headers) + request = Nickserver::Request.new params, headers + response = handle request + send_response response.status, response.content + end + + protected + + def handle(request) + handler = handler_for_request request + handler.call request + rescue RuntimeError => exc + puts "Error: #{exc}" + puts exc.backtrace + ErrorResponse.new(exc.to_s) + end + + def handler_for_request(request) + if request.email + RequestHandlers::EmailHandler.new + elsif request.fingerprint + RequestHandlers::FingerprintHandler.new + else + Proc.new { Nickserver::Response.new(404, "Not Found\n") } + end + end + + def send_response(status = 200, content = '') + responder.respond status, content + end + + attr_reader :responder + + end +end diff --git a/lib/nickserver/error_response.rb b/lib/nickserver/error_response.rb new file mode 100644 index 0000000..1065e4e --- /dev/null +++ b/lib/nickserver/error_response.rb @@ -0,0 +1,11 @@ +require 'nickserver/response' + +module Nickserver + class ErrorResponse < Nickserver::Response + def initialize(message) + @status = 500 + @message = message + "\n" + end + + end +end diff --git a/lib/nickserver/reel_server.rb b/lib/nickserver/reel_server.rb index 549b738..b681577 100644 --- a/lib/nickserver/reel_server.rb +++ b/lib/nickserver/reel_server.rb @@ -2,7 +2,7 @@ silence_warnings do require 'reel' end require 'nickserver/adapters/celluloid_http' -require 'nickserver/request_handler' +require 'nickserver/dispatcher' module Nickserver class ReelServer < Reel::Server::HTTP @@ -32,7 +32,7 @@ module Nickserver protected def handler_for(request) - RequestHandler.new(request) + Dispatcher.new(request) end def params(request) diff --git a/lib/nickserver/request_handler.rb b/lib/nickserver/request_handler.rb deleted file mode 100644 index 1239c44..0000000 --- a/lib/nickserver/request_handler.rb +++ /dev/null @@ -1,106 +0,0 @@ -require 'nickserver/hkp/source' -require 'nickserver/couch_db/source' -require 'nickserver/request' - -module Nickserver - class RequestHandler - - def initialize(responder) - @responder = responder - end - - def respond_to(params, headers) - request = Nickserver::Request.new params, headers - response = handle request - send_response response.status, response.content - end - - protected - - def handle(request) - handler = handler_for_request request - handler.call request - rescue RuntimeError => exc - puts "Error: #{exc}" - puts exc.backtrace - ErrorResponse.new(exc.to_s) - end - - def handler_for_request(request) - if request.email - EmailHandler.new - elsif request.fingerprint - FingerprintHandler.new - else - Proc.new { Nickserver::Response.new(404, "Not Found\n") } - end - end - - class EmailHandler - - def call(request) - email = EmailAddress.new(request.email) - if email.invalid? - ErrorResponse.new("Not a valid address") - else - send_key(email, request) - end - end - - protected - - def send_key(email, request) - if local_address?(email, request) - source = Nickserver::CouchDB::Source.new - else - source = Nickserver::Hkp::Source.new - end - source.query(email) - rescue MissingHostHeader - ErrorResponse.new("HTTP request must include a Host header.") - end - - # - # Return true if the user address is for a user of this service provider. - # e.g. if the provider is example.org, then alice@example.org returns true. - # - # If 'domain' is not configured, we rely on the Host header of the HTTP request. - # - def local_address?(email, request) - domain = Config.domain || request.domain - raise MissingHostHeader if domain == '' - email.domain? domain - end - end - - class FingerprintHandler - - def call(request) - fingerprint = request.fingerprint - if fingerprint.length == 40 && !fingerprint[/\H/] - source = Nickserver::Hkp::Source.new - source.get_key_by_fingerprint(fingerprint) - else - ErrorResponse.new('Fingerprint invalid: ' + fingerprint) - end - end - - end - - class ErrorResponse < Nickserver::Response - def initialize(message) - @status = 500 - @message = message + "\n" - end - end - - def send_response(status = 200, content = '') - responder.respond status, content - end - - attr_reader :responder - - class MissingHostHeader < StandardError - end - end -end diff --git a/lib/nickserver/request_handlers/email_handler.rb b/lib/nickserver/request_handlers/email_handler.rb new file mode 100644 index 0000000..3f7515d --- /dev/null +++ b/lib/nickserver/request_handlers/email_handler.rb @@ -0,0 +1,49 @@ +require 'nickserver/email_address' +require 'nickserver/error_response' +require 'nickserver/hkp/source' +require 'nickserver/couch_db/source' + +module Nickserver + module RequestHandlers + class EmailHandler + + def call(request) + email = EmailAddress.new(request.email) + if email.invalid? + ErrorResponse.new("Not a valid address") + else + send_key(email, request) + end + end + + protected + + def send_key(email, request) + if local_address?(email, request) + source = Nickserver::CouchDB::Source.new + else + source = Nickserver::Hkp::Source.new + end + source.query(email) + rescue MissingHostHeader + ErrorResponse.new("HTTP request must include a Host header.") + end + + # + # Return true if the user address is for a user of this service provider. + # e.g. if the provider is example.org, then alice@example.org returns true. + # + # If 'domain' is not configured, we rely on the Host header of the HTTP request. + # + def local_address?(email, request) + domain = Config.domain || request.domain + raise MissingHostHeader if domain == '' + email.domain? domain + end + end + + + class MissingHostHeader < StandardError + end + end +end diff --git a/lib/nickserver/request_handlers/fingerprint_handler.rb b/lib/nickserver/request_handlers/fingerprint_handler.rb new file mode 100644 index 0000000..3202c41 --- /dev/null +++ b/lib/nickserver/request_handlers/fingerprint_handler.rb @@ -0,0 +1,20 @@ +require 'nickserver/hkp/source' +require 'nickserver/error_response' + +module Nickserver + module RequestHandlers + class FingerprintHandler + + def call(request) + fingerprint = request.fingerprint + if fingerprint.length == 40 && !fingerprint[/\H/] + source = Nickserver::Hkp::Source.new + source.get_key_by_fingerprint(fingerprint) + else + ErrorResponse.new('Fingerprint invalid: ' + fingerprint) + end + end + + end + end +end diff --git a/test/unit/dispatcher_test.rb b/test/unit/dispatcher_test.rb new file mode 100644 index 0000000..60d252b --- /dev/null +++ b/test/unit/dispatcher_test.rb @@ -0,0 +1,73 @@ +require 'test_helper' +require 'nickserver/dispatcher' + +class Nickserver::DispatcherTest < Minitest::Test + + def test_empty_query + handle + assert_response status: 404, content: "404 Not Found\n" + end + + def test_invalid_query + handle address: ['asdf'] + 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 + + def test_email_from_hkp + handle address: ['valid@email.tld'], headers: { "Host" => "http://nickserver.me" } + source = Minitest::Mock.new + source.expect :query, Nickserver::Response.new(200, "fake content"), [Nickserver::EmailAddress] + Nickserver::Hkp::Source.stub :new, source do + assert_response status: 200, content: "200 fake content" + end + end + + def test_fingerprint_to_short + handle fingerprint: ['44F2F455E28'] + assert_response status: 500, content: "500 Fingerprint invalid: 44F2F455E28\n" + end + + def test_fingerprint_is_not_hex + handle fingerprint: ['X36E738D69173C13Z709E44F2F455E2824D18DDX'] + assert_response status: 500, + content: "500 Fingerprint invalid: X36E738D69173C13Z709E44F2F455E2824D18DDX\n" + end + + def test_get_key_with_fingerprint_from_hkp + handle fingerprint: ['E36E738D69173C13D709E44F2F455E2824D18DDF'] + source = Minitest::Mock.new + source.expect :get_key_by_fingerprint, + Nickserver::Response.new(200, "fake fingerprint"), + ['E36E738D69173C13D709E44F2F455E2824D18DDF'] + Nickserver::Hkp::Source.stub :new, source do + assert_response status: 200, content: "200 fake fingerprint" + end + end + + protected + + def handle(params = {}) + @headers = params.delete(:headers) || {} + @params = Hash[ params.map{ |k,v| [k.to_s, v] } ] + end + + def assert_response(args) + responder.expect :respond, nil, [args[:status], args[:content]] + dispatcher.respond_to @params, @headers + responder.verify + end + + def dispatcher + Nickserver::Dispatcher.new responder + end + + def responder + @responder ||= Minitest::Mock.new + end + +end diff --git a/test/unit/request_handler_test.rb b/test/unit/request_handler_test.rb deleted file mode 100644 index d938706..0000000 --- a/test/unit/request_handler_test.rb +++ /dev/null @@ -1,73 +0,0 @@ -require 'test_helper' -require 'nickserver/request_handler' - -class Nickserver::RequestHandlerTest < Minitest::Test - - def test_empty_query - handle - assert_response status: 404, content: "404 Not Found\n" - end - - def test_invalid_query - handle address: ['asdf'] - 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 - - def test_email_from_hkp - handle address: ['valid@email.tld'], headers: { "Host" => "http://nickserver.me" } - source = Minitest::Mock.new - source.expect :query, Nickserver::Response.new(200, "fake content"), [Nickserver::EmailAddress] - Nickserver::Hkp::Source.stub :new, source do - assert_response status: 200, content: "200 fake content" - end - end - - def test_fingerprint_to_short - handle fingerprint: ['44F2F455E28'] - assert_response status: 500, content: "500 Fingerprint invalid: 44F2F455E28\n" - end - - def test_fingerprint_is_not_hex - handle fingerprint: ['X36E738D69173C13Z709E44F2F455E2824D18DDX'] - assert_response status: 500, - content: "500 Fingerprint invalid: X36E738D69173C13Z709E44F2F455E2824D18DDX\n" - end - - def test_get_key_with_fingerprint_from_hkp - handle fingerprint: ['E36E738D69173C13D709E44F2F455E2824D18DDF'] - source = Minitest::Mock.new - source.expect :get_key_by_fingerprint, - Nickserver::Response.new(200, "fake fingerprint"), - ['E36E738D69173C13D709E44F2F455E2824D18DDF'] - Nickserver::Hkp::Source.stub :new, source do - assert_response status: 200, content: "200 fake fingerprint" - end - end - - protected - - def handle(params = {}) - @headers = params.delete(:headers) || {} - @params = Hash[ params.map{ |k,v| [k.to_s, v] } ] - end - - def assert_response(args) - responder.expect :respond, nil, [args[:status], args[:content]] - handler.respond_to @params, @headers - responder.verify - end - - def handler - Nickserver::RequestHandler.new responder - end - - def responder - @responder ||= Minitest::Mock.new - end - -end -- cgit v1.2.3 From becd26b0bdf44b3625caaa7643914d0379a4fea5 Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 29 Aug 2016 10:26:54 +0200 Subject: refactor: let handlers check if they are applicable Instead of testing the preconditions for each handler in the dispatcher the dispatcher hands a request to one handler after the other until one of them responds. This is similar to the Chain of Responsibility patter but we iterate over the 'handler_chain' array instead of a linked list. To change the order of handlers or add other handlers change the array in the handler_chain function. --- lib/nickserver/dispatcher.rb | 33 ++++++++++++++++------ lib/nickserver/request_handlers/email_handler.rb | 9 ++++-- .../request_handlers/fingerprint_handler.rb | 7 +++++ 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/lib/nickserver/dispatcher.rb b/lib/nickserver/dispatcher.rb index b818b98..833a2ad 100644 --- a/lib/nickserver/dispatcher.rb +++ b/lib/nickserver/dispatcher.rb @@ -1,3 +1,18 @@ +# +# Dispatcher +# +# Dispatch a request so it get's handled by the correct handler. +# +# The dispatcher hands a request to one handler after the other until one of +# them responds. +# +# This is similar to the Chain of Responsibility patter but we iterate over the +# 'handler_chain' array instead of a linked list. +# +# To change the order of handlers or add other handlers change the array in the +# handler_chain function. +# + require 'nickserver/request' require 'nickserver/request_handlers/email_handler' require 'nickserver/request_handlers/fingerprint_handler' @@ -18,22 +33,22 @@ module Nickserver protected def handle(request) - handler = handler_for_request request - handler.call request + handler_chain.each do |handler| + response = handler.call request + return response if response + end rescue RuntimeError => exc puts "Error: #{exc}" puts exc.backtrace ErrorResponse.new(exc.to_s) end - def handler_for_request(request) - if request.email - RequestHandlers::EmailHandler.new - elsif request.fingerprint - RequestHandlers::FingerprintHandler.new - else + def handler_chain + [ + RequestHandlers::EmailHandler.new, + RequestHandlers::FingerprintHandler.new, Proc.new { Nickserver::Response.new(404, "Not Found\n") } - end + ] end def send_response(status = 200, content = '') diff --git a/lib/nickserver/request_handlers/email_handler.rb b/lib/nickserver/request_handlers/email_handler.rb index 3f7515d..b163b27 100644 --- a/lib/nickserver/request_handlers/email_handler.rb +++ b/lib/nickserver/request_handlers/email_handler.rb @@ -8,6 +8,13 @@ module Nickserver class EmailHandler def call(request) + return unless request.email + handle_request(request) + end + + protected + + def handle_request(request) email = EmailAddress.new(request.email) if email.invalid? ErrorResponse.new("Not a valid address") @@ -16,8 +23,6 @@ module Nickserver end end - protected - def send_key(email, request) if local_address?(email, request) source = Nickserver::CouchDB::Source.new diff --git a/lib/nickserver/request_handlers/fingerprint_handler.rb b/lib/nickserver/request_handlers/fingerprint_handler.rb index 3202c41..3c04fcd 100644 --- a/lib/nickserver/request_handlers/fingerprint_handler.rb +++ b/lib/nickserver/request_handlers/fingerprint_handler.rb @@ -6,6 +6,13 @@ module Nickserver class FingerprintHandler def call(request) + return unless request.fingerprint + handle_request(request) + end + + protected + + def handle_request(request) fingerprint = request.fingerprint if fingerprint.length == 40 && !fingerprint[/\H/] source = Nickserver::Hkp::Source.new -- cgit v1.2.3 From 55006b3ce5967fde08081bfd56d56f76dbaf7c53 Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 29 Aug 2016 10:36:05 +0200 Subject: cleanup: remove outdated ideas Lookup and InvalidSource were experiments for a design for the dispatching. Our new dispatcher seems better. --- lib/nickserver/invalid_source.rb | 14 -------------- lib/nickserver/lookup.rb | 31 ------------------------------- test/unit/invalid_source_test.rb | 16 ---------------- test/unit/lookup_test.rb | 23 ----------------------- 4 files changed, 84 deletions(-) delete mode 100644 lib/nickserver/invalid_source.rb delete mode 100644 lib/nickserver/lookup.rb delete mode 100644 test/unit/invalid_source_test.rb delete mode 100644 test/unit/lookup_test.rb diff --git a/lib/nickserver/invalid_source.rb b/lib/nickserver/invalid_source.rb deleted file mode 100644 index dac245a..0000000 --- a/lib/nickserver/invalid_source.rb +++ /dev/null @@ -1,14 +0,0 @@ -# -# This is a dummy source for invalid queries. -# It simply always returns 500 and "Not a valid address" -# - -module Nickserver - class InvalidSource - - def query(nick) - yield 500, "Not a valid address" - end - - end -end diff --git a/lib/nickserver/lookup.rb b/lib/nickserver/lookup.rb deleted file mode 100644 index 105e77e..0000000 --- a/lib/nickserver/lookup.rb +++ /dev/null @@ -1,31 +0,0 @@ -require 'nickserver/invalid_source' - -module Nickserver - class Lookup - - attr_reader :nick - - def initialize(nick) - @nick = nick - end - - def respond_with(responder) - query do |status, content| - responder.send_response status: status, content: content - end - end - - protected - - def query(&block) - source.query nick, &block - end - - def source - if nick.invalid? then Nickserver::InvalidSource - elsif nick.local? then Nickserver::Config.local_source - else Nickserver::Config.remote_source - end - end - end -end diff --git a/test/unit/invalid_source_test.rb b/test/unit/invalid_source_test.rb deleted file mode 100644 index 37a38fc..0000000 --- a/test/unit/invalid_source_test.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'test_helper' -require 'nickserver/invalid_source' - -class Nickserver::InvalidSourceTest < Minitest::Test - - def test_query - adapter.query(nil) do |status, content| - assert_equal 500, status - assert_equal "Not a valid address", content - end - end - - def adapter - Nickserver::InvalidSource.new - end -end diff --git a/test/unit/lookup_test.rb b/test/unit/lookup_test.rb deleted file mode 100644 index ac827e0..0000000 --- a/test/unit/lookup_test.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'test_helper' -require 'minitest/mock' -require 'nickserver/lookup' - -class TestLookup < Nickserver::Lookup - - def query - yield 200, 'yeah' - end - -end - -class LookupTest < Minitest::Test - - def test_responding - responder = Minitest::Mock.new - responder.expect :send_response, nil, - [{status: 200, content: 'yeah'}] - lookup = TestLookup.new nil - lookup.respond_with responder - responder.verify - end -end -- cgit v1.2.3 From 0784391a21b75ca52892e992a614b0f927ade00e Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 29 Aug 2016 11:59:54 +0200 Subject: refactor: split EmailHandler in 3 InvalidEmailHandler - handle emails with an invalid format LocalEmailHandler - handle emails on the local domain EmailHandler - handle all other emails by using hkp This is a preparation to add leap provider email lookup and remove hkp eventually. But for now we keep the behaviour the same and only refactor. --- lib/nickserver/dispatcher.rb | 4 ++ lib/nickserver/request_handlers/email_handler.rb | 40 ++---------- .../request_handlers/invalid_email_handler.rb | 15 +++++ .../request_handlers/local_email_handler.rb | 32 ++++++++++ test/integration/dispatcher_test.rb | 73 ++++++++++++++++++++++ test/unit/dispatcher_test.rb | 73 ---------------------- .../request_handlers/local_email_handler_test.rb | 65 +++++++++++++++++++ 7 files changed, 193 insertions(+), 109 deletions(-) create mode 100644 lib/nickserver/request_handlers/invalid_email_handler.rb create mode 100644 lib/nickserver/request_handlers/local_email_handler.rb create mode 100644 test/integration/dispatcher_test.rb delete mode 100644 test/unit/dispatcher_test.rb create mode 100644 test/unit/request_handlers/local_email_handler_test.rb diff --git a/lib/nickserver/dispatcher.rb b/lib/nickserver/dispatcher.rb index 833a2ad..8bcfd05 100644 --- a/lib/nickserver/dispatcher.rb +++ b/lib/nickserver/dispatcher.rb @@ -14,6 +14,8 @@ # require 'nickserver/request' +require 'nickserver/request_handlers/invalid_email_handler' +require 'nickserver/request_handlers/local_email_handler' require 'nickserver/request_handlers/email_handler' require 'nickserver/request_handlers/fingerprint_handler' @@ -45,6 +47,8 @@ module Nickserver def handler_chain [ + RequestHandlers::InvalidEmailHandler.new, + RequestHandlers::LocalEmailHandler.new, RequestHandlers::EmailHandler.new, RequestHandlers::FingerprintHandler.new, Proc.new { Nickserver::Response.new(404, "Not Found\n") } diff --git a/lib/nickserver/request_handlers/email_handler.rb b/lib/nickserver/request_handlers/email_handler.rb index b163b27..96fcaf3 100644 --- a/lib/nickserver/request_handlers/email_handler.rb +++ b/lib/nickserver/request_handlers/email_handler.rb @@ -1,7 +1,5 @@ require 'nickserver/email_address' -require 'nickserver/error_response' require 'nickserver/hkp/source' -require 'nickserver/couch_db/source' module Nickserver module RequestHandlers @@ -9,46 +7,16 @@ module Nickserver def call(request) return unless request.email - handle_request(request) - end - - protected - - def handle_request(request) email = EmailAddress.new(request.email) - if email.invalid? - ErrorResponse.new("Not a valid address") - else - send_key(email, request) - end - end - - def send_key(email, request) - if local_address?(email, request) - source = Nickserver::CouchDB::Source.new - else - source = Nickserver::Hkp::Source.new - end source.query(email) - rescue MissingHostHeader - ErrorResponse.new("HTTP request must include a Host header.") end - # - # Return true if the user address is for a user of this service provider. - # e.g. if the provider is example.org, then alice@example.org returns true. - # - # If 'domain' is not configured, we rely on the Host header of the HTTP request. - # - def local_address?(email, request) - domain = Config.domain || request.domain - raise MissingHostHeader if domain == '' - email.domain? domain - end - end + protected + def source + Nickserver::Hkp::Source.new + end - class MissingHostHeader < StandardError end end end diff --git a/lib/nickserver/request_handlers/invalid_email_handler.rb b/lib/nickserver/request_handlers/invalid_email_handler.rb new file mode 100644 index 0000000..eaf8156 --- /dev/null +++ b/lib/nickserver/request_handlers/invalid_email_handler.rb @@ -0,0 +1,15 @@ +require 'nickserver/email_address' +require 'nickserver/error_response' + +module Nickserver + module RequestHandlers + class InvalidEmailHandler + def call(request) + return unless request.email + email = EmailAddress.new(request.email) + ErrorResponse.new("Not a valid address") if email.invalid? + end + + end + end +end diff --git a/lib/nickserver/request_handlers/local_email_handler.rb b/lib/nickserver/request_handlers/local_email_handler.rb new file mode 100644 index 0000000..1f2abc2 --- /dev/null +++ b/lib/nickserver/request_handlers/local_email_handler.rb @@ -0,0 +1,32 @@ +require 'nickserver/email_address' +require 'nickserver/error_response' +require 'nickserver/couch_db/source' + +module Nickserver + module RequestHandlers + class LocalEmailHandler + + def call(request) + return nil unless request.email + domain = Config.domain || request.domain + return missing_domain_response if domain.nil? || domain == '' + email = EmailAddress.new(request.email) + return nil unless email.domain?(domain) + source.query email + end + + protected + + attr_reader :domain + + def source + Nickserver::CouchDB::Source.new + end + + def missing_domain_response + ErrorResponse.new "HTTP request must include a Host header." + end + + end + end +end diff --git a/test/integration/dispatcher_test.rb b/test/integration/dispatcher_test.rb new file mode 100644 index 0000000..60d252b --- /dev/null +++ b/test/integration/dispatcher_test.rb @@ -0,0 +1,73 @@ +require 'test_helper' +require 'nickserver/dispatcher' + +class Nickserver::DispatcherTest < Minitest::Test + + def test_empty_query + handle + assert_response status: 404, content: "404 Not Found\n" + end + + def test_invalid_query + handle address: ['asdf'] + 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 + + def test_email_from_hkp + handle address: ['valid@email.tld'], headers: { "Host" => "http://nickserver.me" } + source = Minitest::Mock.new + source.expect :query, Nickserver::Response.new(200, "fake content"), [Nickserver::EmailAddress] + Nickserver::Hkp::Source.stub :new, source do + assert_response status: 200, content: "200 fake content" + end + end + + def test_fingerprint_to_short + handle fingerprint: ['44F2F455E28'] + assert_response status: 500, content: "500 Fingerprint invalid: 44F2F455E28\n" + end + + def test_fingerprint_is_not_hex + handle fingerprint: ['X36E738D69173C13Z709E44F2F455E2824D18DDX'] + assert_response status: 500, + content: "500 Fingerprint invalid: X36E738D69173C13Z709E44F2F455E2824D18DDX\n" + end + + def test_get_key_with_fingerprint_from_hkp + handle fingerprint: ['E36E738D69173C13D709E44F2F455E2824D18DDF'] + source = Minitest::Mock.new + source.expect :get_key_by_fingerprint, + Nickserver::Response.new(200, "fake fingerprint"), + ['E36E738D69173C13D709E44F2F455E2824D18DDF'] + Nickserver::Hkp::Source.stub :new, source do + assert_response status: 200, content: "200 fake fingerprint" + end + end + + protected + + def handle(params = {}) + @headers = params.delete(:headers) || {} + @params = Hash[ params.map{ |k,v| [k.to_s, v] } ] + end + + def assert_response(args) + responder.expect :respond, nil, [args[:status], args[:content]] + dispatcher.respond_to @params, @headers + responder.verify + end + + def dispatcher + Nickserver::Dispatcher.new responder + end + + def responder + @responder ||= Minitest::Mock.new + end + +end diff --git a/test/unit/dispatcher_test.rb b/test/unit/dispatcher_test.rb deleted file mode 100644 index 60d252b..0000000 --- a/test/unit/dispatcher_test.rb +++ /dev/null @@ -1,73 +0,0 @@ -require 'test_helper' -require 'nickserver/dispatcher' - -class Nickserver::DispatcherTest < Minitest::Test - - def test_empty_query - handle - assert_response status: 404, content: "404 Not Found\n" - end - - def test_invalid_query - handle address: ['asdf'] - 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 - - def test_email_from_hkp - handle address: ['valid@email.tld'], headers: { "Host" => "http://nickserver.me" } - source = Minitest::Mock.new - source.expect :query, Nickserver::Response.new(200, "fake content"), [Nickserver::EmailAddress] - Nickserver::Hkp::Source.stub :new, source do - assert_response status: 200, content: "200 fake content" - end - end - - def test_fingerprint_to_short - handle fingerprint: ['44F2F455E28'] - assert_response status: 500, content: "500 Fingerprint invalid: 44F2F455E28\n" - end - - def test_fingerprint_is_not_hex - handle fingerprint: ['X36E738D69173C13Z709E44F2F455E2824D18DDX'] - assert_response status: 500, - content: "500 Fingerprint invalid: X36E738D69173C13Z709E44F2F455E2824D18DDX\n" - end - - def test_get_key_with_fingerprint_from_hkp - handle fingerprint: ['E36E738D69173C13D709E44F2F455E2824D18DDF'] - source = Minitest::Mock.new - source.expect :get_key_by_fingerprint, - Nickserver::Response.new(200, "fake fingerprint"), - ['E36E738D69173C13D709E44F2F455E2824D18DDF'] - Nickserver::Hkp::Source.stub :new, source do - assert_response status: 200, content: "200 fake fingerprint" - end - end - - protected - - def handle(params = {}) - @headers = params.delete(:headers) || {} - @params = Hash[ params.map{ |k,v| [k.to_s, v] } ] - end - - def assert_response(args) - responder.expect :respond, nil, [args[:status], args[:content]] - dispatcher.respond_to @params, @headers - responder.verify - end - - def dispatcher - Nickserver::Dispatcher.new responder - end - - def responder - @responder ||= Minitest::Mock.new - end - -end diff --git a/test/unit/request_handlers/local_email_handler_test.rb b/test/unit/request_handlers/local_email_handler_test.rb new file mode 100644 index 0000000..8f303ec --- /dev/null +++ b/test/unit/request_handlers/local_email_handler_test.rb @@ -0,0 +1,65 @@ +require 'test_helper' +require 'nickserver/request_handlers/local_email_handler' + +class LocalEmailHandlerTest < MiniTest::Test + + def test_no_email + assert_refuses + end + + def test_remote_email + assert_refuses email: 'me@remote.tld', domain: 'local.tld' + end + + def test_local_email + assert_handles email: 'me@local.tld', domain: 'local.tld' + end + + def test_missing_host_header + Nickserver::Config.stub :domain, nil do + assert_responds_with_error "HTTP request must include a Host header.", + email: 'me@local.tld' + end + end + + protected + + def handler + Nickserver::RequestHandlers::LocalEmailHandler.new + end + + def source + source = Minitest::Mock.new + source.expect :query, + 'response', + [Nickserver::EmailAddress] + source + end + + def assert_handles(opts) + Nickserver::CouchDB::Source.stub :new, source do + assert_equal 'response', handle(request(opts)) + end + end + + def assert_responds_with_error(msg, opts) + response = handle(request(opts)) + assert_equal 500, response.status + assert_equal "500 #{msg}\n", response.content + end + + def assert_refuses(opts = {}) + assert_nil handle(request(opts)) + end + + def handle(request) + handler.call(request) + end + + def request(opts = {}) + params = {'address' => [opts[:email]]} + headers = {'Host' => opts[:domain]} + Nickserver::Request.new params, headers + end + +end -- cgit v1.2.3