github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-22 10:49:04 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: Prevent recurring automations from getting stalled under specific conditions (#28913)

Browse Source 
Under certain conditions, a recurring automation can end up in a state with no pending automation records, causing it to not execute again until manually triggered.

We use the `RRule` gem to calculate the next execution date and time for recurring automations. The gem takes the interval, frequency, start date, and a time range, and returns all dates/times within this range that meet the recurrence rule. For example:

```ruby
RRule::Rule
  .new("FREQ=DAILY;INTERVAL=1", dtstart: Time.parse("2023-01-01 07:30:00 UTC"))
  .between(Time.zone.now, Time.zone.now + 2.days)
# => [Sat, 14 Sep 2024 07:30:00.000000000 UTC +00:00, Sun, 15 Sep 2024 07:30:00.000000000 UTC +00:00]
```

However, if the time component of the first point provided to `.between()` is slightly ahead of the start date (e.g., `dtstart`), the first date/time returned by `RRule` can fall outside the specified range by the same subsecond amount. For instance:

```ruby
RRule::Rule
  .new("FREQ=DAILY;INTERVAL=1", dtstart: Time.parse("2023-01-01 07:30:00 UTC"))
  .between(Time.parse("2023-01-01 07:30:00.999 UTC"), Time.parse("2023-01-03 07:30:00 UTC"))
  .first
# => Sun, 01 Jan 2023 07:30:00.000000000 UTC +00:00
```

Here, the start date/time given to `.between()` is 999 milliseconds after 07:30:00, but the first date returned is exactly 07:30:00 without the 999 milliseconds. This causes the next recurring date to fall into the past if the automation executes within a subsecond of the start time, leading to the automation stalling.

I'm not sure why `RRule` does this, but it seems intentional judging by the source of the `.between()` method:

b9911b7147/lib/rrule/rule.rb (L28-L32)

This commit fixes the issue by selecting the first date ahead of the current time from the list returned by `RRule`, rather than the first date directly.

Internal topic: t/138045.
This commit is contained in:
Osama Sayegh 2024-09-16 18:23:26 +03:00 committed by GitHub
parent b60f4606e5
commit 4406bbb020
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 133 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
plugins/automation/lib/discourse_automation/triggers/recurring.rb
View File

@ -65,19 +65,19 @@ module DiscourseAutomation
RRule::Rule
.new("FREQ=DAILY;INTERVAL=#{interval}", dtstart: start_date)
.between(Time.now, interval_end.days.from_now)
.first
.find { |date| date > Time.zone.now }
when "weekday"
max_weekends = (interval_end.to_f / 5).ceil
RRule::Rule
.new("FREQ=DAILY;BYDAY=MO,TU,WE,TH,FR", dtstart: start_date)
.between(Time.now.end_of_day, max_weekends.weeks.from_now)
.drop(interval - 1)
.first
.find { |date| date > Time.zone.now }
when "week"
RRule::Rule
.new("FREQ=WEEKLY;INTERVAL=#{interval};BYDAY=#{byday}", dtstart: start_date)
.between(Time.now.end_of_week, interval_end.weeks.from_now)
.first
.find { |date| date > Time.zone.now }
when "month"
count = 0
(start_date.beginning_of_month.to_date..start_date.end_of_month.to_date).each do |date|
@ -87,15 +87,15 @@ module DiscourseAutomation
RRule::Rule
.new("FREQ=MONTHLY;INTERVAL=#{interval};BYDAY=#{count}#{byday}", dtstart: start_date)
.between(Time.now, interval_end.months.from_now)
.first
.find { |date| date > Time.zone.now }
when "year"
RRule::Rule
.new("FREQ=YEARLY;INTERVAL=#{interval}", dtstart: start_date)
.between(Time.now, interval_end.years.from_now)
.first
.find { |date| date > Time.zone.now }
end
if next_trigger_date && next_trigger_date > Time.zone.now
if next_trigger_date
automation.pending_automations.create!(execute_at: next_trigger_date)
else
log_debugging_info(

127
plugins/automation/spec/triggers/recurring_spec.rb
View File

@ -21,6 +21,133 @@ describe "Recurring" do
expect(triggerable.settings[DiscourseAutomation::Triggerable::MANUAL_TRIGGER_KEY]).to eq(true)
end
describe "scheduling next pending automations" do
context "with daily frequency" do
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
automation.upsert_field!(
"start_date",
"date_time",
{ value: Time.parse("2022-11-01 07:30:00 UTC") },
target: "trigger",
)
automation.upsert_field!(
"recurrence",
"period",
{ value: { interval: 1, frequency: "day" } },
target: "trigger",
)
freeze_time(Time.parse("2023-09-13 07:30:00.141775363 UTC")) { automation.trigger! }
pending_automations = automation.reload.pending_automations
expect(pending_automations.count).to eq(1)
expect(pending_automations.first.execute_at).to eq_time(
Time.parse("2023-09-14 07:30:00 UTC"),
)
end
end
context "with weekday frequency" do
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
automation.upsert_field!(
"start_date",
"date_time",
{ value: Time.parse("2024-10-01 23:59:59 UTC") },
target: "trigger",
)
automation.upsert_field!(
"recurrence",
"period",
{ value: { interval: 1, frequency: "weekday" } },
target: "trigger",
)
freeze_time(Time.parse("2024-10-01 23:59:59.141775363 UTC")) { automation.trigger! }
pending_automations = automation.reload.pending_automations
expect(pending_automations.count).to eq(1)
expect(pending_automations.first.execute_at).to eq_time(
Time.parse("2024-10-02 23:59:59 UTC"),
)
end
end
context "with weekly frequency" do
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
automation.upsert_field!(
"start_date",
"date_time",
{ value: Time.parse("2024-09-15 23:59:59 UTC") },
target: "trigger",
)
automation.upsert_field!(
"recurrence",
"period",
{ value: { interval: 1, frequency: "week" } },
target: "trigger",
)
freeze_time(Time.parse("2024-09-15 23:59:59.141775363 UTC")) { automation.trigger! }
pending_automations = automation.reload.pending_automations
expect(pending_automations.count).to eq(1)
expect(pending_automations.first.execute_at).to eq_time(
Time.parse("2024-09-22 23:59:59 UTC"),
)
end
end
context "with monthly frequency" do
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
automation.upsert_field!(
"start_date",
"date_time",
{ value: Time.parse("2023-10-01 07:30:00 UTC") },
target: "trigger",
)
automation.upsert_field!(
"recurrence",
"period",
{ value: { interval: 1, frequency: "month" } },
target: "trigger",
)
freeze_time(Time.parse("2023-10-01 07:30:00.141775363 UTC")) { automation.trigger! }
pending_automations = automation.reload.pending_automations
expect(pending_automations.count).to eq(1)
expect(pending_automations.first.execute_at).to eq_time(
Time.parse("2023-11-05 07:30:00 UTC"),
)
end
end
context "with yearly frequency" do
it "doesn't fail to schedule if the current time is within subsecond of the time component of start_date" do
automation.upsert_field!(
"start_date",
"date_time",
{ value: Time.parse("2023-01-01 07:30:00 UTC") },
target: "trigger",
)
automation.upsert_field!(
"recurrence",
"period",
{ value: { interval: 1, frequency: "year" } },
target: "trigger",
)
freeze_time(Time.parse("2023-01-01 07:30:00.141775363 UTC")) { automation.trigger! }
pending_automations = automation.reload.pending_automations
expect(pending_automations.count).to eq(1)
expect(pending_automations.first.execute_at).to eq_time(
Time.parse("2024-01-01 07:30:00 UTC"),
)
end
end
end
describe "updating trigger" do
context "when date is in future" do
before { freeze_time Time.parse("2021-06-04 10:00 UTC") }