From 93ae4a54f3ef96bb21c8fca54bed84d705d96a0a Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Fri, 30 Oct 2026 16:36:46 -0300 Subject: [PATCH 1/2] feat: Evaluate flags that depend on other flags Segment conditions can read another flag's evaluated result through `$.flags..enabled`, `.value` or `.variant`, as in flagsmith-engine. Flags are resolved lazily and transitively; a flag in a dependency cycle is served its default with reason `ERROR; code=CIRCULAR_DEPENDENCY`. Also aligns with engine-test-data v3.12.1: - Segments are evaluated without an identity. - Every flag result carries a `variant`. - Features without segment overrides report `STATIC`. - Overrides without a priority rank last instead of raising `NameError`. Local `get_environment_flags` now applies segments that need no identity (`$.environment.*`, `$.flags.*`), as the Flagsmith API does, and skips mapping identity overrides. Co-Authored-By: Claude Opus 5.5 (1M context) --- .gitmodules | 2 +- lib/flagsmith.rb | 5 +- lib/flagsmith/engine/core.rb | 116 ++++++++++------- lib/flagsmith/engine/features/constants.rb | 2 + lib/flagsmith/engine/lazy_flags.rb | 118 ++++++++++++++++++ lib/flagsmith/engine/mappers.rb | 31 +++++ lib/flagsmith/engine/segments/constants.rb | 3 + lib/flagsmith/engine/segments/evaluator.rb | 23 ++-- spec/engine-test-data | 2 +- .../unit/evaluation_context_mappers_spec.rb | 19 +++ 10 files changed, 267 insertions(+), 54 deletions(-) create mode 100644 lib/flagsmith/engine/lazy_flags.rb diff --git a/.gitmodules b/.gitmodules index 72eee3a..535b389 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,7 +1,7 @@ [submodule "spec/engine-test-data"] path = spec/engine-test-data url = git@github.com:Flagsmith/engine-test-data.git - branch = v3.7.0 + branch = v3.12.1 [submodule "spec/sdk-benchmarks"] path = spec/sdk-benchmarks url = https://github.com/Flagsmith/sdk-benchmarks.git diff --git a/lib/flagsmith.rb b/lib/flagsmith.rb index 51ae62a..3032c0b 100644 --- a/lib/flagsmith.rb +++ b/lib/flagsmith.rb @@ -222,13 +222,12 @@ def get_identity_segments(identifier, traits = {}) private def environment_flags_from_document # rubocop:disable Metrics/MethodLength - context = Flagsmith::Engine::Mappers.get_evaluation_context(environment) - - unless context + unless environment raise Flagsmith::ClientError, 'Unable to get flags. No environment present.' end + context = Flagsmith::Engine::Mappers.get_environment_flags_evaluation_context(environment) evaluation_result = Flagsmith::Engine.get_evaluation_result(context) Flagsmith::Flags::Collection.from_evaluation_result( diff --git a/lib/flagsmith/engine/core.rb b/lib/flagsmith/engine/core.rb index dded5a5..c123f46 100644 --- a/lib/flagsmith/engine/core.rb +++ b/lib/flagsmith/engine/core.rb @@ -2,11 +2,13 @@ require 'semantic' require 'securerandom' +require 'set' require_relative 'environments/models' require_relative 'features/models' require_relative 'features/constants' require_relative 'identities/models' +require_relative 'lazy_flags' require_relative 'organisations/models' require_relative 'projects/models' require_relative 'segments/evaluator' @@ -16,7 +18,7 @@ module Flagsmith # Core evaluation logic for feature flags - module Engine + module Engine # rubocop:disable Metrics/ModuleLength extend self include Flagsmith::Engine::Utils::HashFunc include Flagsmith::Engine::Features::TargetingReasons @@ -27,9 +29,11 @@ module Engine # @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) + lazy_flags = LazyFlags.new(get_enriched_context(evaluation_context)) + evaluation_context = lazy_flags.context + segments, segment_overrides = evaluate_segments(evaluation_context, lazy_flags) flags = evaluate_features(evaluation_context, segment_overrides) + lazy_flags.apply_to(flags) if lazy_flags.used? { flags: flags, segments: segments @@ -37,27 +41,30 @@ def get_evaluation_result(evaluation_context) end # Returns { segments: EvaluationResultSegments; segmentOverrides: Record; } - def evaluate_segments(evaluation_context) + def evaluate_segments(evaluation_context, lazy_flags) return [], {} if evaluation_context[:segments].nil? - identity_segments = get_segments_from_context(evaluation_context) + matching_segments = evaluation_context[:segments].each_value.select do |segment| + cycle_hits = lazy_flags.cycle_hits + context_in_segment?(evaluation_context, segment) && lazy_flags.cycle_hits == cycle_hits + end - segments = identity_segments.map do |segment| + segments = matching_segments.map do |segment| { name: segment[:name], metadata: segment[:metadata] }.compact end - segment_overrides = process_segment_overrides(identity_segments) + segment_overrides = process_segment_overrides(matching_segments) [segments, segment_overrides] end # Returns Record - def process_segment_overrides(identity_segments) # rubocop:disable Metrics/MethodLength + def process_segment_overrides(matching_segments) # rubocop:disable Metrics/MethodLength segment_overrides = {} - identity_segments.each do |segment| + matching_segments.each do |segment| Array(segment[:overrides]).each do |override| - next unless should_apply_override(override, segment_overrides) + next unless wins_over?(override, segment_overrides[override[:name]]&.dig(:feature)) segment_overrides[override[:name]] = { feature: override, @@ -71,65 +78,94 @@ def process_segment_overrides(identity_segments) # rubocop:disable Metrics/Metho def evaluate_features(evaluation_context, segment_overrides) identity_key = get_identity_key(evaluation_context) + targeted_feature_names = get_targeted_feature_names(evaluation_context) (evaluation_context[:features] || {}).each_with_object({}) do |(_, feature), flags| - segment_override = segment_overrides[feature[:name]] - final_feature = segment_override ? segment_override[:feature] : feature + flags[feature[:name].to_sym] = + get_feature_flag_result(feature, segment_overrides[feature[:name]], identity_key, targeted_feature_names) + end + end + + # Returns the flag result of a feature, served by its segment override if any, or else by its environment default + def get_feature_flag_result(feature, segment_override, identity_key, targeted_feature_names) + return get_flag_result(segment_override[:feature], identity_key, targeting_match_reason(segment_override)) if segment_override + + get_flag_result(feature, identity_key, fallback_reason(feature[:name], targeted_feature_names)) + end - flag_result = build_flag_result(final_feature, identity_key, segment_override) - flags[final_feature[:name].to_sym] = flag_result + # Returns the names of the features overridden by at least one segment + def get_targeted_feature_names(evaluation_context) + names = {} + evaluation_context[:segments]&.each_value do |segment| + segment[:overrides]&.each { |override| names[override[:name]] = true } end + names + end + + # Returns the reason for a flag served by a segment override + def targeting_match_reason(segment_override) + "#{TARGETING_REASON_TARGETING_MATCH}; segment=#{segment_override[:segment_name]}" + end + + # Returns the reason for a flag served by its environment default: + # DEFAULT if the feature has segment overrides that did not match, STATIC if it has none + def fallback_reason(feature_name, targeted_feature_names) + targeted_feature_names.key?(feature_name) ? TARGETING_REASON_DEFAULT : TARGETING_REASON_STATIC end - # Returns {value: any; reason?: string} + # Returns {name: string; enabled: boolean; value: any; reason: string; variant?: string; metadata?: any} + def get_flag_result(feature, identity_key, reason) + evaluated = evaluate_feature_value(feature, identity_key) + + flag_result = { + name: feature[:name], + enabled: feature[:enabled], + value: evaluated[:value], + reason: evaluated[:reason] || reason, + variant: evaluated[:variant] + } + + flag_result[:metadata] = feature[:metadata] if feature[:metadata] + flag_result + end + + # Returns {value: any; reason?: string; variant?: 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 } + { value: feature[:value], reason: nil, variant: nil } end - # Returns {value: any; reason?: string} + # Returns {value: any; reason?: string; variant: 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 } + sorted_variants = (feature[:variants] || []).sort_by { |v| v[:priority] || DEFAULT_PRIORITY } variant = find_matching_variant(sorted_variants, percentage_value) - variant || { value: feature[:value], reason: nil } + variant || { value: feature[:value], reason: nil, variant: 'control' } end def find_matching_variant(sorted_variants, percentage_value) start_percentage = 0 sorted_variants.each do |variant| limit = start_percentage + variant[:weight] - return { value: variant[:value], reason: "#{TARGETING_REASON_SPLIT}; weight=#{variant[:weight]}" } if start_percentage <= percentage_value && percentage_value < limit + if start_percentage <= percentage_value && percentage_value < limit + return { value: variant[:value], reason: "#{TARGETING_REASON_SPLIT}; weight=#{variant[:weight]}", variant: variant[:key] } + end 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]) + # Whether a segment override takes precedence over the one held so far. + # The lowest priority wins, and the first seen wins a tie. + def wins_over?(candidate, incumbent) + incumbent.nil? || (candidate[:priority] || DEFAULT_PRIORITY) < (incumbent[:priority] || DEFAULT_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 @@ -139,9 +175,5 @@ def get_identity_key(evaluation_context) evaluation_context[:identity][:key] end - - def stronger_priority?(priority_a, priority_b) - (priority_a || WEAKEST_PRIORITY) < (priority_b || WEAKEST_PRIORITY) - end end end diff --git a/lib/flagsmith/engine/features/constants.rb b/lib/flagsmith/engine/features/constants.rb index b3baa08..ce566ea 100644 --- a/lib/flagsmith/engine/features/constants.rb +++ b/lib/flagsmith/engine/features/constants.rb @@ -6,8 +6,10 @@ module Features # Targeting reason constants for evaluation results module TargetingReasons TARGETING_REASON_DEFAULT = 'DEFAULT' + TARGETING_REASON_STATIC = 'STATIC' TARGETING_REASON_TARGETING_MATCH = 'TARGETING_MATCH' TARGETING_REASON_SPLIT = 'SPLIT' + TARGETING_REASON_CIRCULAR_DEPENDENCY = 'ERROR; code=CIRCULAR_DEPENDENCY' end end end diff --git a/lib/flagsmith/engine/lazy_flags.rb b/lib/flagsmith/engine/lazy_flags.rb new file mode 100644 index 0000000..adfa0fc --- /dev/null +++ b/lib/flagsmith/engine/lazy_flags.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'set' + +module Flagsmith + module Engine + # The `$.flags` of an evaluation context, resolving a flag the first time a segment condition reads it. + # + # Flags are only reachable by name: JSONPath reads them through `dig`. + class LazyFlags + # The evaluation context, with `$.flags` bound to this instance + attr_reader :context + + # Incremented whenever a dependency cycle is cut. A segment whose evaluation increments it + # read a flag that could not be resolved, so it must not match. + attr_reader :cycle_hits + + def initialize(context) + @context = context.merge(flags: self) + @cycle_hits = 0 + end + + # @param feature_name [String] + # @return [Hash, nil] The flag result, or nil for a feature absent from the context or in a cycle + def dig(feature_name) + start unless used? + resolve(feature_name) unless @resolved.key?(feature_name) + @resolved[feature_name] + end + + # Whether a segment condition read any flag + def used? + !@resolved.nil? + end + + # Applies the flags resolved for segment conditions over the single-pass flag results + # + # @param flags [Hash{Symbol => Hash}] The flag results, keyed by feature name + def apply_to(flags) + @resolved.each_value { |flag| flags[flag[:name].to_sym] = flag } + @cyclic.each do |feature_name| + flag = flags[feature_name.to_sym] + flag[:reason] = Features::TargetingReasons::TARGETING_REASON_CIRCULAR_DEPENDENCY if flag + end + end + + private + + def start + @resolved = {} + @cyclic = Set.new + @resolving = [] + @segment_matches = {} + @segment_keys_by_feature_name = {} + (@context[:segments] || {}).each do |segment_key, segment| + Array(segment[:overrides]).each do |override| + (@segment_keys_by_feature_name[override[:name]] ||= []) << segment_key + end + end + end + + def resolve(feature_name) # rubocop:disable Metrics/MethodLength + if @resolving.include?(feature_name) + @cyclic.merge(@resolving[@resolving.index(feature_name)..]) + @cycle_hits += 1 + return + end + + feature = find_feature(feature_name) + return unless feature + + @resolving.push(feature_name) + begin + segment_override = find_segment_override(feature_name) + ensure + @resolving.pop + end + return if @cyclic.include?(feature_name) + + @resolved[feature_name] = Engine.get_feature_flag_result( + feature, segment_override, @context.dig(:identity, :key), @segment_keys_by_feature_name + ) + end + + def find_feature(feature_name) + features = @context[:features] + return nil unless features + + features[feature_name] || features[feature_name.to_sym] + end + + def find_segment_override(feature_name) + segment_override = nil + @segment_keys_by_feature_name.fetch(feature_name, []).each do |segment_key| + next unless segment_match?(segment_key) + + segment = @context[:segments][segment_key] + segment[:overrides].each do |override| + next unless override[:name] == feature_name && Engine.wins_over?(override, segment_override&.dig(:feature)) + + segment_override = { feature: override, segment_name: segment[:name] } + end + end + segment_override + end + + def segment_match?(segment_key) + return @segment_matches[segment_key] if @segment_matches.key?(segment_key) + + cycle_hits = @cycle_hits + matches = Segments::Evaluator.context_in_segment?(@context, @context[:segments][segment_key]) + return false if @cycle_hits != cycle_hits # Rests on a flag that could not be resolved + + @segment_matches[segment_key] = matches + end + end + end +end diff --git a/lib/flagsmith/engine/mappers.rb b/lib/flagsmith/engine/mappers.rb index 612a84c..dc448bc 100644 --- a/lib/flagsmith/engine/mappers.rb +++ b/lib/flagsmith/engine/mappers.rb @@ -11,6 +11,9 @@ module Mappers STRONGEST_PRIORITY = Float::INFINITY WEAKEST_PRIORITY = -Float::INFINITY + # Condition properties resolvable without an identity + IDENTITY_FREE_PROPERTY_PREFIXES = ['$.environment.', '$.environment[', '$.flags.', '$.flags['].freeze + # @param environment [Flagsmith::Engine::Environment] The environment model # @param identity [Flagsmith::Engine::Identity, nil] Optional identity model # @param override_traits [Array, nil] Optional override traits @@ -37,6 +40,34 @@ def self.map_environment_model_to_evaluation_context(environment) context end + # Maps environment model to an evaluation context for flags served without an identity + # + # @param environment [Flagsmith::Engine::Environment] The environment model + # @return [Hash] Context with :environment, :features, and the :segments not needing an identity + def self.get_environment_flags_evaluation_context(environment) + { + environment: Environment.build_environment_context(environment), + features: Environment.build_features_context(environment.feature_states), + segments: select_identity_free_segments(Segments.build_segments_context(environment.project.segments)) + } + end + + # Selects the segments that can be evaluated without an identity, i.e. reading neither identity nor traits + # + # @param segments [Hash] Segments context, keyed by segment key + # @return [Hash] Segments context without the segments needing an identity + def self.select_identity_free_segments(segments) + segments.select { |_, segment| identity_free_rules?(segment[:rules]) } + end + + def self.identity_free_rules?(rules) + Array(rules).all? do |rule| + Array(rule[:conditions]).all? { |condition| condition[:property].to_s.start_with?(*IDENTITY_FREE_PROPERTY_PREFIXES) } && + identity_free_rules?(rule[:rules]) + end + end + private_class_method :identity_free_rules? + # Maps identity model to identity context # # @param identity [Flagsmith::Engine::Identity] The identity model diff --git a/lib/flagsmith/engine/segments/constants.rb b/lib/flagsmith/engine/segments/constants.rb index 5ecb0c2..ef15143 100644 --- a/lib/flagsmith/engine/segments/constants.rb +++ b/lib/flagsmith/engine/segments/constants.rb @@ -11,6 +11,9 @@ module Constants RULE_TYPES = [ALL_RULE, ANY_RULE, NONE_RULE].freeze + # Segment override and variant priority: the lowest wins + DEFAULT_PRIORITY = Float::INFINITY + # Segment Condition Operators EQUAL = 'EQUAL' GREATER_THAN = 'GREATER_THAN' diff --git a/lib/flagsmith/engine/segments/evaluator.rb b/lib/flagsmith/engine/segments/evaluator.rb index 1deaadf..70258fe 100644 --- a/lib/flagsmith/engine/segments/evaluator.rb +++ b/lib/flagsmith/engine/segments/evaluator.rb @@ -14,6 +14,9 @@ module Evaluator # rubocop:disable Metrics/ModuleLength include Flagsmith::Engine::Segments::Constants include Flagsmith::Engine::Utils::HashFunc + # Compiled JSONPath queries, keyed by condition property + JSON_PATHS = Hash.new { |json_paths, json_path| json_paths[json_path] = JsonPath.new(json_path, use_symbols: true) } + module_function def get_enriched_context(context) @@ -36,12 +39,19 @@ def get_enriched_context(context) 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? + context[:segments].values.select { |segment| context_in_segment?(context, segment) } + end - matches = segment[:rules].all? { |rule| traits_match_segment_rule_from_context(rule, segment[:key], context) } - matches - end + # Whether the context belongs to a segment, i.e. matches all of its rules + # + # @param context [Hash] The evaluation context + # @param segment [Hash] The segment context + # @return [Boolean] False for a segment without rules + def context_in_segment?(context, segment) + rules = segment[:rules] + return false if rules.nil? || rules.empty? + + rules.all? { |rule| traits_match_segment_rule_from_context(rule, segment[:key], context) } end def traits_match_segment_condition(identity_traits, condition, segment_id, identity_id) @@ -227,8 +237,7 @@ def get_trait_value(property, context) def get_context_value(json_path, context) return nil unless context && json_path&.start_with?('$.') - results = JsonPath.new(json_path, use_symbols: true).on(context) - results.first + JSON_PATHS[json_path].first(context) rescue StandardError nil end diff --git a/spec/engine-test-data b/spec/engine-test-data index 4b29dc7..cb06740 160000 --- a/spec/engine-test-data +++ b/spec/engine-test-data @@ -1 +1 @@ -Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f +Subproject commit cb067401093766e55c3354c1de7c73d9331334df diff --git a/spec/engine/unit/evaluation_context_mappers_spec.rb b/spec/engine/unit/evaluation_context_mappers_spec.rb index ce422b7..2c92dca 100644 --- a/spec/engine/unit/evaluation_context_mappers_spec.rb +++ b/spec/engine/unit/evaluation_context_mappers_spec.rb @@ -273,4 +273,23 @@ }) end end + + describe '.select_identity_free_segments' do + it 'keeps only the segments that can be evaluated without an identity' do + # Given + segments = { + 'on_environment' => { rules: [{ type: 'ALL', conditions: [{ property: '$.environment.name', operator: 'EQUAL', value: 'Production' }], rules: [] }] }, + 'on_flag' => { rules: [{ type: 'ANY', conditions: [{ property: '$.flags["payments"].enabled', operator: 'NOT_EQUAL', value: 'true' }], rules: [] }] }, + 'on_nested_trait' => { rules: [{ type: 'ALL', conditions: [], rules: [{ type: 'ANY', conditions: [{ property: 'plan', operator: 'IS_NOT_SET', value: nil }], rules: [] }] }] }, + 'on_identifier' => { rules: [{ type: 'ALL', conditions: [{ property: '$.identity.identifier', operator: 'IN', value: ['user-1'] }], rules: [] }] }, + 'on_identity_split' => { rules: [{ type: 'ALL', conditions: [{ property: nil, operator: 'PERCENTAGE_SPLIT', value: '50' }], rules: [] }] } + } + + # When + identity_free_segments = described_class.select_identity_free_segments(segments) + + # Then + expect(identity_free_segments.keys).to eq(%w[on_environment on_flag]) + end + end end From ca4660009d06ab918f9993d0defb889a02c8b8bd Mon Sep 17 00:00:00 2001 From: Evandro Myller Date: Fri, 30 Oct 2026 16:36:47 -0300 Subject: [PATCH 2/2] ci: Run sdk-spec v0.7.0 with local evaluation scenarios Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/sdk-spec.yml | 5 +++-- sdk-spec/steps.rb | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/sdk-spec.yml b/.github/workflows/sdk-spec.yml index 270f0d3..448dac9 100644 --- a/.github/workflows/sdk-spec.yml +++ b/.github/workflows/sdk-spec.yml @@ -11,7 +11,8 @@ permissions: jobs: sdk-spec: # Keep the ref in `uses:` and `spec-ref` in sync. - uses: Flagsmith/sdk-spec/.github/workflows/run-spec.yml@v0.3.0 + uses: Flagsmith/sdk-spec/.github/workflows/run-spec.yml@v0.7.0 with: - spec-ref: v0.3.0 + spec-ref: v0.7.0 + tags: core or local-evaluation language: ruby diff --git a/sdk-spec/steps.rb b/sdk-spec/steps.rb index aed63d9..d1c35de 100644 --- a/sdk-spec/steps.rb +++ b/sdk-spec/steps.rb @@ -17,6 +17,11 @@ def create_client(environment_key, api_url) Flagsmith::Client.new(environment_key: environment_key, api_url: api_url) end +# Create the SDK client with local evaluation +def create_client_with_local_evaluation(environment_key, api_url) + Flagsmith::Client.new(environment_key: environment_key, api_url: api_url, enable_local_evaluation: true) +end + # When the application gets the environment flags def get_environment_flags(flagsmith) as_spec_flags(flagsmith.get_environment_flags)