Skip to content

Commit 693fe99

Browse files
razor-xclaude
andauthored
fix: Validate the wait_for_action_attempt option (#573)
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4a826c2 commit 693fe99

5 files changed

Lines changed: 148 additions & 2 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ and every wait polls at least once,
168168
even when the `timeout` is shorter than the `polling_interval`.
169169
The `timeout` must not be negative,
170170
and the `polling_interval` must be greater than zero.
171+
The option accepts `true`, `false`, or a hash with `timeout` and `polling_interval` keys;
172+
any other value raises a `Seam::Http::Options::SeamInvalidOptionsError`.
171173

172174
The `error` and `result` values are only present for their matching status:
173175
`error` is `nil` unless the `status` is `"error"`,

‎lib/seam/action_attempt_resolver.rb‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,65 @@
11
# frozen_string_literal: true
22

3+
require_relative "deep_hash_accessor"
34
require_relative "options"
45
require_relative "wait_for_action_attempt"
56

67
module Seam
78
class ActionAttemptResolver
89
TIMEOUT = 5.0
910
POLLING_INTERVAL = 0.5
11+
WAIT_FOR_ACTION_ATTEMPT_OPTION_KEYS = %i[timeout polling_interval].freeze
1012

1113
def self.resolve(action_attempt, client, wait_for_action_attempt)
14+
validate_wait_for_action_attempt(wait_for_action_attempt)
15+
1216
return wait_until_resolved(action_attempt, client) if wait_for_action_attempt == true
1317

1418
options = wait_options(wait_for_action_attempt)
1519
return action_attempt if options.nil?
1620

21+
options = options.transform_keys(&:to_sym)
22+
1723
wait_until_resolved(action_attempt, client, timeout: options[:timeout],
1824
polling_interval: options[:polling_interval])
1925
end
2026

27+
def self.normalize_wait_for_action_attempt(wait_for_action_attempt)
28+
return true if wait_for_action_attempt.nil?
29+
30+
validate_wait_for_action_attempt(wait_for_action_attempt)
31+
32+
wait_for_action_attempt
33+
end
34+
35+
def self.validate_wait_for_action_attempt(wait_for_action_attempt)
36+
return if wait_for_action_attempt == true || wait_for_action_attempt == false
37+
38+
options = wait_options(wait_for_action_attempt)
39+
40+
if options.nil?
41+
raise Http::Options::SeamInvalidOptionsError.new(
42+
"The wait_for_action_attempt option must be true, false, or a hash with " \
43+
"\"timeout\" and \"polling_interval\" keys, got #{wait_for_action_attempt.class}"
44+
)
45+
end
46+
47+
options.each do |key, option|
48+
unless WAIT_FOR_ACTION_ATTEMPT_OPTION_KEYS.include?(key.to_s.to_sym)
49+
raise Http::Options::SeamInvalidOptionsError.new(
50+
"The wait_for_action_attempt option got an unknown key #{key.inspect}, " \
51+
"expected \"timeout\" or \"polling_interval\""
52+
)
53+
end
54+
55+
if option == true || option == false || !option.is_a?(Numeric)
56+
raise Http::Options::SeamInvalidOptionsError.new(
57+
"The wait_for_action_attempt option #{key.inspect} must be a number, got #{option.class}"
58+
)
59+
end
60+
end
61+
end
62+
2163
# The client wraps its defaults in a DeepHashAccessor, so the hash form of
2264
# this option reaches here as an accessor when it comes from the client
2365
# and as a plain Hash when it comes from the method call.

‎lib/seam/http_single_workspace.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# frozen_string_literal: true
22

3+
require_relative "action_attempt_resolver"
34
require_relative "request"
45
require_relative "parse_options"
56
require_relative "resources/index"
@@ -41,7 +42,8 @@ def initialize(client: nil, api_key: OPTION_NOT_PROVIDED, personal_access_token:
4142
)
4243
end
4344

44-
wait_for_action_attempt = true if wait_for_action_attempt.equal?(OPTION_NOT_PROVIDED)
45+
wait_for_action_attempt = nil if wait_for_action_attempt.equal?(OPTION_NOT_PROVIDED)
46+
wait_for_action_attempt = Seam::ActionAttemptResolver.normalize_wait_for_action_attempt(wait_for_action_attempt)
4547
@defaults = Seam::DeepHashAccessor.new({"wait_for_action_attempt" => wait_for_action_attempt})
4648

4749
# A client carries its own endpoint and authorization, so the auth

‎lib/seam/http_without_workspace.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# frozen_string_literal: true
22

3+
require_relative "action_attempt_resolver"
34
require_relative "request"
45
require_relative "parse_options"
56
require_relative "deep_hash_accessor"
@@ -34,7 +35,8 @@ def initialize(client: nil, personal_access_token: OPTION_NOT_PROVIDED, endpoint
3435
)
3536
end
3637

37-
wait_for_action_attempt = true if wait_for_action_attempt.equal?(OPTION_NOT_PROVIDED)
38+
wait_for_action_attempt = nil if wait_for_action_attempt.equal?(OPTION_NOT_PROVIDED)
39+
wait_for_action_attempt = Seam::ActionAttemptResolver.normalize_wait_for_action_attempt(wait_for_action_attempt)
3840
@defaults = Seam::DeepHashAccessor.new({"wait_for_action_attempt" => wait_for_action_attempt})
3941

4042
@client = client || begin
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe "the wait_for_action_attempt option" do
4+
let(:api_key) { "seam_some_api_key" }
5+
6+
def invalid_options_error(message)
7+
raise_error(Seam::Http::Options::SeamInvalidOptionsError, "Seam received invalid options: #{message}")
8+
end
9+
10+
describe "on the client" do
11+
it "rejects a value that is neither a boolean nor a hash" do
12+
expect { Seam.new(api_key: api_key, wait_for_action_attempt: 1) }.to invalid_options_error(
13+
'The wait_for_action_attempt option must be true, false, or a hash with "timeout" and "polling_interval" keys, got Integer'
14+
)
15+
end
16+
17+
it "rejects a value that is neither a boolean nor a hash on the without-workspace client" do
18+
expect do
19+
Seam::Http::WithoutWorkspace.new(personal_access_token: "seam_at_token", wait_for_action_attempt: "true")
20+
end.to invalid_options_error(
21+
'The wait_for_action_attempt option must be true, false, or a hash with "timeout" and "polling_interval" keys, got String'
22+
)
23+
end
24+
25+
it "rejects an unknown option key" do
26+
expect { Seam.new(api_key: api_key, wait_for_action_attempt: {poll_interval: 1}) }.to invalid_options_error(
27+
'The wait_for_action_attempt option got an unknown key :poll_interval, expected "timeout" or "polling_interval"'
28+
)
29+
end
30+
31+
it "rejects a non-numeric option value" do
32+
expect { Seam.new(api_key: api_key, wait_for_action_attempt: {timeout: "5"}) }.to invalid_options_error(
33+
"The wait_for_action_attempt option :timeout must be a number, got String"
34+
)
35+
end
36+
37+
it "rejects a boolean option value" do
38+
expect { Seam.new(api_key: api_key, wait_for_action_attempt: {timeout: true}) }.to invalid_options_error(
39+
"The wait_for_action_attempt option :timeout must be a number, got TrueClass"
40+
)
41+
end
42+
43+
it "treats nil as the default" do
44+
seam = Seam.new(api_key: api_key, wait_for_action_attempt: nil)
45+
46+
expect(seam.defaults.wait_for_action_attempt).to be true
47+
end
48+
49+
it "accepts string keys" do
50+
seam = Seam.new(api_key: api_key, wait_for_action_attempt: {"timeout" => 1, "polling_interval" => 0.1})
51+
52+
expect(seam.defaults.wait_for_action_attempt.to_h).to eq("timeout" => 1, "polling_interval" => 0.1)
53+
end
54+
end
55+
56+
describe "on a route call", recorder: true do
57+
def pending_action_attempt
58+
{action_attempt: {action_attempt_id: "attempt-1", action_type: "UNLOCK_DOOR", status: "pending"}}.to_json
59+
end
60+
61+
def unlock_door(wait_for_action_attempt)
62+
seam.locks.unlock_door(device_id: "device-1", wait_for_action_attempt: wait_for_action_attempt)
63+
end
64+
65+
before { recorder.respond_with(pending_action_attempt) }
66+
67+
it "rejects a truthy value that is not true before polling" do
68+
expect { unlock_door(1) }.to invalid_options_error(
69+
'The wait_for_action_attempt option must be true, false, or a hash with "timeout" and "polling_interval" keys, got Integer'
70+
)
71+
72+
expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door"])
73+
end
74+
75+
it "rejects an unknown option key before polling" do
76+
expect { unlock_door({poll_interval: 1}) }.to invalid_options_error(
77+
'The wait_for_action_attempt option got an unknown key :poll_interval, expected "timeout" or "polling_interval"'
78+
)
79+
80+
expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door"])
81+
end
82+
83+
it "honors string option keys" do
84+
expect { unlock_door({"timeout" => 0.1, "polling_interval" => 3}) }.to raise_error(Seam::ActionAttemptTimeoutError)
85+
86+
expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door", "/action_attempts/get"])
87+
end
88+
89+
it "honors string option keys set on the client" do
90+
seam = Seam.new(api_key: "seam_some_api_key", endpoint: recorder.endpoint,
91+
wait_for_action_attempt: {"timeout" => 0.1, "polling_interval" => 3})
92+
93+
expect { seam.locks.unlock_door(device_id: "device-1") }.to raise_error(Seam::ActionAttemptTimeoutError)
94+
95+
expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door", "/action_attempts/get"])
96+
end
97+
end
98+
end

0 commit comments

Comments
 (0)