diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 846ee44a..c23038f1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,11 +1,11 @@ name: Run CI on: push: - branches: [ main ] + branches: [ main, 'feat/**' ] paths-ignore: - '**.md' # Do not need to run CI for markdown changes. pull_request: - branches: [ main ] + branches: [ main, 'feat/**' ] paths-ignore: - '**.md' diff --git a/contract-tests/client_entity.rb b/contract-tests/client_entity.rb index 871c6b2b..faab8c9d 100644 --- a/contract-tests/client_entity.rb +++ b/contract-tests/client_entity.rb @@ -3,6 +3,7 @@ require 'net/http' require 'launchdarkly-server-sdk' require './big_segment_store_fixture' +require './hook' require 'http' class ClientEntity @@ -62,6 +63,12 @@ def initialize(log, config) } end + if config[:hooks] + opts[:hooks] = config[:hooks][:hooks].map do |hook| + Hook.new(hook[:name], hook[:callbackUri], hook[:data] || {}, hook[:errors] || {}) + end + end + startWaitTimeMs = config[:startWaitTimeMs] || 5_000 @client = LaunchDarkly::LDClient.new( diff --git a/contract-tests/hook.rb b/contract-tests/hook.rb new file mode 100644 index 00000000..0c9eb221 --- /dev/null +++ b/contract-tests/hook.rb @@ -0,0 +1,74 @@ +require 'ldclient-rb' + +class Hook + include LaunchDarkly::Interfaces::Hooks::Hook + + # + # @param name [String] + # @param callback_uri [String] + # @param data [Hash] + # @parm errors [Hash] + # + def initialize(name, callback_uri, data, errors) + @metadata = LaunchDarkly::Interfaces::Hooks::Metadata.new(name) + @callback_uri = callback_uri + @data = data + @errors = errors + @context_filter = LaunchDarkly::Impl::ContextFilter.new(false, []) + end + + def metadata + @metadata + end + + # + # @param evaluation_series_context [LaunchDarkly::Interfaces::Hooks::EvaluationSeriesContext] + # @param data [Hash] + # + def before_evaluation(evaluation_series_context, data) + raise @errors[:beforeEvaluation] if @errors.include? :beforeEvaluation + + payload = { + evaluationSeriesContext: { + flagKey: evaluation_series_context.key, + context: @context_filter.filter(evaluation_series_context.context), + defaultValue: evaluation_series_context.default_value, + method: evaluation_series_context.method, + }, + evaluationSeriesData: data, + stage: 'beforeEvaluation', + } + result = HTTP.post(@callback_uri, json: payload) + + (data || {}).merge(@data[:beforeEvaluation] || {}) + end + + + # + # @param evaluation_series_context [LaunchDarkly::Interfaces::Hooks::EvaluationSeriesContext] + # @param data [Hash] + # @param detail [LaunchDarkly::EvaluationDetail] + # + def after_evaluation(evaluation_series_context, data, detail) + raise @errors[:afterEvaluation] if @errors.include? :afterEvaluation + + payload = { + evaluationSeriesContext: { + flagKey: evaluation_series_context.key, + context: @context_filter.filter(evaluation_series_context.context), + defaultValue: evaluation_series_context.default_value, + method: evaluation_series_context.method, + }, + evaluationSeriesData: data, + evaluationDetail: { + value: detail.value, + variationIndex: detail.variation_index, + reason: detail.reason, + }, + stage: 'afterEvaluation', + } + HTTP.post(@callback_uri, json: payload) + + (data || {}).merge(@data[:afterEvaluation] || {}) + end +end diff --git a/contract-tests/service.rb b/contract-tests/service.rb index 10d66e16..5f252970 100644 --- a/contract-tests/service.rb +++ b/contract-tests/service.rb @@ -39,6 +39,7 @@ 'polling-gzip', 'inline-context', 'anonymous-redaction', + 'evaluation-hooks', ], }.to_json end diff --git a/lib/ldclient-rb/config.rb b/lib/ldclient-rb/config.rb index 7c2642be..09347e20 100644 --- a/lib/ldclient-rb/config.rb +++ b/lib/ldclient-rb/config.rb @@ -43,6 +43,7 @@ class Config # @option opts [BigSegmentsConfig] :big_segments See {#big_segments}. # @option opts [Hash] :application See {#application} # @option opts [String] :payload_filter_key See {#payload_filter_key} + # @option hooks [Array] + # @param evaluation_series_context [EvaluationSeriesContext] + # + # @return [Array] + # + private def execute_before_evaluation(hooks, evaluation_series_context) + hooks.map do |hook| + try_execute_stage(:before_evaluation, hook.metadata.name) do + hook.before_evaluation(evaluation_series_context, {}) + end + end + end + + # + # Execute the :after_evaluation stage of the evaluation series. + # + # This method will return the results of each hook, indexed into an array in the same order as the hooks. If a hook + # raised an uncaught exception, the value will be nil. + # + # @param hooks [Array] + # @param evaluation_series_context [EvaluationSeriesContext] + # @param hook_data [Array] + # @param evaluation_detail [EvaluationDetail] + # + # @return [Array] + # + private def execute_after_evaluation(hooks, evaluation_series_context, hook_data, evaluation_detail) + hooks.zip(hook_data).reverse.map do |(hook, data)| + try_execute_stage(:after_evaluation, hook.metadata.name) do + hook.after_evaluation(evaluation_series_context, data, evaluation_detail) + end + end + end + + # + # Try to execute the provided block. If execution raises an exception, catch and log it, then move on with + # execution. + # + # @return [any] + # + private def try_execute_stage(method, hook_name) + begin + yield + rescue => e + @config.logger.error { "[LDClient] An error occurred in #{method} of the hook #{hook_name}: #{e}" } + nil + end + end + + # + # Return a copy of the existing hooks and a few instance of the EvaluationSeriesContext used for the evaluation series. + # + # @param key [String] + # @param context [LDContext] + # @param default [any] + # @param method [Symbol] + # @return [Array[Array, Interfaces::Hooks::EvaluationSeriesContext]] + # + private def prepare_hooks(key, context, default, method) + # Copy the hooks to use a consistent set during the evaluation series. + # + # Hooks can be added and we want to ensure all correct stages for a given hook execute. For example, we do not + # want to trigger the after_evaluation method without also triggering the before_evaluation method. + hooks = @hooks.dup + evaluation_series_context = Interfaces::Hooks::EvaluationSeriesContext.new(key, context, default, method) + + [hooks, evaluation_series_context] end # @@ -249,20 +384,24 @@ def migration_variation(key, context, default_stage) end context = Impl::Context::make_context(context) - detail, flag, _ = variation_with_flag(key, context, default_stage.to_s) + result = evaluate_with_hooks(key, context, default_stage, :migration_variation) do + detail, flag, _ = variation_with_flag(key, context, default_stage.to_s) - stage = detail.value - stage = stage.to_sym if stage.respond_to? :to_sym + stage = detail.value + stage = stage.to_sym if stage.respond_to? :to_sym - if Migrations::VALID_STAGES.include?(stage) + if Migrations::VALID_STAGES.include?(stage) + tracker = Impl::Migrations::OpTracker.new(@config.logger, key, flag, context, detail, default_stage) + next LaunchDarkly::Impl::EvaluationWithHookResult.new(detail, {stage: stage, tracker: tracker}) + end + + detail = LaunchDarkly::Impl::Evaluator.error_result(LaunchDarkly::EvaluationReason::ERROR_WRONG_TYPE, default_stage.to_s) tracker = Impl::Migrations::OpTracker.new(@config.logger, key, flag, context, detail, default_stage) - return stage, tracker - end - detail = LaunchDarkly::Impl::Evaluator.error_result(LaunchDarkly::EvaluationReason::ERROR_WRONG_TYPE, default_stage.to_s) - tracker = Impl::Migrations::OpTracker.new(@config.logger, key, flag, context, detail, default_stage) + LaunchDarkly::Impl::EvaluationWithHookResult.new(detail, {stage: default_stage, tracker: tracker}) + end - [default_stage, tracker] + [result.results[:stage], result.results[:tracker]] end # @@ -502,7 +641,7 @@ def create_default_data_source(sdk_key, config, diagnostic_accumulator) # # @param key [String] - # @param context [Hash, LDContext] + # @param context [LDContext] # @param default [Object] # # @return [Array] @@ -513,7 +652,7 @@ def variation_with_flag(key, context, default) # # @param key [String] - # @param context [Hash, LDContext] + # @param context [LDContext] # @param default [Object] # @param with_reasons [Boolean] # @@ -530,7 +669,6 @@ def evaluate_internal(key, context, default, with_reasons) return detail, nil, "no context provided" end - context = Impl::Context::make_context(context) unless context.valid? @config.logger.error { "[LDClient] Context was invalid for evaluation of flag '#{key}' (#{context.error}); returning default value" } detail = Evaluator.error_result(EvaluationReason::ERROR_USER_NOT_SPECIFIED, default) diff --git a/spec/ldclient_hooks_spec.rb b/spec/ldclient_hooks_spec.rb new file mode 100644 index 00000000..4c403981 --- /dev/null +++ b/spec/ldclient_hooks_spec.rb @@ -0,0 +1,199 @@ +require "ldclient-rb" + +require "mock_components" +require "model_builders" +require "spec_helper" + +module LaunchDarkly + describe "LDClient hooks tests" do + context "registration" do + it "can register a hook on the config" do + count = 0 + hook = MockHook.new(->(_, _) { count += 1 }, ->(_, _, _) { count += 2 }) + with_client(test_config(hooks: [hook])) do |client| + client.variation("doesntmatter", basic_context, "default") + expect(count).to eq 3 + end + end + + it "can register a hook on the client" do + count = 0 + hook = MockHook.new(->(_, _) { count += 1 }, ->(_, _, _) { count += 2 }) + with_client(test_config()) do |client| + client.add_hook(hook) + client.variation("doesntmatter", basic_context, "default") + + expect(count).to eq 3 + end + end + + it "can register hooks on both" do + count = 0 + config_hook = MockHook.new(->(_, _) { count += 1 }, ->(_, _, _) { count += 2 }) + client_hook = MockHook.new(->(_, _) { count += 4 }, ->(_, _, _) { count += 8 }) + + with_client(test_config(hooks: [config_hook])) do |client| + client.add_hook(client_hook) + client.variation("doesntmatter", basic_context, "default") + + expect(count).to eq 15 + end + end + + it "will drop invalid hooks on config" do + config = test_config(hooks: [true, nil, "example thing"]) + expect(config.hooks.count).to eq 0 + end + + it "will drop invalid hooks on client" do + with_client(test_config) do |client| + client.add_hook(true) + client.add_hook(nil) + client.add_hook("example thing") + + expect(client.instance_variable_get("@hooks").count).to eq 0 + end + + config = test_config(hooks: [true, nil, "example thing"]) + expect(config.hooks.count).to eq 0 + end + end + + context "execution order" do + it "config order is preserved" do + order = [] + first_hook = MockHook.new(->(_, _) { order << "first before" }, ->(_, _, _) { order << "first after" }) + second_hook = MockHook.new(->(_, _) { order << "second before" }, ->(_, _, _) { order << "second after" }) + + with_client(test_config(hooks: [first_hook, second_hook])) do |client| + client.variation("doesntmatter", basic_context, "default") + expect(order).to eq ["first before", "second before", "second after", "first after"] + end + end + + it "client order is preserved" do + order = [] + first_hook = MockHook.new(->(_, _) { order << "first before" }, ->(_, _, _) { order << "first after" }) + second_hook = MockHook.new(->(_, _) { order << "second before" }, ->(_, _, _) { order << "second after" }) + + with_client(test_config()) do |client| + client.add_hook(first_hook) + client.add_hook(second_hook) + client.variation("doesntmatter", basic_context, "default") + + expect(order).to eq ["first before", "second before", "second after", "first after"] + end + end + + it "config hooks precede client hooks" do + order = [] + config_hook = MockHook.new(->(_, _) { order << "config before" }, ->(_, _, _) { order << "config after" }) + client_hook = MockHook.new(->(_, _) { order << "client before" }, ->(_, _, _) { order << "client after" }) + + with_client(test_config(hooks: [config_hook])) do |client| + client.add_hook(client_hook) + client.variation("doesntmatter", basic_context, "default") + + expect(order).to eq ["config before", "client before", "client after", "config after"] + end + end + end + + context "passing data" do + it "hook receives EvaluationDetail" do + td = Integrations::TestData.data_source + td.update(td.flag("flagkey").variations("value").variation_for_all(0)) + + detail = nil + config_hook = MockHook.new(->(_, _) { }, ->(_, _, d) { detail = d }) + with_client(test_config(data_source: td, hooks: [config_hook])) do |client| + client.variation("flagkey", basic_context, "default") + + expect(detail.value).to eq "value" + expect(detail.variation_index).to eq 0 + expect(detail.reason).to eq EvaluationReason::fallthrough + end + end + + it "from before evaluation to after evaluation" do + actual = nil + config_hook = MockHook.new(->(_, _) { "example string returned" }, ->(_, hook_data, _) { actual = hook_data }) + with_client(test_config(hooks: [config_hook])) do |client| + client.variation("doesntmatter", basic_context, "default") + + expect(actual).to eq "example string returned" + end + end + + it "exception receives nil value" do + actual = nil + config_hook = MockHook.new(->(_, _) { raise "example string returned" }, ->(_, hook_data, _) { actual = hook_data }) + with_client(test_config(hooks: [config_hook])) do |client| + client.variation("doesntmatter", basic_context, "default") + + expect(actual).to be_nil + end + end + + it "exceptions do not mess up data passing order" do + data = [] + first_hook = MockHook.new(->(_, _) { "first hook" }, ->(_, hook_data, _) { data << hook_data }) + second_hook = MockHook.new(->(_, _) { raise "second hook" }, ->(_, hook_data, _) { data << hook_data }) + third_hook = MockHook.new(->(_, _) { "third hook" }, ->(_, hook_data, _) { data << hook_data }) + with_client(test_config(hooks: [first_hook, second_hook, third_hook])) do |client| + client.variation("doesntmatter", basic_context, "default") + + # NOTE: These are reversed since the push happens in the after_evaluation (when hooks are reversed) + expect(data).to eq ["third hook", nil, "first hook"] + end + end + end + + context "migration variation" do + it "EvaluationDetail contains stage value" do + td = Integrations::TestData.data_source + td.update(td.flag("flagkey").variations("off").variation_for_all(0)) + + detail = nil + config_hook = MockHook.new(->(_, _) { }, ->(_, _, d) { detail = d }) + with_client(test_config(data_source: td, hooks: [config_hook])) do |client| + client.migration_variation("flagkey", basic_context, LaunchDarkly::Migrations::STAGE_LIVE) + + expect(detail.value).to eq LaunchDarkly::Migrations::STAGE_OFF.to_s + expect(detail.variation_index).to eq 0 + expect(detail.reason).to eq EvaluationReason::fallthrough + end + end + + it "EvaluationDetail gets default if flag doesn't evaluate to stage" do + td = Integrations::TestData.data_source + td.update(td.flag("flagkey").variations("nonstage").variation_for_all(0)) + + detail = nil + config_hook = MockHook.new(->(_, _) { }, ->(_, _, d) { detail = d }) + with_client(test_config(data_source: td, hooks: [config_hook])) do |client| + client.migration_variation("flagkey", basic_context, LaunchDarkly::Migrations::STAGE_LIVE) + + expect(detail.value).to eq LaunchDarkly::Migrations::STAGE_LIVE.to_s + expect(detail.variation_index).to eq nil + expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_WRONG_TYPE) + end + end + + it "EvaluationDetail default gets converted to off if invalid" do + td = Integrations::TestData.data_source + td.update(td.flag("flagkey").variations("nonstage").variation_for_all(0)) + + detail = nil + config_hook = MockHook.new(->(_, _) { }, ->(_, _, d) { detail = d }) + with_client(test_config(data_source: td, hooks: [config_hook])) do |client| + client.migration_variation("flagkey", basic_context, :invalid) + + expect(detail.value).to eq LaunchDarkly::Migrations::STAGE_OFF.to_s + expect(detail.variation_index).to eq nil + expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_WRONG_TYPE) + end + end + end + end +end diff --git a/spec/mock_components.rb b/spec/mock_components.rb index 3866b9f3..4eb0e7a8 100644 --- a/spec/mock_components.rb +++ b/spec/mock_components.rb @@ -97,4 +97,25 @@ def self.adding_to_queue(q) new(->(value) { q << value }) end end + + class MockHook + include Interfaces::Hooks::Hook + + def initialize(before_evaluation, after_evaluation) + @before_evaluation = before_evaluation + @after_evaluation = after_evaluation + end + + def metadata + Interfaces::Hooks::Metadata.new("mock hook") + end + + def before_evaluation(evaluation_series_context, data) + @before_evaluation.call(evaluation_series_context, data) + end + + def after_evaluation(evaluation_series_context, data, detail) + @after_evaluation.call(evaluation_series_context, data, detail) + end + end end