Skip to content

Commit dfc5cf5

Browse files
committed
Keep a keyed reminder name from colliding or overflowing
A keyed name is the operation, a colon, and the key, so an operation holding a colon of its own made two different schedules produce one name. An unkeyed "deliver:item" and a "deliver" keyed "item" shared a row, and the second silently took the first one's alarm along with its operation and arguments. Refusing a colon in the operation keeps unkeyed names free of colons, which leaves the two kinds disjoint, and a key may still hold colons of its own because the operation before the first one cannot. The length was checked on the key alone, so a long operation with a valid key composed a name past the 191 characters the column holds and raised on the insert with the turn already doing work. It is checked on the composed name now, which catches both ends.
1 parent c0d37d8 commit dfc5cf5

5 files changed

Lines changed: 113 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
on rather than for its operation, so one actor can hold an alarm per queued
77
item. Scheduling the same key again moves that item's alarm and leaves the
88
others alone. Without a key the name is still the operation, so existing
9-
reminders keep their names and their coalescing behaviour.
9+
reminders keep their names and their coalescing behaviour. A reminder
10+
operation may no longer hold the colon that separates a key, which keeps
11+
keyed and unkeyed names disjoint, and the length is checked on the composed
12+
name rather than the key alone.
1013

1114
## 0.13.1 - 2026-08-16
1215

README.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -922,8 +922,14 @@ item's alarm and leaves the others alone, which is what makes a keyed reminder
922922
as safe to re-arm as an unkeyed one. The operation still decides which handler
923923
runs; the key only decides which alarm is which.
924924

925-
A key must be non-empty and at most 128 characters, because the name it becomes
926-
shares a 191-character column with the operation.
925+
A key must be non-empty, and the name it becomes must fit the 191-character
926+
column, which is checked on the composed name rather than the key alone so a
927+
long operation and a short key are caught too.
928+
929+
The key is separated from the operation by a colon, so an operation may not hold
930+
one. Otherwise an unkeyed `deliver:item` and a `deliver` keyed `item` would be
931+
one name, and the second would silently take the first one's alarm. A key may
932+
hold colons of its own, because the operation before the first one cannot.
927933

928934
### One alarm for a whole queue
929935

lib/solid_objects/actor.rb

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ module SolidObjects
44
class Actor
55
EffectIntent = Data.define(:name, :arguments, :success_operation, :failure_operation)
66
CommitActionIntent = Data.define(:name, :arguments)
7-
# The reminders table holds a name in 191 characters, and a name is an
8-
# operation, a colon, and a key.
9-
REMINDER_KEY_LIMIT = 128
7+
# The reminders table holds a name in 191 characters.
8+
REMINDER_NAME_LIMIT = 191
9+
REMINDER_KEY_SEPARATOR = ":"
1010

