diff --git a/.github/workflows/pull_request.yaml b/.github/workflows/pull_request.yaml index ed95b9d..9880607 100644 --- a/.github/workflows/pull_request.yaml +++ b/.github/workflows/pull_request.yaml @@ -14,9 +14,6 @@ on: - synchronize - reopened - ready_for_review - branches: - - main - - release/** push: branches: 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.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/evaluation/core.rb b/lib/flagsmith/engine/evaluation/core.rb index 0ddc342..5a7fe58 100644 --- a/lib/flagsmith/engine/evaluation/core.rb +++ b/lib/flagsmith/engine/evaluation/core.rb @@ -1,20 +1,140 @@ # frozen_string_literal: true +require_relative '../utils/hash_func' +require_relative '../features/constants' +require_relative '../segments/evaluator' + module Flagsmith module Engine module Evaluation # Core evaluation logic for feature flags module Core + 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 self.get_evaluation_result(_evaluation_context) - # TODO: Implement core evaluation logic + 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: {}, - segments: [] + flags: flags, + segments: segments + } + end + + # Returns { segments: EvaluationResultSegments; segmentOverrides: Record; } + def evaluate_segments(evaluation_context) + return [], {} if evaluation_context[:segments].nil? + + identity_segments = get_segments_from_context(evaluation_context) + + segments = identity_segments.map do |segment| + { name: segment[:name], metadata: segment[:metadata] }.compact + end + + segment_overrides = process_segment_overrides(identity_segments) + + [segments, segment_overrides] + end + + # Returns Record + def process_segment_overrides(identity_segments) # rubocop:disable Metrics/MethodLength + segment_overrides = {} + + 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 + + segment_overrides + end + + def evaluate_features(evaluation_context, segment_overrides) + identity_key = get_identity_key(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 + + flag_result = build_flag_result(final_feature, identity_key, segment_override) + flags[final_feature[:name].to_sym] = flag_result + 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 + + { value: feature[:value], reason: nil } + end + + # 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 } + + 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] + return { value: variant[:value], reason: "#{TARGETING_REASON_SPLIT}; weight=#{variant[:weight]}" } if start_percentage <= percentage_value && percentage_value < limit + + start_percentage = limit + end + nil + 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) + + 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] || + "#{evaluation_context[:environment][:key]}_#{evaluation_context[:identity][:identifier]}" + end + + # returns boolean + 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 new file mode 100644 index 0000000..b3baa08 --- /dev/null +++ b/lib/flagsmith/engine/features/constants.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Flagsmith + module Engine + module Features + # Targeting reason constants for evaluation results + module TargetingReasons + TARGETING_REASON_DEFAULT = 'DEFAULT' + TARGETING_REASON_TARGETING_MATCH = 'TARGETING_MATCH' + TARGETING_REASON_SPLIT = 'SPLIT' + end + 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 cc4a2f0..c5d55cb 100644 --- a/lib/flagsmith/engine/segments/evaluator.rb +++ b/lib/flagsmith/engine/segments/evaluator.rb @@ -1,22 +1,65 @@ # frozen_string_literal: true +require 'json' +require 'jsonpath' require_relative 'constants' +require_relative 'models' require_relative '../utils/hash_func' module Flagsmith module Engine module Segments # Evaluator methods - module Evaluator + module Evaluator # rubocop:disable Metrics/ModuleLength include Flagsmith::Engine::Segments::Constants include Flagsmith::Engine::Utils::HashFunc - def get_identity_segments(environment, identity, override_traits = nil) - environment.project.segments.select do |s| - evaluate_identity_in_segment(identity, s, override_traits) + module_function + + def get_enriched_context(context) + identity_context = context[:identity] + return context unless identity_context + return context if identity_context[:key] + + enriched_context = context.dup + enriched_context[:identity] = identity_context.merge( + key: "#{context[:environment][:key]}_#{identity_context[:identifier]}" + ) + enriched_context + end + + # 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_segments_from_context(context) + return [] unless context[:identity] && context[:segments] + + context[:segments].values.select do |segment| + next false if segment[:rules].nil? || segment[:rules].empty? + + matches = segment[:rules].all? { |rule| traits_match_segment_rule_from_context(rule, segment[:key], context) } + matches 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 @@ -54,28 +97,149 @@ def traits_match_segment_rule(identity_traits, rule, 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 + # Context-based helper functions + + # Evaluates whether a segment rule matches using context + # + # @param rule [Hash] The rule to evaluate + # @param segment_key [String] The segment key (used for percentage split) + # @param context [Hash] The evaluation context + # @return [Boolean] True if the rule matches + def traits_match_segment_rule_from_context(rule, segment_key, context) + matches_conditions = evaluate_conditions_from_context(rule, segment_key, context) + matches_sub_rules = evaluate_sub_rules_from_context(rule, segment_key, context) + + matches_conditions && matches_sub_rules + end + + # Evaluates rule conditions based on rule type (ALL/ANY/NONE) + # + # @param rule [Hash] The rule containing conditions and type + # @param segment_key [String] The segment key + # @param context [Hash] The evaluation context + # @return [Boolean] True if conditions match according to rule type + def evaluate_conditions_from_context(rule, segment_key, context) + return true unless rule[:conditions]&.any? + + condition_results = rule[:conditions].map do |condition| + traits_match_segment_condition_from_context(condition, segment_key, context) end - trait = identity_traits.find { |t| t.key.to_s == condition.property } + evaluate_rule_conditions(rule[:type], condition_results) + end - return handle_trait_existence_conditions(trait, condition.operator) if [IS_SET, - IS_NOT_SET].include?(condition.operator) + # Evaluates nested sub-rules + # + # @param rule [Hash] The rule containing nested rules + # @param segment_key [String] The segment key + # @param context [Hash] The evaluation context + # @return [Boolean] True if all sub-rules match + def evaluate_sub_rules_from_context(rule, segment_key, context) + return true if rule[:rules].nil? || rule[:rules].empty? - return condition.match_trait_value?(trait.trait_value) if trait + rule[:rules].all? do |sub_rule| + traits_match_segment_rule_from_context(sub_rule, segment_key, context) + end + end - false + # Evaluates a single segment condition using context + # + # @param condition [Hash] The condition to evaluate + # @param segment_key [String] The segment key (used for percentage split hashing) + # @param context [Hash] The evaluation context + # @return [Boolean] True if the condition matches + def traits_match_segment_condition_from_context(condition, segment_key, context) + return handle_percentage_split(condition, segment_key, context) if condition[:operator] == PERCENTAGE_SPLIT + return false if condition[:property].nil? + + trait_value = get_trait_value(condition[:property], context) + evaluate_trait_condition(condition, trait_value) + end + + def handle_percentage_split(condition, segment_key, context) + context_value_key = get_context_value(condition[:property], context) || get_identity_key_from_context(context) + hashed_percentage = hashed_percentage_for_object_ids([segment_key, context_value_key]) + hashed_percentage <= condition[:value].to_f end - private + def evaluate_trait_condition(condition, trait_value) + return !trait_value.nil? if condition[:operator] == IS_SET + return trait_value.nil? if condition[:operator] == IS_NOT_SET + return false if trait_value.nil? - def handle_trait_existence_conditions(matching_trait, operator) - return operator == IS_NOT_SET if matching_trait.nil? + condition_obj = Flagsmith::Engine::Segments::Condition.new( + operator: condition[:operator], + value: condition[:value], + property: condition[:property] + ) + condition_obj.match_trait_value?(trait_value) + end + + # Evaluate rule conditions based on type (ALL/ANY/NONE) + # + # @param rule_type [String] The rule type + # @param condition_results [Array] Array of condition evaluation results + # @return [Boolean] True if conditions match according to rule type + def evaluate_rule_conditions(rule_type, condition_results) + case rule_type + when 'ALL' + condition_results.empty? || condition_results.all? + when 'ANY' + !condition_results.empty? && condition_results.any? + when 'NONE' + condition_results.empty? || condition_results.none? + else + false + end + end + + # Get trait value from context, supporting JSONPath expressions + # + # @param property [String] The property name or JSONPath + # @param context [Hash] The evaluation context + # @return [Object, nil] The trait value or nil + def get_trait_value(property, context) + if property.start_with?('$.') + context_value = get_context_value(property, context) + return context_value if !context_value.nil? && primitive?(context_value) + end + + traits = context.dig(:identity, :traits) || {} + traits[property] || traits[property.to_sym] + end + + # 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?('$.') + + results = JsonPath.new(json_path, use_symbols: true).on(context) + results.first + rescue StandardError + nil + end + + # Get identity key from context + # + # @param context [Hash] The evaluation context + # @return [String, nil] The identity key or generated composite key + def get_identity_key_from_context(context) + return nil unless context[:identity] + + context[:identity][:key] + end + + # Check if value is primitive (not an object or array) + # + # @param value [Object] The value to check + # @return [Boolean] True if value is primitive (not an object or array) + def primitive?(value) + return true if value.nil? - operator == IS_SET + !(value.is_a?(Hash) || value.is_a?(Array)) end end end diff --git a/lib/flagsmith/engine/segments/models.rb b/lib/flagsmith/engine/segments/models.rb index f339112..70a6d03 100644 --- a/lib/flagsmith/engine/segments/models.rb +++ b/lib/flagsmith/engine/segments/models.rb @@ -45,7 +45,7 @@ class Condition CONTAINS => ->(other_value, self_value) { (other_value || false) && other_value.include?(self_value) }, NOT_CONTAINS => ->(other_value, self_value) { (other_value || false) && !other_value.include?(self_value) }, - REGEX => ->(other_value, self_value) { (other_value || false) && other_value.match?(self_value) } + REGEX => ->(other_value, self_value) { (other_value || false) && other_value.to_s.match?(self_value) } }.freeze def initialize(operator:, value:, property: nil) @@ -55,30 +55,48 @@ def initialize(operator:, value:, property: nil) end def match_trait_value?(trait_value) - # handle some exceptions - trait_value = Semantic::Version.new(trait_value.gsub(/:semver$/, '')) if @value.is_a?(String) && @value.match?(/:semver$/) + trait_value = parse_semver_trait_value(trait_value) + return false if trait_value.nil? return match_in_value(trait_value) if @operator == IN return match_modulo_value(trait_value) if @operator == MODULO + return MATCHING_FUNCTIONS[REGEX]&.call(trait_value, @value) if @operator == REGEX + match_with_type_conversion(trait_value) + end + + def parse_semver_trait_value(trait_value) + return trait_value unless @value.is_a?(String) && @value.match?(/:semver$/) + + Semantic::Version.new(trait_value.to_s.gsub(/:semver$/, '')) + rescue ArgumentError, Semantic::Version::ValidationFailed + nil + end + + def match_with_type_conversion(trait_value) type_as_trait_value = format_to_type_of(trait_value) formatted_value = type_as_trait_value ? type_as_trait_value.call(@value) : @value - MATCHING_FUNCTIONS[operator]&.call(trait_value, formatted_value) end - # rubocop:disable Metrics/AbcSize + TYPE_CONVERTERS = { + 'String' => ->(v) { v.to_s }, + 'Semantic::Version' => ->(v) { Semantic::Version.new(v.to_s.gsub(/:semver$/, '')) }, + 'TrueClass' => ->(v) { ['True', 'true', 'TRUE', true, 1, '1'].include?(v) }, + 'FalseClass' => ->(v) { !['False', 'false', 'FALSE', false].include?(v) }, + 'Integer' => lambda { |v| + i = v.to_i + i.to_s == v.to_s ? i : v + }, + 'Float' => lambda { |v| + f = v.to_f + f.to_s == v.to_s ? f : v + } + }.freeze + def format_to_type_of(input) - { - 'String' => ->(v) { v.to_s }, - 'Semantic::Version' => ->(v) { Semantic::Version.new(v.to_s.gsub(/:semver$/, '')) }, - 'TrueClass' => ->(v) { ['True', 'true', 'TRUE', true, 1, '1'].include?(v) }, - 'FalseClass' => ->(v) { !['False', 'false', 'FALSE', false, 0, '0'].include?(v) }, - 'Integer' => ->(v) { v.to_i }, - 'Float' => ->(v) { v.to_f } - }[input.class.to_s] + TYPE_CONVERTERS[input.class.to_s] end - # rubocop:enable Metrics/AbcSize def match_modulo_value(trait_value) divisor, remainder = @value.split('|') @@ -88,9 +106,25 @@ def match_modulo_value(trait_value) end def match_in_value(trait_value) - return @value.split(',').include?(trait_value.to_s) if trait_value.is_a?(String) || trait_value.is_a?(Integer) + return false if invalid_in_value?(trait_value) + return @value.include?(trait_value.to_s) if @value.is_a?(Array) - false + parse_and_match_string_value(trait_value) + end + + def invalid_in_value?(trait_value) + trait_value.nil? || [TrueClass, FalseClass].any? { |klass| trait_value.is_a?(klass) } + end + + def parse_and_match_string_value(trait_value) # rubocop:disable Metrics/AbcSize + return @value.to_s.split(',').include?(trait_value.to_s) unless @value.is_a?(String) + + parsed = JSON.parse(@value) + return parsed.include?(trait_value.to_s) if parsed.is_a?(Array) + + @value.to_s.split(',').include?(trait_value.to_s) + rescue JSON::ParserError + @value.to_s.split(',').include?(trait_value.to_s) end class << self 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 77c9f92..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,12 +32,10 @@ def load_test_file(filepath) test_evaluation_context = test_case[:context] test_expected_result = test_case[:result] - # TODO: Implement evaluation logic - evaluation_result = {} + evaluation_result = Flagsmith::Engine.get_evaluation_result(test_evaluation_context) - - # TODO: Uncomment when evaluation is implemented - # expect(evaluation_result).to eq(test_expected_result) + expect(evaluation_result[:flags]).to eq(test_expected_result[:flags]) + expect(evaluation_result[:segments]).to eq(test_expected_result[:segments]) end end 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 diff --git a/spec/engine/unit/segments/models_spec.rb b/spec/engine/unit/segments/models_spec.rb index 517657a..1c50637 100644 --- a/spec/engine/unit/segments/models_spec.rb +++ b/spec/engine/unit/segments/models_spec.rb @@ -94,9 +94,7 @@ ['IN', 1, '1,2,3,4', true], ['IN', 1, '', false], ['IN', 1, '1', true], - # Flagsmith's engine does not evaluate `IN` condition for floats/doubles and booleans - # due to ambiguous serialization across supported platforms. - ['IN', 1.5, '1.5', false], + ['IN', 1.5, '1.5', true], ['IN', false, 'false', false], ].freeze