From 26340a930461d440742dc29691137913e21dbe91 Mon Sep 17 00:00:00 2001
From: Colin Darie <colin@darie.eu>
Date: Mon, 4 Jun 2018 14:06:37 +0200
Subject: [PATCH] Notifier service architecture

---
 Guardfile                                    |   1 +
 app/models/check.rb                          |   9 +-
 app/models/notification.rb                   |  16 ++-
 app/models/user.rb                           |   2 +
 app/services/notifier.rb                     |   9 ++
 app/services/notifier/channels/base.rb       |  41 +++++++
 app/services/notifier/processor.rb           |  56 +++++++++
 app/services/notifier/resolver.rb            |  31 +++++
 config/chexpire.example.yml                  |   3 +
 config/chexpire.test.yml                     |   3 +
 test/factories/.rubocop.yml                  |   3 +
 test/factories/checks.rb                     |  18 +++
 test/factories/notifications.rb              |   1 +
 test/factories/users.rb                      |   2 +-
 test/models/check_test.rb                    |  23 +++-
 test/services/notifier/channels/base_test.rb |  49 ++++++++
 test/services/notifier/processor_test.rb     |  31 +++++
 test/services/notifier/resolver_test.rb      | 113 +++++++++++++++++++
 test/services/notifier_test.rb               |  25 ++++
 test/test_helper.rb                          |   3 +-
 20 files changed, 430 insertions(+), 9 deletions(-)
 create mode 100644 app/services/notifier.rb
 create mode 100644 app/services/notifier/channels/base.rb
 create mode 100644 app/services/notifier/processor.rb
 create mode 100644 app/services/notifier/resolver.rb
 create mode 100644 test/services/notifier/channels/base_test.rb
 create mode 100644 test/services/notifier/processor_test.rb
 create mode 100644 test/services/notifier/resolver_test.rb
 create mode 100644 test/services/notifier_test.rb

diff --git a/Guardfile b/Guardfile
index 38acbea..03d7038 100644
--- a/Guardfile
+++ b/Guardfile
@@ -21,6 +21,7 @@ guard "minitest", spring: "bin/rails test" do
   watch(%r{^app/controllers/application_controller\.rb$}) { "test/controllers" }
   watch(%r{^app/controllers/(.+)_controller\.rb$}) { |m| "test/integration/#{m[1]}_test.rb" }
   watch(%r{^app/views/(.+)_mailer/.+}) { |m| "test/mailers/#{m[1]}_mailer_test.rb" }
+  watch(%r{^app/services/notifier/.+\.rb}) { |_m| "test/services/notifier" }
   watch(%r{^app/services/whois/.+\.rb}) { |_m| "test/services/whois" }
   watch(%r{^lib/(.*/)?([^/]+)\.rb$}) { |m| "test/#{m[1]}test_#{m[2]}.rb" }
   watch(%r{^test/.+_test\.rb$})
diff --git a/app/models/check.rb b/app/models/check.rb
index 4971ccc..82e9749 100644
--- a/app/models/check.rb
+++ b/app/models/check.rb
@@ -46,9 +46,10 @@ class Check < ApplicationRecord
   validates :comment, length: { maximum: 255 }
   validates :vendor, length: { maximum: 255 }
 
+  after_update :reset_notifications
   after_save :enqueue_sync
 
-  protected
+  private
 
   def domain_created_at_past
     errors.add(:domain_created_at, :past) if domain_created_at.present? && domain_created_at.future?
@@ -64,4 +65,10 @@ class Check < ApplicationRecord
 
     WhoisSyncJob.perform_later(id) if domain?
   end
+
+  def reset_notifications
+    return unless (saved_changes.keys & %w[domain domain_expires_at]).present?
+
+    notifications.each(&:reset!)
+  end
 end
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 5ead941..21983f3 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -24,11 +24,21 @@
 class Notification < ApplicationRecord
   belongs_to :check
 
-  enum kind: [:email]
+  enum channel: [:email]
   enum status: [:pending, :ongoing, :succeed, :failed]
 
-  validates :kind, presence: true
   validates :channel, presence: true
-  validates :recipient, presence: true
   validates :delay, presence: true
