From 0f5250a6a29dfb22552699cac1b464b2a6924e85 Mon Sep 17 00:00:00 2001 From: wadii Date: Mon, 27 Oct 2025 17:42:15 +0100 Subject: [PATCH 1/9] fix: sdk-uses-new-engine-methods --- lib/flagsmith.rb | 49 +++++++++++-- lib/flagsmith/engine/core.rb | 69 +------------------ lib/flagsmith/engine/evaluation/core.rb | 2 +- lib/flagsmith/engine/segments/evaluator.rb | 11 +-- lib/flagsmith/engine/segments/models.rb | 3 + lib/flagsmith/sdk/models/flags.rb | 33 ++++++++- spec/engine-test-data | 2 +- spec/engine/e2e/engine_spec.rb | 5 -- .../unit/{core_spec.rb => core_spec.rb.old} | 0 9 files changed, 82 insertions(+), 92 deletions(-) rename spec/engine/unit/{core_spec.rb => core_spec.rb.old} (100%) diff --git a/lib/flagsmith.rb b/lib/flagsmith.rb index 6e0464e..9a25635 100644 --- a/lib/flagsmith.rb +++ b/lib/flagsmith.rb @@ -217,15 +217,40 @@ def get_identity_segments(identifier, traits = {}) end 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::EvaluationContext::Mappers.get_evaluation_context( + environment, identity_model + ) + + unless context + raise Flagsmith::ClientError, + 'Local evaluation required to obtain identity segments' + end + + evaluation_result = Flagsmith::Engine::Evaluation::Core.get_evaluation_result(context) + + evaluation_result[:segments].map do |segment_result| + flagsmith_id = segment_result.dig(:metadata, :flagsmith_id) + next unless flagsmith_id + + Flagsmith::Segments::Segment.new(id: flagsmith_id, name: segment_result[:name]) + end.compact end private def environment_flags_from_document - Flagsmith::Flags::Collection.from_feature_state_models( - engine.get_environment_feature_states(environment), + context = Flagsmith::Engine::EvaluationContext::Mappers.get_evaluation_context(environment) + + unless context + raise Flagsmith::ClientError, + 'Unable to get flags. No environment present.' + end + + evaluation_result = Flagsmith::Engine::Evaluation::Core.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 @@ -235,9 +260,19 @@ def environment_flags_from_document def get_identity_flags_from_document(identifier, traits = {}) identity_model = get_identity_model(identifier, traits) - Flagsmith::Flags::Collection.from_feature_state_models( - engine.get_identity_feature_states(environment, identity_model), - identity_id: identity_model.composite_key, + context = Flagsmith::Engine::EvaluationContext::Mappers.get_evaluation_context( + environment, identity_model + ) + + unless context + raise Flagsmith::ClientError, + 'Unable to get flags. No environment present.' + end + + evaluation_result = Flagsmith::Engine::Evaluation::Core.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 diff --git a/lib/flagsmith/engine/core.rb b/lib/flagsmith/engine/core.rb index cb872b0..8c93d70 100644 --- a/lib/flagsmith/engine/core.rb +++ b/lib/flagsmith/engine/core.rb @@ -17,74 +17,11 @@ module Flagsmith module Engine # Flags engine methods + # NOTE: This class is kept for backwards compatibility but no longer contains + # the old model-based evaluation methods. Use the context-based evaluation + # via Flagsmith::Engine::Evaluation::Core.get_evaluation_result instead. 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 - - feature_state = feature_states.find { |f| f.feature.name == feature_name } - - raise Flagsmith::FeatureStateNotFound, 'Feature State Not Found' if feature_state.nil? - - feature_state - end - - def get_identity_feature_states(environment, identity, override_traits = nil) - feature_states = get_identity_feature_states_dict(environment, identity, override_traits).values - - return feature_states.select(&:enabled?) if environment.project.hide_disabled_flags - - feature_states - end - - def get_environment_feature_state(environment, feature_name) - features_state = environment.feature_states.find { |f| f.feature.name == feature_name } - - raise Flagsmith::FeatureStateNotFound, 'Feature State Not Found' if features_state.nil? - - features_state - end - - def get_environment_feature_states(environment) - return environment.feature_states.select(&:enabled?) if environment.project.hide_disabled_flags - - environment.feature_states - end - - private - - 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) - - override_by_matching_segments(environment, identity, override_traits) do |fs| - override.call(fs) unless higher_segment_priority?(feature_states, fs) - end - - # Override with any feature states defined directly the identity - identity.identity_features.each(&override) - feature_states - 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 - - def higher_segment_priority?(collection, feature_state) - collection.key?(feature_state.feature.id) && - collection[feature_state.feature.id].higher_segment_priority?( - feature_state - ) - end end end end diff --git a/lib/flagsmith/engine/evaluation/core.rb b/lib/flagsmith/engine/evaluation/core.rb index 90154f2..c976397 100644 --- a/lib/flagsmith/engine/evaluation/core.rb +++ b/lib/flagsmith/engine/evaluation/core.rb @@ -33,7 +33,7 @@ def evaluate_segments(evaluation_context) return [], {} end - identity_segments = get_identity_segments_from_context(evaluation_context) + identity_segments = get_identity_segments(evaluation_context) segments = identity_segments.map do |segment| result = { diff --git a/lib/flagsmith/engine/segments/evaluator.rb b/lib/flagsmith/engine/segments/evaluator.rb index d1031f6..1db553c 100644 --- a/lib/flagsmith/engine/segments/evaluator.rb +++ b/lib/flagsmith/engine/segments/evaluator.rb @@ -13,12 +13,12 @@ module Evaluator include Flagsmith::Engine::Segments::Constants include Flagsmith::Engine::Utils::HashFunc - # Context-based segment evaluation (new approach) + # Context-based segment evaluation # Returns all segments that the identity 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 - def get_identity_segments_from_context(context) + def get_identity_segments(context) return [] unless context[:identity] && context[:segments] matching_segments = context[:segments].values.select do |segment| @@ -31,13 +31,6 @@ def get_identity_segments_from_context(context) matching_segments end - # Model-based segment evaluation (existing approach) - 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 - # Evaluates whether a given identity is in the provided segment. # # :param identity: identity model object to evaluate diff --git a/lib/flagsmith/engine/segments/models.rb b/lib/flagsmith/engine/segments/models.rb index a95d547..f0f7369 100644 --- a/lib/flagsmith/engine/segments/models.rb +++ b/lib/flagsmith/engine/segments/models.rb @@ -103,6 +103,9 @@ def match_modulo_value(trait_value) def match_in_value(trait_value) return false if trait_value.nil? || trait_value.is_a?(TrueClass) || trait_value.is_a?(FalseClass) + # Floats/doubles are not supported by the engine due to ambiguous serialization across supported platforms. (segments/models_spec.rb) + return false unless trait_value.is_a?(String) || trait_value.is_a?(Integer) + if @value.is_a?(Array) return @value.include?(trait_value.to_s) end diff --git a/lib/flagsmith/sdk/models/flags.rb b/lib/flagsmith/sdk/models/flags.rb index d851c98..a201f59 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 @@ -184,6 +186,31 @@ def from_feature_state_models(feature_states, identity_id: nil, **args) ) end + def from_evaluation_result(evaluation_result, **args) + to_flag_object = lambda { |flag_result, acc| + flagsmith_id = flag_result.dig(:metadata, :flagsmith_id) + + if flagsmith_id.nil? + raise Flagsmith::ClientError, + "FlagResult metadata.flagsmith_id is missing for feature \"#{flag_result[:name]}\". " \ + "This indicates a bug in the SDK, please report it." + end + + acc[normalize_key(flag_result[:name])] = Flagsmith::Flags::Flag.new( + feature_name: flag_result[:name], + enabled: flag_result[:enabled], + value: flag_result[:value], + feature_id: flagsmith_id, + reason: flag_result[:reason] + ) + } + + new( + evaluation_result[:flags].each_value.each_with_object({}, &to_flag_object), + **args + ) + end + def normalize_key(key) key.to_s.downcase end diff --git a/spec/engine-test-data b/spec/engine-test-data index 6ab57ec..218757f 160000 --- a/spec/engine-test-data +++ b/spec/engine-test-data @@ -1 +1 @@ -Subproject commit 6ab57ec67bc84659e8b5aa41534b04fe45cc4cbe +Subproject commit 218757fd23f932760be681a09c686c9d6ef55fad diff --git a/spec/engine/e2e/engine_spec.rb b/spec/engine/e2e/engine_spec.rb index 0a40dbe..df633b4 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) - - # 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.old similarity index 100% rename from spec/engine/unit/core_spec.rb rename to spec/engine/unit/core_spec.rb.old From 60dfef6472eca8439341c02d1c7984a835becbd6 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 28 Oct 2025 15:58:39 +0100 Subject: [PATCH 2/9] feat: introduced-jsonpath-library --- Gemfile.lock | 4 + flagsmith.gemspec | 1 + lib/flagsmith/engine/evaluation/core.rb | 4 +- lib/flagsmith/engine/features/models.rb | 2 +- lib/flagsmith/engine/segments/evaluator.rb | 83 ++------------ lib/flagsmith/sdk/models/flags.rb | 12 --- spec/engine/unit/core_spec.rb.old | 114 -------------------- spec/engine/unit/segments/evaluator_spec.rb | 29 ----- 8 files changed, 14 insertions(+), 235 deletions(-) delete mode 100644 spec/engine/unit/core_spec.rb.old delete mode 100644 spec/engine/unit/segments/evaluator_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index bed803f..249a91e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,6 +4,7 @@ PATH flagsmith (4.3.0) faraday (>= 2.0.1) faraday-retry + jsonpath (~> 1.1) semantic GEM @@ -21,8 +22,11 @@ GEM faraday (~> 2.0) gem-release (2.2.2) json (2.7.1) + jsonpath (1.1.5) + multi_json language_server-protocol (3.17.0.3) method_source (1.0.0) + multi_json (1.17.0) net-http (0.4.1) uri parallel (1.24.0) diff --git a/flagsmith.gemspec b/flagsmith.gemspec index a4b950d..a1e9459 100644 --- a/flagsmith.gemspec +++ b/flagsmith.gemspec @@ -34,6 +34,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'faraday', '>= 2.0.1' spec.add_dependency 'faraday-retry' + spec.add_dependency 'jsonpath', '~> 1.1' spec.add_dependency 'semantic' spec.metadata['rubygems_mfa_required'] = 'true' end diff --git a/lib/flagsmith/engine/evaluation/core.rb b/lib/flagsmith/engine/evaluation/core.rb index c976397..9ce22cb 100644 --- a/lib/flagsmith/engine/evaluation/core.rb +++ b/lib/flagsmith/engine/evaluation/core.rb @@ -122,7 +122,7 @@ def evaluate_feature_value(feature, identity_key = nil) # 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] || Float::INFINITY } + sorted_variants = (feature[:variants] || []).sort_by { |v| v[:priority] || WEAKEST_PRIORITY } start_percentage = 0 sorted_variants.each do |variant| @@ -160,7 +160,7 @@ def get_identity_key(evaluation_context) # returns boolean def higher_priority?(priority_a, priority_b) - (priority_a || Float::INFINITY) < (priority_b || Float::INFINITY) + (priority_a || WEAKEST_PRIORITY) < (priority_b || WEAKEST_PRIORITY) end def get_targeting_match_reason(match_object) 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/segments/evaluator.rb b/lib/flagsmith/engine/segments/evaluator.rb index 1db553c..fe89129 100644 --- a/lib/flagsmith/engine/segments/evaluator.rb +++ b/lib/flagsmith/engine/segments/evaluator.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'json' +require 'jsonpath' require_relative 'constants' require_relative 'models' require_relative '../utils/hash_func' @@ -31,60 +33,7 @@ def get_identity_segments(context) matching_segments end - # Evaluates whether a given identity is in the provided segment. - # - # :param identity: identity model object to evaluate - # :param segment: segment model object to evaluate - # :param override_traits: pass in a list of traits to use instead of those on the - # identity model itself - # :return: True if the identity is in the segment, False otherwise - def evaluate_identity_in_segment(identity, segment, override_traits = nil) - segment.rules&.length&.positive? && - segment.rules.all? do |rule| - traits_match_segment_rule( - override_traits || identity.identity_traits, - rule, - segment.id, - identity.django_id || identity.composite_key - ) - end - end - - # rubocop:disable Metrics/MethodLength - def traits_match_segment_rule(identity_traits, rule, segment_id, identity_id) - matching_block = lambda { |condition| - traits_match_segment_condition(identity_traits, condition, segment_id, identity_id) - } - - matches_conditions = - if rule.conditions&.length&.positive? - rule.conditions.send(rule.matching_function, &matching_block) - else - true - end - - matches_conditions && - rule.rules.all? { |r| traits_match_segment_rule(identity_traits, r, segment_id, identity_id) } - end - # rubocop:enable Metrics/MethodLength - - 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 - - # Context-based helper functions (new approach) + # Context-based helper functions # Evaluates whether a segment rule matches using context # @@ -143,9 +92,7 @@ def traits_match_segment_condition_from_context(condition, segment_key, context) end return false if condition[:property].nil? - trait_value = get_trait_value(condition[:property], context) - return trait_value != nil if condition[:operator] == IS_SET return trait_value.nil? if condition[:operator] == IS_NOT_SET @@ -197,25 +144,15 @@ def get_trait_value(property, context) traits[property] || traits[property.to_sym] end - # Get value from context using JSONPath-like syntax + # Get value from context using JSONPath syntax # # @param json_path [String] JSONPath expression (e.g., '$.identity.identifier') # @param context [Hash] The evaluation context # @return [Object, nil] The value at the path or nil def get_context_value(json_path, context) return nil unless context && json_path&.start_with?('$.') - - # Simple JSONPath implementation - handle basic cases - path_parts = json_path.sub('$.', '').split('.') - current = context - - path_parts.each do |part| - return nil unless current.is_a?(Hash) - - current = current[part.to_sym] - end - - current + results = JsonPath.new(json_path, use_symbols: true).on(context) + results.first rescue StandardError nil end @@ -240,14 +177,6 @@ def non_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 a201f59..55f16a0 100644 --- a/lib/flagsmith/sdk/models/flags.rb +++ b/lib/flagsmith/sdk/models/flags.rb @@ -174,18 +174,6 @@ 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) - } - - new( - feature_states.each_with_object({}, &to_flag_object), - **args - ) - end - def from_evaluation_result(evaluation_result, **args) to_flag_object = lambda { |flag_result, acc| flagsmith_id = flag_result.dig(:metadata, :flagsmith_id) diff --git a/spec/engine/unit/core_spec.rb.old b/spec/engine/unit/core_spec.rb.old deleted file mode 100644 index ad94a63..0000000 --- a/spec/engine/unit/core_spec.rb.old +++ /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/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 From 646a8fcc616df07a9da7d7386d9af3bd7e394325 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 28 Oct 2025 17:56:31 +0100 Subject: [PATCH 3/9] feat: fixed-conflict --- lib/flagsmith/engine/segments/models.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/flagsmith/engine/segments/models.rb b/lib/flagsmith/engine/segments/models.rb index 2c77fbc..5e4b51d 100644 --- a/lib/flagsmith/engine/segments/models.rb +++ b/lib/flagsmith/engine/segments/models.rb @@ -103,16 +103,12 @@ def match_modulo_value(trait_value) def match_in_value(trait_value) return false if trait_value.nil? || trait_value.is_a?(TrueClass) || trait_value.is_a?(FalseClass) -<<<<<<< HEAD # Floats/doubles are not supported by the engine due to ambiguous serialization across supported platforms. (segments/models_spec.rb) return false unless trait_value.is_a?(String) || trait_value.is_a?(Integer) if @value.is_a?(Array) return @value.include?(trait_value.to_s) end -======= - return @value.include?(trait_value.to_s) if @value.is_a?(Array) ->>>>>>> 6a6a129d14a0f15bbf3252a2c6b539681dfa7e85 if @value.is_a?(String) begin From 1f5a0a849cd5e9cbde7cca908e5355495e81844c Mon Sep 17 00:00:00 2001 From: Zaimwa9 Date: Mon, 10 Nov 2025 17:08:11 +0100 Subject: [PATCH 4/9] Update lib/flagsmith/engine/segments/models.rb Co-authored-by: Kim Gustyr --- lib/flagsmith/engine/segments/models.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/flagsmith/engine/segments/models.rb b/lib/flagsmith/engine/segments/models.rb index 5e4b51d..b5dd9d5 100644 --- a/lib/flagsmith/engine/segments/models.rb +++ b/lib/flagsmith/engine/segments/models.rb @@ -103,8 +103,8 @@ def match_modulo_value(trait_value) def match_in_value(trait_value) return false if trait_value.nil? || trait_value.is_a?(TrueClass) || trait_value.is_a?(FalseClass) - # Floats/doubles are not supported by the engine due to ambiguous serialization across supported platforms. (segments/models_spec.rb) - return false unless trait_value.is_a?(String) || trait_value.is_a?(Integer) +(segments/models_spec.rb) + return false unless ![true, false].include? trait_value if @value.is_a?(Array) return @value.include?(trait_value.to_s) From 2dc4816a12d1fe349db978ae51be029ce148de7a Mon Sep 17 00:00:00 2001 From: Zaimwa9 Date: Mon, 10 Nov 2025 17:08:39 +0100 Subject: [PATCH 5/9] Update lib/flagsmith/engine/segments/models.rb --- lib/flagsmith/engine/segments/models.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/flagsmith/engine/segments/models.rb b/lib/flagsmith/engine/segments/models.rb index b5dd9d5..4f03fce 100644 --- a/lib/flagsmith/engine/segments/models.rb +++ b/lib/flagsmith/engine/segments/models.rb @@ -103,7 +103,6 @@ def match_modulo_value(trait_value) def match_in_value(trait_value) return false if trait_value.nil? || trait_value.is_a?(TrueClass) || trait_value.is_a?(FalseClass) -(segments/models_spec.rb) return false unless ![true, false].include? trait_value if @value.is_a?(Array) From 9755939c518c09d1f92d3edd9830bc3384a743c4 Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 11 Nov 2025 12:15:21 +0100 Subject: [PATCH 6/9] feat: removed-normalize --- lib/flagsmith/sdk/models/flags.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/flagsmith/sdk/models/flags.rb b/lib/flagsmith/sdk/models/flags.rb index 53ea047..e5601b3 100644 --- a/lib/flagsmith/sdk/models/flags.rb +++ b/lib/flagsmith/sdk/models/flags.rb @@ -184,7 +184,7 @@ def from_evaluation_result(evaluation_result, **args) 'This indicates a bug in the SDK, please report it.' end - acc[normalize_key(flag_result[:name])] = Flagsmith::Flags::Flag.new( + acc[flag_result[:name]] = Flagsmith::Flags::Flag.new( feature_name: flag_result[:name], enabled: flag_result[:enabled], value: flag_result[:value], From 1bdaad6cd8d3fa462117e85a488e609712f1eb3d Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 11 Nov 2025 12:47:32 +0100 Subject: [PATCH 7/9] feat: linter --- lib/flagsmith.rb | 42 ++++------------- lib/flagsmith/engine/core.rb | 78 +++++++++++++------------------ lib/flagsmith/sdk/models/flags.rb | 25 +++++----- 3 files changed, 54 insertions(+), 91 deletions(-) diff --git a/lib/flagsmith.rb b/lib/flagsmith.rb index ec47520..5532ec2 100644 --- a/lib/flagsmith.rb +++ b/lib/flagsmith.rb @@ -206,35 +206,22 @@ 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) - - context = Flagsmith::Engine::Mappers.get_evaluation_context( - environment, identity_model - ) - - unless context - raise Flagsmith::ClientError, - 'Local evaluation required to obtain identity segments' - end + 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].map do |segment_result| + evaluation_result[:segments].filter_map do |segment_result| flagsmith_id = segment_result.dig(:metadata, :flagsmith_id) - next unless flagsmith_id - - Flagsmith::Segments::Segment.new(id: flagsmith_id, name: segment_result[:name]) - end.compact + Flagsmith::Segments::Segment.new(id: flagsmith_id, name: segment_result[:name]) if flagsmith_id + end end private - def environment_flags_from_document + def environment_flags_from_document # rubocop:disable Metrics/MethodLength context = Flagsmith::Engine::Mappers.get_evaluation_context(environment) unless context @@ -254,22 +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 - ) - - unless context - raise Flagsmith::ClientError, - 'Unable to get flags. No environment present.' - end + context = Flagsmith::Engine::Mappers.get_evaluation_context(environment, identity_model) + raise Flagsmith::ClientError, 'Unable to get flags. No environment present.' unless context 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, + 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 a53cc47..05d2cfc 100644 --- a/lib/flagsmith/engine/core.rb +++ b/lib/flagsmith/engine/core.rb @@ -44,13 +44,7 @@ def evaluate_segments(evaluation_context) identity_segments = get_segments_from_context(evaluation_context) segments = identity_segments.map do |segment| - result = { - name: segment[:name] - } - - result[:metadata] = segment[:metadata] if segment[:metadata] - - result + { name: segment[:name], metadata: segment[:metadata] }.compact end segment_overrides = process_segment_overrides(identity_segments) @@ -59,15 +53,11 @@ def evaluate_segments(evaluation_context) end # Returns Record - def process_segment_overrides(identity_segments) + def process_segment_overrides(identity_segments) # rubocop:disable Metrics/MethodLength segment_overrides = {} identity_segments.each do |segment| - next unless segment[:overrides] - - overrides_list = segment[:overrides].is_a?(Array) ? segment[:overrides] : [] - - overrides_list.each do |override| + Array(segment[:overrides]).each do |override| next unless should_apply_override(override, segment_overrides) segment_overrides[override[:name]] = { @@ -82,33 +72,15 @@ def process_segment_overrides(identity_segments) # returns EvaluationResultFlags def evaluate_features(evaluation_context, segment_overrides) - flags = {} + identity_key = get_identity_key(evaluation_context) - (evaluation_context[:features] || {}).each_value do |feature| + (evaluation_context[:features] || {}).each_with_object({}) do |(_, feature), flags| segment_override = segment_overrides[feature[:name]] final_feature = segment_override ? segment_override[:feature] : feature - has_override = !segment_override.nil? - - # Evaluate feature value - evaluated = evaluate_feature_value(final_feature, get_identity_key(evaluation_context)) - - # Build flag result - flag_result = { - name: final_feature[:name], - enabled: final_feature[:enabled], - value: evaluated[:value] - } - - # Add metadata if present - flag_result[:metadata] = final_feature[:metadata] if final_feature[:metadata] - # Set reason - flag_result[:reason] = evaluated[:reason] || - (has_override ? "#{TARGETING_REASON_TARGETING_MATCH}; segment=#{segment_override[:segment_name]}" : TARGETING_REASON_DEFAULT) + flag_result = build_flag_result(final_feature, identity_key, segment_override) flags[final_feature[:name].to_sym] = flag_result end - - flags end # Returns {value: any; reason?: string} @@ -123,19 +95,19 @@ 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 } + end + + def find_matching_variant(sorted_variants, percentage_value) start_percentage = 0 sorted_variants.each do |variant| limit = start_percentage + variant[:weight] - if start_percentage <= percentage_value && percentage_value < limit - return { - value: variant[:value], - reason: "#{TARGETING_REASON_SPLIT}; weight=#{variant[:weight]}" - } - end + return { value: variant[:value], reason: "#{TARGETING_REASON_SPLIT}; weight=#{variant[:weight]}" } if start_percentage <= percentage_value && percentage_value < limit + start_percentage = limit end - - { value: feature[:value], reason: nil } + nil end # returns boolean @@ -144,12 +116,22 @@ def should_apply_override(override, existing_overrides) !current_override || stronger_priority?(override[:priority], current_override[:feature][:priority]) end - def stronger_priority?(priority_a, priority_b) - (priority_a || WEAKEST_PRIORITY) < (priority_b || WEAKEST_PRIORITY) - 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 @@ -159,5 +141,9 @@ 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/sdk/models/flags.rb b/lib/flagsmith/sdk/models/flags.rb index e5601b3..4def197 100644 --- a/lib/flagsmith/sdk/models/flags.rb +++ b/lib/flagsmith/sdk/models/flags.rb @@ -177,25 +177,24 @@ def from_api(json_data, **args) def from_evaluation_result(evaluation_result, **args) to_flag_object = lambda { |flag_result, acc| flagsmith_id = flag_result.dig(:metadata, :flagsmith_id) - if flagsmith_id.nil? raise Flagsmith::ClientError, - "FlagResult metadata.flagsmith_id is missing for feature \"#{flag_result[:name]}\". " \ - 'This indicates a bug in the SDK, please report it.' + "FlagResult metadata.flagsmith_id is missing for feature \"#{flag_result[:name]}\". This indicates a bug in the SDK, please report it." end - acc[flag_result[:name]] = Flagsmith::Flags::Flag.new( - feature_name: flag_result[:name], - enabled: flag_result[:enabled], - value: flag_result[:value], - feature_id: flagsmith_id, - reason: flag_result[:reason] - ) + acc[flag_result[:name]] = Collection.map_evaluated_flag_to_flag_result(flag_result, flagsmith_id) } - new( - evaluation_result[:flags].each_value.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, flagsmith_id) + Flagsmith::Flags::Flag.new( + feature_name: flag_result[:name], + enabled: flag_result[:enabled], + value: flag_result[:value], + feature_id: flagsmith_id, + reason: flag_result[:reason] ) end From 99f379c2d97213ade0a2297745887aa56227708c Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 11 Nov 2025 12:48:17 +0100 Subject: [PATCH 8/9] feat: replaced-flagsmith-id-with-id --- lib/flagsmith.rb | 4 ++-- lib/flagsmith/engine/mappers/environment.rb | 2 +- lib/flagsmith/engine/mappers/identity.rb | 2 +- lib/flagsmith/engine/mappers/segments.rb | 4 ++-- lib/flagsmith/sdk/models/flags.rb | 12 ++++++------ spec/engine/unit/evaluation_context_mappers_spec.rb | 6 +++--- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/flagsmith.rb b/lib/flagsmith.rb index 5532ec2..51ae62a 100644 --- a/lib/flagsmith.rb +++ b/lib/flagsmith.rb @@ -214,8 +214,8 @@ def get_identity_segments(identifier, traits = {}) evaluation_result = Flagsmith::Engine.get_evaluation_result(context) evaluation_result[:segments].filter_map do |segment_result| - flagsmith_id = segment_result.dig(:metadata, :flagsmith_id) - Flagsmith::Segments::Segment.new(id: flagsmith_id, name: segment_result[:name]) if flagsmith_id + id = segment_result.dig(:metadata, :id) + Flagsmith::Segments::Segment.new(id: id, name: segment_result[:name]) if id end 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/sdk/models/flags.rb b/lib/flagsmith/sdk/models/flags.rb index 4def197..59cea07 100644 --- a/lib/flagsmith/sdk/models/flags.rb +++ b/lib/flagsmith/sdk/models/flags.rb @@ -176,24 +176,24 @@ def from_api(json_data, **args) def from_evaluation_result(evaluation_result, **args) to_flag_object = lambda { |flag_result, acc| - flagsmith_id = flag_result.dig(:metadata, :flagsmith_id) - if flagsmith_id.nil? + id = flag_result.dig(:metadata, :id) + if id.nil? raise Flagsmith::ClientError, - "FlagResult metadata.flagsmith_id is missing for feature \"#{flag_result[:name]}\". This indicates a bug in the SDK, please report it." + "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, flagsmith_id) + acc[flag_result[:name]] = Collection.map_evaluated_flag_to_flag_result(flag_result, id) } new(evaluation_result[:flags].each_value.each_with_object({}, &to_flag_object), **args) end - def map_evaluated_flag_to_flag_result(flag_result, flagsmith_id) + 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: flagsmith_id, + feature_id: id, reason: flag_result[:reason] ) 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') From cefe981ae6351b75d683d5565db6ee925e47eeff Mon Sep 17 00:00:00 2001 From: wadii Date: Tue, 11 Nov 2025 14:59:05 +0100 Subject: [PATCH 9/9] feat: removed-comments --- lib/flagsmith/engine/core.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/flagsmith/engine/core.rb b/lib/flagsmith/engine/core.rb index 05d2cfc..dded5a5 100644 --- a/lib/flagsmith/engine/core.rb +++ b/lib/flagsmith/engine/core.rb @@ -26,7 +26,6 @@ module Engine # # @param evaluation_context [Hash] The evaluation context # @return [Hash] Evaluation result with flags and segments - # returns EvaluationResultWithMetadata def get_evaluation_result(evaluation_context) evaluation_context = get_enriched_context(evaluation_context) segments, segment_overrides = evaluate_segments(evaluation_context) @@ -70,7 +69,6 @@ def process_segment_overrides(identity_segments) # rubocop:disable Metrics/Metho segment_overrides end - # returns EvaluationResultFlags def evaluate_features(evaluation_context, segment_overrides) identity_key = get_identity_key(evaluation_context)