Skip to content

Commit 95d63f0

Browse files
committed
refactor: simplify provider initialization with explicit blocking control
Replace event-based flow control in set_provider_and_wait with direct blocking initialization following Go/Java SDK patterns. - Add wait_for_init parameter to set_provider_internal - Remove timeout and event handler cleanup from set_provider_and_wait - Extract init_provider helper for unified initialization logic - Preserve event emission in both sync and async paths - Remove unused private methods Reduces complexity by 118 lines while maintaining 100% test coverage.
1 parent ba4a136 commit 95d63f0

File tree

3 files changed

+47
-168
lines changed

3 files changed

+47
-168
lines changed

lib/open_feature/sdk/configuration.rb

Lines changed: 35 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -52,68 +52,19 @@ def clear_all_handlers
5252

5353
def set_provider(provider, domain: nil)
5454
@provider_mutex.synchronize do
55-
set_provider_internal(provider, domain:)
55+
set_provider_internal(provider, domain: domain, wait_for_init: false)
5656
end
5757
end
5858

59-
def set_provider_and_wait(provider, domain: nil, timeout: 30)
60-
completion_queue = Queue.new
61-
62-
ready_handler = lambda do |event_details|
63-
if event_details[:provider] == provider
64-
completion_queue << {status: :ready}
65-
end
66-
end
67-
68-
error_handler = lambda do |event_details|
69-
if event_details[:provider] == provider
70-
completion_queue << {
71-
status: :error,
72-
message: event_details[:message] || "an unspecified error occurred",
73-
error_code: event_details[:error_code]
74-
}
75-
end
76-
end
77-
78-
add_handler(ProviderEvent::PROVIDER_READY, ready_handler)
79-
add_handler(ProviderEvent::PROVIDER_ERROR, error_handler)
80-
81-
# Lock only while mutating shared state
59+
def set_provider_and_wait(provider, domain: nil)
8260
@provider_mutex.synchronize do
83-
set_provider_internal(provider, domain:)
84-
end
85-
86-
begin
87-
# Wait for initialization to complete, outside the main provider mutex
88-
Timeout.timeout(timeout) do
89-
result = completion_queue.pop
90-
91-
if result[:status] == :error
92-
error_code = result[:error_code] || Provider::ErrorCode::PROVIDER_FATAL
93-
message = result[:message]
94-
raise ProviderInitializationError.new(
95-
"Provider #{provider.class.name} initialization failed: #{message}",
96-
provider:,
97-
error_code:,
98-
original_error: nil # Exceptions not included in events
99-
)
100-
end
101-
end
102-
rescue Timeout::Error => e
103-
raise ProviderInitializationError.new(
104-
"Provider #{provider.class.name} initialization timed out after #{timeout} seconds",
105-
provider:,
106-
original_error: e
107-
)
108-
ensure
109-
remove_handler(ProviderEvent::PROVIDER_READY, ready_handler)
110-
remove_handler(ProviderEvent::PROVIDER_ERROR, error_handler)
61+
set_provider_internal(provider, domain: domain, wait_for_init: true)
11162
end
11263
end
11364

11465
private
11566

116-
def set_provider_internal(provider, domain: nil)
67+
def set_provider_internal(provider, domain:, wait_for_init:)
11768
old_provider = @providers[domain]
11869

11970
begin
@@ -136,23 +87,41 @@ def set_provider_internal(provider, domain: nil)
13687
# Capture evaluation context to prevent race condition
13788
context_for_init = @evaluation_context
13889

139-
Thread.new do
140-
if provider.respond_to?(:init)
141-
init_method = provider.method(:init)
142-
if init_method.parameters.empty?
143-
provider.init
144-
else
145-
provider.init(context_for_init)
146-
end
90+
if wait_for_init
91+
init_provider(provider, context_for_init, raise_on_error: true)
92+
else
93+
Thread.new do
94+
init_provider(provider, context_for_init, raise_on_error: false)
14795
end
96+
end
97+
end
14898

