5 files changed, 22 insertions, 51 deletions

diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index dcba136d163..8e9a74a68d0 100644
--- a/app/models/hooks/project_hook.rb
+++ b/app/models/hooks/project_hook.rb
@@ -2,6 +2,7 @@
 
 class ProjectHook < WebHook
   include TriggerableHooks
+  include WebHooks::AutoDisabling
   include Presentable
   include Limitable
   extend ::Gitlab::Utils::Override
@@ -45,14 +46,18 @@ class ProjectHook < WebHook
 
   override :update_last_failure
   def update_last_failure
-    return if executable?
+    if executable?
+      project.cache_web_hook_failure if project.get_web_hook_failure # may need update
+    else
+      project.cache_web_hook_failure(true) # definitely failing, no need to check
 
-    key = "web_hooks:last_failure:project-#{project_id}"
-    time = Time.current.utc.iso8601
+      Gitlab::Redis::SharedState.with do |redis|
+        last_failure_key = project.last_failure_redis_key
+        time = Time.current.utc.iso8601
+        prev = redis.get(last_failure_key)
 
-    Gitlab::Redis::SharedState.with do |redis|
-      prev = redis.get(key)
-      redis.set(key, time) if !prev || prev < time
+        redis.set(last_failure_key, time) if !prev || prev < time
+      end
     end
   end
 end
diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb
index 94ced96bbde..6af70c249a0 100644
--- a/app/models/hooks/service_hook.rb
+++ b/app/models/hooks/service_hook.rb
@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
 class ServiceHook < WebHook
+  include WebHooks::Unstoppable
   include Presentable
+
   extend ::Gitlab::Utils::Override
 
   belongs_to :integration
@@ -13,9 +15,4 @@ class ServiceHook < WebHook
 
   override :parent
   delegate :parent, to: :integration
-
-  override :executable?
-  def executable?
-    true
-  end
 end
diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb
index 3c7f0ef9ffc..eaffe83cab3 100644
--- a/app/models/hooks/system_hook.rb
+++ b/app/models/hooks/system_hook.rb
@@ -2,6 +2,7 @@
 
 class SystemHook < WebHook
   include TriggerableHooks
+  include WebHooks::Unstoppable
 
   triggerable_hooks [
     :repository_update_hooks,
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 49418cda3ac..819152a38c8 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -56,31 +56,6 @@ class WebHook < ApplicationRecord
     all_branches: 2
   }, _prefix: true
 
-  scope :executable, -> do
-    where('recent_failures <= ? AND (disabled_until IS NULL OR disabled_until < ?)', FAILURE_THRESHOLD, Time.current)
-  end
-
-  # Inverse of executable
-  scope :disabled, -> do
-    where('recent_failures > ? OR disabled_until >= ?', FAILURE_THRESHOLD, Time.current)
-  end
-
-  def executable?
-    !temporarily_disabled? && !permanently_disabled?
-  end
-
-  def temporarily_disabled?
-    return false if recent_failures <= FAILURE_THRESHOLD
-
-    disabled_until.present? && disabled_until >= Time.current
-  end
-
-  def permanently_disabled?
-    return false if disabled_until.present?
-
-    recent_failures > FAILURE_THRESHOLD
-  end
-
   # rubocop: disable CodeReuse/ServiceClass
   def execute(data, hook_name, force: false)
     # hook.executable? is checked in WebHookService#execute
@@ -112,8 +87,6 @@ class WebHook < ApplicationRecord
   end
 
   def disable!
-    return if permanently_disabled?
-
     update_attribute(:recent_failures, EXCEEDED_FAILURE_THRESHOLD)
   end
 
@@ -127,8 +100,6 @@ class WebHook < ApplicationRecord
   # Don't actually back-off until FAILURE_THRESHOLD failures have been seen
   # we mark the grace-period using the recent_failures counter
   def backoff!
-    return if permanently_disabled? || (backoff_count >= MAX_FAILURES && temporarily_disabled?)
-
     attrs = { recent_failures: next_failure_count }
 
     if recent_failures >= FAILURE_THRESHOLD
@@ -137,7 +108,7 @@ class WebHook < ApplicationRecord
     end
 
     assign_attributes(attrs)
-    save(validate: false)
+    save(validate: false) if changed?
   end
 
   def failed!
@@ -167,16 +138,6 @@ class WebHook < ApplicationRecord
     { related_class: type }
   end
 
-  def alert_status
-    if temporarily_disabled?
-      :temporarily_disabled
-    elsif permanently_disabled?
-      :disabled
-    else
-      :executable
-    end
-  end
-
   # Exclude binary columns by default - they have no sensible JSON encoding
   def serializable_hash(options = nil)
     options = options.try(:dup) || {}
diff --git a/app/models/hooks/web_hook_log.rb b/app/models/hooks/web_hook_log.rb
index 9de6f2a1b57..e08294058e4 100644
--- a/app/models/hooks/web_hook_log.rb
+++ b/app/models/hooks/web_hook_log.rb
@@ -9,6 +9,8 @@ class WebHookLog < ApplicationRecord
 
   OVERSIZE_REQUEST_DATA = { 'oversize' => true }.freeze
 
+  attr_accessor :interpolated_url
+
   self.primary_key = :id
 
   partitioned_by :created_at, strategy: :monthly, retain_for: 3.months
@@ -23,6 +25,7 @@ class WebHookLog < ApplicationRecord
 
   before_save :obfuscate_basic_auth
   before_save :redact_user_emails
+  before_save :set_url_hash, if: -> { interpolated_url.present? }
 
   def self.recent
     where(created_at: 2.days.ago.beginning_of_day..Time.zone.now)
@@ -66,4 +69,8 @@ class WebHookLog < ApplicationRecord
       value.to_s =~ URI::MailTo::EMAIL_REGEXP ? _('[REDACTED]') : value
     end
   end
+
+  def set_url_hash
+    self.url_hash = Gitlab::CryptoHelper.sha256(interpolated_url)
+  end
 end