diff --git a/.gitmodules b/.gitmodules index b2bee4c..78589c8 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "spec/engine-test-data"] path = spec/engine-test-data url = git@github.com:Flagsmith/engine-test-data.git - branch = v3.7.0 + branch = v3.12.1 diff --git a/lib/flagsmith/engine/core.rb b/lib/flagsmith/engine/core.rb index dded5a5..f663733 100644 --- a/lib/flagsmith/engine/core.rb +++ b/lib/flagsmith/engine/core.rb @@ -13,10 +13,11 @@ require_relative 'segments/models' require_relative 'utils/hash_func' require_relative 'mappers' +require_relative 'flag_dependencies' module Flagsmith # Core evaluation logic for feature flags - module Engine + module Engine # rubocop:disable Metrics/ModuleLength extend self include Flagsmith::Engine::Utils::HashFunc include Flagsmith::Engine::Features::TargetingReasons @@ -28,25 +29,27 @@ module Engine # @return [Hash] Evaluation result with flags and segments def get_evaluation_result(evaluation_context) evaluation_context = get_enriched_context(evaluation_context) - segments, segment_overrides = evaluate_segments(evaluation_context) + resolved_flags = LazyFlags.new + evaluation_context = evaluation_context.merge(flags: resolved_flags) + resolved_flags.bind(evaluation_context) + + segments, segment_overrides = evaluate_segments(evaluation_context, resolved_flags) flags = evaluate_features(evaluation_context, segment_overrides) - { - flags: flags, - segments: segments - } + apply_resolved_flags(flags, resolved_flags) if resolved_flags.used? + { flags: flags, segments: segments } end # Returns { segments: EvaluationResultSegments; segmentOverrides: Record; } - def evaluate_segments(evaluation_context) + def evaluate_segments(evaluation_context, resolved_flags) return [], {} if evaluation_context[:segments].nil? - identity_segments = get_segments_from_context(evaluation_context) + matched_segments = evaluation_context[:segments].values.select { |segment| resolved_flags.segment_matches?(segment) } - segments = identity_segments.map do |segment| + segments = matched_segments.map do |segment| { name: segment[:name], metadata: segment[:metadata] }.compact end - segment_overrides = process_segment_overrides(identity_segments) + segment_overrides = process_segment_overrides(matched_segments) [segments, segment_overrides] end @@ -71,63 +74,90 @@ def process_segment_overrides(identity_segments) # rubocop:disable Metrics/Metho def evaluate_features(evaluation_context, segment_overrides) identity_key = get_identity_key(evaluation_context) + targeted_feature_names = get_targeted_feature_names(evaluation_context) (evaluation_context[:features] || {}).each_with_object({}) do |(_, feature), flags| segment_override = segment_overrides[feature[:name]] final_feature = segment_override ? segment_override[:feature] : feature + reason = segment_override ? get_targeting_match_reason(segment_override) : get_fallback_reason(feature[:name], targeted_feature_names) - flag_result = build_flag_result(final_feature, identity_key, segment_override) - flags[final_feature[:name].to_sym] = flag_result + flags[final_feature[:name].to_sym] = get_flag_result(final_feature, identity_key, reason) end end - # Returns {value: any; reason?: string} - def evaluate_feature_value(feature, identity_key = nil) - return get_multivariate_feature_value(feature, identity_key) if feature[:variants]&.any? && identity_key + # Returns {name: string; enabled: boolean; value: any; reason: string; variant?: string; metadata?: any} + def get_flag_result(feature, identity_key, reason) + value = feature[:value] + variant = nil + + if identity_key && feature[:variants]&.any? + variant = 'control' + matching_variant = get_multivariate_feature_value(feature, identity_key) + value, variant, reason = matching_variant.values_at(:value, :variant, :reason) if matching_variant + end - { value: feature[:value], reason: nil } + flag_result = { name: feature[:name], enabled: feature[:enabled], value: value, reason: reason, variant: variant } + flag_result[:metadata] = feature[:metadata] if feature[:metadata] + flag_result end - # Returns {value: any; reason?: string} + # Returns {value: any; variant?: string; reason: string} or nil def get_multivariate_feature_value(feature, identity_key) percentage_value = hashed_percentage_for_object_ids([feature[:key], identity_key]) sorted_variants = (feature[:variants] || []).sort_by { |v| v[:priority] || WEAKEST_PRIORITY } - variant = find_matching_variant(sorted_variants, percentage_value) - variant || { value: feature[:value], reason: nil } + find_matching_variant(sorted_variants, percentage_value) end def find_matching_variant(sorted_variants, percentage_value) start_percentage = 0 sorted_variants.each do |variant| limit = start_percentage + variant[:weight] - return { value: variant[:value], reason: "#{TARGETING_REASON_SPLIT}; weight=#{variant[:weight]}" } if start_percentage <= percentage_value && percentage_value < limit + if start_percentage <= percentage_value && percentage_value < limit + return { value: variant[:value], variant: variant[:key], reason: "#{TARGETING_REASON_SPLIT}; weight=#{variant[:weight]}" } + end start_percentage = limit end nil end + # Returns the names of the features overridden by at least one segment + def get_targeted_feature_names(evaluation_context) + names = {} + evaluation_context[:segments]&.each_value do |segment| + segment[:overrides]&.each { |override| names[override[:name]] = true } + end + names + end + + # Returns DEFAULT for a feature whose segment overrides did not match, STATIC for a feature without any + def get_fallback_reason(feature_name, targeted_feature_names) + targeted_feature_names.key?(feature_name) ? TARGETING_REASON_DEFAULT : TARGETING_REASON_STATIC + end + + def get_targeting_match_reason(segment_override) + "#{TARGETING_REASON_TARGETING_MATCH}; segment=#{segment_override[:segment_name]}" + end + # returns boolean def should_apply_override(override, existing_overrides) current_override = existing_overrides[override[:name]] !current_override || stronger_priority?(override[:priority], current_override[:feature][:priority]) end - private - - def build_flag_result(feature, identity_key, segment_override) - evaluated = evaluate_feature_value(feature, identity_key) + def stronger_priority?(priority_a, priority_b) + (priority_a || WEAKEST_PRIORITY) < (priority_b || WEAKEST_PRIORITY) + end - flag_result = { - name: feature[:name], - enabled: feature[:enabled], - value: evaluated[:value], - reason: evaluated[:reason] || (segment_override ? "#{TARGETING_REASON_TARGETING_MATCH}; segment=#{segment_override[:segment_name]}" : TARGETING_REASON_DEFAULT) - } + private - flag_result[:metadata] = feature[:metadata] if feature[:metadata] - flag_result + # Flags resolved while evaluating segment conditions take precedence, as they were resolved with the cycle guard + def apply_resolved_flags(flags, resolved_flags) + resolved_flags.each { |feature_name, flag_result| flags[feature_name] = flag_result } + resolved_flags.cyclic_feature_names.each do |feature_name| + flags[feature_name] = flags[feature_name].merge(reason: TARGETING_REASON_CIRCULAR_DEPENDENCY) if flags.key?(feature_name) + end end # Extract identity key from evaluation context @@ -139,9 +169,5 @@ def get_identity_key(evaluation_context) evaluation_context[:identity][:key] end - - def stronger_priority?(priority_a, priority_b) - (priority_a || WEAKEST_PRIORITY) < (priority_b || WEAKEST_PRIORITY) - end end end diff --git a/lib/flagsmith/engine/features/constants.rb b/lib/flagsmith/engine/features/constants.rb index b3baa08..1e2fde0 100644 --- a/lib/flagsmith/engine/features/constants.rb +++ b/lib/flagsmith/engine/features/constants.rb @@ -6,6 +6,8 @@ module Features # Targeting reason constants for evaluation results module TargetingReasons TARGETING_REASON_DEFAULT = 'DEFAULT' + TARGETING_REASON_STATIC = 'STATIC' + TARGETING_REASON_CIRCULAR_DEPENDENCY = 'ERROR; code=CIRCULAR_DEPENDENCY' TARGETING_REASON_TARGETING_MATCH = 'TARGETING_MATCH' TARGETING_REASON_SPLIT = 'SPLIT' end diff --git a/lib/flagsmith/engine/flag_dependencies.rb b/lib/flagsmith/engine/flag_dependencies.rb new file mode 100644 index 0000000..9c701a2 --- /dev/null +++ b/lib/flagsmith/engine/flag_dependencies.rb @@ -0,0 +1,140 @@ +# frozen_string_literal: true + +require 'set' + +module Flagsmith + module Engine + # The `$.flags` mapping of an evaluation context, resolving a flag when a segment condition first reads it, + # and the memo of the context's segment verdicts. + class LazyFlags < Hash + # Incremented whenever a cycle is cut, so a segment evaluated meanwhile is known to rest on an unresolved flag. + attr_accessor :cycle_hits + + def initialize + super() { |_, feature_name| resolve(feature_name) } + @cycle_hits = 0 + @used = false + end + + def bind(context) + @context = context + end + + # Names of the flags found to be in a dependency cycle. + def cyclic_feature_names + @cyclic_feature_names ||= Set.new + end + + # Whether a segment condition has read a flag. + def used? + @used + end + + # Whether the context belongs to a segment, memoised once flags are read unless a dependency cycle was cut meanwhile. + def segment_matches?(segment) + return @segment_matches[segment] if @segment_matches&.key?(segment) + + cycle_hits = @cycle_hits + matches = Flagsmith::Engine::Segments::Evaluator.context_in_segment?(@context, segment) + return false if @cycle_hits != cycle_hits + + (@segment_matches ||= {}.compare_by_identity)[segment] = matches if @used + matches + end + + def key?(feature_name) + !self[feature_name].nil? + end + + private + + def resolve(feature_name) + @used = true + resolver.resolve_feature(feature_name) + fetch(feature_name, nil) + end + + def resolver + @resolver ||= DependencyResolver.new(@context, self) + end + end + + # Resolves the flags of a context whose segments depend on other flags. + class DependencyResolver + include Flagsmith::Engine::Features::TargetingReasons + + def initialize(context, flags) + @context = context + @flags = flags + @segments = context[:segments] || {} + @features_by_name = (context[:features] || {}).values.to_h { |feature| [feature[:name].to_sym, feature] } + @segment_keys_by_feature_name = segment_keys_by_feature_name + @resolving = [] + end + + def resolve_feature(feature_name) + return cut_cycle(feature_name) if @resolving.include?(feature_name) + + feature = @features_by_name[feature_name] + return unless feature + + segment_override = resolve_segment_override(feature_name) + return if @flags.cyclic_feature_names.include?(feature_name) + + @flags.store(feature_name, flag_result(feature_name, feature, segment_override)) + end + + private + + def cut_cycle(feature_name) + @flags.cyclic_feature_names.merge(@resolving.drop(@resolving.index(feature_name))) + @flags.cycle_hits += 1 + end + + def resolve_segment_override(feature_name) + @resolving.push(feature_name) + find_segment_override(feature_name) + ensure + @resolving.pop # A raise swallowed by JSONPath evaluation must not leave the name looking like a cycle + end + + def find_segment_override(feature_name) + matching_segment_overrides(feature_name).reduce(nil) do |winner, candidate| + next candidate if winner.nil? || Flagsmith::Engine.stronger_priority?(candidate[:feature][:priority], winner[:feature][:priority]) + + winner + end + end + + def matching_segment_overrides(feature_name) + @segment_keys_by_feature_name.fetch(feature_name, []).flat_map do |segment_key| + segment = @segments[segment_key] + next [] unless @flags.segment_matches?(segment) + + Array(segment[:overrides]) + .select { |override| override[:name].to_sym == feature_name } + .map { |override| { feature: override, segment_name: segment[:name] } } + end + end + + def flag_result(feature_name, feature, segment_override) + identity_key = @context.dig(:identity, :key) + return Flagsmith::Engine.get_flag_result(feature, identity_key, fallback_reason(feature_name)) unless segment_override + + Flagsmith::Engine.get_flag_result(segment_override[:feature], identity_key, Flagsmith::Engine.get_targeting_match_reason(segment_override)) + end + + def fallback_reason(feature_name) + @segment_keys_by_feature_name.key?(feature_name) ? TARGETING_REASON_DEFAULT : TARGETING_REASON_STATIC + end + + def segment_keys_by_feature_name + @segments.each_with_object({}) do |(segment_key, segment), keys_by_name| + Array(segment[:overrides]).each do |override| + (keys_by_name[override[:name].to_sym] ||= []) << segment_key + end + end + end + end + end +end diff --git a/lib/flagsmith/engine/segments/evaluator.rb b/lib/flagsmith/engine/segments/evaluator.rb index 1deaadf..504f0b2 100644 --- a/lib/flagsmith/engine/segments/evaluator.rb +++ b/lib/flagsmith/engine/segments/evaluator.rb @@ -13,6 +13,7 @@ module Segments module Evaluator # rubocop:disable Metrics/ModuleLength include Flagsmith::Engine::Segments::Constants include Flagsmith::Engine::Utils::HashFunc + extend Flagsmith::Engine::Utils::HashFunc module_function @@ -29,19 +30,25 @@ def get_enriched_context(context) end # Context-based segment evaluation - # Returns all segments that the identity belongs to based on segment rules evaluation + # Returns all segments that the context belongs to based on segment rules evaluation # - # @param context [Hash] Evaluation context containing identity and segment definitions - # @return [Array] Array of segments that the identity matches + # @param context [Hash] Evaluation context containing segment definitions + # @return [Array] Array of segments that the context matches def get_segments_from_context(context) - return [] unless context[:identity] && context[:segments] + return [] unless context[:segments] - context[:segments].values.select do |segment| - next false if segment[:rules].nil? || segment[:rules].empty? + context[:segments].values.select { |segment| context_in_segment?(context, segment) } + end - matches = segment[:rules].all? { |rule| traits_match_segment_rule_from_context(rule, segment[:key], context) } - matches - end + # Evaluates whether the context belongs to a segment + # + # @param context [Hash] The evaluation context + # @param segment [Hash] The segment definition + # @return [Boolean] True if every rule of the segment matches + def context_in_segment?(context, segment) + return false if segment[:rules].nil? || segment[:rules].empty? + + segment[:rules].all? { |rule| traits_match_segment_rule_from_context(rule, segment[:key], context) } end def traits_match_segment_condition(identity_traits, condition, segment_id, identity_id) @@ -227,12 +234,49 @@ def get_trait_value(property, context) def get_context_value(json_path, context) return nil unless context && json_path&.start_with?('$.') - results = JsonPath.new(json_path, use_symbols: true).on(context) - results.first + keys = json_path_keys(json_path) + keys ? dig_context_value(json_path, keys, context) : compiled_json_path(json_path).first(context) rescue StandardError nil end + # Digs a dotted JSONPath through hashes, deferring to JsonPath for any other node + # + # @param json_path [String] JSONPath expression + # @param keys [Array] The keys of the JSONPath + # @param context [Hash] The evaluation context + # @return [Object, nil] The value at the path or nil + def dig_context_value(json_path, keys, context) + keys.reduce(context) do |value, key| + return compiled_json_path(json_path).first(context) unless value.is_a?(Hash) + return nil unless value.key?(key) + + value[key] + end + end + + DOTTED_JSON_PATH = /\A\$(?:\.[\p{Alnum}_-]+)+\z/ + JSON_PATH_KEYS = {} # rubocop:disable Style/MutableConstant + COMPILED_JSON_PATHS = {} # rubocop:disable Style/MutableConstant + + # Splits a JSONPath made only of dotted names, which digs through hashes exactly as JsonPath does + # + # @param json_path [String] JSONPath expression + # @return [Array, false] The keys to dig, or false for any other expression + def json_path_keys(json_path) + JSON_PATH_KEYS.fetch(json_path) do + JSON_PATH_KEYS[json_path] = DOTTED_JSON_PATH.match?(json_path) && json_path.split('.').drop(1).map(&:to_sym) + end + end + + # Compiles a JSONPath expression once, as parsing it costs more than evaluating it + # + # @param json_path [String] JSONPath expression + # @return [JsonPath] The compiled expression + def compiled_json_path(json_path) + COMPILED_JSON_PATHS[json_path] ||= JsonPath.new(json_path, use_symbols: true) + end + # Get identity key from context # # @param context [Hash] The evaluation context diff --git a/spec/engine-test-data b/spec/engine-test-data index 4b29dc7..cb06740 160000 --- a/spec/engine-test-data +++ b/spec/engine-test-data @@ -1 +1 @@ -Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f +Subproject commit cb067401093766e55c3354c1de7c73d9331334df diff --git a/spec/sdk/local_evaluation_spec.rb b/spec/sdk/local_evaluation_spec.rb index 334be6c..44f9490 100644 --- a/spec/sdk/local_evaluation_spec.rb +++ b/spec/sdk/local_evaluation_spec.rb @@ -34,4 +34,120 @@ expect(flag.value).to eq("8888") end end + + describe 'flag dependency' do + let(:api_environment_document_response) do + environment_document = JSON.parse(File.read('spec/sdk/fixtures/environment.json')) + environment_document['project']['segments'] << { + 'id' => 2, + 'name' => 'some_feature_enabled', + 'rules' => [ + { + 'type' => 'ALL', + 'conditions' => [{ 'operator' => 'EQUAL', 'property_' => '$.flags.some_feature.enabled', 'value' => 'true' }], + 'rules' => [] + } + ], + 'feature_states' => [ + { + 'django_id' => 81028, + 'enabled' => true, + 'feature_state_value' => 'gated-value', + 'multivariate_feature_state_values' => [], + 'feature' => { 'id' => 83755, 'name' => 'test_mv', 'type' => 'MULTIVARIATE' } + } + ] + } + environment_document.to_json + end + + it 'serves an identity the override of a flag gated on another enabled flag' do + # Given + allow_any_instance_of(Flagsmith::Client).to receive(:api_client).and_return(mock_api_client) + flagsmith = Flagsmith::Client.new(environment_key: mock_api_key, api_url: mock_api_url, enable_local_evaluation: true) + + # When + flag = flagsmith.get_identity_flags('some-identifier').get_flag('test_mv') + + # Then + expect(flag.enabled).to be_truthy + expect(flag.value).to eq('gated-value') + expect(flag.reason).to eq('TARGETING_MATCH; segment=some_feature_enabled') + end + + it 'serves the environment the override of a flag gated on another enabled flag' do + # Given + allow_any_instance_of(Flagsmith::Client).to receive(:api_client).and_return(mock_api_client) + flagsmith = Flagsmith::Client.new(environment_key: mock_api_key, api_url: mock_api_url, enable_local_evaluation: true) + + # When + flag = flagsmith.get_environment_flags.get_flag('test_mv') + + # Then + expect(flag.enabled).to be_truthy + expect(flag.value).to eq('gated-value') + expect(flag.reason).to eq('TARGETING_MATCH; segment=some_feature_enabled') + end + + context 'when the other flag is overridden for a percentage of identities' do + let(:api_environment_document_response) do + environment_document = JSON.parse(File.read('spec/sdk/fixtures/environment.json')) + environment_document['project']['segments'] << { + 'id' => 2, + 'name' => 'some_feature_split', + 'rules' => [ + { + 'type' => 'ALL', + 'conditions' => [{ 'operator' => 'EQUAL', 'property_' => '$.flags.some_feature.value', 'value' => 'split-value' }], + 'rules' => [] + } + ], + 'feature_states' => [ + { + 'django_id' => 81028, + 'enabled' => true, + 'feature_state_value' => 'gated-value', + 'multivariate_feature_state_values' => [], + 'feature' => { 'id' => 83755, 'name' => 'test_mv', 'type' => 'MULTIVARIATE' } + } + ] + } + environment_document['project']['segments'] << { + 'id' => 3, + 'name' => 'everyone', + 'rules' => [ + { + 'type' => 'ALL', + 'conditions' => [{ 'operator' => 'PERCENTAGE_SPLIT', 'value' => '100' }], + 'rules' => [] + } + ], + 'feature_states' => [ + { + 'django_id' => 81029, + 'enabled' => true, + 'feature_state_value' => 'split-value', + 'multivariate_feature_state_values' => [], + 'feature' => { 'id' => 1, 'name' => 'some_feature', 'type' => 'STANDARD' } + } + ] + } + environment_document.to_json + end + + it 'serves an identity the override of a flag gated on the other flag' do + # Given + allow_any_instance_of(Flagsmith::Client).to receive(:api_client).and_return(mock_api_client) + flagsmith = Flagsmith::Client.new(environment_key: mock_api_key, api_url: mock_api_url, enable_local_evaluation: true) + + # When + flag = flagsmith.get_identity_flags('some-identifier').get_flag('test_mv') + + # Then + expect(flag.enabled).to be_truthy + expect(flag.value).to eq('gated-value') + expect(flag.reason).to eq('TARGETING_MATCH; segment=some_feature_split') + end + end + end end