Simplify delinquency tracking
This commit is contained in:
@@ -1,35 +1,38 @@
|
|||||||
class Webhook::DelinquencyTracker < ApplicationRecord
|
class Webhook::DelinquencyTracker < ApplicationRecord
|
||||||
LOW_VOLUME_TRESHOLD = 10
|
DELINQUENCY_THRESHOLD = 10
|
||||||
RESET_INTERVAL = 1.hour
|
CHECK_INTERVAL = 1.hour
|
||||||
|
|
||||||
belongs_to :webhook
|
belongs_to :webhook
|
||||||
|
|
||||||
before_validation { self.last_reset_at ||= Time.current }
|
|
||||||
|
|
||||||
def record_delivery_of(delivery)
|
def record_delivery_of(delivery)
|
||||||
if delivery.failed? && high_volume? && reset_due?
|
if delivery.succeeded?
|
||||||
webhook.deactivate if delinquent?
|
|
||||||
reset
|
reset
|
||||||
else
|
else
|
||||||
increment!(:total_count)
|
mark_first_failure_time if consecutive_failures_count.zero?
|
||||||
increment!(:failed_count) if delivery.failed?
|
increment!(:consecutive_failures_count)
|
||||||
|
|
||||||
|
webhook.deactivate if delinquent?
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
def high_volume?
|
def reset
|
||||||
total_count > LOW_VOLUME_TRESHOLD
|
update_columns consecutive_failures_count: 0, first_failure_at: nil
|
||||||
|
end
|
||||||
|
|
||||||
|
def mark_first_failure_time
|
||||||
|
update_columns first_failure_at: Time.current
|
||||||
end
|
end
|
||||||
|
|
||||||
def delinquent?
|
def delinquent?
|
||||||
failed_count == total_count
|
enough_time_passed? && (consecutive_failures_count >= DELINQUENCY_THRESHOLD)
|
||||||
end
|
end
|
||||||
|
|
||||||
def reset_due?
|
def enough_time_passed?
|
||||||
last_reset_at.before?(RESET_INTERVAL.ago)
|
if first_failure_at
|
||||||
end
|
first_failure_at.before?(CHECK_INTERVAL.ago)
|
||||||
|
else
|
||||||
def reset
|
false
|
||||||
update_columns total_count: 0, failed_count: 0, last_reset_at: Time.current
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
+10
@@ -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
|
||||||
Generated
+2
-3
@@ -456,10 +456,9 @@ ActiveRecord::Schema[8.1].define(version: 2025_09_15_170056) do
|
|||||||
end
|
end
|
||||||
|
|
||||||
create_table "webhook_delinquency_trackers", force: :cascade do |t|
|
create_table "webhook_delinquency_trackers", force: :cascade do |t|
|
||||||
|
t.integer "consecutive_failures_count"
|
||||||
t.datetime "created_at", null: false
|
t.datetime "created_at", null: false
|
||||||
t.integer "failed_count", default: 0, null: false
|
t.datetime "first_failure_at"
|
||||||
t.datetime "last_reset_at"
|
|
||||||
t.integer "total_count", default: 0, null: false
|
|
||||||
t.datetime "updated_at", null: false
|
t.datetime "updated_at", null: false
|
||||||
t.integer "webhook_id", null: false
|
t.integer "webhook_id", null: false
|
||||||
t.index ["webhook_id"], name: "index_webhook_delinquency_trackers_on_webhook_id"
|
t.index ["webhook_id"], name: "index_webhook_delinquency_trackers_on_webhook_id"
|
||||||
|
|||||||
+6
-16
@@ -1319,21 +1319,20 @@ columns:
|
|||||||
collation:
|
collation:
|
||||||
comment:
|
comment:
|
||||||
webhook_delinquency_trackers:
|
webhook_delinquency_trackers:
|
||||||
- *5
|
|
||||||
- !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
|
- !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
|
||||||
auto_increment:
|
auto_increment:
|
||||||
name: failed_count
|
name: consecutive_failures_count
|
||||||
cast_type: *3
|
cast_type: *3
|
||||||
sql_type_metadata: *4
|
sql_type_metadata: *4
|
||||||
'null': false
|
'null': true
|
||||||
default: 0
|
default:
|
||||||
default_function:
|
default_function:
|
||||||
collation:
|
collation:
|
||||||
comment:
|
comment:
|
||||||
- *6
|
- *5
|
||||||
- !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
|
- !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
|
||||||
auto_increment:
|
auto_increment:
|
||||||
name: last_reset_at
|
name: first_failure_at
|
||||||
cast_type: *1
|
cast_type: *1
|
||||||
sql_type_metadata: *2
|
sql_type_metadata: *2
|
||||||
'null': true
|
'null': true
|
||||||
@@ -1341,16 +1340,7 @@ columns:
|
|||||||
default_function:
|
default_function:
|
||||||
collation:
|
collation:
|
||||||
comment:
|
comment:
|
||||||
- !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
|
- *6
|
||||||
auto_increment:
|
|
||||||
name: total_count
|
|
||||||
cast_type: *3
|
|
||||||
sql_type_metadata: *4
|
|
||||||
'null': false
|
|
||||||
default: 0
|
|
||||||
default_function:
|
|
||||||
collation:
|
|
||||||
comment:
|
|
||||||
- *9
|
- *9
|
||||||
- &37 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
|
- &37 !ruby/object:ActiveRecord::ConnectionAdapters::SQLite3::Column
|
||||||
auto_increment:
|
auto_increment:
|
||||||
|
|||||||
+4
-6
@@ -2,12 +2,10 @@
|
|||||||
|
|
||||||
active_webhook_tracker:
|
active_webhook_tracker:
|
||||||
webhook: active
|
webhook: active
|
||||||
last_reset_at: <%= 1.hour.ago %>
|
consecutive_failures_count: 1
|
||||||
total_count: 1
|
first_failure_at: <%= 1.hour.ago %>
|
||||||
failed_count: 1
|
|
||||||
|
|
||||||
inactive_webhook_tracker:
|
inactive_webhook_tracker:
|
||||||
webhook: inactive
|
webhook: inactive
|
||||||
last_reset_at: <%= 1.hour.ago %>
|
consecutive_failures_count: 1
|
||||||
total_count: 1
|
first_failure_at: <%= 1.hour.ago %>
|
||||||
failed_count: 1
|
|
||||||
|
|||||||
@@ -7,31 +7,28 @@ class Webhook::DelinquencyTrackerTest < ActiveSupport::TestCase
|
|||||||
successful_delivery = webhook_deliveries(:successfully_completed)
|
successful_delivery = webhook_deliveries(:successfully_completed)
|
||||||
failed_delivery = webhook_deliveries(:errored)
|
failed_delivery = webhook_deliveries(:errored)
|
||||||
|
|
||||||
assert_difference -> { tracker.reload.total_count }, +1 do
|
tracker.update!(consecutive_failures_count: 5)
|
||||||
assert_no_difference -> { tracker.reload.failed_count } do
|
tracker.record_delivery_of(successful_delivery)
|
||||||
tracker.record_delivery_of(successful_delivery)
|
tracker.reload
|
||||||
end
|
|
||||||
|
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
|
end
|
||||||
|
|
||||||
assert_difference -> { tracker.reload.total_count }, +1 do
|
tracker.reload
|
||||||
assert_difference -> { tracker.reload.failed_count }, +1 do
|
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)
|
tracker.record_delivery_of(failed_delivery)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
travel_to 13.hours.from_now do
|
travel_to 2.hours.from_now do
|
||||||
tracker.update!(total_count: 11, failed_count: 5)
|
tracker.update!(consecutive_failures_count: 9)
|
||||||
|
|
||||||
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)
|
|
||||||
webhook.activate
|
webhook.activate
|
||||||
|
|
||||||
assert_changes -> { webhook.reload.active? }, from: true, to: false do
|
assert_changes -> { webhook.reload.active? }, from: true, to: false do
|
||||||
|
|||||||
@@ -56,10 +56,10 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase
|
|||||||
assert_equal "pending", delivery.state
|
assert_equal "pending", delivery.state
|
||||||
|
|
||||||
tracker = delivery.webhook.delinquency_tracker
|
tracker = delivery.webhook.delinquency_tracker
|
||||||
assert_difference -> { tracker.reload.total_count }, 1 do
|
tracker.update!(consecutive_failures_count: 0)
|
||||||
assert_no_difference -> { tracker.reload.failed_count } do
|
|
||||||
delivery.deliver
|
assert_no_difference -> { tracker.reload.consecutive_failures_count } do
|
||||||
end
|
delivery.deliver
|
||||||
end
|
end
|
||||||
|
|
||||||
assert delivery.persisted?
|
assert delivery.persisted?
|
||||||
@@ -75,15 +75,13 @@ class Webhook::DeliveryTest < ActiveSupport::TestCase
|
|||||||
stub_request(:post, delivery.webhook.url).to_timeout
|
stub_request(:post, delivery.webhook.url).to_timeout
|
||||||
|
|
||||||
tracker = delivery.webhook.delinquency_tracker
|
tracker = delivery.webhook.delinquency_tracker
|
||||||
assert_difference -> { tracker.reload.total_count }, 1 do
|
assert_difference -> { tracker.reload.consecutive_failures_count }, 1 do
|
||||||
assert_difference -> { tracker.reload.failed_count }, 1 do
|
delivery.deliver
|
||||||
delivery.deliver
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
assert_equal "completed", delivery.state
|
assert_equal "completed", delivery.state
|
||||||
assert_equal "connection_timeout", delivery.response[:error]
|
assert_equal "connection_timeout", delivery.response[:error]
|
||||||
assert delivery.failed?
|
assert_not delivery.succeeded?
|
||||||
end
|
end
|
||||||
|
|
||||||
test "deliver when the connection is refused" do
|
test "deliver when the connection is refused" do
|
||||||
|
|||||||
Reference in New Issue
Block a user