From c2f8d04e75bd287319b2f776b790fa0cc37674b5 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 14:14:35 -0400 Subject: [PATCH 1/3] feat(core): pass bound domain to provider init and enforce domain-scoped binding Signed-off-by: Jonathan Norris Signed-off-by: Jose Colella --- lib/open_feature/sdk/configuration.rb | 102 +++++++- sig/open_feature/sdk/configuration.rbs | 2 +- sig/open_feature/sdk/provider.rbs | 3 +- .../domain_scoped_provider_spec.rb | 226 ++++++++++++++++++ spec/support/legacy_init_provider.rb | 15 ++ 5 files changed, 333 insertions(+), 15 deletions(-) create mode 100644 spec/specification/domain_scoped_provider_spec.rb create mode 100644 spec/support/legacy_init_provider.rb diff --git a/lib/open_feature/sdk/configuration.rb b/lib/open_feature/sdk/configuration.rb index f5ce74d8..07f61204 100644 --- a/lib/open_feature/sdk/configuration.rb +++ b/lib/open_feature/sdk/configuration.rb @@ -133,6 +133,8 @@ def set_provider_internal(provider, domain:, wait_for_init:) needs_shutdown = false @provider_mutex.synchronize do + validate_domain_scoped_binding!(provider, domain) + old_provider = @providers[domain] new_providers = @providers.dup @@ -140,8 +142,11 @@ def set_provider_internal(provider, domain:, wait_for_init:) @providers = new_providers # Spec 1.1.2.2: Only initialize if the provider is not already active - # (i.e., not already bound to another domain) - already_active = @providers.any? { |d, p| d != domain && p.equal?(provider) && @provider_state_registry.tracked?(p) } + was_bound_elsewhere = @providers.any? do |d, p| + d != domain && p.equal?(provider) && @provider_state_registry.tracked?(p) + end + was_bound_here = old_provider&.equal?(provider) && @provider_state_registry.tracked?(provider) + already_active = was_bound_elsewhere || was_bound_here needs_init = !already_active if needs_init @@ -171,10 +176,10 @@ def set_provider_internal(provider, domain:, wait_for_init:) # Initialize provider outside the mutex to avoid blocking other operations if needs_init if wait_for_init - init_provider(provider, context_for_init, raise_on_error: true) + init_provider(provider, context_for_init, domain: domain, raise_on_error: true) else Thread.new do - init_provider(provider, context_for_init, raise_on_error: false) + init_provider(provider, context_for_init, domain: domain, raise_on_error: false) end end elsif wait_for_init @@ -183,15 +188,8 @@ def set_provider_internal(provider, domain:, wait_for_init:) end end - def init_provider(provider, context, raise_on_error: false) - if provider.respond_to?(:init) - init_method = provider.method(:init) - if init_method.parameters.empty? - provider.init - else - provider.init(context) - end - end + def init_provider(provider, context, domain:, raise_on_error: false) + call_init(provider, context, domain) dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY) rescue => e @@ -239,6 +237,84 @@ def extract_provider_name(provider) provider.respond_to?(:metadata) ? provider.metadata.name : provider.class.name end + def domain_scoped?(provider) + return false unless declares_domain_scoped?(provider) + + provider.domain_scoped? + end + + def declares_domain_scoped?(provider) + class_declares_method?(provider.class, :domain_scoped?) + end + + def provider_bindings(provider) + @providers.select { |_, p| p.equal?(provider) }.keys + end + + def validate_domain_scoped_binding!(provider, domain) + return unless domain_scoped?(provider) + + bindings = provider_bindings(provider) + return if bindings.empty? + return if bindings.include?(domain) + + raise ArgumentError, "Cannot bind domain-scoped provider to more than one domain" + end + + def call_init(provider, context, domain) + return unless provider.respond_to?(:init) + + if init_accepts_domain?(provider) + if init_accepts_evaluation_context?(provider) + provider.init(context, domain: domain) + else + provider.init(domain: domain) + end + elsif init_accepts_evaluation_context?(provider) + provider.init(context) + else + provider.init + end + end + + def init_accepts_domain?(provider) + init_parameters(provider).any? do |kind, name| + name == :domain || kind == :keyrest + end + end + + def init_accepts_evaluation_context?(provider) + init_parameters(provider).any? do |kind, name| + next false if name == :domain + next true if kind == :keyrest + + kind == :opt || kind == :req + end + end + + def init_parameters(provider) + method = instance_method_on_class(provider.class, :init) + method&.parameters || [] + end + + def instance_method_on_class(klass, method_name) + while klass && klass != Object + if klass.method_defined?(method_name, false) + return klass.instance_method(method_name) + end + klass = klass.superclass + end + nil + end + + def class_declares_method?(klass, method_name) + while klass && klass != Object + return true if klass.method_defined?(method_name, false) + klass = klass.superclass + end + false + end + def run_handlers_for_provider(provider, event_type, event_details) # Run global handlers (API-level, no domain filtering) @event_dispatcher.trigger_event(event_type, event_details) diff --git a/sig/open_feature/sdk/configuration.rbs b/sig/open_feature/sdk/configuration.rbs index c876eedf..f7133dad 100644 --- a/sig/open_feature/sdk/configuration.rbs +++ b/sig/open_feature/sdk/configuration.rbs @@ -32,7 +32,7 @@ module OpenFeature def reset: () -> void def set_provider_internal: (untyped provider, domain: String?, wait_for_init: bool) -> void - def init_provider: (untyped provider, EvaluationContext? context, ?raise_on_error: bool) -> void + def init_provider: (untyped provider, EvaluationContext? context, domain: String?, ?raise_on_error: bool) -> void def dispatch_provider_event: (untyped provider, String event_type, ?Hash[Symbol, untyped] details) -> void def provider_bound?: (untyped provider) -> bool def extract_provider_name: (untyped provider) -> String diff --git a/sig/open_feature/sdk/provider.rbs b/sig/open_feature/sdk/provider.rbs index 008d6de8..3121988a 100644 --- a/sig/open_feature/sdk/provider.rbs +++ b/sig/open_feature/sdk/provider.rbs @@ -11,7 +11,8 @@ module OpenFeature end interface _LifecycleProvider - def init: (?EvaluationContext? evaluation_context) -> void + def init: (?EvaluationContext? evaluation_context, ?domain: String?) -> void + def domain_scoped?: () -> bool def shutdown: () -> void def metadata: () -> ProviderMetadata end diff --git a/spec/specification/domain_scoped_provider_spec.rb b/spec/specification/domain_scoped_provider_spec.rb new file mode 100644 index 00000000..9b4b0cfa --- /dev/null +++ b/spec/specification/domain_scoped_provider_spec.rb @@ -0,0 +1,226 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "Domain-scoped provider binding" do + before do + OpenFeature::SDK.configure do |config| + config.evaluation_context = OpenFeature::SDK::EvaluationContext.new(targeting_key: "user") + end + end + + after do + OpenFeature::SDK.shutdown + end + + let(:domain_scoped_test_provider_class) do + Class.new do + attr_reader :init_domain, :init_calls + + def initialize + @metadata = OpenFeature::SDK::Provider::ProviderMetadata.new(name: "domain-scoped-test") + @init_calls = 0 + @init_domain = :unset + end + + attr_reader :metadata + + def domain_scoped? + true + end + + def init(_evaluation_context = nil, domain: nil) + @init_calls += 1 + @init_domain = domain + end + + def fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_string_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_number_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_integer_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_float_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_object_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + end + end + + context "Requirement 2.4.1" do + specify "the provider mutator passes the bound domain to init for a named provider" do + provider = domain_scoped_test_provider_class.new + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "my-domain") + + expect(provider.init_domain).to eq("my-domain") + end + + specify "the provider mutator passes nil domain to init for the default provider" do + provider = domain_scoped_test_provider_class.new + + OpenFeature::SDK.set_provider_and_wait(provider) + + expect(provider.init_domain).to be_nil + end + end + + let(:configuration) { OpenFeature::SDK::API.instance.configuration } + + context "Condition 1.1.8" do + specify "the provider mutator MUST NOT bind a domain-scoped provider to a second named domain" do + provider = domain_scoped_test_provider_class.new + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-a") + + expect do + OpenFeature::SDK.set_provider(provider, domain: "domain-b") + end.to raise_error(ArgumentError, "Cannot bind domain-scoped provider to more than one domain") + + expect(configuration.provider(domain: "domain-a")).to be(provider) + expect(configuration.instance_variable_get(:@providers)["domain-b"]).to be_nil + end + + specify "the provider mutator MUST NOT bind a domain-scoped named provider as the default provider" do + provider = domain_scoped_test_provider_class.new + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-a") + + expect do + OpenFeature::SDK.set_provider(provider) + end.to raise_error(ArgumentError, "Cannot bind domain-scoped provider to more than one domain") + + expect(configuration.provider(domain: "domain-a")).to be(provider) + expect(configuration.instance_variable_get(:@providers)[nil]).not_to be(provider) + end + + specify "the provider mutator MUST NOT bind a domain-scoped default provider to a named domain" do + provider = domain_scoped_test_provider_class.new + + OpenFeature::SDK.set_provider_and_wait(provider) + + expect do + OpenFeature::SDK.set_provider(provider, domain: "domain-a") + end.to raise_error(ArgumentError, "Cannot bind domain-scoped provider to more than one domain") + + expect(configuration.provider).to be(provider) + expect(configuration.instance_variable_get(:@providers)["domain-a"]).to be_nil + end + + specify "allows rebinding the same domain-scoped provider instance to the same named domain" do + provider = domain_scoped_test_provider_class.new + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-a") + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-a") + + expect(provider.init_calls).to eq(1) + expect(provider.init_domain).to eq("domain-a") + end + + specify "allows rebinding the same domain-scoped provider instance as the default provider" do + provider = domain_scoped_test_provider_class.new + + OpenFeature::SDK.set_provider_and_wait(provider) + OpenFeature::SDK.set_provider_and_wait(provider) + + expect(provider.init_calls).to eq(1) + expect(provider.init_domain).to be_nil + end + + specify "allows a non-domain-scoped provider to back multiple domains" do + provider = OpenFeature::SDK::Provider::InMemoryProvider.new + allow(provider).to receive(:init).and_call_original + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-a") + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-b") + + expect(OpenFeature::SDK.provider(domain: "domain-a")).to be(provider) + expect(OpenFeature::SDK.provider(domain: "domain-b")).to be(provider) + expect(provider).to have_received(:init).once + end + end + + context "legacy init compatibility" do + specify "initializes legacy single-argument init providers when bound to a domain" do + provider = OpenFeature::SDK::LegacyInitProvider.new + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "test-domain") + + expect(provider.init_calls).to eq(1) + expect(provider.last_context).to eq(OpenFeature::SDK.evaluation_context) + expect(configuration.provider_state(provider)).to eq(OpenFeature::SDK::ProviderState::READY) + end + + specify "does not pass domain to legacy init providers" do + provider = OpenFeature::SDK::LegacyInitProvider.new + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "test-domain") + + expect(provider.init_calls).to eq(1) + expect(provider.last_context).to eq(OpenFeature::SDK.evaluation_context) + end + + specify "does not retry init when a domain-aware provider raises" do + broken_provider_class = Class.new do + attr_reader :call_count + + def initialize + @metadata = OpenFeature::SDK::Provider::ProviderMetadata.new(name: "broken") + @call_count = 0 + end + + attr_reader :metadata + + def init(_evaluation_context = nil, domain: nil) + @call_count += 1 + raise TypeError, "configuration error" + end + + def fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_string_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_number_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_integer_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_float_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + + def fetch_object_value(flag_key:, default_value:, evaluation_context: nil) + OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + end + end + + provider = broken_provider_class.new + + expect do + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain") + end.to raise_error(OpenFeature::SDK::ProviderInitializationError) + + expect(provider.call_count).to eq(1) + end + end +end diff --git a/spec/support/legacy_init_provider.rb b/spec/support/legacy_init_provider.rb new file mode 100644 index 00000000..2fa44edb --- /dev/null +++ b/spec/support/legacy_init_provider.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module OpenFeature + module SDK + # Provider mirroring contrib overrides: init(evaluation_context) without domain. + class LegacyInitProvider < Provider::InMemoryProvider + attr_reader :init_calls, :last_context + + def init(evaluation_context = nil) + @init_calls = (@init_calls || 0) + 1 + @last_context = evaluation_context + end + end + end +end From 377741c3b1e7ab4efcb00dcb6142c2ee070b43f2 Mon Sep 17 00:00:00 2001 From: Jonathan Norris Date: Fri, 3 Jul 2026 14:29:50 -0400 Subject: [PATCH 2/3] test: cover domain-aware init paths and fix kwargs-only init dispatch Signed-off-by: Jonathan Norris Signed-off-by: Jose Colella --- lib/open_feature/sdk/configuration.rb | 1 - spec/open_feature/sdk/configuration_spec.rb | 58 +++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/lib/open_feature/sdk/configuration.rb b/lib/open_feature/sdk/configuration.rb index 07f61204..8c2fddfb 100644 --- a/lib/open_feature/sdk/configuration.rb +++ b/lib/open_feature/sdk/configuration.rb @@ -286,7 +286,6 @@ def init_accepts_domain?(provider) def init_accepts_evaluation_context?(provider) init_parameters(provider).any? do |kind, name| next false if name == :domain - next true if kind == :keyrest kind == :opt || kind == :req end diff --git a/spec/open_feature/sdk/configuration_spec.rb b/spec/open_feature/sdk/configuration_spec.rb index e34dad54..c12b7d53 100644 --- a/spec/open_feature/sdk/configuration_spec.rb +++ b/spec/open_feature/sdk/configuration_spec.rb @@ -278,6 +278,64 @@ def metadata expect(provider.init_called).to be true end + + it "passes domain to init when init accepts evaluation context and domain" do + provider = Class.new do + attr_reader :init_domain, :init_context + + def init(evaluation_context = nil, domain: nil) + @init_context = evaluation_context + @init_domain = domain + end + + def metadata + OpenFeature::SDK::Provider::ProviderMetadata.new(name: "DomainAwareProvider") + end + end.new + + context = OpenFeature::SDK::EvaluationContext.new(targeting_key: "user") + configuration.evaluation_context = context + configuration.set_provider_and_wait(provider, domain: "billing") + + expect(provider.init_context).to eq(context) + expect(provider.init_domain).to eq("billing") + end + + it "passes domain to init when init only accepts domain" do + provider = Class.new do + attr_reader :init_domain + + def init(domain: nil) + @init_domain = domain + end + + def metadata + OpenFeature::SDK::Provider::ProviderMetadata.new(name: "DomainOnlyProvider") + end + end.new + + configuration.set_provider_and_wait(provider, domain: "billing") + + expect(provider.init_domain).to eq("billing") + end + + it "passes domain to init when init accepts keyword rest" do + provider = Class.new do + attr_reader :init_kwargs + + def init(**kwargs) + @init_kwargs = kwargs + end + + def metadata + OpenFeature::SDK::Provider::ProviderMetadata.new(name: "KwargsProvider") + end + end.new + + configuration.set_provider_and_wait(provider, domain: "billing") + + expect(provider.init_kwargs[:domain]).to eq("billing") + end end describe "event handler error logging" do From ac6244b0d175e65223132d2cb7855939f8a9ea53 Mon Sep 17 00:00:00 2001 From: Jose Colella Date: Thu, 22 Oct 2026 18:55:44 -0700 Subject: [PATCH 3/3] fix: harden domain-aware init and domain-scoped binding - Read the init signature from the provider object, so delegators, module-defined init and forwarding signatures get the context instead of raising. Pass domain: only to an explicit keyword or named **opts. - Evaluate domain_scoped? before taking the provider lock and detect it via respond_to?; warn when a domain-scoped init can't take domain:. - Track one init attempt per registration: a failed init is retried on the next set, a running init is waited on instead of started twice, and a provider is never reported READY without a successful init. - Settle the attempt before READY/ERROR handlers run so they can register the provider again, fail attempts that no init took over, and release waiters on shutdown. - Test providers subclass InMemoryProvider; drop the duplicate legacy test and the one-use support class. Signed-off-by: Jose Colella Co-Authored-By: Claude Opus 5.5 --- README.md | 10 +- lib/open_feature/sdk/configuration.rb | 151 +++++++------- sig/open_feature/sdk/configuration.rbs | 8 +- sig/open_feature/sdk/provider.rbs | 5 +- spec/open_feature/sdk/configuration_spec.rb | 70 +++++++ .../domain_scoped_provider_spec.rb | 136 ++++++------- .../specification/flag_evaluation_api_spec.rb | 189 ++++++++++++++++++ spec/support/legacy_init_provider.rb | 15 -- 8 files changed, 411 insertions(+), 173 deletions(-) delete mode 100644 spec/support/legacy_init_provider.rb diff --git a/README.md b/README.md index d290f9ac..8482269f 100644 --- a/README.md +++ b/README.md @@ -442,13 +442,21 @@ You’ll then need to write the provider by implementing the `Provider` duck. ```ruby class MyProvider - def init + # Both arguments are optional; declare only the ones you need. + # `domain` is the domain the provider was bound to, or nil for the default provider. + def init(evaluation_context = nil, domain: nil) # Perform any initialization steps with flag management system here # Return value is ignored # **Note** The OpenFeature spec defines a lifecycle method called `initialize` to be called when a new provider is set. # To avoid conflicting with the Ruby `initialize` method, this method should be named `init` when creating a provider. end + # Optional: return true if this provider keeps state for a single domain. + # The SDK then raises ArgumentError if the same instance is bound to a second domain. + def domain_scoped? + false + end + def shutdown # Perform any shutdown/reclamation steps with flag management system here # Return value is ignored diff --git a/lib/open_feature/sdk/configuration.rb b/lib/open_feature/sdk/configuration.rb index 8c2fddfb..fef6839d 100644 --- a/lib/open_feature/sdk/configuration.rb +++ b/lib/open_feature/sdk/configuration.rb @@ -20,9 +20,14 @@ class Configuration attr_accessor :evaluation_context, :hooks, :transaction_context_propagator attr_reader :logger + # One per registration; +done+ is closed once init finishes, so waiters can pop on it + InitAttempt = Struct.new(:state, :error, :done) + private_constant :InitAttempt + def initialize @hooks = [] @providers = {} + @init_attempts = {}.compare_by_identity @provider_mutex = Mutex.new @logger = nil @event_dispatcher = EventDispatcher.new(@logger) @@ -118,6 +123,9 @@ def reset @provider_state_registry.clear @provider_mutex.synchronize do @providers.clear + # Release anyone waiting on an init that shutdown cut short + @init_attempts.each_value { |attempt| attempt.done.close } + @init_attempts.clear end @hooks.clear @evaluation_context = nil @@ -129,11 +137,16 @@ def set_provider_internal(provider, domain:, wait_for_init:) context_for_init = @evaluation_context old_provider = nil + attempt = nil needs_init = false + init_thread = nil needs_shutdown = false + # Provider code runs before taking the lock: it may call back into the SDK + scoped = domain_scoped?(provider) + @provider_mutex.synchronize do - validate_domain_scoped_binding!(provider, domain) + validate_domain_scoped_binding!(provider, domain, scoped) old_provider = @providers[domain] @@ -141,15 +154,12 @@ def set_provider_internal(provider, domain:, wait_for_init:) new_providers[domain] = provider @providers = new_providers - # Spec 1.1.2.2: Only initialize if the provider is not already active - was_bound_elsewhere = @providers.any? do |d, p| - d != domain && p.equal?(provider) && @provider_state_registry.tracked?(p) - end - was_bound_here = old_provider&.equal?(provider) && @provider_state_registry.tracked?(provider) - already_active = was_bound_elsewhere || was_bound_here - needs_init = !already_active + # Spec 1.1.2.2: initialize once per registration; a failed init is retried on the next set + attempt = @init_attempts[provider] + needs_init = attempt.nil? || attempt.state == :failed if needs_init + attempt = @init_attempts[provider] = InitAttempt.new(:running, nil, Thread::Queue.new) @provider_state_registry.set_initial_state(provider) provider.send(:attach, self) if provider.is_a?(Provider::EventEmitter) end @@ -158,6 +168,7 @@ def set_provider_internal(provider, domain:, wait_for_init:) if old_provider && !old_provider.equal?(provider) needs_shutdown = !provider_bound?(old_provider) if needs_shutdown + @init_attempts.delete(old_provider) @provider_state_registry.remove_provider(old_provider) old_provider.send(:detach) if old_provider.is_a?(Provider::EventEmitter) end @@ -176,38 +187,65 @@ def set_provider_internal(provider, domain:, wait_for_init:) # Initialize provider outside the mutex to avoid blocking other operations if needs_init if wait_for_init - init_provider(provider, context_for_init, domain: domain, raise_on_error: true) + init_provider(provider, context_for_init, domain:, attempt:, raise_on_error: true) else - Thread.new do - init_provider(provider, context_for_init, domain: domain, raise_on_error: false) + init_thread = Thread.new do + init_provider(provider, context_for_init, domain:, attempt:, raise_on_error: false) end end elsif wait_for_init - # Provider already active; no init needed but still dispatch READY + # Already registered: wait for an init still in flight, then report its outcome + attempt.done.pop + unless attempt.state == :succeeded + raise attempt.error || ProviderInitializationError.new( + "Provider #{provider.class.name} initialization did not complete", + provider:, + error_code: Provider::ErrorCode::GENERAL + ) + end + dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY) end + ensure + # Fail an attempt no init took over (e.g. a Timeout hit first), or it would stay running forever + finish_init_attempt(attempt, :failed) if needs_init && !init_thread end - def init_provider(provider, context, domain:, raise_on_error: false) + def init_provider(provider, context, domain:, attempt:, raise_on_error: false) call_init(provider, context, domain) + # Settle the attempt before handlers run, so a handler can register this provider again + finish_init_attempt(attempt, :succeeded) dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY) rescue => e # Spec 1.7.5: PROVIDER_FATAL from init makes the provider FATAL error_code = EvaluationError.code_for(e) + error = ProviderInitializationError.new( + "Provider #{provider.class.name} initialization failed: #{e.message}", + provider:, + error_code: error_code, + original_error: e + ) + finish_init_attempt(attempt, :failed, error) dispatch_provider_event(provider, ProviderEvent::PROVIDER_ERROR, error_code: error_code, message: e.message) - if raise_on_error - raise ProviderInitializationError.new( - "Provider #{provider.class.name} initialization failed: #{e.message}", - provider:, - error_code: error_code, - original_error: e - ) + raise error if raise_on_error + ensure + # Reached unsettled only when init raised something other than a StandardError + finish_init_attempt(attempt, :failed) + end + + def finish_init_attempt(attempt, state, error = nil) + @provider_mutex.synchronize do + return unless attempt.state == :running + + attempt.state = state + attempt.error = error end + attempt.done.close end def dispatch_provider_event(provider, event_type, details = {}) @@ -238,25 +276,11 @@ def extract_provider_name(provider) end def domain_scoped?(provider) - return false unless declares_domain_scoped?(provider) - - provider.domain_scoped? - end - - def declares_domain_scoped?(provider) - class_declares_method?(provider.class, :domain_scoped?) - end - - def provider_bindings(provider) - @providers.select { |_, p| p.equal?(provider) }.keys + provider.respond_to?(:domain_scoped?) && provider.domain_scoped? end - def validate_domain_scoped_binding!(provider, domain) - return unless domain_scoped?(provider) - - bindings = provider_bindings(provider) - return if bindings.empty? - return if bindings.include?(domain) + def validate_domain_scoped_binding!(provider, domain, scoped) + return unless scoped && provider_bound?(provider) && !@providers[domain].equal?(provider) raise ArgumentError, "Cannot bind domain-scoped provider to more than one domain" end @@ -264,54 +288,25 @@ def validate_domain_scoped_binding!(provider, domain) def call_init(provider, context, domain) return unless provider.respond_to?(:init) - if init_accepts_domain?(provider) - if init_accepts_evaluation_context?(provider) - provider.init(context, domain: domain) - else - provider.init(domain: domain) - end - elsif init_accepts_evaluation_context?(provider) - provider.init(context) - else - provider.init - end - end - - def init_accepts_domain?(provider) - init_parameters(provider).any? do |kind, name| - name == :domain || kind == :keyrest + parameters = provider.method(:init).parameters + takes_domain = init_accepts_domain?(parameters) + if !takes_domain && domain_scoped?(provider) + @logger&.warn("Provider #{extract_provider_name(provider)} is domain-scoped but its init doesn't accept domain:") end - end - - def init_accepts_evaluation_context?(provider) - init_parameters(provider).any? do |kind, name| - next false if name == :domain - kind == :opt || kind == :req - end + args = init_accepts_evaluation_context?(parameters) ? [context] : [] + provider.init(*args, **(takes_domain ? {domain:} : {})) end - def init_parameters(provider) - method = instance_method_on_class(provider.class, :init) - method&.parameters || [] - end - - def instance_method_on_class(klass, method_name) - while klass && klass != Object - if klass.method_defined?(method_name, false) - return klass.instance_method(method_name) - end - klass = klass.superclass + # Anonymous `**` (from `...`) and delegators' bare `*` can't be trusted to accept `domain:` + def init_accepts_domain?(parameters) + parameters.any? do |kind, name| + ((kind == :key || kind == :keyreq) && name == :domain) || (kind == :keyrest && name && name != :**) end - nil end - def class_declares_method?(klass, method_name) - while klass && klass != Object - return true if klass.method_defined?(method_name, false) - klass = klass.superclass - end - false + def init_accepts_evaluation_context?(parameters) + parameters.any? { |kind, _| kind == :req || kind == :opt || kind == :rest } end def run_handlers_for_provider(provider, event_type, event_details) diff --git a/sig/open_feature/sdk/configuration.rbs b/sig/open_feature/sdk/configuration.rbs index f7133dad..54154aca 100644 --- a/sig/open_feature/sdk/configuration.rbs +++ b/sig/open_feature/sdk/configuration.rbs @@ -32,9 +32,15 @@ module OpenFeature def reset: () -> void def set_provider_internal: (untyped provider, domain: String?, wait_for_init: bool) -> void - def init_provider: (untyped provider, EvaluationContext? context, domain: String?, ?raise_on_error: bool) -> void + def init_provider: (untyped provider, EvaluationContext? context, domain: String?, attempt: untyped, ?raise_on_error: bool) -> void + def finish_init_attempt: (untyped attempt, Symbol state, ?ProviderInitializationError? error) -> void def dispatch_provider_event: (untyped provider, String event_type, ?Hash[Symbol, untyped] details) -> void def provider_bound?: (untyped provider) -> bool + def domain_scoped?: (untyped provider) -> bool + def validate_domain_scoped_binding!: (untyped provider, String? domain, bool scoped) -> void + def call_init: (untyped provider, EvaluationContext? context, String? domain) -> void + def init_accepts_domain?: (Array[[Symbol, Symbol?]] parameters) -> bool + def init_accepts_evaluation_context?: (Array[[Symbol, Symbol?]] parameters) -> bool def extract_provider_name: (untyped provider) -> String def run_handlers_for_provider: (untyped provider, String event_type, Hash[Symbol, untyped] event_details) -> void def run_immediate_handler: (String event_type, _ToProc handler, Client? client) -> void diff --git a/sig/open_feature/sdk/provider.rbs b/sig/open_feature/sdk/provider.rbs index 3121988a..1dc83234 100644 --- a/sig/open_feature/sdk/provider.rbs +++ b/sig/open_feature/sdk/provider.rbs @@ -12,11 +12,14 @@ module OpenFeature interface _LifecycleProvider def init: (?EvaluationContext? evaluation_context, ?domain: String?) -> void - def domain_scoped?: () -> bool def shutdown: () -> void def metadata: () -> ProviderMetadata end + interface _DomainScopedProvider + def domain_scoped?: () -> bool + end + interface _TrackableProvider def track: (String tracking_event_name, ?evaluation_context: EvaluationContext?, ?tracking_event_details: TrackingEventDetails?) -> void end diff --git a/spec/open_feature/sdk/configuration_spec.rb b/spec/open_feature/sdk/configuration_spec.rb index c12b7d53..97a89fb2 100644 --- a/spec/open_feature/sdk/configuration_spec.rb +++ b/spec/open_feature/sdk/configuration_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "spec_helper" +require "delegate" RSpec.describe OpenFeature::SDK::Configuration do subject(:configuration) { described_class.new } @@ -191,6 +192,7 @@ it "handles provider that responds_to init but init is nil" do allow(provider).to receive(:respond_to?).with(:init).and_return(true) allow(provider).to receive(:respond_to?).with(:metadata).and_call_original + allow(provider).to receive(:respond_to?).with(:domain_scoped?).and_call_original allow(provider).to receive(:init).and_return(nil) configuration.set_provider_and_wait(provider) @@ -336,6 +338,74 @@ def metadata expect(provider.init_kwargs[:domain]).to eq("billing") end + + context "when the init signature doesn't name the domain" do + let(:context) { OpenFeature::SDK::EvaluationContext.new(targeting_key: "user") } + + before { configuration.evaluation_context = context } + + def build_provider(&block) + Class.new do + attr_reader :received + + def metadata = OpenFeature::SDK::Provider::ProviderMetadata.new(name: "SignatureProvider") + + class_eval(&block) + end.new + end + + it "passes only the context to init(...) that forwards to a legacy init" do + provider = build_provider do + def init(...) = legacy_init(...) + + def legacy_init(evaluation_context) = @received = evaluation_context + end + + expect { configuration.set_provider_and_wait(provider, domain: "billing") }.not_to raise_error + expect(provider.received).to eq(context) + end + + it "passes only the context to init(*args)" do + provider = build_provider do + def init(*args) = @received = args + end + + configuration.set_provider_and_wait(provider, domain: "billing") + + expect(provider.received).to eq([context]) + end + + it "passes the context to init(evaluation_context) defined in an included module" do + legacy_init = Module.new do + def init(evaluation_context) = @received = evaluation_context + end + provider = build_provider { include legacy_init } + + configuration.set_provider_and_wait(provider, domain: "billing") + + expect(provider.received).to eq(context) + end + + it "passes the context through a SimpleDelegator wrapper" do + inner = build_provider do + def init(evaluation_context) = @received = evaluation_context + end + + configuration.set_provider_and_wait(SimpleDelegator.new(inner), domain: "billing") + + expect(inner.received).to eq(context) + end + + it "passes the domain to init(evaluation_context = nil, **opts)" do + provider = build_provider do + def init(evaluation_context = nil, **opts) = @received = [evaluation_context, opts] + end + + configuration.set_provider_and_wait(provider, domain: "billing") + + expect(provider.received).to eq([context, {domain: "billing"}]) + end + end end describe "event handler error logging" do diff --git a/spec/specification/domain_scoped_provider_spec.rb b/spec/specification/domain_scoped_provider_spec.rb index 9b4b0cfa..0905c84a 100644 --- a/spec/specification/domain_scoped_provider_spec.rb +++ b/spec/specification/domain_scoped_provider_spec.rb @@ -14,53 +14,25 @@ end let(:domain_scoped_test_provider_class) do - Class.new do + Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do attr_reader :init_domain, :init_calls def initialize - @metadata = OpenFeature::SDK::Provider::ProviderMetadata.new(name: "domain-scoped-test") + super @init_calls = 0 @init_domain = :unset end - attr_reader :metadata - - def domain_scoped? - true - end + def domain_scoped? = true def init(_evaluation_context = nil, domain: nil) @init_calls += 1 @init_domain = domain end - - def fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end - - def fetch_string_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end - - def fetch_number_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end - - def fetch_integer_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end - - def fetch_float_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end - - def fetch_object_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end end end - context "Requirement 2.4.1" do + context "Requirements 1.1.2.2 and 2.4.1" do specify "the provider mutator passes the bound domain to init for a named provider" do provider = domain_scoped_test_provider_class.new @@ -80,7 +52,7 @@ def fetch_object_value(flag_key:, default_value:, evaluation_context: nil) let(:configuration) { OpenFeature::SDK::API.instance.configuration } - context "Condition 1.1.8" do + context "Requirement 1.1.8.1" do specify "the provider mutator MUST NOT bind a domain-scoped provider to a second named domain" do provider = domain_scoped_test_provider_class.new @@ -151,70 +123,80 @@ def fetch_object_value(flag_key:, default_value:, evaluation_context: nil) expect(OpenFeature::SDK.provider(domain: "domain-b")).to be(provider) expect(provider).to have_received(:init).once end - end - context "legacy init compatibility" do - specify "initializes legacy single-argument init providers when bound to a domain" do - provider = OpenFeature::SDK::LegacyInitProvider.new + specify "rejects a domain-scoped provider whose domain_scoped? comes from an included module" do + scoped = Module.new { def domain_scoped? = true } + provider = Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do + include scoped - OpenFeature::SDK.set_provider_and_wait(provider, domain: "test-domain") + def init(_evaluation_context = nil, domain: nil) = nil + end.new - expect(provider.init_calls).to eq(1) - expect(provider.last_context).to eq(OpenFeature::SDK.evaluation_context) - expect(configuration.provider_state(provider)).to eq(OpenFeature::SDK::ProviderState::READY) + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-a") + + expect do + OpenFeature::SDK.set_provider(provider, domain: "domain-b") + end.to raise_error(ArgumentError, /more than one domain/) end - specify "does not pass domain to legacy init providers" do - provider = OpenFeature::SDK::LegacyInitProvider.new + specify "evaluates domain_scoped? without holding the provider lock" do + provider_mutex = configuration.instance_variable_get(:@provider_mutex) + lock_held = nil + provider = domain_scoped_test_provider_class.new + provider.define_singleton_method(:domain_scoped?) do + lock_held = provider_mutex.owned? + true + end - OpenFeature::SDK.set_provider_and_wait(provider, domain: "test-domain") + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-a") - expect(provider.init_calls).to eq(1) - expect(provider.last_context).to eq(OpenFeature::SDK.evaluation_context) + expect(lock_held).to be(false) end + end - specify "does not retry init when a domain-aware provider raises" do - broken_provider_class = Class.new do - attr_reader :call_count + context "Requirement 2.4.4" do + after { configuration.logger = nil } - def initialize - @metadata = OpenFeature::SDK::Provider::ProviderMetadata.new(name: "broken") - @call_count = 0 - end + specify "warns when a domain-scoped provider's init can't accept the domain" do + logger = spy("Logger") + configuration.logger = logger + provider = domain_scoped_test_provider_class.new + provider.define_singleton_method(:init) { |_evaluation_context = nil| @init_calls += 1 } - attr_reader :metadata + OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain-a") - def init(_evaluation_context = nil, domain: nil) - @call_count += 1 - raise TypeError, "configuration error" - end + expect(logger).to have_received(:warn).with(/domain-scoped.*domain/) + expect(provider.init_calls).to eq(1) + end + end - def fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end + context "legacy init compatibility" do + specify "initializes a legacy single-argument init provider bound to a domain without passing the domain" do + provider = Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do + attr_reader :init_calls, :last_context - def fetch_string_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + def init(evaluation_context = nil) + @init_calls = (@init_calls || 0) + 1 + @last_context = evaluation_context end + end.new - def fetch_number_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end + OpenFeature::SDK.set_provider_and_wait(provider, domain: "test-domain") - def fetch_integer_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end + expect(provider.init_calls).to eq(1) + expect(provider.last_context).to eq(OpenFeature::SDK.evaluation_context) + expect(configuration.provider_state(provider)).to eq(OpenFeature::SDK::ProviderState::READY) + end - def fetch_float_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) - end + specify "does not retry init when a domain-aware provider raises" do + provider = Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do + attr_reader :call_count - def fetch_object_value(flag_key:, default_value:, evaluation_context: nil) - OpenFeature::SDK::Provider::ResolutionDetails.new(value: default_value) + def init(_evaluation_context = nil, domain: nil) + @call_count = (@call_count || 0) + 1 + raise TypeError, "configuration error" end - end - - provider = broken_provider_class.new + end.new expect do OpenFeature::SDK.set_provider_and_wait(provider, domain: "domain") diff --git a/spec/specification/flag_evaluation_api_spec.rb b/spec/specification/flag_evaluation_api_spec.rb index 30df8ad8..ac0f4eda 100644 --- a/spec/specification/flag_evaluation_api_spec.rb +++ b/spec/specification/flag_evaluation_api_spec.rb @@ -183,6 +183,195 @@ def capture_threads end end + context "Requirement 1.1.2.2: init outcome" do + before { OpenFeature::SDK.configuration.send(:reset) } + after { OpenFeature::SDK.configuration.send(:reset) } + + let(:outcomes) { Queue.new } + let(:aborted_error) { Class.new(Exception) } # rubocop:disable Lint/InheritException + # Each init blocks until an outcome is pushed; :fail raises, :abort raises a non-StandardError + let(:provider) do + queue = outcomes + aborted = aborted_error + Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do + attr_reader :init_calls + + define_method(:init) do |_evaluation_context = nil| + @init_calls = (@init_calls || 0) + 1 + outcome = queue.pop + raise "init failed" if outcome == :fail + raise aborted if outcome == :abort + end + end.new + end + + # Blocks until a set_provider_and_wait is waiting on the provider's running init + def wait_for_waiter(provider) + attempt = OpenFeature::SDK.configuration.instance_variable_get(:@init_attempts)[provider] + 200.times do + break if attempt.done.num_waiting.positive? + sleep 0.01 + end + end + + # A thread already waiting on the running init; its value is the error it raised, if any + def start_waiter(domain) + waiter = background do + OpenFeature::SDK.set_provider_and_wait(provider, domain:) + rescue OpenFeature::SDK::ProviderInitializationError => e + e + end + wait_for_waiter(provider) + waiter + end + + specify "a provider whose init failed is initialized again when set again" do + outcomes << :fail << :ok + + expect { OpenFeature::SDK.set_provider_and_wait(provider) }.to raise_error(OpenFeature::SDK::ProviderInitializationError) + OpenFeature::SDK.set_provider_and_wait(provider) + + expect(provider.init_calls).to eq(2) + expect(OpenFeature::SDK.configuration.provider_state(provider)).to eq(OpenFeature::SDK::ProviderState::READY) + end + + specify "a provider whose init failed is initialized again when bound to another domain" do + outcomes << :fail << :ok + + expect { OpenFeature::SDK.set_provider_and_wait(provider, domain: "a") }.to raise_error(OpenFeature::SDK::ProviderInitializationError) + OpenFeature::SDK.set_provider_and_wait(provider, domain: "b") + + expect(provider.init_calls).to eq(2) + end + + specify "binding another domain while init is running starts no second init" do + threads = capture_threads + + OpenFeature::SDK.set_provider(provider, domain: "a") + OpenFeature::SDK.set_provider(provider, domain: "b") + outcomes << :ok << :ok + threads.each { |t| t.join(5) } + + expect(provider.init_calls).to eq(1) + end + + specify "set_provider_and_wait waits for an init that's already running" do + OpenFeature::SDK.set_provider(provider, domain: "a") + waiter = start_waiter("b") + + outcomes << :ok << :ok + + expect(waiter.join(5)).to be(waiter) + expect(provider.init_calls).to eq(1) + expect(OpenFeature::SDK.configuration.provider_state(provider)).to eq(OpenFeature::SDK::ProviderState::READY) + end + + specify "set_provider_and_wait raises when the running init it waited for fails" do + OpenFeature::SDK.set_provider(provider, domain: "a") + waiter = start_waiter("b") + + outcomes << :fail << :ok + + expect(waiter.join(5)&.value).to be_a(OpenFeature::SDK::ProviderInitializationError) + expect(provider.init_calls).to eq(1) + end + + specify "a provider that reported ERROR after a successful init isn't initialized again" do + outcomes << :ok << :ok + OpenFeature::SDK.set_provider_and_wait(provider, domain: "a") + provider.emit_event(OpenFeature::SDK::ProviderEvent::PROVIDER_ERROR, message: "lost connection") + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "b") + + expect(provider.init_calls).to eq(1) + end + + specify "a provider set again after shutdown interrupted its init is initialized again" do + threads = capture_threads + OpenFeature::SDK.set_provider(provider) + OpenFeature::SDK.shutdown + outcomes << :ok + threads.each { |t| t.join(5) } + outcomes << :ok + + OpenFeature::SDK.set_provider_and_wait(provider) + + expect(provider.init_calls).to eq(2) + end + + specify "a READY handler can bind the same provider to another domain" do + outcomes << :ok + OpenFeature::SDK.add_handler(OpenFeature::SDK::ProviderEvent::PROVIDER_READY, lambda do |_| + next if OpenFeature::SDK.provider(domain: "b").equal?(provider) + + OpenFeature::SDK.set_provider_and_wait(provider, domain: "b") + end) + + setter = Thread.new { OpenFeature::SDK.set_provider_and_wait(provider, domain: "a") } + + expect(setter.join(2)).to be(setter) + expect(OpenFeature::SDK.configuration.instance_variable_get(:@providers)["b"]).to be(provider) + end + + specify "an ERROR handler can retry a failed init" do + outcomes << :fail << :ok + retried = false + OpenFeature::SDK.add_handler(OpenFeature::SDK::ProviderEvent::PROVIDER_ERROR, lambda do |_| + next if retried + + retried = true + OpenFeature::SDK.set_provider_and_wait(provider) + end) + + setter = Thread.new do + OpenFeature::SDK.set_provider_and_wait(provider) + rescue OpenFeature::SDK::ProviderInitializationError + :raised + end + + expect(setter.join(2)&.value).to eq(:raised) + expect(provider.init_calls).to eq(2) + expect(OpenFeature::SDK.configuration.provider_state(provider)).to eq(OpenFeature::SDK::ProviderState::READY) + end + + specify "an exception before init starts leaves the provider able to initialize" do + error_class = aborted_error + old = Class.new(OpenFeature::SDK::Provider::InMemoryProvider) { define_method(:shutdown) { raise error_class } }.new + OpenFeature::SDK.set_provider_and_wait(old) + + expect { OpenFeature::SDK.set_provider_and_wait(provider) }.to raise_error(error_class) + outcomes << :ok + setter = Thread.new { OpenFeature::SDK.set_provider_and_wait(provider) } + + expect(setter.join(2)).to be(setter) + expect(provider.init_calls).to eq(1) + expect(OpenFeature::SDK.configuration.provider_state(provider)).to eq(OpenFeature::SDK::ProviderState::READY) + end + + specify "a waiter raises when the init it waited for was stopped by a non-StandardError" do + threads = capture_threads + OpenFeature::SDK.set_provider(provider, domain: "a") + waiter = start_waiter("b") + threads.each { |t| t.report_on_exception = false } + + outcomes << :abort + + expect(waiter.join(2)&.value).to be_a(OpenFeature::SDK::ProviderInitializationError) + expect(OpenFeature::SDK.configuration.provider_state(provider)).not_to eq(OpenFeature::SDK::ProviderState::READY) + end + + specify "shutdown releases a waiter whose init hasn't finished" do + OpenFeature::SDK.set_provider(provider, domain: "a") + waiter = start_waiter("b") + + OpenFeature::SDK.shutdown + + expect(waiter.join(2)&.value).to be_a(OpenFeature::SDK::ProviderInitializationError) + ensure + outcomes << :ok + end + end + context "Requirement 1.1.2.4" do specify "The API SHOULD provide functions to set a provider and wait for the initialize function to complete or abnormally terminate" do provider = OpenFeature::SDK::Provider::InMemoryProvider.new diff --git a/spec/support/legacy_init_provider.rb b/spec/support/legacy_init_provider.rb deleted file mode 100644 index 2fa44edb..00000000 --- a/spec/support/legacy_init_provider.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module OpenFeature - module SDK - # Provider mirroring contrib overrides: init(evaluation_context) without domain. - class LegacyInitProvider < Provider::InMemoryProvider - attr_reader :init_calls, :last_context - - def init(evaluation_context = nil) - @init_calls = (@init_calls || 0) + 1 - @last_context = evaluation_context - end - end - end -end