Add SchoolEmailDomainValidator

Extract domain validation from school_email_domain model to module
Call it from SchoolEmailDomain and School

Author: PetarSimonovic

app/models/school.rb

@@ -118,7 +118,10 @@ def import_in_progress?
   end
 
   def valid_domain?(candidate_domain)
-    school_email_domains.exists?(domain: candidate_domain)
+    validated_domain = SchoolEmailDomainValidator.call(candidate_domain)
+    school_email_domains.exists?(domain: validated_domain)
+  rescue ::SchoolEmailDomainValidator::Error
+    false
   end
 
   private

app/models/school_email_domain.rb

@@ -11,32 +11,8 @@ class SchoolEmailDomain < ApplicationRecord
   private
 
   def validate_domain
-    return if domain.blank?
-
-    value = domain.strip.downcase
-    # Add a scheme unless it already has one, so URI can parse it
-    value = "http://#{value}" unless %r{\A[a-z][a-z0-9+\-.]*://}i.match?(value)
-    uri = URI.parse(value)
-    host = uri.host&.delete_suffix('.')
-
-    validate_host(host)
-  rescue URI::InvalidURIError
-    errors.add(:domain, :invalid)
-  end
-
-  def validate_host(host)
-    accounts_host_format =
-      /\A\s*(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)+[A-Za-z]{2,63}\s*\z/i
-
-    unless host&.match?(accounts_host_format)
-      errors.add(:domain, :invalid)
-      return
-    end
-
-    if PublicSuffix.valid?(host)
-      self.domain = host
-    else
-      errors.add(:domain, :invalid)
-    end
+    self.domain = SchoolEmailDomainValidator.call(domain)
+  rescue ::SchoolEmailDomainValidator::Error => e
+    errors.add(:domain, e.error_code)
   end
 end

lib/school_email_domain_validator.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module SchoolEmailDomainValidator
+  class Error < StandardError
+    def error_code
+      :invalid
+    end
+  end
+
+  class BlankDomainError < Error
+    def error_code
+      :blank
+    end
+  end
+
+  class InvalidURIError < Error
+    def error_code
+      :invalid_uri
+    end
+  end
+
+  class InvalidHostError < Error
+    def error_code
+      :invalid_host
+    end
+  end
+
+  class PublicSuffixError < Error
+    def error_code
+      :invalid_public_suffix
+    end
+  end
+
+  def self.call(domain)
+    raise BlankDomainError if domain.blank?
+
+    validate_domain(domain)
+  end
+
+  def self.validate_domain(domain)
+    value = domain.strip.downcase
+    # Add a scheme unless it already has one, so URI can parse it
+    value = "http://#{value}" unless %r{\A[a-z][a-z0-9+\-.]*://}i.match?(value)
+    uri = URI.parse(value)
+    host = uri.host&.delete_suffix('.')
+
+    validate_host(host)
+  rescue URI::InvalidURIError
+    raise InvalidURIError
+  end
+
+  def self.validate_host(host)
+    accounts_host_format =
+      /\A\s*(?:[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?\.)+[A-Za-z]{2,63}\s*\z/i
+
+    raise InvalidHostError unless host&.match?(accounts_host_format)
+
+    raise PublicSuffixError, 'domain has no registered public suffix' unless PublicSuffix.valid?(host)
+
+    host
+  end
+end

spec/lib/school_email_domain_validator_spec.rb

@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe SchoolEmailDomainValidator do
+  describe '.call' do
+    it 'returns success with a normalised host for a valid domain' do
+      result = described_class.call('example.com')
+
+      expect(result).to eq('example.com')
+    end
+
+    context 'with a valid domain' do
+      it 'extracts the host for http' do
+        result = described_class.call('http://mail.school.edu/path?query=1')
+        expect(result).to eq('mail.school.edu')
+      end
+
+      it 'extracts the host for https' do
+        result = described_class.call('https://mail.school.edu/path?query=1')
+        expect(result).to eq('mail.school.edu')
+      end
+
+      it 'removes a trailing dot' do
+        result = described_class.call('example.edu.')
+        expect(result).to eq('example.edu')
+      end
+
+      it 'lowercases the host' do
+        result = described_class.call('EXAMPLE.EDU')
+        expect(result).to eq('example.edu')
+      end
+
+      it 'removes a leading @' do
+        result = described_class.call('@example.edu')
+        expect(result).to eq('example.edu')
+      end
+
+      it 'accepts a subdomain' do
+        result = described_class.call('mail.example.edu')
+        expect(result).to eq('mail.example.edu')
+      end
+
+      it 'accepts a hostname er a multi-part public suffix' do
+        result = described_class.call('school.example.co.uk')
+        expect(result).to eq('school.example.co.uk')
+      end
+
+      it 'accepts a district-style with a multi-part public suffix' do
+        result = described_class.call('school.k12.tx.us')
+        expect(result).to eq('school.k12.tx.us')
+      end
+    end
+
+    context 'when the domain is blank' do
+      it 'raises BlankDomain' do
+        expect { described_class.call('') }.to raise_error(SchoolEmailDomainValidator::BlankDomainError)
+      end
+    end
+
+    context 'when the domain has an invalid URI' do
+      it 'raises InvalidURIError' do
+        expect { described_class.call('https://exa mple.com') }.to raise_error(SchoolEmailDomainValidator::InvalidURIError)
+      end
+    end
+
+    context 'when the host does not match accounts_host_format' do
+      it 'raises InvalidHostError' do
+        expect { described_class.call('school_domain.edu') }
+          .to raise_error(SchoolEmailDomainValidator::InvalidHostError)
+      end
+    end
+
+    context 'when the host fails PublicSuffix validation' do
+      it 'raises PublicSuffixError' do
+        expect { described_class.call('co.uk') }
+          .to raise_error(SchoolEmailDomainValidator::PublicSuffixError)
+      end
+    end
+  end
+end

spec/models/school_email_domain_spec.rb

@@ -15,79 +15,15 @@
   end
 
   context 'with a valid domain' do
-    describe 'domain normalisation' do
-      context 'when given a full url' do
-        let(:domain) { 'http://mail.school.edu/path?query=1' }
-
-        it 'extracts the host' do
-          expect(school_email_domain.domain).to eq('mail.school.edu')
-        end
-      end
-
-      context 'when given a full https url' do
-        let(:domain) { 'https://mail.school.edu/path' }
-
-        it 'extracts the host' do
-          expect(school_email_domain.domain).to eq('mail.school.edu')
-        end
-      end
-
-      context 'when given a domain with a trailing dot' do
-        let(:domain) { 'EXAMPLE.EDU.' }
-
-        it 'stores the host without the trailing dot' do
-          expect(school_email_domain.domain).to eq('example.edu')
-        end
-      end
-
-      context 'when given a capitalised host' do
-        let(:domain) { 'EXAMPLE.EDU' }
-
-        it 'downcases the host' do
-          expect(school_email_domain.domain).to eq('example.edu')
-        end
-      end
-
-      context 'with a leading @' do
-        let(:domain) { '@example.edu' }
-
-        it 'removes the @' do
-          expect(school_email_domain.domain).to eq('example.edu')
-        end
-      end
+    it 'accepts the domain' do
+      expect(school_email_domain.domain).to eq('example.edu')
     end
 
-    describe 'public suffix list validation' do
-      context 'when there is at least one registrable label before the public suffix' do
-        let(:domain) { 'example.edu' }
-
-        it 'accepts the domain' do
-          expect(school_email_domain.domain).to eq('example.edu')
-        end
-      end
-
-      context 'when there is a subdomain before a valid public suffix' do
-        let(:domain) { 'mail.example.edu' }
-
-        it 'accepts the domain' do
-          expect(school_email_domain.domain).to eq('mail.example.edu')
-        end
-      end
-
-      context 'when there is a hostname under a multi-part public suffix' do
-        let(:domain) { 'school.example.co.uk' }
-
-        it 'accepts the domain' do
-          expect(school_email_domain.domain).to eq('school.example.co.uk')
-        end
-      end
-
-      context 'when given a district-style host with a multi-part public suffix' do
-        let(:domain) { 'school.k12.tx.us' }
+    describe 'domain normalisation' do
+      let(:domain) { '@EXAMPLE.EDU.' }
 
-        it 'accepts the domain' do
-          expect(school_email_domain.domain).to eq('school.k12.tx.us')
-        end
+      it 'normalises a domain' do
+        expect(school_email_domain.domain).to eq('example.edu')
       end
     end
 

spec/models/school_spec.rb

@@ -685,7 +685,8 @@
 
   describe '#valid_domain?' do
     let(:valid_domain) { 'valid.edu' }
-    let(:invalid_domain) { 'invalid.edu' }
+    let(:unregistered_domain) { 'invalid.edu' }
+    let(:invalid_domain) { 'not a domain' }
 
     before do
       SchoolEmailDomain.create!(school:, domain: valid_domain)
@@ -696,7 +697,7 @@
     end
 
     it 'returns false when school has not registered the email domain' do
-      expect(school.valid_domain?(invalid_domain)).to be(false)
+      expect(school.valid_domain?(unregistered_domain)).to be(false)
     end
   end
 end