Update checks#consecutive_failures after a success or failure

app/models/check.rb

@@ -52,6 +52,7 @@ class Check < ApplicationRecord
   validates :comment, length: { maximum: 255 }
   validates :vendor, length: { maximum: 255 }
 
+  before_save :reset_consecutive_failures
   after_update :reset_notifications
   after_save :enqueue_sync
 
@@ -88,6 +89,11 @@ class Check < ApplicationRecord
     (Date.today - last_success_at.to_date).to_i
   end
 
+  def increment_consecutive_failures!
+    self.consecutive_failures += 1
+    save!
+  end
+
   private
 
   def domain_created_at_past
@@ -110,4 +116,11 @@ class Check < ApplicationRecord
 
     notifications.each(&:reset!)
   end
+
+  def reset_consecutive_failures
+    return unless last_success_at_changed?
+    return if consecutive_failures_changed?
+
+    self.consecutive_failures = 0
+  end
 end

app/services/check_logger.rb

@@ -1,8 +1,11 @@
 class CheckLogger
+  attr_reader :check
   attr_reader :check_log
 
   def initialize(check)
+    @check = check
     @check_log = CheckLog.create!(check: check, status: :pending)
+    @error_logged = false
   end
 
   def log(event, message)
@@ -12,7 +15,13 @@ class CheckLogger
     when :parsed_response
       log_parsed_response(message)
     when :parser_error, :service_error, :standard_error
+      # avoid multiple logging & wrong incrementation of consecutive failures
+      # (because a Service exception could be re-raised from a Job)
+      return if error_logged?
+
       log_error(message)
+
+      @error_logged = true
     end
   end
 
@@ -41,7 +50,12 @@ class CheckLogger
   end
 
   def log_error(exception)
+    check.increment_consecutive_failures!
     check_log.error = [exception.message, exception.backtrace].join("\n")
     check_log.failed!
   end
+
+  def error_logged?
+    @error_logged
+  end
 end

test/jobs/ssl_sync_job_test.rb

@@ -22,7 +22,6 @@ class SSLSyncJobTest < ActiveJob::TestCase
   test "ignore invalid response" do
     domain = "domain.fr"
     check = create(:check, :nil_dates, domain: domain)
-    original_updated_at = check.updated_at
 
     mock_system_command("check_http", expected_command_arg(domain), stdout: "not a response") do
       SSLSyncJob.perform_now(check.id)
@@ -32,8 +31,8 @@ class SSLSyncJobTest < ActiveJob::TestCase
 
     assert_just_now check.last_run_at
     assert_nil check.last_success_at
-    assert_equal original_updated_at, check.updated_at
     assert check.active?
+    assert_equal 1, check.consecutive_failures
   end
 
   test "should ignore not found (removed) checks" do
@@ -51,9 +50,25 @@ class SSLSyncJobTest < ActiveJob::TestCase
       end
     end
 
+    check.reload
+
     assert_equal 1, check.logs.count
     assert_match(/undefined method \W+valid\?/, check.logs.last.error)
     assert check.logs.last.failed?
+    assert_equal 1, check.consecutive_failures
+  end
+
+  test "should reset consecutive failures with a valid response" do
+    domain = "ssl0.domain.org"
+    check = create(:check, :nil_dates, domain: domain, consecutive_failures: 1)
+
+    mock_system_command("check_http", expected_command_arg(domain), stdout: ssl_response(domain)) do
+      SSLSyncJob.perform_now(check.id)
+    end
+
+    check.reload
+
+    assert_equal 0, check.consecutive_failures
   end
 
   private

test/jobs/whois_sync_job_test.rb

@@ -33,6 +33,7 @@ class WhoisSyncJobTest < ActiveJob::TestCase
     assert_nil check.last_success_at
     assert_equal original_updated_at, check.updated_at
     assert check.active?
+    assert_equal 1, check.consecutive_failures
   end
 
   test "should ignore not found (removed) checks" do
@@ -50,9 +51,12 @@ class WhoisSyncJobTest < ActiveJob::TestCase
       end
     end
 
+    check.reload
+
     assert_equal 1, check.logs.count
     assert_match(/undefined method \W+valid\?/, check.logs.last.error)
     assert check.logs.last.failed?
+    assert_equal 1, check.consecutive_failures
   end
 
   test "disable check when whois responds domain not found" do
@@ -68,6 +72,7 @@ class WhoisSyncJobTest < ActiveJob::TestCase
     refute check.active?
     assert_just_now check.last_run_at
     assert_nil check.last_success_at
+    assert_equal 1, check.consecutive_failures
   end
 
   test "default logger is CheckLogger" do
@@ -80,6 +85,19 @@ class WhoisSyncJobTest < ActiveJob::TestCase
     assert_equal 1, check.logs.count
   end
 
+  test "should reset consecutive failures with a valid response" do
+    domain = "domain.fr"
+    check = create(:check, :nil_dates, domain: domain, consecutive_failures: 1)
+
+    mock_system_command("whois", domain, stdout: whois_response(domain)) do
+      WhoisSyncJob.perform_now(check.id)
+    end
+
+    check.reload
+
+    assert_equal 0, check.consecutive_failures
+  end
+
   private
 
   def whois_response(domain)

test/models/check_test.rb

@@ -87,7 +87,7 @@ class CheckTest < ActiveSupport::TestCase
 
   test "in_error? when last check occured a few days ago without error" do
     check = build(:check, created_at: 3.weeks.ago,
-      last_success_at: 10.days.ago, last_run_at: 10.days.ago)
+                          last_success_at: 10.days.ago, last_run_at: 10.days.ago)
     refute check.in_error?
   end