149-
unless provider.is_a?(Provider::EventHandler)
150-
dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY)
99+
def init_provider(provider, context, raise_on_error: false)
100+
if provider.respond_to?(:init)
101+
init_method = provider.method(:init)
102+
if init_method.parameters.empty?
103+
provider.init
104+
else
105+
provider.init(context)
151106
end
152-
rescue => e
153-
dispatch_provider_event(provider, ProviderEvent::PROVIDER_ERROR,
107+
end
108+
109+
unless provider.is_a?(Provider::EventHandler)
110+
dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY)
111+
end
112+
rescue => e
113+
dispatch_provider_event(provider, ProviderEvent::PROVIDER_ERROR,
114+
error_code: Provider::ErrorCode::PROVIDER_FATAL,
115+
message: e.message)
116+
117+
if raise_on_error
118+
# Re-raise as ProviderInitializationError for synchronous callers
119+
raise ProviderInitializationError.new(
120+
"Provider #{provider.class.name} initialization failed: #{e.message}",
121+
provider:,
154122
error_code: Provider::ErrorCode::PROVIDER_FATAL,
155-
message: e.message)
123+
original_error: e
124+
)
156125
end
157126
end
158127

@@ -174,14 +143,6 @@ def provider_state(provider)
174143

175144
private
176145

177-
def handler_count(event_type)
178-
@event_emitter.handler_count(event_type)
179-
end
180-
181-
def total_handler_count
182-
ProviderEvent::ALL_EVENTS.sum { |event_type| handler_count(event_type) }
183-
end
184-
185146
class ProviderEventDispatcher
186147
def initialize(config)
187148
@config = config

spec/open_feature/sdk/configuration_async_spec.rb

Lines changed: 9 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,9 @@ def create_event_aware_provider(init_time: 0.1, &on_init)
3333
include OpenFeature::SDK::Provider::EventHandler
3434

3535
define_method :init do |_evaluation_context|
36-
Thread.new do
37-
sleep(init_time)
38-
on_init&.call
39-
emit_event(OpenFeature::SDK::ProviderEvent::PROVIDER_READY)
40-
end
36+
sleep(init_time)
37+
on_init&.call
38+
emit_event(OpenFeature::SDK::ProviderEvent::PROVIDER_READY)
4139
end
4240

4341
def shutdown
@@ -59,12 +57,8 @@ def create_failing_provider(error_message = "Init failed")
5957
include OpenFeature::SDK::Provider::EventHandler
6058

6159
define_method :init do |_evaluation_context|
62-
Thread.new do
63-
sleep(0.05)
64-
emit_event(OpenFeature::SDK::ProviderEvent::PROVIDER_ERROR,
65-
error_code: OpenFeature::SDK::Provider::ErrorCode::PROVIDER_FATAL,
66-
message: error_message)
67-
end
60+
sleep(0.05) # Simulate some initialization time
61+
raise StandardError, error_message
6862
end
6963

7064
def shutdown
@@ -159,15 +153,15 @@ def shutdown
159153
provider = create_slow_provider(init_time: 0.1) { initialized = true }
160154

161155
expect(initialized).to be false
162-
configuration.set_provider_and_wait(provider, timeout: 1)
156+
configuration.set_provider_and_wait(provider)
163157
expect(initialized).to be true
164158
end
165159

166160
it "returns only after PROVIDER_READY event" do
167161
provider = create_event_aware_provider(init_time: 0.1)
168162

169163
start_time = Time.now
170-
configuration.set_provider_and_wait(provider, timeout: 1)
164+
configuration.set_provider_and_wait(provider)
171165
elapsed = Time.now - start_time
172166

173167
expect(elapsed).to be >= 0.1 # Should wait at least as long as init time
@@ -179,56 +173,11 @@ def shutdown
179173
provider = create_failing_provider("Custom error")
180174

