diff --git a/lib/flagsmith.rb b/lib/flagsmith.rb index 6e0464e..51ae62a 100644 --- a/lib/flagsmith.rb +++ b/lib/flagsmith.rb @@ -69,7 +69,6 @@ def initialize(config) api_client analytics_processor environment_data_polling_manager - engine load_offline_handler end @@ -99,10 +98,6 @@ def realtime_client @realtime_client ||= Flagsmith::RealtimeClient.new(@config) end - def engine - @engine ||= Flagsmith::Engine::Engine.new - end - def analytics_processor return nil unless @config.enable_analytics? @@ -211,21 +206,33 @@ def get_value_for_identity(feature_name, user_id = nil, default: nil) end def get_identity_segments(identifier, traits = {}) - unless environment - raise Flagsmith::ClientError, - 'Local evaluation or offline handler is required to obtain identity segments.' - end + raise Flagsmith::ClientError, 'Local evaluation or offline handler is required to obtain identity segments.' unless environment identity_model = get_identity_model(identifier, traits) - segment_models = engine.get_identity_segments(environment, identity_model) - segment_models.map { |sm| Flagsmith::Segments::Segment.new(id: sm.id, name: sm.name) }.compact + context = Flagsmith::Engine::Mappers.get_evaluation_context(environment, identity_model) + raise Flagsmith::ClientError, 'Local evaluation required to obtain identity segments' unless context + + evaluation_result = Flagsmith::Engine.get_evaluation_result(context) + evaluation_result[:segments].filter_map do |segment_result| + id = segment_result.dig(:metadata, :id) + Flagsmith::Segments::Segment.new(id: id, name: segment_result[:name]) if id + end end private - def environment_flags_from_document - Flagsmith::Flags::Collection.from_feature_state_models( - engine.get_environment_feature_states(environment), + def environment_flags_from_document # rubocop:disable Metrics/MethodLength + context = Flagsmith::Engine::Mappers.get_evaluation_context(environment) + + unless context + raise Flagsmith::ClientError, + 'Unable to get flags. No environment present.' + end + + evaluation_result = Flagsmith::Engine.get_evaluation_result(context) + + Flagsmith::Flags::Collection.from_evaluation_result( + evaluation_result, analytics_processor: analytics_processor, default_flag_handler: default_flag_handler, offline_handler: offline_handler @@ -234,12 +241,13 @@ def environment_flags_from_document def get_identity_flags_from_document(identifier, traits = {}) identity_model = get_identity_model(identifier, traits) + context = Flagsmith::Engine::Mappers.get_evaluation_context(environment, identity_model) + raise Flagsmith::ClientError, 'Unable to get flags. No environment present.' unless context - Flagsmith::Flags::Collection.from_feature_state_models( - engine.get_identity_feature_states(environment, identity_model), - identity_id: identity_model.composite_key, - analytics_processor: analytics_processor, - default_flag_handler: default_flag_handler, + evaluation_result = Flagsmith::Engine.get_evaluation_result(context) + Flagsmith::Flags::Collection.from_evaluation_result( + evaluation_result, + analytics_processor: analytics_processor, default_flag_handler: default_flag_handler, offline_handler: offline_handler ) end diff --git a/lib/flagsmith/engine/core.rb b/lib/flagsmith/engine/core.rb index 3e64f1c..dded5a5 100644 --- a/lib/flagsmith/engine/core.rb +++ b/lib/flagsmith/engine/core.rb @@ -5,6 +5,7 @@ require_relative 'environments/models' require_relative 'features/models' +require_relative 'features/constants' require_relative 'identities/models' require_relative 'organisations/models' require_relative 'projects/models' @@ -12,79 +13,135 @@ require_relative 'segments/models' require_relative 'utils/hash_func' require_relative 'mappers' -require_relative 'evaluation/core' module Flagsmith + # Core evaluation logic for feature flags module Engine - # Flags engine methods - class Engine - include Flagsmith::Engine::Segments::Evaluator - - def get_identity_feature_state(environment, identity, feature_name, override_traits = nil) - feature_states = get_identity_feature_states_dict(environment, identity, override_traits).values + extend self + include Flagsmith::Engine::Utils::HashFunc + include Flagsmith::Engine::Features::TargetingReasons + include Flagsmith::Engine::Segments::Evaluator + + # Get evaluation result from evaluation context + # + # @param evaluation_context [Hash] The evaluation context + # @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) + flags = evaluate_features(evaluation_context, segment_overrides) + { + flags: flags, + segments: segments + } + end - feature_state = feature_states.find { |f| f.feature.name == feature_name } + # Returns { segments: EvaluationResultSegments; segmentOverrides: Record; } + def evaluate_segments(evaluation_context) + return [], {} if evaluation_context[:segments].nil? - raise Flagsmith::FeatureStateNotFound, 'Feature State Not Found' if feature_state.nil? + identity_segments = get_segments_from_context(evaluation_context) - feature_state + segments = identity_segments.map do |segment| + { name: segment[:name], metadata: segment[:metadata] }.compact end - def get_identity_feature_states(environment, identity, override_traits = nil) - feature_states = get_identity_feature_states_dict(environment, identity, override_traits).values + segment_overrides = process_segment_overrides(identity_segments) + + [segments, segment_overrides] + end - return feature_states.select(&:enabled?) if environment.project.hide_disabled_flags + # Returns Record + def process_segment_overrides(identity_segments) # rubocop:disable Metrics/MethodLength + segment_overrides = {} - feature_states + identity_segments.each do |segment| + Array(segment[:overrides]).each do |override| + next unless should_apply_override(override, segment_overrides) + + segment_overrides[override[:name]] = { + feature: override, + segment_name: segment[:name] + } + end end - def get_environment_feature_state(environment, feature_name) - features_state = environment.feature_states.find { |f| f.feature.name == feature_name } + segment_overrides + end - raise Flagsmith::FeatureStateNotFound, 'Feature State Not Found' if features_state.nil? + def evaluate_features(evaluation_context, segment_overrides) + identity_key = get_identity_key(evaluation_context) - features_state + (evaluation_context[:features] || {}).each_with_object({}) do |(_, feature), flags| + segment_override = segment_overrides[feature[:name]] + final_feature = segment_override ? segment_override[:feature] : feature + + flag_result = build_flag_result(final_feature, identity_key, segment_override) + flags[final_feature[:name].to_sym] = flag_result end + end - def get_environment_feature_states(environment) - return environment.feature_states.select(&:enabled?) if environment.project.hide_disabled_flags + # 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 - environment.feature_states - end + { value: feature[:value], reason: nil } + end - private + # Returns {value: any; reason?: string} + 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 } - def get_identity_feature_states_dict(environment, identity, override_traits = nil) - # Get feature states from the environment - feature_states = {} - override = ->(fs) { feature_states[fs.feature.id] = fs } - environment.feature_states.each(&override) + variant = find_matching_variant(sorted_variants, percentage_value) + variant || { value: feature[:value], reason: nil } + end - override_by_matching_segments(environment, identity, override_traits) do |fs| - override.call(fs) unless higher_segment_priority?(feature_states, fs) - 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 - # Override with any feature states defined directly the identity - identity.identity_features.each(&override) - feature_states + start_percentage = limit end + nil + end - # Override with any feature states defined by matching segments - def override_by_matching_segments(environment, identity, override_traits) - identity_segments = get_identity_segments(environment, identity, override_traits) - identity_segments.each do |matching_segment| - matching_segment.feature_states.each do |feature_state| - yield feature_state if block_given? - end - end - 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 - def higher_segment_priority?(collection, feature_state) - collection.key?(feature_state.feature.id) && - collection[feature_state.feature.id].higher_segment_priority?( - feature_state - ) - end + private + + def build_flag_result(feature, identity_key, segment_override) + evaluated = evaluate_feature_value(feature, identity_key) + + 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) + } + + flag_result[:metadata] = feature[:metadata] if feature[:metadata] + flag_result + end + + # Extract identity key from evaluation context + # + # @param evaluation_context [Hash] The evaluation context + # @return [String, nil] The identity key or nil if no identity + def get_identity_key(evaluation_context) + return nil unless evaluation_context[:identity] + + 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/models.rb b/lib/flagsmith/engine/features/models.rb index d6d0ff2..de7ae0f 100644 --- a/lib/flagsmith/engine/features/models.rb +++ b/lib/flagsmith/engine/features/models.rb @@ -58,7 +58,7 @@ def multivariate_value(identity_id) # but `self` does. # 2. `other` have a feature segment with high priority def higher_segment_priority?(other) - feature_segment.priority.to_i < (other&.feature_segment&.priority || Float::INFINITY) + feature_segment.priority.to_i < (other&.feature_segment&.priority || WEAKEST_PRIORITY) rescue TypeError, NoMethodError false end diff --git a/lib/flagsmith/engine/mappers/environment.rb b/lib/flagsmith/engine/mappers/environment.rb index 75539f6..4931ace 100644 --- a/lib/flagsmith/engine/mappers/environment.rb +++ b/lib/flagsmith/engine/mappers/environment.rb @@ -26,7 +26,7 @@ def self.build_feature_hash(feature_state) name: feature_state.feature.name, enabled: feature_state.enabled, value: feature_state.get_value, - metadata: { flagsmith_id: feature_state.feature.id } + metadata: { id: feature_state.feature.id } } add_variants_to_feature(feature_hash, feature_state) add_priority_to_feature(feature_hash, feature_state) diff --git a/lib/flagsmith/engine/mappers/identity.rb b/lib/flagsmith/engine/mappers/identity.rb index 84c278c..45dc374 100644 --- a/lib/flagsmith/engine/mappers/identity.rb +++ b/lib/flagsmith/engine/mappers/identity.rb @@ -52,7 +52,7 @@ def self.build_overrides_key(identity_features) enabled: feature_state.enabled, value: feature_state.get_value, priority: Mappers::STRONGEST_PRIORITY, - metadata: { flagsmith_id: feature_state.feature.id } + metadata: { id: feature_state.feature.id } } end end diff --git a/lib/flagsmith/engine/mappers/segments.rb b/lib/flagsmith/engine/mappers/segments.rb index 7020be1..f842e6f 100644 --- a/lib/flagsmith/engine/mappers/segments.rb +++ b/lib/flagsmith/engine/mappers/segments.rb @@ -21,7 +21,7 @@ def self.build_segment_hash(segment) overrides: build_overrides(segment.feature_states), metadata: { source: 'API', - flagsmith_id: segment.id + id: segment.id } } end @@ -33,7 +33,7 @@ def self.build_overrides(feature_states) # rubocop:disable Metrics/MethodLength name: feature_state.feature.name, enabled: feature_state.enabled, value: feature_state.get_value, - metadata: { flagsmith_id: feature_state.feature.id } + metadata: { id: feature_state.feature.id } } add_priority_to_override(override_hash, feature_state) override_hash diff --git a/lib/flagsmith/engine/segments/evaluator.rb b/lib/flagsmith/engine/segments/evaluator.rb index 79a9cf3..c5d55cb 100644 --- a/lib/flagsmith/engine/segments/evaluator.rb +++ b/lib/flagsmith/engine/segments/evaluator.rb @@ -14,29 +14,6 @@ module Evaluator # rubocop:disable Metrics/ModuleLength include Flagsmith::Engine::Segments::Constants include Flagsmith::Engine::Utils::HashFunc - # Model-based segment evaluation - def get_identity_segments(environment, identity, override_traits = nil) - environment.project.segments.select do |s| - evaluate_identity_in_segment(identity, s, override_traits) - end - end - - def traits_match_segment_condition(identity_traits, condition, segment_id, identity_id) - if condition.operator == PERCENTAGE_SPLIT - return hashed_percentage_for_object_ids([segment_id, - identity_id]) <= condition.value.to_f - end - - trait = identity_traits.find { |t| t.key.to_s == condition.property } - - return handle_trait_existence_conditions(trait, condition.operator) if [IS_SET, - IS_NOT_SET].include?(condition.operator) - - return condition.match_trait_value?(trait.trait_value) if trait - - false - end - module_function def get_enriched_context(context) @@ -67,6 +44,22 @@ def get_segments_from_context(context) end end + def traits_match_segment_condition(identity_traits, condition, segment_id, identity_id) + if condition.operator == PERCENTAGE_SPLIT + return hashed_percentage_for_object_ids([segment_id, + identity_id]) <= condition.value.to_f + end + + trait = identity_traits.find { |t| t.key.to_s == condition.property } + + return handle_trait_existence_conditions(trait, condition.operator) if [IS_SET, + IS_NOT_SET].include?(condition.operator) + + return condition.match_trait_value?(trait.trait_value) if trait + + false + end + # Evaluates whether a given identity is in the provided segment. # # :param identity: identity model object to evaluate @@ -248,14 +241,6 @@ def primitive?(value) !(value.is_a?(Hash) || value.is_a?(Array)) end - - private - - def handle_trait_existence_conditions(matching_trait, operator) - return operator == IS_NOT_SET if matching_trait.nil? - - operator == IS_SET - end end end end diff --git a/lib/flagsmith/sdk/models/flags.rb b/lib/flagsmith/sdk/models/flags.rb index d851c98..59cea07 100644 --- a/lib/flagsmith/sdk/models/flags.rb +++ b/lib/flagsmith/sdk/models/flags.rb @@ -32,12 +32,13 @@ def initialize(enabled:, value:) # 'live' Flag class as returned by API or local evaluation class Flag < BaseFlag - attr_reader :feature_name, :feature_id + attr_reader :feature_name, :feature_id, :reason - def initialize(feature_name:, enabled:, value:, feature_id:) + def initialize(feature_name:, enabled:, value:, feature_id:, reason: nil) super(enabled: enabled, value: value, default: false) @feature_name = feature_name @feature_id = feature_id + @reason = reason end def <=>(other) @@ -54,7 +55,8 @@ def to_h feature_name: feature_name, value: value, enabled: enabled, - default: default + default: default, + reason: reason } end @@ -172,15 +174,27 @@ def from_api(json_data, **args) ) end - def from_feature_state_models(feature_states, identity_id: nil, **args) - to_flag_object = lambda { |feature_state, acc| - acc[normalize_key(feature_state.feature.name)] = - Flagsmith::Flags::Flag.from_feature_state_model(feature_state, identity_id) + def from_evaluation_result(evaluation_result, **args) + to_flag_object = lambda { |flag_result, acc| + id = flag_result.dig(:metadata, :id) + if id.nil? + raise Flagsmith::ClientError, + "FlagResult metadata.id is missing for feature \"#{flag_result[:name]}\". This indicates a bug in the SDK, please report it." + end + + acc[flag_result[:name]] = Collection.map_evaluated_flag_to_flag_result(flag_result, id) } - new( - feature_states.each_with_object({}, &to_flag_object), - **args + new(evaluation_result[:flags].each_value.each_with_object({}, &to_flag_object), **args) + end + + def map_evaluated_flag_to_flag_result(flag_result, id) + Flagsmith::Flags::Flag.new( + feature_name: flag_result[:name], + enabled: flag_result[:enabled], + value: flag_result[:value], + feature_id: id, + reason: flag_result[:reason] ) end diff --git a/spec/engine/e2e/engine_spec.rb b/spec/engine/e2e/engine_spec.rb index 0a40dbe..8fe8703 100644 --- a/spec/engine/e2e/engine_spec.rb +++ b/spec/engine/e2e/engine_spec.rb @@ -8,8 +8,6 @@ def get_test_files end def parse_jsonc(content) - # Simple JSONC parser: remove single-line comments - # JSON.parse will handle the rest cleaned = content.lines.reject { |line| line.strip.start_with?('//') }.join JSON.parse(cleaned, symbolize_names: true) end @@ -34,11 +32,8 @@ def load_test_file(filepath) test_evaluation_context = test_case[:context] test_expected_result = test_case[:result] - # TODO: Implement evaluation logic - evaluation_result = Flagsmith::Engine::Evaluation::Core.get_evaluation_result(test_evaluation_context) + evaluation_result = Flagsmith::Engine.get_evaluation_result(test_evaluation_context) - - # TODO: Uncomment when evaluation is implemented expect(evaluation_result[:flags]).to eq(test_expected_result[:flags]) expect(evaluation_result[:segments]).to eq(test_expected_result[:segments]) end diff --git a/spec/engine/unit/core_spec.rb b/spec/engine/unit/core_spec.rb deleted file mode 100644 index ad94a63..0000000 --- a/spec/engine/unit/core_spec.rb +++ /dev/null @@ -1,114 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Flagsmith::Engine, type: :model do - subject { Flagsmith::Engine::Engine.new } - - it 'test_identity_get_feature_state_without_any_override' do - feature_state = subject.get_identity_feature_state( - environment, identity, feature1.name - ) - - expect(feature_state.feature).to eq(feature1) - end - - it 'test_identity_get_feature_state_without_any_override_no_fs' do - expect { - subject.get_identity_feature_state(environment, identity, 'nonExistentName') - }.to raise_error(Flagsmith::FeatureStateNotFound) - end - - it 'test_identity_get_all_feature_states_no_segments' do - env = environment - ident = identity - overridden_feature = Flagsmith::Engine::Feature.new( - id: 3, name: 'overridden_feature', type: 'STANDARD' - ) - - env.feature_states << Flagsmith::Engine::FeatureState.new( - feature: overridden_feature, enabled: false, django_id: 3 - ) - - ident.identity_features << Flagsmith::Engine::FeatureState.new( - feature: overridden_feature, enabled: true, id: 4 - ) - - feature_states = subject.get_identity_feature_states(env, ident) - - expect(feature_states.length).to eq(3) - - feature_states.each do |feature_state| - environment_feature_state = get_environment_feature_state_for_feature( - env, feature_state.feature - ) - expected = - if environment_feature_state&.feature == overridden_feature then true - else environment_feature_state&.enabled - end - expect(feature_state.enabled?).to eq(expected) - end - end - - it 'test_identity_get_all_feature_states_with_traits' do - trait_models = [Flagsmith::Engine::Identities::Trait.new( - trait_key: Engine::Builders::SEGMENT_CONDITION_PROPERTY, - trait_value: Engine::Builders::SEGMENT_CONDITION_STRING_VALUE - )] - - feature_states = subject.get_identity_feature_states( - environment_with_segment_override, identity_in_segment, trait_models - ) - - expect(feature_states.first.get_value).to eq(Engine::Builders::SEGMENT_OVERRIDE_FEATURE_STATE_VALUE) - end - - it 'test_identity_get_all_feature_states_with_traits_hideDisabledFlags' do - trait_models = [Flagsmith::Engine::Identities::Trait.new( - trait_key: Engine::Builders::SEGMENT_CONDITION_PROPERTY, - trait_value: Engine::Builders::SEGMENT_CONDITION_STRING_VALUE - )] - - env = environment_with_segment_override - env.project.hide_disabled_flags = true - - feature_states = subject.get_identity_feature_states( - env, identity_in_segment, trait_models - ) - expect(feature_states.length).to eq(0) - end - - it 'test_environment_get_all_feature_states' do - env = environment - feature_states = subject.get_environment_feature_states(env) - - expect(feature_states).to eq(env.feature_states) - end - - it 'test_environment_get_feature_states_hides_disabled_flags_if_enabled' do - env = environment - - env.project.hide_disabled_flags = true - - feature_states = subject.get_environment_feature_states(env) - - expect(feature_states).to_not eq(env.feature_states) - feature_states.each do |fs| - expect(fs.enabled).to be_truthy - end - end - - it 'test_environment_get_feature_state' do - env = environment - feature = feature1 - feature_state = subject.get_environment_feature_state(env, feature.name) - - expect(feature_state.feature).to eq(feature) - end - - it 'test_environment_get_feature_state_raises_feature_state_not_found' do - expect { - subject.get_environment_feature_state(environment, 'not_a_feature_name') - }.to raise_error(Flagsmith::FeatureStateNotFound) - end -end diff --git a/spec/engine/unit/evaluation_context_mappers_spec.rb b/spec/engine/unit/evaluation_context_mappers_spec.rb index 0af9686..ce422b7 100644 --- a/spec/engine/unit/evaluation_context_mappers_spec.rb +++ b/spec/engine/unit/evaluation_context_mappers_spec.rb @@ -33,7 +33,7 @@ expect(segment[:rules].length).to eq(1) expect(segment[:overrides]).to be_empty.or be_an(Array) expect(segment[:metadata][:source]).to eq('API') - expect(segment[:metadata][:flagsmith_id]).to eq(1) + expect(segment[:metadata][:id]).to eq(1) # Verify segment rules expect(segment[:rules][0][:type]).to eq('ALL') @@ -72,7 +72,7 @@ expect(override[:enabled]).to be false expect(override[:value]).to eq('some-overridden-value') expect(override[:priority]).to eq(Flagsmith::Engine::Mappers::STRONGEST_PRIORITY) - expect(override[:metadata][:flagsmith_id]).to eq(1) + expect(override[:metadata][:id]).to eq(1) # Verify features expect(context[:features]).to be_a(Hash) @@ -83,7 +83,7 @@ expect(some_feature[:enabled]).to be true expect(some_feature[:value]).to eq('some-value') expect(some_feature[:priority]).to be_nil - expect(some_feature[:metadata][:flagsmith_id]).to eq(1) + expect(some_feature[:metadata][:id]).to eq(1) # Verify multivariate feature expect(context[:features]).to have_key('test_mv') diff --git a/spec/engine/unit/segments/evaluator_spec.rb b/spec/engine/unit/segments/evaluator_spec.rb deleted file mode 100644 index d89106b..0000000 --- a/spec/engine/unit/segments/evaluator_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -# list of test cases containing: operator, property, value, traits (list of dicts), expected_result -TEST_CASES = [ - ['IS_SET', 'foo', nil, {}, false], - ['IS_SET', 'foo', nil, {'foo': 'bar'}, true], - ['IS_NOT_SET', 'foo', nil, {}, true], - ['IS_NOT_SET', 'foo', nil, {'foo': 'bar'}, false], -] - -RSpec.describe Flagsmith::Engine::Segments::Evaluator do - subject { Class.new { extend Flagsmith::Engine::Segments::Evaluator } } - - TEST_CASES.each do |(operator, property, value, traits, expected_result)| - it "traits: #{traits} #traits_match_segment_condition(#{operator}, #{property}, #{value || 'No value'}) should be #{expected_result}" do - condition = Flagsmith::Engine::Segments::Condition.new( - operator: operator, property: property, value: value - ) - trait_models = traits.map { - |k,v| Flagsmith::Engine::Identities::Trait.new(trait_key: k, trait_value: v) - } - - expect(subject.traits_match_segment_condition( - trait_models, condition, 1, 1)).to eq(expected_result) - end - end -end