diff --git a/lib/open_feature/sdk/api.rb b/lib/open_feature/sdk/api.rb index 7b52d61f..f714ee09 100644 --- a/lib/open_feature/sdk/api.rb +++ b/lib/open_feature/sdk/api.rb @@ -8,6 +8,7 @@ require_relative "evaluation_context_builder" require_relative "evaluation_details" require_relative "client_metadata" +require_relative "hooks" require_relative "client" require_relative "provider" diff --git a/lib/open_feature/sdk/client.rb b/lib/open_feature/sdk/client.rb index 428e3fd4..4e76b244 100644 --- a/lib/open_feature/sdk/client.rb +++ b/lib/open_feature/sdk/client.rb @@ -15,6 +15,7 @@ class Client }.freeze RESULT_TYPE = TYPE_CLASS_MAP.keys.freeze SUFFIXES = %i[value details].freeze + EMPTY_HINTS = Hooks::Hints.new.freeze attr_reader :metadata, :evaluation_context @@ -40,11 +41,8 @@ def remove_handler(event_type, handler = nil, &block) RESULT_TYPE.each do |result_type| SUFFIXES.each do |suffix| class_eval <<-RUBY, __FILE__, __LINE__ + 1 - # def fetch_boolean_details(flag_key:, default_value:, evaluation_context: nil) - # result = @provider.fetch_boolean_value(flag_key: flag_key, default_value: default_value, evaluation_context: evaluation_context) - # end - def fetch_#{result_type}_#{suffix}(flag_key:, default_value:, evaluation_context: nil) - evaluation_details = fetch_details(type: :#{result_type}, flag_key:, default_value:, evaluation_context:) + def fetch_#{result_type}_#{suffix}(flag_key:, default_value:, evaluation_context: nil, hooks: [], hook_hints: nil) + evaluation_details = fetch_details(type: :#{result_type}, flag_key:, default_value:, evaluation_context:, invocation_hooks: hooks, hook_hints: hook_hints) #{"evaluation_details.value" if suffix == :value} end RUBY @@ -53,12 +51,54 @@ def fetch_#{result_type}_#{suffix}(flag_key:, default_value:, evaluation_context private - def fetch_details(type:, flag_key:, default_value:, evaluation_context: nil) + def fetch_details(type:, flag_key:, default_value:, evaluation_context: nil, invocation_hooks: [], hook_hints: nil) validate_default_value_type(type, default_value) - built_context = EvaluationContextBuilder.new.call(api_context: OpenFeature::SDK.evaluation_context, client_context: self.evaluation_context, invocation_context: evaluation_context) + built_context = EvaluationContextBuilder.new.call( + api_context: OpenFeature::SDK.evaluation_context, + client_context: self.evaluation_context, + invocation_context: evaluation_context + ) - resolution_details = @provider.send(:"fetch_#{type}_value", flag_key:, default_value:, evaluation_context: built_context) + # Assemble ordered hooks: API → Client → Invocation → Provider (spec 4.4.2) + provider_hooks = @provider.respond_to?(:hooks) ? Array(@provider.hooks) : [] + ordered_hooks = [*OpenFeature::SDK.hooks, *@hooks, *invocation_hooks, *provider_hooks] + + # Fast path: skip hook ceremony when no hooks are registered + if ordered_hooks.empty? + return evaluate_flag(type: type, flag_key: flag_key, default_value: default_value, evaluation_context: built_context) + end + + hook_context = Hooks::HookContext.new( + flag_key: flag_key, + flag_value_type: type, + default_value: default_value, + evaluation_context: built_context, + client_metadata: @metadata, + provider_metadata: @provider.respond_to?(:metadata) ? @provider.metadata : nil + ) + + hints = if hook_hints.is_a?(Hooks::Hints) + hook_hints + elsif hook_hints + Hooks::Hints.new(hook_hints) + else + EMPTY_HINTS + end + + executor = Hooks::HookExecutor.new(logger: OpenFeature::SDK.configuration.logger) + executor.execute(ordered_hooks: ordered_hooks, hook_context: hook_context, hints: hints) do |hctx| + evaluate_flag(type: type, flag_key: flag_key, default_value: default_value, evaluation_context: hctx.evaluation_context) + end + end + + def evaluate_flag(type:, flag_key:, default_value:, evaluation_context:) + resolution_details = @provider.send( + :"fetch_#{type}_value", + flag_key: flag_key, + default_value: default_value, + evaluation_context: evaluation_context + ) if TYPE_CLASS_MAP[type].none? { |klass| resolution_details.value.is_a?(klass) } resolution_details.value = default_value @@ -66,7 +106,7 @@ def fetch_details(type:, flag_key:, default_value:, evaluation_context: nil) resolution_details.reason = Provider::Reason::ERROR end - EvaluationDetails.new(flag_key:, resolution_details:) + EvaluationDetails.new(flag_key: flag_key, resolution_details: resolution_details) end def validate_default_value_type(type, default_value) diff --git a/lib/open_feature/sdk/hooks.rb b/lib/open_feature/sdk/hooks.rb new file mode 100644 index 00000000..77af6916 --- /dev/null +++ b/lib/open_feature/sdk/hooks.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +require_relative "hooks/hints" +require_relative "hooks/hook" +require_relative "hooks/hook_context" +require_relative "hooks/hook_executor" diff --git a/lib/open_feature/sdk/hooks/hints.rb b/lib/open_feature/sdk/hooks/hints.rb index c9a0f730..57eb7480 100644 --- a/lib/open_feature/sdk/hooks/hints.rb +++ b/lib/open_feature/sdk/hooks/hints.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "delegate" + module OpenFeature module SDK module Hooks diff --git a/lib/open_feature/sdk/hooks/hook.rb b/lib/open_feature/sdk/hooks/hook.rb new file mode 100644 index 00000000..99638171 --- /dev/null +++ b/lib/open_feature/sdk/hooks/hook.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module OpenFeature + module SDK + module Hooks + # Module that hooks include. Provides default no-op implementations + # for all four lifecycle stages. A hook overrides the stages it cares about. + # + # Spec 4.3.1: Hooks MUST specify at least one stage. + module Hook + # Called before flag evaluation. May return an EvaluationContext + # that gets merged into the existing context (spec 4.3.2.1, 4.3.4, 4.3.5). + def before(hook_context:, hints:) + nil + end + + # Called after successful flag evaluation (spec 4.3.3). + def after(hook_context:, evaluation_details:, hints:) + nil + end + + # Called when an error occurs during flag evaluation (spec 4.3.6). + def error(hook_context:, exception:, hints:) + nil + end + + # Called unconditionally after flag evaluation (spec 4.3.7). + def finally(hook_context:, evaluation_details:, hints:) + nil + end + end + end + end +end diff --git a/lib/open_feature/sdk/hooks/hook_context.rb b/lib/open_feature/sdk/hooks/hook_context.rb new file mode 100644 index 00000000..b8b2c290 --- /dev/null +++ b/lib/open_feature/sdk/hooks/hook_context.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module OpenFeature + module SDK + module Hooks + # Provides context to hook stages during flag evaluation. + # + # Per spec 4.1.1-4.1.5: + # - 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) + class HookContext + attr_reader :flag_key, :flag_value_type, :default_value, + :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_value_type = flag_value_type.freeze + @default_value = default_value.freeze + @evaluation_context = evaluation_context + @client_metadata = client_metadata + @provider_metadata = provider_metadata + end + end + end + end +end diff --git a/lib/open_feature/sdk/hooks/hook_executor.rb b/lib/open_feature/sdk/hooks/hook_executor.rb new file mode 100644 index 00000000..c0f1a47f --- /dev/null +++ b/lib/open_feature/sdk/hooks/hook_executor.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +module OpenFeature + module SDK + module Hooks + # Orchestrates the full hook lifecycle for flag evaluation. + # + # Hook execution order (spec 4.4.2): + # Before: API → Client → Invocation → Provider + # After/Error/Finally: Provider → Invocation → Client → API (reverse) + # + # Error handling (spec 4.4.3-4.4.7): + # - 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 + class HookExecutor + def initialize(logger: nil) + @logger = logger + end + + # Executes the full hook lifecycle around the flag evaluation block. + # + # @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 evaluate_block [Proc] the flag evaluation to wrap + # @return [EvaluationDetails] the evaluation result + def execute(ordered_hooks:, hook_context:, hints:, &evaluate_block) + evaluation_details = nil + + begin + run_before_hooks(ordered_hooks, hook_context, hints) + evaluation_details = evaluate_block.call(hook_context) + run_after_hooks(ordered_hooks, hook_context, evaluation_details, hints) + rescue => e + run_error_hooks(ordered_hooks, hook_context, e, hints) + + evaluation_details = EvaluationDetails.new( + flag_key: hook_context.flag_key, + resolution_details: Provider::ResolutionDetails.new( + value: hook_context.default_value, + error_code: Provider::ErrorCode::GENERAL, + reason: Provider::Reason::ERROR, + error_message: e.message + ) + ) + ensure + run_finally_hooks(ordered_hooks, hook_context, evaluation_details, hints) + end + + evaluation_details + end + + private + + # Spec 4.4.2: Before hooks run in order: API → Client → Invocation → Provider + # Spec 4.3.4/4.3.5: If a before hook returns an EvaluationContext, it is merged + # into the existing context for subsequent hooks and evaluation. + def run_before_hooks(hooks, hook_context, hints) + hooks.each do |hook| + next unless hook.respond_to?(:before) + result = hook.before(hook_context: hook_context, hints: hints) + if result.is_a?(EvaluationContext) + existing = hook_context.evaluation_context + hook_context.evaluation_context = existing ? existing.merge(result) : result + end + end + end + + # Spec 4.4.2: After hooks run in reverse order: Provider → Invocation → Client → API + def run_after_hooks(hooks, hook_context, evaluation_details, hints) + hooks.reverse_each do |hook| + next unless hook.respond_to?(:after) + hook.after(hook_context: hook_context, evaluation_details: evaluation_details, hints: hints) + end + end + + # 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) + hooks.reverse_each do |hook| + next unless hook.respond_to?(:error) + hook.error(hook_context: hook_context, exception: exception, hints: hints) + rescue => e + @logger&.error("Error hook #{hook.class.name} failed: #{e.message}") + end + end + + # 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) + hooks.reverse_each do |hook| + next unless hook.respond_to?(:finally) + hook.finally(hook_context: hook_context, evaluation_details: evaluation_details, hints: hints) + rescue => e + @logger&.error("Finally hook #{hook.class.name} failed: #{e.message}") + end + end + end + end + end +end diff --git a/spec/open_feature/sdk/client_spec.rb b/spec/open_feature/sdk/client_spec.rb index 74c4722d..a8e5da64 100644 --- a/spec/open_feature/sdk/client_spec.rb +++ b/spec/open_feature/sdk/client_spec.rb @@ -431,5 +431,49 @@ end end end + + context "Hook hints" do + let(:capturing_hook) do + captured = nil + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + captured = hints + nil + end + end.new + [hook, -> { captured }] + end + + it "accepts a Hash as hook_hints and converts it to Hints" do + hook, get_hints = capturing_hook + + client.fetch_boolean_value( + flag_key: "test-flag", + default_value: false, + hooks: [hook], + hook_hints: {key: "value"} + ) + + expect(get_hints.call).to be_a(OpenFeature::SDK::Hooks::Hints) + expect(get_hints.call[:key]).to eq("value") + end + + it "passes through a Hints instance directly" do + hook, get_hints = capturing_hook + hints = OpenFeature::SDK::Hooks::Hints.new(source: "direct") + + client.fetch_boolean_value( + flag_key: "test-flag", + default_value: false, + hooks: [hook], + hook_hints: hints + ) + + expect(get_hints.call).to be(hints) + expect(get_hints.call[:source]).to eq("direct") + end + 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 new file mode 100644 index 00000000..00641acb --- /dev/null +++ b/spec/open_feature/sdk/hooks/hook_context_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe OpenFeature::SDK::Hooks::HookContext do + let(:evaluation_context) { OpenFeature::SDK::EvaluationContext.new(targeting_key: "user-123") } + let(:client_metadata) { OpenFeature::SDK::ClientMetadata.new(domain: "test-domain") } + let(:provider_metadata) { OpenFeature::SDK::Provider::ProviderMetadata.new(name: "test-provider") } + + subject(:hook_context) do + described_class.new( + flag_key: "my-flag", + flag_value_type: :boolean, + default_value: false, + evaluation_context: evaluation_context, + client_metadata: client_metadata, + provider_metadata: provider_metadata + ) + end + + describe "immutable fields" do + it "has a frozen flag_key" do + expect(hook_context.flag_key).to eq("my-flag") + expect(hook_context.flag_key).to be_frozen + end + + it "has a frozen flag_value_type" do + expect(hook_context.flag_value_type).to eq(:boolean) + expect(hook_context.flag_value_type).to be_frozen + end + + it "has a frozen default_value" do + ctx = described_class.new( + flag_key: "flag", + flag_value_type: :object, + default_value: {key: "value"}, + evaluation_context: nil + ) + expect(ctx.default_value).to be_frozen + end + + it "exposes the default_value" do + expect(hook_context.default_value).to eq(false) + end + + it "does not allow setting flag_key" do + expect { hook_context.flag_key = "other" }.to raise_error(NoMethodError) + end + + it "does not allow setting flag_value_type" do + expect { hook_context.flag_value_type = :string }.to raise_error(NoMethodError) + end + end + + describe "optional fields" do + it "exposes client_metadata" do + expect(hook_context.client_metadata.domain).to eq("test-domain") + end + + it "exposes provider_metadata" do + expect(hook_context.provider_metadata.name).to eq("test-provider") + end + + it "allows nil client_metadata" do + ctx = described_class.new( + flag_key: "flag", + flag_value_type: :string, + default_value: "", + evaluation_context: nil + ) + expect(ctx.client_metadata).to be_nil + end + + it "allows nil provider_metadata" do + ctx = described_class.new( + flag_key: "flag", + flag_value_type: :string, + default_value: "", + evaluation_context: nil + ) + expect(ctx.provider_metadata).to be_nil + end + end + + describe "mutable 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 + end +end diff --git a/spec/open_feature/sdk/hooks/hook_executor_spec.rb b/spec/open_feature/sdk/hooks/hook_executor_spec.rb new file mode 100644 index 00000000..28109b76 --- /dev/null +++ b/spec/open_feature/sdk/hooks/hook_executor_spec.rb @@ -0,0 +1,324 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe OpenFeature::SDK::Hooks::HookExecutor do + subject(:executor) { described_class.new(logger: logger) } + + let(:logger) { instance_double("Logger", error: nil, warn: nil) } + + let(:evaluation_context) { OpenFeature::SDK::EvaluationContext.new(targeting_key: "user-1") } + let(:hints) { OpenFeature::SDK::Hooks::Hints.new } + let(:hook_context) do + OpenFeature::SDK::Hooks::HookContext.new( + flag_key: "test-flag", + flag_value_type: :boolean, + default_value: false, + evaluation_context: evaluation_context + ) + end + + let(:successful_details) do + OpenFeature::SDK::EvaluationDetails.new( + flag_key: "test-flag", + resolution_details: OpenFeature::SDK::Provider::ResolutionDetails.new( + value: true, + reason: OpenFeature::SDK::Provider::Reason::STATIC + ) + ) + end + + describe "#execute" do + context "successful evaluation" do + it "calls before, evaluate, after, finally in order" do + call_log = [] + hook = recording_hook("h1", call_log) + + executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |_hctx| + call_log << "evaluate" + successful_details + end + + expect(call_log).to eq(["h1:before", "evaluate", "h1:after", "h1:finally"]) + end + + it "returns the evaluation details from the block" do + result = executor.execute(ordered_hooks: [], hook_context: hook_context, hints: hints) do |_hctx| + successful_details + end + + expect(result.value).to eq(true) + expect(result.flag_key).to eq("test-flag") + end + end + + context "hook execution order" do + it "runs before hooks in insertion order (API → Client → Invocation → Provider)" do + call_log = [] + hooks = [ + recording_hook("api", call_log), + recording_hook("client", call_log), + recording_hook("invocation", call_log), + recording_hook("provider", call_log) + ] + + executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints) do |_hctx| + successful_details + end + + before_calls = call_log.select { |c| c.end_with?(":before") } + expect(before_calls).to eq(["api:before", "client:before", "invocation:before", "provider:before"]) + end + + it "runs after hooks in reverse order (Provider → Invocation → Client → API)" do + call_log = [] + hooks = [ + recording_hook("api", call_log), + recording_hook("client", call_log), + recording_hook("invocation", call_log), + recording_hook("provider", call_log) + ] + + executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints) do |_hctx| + successful_details + end + + after_calls = call_log.select { |c| c.end_with?(":after") } + expect(after_calls).to eq(["provider:after", "invocation:after", "client:after", "api:after"]) + end + + it "runs finally hooks in reverse order" do + call_log = [] + hooks = [ + recording_hook("api", call_log), + recording_hook("client", call_log), + recording_hook("provider", call_log) + ] + + executor.execute(ordered_hooks: hooks, hook_context: hook_context, hints: hints) do |_hctx| + successful_details + end + + finally_calls = call_log.select { |c| c.end_with?(":finally") } + expect(finally_calls).to eq(["provider:finally", "client:finally", "api:finally"]) + end + end + + context "before hook context merging" do + it "merges EvaluationContext returned by before hook" do + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(hook_context:, hints:) + OpenFeature::SDK::EvaluationContext.new(extra_key: "extra_value") + end + end.new + + captured_context = nil + executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |hctx| + captured_context = hctx.evaluation_context + successful_details + end + + expect(captured_context.field("extra_key")).to eq("extra_value") + expect(captured_context.targeting_key).to eq("user-1") + end + + it "does not merge non-EvaluationContext returns" do + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(hook_context:, hints:) + "not a context" + end + end.new + + captured_context = nil + executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |hctx| + captured_context = hctx.evaluation_context + successful_details + end + + expect(captured_context).to eq(evaluation_context) + end + + it "passes merged context to subsequent before hooks" do + contexts_seen = [] + + hook1 = Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(hook_context:, hints:) + OpenFeature::SDK::EvaluationContext.new(from_hook1: "yes") + end + end.new + + hook2_class = Class.new do + include OpenFeature::SDK::Hooks::Hook + define_method(:initialize) { |log| @log = log } + + define_method(:before) do |hook_context:, hints:| + @log << hook_context.evaluation_context.field("from_hook1") + nil + end + end + + hook2 = hook2_class.new(contexts_seen) + + executor.execute(ordered_hooks: [hook1, hook2], hook_context: hook_context, hints: hints) do |_hctx| + successful_details + end + + expect(contexts_seen).to eq(["yes"]) + end + end + + context "error handling" do + it "runs error hooks when before hook raises" do + call_log = [] + error_hook = recording_hook("h1", call_log) + + failing_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + raise "before failed" + end + + define_method(:error) do |hook_context:, exception:, hints:| + call_log << "failing:error" + end + + define_method(:finally) do |hook_context:, evaluation_details:, hints:| + call_log << "failing:finally" + end + end.new + + result = executor.execute(ordered_hooks: [failing_hook, error_hook], hook_context: hook_context, hints: hints) do |_hctx| + call_log << "evaluate" + successful_details + end + + expect(call_log).not_to include("evaluate") + expect(call_log).to include("failing:error") + expect(result.value).to eq(false) # default value + expect(result.error_code).to eq(OpenFeature::SDK::Provider::ErrorCode::GENERAL) + end + + it "runs error hooks when after hook raises" do + call_log = [] + + failing_after_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:after) do |hook_context:, evaluation_details:, hints:| + raise "after failed" + end + + define_method(:error) do |hook_context:, exception:, hints:| + call_log << "error_ran" + end + + define_method(:finally) do |hook_context:, evaluation_details:, hints:| + call_log << "finally_ran" + end + end.new + + result = executor.execute(ordered_hooks: [failing_after_hook], hook_context: hook_context, hints: hints) do |_hctx| + successful_details + end + + expect(call_log).to include("error_ran") + expect(call_log).to include("finally_ran") + expect(result.value).to eq(false) # default value + end + + it "runs error hooks when evaluation block raises" do + call_log = [] + hook = recording_hook("h1", call_log) + + result = executor.execute(ordered_hooks: [hook], hook_context: hook_context, hints: hints) do |_hctx| + raise "evaluation failed" + end + + expect(call_log).to include("h1:error") + expect(call_log).to include("h1:finally") + expect(result.value).to eq(false) # default value + expect(result.error_message).to eq("evaluation failed") + end + + it "continues remaining error hooks when one error hook fails" do + call_log = [] + + failing_error_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:error) do |hook_context:, exception:, hints:| + raise "error hook failed" + end + end.new + + good_error_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:error) do |hook_context:, exception:, hints:| + call_log << "good_error_ran" + end + end.new + + # 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| + raise "eval failed" + end + + expect(call_log).to include("good_error_ran") + expect(logger).to have_received(:error).with(/error hook failed/) + end + + it "continues remaining finally hooks when one finally hook fails" do + call_log = [] + + failing_finally_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:finally) do |hook_context:, evaluation_details:, hints:| + raise "finally hook failed" + end + end.new + + good_finally_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:finally) do |hook_context:, evaluation_details:, hints:| + call_log << "good_finally_ran" + end + 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| + successful_details + end + + expect(call_log).to include("good_finally_ran") + expect(logger).to have_received(:error).with(/finally hook failed/) + end + end + + context "hooks that only implement some stages" do + it "skips hooks that do not respond to a stage" do + before_only_hook = Class.new do + def before(hook_context:, hints:) + nil + end + end.new + + result = executor.execute(ordered_hooks: [before_only_hook], hook_context: hook_context, hints: hints) do |_hctx| + successful_details + end + + expect(result.value).to eq(true) + end + end + end +end diff --git a/spec/open_feature/sdk/hooks/hook_spec.rb b/spec/open_feature/sdk/hooks/hook_spec.rb new file mode 100644 index 00000000..69655937 --- /dev/null +++ b/spec/open_feature/sdk/hooks/hook_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe OpenFeature::SDK::Hooks::Hook do + let(:hook_class) do + Class.new do + include OpenFeature::SDK::Hooks::Hook + end + end + + subject(:hook) { hook_class.new } + + describe "default implementations" do + it "before returns nil by default" do + result = hook.before(hook_context: double, hints: double) + expect(result).to be_nil + end + + it "after returns nil by default" do + result = hook.after(hook_context: double, evaluation_details: double, hints: double) + expect(result).to be_nil + end + + it "error returns nil by default" do + result = hook.error(hook_context: double, exception: double, hints: double) + expect(result).to be_nil + end + + it "finally returns nil by default" do + result = hook.finally(hook_context: double, evaluation_details: double, hints: double) + expect(result).to be_nil + end + end + + describe "overriding stages" do + let(:custom_hook_class) do + Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(hook_context:, hints:) + OpenFeature::SDK::EvaluationContext.new(custom_key: "custom_value") + end + end + end + + it "allows overriding specific stages" do + custom_hook = custom_hook_class.new + result = custom_hook.before(hook_context: double, hints: double) + expect(result).to be_a(OpenFeature::SDK::EvaluationContext) + expect(result.field("custom_key")).to eq("custom_value") + end + + it "keeps default implementations for non-overridden stages" do + custom_hook = custom_hook_class.new + expect(custom_hook.after(hook_context: double, evaluation_details: double, hints: double)).to be_nil + expect(custom_hook.error(hook_context: double, exception: double, hints: double)).to be_nil + expect(custom_hook.finally(hook_context: double, evaluation_details: double, hints: double)).to be_nil + end + end +end diff --git a/spec/specification/flag_evaluation_api_spec.rb b/spec/specification/flag_evaluation_api_spec.rb index 07b3db19..7b86ca48 100644 --- a/spec/specification/flag_evaluation_api_spec.rb +++ b/spec/specification/flag_evaluation_api_spec.rb @@ -101,9 +101,31 @@ end context "Requirement 1.1.4" do - pending "The API must provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks." + before(:each) do + OpenFeature::SDK.hooks.clear + end + + specify "The API must provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks." do + hook1 = Class.new { include OpenFeature::SDK::Hooks::Hook }.new + hook2 = Class.new { include OpenFeature::SDK::Hooks::Hook }.new + + OpenFeature::SDK.hooks << hook1 + OpenFeature::SDK.hooks << hook2 + + expect(OpenFeature::SDK.hooks).to eq([hook1, hook2]) + end + + specify "When new hooks are added, previously added hooks are not removed." do + hook1 = Class.new { include OpenFeature::SDK::Hooks::Hook }.new + OpenFeature::SDK.hooks << hook1 - pending "When new hooks are added, previously added hooks are not removed." + hook2 = Class.new { include OpenFeature::SDK::Hooks::Hook }.new + OpenFeature::SDK.hooks << hook2 + + expect(OpenFeature::SDK.hooks).to include(hook1) + expect(OpenFeature::SDK.hooks).to include(hook2) + expect(OpenFeature::SDK.hooks.size).to eq(2) + end end context "Requirement 1.1.5" do @@ -165,7 +187,20 @@ context "1.2 - Client Usage" do context "Requirement 1.2.1" do - pending "The client MUST provide a method to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed." + specify "The client MUST provide a method to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed." do + provider = OpenFeature::SDK::Provider::NoOpProvider.new + OpenFeature::SDK.set_provider(provider) + client = OpenFeature::SDK.build_client + + hook1 = Class.new { include OpenFeature::SDK::Hooks::Hook }.new + hook2 = Class.new { include OpenFeature::SDK::Hooks::Hook }.new + + client.hooks << hook1 + client.hooks << hook2 + + expect(client.hooks).to eq([hook1, hook2]) + expect(client.hooks).to include(hook1) + end end context "Requirement 1.2.2" do diff --git a/spec/specification/hooks_spec.rb b/spec/specification/hooks_spec.rb new file mode 100644 index 00000000..03621a41 --- /dev/null +++ b/spec/specification/hooks_spec.rb @@ -0,0 +1,455 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "Hooks Specification" do + before(:each) do + OpenFeature::SDK::API.instance.send(:configuration).send(:reset) + end + + context "4.1 - Hook Context" do + let(:provider) { OpenFeature::SDK::Provider::InMemoryProvider.new({"flag-1" => true}) } + + before do + OpenFeature::SDK.set_provider_and_wait(provider) + end + + context "Requirement 4.1.1" do + specify "Hook context MUST provide: flag key, flag value type, default value, evaluation context" do + captured_context = nil + + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + captured_context = hook_context + nil + end + end.new + + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + + expect(captured_context.flag_key).to eq("flag-1") + expect(captured_context.flag_value_type).to eq(:boolean) + expect(captured_context.default_value).to eq(false) + expect(captured_context.evaluation_context).to be_nil.or be_a(OpenFeature::SDK::EvaluationContext) + end + end + + context "Requirement 4.1.2" do + specify "Hook context SHOULD provide: client metadata, provider metadata" do + captured_context = nil + + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + captured_context = hook_context + nil + end + end.new + + client = OpenFeature::SDK.build_client(domain: "test-domain") + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + + expect(captured_context.client_metadata).to be_a(OpenFeature::SDK::ClientMetadata) + expect(captured_context.client_metadata.domain).to eq("test-domain") + expect(captured_context.provider_metadata).to be_a(OpenFeature::SDK::Provider::ProviderMetadata) + expect(captured_context.provider_metadata.name).to eq("In-memory Provider") + end + end + + context "Requirement 4.1.3" do + specify "flag key and flag value type MUST be immutable" do + captured_context = nil + + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + captured_context = hook_context + nil + end + end.new + + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + + expect(captured_context.flag_key).to be_frozen + expect(captured_context.flag_value_type).to be_frozen + end + end + + context "Requirement 4.1.4" do + specify "evaluation context MUST be mutable" do + captured_context = nil + + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + captured_context = hook_context + nil + 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] + ) + + expect { captured_context.evaluation_context = OpenFeature::SDK::EvaluationContext.new }.not_to raise_error + end + end + end + + context "4.3 - Hook Stages" do + let(:provider) { OpenFeature::SDK::Provider::InMemoryProvider.new({"flag-1" => true}) } + + before do + OpenFeature::SDK.set_provider_and_wait(provider) + end + + context "Requirement 4.3.1" do + specify "Hooks MUST specify at least one stage" do + hook_module = OpenFeature::SDK::Hooks::Hook + + expect(hook_module.instance_methods).to include(:before, :after, :error, :finally) + end + end + + context "Requirement 4.3.2" do + specify "The before stage MUST run before flag evaluation" do + call_log = [] + + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + call_log << "before" + nil + end + end.new + + original_fetch = provider.method(:fetch_boolean_value) + allow(provider).to receive(:fetch_boolean_value) do |**args| + call_log << "evaluate" + original_fetch.call(**args) + end + + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + + expect(call_log).to eq(["before", "evaluate"]) + end + end + + context "Requirement 4.3.3" do + specify "The after stage MUST run after flag evaluation succeeds" do + after_ran = false + + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:after) do |hook_context:, evaluation_details:, hints:| + after_ran = true + end + end.new + + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + + expect(after_ran).to be true + end + end + + context "Requirement 4.3.4" do + specify "If a before hook returns an evaluation context, it MUST be merged with the existing context" do + captured_eval_context = nil + + context_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(hook_context:, hints:) + OpenFeature::SDK::EvaluationContext.new(hook_key: "hook_value") + end + end.new + + original_fetch = provider.method(:fetch_boolean_value) + allow(provider).to receive(:fetch_boolean_value) do |**args| + captured_eval_context = args[:evaluation_context] + original_fetch.call(**args) + end + + 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: [context_hook] + ) + + expect(captured_eval_context.field("hook_key")).to eq("hook_value") + expect(captured_eval_context.targeting_key).to eq("user-1") + end + end + + context "Requirement 4.3.6" do + specify "The error stage MUST run when errors are raised" do + error_ran = false + captured_exception = nil + + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:error) do |hook_context:, exception:, hints:| + error_ran = true + captured_exception = exception + end + end.new + + allow(provider).to receive(:fetch_boolean_value).and_raise("provider error") + + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + + expect(error_ran).to be true + expect(captured_exception.message).to eq("provider error") + end + end + + context "Requirement 4.3.7" do + specify "The finally stage MUST run unconditionally" do + finally_count = 0 + + hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:finally) do |hook_context:, evaluation_details:, hints:| + finally_count += 1 + end + end.new + + client = OpenFeature::SDK.build_client + + # Success case + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + expect(finally_count).to eq(1) + + # Error case + allow(provider).to receive(:fetch_boolean_value).and_raise("error") + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [hook]) + expect(finally_count).to eq(2) + end + end + end + + context "4.4 - Hook Execution" do + let(:provider) { OpenFeature::SDK::Provider::InMemoryProvider.new({"flag-1" => true}) } + + before do + OpenFeature::SDK.set_provider_and_wait(provider) + end + + context "Requirement 4.4.2" do + specify "Before hooks run in order: API → Client → Invocation → Provider" do + call_log = [] + api_hook = recording_hook("api", call_log) + client_hook = recording_hook("client", call_log) + invocation_hook = recording_hook("invocation", call_log) + provider_hook = recording_hook("provider", call_log) + + OpenFeature::SDK.hooks << api_hook + allow(provider).to receive(:hooks).and_return([provider_hook]) + + client = OpenFeature::SDK.build_client + client.hooks << client_hook + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [invocation_hook]) + + before_calls = call_log.select { |c| c.end_with?(":before") } + expect(before_calls).to eq(["api:before", "client:before", "invocation:before", "provider:before"]) + end + + specify "After, Error, and Finally hooks run in reverse order: Provider → Invocation → Client → API" do + call_log = [] + api_hook = recording_hook("api", call_log) + client_hook = recording_hook("client", call_log) + invocation_hook = recording_hook("invocation", call_log) + provider_hook = recording_hook("provider", call_log) + + OpenFeature::SDK.hooks << api_hook + allow(provider).to receive(:hooks).and_return([provider_hook]) + + client = OpenFeature::SDK.build_client + client.hooks << client_hook + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [invocation_hook]) + + after_calls = call_log.select { |c| c.end_with?(":after") } + expect(after_calls).to eq(["provider:after", "invocation:after", "client:after", "api:after"]) + + finally_calls = call_log.select { |c| c.end_with?(":finally") } + expect(finally_calls).to eq(["provider:finally", "invocation:finally", "client:finally", "api:finally"]) + end + end + + context "Requirement 4.4.3" do + specify "If a finally hook abnormally terminates, remaining finally hooks MUST still run" do + call_log = [] + + failing_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:finally) do |hook_context:, evaluation_details:, hints:| + raise "finally failed" + end + end.new + + surviving_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:finally) do |hook_context:, evaluation_details:, hints:| + call_log << "surviving_finally" + end + end.new + + # surviving_hook is first in order, so reversed it comes last → runs after failing_hook + # failing_hook is second in order, so reversed it comes first + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [surviving_hook, failing_hook]) + + expect(call_log).to include("surviving_finally") + end + end + + context "Requirement 4.4.4" do + specify "If an error hook abnormally terminates, remaining error hooks MUST still run" do + call_log = [] + + failing_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:error) do |hook_context:, exception:, hints:| + raise "error hook failed" + end + end.new + + surviving_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:error) do |hook_context:, exception:, hints:| + call_log << "surviving_error" + end + end.new + + allow(provider).to receive(:fetch_boolean_value).and_raise("provider error") + + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [surviving_hook, failing_hook]) + + expect(call_log).to include("surviving_error") + end + end + + context "Requirement 4.4.5" do + specify "If a before/after hook raises, error hooks MUST be invoked" do + error_ran = false + + failing_before = Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(hook_context:, hints:) + raise "before failed" + end + end.new + + error_catcher = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:error) do |hook_context:, exception:, hints:| + error_ran = true + end + end.new + + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [error_catcher, failing_before]) + + expect(error_ran).to be true + end + end + + context "Requirement 4.4.6" do + specify "If a before hook abnormally terminates, remaining before hooks MUST NOT be invoked" do + second_before_ran = false + + failing_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(hook_context:, hints:) + raise "before failed" + end + end.new + + second_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + define_method(:before) do |hook_context:, hints:| + second_before_ran = true + nil + end + end.new + + # failing_hook is first in order, so second_hook.before should not run + client = OpenFeature::SDK.build_client + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [failing_hook, second_hook]) + + expect(second_before_ran).to be false + end + end + + context "Requirement 4.4.7" do + specify "When a hook abnormally terminates, the default value MUST be returned" do + failing_hook = Class.new do + include OpenFeature::SDK::Hooks::Hook + + def before(hook_context:, hints:) + raise "before failed" + end + end.new + + client = OpenFeature::SDK.build_client + result = client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [failing_hook]) + + expect(result).to eq(false) + end + end + end + + context "4.5 - Hook Registration" do + before(:each) do + OpenFeature::SDK::API.instance.send(:configuration).send(:reset) + end + + context "Requirement 4.5.1" do + specify "API, Client, and invocation hooks MUST be registered" do + provider = OpenFeature::SDK::Provider::InMemoryProvider.new({"flag-1" => true}) + OpenFeature::SDK.set_provider_and_wait(provider) + + call_log = [] + api_hook = recording_hook("api", call_log) + client_hook = recording_hook("client", call_log) + invocation_hook = recording_hook("invocation", call_log) + + OpenFeature::SDK.hooks << api_hook + client = OpenFeature::SDK.build_client + client.hooks << client_hook + client.fetch_boolean_value(flag_key: "flag-1", default_value: false, hooks: [invocation_hook]) + + expect(call_log).to include("api:before", "client:before", "invocation:before") + end + end + end +end diff --git a/spec/support/recording_hook_helper.rb b/spec/support/recording_hook_helper.rb new file mode 100644 index 00000000..71404fce --- /dev/null +++ b/spec/support/recording_hook_helper.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module RecordingHookHelper + def recording_hook(name, call_log) + Class.new do + include OpenFeature::SDK::Hooks::Hook + define_method(:hook_name) { name } + + define_method(:before) do |hook_context:, hints:| + call_log << "#{name}:before" + nil + end + + define_method(:after) do |hook_context:, evaluation_details:, hints:| + call_log << "#{name}:after" + end + + define_method(:error) do |hook_context:, exception:, hints:| + call_log << "#{name}:error" + end + + define_method(:finally) do |hook_context:, evaluation_details:, hints:| + call_log << "#{name}:finally" + end + end.new + end +end + +RSpec.configure do |config| + config.include RecordingHookHelper +end