diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ee90b93e..a063ab6f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -37,12 +37,12 @@ jobs: run: bundle exec rspec - name: Upload coverage to Codecov if: ${{ strategy.job-index == 0 }} # only run codecov on first run - uses: codecov/codecov-action@v6.0.1 + uses: codecov/codecov-action@8cad3ba95e5920c42f44492e54bc9639cba47959 # v6.0.2 with: token: ${{ secrets.CODECOV_TOKEN }} fail_ci_if_error: true verbose: true - file: coverage/coverage.xml + files: coverage/coverage.xml standard: name: Standard runs-on: ubuntu-latest diff --git a/Gemfile.lock b/Gemfile.lock index 07ba489f..ec9a20fc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -10,7 +10,7 @@ GEM base64 (0.3.0) bigdecimal (4.1.1) builder (3.3.0) - concurrent-ruby (1.3.6) + concurrent-ruby (1.3.7) csv (3.3.5) cucumber (11.0.0) base64 (~> 0.2) @@ -43,7 +43,7 @@ GEM reline (>= 0.3.8) diff-lcs (1.6.2) docile (1.4.1) - erb (6.0.2) + erb (6.0.4) ffi (1.17.4) fileutils (1.8.0) io-console (0.8.2) diff --git a/lib/open_feature/sdk/configuration.rb b/lib/open_feature/sdk/configuration.rb index 43ee0930..7784e560 100644 --- a/lib/open_feature/sdk/configuration.rb +++ b/lib/open_feature/sdk/configuration.rb @@ -127,6 +127,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 @@ -134,8 +136,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 @@ -163,10 +168,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 @@ -175,15 +180,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 @@ -226,6 +224,83 @@ 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 + + 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 05a2f5ad..1eace31c 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 extract_provider_name: (untyped provider) -> String def run_handlers_for_provider: (untyped provider, String event_type, Hash[Symbol, untyped] event_details) -> void 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/open_feature/sdk/configuration_spec.rb b/spec/open_feature/sdk/configuration_spec.rb index 7833d870..4dde4645 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 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