+  validates :recipient, presence: true
+
+  def pending!
+    self.sent_at = nil
+    super
+  end
+  alias reset! pending!
+
+  def ongoing!
+    self.sent_at = Time.now
+    super
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 33f6a36..adbbc7b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -37,4 +37,6 @@ class User < ApplicationRecord
 
   has_many :checks
   validates :tos_accepted, acceptance: true
+
+  scope :notifications_disabled, -> { where(notifications_enabled: false) }
 end
diff --git a/app/services/notifier.rb b/app/services/notifier.rb
new file mode 100644
index 0000000..5927cde
--- /dev/null
+++ b/app/services/notifier.rb
@@ -0,0 +1,9 @@
+module Notifier
+  class << self
+    def process_all(configuration = nil)
+      processor = Processor.new(configuration)
+      processor.process_expires_soon
+      processor.process_recurrent_failures
+    end
+  end
+end
diff --git a/app/services/notifier/channels/base.rb b/app/services/notifier/channels/base.rb
new file mode 100644
index 0000000..b63a52d
--- /dev/null
+++ b/app/services/notifier/channels/base.rb
@@ -0,0 +1,41 @@
+module Notifier
+  module Channels
+    class Base
+      def notify(reason, notification) # rubocop:disable Metrics/MethodLength
+        return unless supports?(reason, notification)
+
+        notification.ongoing!
+
+        case [notification.check.kind.to_sym, reason]
+        when [:domain, :expires_soon]
+          domain_notify_expires_soon(notification)
+        when [:domain, :recurrent_failures]
+          domain_notify_recurrent_failures(notification)
+        else
+          fail ArgumentError,
+            "Invalid notification reason `#{reason}` for check kind `#{notification.check.kind}`."
+        end
+      end
+
+      private
+
+      # :nocov:
+      def supports?(_reason, _notification)
+        fail NotImplementedError,
+          "#{self.class.name} channel did not implemented method #{__callee__}"
+      end
+
+      # domain notifications
+      def domain_notify_expires_soon(_notification)
+        fail NotImplementedError,
+          "Channel #{self.class.name} does not implement #{__callee__}"
+      end
+
+      def domain_notify_recurrent_failures(_notification)
+        fail NotImplementedError,
+          "Channel #{self.class.name} does not implement #{__callee__}"
+      end
+      # :nocov:
+    end
+  end
+end
diff --git a/app/services/notifier/processor.rb b/app/services/notifier/processor.rb
new file mode 100644
index 0000000..2d7fe15
--- /dev/null
+++ b/app/services/notifier/processor.rb
@@ -0,0 +1,56 @@
+module Notifier
+  Configuration = Struct.new(:interval, :failure_days)
+
+  class Processor
+    attr_reader :configuration
+    attr_reader :channels
+    attr_reader :resolver
+
+    def initialize(configuration = nil)
+      @configuration = configuration || default_configuration
+
+      @resolver = Resolver.new
+      @channels = {
+        email: Channels::Email.new,
+      }
+    end
+
+    def process_expires_soon
+      resolver.resolve_expires_soon.find_each do |notification|
+        notifier_channel_for(notification).notify(:expires_soon, notification)
+
+        sleep configuration.interval
+      end
+    end
+
+    def process_recurrent_failures
+      resolver.resolve_check_failed.find_each do |notification|
+        next unless should_notify_for_recurrent_failures?(notification)
+
+        notifier_channel_for(notification).notify(:recurrent_failures, notification)
+
+        sleep configuration.interval
+      end
+    end
+
+    private
+
+    def default_configuration
+      config = Rails.configuration.chexpire.fetch("notifier", {})
+
+      Configuration.new(
+        config.fetch("interval") { 0.00 },
+        config.fetch("failures_days") { 3 },
+      )
+    end
+
+    def notifier_channel_for(notification)
+      channels.fetch(notification.channel.to_sym)
+    end
+
+    def should_notify_for_recurrent_failures?(_notification)
+      true
+      # TODO: dependent of logs consecutive failures
+    end
+  end
+end
diff --git a/app/services/notifier/resolver.rb b/app/services/notifier/resolver.rb
new file mode 100644
index 0000000..21e1fa8
--- /dev/null
+++ b/app/services/notifier/resolver.rb
@@ -0,0 +1,31 @@
+module Notifier
+  class Resolver
+    def resolve_expires_soon
+      scope
+        .where("checks.domain_expires_at >= CURDATE()")
+        .where("DATE(checks.domain_expires_at)
+          <= DATE_ADD(CURDATE(), INTERVAL notifications.delay DAY)")
+    end
+
+    def resolve_check_failed
+      # Only gets here the checks having its last run in error
+      # Logical rules are in plain ruby inside processor
+      scope
+        .includes(check: :logs)
+        .where("checks.last_success_at <= DATE_SUB(checks.last_run_at, INTERVAL 5 MINUTE)")
+    end
+
+    private
+
+    def scope
+      Notification
+        .includes(:check)
+        .where(status: [:pending, :failed], checks: { active: true })
+        .where.not(checks: { user: ignore_users })
+    end
+
+    def ignore_users
+      @ignore_users ||= User.notifications_disabled.pluck(:id)
+    end
+  end
+end
diff --git a/config/chexpire.example.yml b/config/chexpire.example.yml
index 0b133c0..3543eaf 100644
--- a/config/chexpire.example.yml
+++ b/config/chexpire.example.yml
@@ -1,5 +1,8 @@
 default: &default
   mailer_default_from: "from@example.org"
+  notifier:
+    interval: 0.00
+    failure_days: 3
 
 development:
   <<: *default
diff --git a/config/chexpire.test.yml b/config/chexpire.test.yml
index 152302f..371a542 100644
--- a/config/chexpire.test.yml
+++ b/config/chexpire.test.yml
@@ -1,3 +1,6 @@
 test:
   mailer_default_from: "contact@chexpire.org"
   host: "localhost"
+  notifier:
+    interval: 0.00
+    failure_days: 3
diff --git a/test/factories/.rubocop.yml b/test/factories/.rubocop.yml
index 3a888e5..dd2089a 100644
--- a/test/factories/.rubocop.yml
+++ b/test/factories/.rubocop.yml
@@ -2,3 +2,6 @@ inherit_from: ../../.rubocop.yml
 
 Style/BlockDelimiters:
   EnforcedStyle: line_count_based
+
+Metrics/BlockLength:
+  Enabled: false
diff --git a/test/factories/checks.rb b/test/factories/checks.rb
index b7dfd84..181327f 100644
--- a/test/factories/checks.rb
+++ b/test/factories/checks.rb
@@ -49,5 +49,23 @@ FactoryBot.define do
       domain_updated_at nil
       domain_expires_at nil
     end
+
+    trait :expires_next_week do
+      domain_expires_at 1.week.from_now
+    end
+
+    trait :last_runs_failed do
+      last_run_at Time.now - 90.minutes
+      last_success_at 1.week.ago - 2.hours
+    end
+
+    trait :last_run_succeed do
+      last_run_at 1.hour.ago
+      last_success_at 1.hour.ago
+    end
+
+    trait :inactive do
+      active false
+    end
   end
 end
diff --git a/test/factories/notifications.rb b/test/factories/notifications.rb
index 8dc58f6..8bb069f 100644
--- a/test/factories/notifications.rb
+++ b/test/factories/notifications.rb
@@ -40,6 +40,7 @@ FactoryBot.define do
 
     trait :succeed do
       status :succeed
+      sent_at { 1.day.ago }
     end
 
     trait :failed do
diff --git a/test/factories/users.rb b/test/factories/users.rb
index 6620b56..d73cba4 100644
--- a/test/factories/users.rb
+++ b/test/factories/users.rb
@@ -32,7 +32,7 @@ require "securerandom"
 
 FactoryBot.define do
   factory :user do
-    email { "user-#{SecureRandom.random_number}@chexpire.org" }
+    sequence(:email) { |n| "user-#{n}@chexpire.org" }
     password "password"
     confirmed_at Time.new(2018, 4, 1, 12, 0, 0, "+02:00")
     notifications_enabled true
diff --git a/test/models/check_test.rb b/test/models/check_test.rb
index 83f77aa..1dde49b 100644
--- a/test/models/check_test.rb
+++ b/test/models/check_test.rb
@@ -29,7 +29,24 @@
 require "test_helper"
 
 class CheckTest < ActiveSupport::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  test "notifications are resetted when domain expiration date has changed" do
+    check = create(:check)
+    notification = create(:notification, :succeed, check: check)
+
+    check.comment = "Will not reset because of this attribute"
+    check.save!
+
+    notification.reload
+
+    assert notification.succeed?
+    assert_not_nil notification.sent_at
+
+    check.domain_expires_at = 1.year.from_now
+    check.save!
+
+    notification.reload
+
+    assert notification.pending?
+    assert_nil notification.sent_at
+  end
 end
diff --git a/test/services/notifier/channels/base_test.rb b/test/services/notifier/channels/base_test.rb
new file mode 100644
index 0000000..16856e9
--- /dev/null
+++ b/test/services/notifier/channels/base_test.rb
@@ -0,0 +1,49 @@
+require "test_helper"
+
+module Notifier
+  module Channels
+    class BaseTest < ActiveSupport::TestCase
+      setup do
+        class FakeChannel < Base
+          def supports?(reason, _notification)
+            reason != :unsupported
+          end
+
+          def domain_notify_expires_soon(*); end
+        end
+
+        @channel = FakeChannel.new
+      end
+
+      test "#notify change the status of the notification" do
+        notification = create(:notification)
+
+        @channel.notify(:expires_soon, notification)
+
+        notification.reload
+
+        assert notification.ongoing?
+        assert_just_now notification.sent_at
+      end
+
+      test "#notify raises an exception for a invalid reason" do
+        notification = build(:notification)
+
+        assert_raises ArgumentError do
+          @channel.notify(:unknown, notification)
+        end
+      end
+
+      test "#notify does nothing when channel does not support a reason" do
+        notification = create(:notification)
+
+        @channel.notify(:unsupported, notification)
+
+        notification.reload
+
+        assert notification.pending?
+        assert_nil notification.sent_at
+      end
+    end
+  end
+end
diff --git a/test/services/notifier/processor_test.rb b/test/services/notifier/processor_test.rb
new file mode 100644
index 0000000..1b50ce1
--- /dev/null
+++ b/test/services/notifier/processor_test.rb
@@ -0,0 +1,31 @@
+require "test_helper"
+
+module Notifier
+  class ProcessorTest < ActiveSupport::TestCase
+
+    private
+
+    # rubocop:disable Metrics/MethodLength
+    def test_interval_respected(process_method, count_expected)
+      configuration = Minitest::Mock.new
+      count_expected.times do
+        configuration.expect(:interval, 0.000001)
+      end
+      processor = Processor.new(configuration)
+
+      mock = Minitest::Mock.new
+      assert_stub = lambda { |actual_time|
+        assert_equal 0.000001, actual_time
+        mock
+      }
+
+      processor.stub :sleep, assert_stub do
+        processor.public_send(process_method)
+      end
+
+      configuration.verify
+      mock.verify
+    end
+    # rubocop:enable Metrics/MethodLength
+  end
+end
diff --git a/test/services/notifier/resolver_test.rb b/test/services/notifier/resolver_test.rb
new file mode 100644
index 0000000..d8a4ca9
--- /dev/null
+++ b/test/services/notifier/resolver_test.rb
@@ -0,0 +1,113 @@
+require "test_helper"
+
+module Notifier
+  class ResolverTest < ActiveSupport::TestCase
+    setup do
+      @resolver = Notifier::Resolver.new
+    end
+
+    test "#resolve_expires_soon ignores user having notification disabled" do
+      n1 = create(:notification, check: build(:check, :expires_next_week))
+      n1.check.user.update_attribute(:notifications_enabled, false)
+      n2 = create(:notification, check: build(:check, :expires_next_week))
+
+      notifications = @resolver.resolve_expires_soon
+
+      assert_not_includes notifications, n1
+      assert_includes notifications, n2
+    end
+
+    test "#resolve_expires_soon ignores inactive checks" do
+      n1 = create(:notification, check: build(:check, :expires_next_week, :inactive))
+      n2 = create(:notification, check: build(:check, :expires_next_week))
+
+      notifications = @resolver.resolve_expires_soon
+
+      assert_not_includes notifications, n1
+      assert_includes notifications, n2
+    end
+
+    test "#resolve_expires_soon gets only checks inside delay" do
+      n1 = create(:notification, check: build(:check, :expires_next_week), delay: 6)
+      n2 = create(:notification, check: build(:check, :expires_next_week), delay: 7)
+
+      notifications = @resolver.resolve_expires_soon
+
+      assert_not_includes notifications, n1
+      assert_includes notifications, n2
+    end
+
+    test "#resolve_expires_soon can gets several notifications for a same check" do
+      check = create(:check, :expires_next_week)
+      n1 = create(:notification, check: check, delay: 3)
+      n2 = create(:notification, check: check, delay: 10)
+      n3 = create(:notification, check: check, delay: 30)
+
+      notifications = @resolver.resolve_expires_soon
+
+      assert_not_includes notifications, n1
+      assert_includes notifications, n2
+      assert_includes notifications, n3
+    end
+
+    test "#resolve_expires_soon takes care of the status" do
+      check = create(:check, :expires_next_week)
+      n1 = create(:notification, check: check)
+      n2 = create(:notification, :failed, check: check)
+      n3 = create(:notification, :ongoing, check: check)
+      n4 = create(:notification, :succeed, check: check)
+
+      notifications = @resolver.resolve_expires_soon
+
+      assert_includes notifications, n1
+      assert_includes notifications, n2
+      assert_not_includes notifications, n3
+      assert_not_includes notifications, n4
+    end
+
+    test "#resolve_expires_soon ignores checks expired and without date" do
+      n1 = create(:notification, check: build(:check, :expires_next_week))
+      n2 = create(:notification, check: build(:check, domain_expires_at: 1.week.ago))
+      n3 = create(:notification, check: build(:check, :nil_dates))
+
+      notifications = @resolver.resolve_expires_soon
+
+      assert_includes notifications, n1
+      assert_not_includes notifications, n2
+      assert_not_includes notifications, n3
+    end
+
+    test "#resolve_check_failed ignores inactive checks" do
+      n1 = create(:notification, check: build(:check, :last_runs_failed, :inactive))
+      n2 = create(:notification, check: build(:check, :last_runs_failed))
+
+      notifications = @resolver.resolve_check_failed
+
+      assert_not_includes notifications, n1
+      assert_includes notifications, n2
+    end
+
+    test "#resolve_check_failed ignores user having notification disabled" do
+      n1 = create(:notification, check: build(:check, :last_runs_failed))
+      n1.check.user.update_attribute(:notifications_enabled, false)
+      n2 = create(:notification, check: build(:check, :last_runs_failed))
+
+      notifications = @resolver.resolve_check_failed
+
+      assert_not_includes notifications, n1
+      assert_includes notifications, n2
+    end
+
+    test "#resolve_check_failed gets only checks having last run in error" do
+      n1 = create(:notification, check: build(:check, :nil_dates))
+      n2 = create(:notification, check: build(:check, :last_run_succeed))
+      n3 = create(:notification, check: build(:check, :last_runs_failed))
+
+      notifications = @resolver.resolve_check_failed
+
+      assert_not_includes notifications, n1
+      assert_not_includes notifications, n2
+      assert_includes notifications, n3
+    end
+  end
+end
diff --git a/test/services/notifier_test.rb b/test/services/notifier_test.rb
new file mode 100644
index 0000000..a1d3e92
--- /dev/null
+++ b/test/services/notifier_test.rb
@@ -0,0 +1,25 @@
+require "test_helper"
+require "notifier"
+
+class NotifierTest < ActiveSupport::TestCase
+  test "#process_all process expirable & failures notifications" do
+    mock = Minitest::Mock.new
+    mock.expect(:process_expires_soon, nil)
+    mock.expect(:process_recurrent_failures, nil)
+
+    myconfig = "test config"
+
+    assert_stub = lambda { |actual_config|
+      assert_equal myconfig, actual_config,
+       "Processor was not initialized with the expected configuration"
+
+      mock
+    }
+
+    Notifier::Processor.stub :new, assert_stub do
+      Notifier.process_all(myconfig)
+    end
+
+    mock.verify
+  end
+end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 3b99eb7..fda0a43 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -4,8 +4,9 @@ require "pry"
 if !ENV["NO_COVERAGE"] && (ARGV.empty? || ARGV.include?("test/test_helper.rb"))
   require "simplecov"
   SimpleCov.start "rails" do
+    add_group "Notifier", "app/services/notifier"
+    add_group "Whois", "app/services/whois"
     add_group "Services", "app/services"
-    add_group "Notifier", "app/notifier"
     add_group "Policies", "app/policies"
   end
 end