1111
ReminderIntent = Data.define(:name, :operation, :at, :arguments, :interval_seconds, :missed_policy)
1212
OutboundMessageIntent = Data.define(:actor_type, :actor_id, :operation, :arguments, :available_at, :idempotency_key)
@@ -223,7 +223,7 @@ def schedule(at:, every: nil, missed: :latest, key: nil)
223223
handlers: self.class.definition.messages
224224
) do |operation, arguments|
225225
ReminderIntent.new(
226-
name: reminder_key ? "#{operation}:#{reminder_key}" : operation.to_s,
226+
name: reminder_name(operation:, key: reminder_key),
227227
operation: operation.to_s,
228228
at:,
229229
arguments: Serialization.dump(arguments),
@@ -236,24 +236,45 @@ def schedule(at:, every: nil, missed: :latest, key: nil)
236236
end
237237
end
238238

239-
# The key becomes part of the reminder name, which the database holds in 191
240-
# characters alongside the operation, so it is bounded here rather than
241-
# failing on the insert once the turn is already doing work.
242239
# @rbs ((String | Symbol | Integer)?) -> String?
243240
def validated_reminder_key(key)
244241
return nil if key.nil?
245242

246243
reminder_key = key.to_s
247-
if reminder_key.empty?
248-
raise ArgumentError, "reminder key must not be empty"
249-
end
250-
if reminder_key.length > REMINDER_KEY_LIMIT
251-
raise ArgumentError, "reminder key must be at most #{REMINDER_KEY_LIMIT} characters"
252-
end
244+
raise ArgumentError, "reminder key must not be empty" if reminder_key.empty?
253245

254246
reminder_key
255247
end
256248

249+
# A keyed name is the operation, a colon, and the key, so an operation
250+
# holding a colon of its own would make two different schedules produce one
251+
# name: an unkeyed "deliver:item" and a "deliver" keyed "item" would share a
252+
# row, and the second would silently take the first one's alarm. Refusing a
253+
# colon in the operation keeps unkeyed names free of colons, which leaves
254+
# the two kinds of name disjoint and lets a key hold colons of its own.
255+
#
256+
# The length is checked on the composed name rather than the key alone,
257+
# because a long operation and a short key can exceed the column just as
258+
# easily as the reverse. Both are refused here rather than at the insert,
259+
# once the turn is already doing work.
260+
# @rbs (operation: Symbol | String, key: String?) -> String
261+
def reminder_name(operation:, key:)
262+
operation_name = operation.to_s
263+
if operation_name.include?(REMINDER_KEY_SEPARATOR)
264+
raise ArgumentError,
265+
"reminder operation #{operation_name.inspect} must not contain #{REMINDER_KEY_SEPARATOR.inspect}"
266+
end
267+
return operation_name if key.nil?
268+
269+
name = "#{operation_name}#{REMINDER_KEY_SEPARATOR}#{key}"
270+
if name.length > REMINDER_NAME_LIMIT
271+
raise ArgumentError,
272+
"reminder name #{name.length} characters exceeds the #{REMINDER_NAME_LIMIT} the database holds"
273+
end
274+
275+
name
276+
end
277+
257278
# @rbs (Reference, ?available_at: Time?, ?idempotency_key: String?) -> OperationDispatcher
258279
def send_to(reference, available_at: nil, idempotency_key: nil)
259280
actor_class = SolidObjects.registry.fetch(reference.actor_type)

sig/generated/lib/solid_objects/actor.rbs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,10 @@ module SolidObjects
3232
def members: () -> [ :name, :arguments ]
3333
end
3434

35-
# The reminders table holds a name in 191 characters, and a name is an
36-
# operation, a colon, and a key.
37-
REMINDER_KEY_LIMIT: ::Integer
35+
# The reminders table holds a name in 191 characters.
36+
REMINDER_NAME_LIMIT: ::Integer
37+
38+
REMINDER_KEY_SEPARATOR: ::String
3839

3940
class ReminderIntent < Data
4041
attr_reader name(): untyped
@@ -171,12 +172,23 @@ module SolidObjects
171172
# @rbs (at: Time, ?every: Numeric?, ?missed: Symbol | String, ?key: (String | Symbol | Integer)?) -> OperationDispatcher
172173
def schedule: (at: Time, ?every: Numeric?, ?missed: Symbol | String, ?key: (String | Symbol | Integer)?) -> OperationDispatcher
173174

174-
# The key becomes part of the reminder name, which the database holds in 191
175-
# characters alongside the operation, so it is bounded here rather than
176-
# failing on the insert once the turn is already doing work.
177175
# @rbs ((String | Symbol | Integer)?) -> String?
178176
def validated_reminder_key: ((String | Symbol | Integer)?) -> String?
179177

178+
# A keyed name is the operation, a colon, and the key, so an operation
179+
# holding a colon of its own would make two different schedules produce one
180+
# name: an unkeyed "deliver:item" and a "deliver" keyed "item" would share a
181+
# row, and the second would silently take the first one's alarm. Refusing a
182+
# colon in the operation keeps unkeyed names free of colons, which leaves
183+
# the two kinds of name disjoint and lets a key hold colons of its own.
184+
#
185+
# The length is checked on the composed name rather than the key alone,
186+
# because a long operation and a short key can exceed the column just as
187+
# easily as the reverse. Both are refused here rather than at the insert,
188+
# once the turn is already doing work.
189+
# @rbs (operation: Symbol | String, key: String?) -> String
190+
def reminder_name: (operation: Symbol | String, key: String?) -> String
191+
180192
# @rbs (Reference, ?available_at: Time?, ?idempotency_key: String?) -> OperationDispatcher
181193
def send_to: (Reference, ?available_at: Time?, ?idempotency_key: String?) -> OperationDispatcher
182194

test/unit/actor_test.rb

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,28 @@ def reset
6969
assert CartActor.definition.queries.key?(:items)
7070
end
7171

72+
class ColonOperationActor < SolidObjects::Actor
73+
actor_type "test-colon-operation"
74+
75+
message("fire:item") { nil }
76+
77+
def arm
78+
schedule(at: Time.now.utc + 60).public_send(:"fire:item")
79+
end
80+
end
81+
82+
class LongOperationActor < SolidObjects::Actor
83+
actor_type "test-long-operation"
84+
85+
LONG_OPERATION = ("o" * 100).freeze
86+
87+
message(LONG_OPERATION) { nil }
88+
89+
def arm(key:)
90+
schedule(at: Time.now.utc + 60, key:).public_send(LONG_OPERATION.to_sym)
91+
end
92+
end
93+
7294
class KeyedReminderActor < SolidObjects::Actor
7395
actor_type "test-keyed-reminders"
7496

@@ -104,12 +126,38 @@ def fire
104126
assert_raises(ArgumentError) { actor.invoke("arm", { "key" => "" }) }
105127
end
106128

107-
test "a reminder key too long for the name column is refused" do
129+
test "a composed reminder name too long for the column is refused" do
108130
actor = keyed_reminder_actor
109131

110132
assert_raises(ArgumentError) { actor.invoke("arm", { "key" => "k" * 200 }) }
111133
end
112134

135+
# An unkeyed "fire:item" and a "fire" keyed "item" would otherwise be one name.
136+
test "a reminder operation holding the key separator is refused" do
137+
actor = SolidObjects::State.new(ColonOperationActor.definition.state_definition).then do |state|
138+
ColonOperationActor.new(actor_id: "unit", state:)
139+
end
140+
141+
assert_raises(ArgumentError) { actor.invoke("arm", {}) }
142+
end
143+
144+
test "a key may hold the separator, since the operation may not" do
145+
actor = keyed_reminder_actor
146+
actor.invoke("arm", { "key" => "group:7" })
147+
intent = actor.drain_reminder_intents.sole
148+
149+
assert_equal "fire:group:7", intent.name
150+
assert_equal "fire", intent.operation
151+
end
152+
153+
test "a long operation with a short key is refused when together they overflow" do
154+
actor = SolidObjects::State.new(LongOperationActor.definition.state_definition).then do |state|
155+
LongOperationActor.new(actor_id: "unit", state:)
156+
end
157+
158+
assert_raises(ArgumentError) { actor.invoke("arm", { "key" => "k" * 120 }) }
159+
end
160+
113161
test "reads and writes attributes through actor methods" do
114162
actor = SolidObjects::State.new(CounterActor.definition.state_definition).then do |state|
115163
CounterActor.new(actor_id: "global", state:)

0 commit comments

Comments
 (0)