From dd3063b01df2a7d2a25f9a1a5be82d5532683d33 Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Mon, 15 Sep 2025 17:03:32 +0200 Subject: [PATCH] Simplify delinquency tracking --- app/models/webhook/delinquency_tracker.rb | 37 ++++++++++--------- ...s_count_to_webhook_delinquency_trackers.rb | 10 +++++ db/schema.rb | 5 +-- db/schema_cache.yml | 22 +++-------- .../fixtures/webhook/delinquency_trackers.yml | 10 ++--- .../webhook/delinquency_tracker_test.rb | 35 ++++++++---------- test/models/webhook/delivery_test.rb | 16 ++++---- 7 files changed, 65 insertions(+), 70 deletions(-) create mode 100644 db/migrate/20250915143513_add_consecutive_failures_count_to_webhook_delinquency_trackers.rb diff --git a/app/models/webhook/delinquency_tracker.rb b/app/models/webhook/delinquency_tracker.rb index f3eafc06e..4fb2d174a 100644 --- a/app/models/webhook/delinquency_tracker.rb +++ b/app/models/webhook/delinquency_tracker.rb @@ -1,35 +1,38 @@ class Webhook::DelinquencyTracker < ApplicationRecord - LOW_VOLUME_TRESHOLD = 10 - RESET_INTERVAL = 1.hour + DELINQUENCY_THRESHOLD = 10 + CHECK_INTERVAL = 1.hour belongs_to :webhook - before_validation { self.last_reset_at ||= Time.current } - def record_delivery_of(delivery) - if delivery.failed? && high_volume? && reset_due? - webhook.deactivate if delinquent? + if delivery.succeeded? reset else - increment!(:total_count) - increment!(:failed_count) if delivery.failed? + mark_first_failure_time if consecutive_failures_count.zero? + increment!(:consecutive_failures_count) + + webhook.deactivate if delinquent? end end private - def high_volume? - total_count > LOW_VOLUME_TRESHOLD + def reset + update_columns consecutive_failures_count: 0, first_failure_at: nil + end + + def mark_first_failure_time + update_columns first_failure_at: Time.current end def delinquent? - failed_count == total_count + enough_time_passed? && (consecutive_failures_count >= DELINQUENCY_THRESHOLD) end - def reset_due? - last_reset_at.before?(RESET_INTERVAL.ago) - end - - def reset - update_columns total_count: 0, failed_count: 0, last_reset_at: Time.current + def enough_time_passed? + if first_failure_at + first_failure_at.before?(CHECK_INTERVAL.ago) + else + false + end end end diff --git a/db/migrate/20250915143513_add_consecutive_failures_count_to_webhook_delinquency_trackers.rb b/db/migrate/20250915143513_add_consecutive_failures_count_to_webhook_delinquency_trackers.rb new file mode 100644 index 000000000..13d1acf2d --- /dev/null +++ b/db/migrate/20250915143513_add_consecutive_failures_count_to_webhook_delinquency_trackers.rb @@ -0,0 +1,10 @@ +class AddConsecutiveFailuresCountToWebhookDelinquencyTrackers < ActiveRecord::Migration[8.1] + def change + add_column :webhook_delinquency_trackers, :consecutive_failures_count, :integer + add_column :webhook_delinquency_trackers, :first_failure_at, :datetime + + remove_column :webhook_delinquency_trackers, :total_count, :integer + remove_column :webhook_delinquency_trackers, :failed_count, :integer + remove_column :webhook_delinquency_trackers, :last_reset_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 319f33939..ecf0c9097 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -456,10 +456,9 @@ ActiveRecord::Schema[8.1].define(version: 2025_09_15_170056) do end create_table "webhook_delinquency_trackers", force: :cascade do |t| + t.integer "consecutive_failures_count" t.datetime "created_at", null: false - t.integer "failed_count", default: 0, null: false - t.datetime "last_reset_at" - t.integer "total_count", default: 0, null: false + t.datetime "first_failure_at" t.datetime "updated_at", null: false t.integer "webhook_id", null: false t.index ["webhook_id"], name: "index_webhook_delinquency_trackers_on_webhook_id" diff --git a/db/schema_cache.yml b/db/schema_cache.yml index 3b8395663..0f88cc151 100644 --- a/db/schema_cache.yml +++ b/db/schema_cache.yml @@ -1319,21 +1319,20 @@ columns: collation: comment: webhook_delinquency_trackers: - - *5 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: failed_count + name: consecutive_failures_count cast_type: *3 sql_type_metadata: *4 - 'null': false - default: 0 + 'null': true + default: default_function: collation: comment: - - *6 + - *5 - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: - name: last_reset_at + name: first_failure_at cast_type: *1 sql_type_metadata: *2 'null': true @@ -1341,16 +1340,7 @@ columns: default_function: collation: comment: - - !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column - auto_increment: - name: total_count - cast_type: *3 - sql_type_metadata: *4 - 'null': false - default: 0 - default_function: - collation: - comment: + - *6 - *9 - &37 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column auto_increment: diff --git a/test/fixtures/webhook/delinquency_trackers.yml b/test/fixtures/webhook/delinquency_trackers.yml index b4d4230b7..4a5086e69 100644 --- a/test/fixtures/webhook/delinquency_trackers.yml +++ b/test/fixtures/webhook/delinquency_trackers.yml @@ -2,12 +2,10 @@ active_webhook_tracker: webhook: active - last_reset_at: <%= 1.hour.ago %> - total_count: 1 - failed_count: 1 + consecutive_failures_count: 1 + first_failure_at: <%= 1.hour.ago %> inactive_webhook_tracker: webhook: inactive - last_reset_at: <%= 1.hour.ago %> - total_count: 1 - failed_count: 1 + consecutive_failures_count: 1 + first_failure_at: <%= 1.hour.ago %> diff --git a/test/models/webhook/delinquency_tracker_test.rb b/test/models/webhook/delinquency_tracker_test.rb index 80c55c2ba..e49d9332e 100644 --- a/test/models/webhook/delinquency_tracker_test.rb +++ b/test/models/webhook/delinquency_tracker_test.rb @@ -7,31 +7,28 @@ class Webhook::DelinquencyTrackerTest < ActiveSupport::TestCase successful_delivery = webhook_deliveries(:successfully_completed) failed_delivery = webhook_deliveries(:errored) - assert_difference -> { tracker.reload.total_count }, +1 do - assert_no_difference -> { tracker.reload.failed_count } do - tracker.record_delivery_of(successful_delivery) - end + tracker.update!(consecutive_failures_count: 5) + tracker.record_delivery_of(successful_delivery) + tracker.reload + + assert_equal 0, tracker.consecutive_failures_count + assert_nil tracker.first_failure_at + + assert_difference -> { tracker.reload.consecutive_failures_count }, +1 do + tracker.record_delivery_of(failed_delivery) end - assert_difference -> { tracker.reload.total_count }, +1 do - assert_difference -> { tracker.reload.failed_count }, +1 do + tracker.reload + assert_not_nil tracker.first_failure_at + + assert_difference -> { tracker.reload.consecutive_failures_count }, +1 do + assert_no_difference -> { tracker.reload.first_failure_at } do tracker.record_delivery_of(failed_delivery) end end - travel_to 13.hours.from_now do - tracker.update!(total_count: 11, failed_count: 5) - - tracker.record_delivery_of(failed_delivery) - tracker.reload - - assert_equal 0, tracker.total_count - assert_equal 0, tracker.failed_count - assert tracker.last_reset_at > 1.minute.ago - end - - travel_to 26.hours.from_now do - tracker.update!(total_count: 50, failed_count: 50) + travel_to 2.hours.from_now do + tracker.update!(consecutive_failures_count: 9) webhook.activate assert_changes -> { webhook.reload.active? }, from: true, to: false do diff --git a/test/models/webhook/delivery_test.rb b/test/models/webhook/delivery_test.rb index d79713717..53828c206 100644 --- a/test/models/webhook/delivery_test.rb +++ b/test/models/webhook/delivery_test.rb @@ -56,10 +56,10 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase assert_equal "pending", delivery.state tracker = delivery.webhook.delinquency_tracker - assert_difference -> { tracker.reload.total_count }, 1 do - assert_no_difference -> { tracker.reload.failed_count } do - delivery.deliver - end + tracker.update!(consecutive_failures_count: 0) + + assert_no_difference -> { tracker.reload.consecutive_failures_count } do + delivery.deliver end assert delivery.persisted? @@ -75,15 +75,13 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase stub_request(:post, delivery.webhook.url).to_timeout tracker = delivery.webhook.delinquency_tracker - assert_difference -> { tracker.reload.total_count }, 1 do - assert_difference -> { tracker.reload.failed_count }, 1 do - delivery.deliver - end + assert_difference -> { tracker.reload.consecutive_failures_count }, 1 do + delivery.deliver end assert_equal "completed", delivery.state assert_equal "connection_timeout", delivery.response[:error] - assert delivery.failed? + assert_not delivery.succeeded? end test "deliver when the connection is refused" do