Skip to content

Commit 47c44e2

Browse files
committed
fix: refuse an unknown operation in unschedule
schedule already raised UnknownMessage for an operation the actor does not declare, because OperationDispatcher checks it. unschedule and unschedule_all took any symbol, composed a name from it, and deleted nothing. A typo cancelled quietly and left a recurring reminder running, which is the failure this feature exists to prevent. Both now check the operation against the declared messages and raise the same error schedule raises. A handle skips the check, because schedule validated the operation that produced it.
1 parent d12b2e2 commit 47c44e2

4 files changed

Lines changed: 39 additions & 1 deletion

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## Unreleased
44

5+
- Refuse an unknown operation in `unschedule` and `unschedule_all`. `schedule`
6+
already raised `UnknownMessage` for one, so a typo cancelled nothing quietly
7+
and left a recurring reminder running.
58
- Add reminder cancellation. `unschedule` removes one reminder by operation and
69
optional key, or by the handle `schedule` now returns. `unschedule_all`
710
removes every key of one operation. Both stage an intent, so a cancel commits

‎lib/solid_objects/actor.rb‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,12 +279,13 @@ def schedule(at:, every: nil, missed: :latest, key: nil)
279279
def unschedule(operation_or_handle, key: nil)
280280
return unschedule_name(handle_name(operation_or_handle, key:)) if operation_or_handle.is_a?(Hash)
281281

282+
validated_reminder_operation(operation_or_handle)
282283
unschedule_name(reminder_name(operation: operation_or_handle, key: validated_reminder_key(key)))
283284
end
284285

285286
# @rbs (Symbol | String) -> nil
286287
def unschedule_all(operation)
287-
reminder_intents << UnscheduleAllIntent.new(operation: operation.to_s)
288+
reminder_intents << UnscheduleAllIntent.new(operation: validated_reminder_operation(operation))
288289
nil
289290
end
290291

@@ -303,6 +304,14 @@ def reminders(operation)
303304

304305
attr_reader :instance_id
305306

307+
# @rbs (Symbol | String) -> String
308+
def validated_reminder_operation(operation)
309+
name = operation.to_s
310+
return name if self.class.definition.messages.key?(name.to_sym)
311+
312+
raise UnknownMessage, "unknown message #{name.inspect} for #{self.class.actor_type}"
313+
end
314+
306315
# @rbs (String) -> nil
307316
def unschedule_name(name)
308317
reminder_intents << UnscheduleIntent.new(name:)

‎sig/generated/lib/solid_objects/actor.rbs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,9 @@ module SolidObjects
266266

267267
attr_reader instance_id: untyped
268268

269+
# @rbs (Symbol | String) -> String
270+
def validated_reminder_operation: (Symbol | String) -> String
271+
269272
# @rbs (String) -> nil
270273
def unschedule_name: (String) -> nil
271274

‎test/integration/reminder_cancellation_test.rb‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@ def convert_then_raise
3838
raise "turn failed"
3939
end
4040

41+
def cancel_unknown
42+
unschedule(:no_such_operation)
43+
end
44+
45+
def cancel_all_unknown
46+
unschedule_all(:no_such_operation)
47+
end
48+
4149
def convert_with_bad_handle
4250
unschedule({ "not_a_reminder" => "x" })
4351
end
@@ -275,6 +283,21 @@ def state_of(actor_type)
275283
assert_empty reminders_for("cancel-trial")
276284
end
277285

286+
test "an unknown operation is refused rather than cancelling nothing" do
287+
SolidObjects.configuration.max_attempts = 1
288+
reference = TrialActor.ref("alice")
289+
reference.async.start_trial
290+
drain
291+
reference.async.cancel_unknown
292+
reference.async.cancel_all_unknown
293+
drain
294+
295+
assert_equal 2, SolidObjects::DeadLetter.where(actor_type: "cancel-trial").count
296+
assert_equal [ "SolidObjects::UnknownMessage" ],
297+
SolidObjects::DeadLetter.where(actor_type: "cancel-trial").distinct.pluck(:exception_class)
298+
assert_equal 1, reminders_for("cancel-trial").count
299+
end
300+
278301
test "a malformed handle is rejected" do
279302
SolidObjects.configuration.max_attempts = 1
280303
TrialActor.ref("alice").async.convert_with_bad_handle

0 commit comments

Comments
 (0)