diff --git a/README.md b/README.md index 5645dffb..864109a2 100644 --- a/README.md +++ b/README.md @@ -566,6 +566,11 @@ class MyLoggingHook end ``` +Hooks can read their inputs but not change them: + +- Only `before` hooks can change the evaluation context: return an `EvaluationContext` for the SDK to merge in, or assign `hook_context.evaluation_context`. By the time `after`, `error` and `finally` run, it's a frozen copy, so assigning it or writing to its `fields` raises `FrozenError`. Nested field values aren't frozen, because they belong to the caller. +- The flag key, the default value, the hints, and the client and provider metadata are frozen copies. Your own objects aren't frozen. + > Built a new hook? [Let us know](https://github.com/open-feature/openfeature.dev/issues/new?assignees=&labels=hook&projects=&template=document-hook.yaml&title=%5BHook%5D%3A+) so we can add it to the docs! diff --git a/lib/open_feature/sdk/client.rb b/lib/open_feature/sdk/client.rb index e38d2eb3..c710d97d 100644 --- a/lib/open_feature/sdk/client.rb +++ b/lib/open_feature/sdk/client.rb @@ -134,7 +134,7 @@ def fetch_details(type:, flag_key:, default_value:, evaluation_context: nil, inv executor.run_short_circuit(ordered_hooks: ordered_hooks, hook_context: hook_context, hints: hints, evaluation_details: evaluation_details) evaluation_details else - executor.execute(ordered_hooks: ordered_hooks, hook_context: hook_context, hints: hints) do |hctx| + executor.execute(ordered_hooks: ordered_hooks, hook_context: hook_context, hints: hints, default_value:) do |hctx| evaluate_flag(provider: provider, type: type, flag_key: flag_key, default_value: default_value, evaluation_context: hctx.evaluation_context) end end diff --git a/lib/open_feature/sdk/evaluation_context.rb b/lib/open_feature/sdk/evaluation_context.rb index e4fb3581..8fb8c940 100644 --- a/lib/open_feature/sdk/evaluation_context.rb +++ b/lib/open_feature/sdk/evaluation_context.rb @@ -26,6 +26,16 @@ def merge(overriding_context) ) end + def initialize_copy(source) + super + @fields = @fields.dup + end + + def freeze + fields.freeze + super + end + def ==(other) fields == other.fields end diff --git a/lib/open_feature/sdk/hooks/frozen_copy.rb b/lib/open_feature/sdk/hooks/frozen_copy.rb new file mode 100644 index 00000000..9b8e3d3b --- /dev/null +++ b/lib/open_feature/sdk/hooks/frozen_copy.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module OpenFeature + module SDK + module Hooks + # A deep-frozen copy of a hook input, so hooks can't change it and the caller's object isn't frozen (spec 4.1.3, 4.2.2) + def self.frozen_copy(value) + Ractor.make_shareable(value, copy: true) + rescue + value.dup.freeze # shortcut: when something inside can't be copied (a Mutex, Queue or Proc), only the top level is frozen + end + end + end +end diff --git a/lib/open_feature/sdk/hooks/hints.rb b/lib/open_feature/sdk/hooks/hints.rb index 57eb7480..6ce0e727 100644 --- a/lib/open_feature/sdk/hooks/hints.rb +++ b/lib/open_feature/sdk/hooks/hints.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "delegate" +require_relative "frozen_copy" module OpenFeature module SDK @@ -13,8 +14,7 @@ def initialize(hash = {}) assert_allowed_key(key) assert_allowed_value(value) end - @hash = hash.dup - super(@hash) + super(Hooks.frozen_copy(hash)) freeze end diff --git a/lib/open_feature/sdk/hooks/hook_context.rb b/lib/open_feature/sdk/hooks/hook_context.rb index 13ffe01a..eda378ec 100644 --- a/lib/open_feature/sdk/hooks/hook_context.rb +++ b/lib/open_feature/sdk/hooks/hook_context.rb @@ -1,28 +1,45 @@ # frozen_string_literal: true +require_relative "frozen_copy" + module OpenFeature module SDK module Hooks # Provides context to hook stages during flag evaluation. # - # Per spec 4.1.1-4.1.5: + # Per spec 4.1.1-4.1.5 and 4.2.2: # - flag_key, flag_value_type, default_value are immutable (4.1.3) - # - client_metadata, provider_metadata are optional (4.1.2) - # - evaluation_context is mutable (for before hooks to modify, 4.1.4.1) + # - client_metadata, provider_metadata are optional (4.1.2) and immutable (4.2.2.2, 4.2.2.3) + # - evaluation_context is mutable only in before hooks; HookExecutor freezes the hook context after them (4.1.4.1) class HookContext - attr_reader :flag_key, :flag_value_type, :default_value, - :client_metadata, :provider_metadata + attr_reader :flag_key, :flag_value_type, :client_metadata, :provider_metadata attr_accessor :evaluation_context def initialize(flag_key:, flag_value_type:, default_value:, evaluation_context:, client_metadata: nil, provider_metadata: nil) - @flag_key = flag_key.freeze + @flag_key = Hooks.frozen_copy(flag_key) @flag_value_type = flag_value_type.freeze - @default_value = default_value.freeze + @source_default_value = default_value @evaluation_context = evaluation_context @client_metadata = client_metadata - @provider_metadata = provider_metadata + @provider_metadata = Hooks.frozen_copy(provider_metadata) @hook_data = {} + @copies = {} + end + + # Copied on first read: most hooks never read it, and the copy grows with the value + def default_value + @copies.fetch(:default_value) { @copies[:default_value] = Hooks.frozen_copy(@source_default_value) } + end + + # Swaps in a frozen copy of the evaluation context, so neither the caller's nor a before hook's object is frozen. + # Top-level strings are copied too, so a hook can't edit a targeting key in place. + def freeze + return self if frozen? + @evaluation_context &&= @evaluation_context.dup.tap do |copy| + copy.fields.transform_values! { |value| (value.is_a?(String) && !value.frozen?) ? value.dup.freeze : value } + end.freeze + super end # Returns a mutable hash scoped to the given hook instance. diff --git a/lib/open_feature/sdk/hooks/hook_executor.rb b/lib/open_feature/sdk/hooks/hook_executor.rb index ba85ce93..beffa9da 100644 --- a/lib/open_feature/sdk/hooks/hook_executor.rb +++ b/lib/open_feature/sdk/hooks/hook_executor.rb @@ -13,6 +13,9 @@ module Hooks # - Before/after hook error → stop remaining hooks, run error hooks, return default # - Error hook error → log, continue remaining error hooks # - Finally hook error → log, continue remaining finally hooks + # + # The hook context is frozen before after, error and finally hooks run, so only before hooks + # can change the evaluation context (spec 4.1.4.1). Not earlier: some providers write to it. class HookExecutor def initialize(logger: nil) @logger = logger @@ -37,9 +40,10 @@ def run_short_circuit(ordered_hooks:, hook_context:, hints:, evaluation_details: # @param ordered_hooks [Array] hooks in before-order (API, Client, Invocation, Provider) # @param hook_context [HookContext] the hook context # @param hints [Hints] hook hints + # @param default_value [Object] returned if evaluation raises; the caller's own object, not the hooks' frozen copy # @param evaluate_block [Proc] the flag evaluation to wrap # @return [EvaluationDetails] the evaluation result - def execute(ordered_hooks:, hook_context:, hints:, &evaluate_block) + def execute(ordered_hooks:, hook_context:, hints:, default_value:, &evaluate_block) evaluation_details = nil begin @@ -60,7 +64,7 @@ def execute(ordered_hooks:, hook_context:, hints:, &evaluate_block) evaluation_details = EvaluationDetails.new( flag_key: hook_context.flag_key, resolution_details: Provider::ResolutionDetails.new( - value: hook_context.default_value, + value: default_value, error_code: EvaluationError.code_for(e), reason: Provider::Reason::ERROR, error_message: e.message @@ -91,6 +95,7 @@ def run_before_hooks(hooks, hook_context, hints) # Spec 4.4.2: After hooks run in reverse order: Provider → Invocation → Client → API def run_after_hooks(hooks, hook_context, evaluation_details, hints) + hook_context.freeze hooks.reverse_each do |hook| next unless hook.respond_to?(:after) hook.after(hook_context: hook_context, evaluation_details: evaluation_details, hints: hints) @@ -100,6 +105,7 @@ def run_after_hooks(hooks, hook_context, evaluation_details, hints) # Spec 4.4.4: Error hooks run in reverse order. # If an error hook itself errors, log and continue remaining error hooks. def run_error_hooks(hooks, hook_context, exception, hints) + hook_context.freeze hooks.reverse_each do |hook| next unless hook.respond_to?(:error) hook.error(hook_context: hook_context, exception: exception, hints: hints) @@ -111,6 +117,7 @@ def run_error_hooks(hooks, hook_context, exception, hints) # Spec 4.4.3: Finally hooks run in reverse order unconditionally. # If a finally hook errors, log and continue remaining finally hooks. def run_finally_hooks(hooks, hook_context, evaluation_details, hints) + hook_context.freeze hooks.reverse_each do |hook| next unless hook.respond_to?(:finally) hook.finally(hook_context: hook_context, evaluation_details: evaluation_details, hints: hints) diff --git a/sig/open_feature/sdk/evaluation_context.rbs b/sig/open_feature/sdk/evaluation_context.rbs index 919fc1ff..6fb22fc2 100644 --- a/sig/open_feature/sdk/evaluation_context.rbs +++ b/sig/open_feature/sdk/evaluation_context.rbs @@ -13,6 +13,10 @@ module OpenFeature def merge: (EvaluationContext overriding_context) -> EvaluationContext + def initialize_copy: (EvaluationContext source) -> void + + def freeze: () -> self + def ==: (untyped other) -> bool end end diff --git a/sig/open_feature/sdk/hooks/frozen_copy.rbs b/sig/open_feature/sdk/hooks/frozen_copy.rbs new file mode 100644 index 00000000..014fd8ec --- /dev/null +++ b/sig/open_feature/sdk/hooks/frozen_copy.rbs @@ -0,0 +1,7 @@ +module OpenFeature + module SDK + module Hooks + def self.frozen_copy: [T] (T value) -> T + end + end +end diff --git a/sig/open_feature/sdk/hooks/hook_context.rbs b/sig/open_feature/sdk/hooks/hook_context.rbs index 21325382..61d36f62 100644 --- a/sig/open_feature/sdk/hooks/hook_context.rbs +++ b/sig/open_feature/sdk/hooks/hook_context.rbs @@ -4,13 +4,16 @@ module OpenFeature class HookContext attr_reader flag_key: String attr_reader flag_value_type: Symbol - attr_reader default_value: untyped attr_reader client_metadata: ClientMetadata? attr_reader provider_metadata: Provider::ProviderMetadata? attr_accessor evaluation_context: EvaluationContext? def initialize: (flag_key: String, flag_value_type: Symbol, default_value: untyped, evaluation_context: EvaluationContext?, ?client_metadata: ClientMetadata?, ?provider_metadata: Provider::ProviderMetadata?) -> void + def default_value: () -> untyped + + def freeze: () -> self + def hook_data_for: (untyped hook) -> Hash[untyped, untyped] end end diff --git a/sig/open_feature/sdk/hooks/hook_executor.rbs b/sig/open_feature/sdk/hooks/hook_executor.rbs index 19396cf1..49fc5cbc 100644 --- a/sig/open_feature/sdk/hooks/hook_executor.rbs +++ b/sig/open_feature/sdk/hooks/hook_executor.rbs @@ -6,7 +6,7 @@ module OpenFeature def run_short_circuit: (ordered_hooks: Array[untyped], hook_context: HookContext, hints: Hints, evaluation_details: EvaluationDetails) -> void - def execute: (ordered_hooks: Array[untyped], hook_context: HookContext, hints: Hints) { (HookContext) -> EvaluationDetails } -> EvaluationDetails? + def execute: (ordered_hooks: Array[untyped], hook_context: HookContext, hints: Hints, default_value: untyped) { (HookContext) -> EvaluationDetails } -> EvaluationDetails? private diff --git a/spec/open_feature/sdk/hooks/hints_spec.rb b/spec/open_feature/sdk/hooks/hints_spec.rb index f73d2537..72bd61fe 100644 --- a/spec/open_feature/sdk/hooks/hints_spec.rb +++ b/spec/open_feature/sdk/hooks/hints_spec.rb @@ -16,9 +16,9 @@ expect { hints[:new_key] = "new_value" }.to raise_error(FrozenError) end - it "does allow modification of existing values" do - expect(hints[:key]).to_not be_frozen - expect { hints[:key] << "abc" }.to_not raise_error + it "does not allow modification of nested values" do + expect(hints[:nested][:key]).to be_frozen + expect { hints[:key] << "abc" }.to raise_error(FrozenError) end it "does not allow deletion of keys" do @@ -41,8 +41,18 @@ end end - it "does not freeze the original hash" do + it "does not freeze the original hash or its values" do expect(hint_hash).not_to be_frozen + expect(hint_hash[:nested][:key]).not_to be_frozen + end + + it "freezes only the top level when a nested value can't be copied" do + source = {key: {lock: Mutex.new}} + hints = described_class.new(source) + + expect(hints[:key]).to eq(source[:key]) + expect { hints[:other] = 1 }.to raise_error(FrozenError) + expect(source).not_to be_frozen end end end diff --git a/spec/open_feature/sdk/hooks/hook_context_spec.rb b/spec/open_feature/sdk/hooks/hook_context_spec.rb index 00641acb..245529ea 100644 --- a/spec/open_feature/sdk/hooks/hook_context_spec.rb +++ b/spec/open_feature/sdk/hooks/hook_context_spec.rb @@ -39,6 +39,11 @@ expect(ctx.default_value).to be_frozen end + it "copies the default_value on first read, even once frozen" do + ctx = described_class.new(flag_key: "flag", flag_value_type: :object, default_value: {"list" => []}, evaluation_context: nil).freeze + expect(ctx.default_value["list"]).to be_frozen + end + it "exposes the default_value" do expect(hook_context.default_value).to eq(false) end @@ -82,11 +87,41 @@ end end - describe "mutable evaluation_context" do + describe "evaluation_context" do it "allows setting evaluation_context" do new_context = OpenFeature::SDK::EvaluationContext.new(targeting_key: "user-456") hook_context.evaluation_context = new_context expect(hook_context.evaluation_context.targeting_key).to eq("user-456") end + + it "keeps the evaluation context's class once frozen" do + subclass = Class.new(OpenFeature::SDK::EvaluationContext) { def tenant = field("tenant") } + ctx = described_class.new(flag_key: "flag", flag_value_type: :boolean, default_value: false, + evaluation_context: subclass.new(tenant: "acme")).freeze + + expect(ctx.evaluation_context.tenant).to eq("acme") + end + + it "is a frozen copy once the hook context is frozen" do + hook_context.freeze + + expect(hook_context.evaluation_context).to eq(evaluation_context) + expect(hook_context.evaluation_context.fields).to be_frozen + expect(evaluation_context).not_to be_frozen + expect { hook_context.evaluation_context = evaluation_context }.to raise_error(FrozenError) + end + end + + describe "values that can't be copied" do + [Mutex.new, Thread::Queue.new, -> {}].each do |uncopyable| + it "freezes a shallow copy when it holds a #{uncopyable.class}" do + default = {held: uncopyable} + ctx = described_class.new(flag_key: "flag", flag_value_type: :object, default_value: default, evaluation_context: nil) + + expect(ctx.default_value).to eq(default) + expect(ctx.default_value).to be_frozen + expect(default).not_to be_frozen + end + end end end diff --git a/spec/open_feature/sdk/hooks/hook_executor_spec.rb b/spec/open_feature/sdk/hooks/hook_executor_spec.rb index a1814a97..0031ce89 100644 --- a/spec/open_feature/sdk/hooks/hook_executor_spec.rb +++ b/spec/open_feature/sdk/hooks/hook_executor_spec.rb @@ -34,7 +34,7 @@ call_log = [] hook = recording_hook("h1", call_log) - executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |_hctx| + executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| call_log << "evaluate" successful_details end @@ -43,7 +43,7 @@ end it "returns the evaluation details from the block" do - result = executor.execute(ordered_hooks: [], hook_context: hook_context, hints: hints) do |_hctx| + result = executor.execute(ordered_hooks: [], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| successful_details end @@ -62,7 +62,7 @@ recording_hook("provider", call_log) ] - executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints) do |_hctx| + executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints, default_value: false) do |_hctx| successful_details end @@ -79,7 +79,7 @@ recording_hook("provider", call_log) ] - executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints) do |_hctx| + executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints, default_value: false) do |_hctx| successful_details end @@ -95,7 +95,7 @@ recording_hook("provider", call_log) ] - executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints) do |_hctx| + executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints, default_value: false) do |_hctx| successful_details end @@ -115,7 +115,7 @@ def before(hook_context:, hints:) end.new captured_context = nil - executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |hctx| + executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints, default_value: false) do |hctx| captured_context = hctx.evaluation_context successful_details end @@ -134,7 +134,7 @@ def before(hook_context:, hints:) end.new captured_context = nil - executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |hctx| + executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints, default_value: false) do |hctx| captured_context = hctx.evaluation_context successful_details end @@ -166,7 +166,7 @@ def before(hook_context:, hints:) hook2 = hook2_class.new(contexts_seen) - executor.execute(ordered_hooks: [hook1, hook2], hook_context: hook_context, hints: hints) do |_hctx| + executor.execute(ordered_hooks: [hook1, hook2], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| successful_details end @@ -195,7 +195,7 @@ def before(hook_context:, hints:) end end.new - result = executor.execute(ordered_hooks: [failing_hook, error_hook], hook_context: hook_context, hints: hints) do |_hctx| + result = executor.execute(ordered_hooks: [failing_hook, error_hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| call_log << "evaluate" successful_details end @@ -225,7 +225,7 @@ def before(hook_context:, hints:) end end.new - result = executor.execute(ordered_hooks: [failing_after_hook], hook_context: hook_context, hints: hints) do |_hctx| + result = executor.execute(ordered_hooks: [failing_after_hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| successful_details end @@ -248,7 +248,7 @@ def before(hook_context:, hints:) ) ) - result = executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |_hctx| + result = executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| call_log << "evaluate" error_details end @@ -262,7 +262,7 @@ def before(hook_context:, hints:) call_log = [] hook = recording_hook("h1", call_log) - result = executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |_hctx| + result = executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| raise "evaluation failed" end @@ -293,7 +293,7 @@ def before(hook_context:, hints:) # Hooks are reversed for error stage, so good_error_hook (first in ordered) # runs last in error stage. failing_error_hook (second) runs first in error stage. - executor.execute(ordered_hooks: [good_error_hook, failing_error_hook], hook_context: hook_context, hints: hints) do |_hctx| + executor.execute(ordered_hooks: [good_error_hook, failing_error_hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| raise "eval failed" end @@ -321,7 +321,7 @@ def before(hook_context:, hints:) end.new # Hooks are reversed for finally stage - executor.execute(ordered_hooks: [good_finally_hook, failing_finally_hook], hook_context: hook_context, hints: hints) do |_hctx| + executor.execute(ordered_hooks: [good_finally_hook, failing_finally_hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| successful_details end @@ -338,7 +338,7 @@ def before(hook_context:, hints:) end end.new - result = executor.execute(ordered_hooks: [before_only_hook], hook_context: hook_context, hints: hints) do |_hctx| + result = executor.execute(ordered_hooks: [before_only_hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| successful_details end @@ -373,7 +373,7 @@ def error_details(error_code:, error_message: nil) end it "carries the resolution's error code and message" do - executor.execute(ordered_hooks: [capturing_hook], hook_context: hook_context, hints: hints) do |_hctx| + executor.execute(ordered_hooks: [capturing_hook], hook_context: hook_context, hints: hints, default_value: false) do |_hctx| error_details(error_code: "FLAG_NOT_FOUND", error_message: "nope") end @@ -392,5 +392,20 @@ def error_details(error_code:, error_message: nil) expect(capturing_hook.exception.error_code).to eq("PROVIDER_NOT_READY") end + + it "freezes the evaluation context before error and finally hooks run" do + frozen_in = [] + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:error) { |hook_context:, **| frozen_in << [:error, hook_context.evaluation_context.fields.frozen?] } + define_method(:finally) { |hook_context:, **| frozen_in << [:finally, hook_context.evaluation_context.fields.frozen?] } + end.new + + executor.run_short_circuit(ordered_hooks: [hook], hook_context: hook_context, hints: hints, + evaluation_details: error_details(error_code: "PROVIDER_NOT_READY")) + + expect(frozen_in).to eq([[:error, true], [:finally, true]]) + end end end diff --git a/spec/specification/hooks_spec.rb b/spec/specification/hooks_spec.rb index beb37432..0512df07 100644 --- a/spec/specification/hooks_spec.rb +++ b/spec/specification/hooks_spec.rb @@ -7,6 +7,20 @@ OpenFeature::SDK::API.instance.send(:configuration).send(:reset) end + let(:captured) { {} } + let(:capture_hook) do + seen = captured + Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + seen[:hook_context] = hook_context + seen[:hints] = hints + nil + end + end.new + end + context "4.1 - Hook Context" do let(:provider) { OpenFeature::SDK::Provider::InMemoryProvider.new({"flag-1" => true}) } @@ -79,6 +93,30 @@ expect(captured_context.flag_key).to be_frozen expect(captured_context.flag_value_type).to be_frozen end + + specify "default value MUST be immutable, without freezing the caller's objects" do + flag_key = +"missing-flag" + default = {"nested" => {"list" => []}} + + OpenFeature::SDK.build_client.fetch_object_value(flag_key:, default_value: default, hooks: [capture_hook]) + + expect(captured[:hook_context].default_value).to eq(default) + expect(captured[:hook_context].default_value["nested"]["list"]).to be_frozen + expect(default["nested"]).not_to be_frozen + expect(default).not_to be_frozen + expect(flag_key).not_to be_frozen + end + + specify "the caller gets their own default back when the provider raises" do + allow(provider).to receive(:fetch_object_value).and_raise("boom") + default = {"rules" => []} + + details = OpenFeature::SDK.build_client.fetch_object_details(flag_key: "flag-1", default_value: default, hooks: [capture_hook]) + + expect(details.error_code).to eq(OpenFeature::SDK::Provider::ErrorCode::GENERAL) + expect(details.value).to equal(default) + expect(default).not_to be_frozen + end end context "Requirement 4.1.5" do @@ -147,28 +185,157 @@ end end - context "Requirement 4.1.4" do - specify "evaluation context MUST be mutable" do - captured_context = nil + context "Conditional Requirement 4.1.4.1" do + let(:user) { OpenFeature::SDK::EvaluationContext.new(targeting_key: "user-1") } + let(:stages_seen) { {} } + + def context_probe(*stages) + seen = stages_seen + Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:attempt) do |&change| + change.call + :mutable + rescue FrozenError + :frozen + end + + stages.each do |stage| + define_method(stage) do |hook_context:, **| + ctx = hook_context.evaluation_context + seen[stage] = [attempt { ctx.fields["probe"] = stage }, attempt { hook_context.evaluation_context = ctx }] + end + end + end.new + end + + specify "the evaluation context can be changed or replaced in before hooks, and not in after and finally hooks" do + OpenFeature::SDK.build_client.fetch_boolean_value(flag_key: "flag-1", default_value: false, evaluation_context: user, + hooks: [context_probe(:before, :after, :finally)]) + expect(stages_seen).to eq(before: [:mutable, :mutable], after: [:frozen, :frozen], finally: [:frozen, :frozen]) + expect(user).not_to be_frozen + expect(user.fields).not_to be_frozen + end + + specify "after hooks can't edit the caller's strings in the evaluation context in place" do + targeting_key = +"user-1" + edited = nil hook = Class.new do include OpenFeature::SDK::Hooks::Hook - define_method(:before) do |hook_context:, hints:| - captured_context = hook_context - nil + define_method(:after) do |hook_context:, **| + hook_context.evaluation_context.targeting_key.replace("other-user") + edited = true + rescue FrozenError + edited = false end end.new - client = OpenFeature::SDK.build_client - client.fetch_boolean_value( - flag_key: "flag-1", - default_value: false, - evaluation_context: OpenFeature::SDK::EvaluationContext.new(targeting_key: "user-1"), - hooks: [hook] - ) + OpenFeature::SDK.build_client.fetch_boolean_value(flag_key: "flag-1", default_value: false, + evaluation_context: OpenFeature::SDK::EvaluationContext.new(targeting_key:), hooks: [hook]) + + expect(edited).to be(false) + expect(targeting_key).to eq("user-1") + expect(targeting_key).not_to be_frozen + end + + specify "the evaluation context is frozen in error and finally hooks when a before hook raises" do + failing_before = Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(**) = raise("boom") + end.new + + OpenFeature::SDK.build_client.fetch_boolean_value(flag_key: "flag-1", default_value: false, evaluation_context: user, + hooks: [context_probe(:error, :finally), failing_before]) + + expect(stages_seen).to eq(error: [:frozen, :frozen], finally: [:frozen, :frozen]) + end + + specify "a provider can still write to the evaluation context while hooks are registered" do + writing_provider = Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do + def fetch_boolean_value(evaluation_context:, **) + evaluation_context.fields["written_by_provider"] = true + super + end + end.new({"flag-1" => true}) + OpenFeature::SDK.set_provider_and_wait(writing_provider) + + details = OpenFeature::SDK.build_client.fetch_boolean_details(flag_key: "flag-1", default_value: false, + evaluation_context: user, hooks: [context_probe(:finally)]) + + expect(details.value).to be(true) + expect(details.error_code).to be_nil + end + + specify "a context a before hook returns isn't frozen afterwards" do + returned = OpenFeature::SDK::EvaluationContext.new(targeting_key: "from-hook") + seen_targeting_key = nil + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) { |**| returned } + define_method(:after) { |hook_context:, **| seen_targeting_key = hook_context.evaluation_context.targeting_key } + end.new + + OpenFeature::SDK.build_client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + + expect(seen_targeting_key).to eq("from-hook") + expect(returned).not_to be_frozen + end + end + end + + context "4.2 - Hook Hints" do + let(:provider) { OpenFeature::SDK::Provider::InMemoryProvider.new({"flag-1" => true}) } + + before { OpenFeature::SDK.set_provider_and_wait(provider) } + + context "Conditional Requirement 4.2.2.1" do + specify "hook hints MUST be immutable, including nested values, without freezing the caller's hash" do + hint_hash = {"nested" => {"list" => []}} + + OpenFeature::SDK.build_client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [capture_hook], hook_hints: hint_hash) + + expect(captured[:hints]["nested"]["list"]).to be_frozen + expect(hint_hash["nested"]["list"]).not_to be_frozen + end + + specify "hints that can't be deep-copied still evaluate the flag" do + details = OpenFeature::SDK.build_client.fetch_boolean_details(flag_key: "flag-1", default_value: false, + hooks: [capture_hook], hook_hints: {"lookup" => Hash.new { |h, k| h[k] = k }}) + + expect(details.value).to be(true) + expect(captured[:hints]).to be_frozen + end + end + + context "Conditional Requirement 4.2.2.2" do + specify "the client metadata in the hook context MUST be immutable" do + OpenFeature::SDK.build_client(domain: "my-domain").fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [capture_hook]) + + expect(captured[:hook_context].client_metadata).to be_frozen + expect(captured[:hook_context].client_metadata.domain).to be_frozen + end + end + + context "Conditional Requirement 4.2.2.3" do + let(:provider) do + Class.new(OpenFeature::SDK::Provider::InMemoryProvider) do + def metadata = @unfrozen_metadata ||= OpenFeature::SDK::Provider::ProviderMetadata.new(name: +"custom") + end.new({"flag-1" => true}) + end + + specify "the provider metadata in the hook context MUST be immutable, and the provider's own is left alone" do + OpenFeature::SDK.build_client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [capture_hook]) - expect { captured_context.evaluation_context = OpenFeature::SDK::EvaluationContext.new }.not_to raise_error + hook_metadata = captured[:hook_context].provider_metadata + expect { hook_metadata.name = "renamed" }.to raise_error(FrozenError) + expect { hook_metadata.name << "!" }.to raise_error(FrozenError) + expect(provider.metadata.name).to eq("custom") + expect(provider.metadata).not_to be_frozen end end end