181175
expect do
182-
configuration.set_provider_and_wait(provider, timeout: 1)
176+
configuration.set_provider_and_wait(provider)
183177
end.to raise_error(OpenFeature::SDK::ProviderInitializationError) do |error|
184178
expect(error.message).to include("Custom error")
185179
end
186180
end
187-
188-
it "raises ProviderInitializationError on timeout" do
189-
provider = create_slow_provider(init_time: 2.0) # 2 seconds
190-
191-
expect do
192-
configuration.set_provider_and_wait(provider, timeout: 0.5)
193-
end.to raise_error(OpenFeature::SDK::ProviderInitializationError) do |error|
194-
expect(error.message).to include("timed out after 0.5 seconds")
195-
end
196-
end
197-
end
198-
199-
context "event handler cleanup" do
200-
it "removes event handlers after completion" do
201-
provider = create_slow_provider(init_time: 0.05)
202-
203-
# Get initial handler count
204-
initial_ready_count = configuration.send(:handler_count, OpenFeature::SDK::ProviderEvent::PROVIDER_READY)
205-
initial_error_count = configuration.send(:handler_count, OpenFeature::SDK::ProviderEvent::PROVIDER_ERROR)
206-
207-
configuration.set_provider_and_wait(provider, timeout: 1)
208-
209-
# Handler counts should be back to initial
210-
final_ready_count = configuration.send(:handler_count, OpenFeature::SDK::ProviderEvent::PROVIDER_READY)
211-
final_error_count = configuration.send(:handler_count, OpenFeature::SDK::ProviderEvent::PROVIDER_ERROR)
212-
213-
expect(final_ready_count).to eq(initial_ready_count)
214-
expect(final_error_count).to eq(initial_error_count)
215-
end
216-
217-
it "removes event handlers even on error" do
218-
provider = create_failing_provider
219-
220-
# Get initial handler count
221-
initial_count = configuration.send(:total_handler_count)
222-
223-
expect do
224-
configuration.set_provider_and_wait(provider, timeout: 1)
225-
end.to raise_error(OpenFeature::SDK::ProviderInitializationError)
226-
227-
# Handler count should be back to initial
228-
final_count = configuration.send(:total_handler_count)
229-
230-
expect(final_count).to eq(initial_count)
231-
end
232181
end
233182
end
234183

@@ -261,7 +210,7 @@ def shutdown
261210
provider = OpenFeature::SDK::Provider::NoOpProvider.new
262211

263212
expect do
264-
configuration.set_provider_and_wait(provider, timeout: 1)
213+
configuration.set_provider_and_wait(provider)
265214
end.not_to raise_error
266215

267216
expect(configuration.provider).to eq(provider)

spec/open_feature/sdk/configuration_spec.rb

Lines changed: 3 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@
7474
expect(configuration.provider).to be(provider)
7575
end
7676

77-
it "supports custom timeout" do
77+
it "initializes the provider synchronously" do
7878
expect(provider).to receive(:init).once
7979

80-
configuration.set_provider_and_wait(provider, timeout: 60)
80+
configuration.set_provider_and_wait(provider)
8181

8282
expect(configuration.provider).to be(provider)
8383
end
@@ -119,7 +119,7 @@
119119
expect(error.message).to include("Provider initialization failed")
120120
expect(error.message).to include(error_message)
121121
expect(error.provider).to be(provider)
122-
expect(error.original_error).to be_nil # Provider init errors come through events, so no original exception
122+
expect(error.original_error).to be_a(StandardError) # Synchronous init preserves original exception
123123
expect(error.error_code).to eq(OpenFeature::SDK::Provider::ErrorCode::PROVIDER_FATAL)
124124
end
125125
end
@@ -135,37 +135,6 @@
135135
end
136136
end
137137

138-
context "when provider init times out" do
139-
let(:provider) { OpenFeature::SDK::Provider::InMemoryProvider.new }
140-
141-
before do
142-
allow(provider).to receive(:init) do
143-
sleep 2 # Simulate slow initialization
144-
end
145-
end
146-
147-
it "raises ProviderInitializationError after timeout" do
148-
expect do
149-
configuration.set_provider_and_wait(provider, timeout: 0.1)
150-
end.to raise_error(OpenFeature::SDK::ProviderInitializationError) do |error|
151-
expect(error.message).to include("Provider initialization timed out after 0.1 seconds")
152-
expect(error.provider).to be(provider)
153-
expect(error.original_error).to be_a(Timeout::Error)
154-
expect(error.error_code).to eq(OpenFeature::SDK::Provider::ErrorCode::PROVIDER_FATAL)
155-
end
156-
end
157-
158-
it "leaves the failed provider in place when init times out" do
159-
configuration.provider
160-
161-
expect do
162-
configuration.set_provider_and_wait(provider, timeout: 0.1)
163-
end.to raise_error(OpenFeature::SDK::ProviderInitializationError)
164-
165-
expect(configuration.provider).to be(provider)
166-
end
167-
end
168-
169138
context "when shutting down the old provider fails" do
170139
let(:old_provider) { OpenFeature::SDK::Provider::InMemoryProvider.new }
171140
let(:new_provider) { OpenFeature::SDK::Provider::InMemoryProvider.new }

0 commit comments

Comments
 (0)