diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index f620cbd..d83de1e 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -27,7 +27,7 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Set up Ruby uses: ruby/setup-ruby@v1 @@ -45,7 +45,7 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Set up Ruby uses: ruby/setup-ruby@v1 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4a81d06..5503978 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -19,7 +19,7 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Set up Ruby uses: ruby/setup-ruby@v1 diff --git a/Gemfile.lock b/Gemfile.lock index 617d108..b6c5195 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - featurevisor (1.1.0) + featurevisor (2.0.0) benchmark (>= 0, < 1) GEM diff --git a/LICENSE b/LICENSE index 77b1f48..f94e553 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ The MIT License (MIT) -Copyright (c) 2025 Fahad Heylaal (https://fahad19.com) +Copyright (c) 2026 Fahad Heylaal (https://fahad19.com) Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/README.md b/README.md index af50c17..bb845a0 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,8 @@ f = Featurevisor.create_featurevisor( Most applications only need this factory and the returned `Featurevisor::Instance`. Public extension and observability APIs include modules, diagnostics, events, and the datafile structures accepted by the factory. +Concurrent evaluations are safe after an instance is configured. Do not mutate or close the same instance concurrently with evaluations. Serialize calls to `set_datafile`, `set_context`, `set_sticky`, `add_module`, `remove_module`, and `close`. Module, event, and diagnostic callbacks must synchronize mutable state that they capture. + ## Initialization The SDK can be initialized by passing [datafile](https://featurevisor.com/docs/building-datafiles/) content directly: @@ -697,6 +699,8 @@ f.remove_module('my-custom-module') ## Child instance +A child snapshots the parent keys that exist when it is spawned. Child values win for those keys. Parent keys introduced later are still inherited. Calling `close` removes both child-owned listeners and subscriptions delegated to the parent. + When dealing with purely client-side applications, it is understandable that there is only one user involved, like in browser or mobile applications. But when using Featurevisor SDK in server-side applications, where a single server instance can handle multiple user requests simultaneously, it is important to isolate the context for each request. @@ -722,8 +726,11 @@ Similar to parent SDK, child instances also support several additional methods: - `set_context` - `set_sticky` +- `evaluate_flag` - `is_enabled` +- `evaluate_variation` - `get_variation` +- `evaluate_variable` - `get_variable` - `get_variable_boolean` - `get_variable_string` @@ -821,7 +828,7 @@ The provider currently requires Ruby 3.4 or newer because that is the minimum ve Install the provider: ```ruby -gem "featurevisor-openfeature", "~> 1.1" +gem "featurevisor-openfeature", "~> 2.0" ``` It installs the matching `featurevisor` gem and the official `openfeature-sdk` dependency. The provider and base SDK deliberately share the same version, and the provider requires that exact Featurevisor version. @@ -901,7 +908,7 @@ The build produces `featurevisor-VERSION.gem` and `featurevisor-openfeature-VERS - Run `bundle install` - Push commit to `main` branch - Wait for CI to complete -- Tag the release with the same version number, for example `v1.1.0` +- Tag the release with the same version number, for example `v2.0.0` - The workflow verifies that the tag matches the shared version - The workflow publishes `featurevisor` first, followed by `featurevisor-openfeature` diff --git a/bin/commands/test.rb b/bin/commands/test.rb index e6e3856..ca7b717 100644 --- a/bin/commands/test.rb +++ b/bin/commands/test.rb @@ -671,7 +671,7 @@ def run_test_segment(assertion, segment, level) if assertion.key?(:expectedToMatch) expected_to_match = assertion[:expectedToMatch] - actual = instance.instance_variable_get(:@datafile_reader).all_conditions_are_matched(conditions, context) + actual = instance.instance_variable_get(:@datafile).all_conditions_are_matched(conditions, context) if actual != expected_to_match has_error = true diff --git a/conformance/sdk-v3.json b/conformance/sdk-v3.json index ceae692..49396ce 100644 --- a/conformance/sdk-v3.json +++ b/conformance/sdk-v3.json @@ -1,5 +1,5 @@ { - "version": 1, + "version": 2, "description": "Featurevisor v3 cross SDK compatibility contracts", "bucketing": { "minimum": 0, @@ -26,7 +26,47 @@ "pattern": "chrome", "flags": "g", "values": ["chrome", "chrome", "firefox", "chrome"], - "matches": [true, true, false, true] + "matches": [true, true, false, true], + "portableCases": [ + { + "pattern": "^chrome$", + "flags": "", + "value": "chrome", + "expected": true + }, + { + "pattern": "^(chrome|firefox)$", + "flags": "i", + "value": "Firefox", + "expected": true + }, + { + "pattern": "^second$", + "flags": "m", + "value": "first\nsecond", + "expected": true + }, + { + "pattern": "first.*second", + "flags": "s", + "value": "first\nsecond", + "expected": true + }, + { + "pattern": "\\(literal\\)", + "flags": "g", + "value": "(literal)", + "expected": true + } + ], + "rejectedSyntax": [ + "foo(?=bar)", + "(?<=foo)bar", + "(?:foo|bar)", + "(?foo)", + "(foo)\\1", + "foo++" + ] }, "typedVariables": [ { "type": "integer", "value": 1, "valid": true }, @@ -44,6 +84,135 @@ "diagnostics": { "requiredFields": ["level", "code", "message", "details"], "detailsType": "object", - "emptyDetailsJson": "{}" + "emptyDetailsJson": "{}", + "evaluationDetailFields": ["featureKey", "variableKey", "reason", "evaluation"], + "moduleEnvelopeFields": ["module", "moduleName", "originalError"], + "errorEventLevels": ["error"] + }, + "numericBucketKeys": [ + { "value": 1.2345678901234567, "expected": "1.2345678901234567" }, + { "value": 0.30000000000000004, "expected": "0.30000000000000004" }, + { "value": 0.000001, "expected": "0.000001" }, + { "value": 1e-7, "expected": "1e-7" }, + { "value": 100000000000000000000, "expected": "100000000000000000000" }, + { "value": 1e21, "expected": "1e+21" } + ], + "portableConditions": { + "regexFlags": ["g", "i", "m", "s"], + "rejectedRegexFlags": ["d", "u", "v", "y"], + "dateFormat": "ISO 8601 with an explicit timezone", + "dates": [ + "2024-01-01T00:00:00Z", + "2024-01-01T01:00:00+01:00", + "2024-01-01T00:00:00.250Z", + "2024-01-01T01:00:00.250+01:00" + ], + "semanticVersions": [ + "1.2.3", + "1.2.3-beta.1", + "1.2.3+build.5" + ], + "invalidSemanticVersion": "invalid", + "invalidSemanticVersionDiagnosticCode": "condition_match_error" + }, + "conditionCases": [ + { + "name": "strict primitive equality", + "condition": { + "attribute": "value", + "operator": "equals", + "value": 1 + }, + "context": { "value": "1" }, + "expected": false + }, + { + "name": "not negates implicit and", + "condition": { + "not": [ + { "attribute": "country", "operator": "equals", "value": "us" }, + { "attribute": "device", "operator": "equals", "value": "mobile" } + ] + }, + "context": { "country": "us", "device": "desktop" }, + "expected": true + }, + { + "name": "not with nested or means none match", + "condition": { + "not": [ + { + "or": [ + { "attribute": "country", "operator": "equals", "value": "us" }, + { "attribute": "country", "operator": "equals", "value": "nl" } + ] + } + ] + }, + "context": { "country": "de" }, + "expected": true + }, + { + "name": "empty not fails defensively", + "condition": { "not": [] }, + "context": {}, + "expected": false + }, + { + "name": "fractional ISO date with offset", + "condition": { + "attribute": "date", + "operator": "before", + "value": "2024-01-01T00:00:00.500Z" + }, + "context": { "date": "2024-01-01T01:00:00.250+01:00" }, + "expected": true + } + ], + "childInstances": { + "contextModel": "snapshot existing parent keys at spawn, inherit newly introduced parent keys, child keys win", + "closeRemovesLocalAndDelegatedSubscriptions": true, + "detailedEvaluationMethods": ["flag", "variation", "variable"], + "contextCase": { + "parentAtSpawn": { "country": "nl", "plan": "free" }, + "child": { "country": "de" }, + "parentAfterSpawn": { "country": "us", "plan": "pro", "region": "eu" }, + "expected": { "country": "de", "plan": "free", "region": "eu" } + } + }, + "defaults": { + "presenceBased": true, + "values": ["", 0, false, null], + "aggregateEvaluationPreservesEmptyVariation": true, + "aggregateCase": { + "datafile": { + "schemaVersion": "2", + "revision": "defaults", + "segments": {}, + "features": { + "experiment": { + "key": "experiment", + "bucketBy": "userId", + "variations": [{ "value": "control" }], + "traffic": [] + } + } + }, + "defaultVariationValue": "", + "expected": { + "enabled": false, + "variation": "" + } + } + }, + "diagnosticCase": { + "featureKey": "missing", + "expectedLevel": "warn", + "expectedCode": "feature_not_found", + "detailsMustBeObject": true + }, + "nativeContexts": { + "numericTypesUseOneComparisonContract": true, + "primitiveNativeSlicesSupportIncludes": true } } diff --git a/gemfiles/base.gemfile.lock b/gemfiles/base.gemfile.lock index 711ad38..4c9f890 100644 --- a/gemfiles/base.gemfile.lock +++ b/gemfiles/base.gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - featurevisor (1.1.0) + featurevisor (2.0.0) benchmark (>= 0, < 1) GEM diff --git a/lib/featurevisor.rb b/lib/featurevisor.rb index c1a7725..3fbe9dc 100644 --- a/lib/featurevisor.rb +++ b/lib/featurevisor.rb @@ -1,10 +1,10 @@ require_relative "featurevisor/version" require_relative "featurevisor/murmurhash" require_relative "featurevisor/compare_versions" -require_relative "featurevisor/logger" +require_relative "featurevisor/diagnostics" require_relative "featurevisor/emitter" require_relative "featurevisor/conditions" -require_relative "featurevisor/datafile_reader" +require_relative "featurevisor/evaluation_data_provider" require_relative "featurevisor/bucketer" require_relative "featurevisor/modules" require_relative "featurevisor/evaluate" @@ -15,6 +15,6 @@ module Featurevisor class Error < StandardError; end - private_constant :DatafileReader - private_constant :Logger + private_constant :InstanceEvaluationDataProvider + private_constant :DiagnosticReporter end diff --git a/lib/featurevisor/bucketer.rb b/lib/featurevisor/bucketer.rb index 6b75e2d..1408a62 100644 --- a/lib/featurevisor/bucketer.rb +++ b/lib/featurevisor/bucketer.rb @@ -30,16 +30,16 @@ def self.get_bucketed_number(bucket_key) # - feature_key [String] The feature key # - bucket_by [String, Array, Hash] Bucketing strategy # - context [Hash] User context - # - logger [Logger] Logger instance + # - diagnostics [DiagnosticReporter] Diagnostic reporter # @return [String] The bucket key # @raise [StandardError] If bucket_by is invalid def self.get_bucket_key(options) feature_key = options[:feature_key] bucket_by = options[:bucket_by] context = options[:context] - logger = options[:logger] + diagnostics = options[:diagnostics] - type, attribute_keys = parse_bucket_by(bucket_by, logger, feature_key) + type, attribute_keys = parse_bucket_by(bucket_by, diagnostics, feature_key) bucket_key = build_bucket_key(attribute_keys, context, type, feature_key) @@ -50,10 +50,10 @@ def self.get_bucket_key(options) # Parse bucket_by configuration to determine type and attribute keys # @param bucket_by [String, Array, Hash] Bucketing strategy - # @param logger [Logger] Logger instance + # @param diagnostics [DiagnosticReporter] Diagnostic reporter # @param feature_key [String] Feature key for error logging # @return [Array] Tuple of [type, attribute_keys] - def self.parse_bucket_by(bucket_by, logger, feature_key) + def self.parse_bucket_by(bucket_by, diagnostics, feature_key) if bucket_by.is_a?(String) ["plain", [bucket_by]] elsif bucket_by.is_a?(Array) @@ -61,7 +61,7 @@ def self.parse_bucket_by(bucket_by, logger, feature_key) elsif bucket_by.is_a?(Hash) && bucket_by[:or].is_a?(Array) ["or", bucket_by[:or]] else - logger.error("invalid bucketBy", { feature_key: feature_key, bucket_by: bucket_by }) + diagnostics.error("invalid bucketBy", { feature_key: feature_key, bucket_by: bucket_by }) raise StandardError, "invalid bucketBy" end end @@ -78,7 +78,7 @@ def self.build_bucket_key(attribute_keys, context, type, feature_key) attribute_keys.each do |attribute_key| attribute_value = Featurevisor::Conditions.get_value_from_context(context, attribute_key) - next if attribute_value.nil? + next if attribute_value.nil? && !context_path_exists?(context, attribute_key) if type == "plain" || type == "and" bucket_key << attribute_value @@ -89,7 +89,61 @@ def self.build_bucket_key(attribute_keys, context, type, feature_key) end bucket_key << feature_key - bucket_key + bucket_key.map { |value| javascript_string(value) } end + + def self.context_path_exists?(context, path) + path.split(".").reduce(context) do |current, key| + return false unless current.is_a?(Hash) + return false unless current.key?(key.to_sym) || current.key?(key) + + current.key?(key.to_sym) ? current[key.to_sym] : current[key] + end + true + end + + def self.javascript_string(value) + return "" if value.nil? + return value ? "true" : "false" if value == true || value == false + if value.is_a?(Float) + return "NaN" if value.nan? + return value.positive? ? "Infinity" : "-Infinity" if value.infinite? + return "0" if value.zero? + return value.to_i.to_s if value == value.to_i && value.abs < 1e21 + if value.abs >= 1e-6 && value.abs < 1e21 + return scientific_to_fixed(value.to_s) + end + + coefficient, exponent = value.to_s.downcase.split("e") + return "#{coefficient.delete_suffix('.0')}e#{Integer(exponent) >= 0 ? '+' : ''}#{Integer(exponent)}" + end + return value.map { |item| javascript_string(item) }.join(",") if value.is_a?(Array) + return "[object Object]" if value.is_a?(Hash) + + value.to_s + end + + def self.scientific_to_fixed(value) + return value unless value.downcase.include?("e") + + coefficient, exponent_text = value.downcase.split("e") + exponent = Integer(exponent_text) + negative = coefficient.start_with?("-") + digits = coefficient.delete_prefix("-").delete(".") + decimal_position = coefficient.delete_prefix("-").index(".") || coefficient.delete_prefix("-").length + decimal_position += exponent + + result = if decimal_position <= 0 + "0.#{"0" * -decimal_position}#{digits}" + elsif decimal_position >= digits.length + "#{digits}#{"0" * (decimal_position - digits.length)}" + else + "#{digits[0...decimal_position]}.#{digits[decimal_position..]}" + end + + result = result.sub(/(\.\d*?)0+\z/, '\1').delete_suffix(".") + negative ? "-#{result}" : result + end + private_class_method :scientific_to_fixed end end diff --git a/lib/featurevisor/child_instance.rb b/lib/featurevisor/child_instance.rb index 87535ca..4f3fab0 100644 --- a/lib/featurevisor/child_instance.rb +++ b/lib/featurevisor/child_instance.rb @@ -13,6 +13,7 @@ def initialize(options) @context = options[:context] || {} @sticky = options[:sticky] || {} @emitter = Featurevisor::Emitter.new + @parent_unsubscribers = [] end # Subscribe to an event @@ -25,12 +26,25 @@ def on(event_name, callback = nil, &block) if event_name == "context_set" || event_name == "sticky_set" @emitter.on(event_name, callback) else - @parent.on(event_name, callback) + parent_unsubscribe = @parent.on(event_name, callback) + active = true + unsubscribe = nil + unsubscribe = proc do + next unless active + + active = false + parent_unsubscribe.call + @parent_unsubscribers.delete(unsubscribe) + end + @parent_unsubscribers << unsubscribe + unsubscribe end end # Close the child instance def close + @parent_unsubscribers.dup.each(&:call) + @parent_unsubscribers.clear @emitter.clear_all end @@ -98,6 +112,15 @@ def is_enabled(feature_key, context = {}, options = {}) ) end + # Evaluate a feature flag and return its full evaluation details. + def evaluate_flag(feature_key, context = {}, options = {}) + @parent.evaluate_flag( + feature_key, + { **@context, **context }, + { **options, __featurevisor_child_sticky: @sticky } + ) + end + # Get variation value # @param feature_key [String] Feature key # @param context [Hash] Context @@ -117,6 +140,15 @@ def get_variation(feature_key, context = {}, options = {}) ) end + # Evaluate a variation and return its full evaluation details. + def evaluate_variation(feature_key, context = {}, options = {}) + @parent.evaluate_variation( + feature_key, + { **@context, **context }, + { **options, __featurevisor_child_sticky: @sticky } + ) + end + # Get variable value # @param feature_key [String] Feature key # @param variable_key [String] Variable key @@ -138,6 +170,16 @@ def get_variable(feature_key, variable_key, context = {}, options = {}) ) end + # Evaluate a variable and return its full evaluation details. + def evaluate_variable(feature_key, variable_key, context = {}, options = {}) + @parent.evaluate_variable( + feature_key, + variable_key, + { **@context, **context }, + { **options, __featurevisor_child_sticky: @sticky } + ) + end + # Get variable as boolean # @param feature_key [String] Feature key # @param variable_key [String] Variable key diff --git a/lib/featurevisor/conditions.rb b/lib/featurevisor/conditions.rb index 74170cd..edffbc9 100644 --- a/lib/featurevisor/conditions.rb +++ b/lib/featurevisor/conditions.rb @@ -1,10 +1,14 @@ # frozen_string_literal: true require "date" +require "time" module Featurevisor # Conditions module for evaluating feature flags and segments module Conditions + MISSING = Object.new.freeze + private_constant :MISSING + # Get value from context object using dot notation path # @param obj [Hash] Context object # @param path [String] Dot-separated path to the value @@ -12,12 +16,36 @@ module Conditions def self.get_value_from_context(obj, path) return nil if obj.nil? || path.nil? - if path.index(".") == -1 - return obj[path.to_sym] || obj[path] + value = get_value_with_presence(obj, path) + value.equal?(MISSING) ? nil : value + end + + def self.get_value_with_presence(obj, path) + return MISSING unless obj.is_a?(Hash) && path.is_a?(String) + + path.split(".").reduce(obj) do |current, key| + break MISSING unless current.is_a?(Hash) + + if current.key?(key.to_sym) + current[key.to_sym] + elsif current.key?(key) + current[key] + else + break MISSING + end end + end + private_class_method :get_value_with_presence + + def self.strict_equal?(left, right) + return true if left.nil? && right.nil? + return left.to_f == right.to_f if left.is_a?(Numeric) && right.is_a?(Numeric) + return left == right if left.is_a?(String) && right.is_a?(String) + return left == right if (left == true || left == false) && (right == true || right == false) - path.split(".").reduce(obj) { |o, i| o&.[](i.to_sym) || o&.[](i) } + false end + private_class_method :strict_equal? # Check if a condition is matched against context # @param condition [Hash] Condition to evaluate @@ -30,18 +58,19 @@ def self.condition_is_matched(condition, context, get_regex) value = condition["value"] || condition[:value] regex_flags = condition["regexFlags"] || condition[:regexFlags] - context_value_from_path = get_value_from_context(context, attribute) + raw_context_value = get_value_with_presence(context, attribute) + context_value_from_path = raw_context_value.equal?(MISSING) ? nil : raw_context_value + attribute_exists = !raw_context_value.equal?(MISSING) case operator when "equals" - context_value_from_path == value + attribute_exists && strict_equal?(context_value_from_path, value) when "notEquals" - context_value_from_path != value + !attribute_exists || !strict_equal?(context_value_from_path, value) when "before", "after" - # date comparisons - value_in_context = context_value_from_path - date_in_context = value_in_context.is_a?(Date) ? value_in_context : Date.parse(value_in_context.to_s) - date_in_condition = value.is_a?(Date) ? value : Date.parse(value.to_s) + date_in_context = portable_date(context_value_from_path) + date_in_condition = portable_date(value) + return false unless date_in_context && date_in_condition if operator == "before" date_in_context < date_in_condition @@ -50,21 +79,13 @@ def self.condition_is_matched(condition, context, get_regex) end when "in", "notIn" # in / notIn (where condition value is an array) - if value.is_a?(Array) && (context_value_from_path.is_a?(String) || context_value_from_path.is_a?(Numeric) || context_value_from_path.nil?) - # Check if the attribute key actually exists in the context - key_exists = context.key?(attribute.to_sym) || context.key?(attribute.to_s) - - # If key doesn't exist, notIn should fail (return false), in should also fail - if !key_exists - return false - end - - value_in_context = context_value_from_path.to_s - + if attribute_exists && value.is_a?(Array) && + (context_value_from_path.is_a?(String) || context_value_from_path.is_a?(Numeric) || context_value_from_path.nil?) + matched = value.any? { |candidate| strict_equal?(candidate, context_value_from_path) } if operator == "in" - value.include?(value_in_context) + matched else # notIn - !value.include?(value_in_context) + !matched end else false @@ -124,18 +145,18 @@ def self.condition_is_matched(condition, context, get_regex) false end when "exists" - context_value_from_path != nil + attribute_exists when "notExists" - context_value_from_path.nil? + !attribute_exists when "includes", "notIncludes" # includes / notIncludes (where context value is an array) - if context_value_from_path.is_a?(Array) && value.is_a?(String) - value_in_context = context_value_from_path - + if context_value_from_path.is_a?(Array) && + (value.is_a?(String) || value.is_a?(Numeric) || value == true || value == false || value.nil?) + matched = context_value_from_path.any? { |candidate| strict_equal?(candidate, value) } if operator == "includes" - value_in_context.include?(value) + matched else # notIncludes - !value_in_context.include?(value) + !matched end else false @@ -143,10 +164,18 @@ def self.condition_is_matched(condition, context, get_regex) else false end - rescue => e - # Log error but don't stop execution - warn "Error in condition evaluation: #{e.message}" - false end + + def self.portable_date(value) + return value if value.is_a?(Time) || value.is_a?(DateTime) + return value.to_time if value.is_a?(Date) + return nil unless value.is_a?(String) + return nil unless value.match?(/T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+\-]\d{2}:\d{2})\z/) + + Time.iso8601(value) + rescue ArgumentError + nil + end + private_class_method :portable_date end end diff --git a/lib/featurevisor/logger.rb b/lib/featurevisor/diagnostics.rb similarity index 54% rename from lib/featurevisor/logger.rb rename to lib/featurevisor/diagnostics.rb index d76a6b3..a1957b8 100644 --- a/lib/featurevisor/logger.rb +++ b/lib/featurevisor/diagnostics.rb @@ -1,21 +1,22 @@ # frozen_string_literal: true module Featurevisor - # Log levels for the logger + # Diagnostic severity levels LOG_LEVELS = %w[fatal error warn info debug].freeze DEFAULT_LOG_LEVEL = "info".freeze - LOGGER_PREFIX = "[Featurevisor]".freeze + DIAGNOSTIC_PREFIX = "[Featurevisor]".freeze - # Logger class for handling different log levels - class Logger + # Private evaluator adapter for structured diagnostics. + class DiagnosticReporter attr_reader :level, :handler - # Initialize a new logger - # @param options [Hash] Logger options + # Initialize a diagnostic reporter. + # @param options [Hash] Reporter options # @option options [String] :level Log level (default: "info") - # @option options [Proc] :handler Custom log handler (default: default_log_handler) + # @option options [Proc] :handler Internal structured diagnostic sink def initialize(options = {}) @level = options[:level] || DEFAULT_LOG_LEVEL + @filter = !options.key?(:handler) @handler = options[:handler] || method(:default_log_handler) end @@ -25,12 +26,13 @@ def set_level(level) @level = level end - # Log a message at a specific level - # @param level [String] Log level - # @param message [String] Log message + # Forward an evaluator diagnostic. Filtering is performed centrally by + # Instance so module subscriptions and error events remain independent. + # @param level [String] Diagnostic level + # @param message [String] Diagnostic message # @param details [Hash, nil] Additional details def log(level, message, details = nil) - return unless should_handle?(level) + return if @filter && !should_handle?(level) @handler.call(level, message, details) end @@ -72,16 +74,8 @@ def fatal(message, details = nil) private - # Check if the current level should handle the given log level - # @param log_level [String] Log level to check - # @return [Boolean] True if should handle - def should_handle?(log_level) - current_index = LOG_LEVELS.index(@level) - target_index = LOG_LEVELS.index(log_level) - - return false if current_index.nil? || target_index.nil? - - current_index >= target_index + def should_handle?(level) + LOG_LEVELS.index(level).to_i <= LOG_LEVELS.index(@level).to_i end # Default log handler that outputs to console @@ -99,45 +93,18 @@ def default_log_handler(level, message, details = nil) case method_name when "puts" if details && !details.empty? - Kernel.puts("#{LOGGER_PREFIX} #{message} #{details.inspect}") + Kernel.puts("#{DIAGNOSTIC_PREFIX} #{message} #{details.inspect}") else - Kernel.puts("#{LOGGER_PREFIX} #{message}") + Kernel.puts("#{DIAGNOSTIC_PREFIX} #{message}") end when "warn" if details && !details.empty? - Kernel.warn("#{LOGGER_PREFIX} #{message} #{details.inspect}") + Kernel.warn("#{DIAGNOSTIC_PREFIX} #{message} #{details.inspect}") else - Kernel.warn("#{LOGGER_PREFIX} #{message}") + Kernel.warn("#{DIAGNOSTIC_PREFIX} #{message}") end end end end - # Default log handler function - # @param level [String] Log level - # @param message [String] Log message - # @param details [Hash, nil] Additional details - def self.default_log_handler(level, message, details = nil) - method_name = case level - when "info" then "puts" - when "warn" then "warn" - when "error", "fatal" then "warn" - else "puts" - end - - case method_name - when "puts" - if details && !details.empty? - Kernel.puts("#{LOGGER_PREFIX} #{message} #{details.inspect}") - else - Kernel.puts("#{LOGGER_PREFIX} #{message}") - end - when "warn" - if details && !details.empty? - Kernel.warn("#{LOGGER_PREFIX} #{message} #{details.inspect}") - else - Kernel.warn("#{LOGGER_PREFIX} #{message}") - end - end - end end diff --git a/lib/featurevisor/evaluate.rb b/lib/featurevisor/evaluate.rb index 9d8c03e..c8ea72f 100644 --- a/lib/featurevisor/evaluate.rb +++ b/lib/featurevisor/evaluate.rb @@ -55,14 +55,15 @@ def self.evaluate_with_modules(options) # Default: variation if options.key?(:default_variation_value) && evaluation[:type] == "variation" && - evaluation[:variation_value].nil? + !evaluation.key?(:variation_value) && + !evaluation.key?(:variation) evaluation[:variation_value] = options[:default_variation_value] end # Default: variable if options.key?(:default_variable_value) && evaluation[:type] == "variable" && - evaluation[:variable_value].nil? + !evaluation.key?(:variable_value) evaluation[:variable_value] = options[:default_variable_value] end @@ -76,7 +77,7 @@ def self.evaluate_with_modules(options) type = options[:type] feature_key = options[:feature_key] variable_key = options[:variable_key] - logger = options[:logger] + diagnostics = options[:diagnostics] evaluation = { type: type, @@ -86,7 +87,7 @@ def self.evaluate_with_modules(options) error: e } - logger.error("error during evaluation", evaluation) + diagnostics.error("Error during evaluation", evaluation) evaluation end @@ -100,8 +101,8 @@ def self.evaluate(options) feature_key = options[:feature_key] variable_key = options[:variable_key] context = options[:context] - logger = options[:logger] - datafile_reader = options[:datafile_reader] + diagnostics = options[:diagnostics] + datafile = options[:datafile] sticky = options[:sticky] modules_manager = options[:modules_manager] evaluation = nil @@ -120,7 +121,7 @@ def self.evaluate(options) reason: Featurevisor::EvaluationReason::DISABLED } - feature = datafile_reader.get_feature(feature_key) + feature = datafile.get_feature(feature_key) # serve variable default value if feature is disabled (if explicitly specified) if type == "variable" @@ -156,7 +157,7 @@ def self.evaluate(options) end # serve disabled variation value if feature is disabled (if explicitly specified) - if type == "variation" && feature && feature[:disabledVariationValue] + if type == "variation" && feature && feature.key?(:disabledVariationValue) evaluation = { type: type, feature_key: feature_key, @@ -166,7 +167,7 @@ def self.evaluate(options) } end - logger.debug("feature is disabled", evaluation) + diagnostics.debug("feature is disabled", evaluation) return evaluation end @@ -186,7 +187,7 @@ def self.evaluate(options) enabled: sticky_feature[:enabled] } - logger.debug("using sticky enabled", evaluation) + diagnostics.debug("using sticky enabled", evaluation) return evaluation end @@ -195,7 +196,7 @@ def self.evaluate(options) if type == "variation" variation_value = sticky_feature[:variation] - unless variation_value.nil? + if sticky_feature.key?(:variation) evaluation = { type: type, feature_key: feature_key, @@ -203,7 +204,7 @@ def self.evaluate(options) variation_value: variation_value } - logger.debug("using sticky variation", evaluation) + diagnostics.debug("using sticky variation", evaluation) return evaluation end @@ -223,7 +224,7 @@ def self.evaluate(options) variable_value: variable_value } - logger.debug("using sticky variable", evaluation) + diagnostics.debug("using sticky variable", evaluation) return evaluation end @@ -231,7 +232,7 @@ def self.evaluate(options) end # Feature - feature = feature_key.is_a?(String) ? datafile_reader.get_feature(feature_key) : feature_key + feature = feature_key.is_a?(String) ? datafile.get_feature(feature_key) : feature_key # feature: not found unless feature @@ -241,14 +242,14 @@ def self.evaluate(options) reason: Featurevisor::EvaluationReason::FEATURE_NOT_FOUND } - logger.warn("feature not found", evaluation) + diagnostics.warn("feature not found", evaluation) return evaluation end # feature: deprecated if type == "flag" && feature[:deprecated] - logger.warn("feature is deprecated", { feature_key: feature_key }) + diagnostics.warn("feature is deprecated", { feature_key: feature_key }) end # variableSchema @@ -268,13 +269,13 @@ def self.evaluate(options) variable_key: variable_key } - logger.warn("variable schema not found", evaluation) + diagnostics.warn("variable schema not found", evaluation) return evaluation end if variable_schema[:deprecated] - logger.warn("variable is deprecated", { + diagnostics.warn("variable is deprecated", { feature_key: feature_key, variable_key: variable_key }) @@ -289,13 +290,13 @@ def self.evaluate(options) reason: Featurevisor::EvaluationReason::NO_VARIATIONS } - logger.warn("no variations", evaluation) + diagnostics.warn("no variations", evaluation) return evaluation end # Forced - force_result = datafile_reader.get_matched_force(feature, context) + force_result = datafile.get_matched_force(feature, context) force = force_result[:force] force_index = force_result[:forceIndex] @@ -311,7 +312,7 @@ def self.evaluate(options) enabled: force[:enabled] } - logger.debug("forced enabled found", evaluation) + diagnostics.debug("forced enabled found", evaluation) return evaluation end @@ -330,7 +331,7 @@ def self.evaluate(options) variation: variation } - logger.debug("forced variation found", evaluation) + diagnostics.debug("forced variation found", evaluation) return evaluation end @@ -350,7 +351,7 @@ def self.evaluate(options) variable_value: variable_value } - logger.debug("forced variable", evaluation) + diagnostics.debug("forced variable", evaluation) return evaluation end @@ -400,7 +401,7 @@ def self.evaluate(options) enabled: required_features_are_enabled } - logger.debug("required features not enabled", evaluation) + diagnostics.debug("required features not enabled", evaluation) return evaluation end @@ -412,7 +413,7 @@ def self.evaluate(options) feature_key: feature_key, bucket_by: feature[:bucketBy], context: context, - logger: logger + diagnostics: diagnostics }) # Run bucket key modules @@ -438,13 +439,13 @@ def self.evaluate(options) matched_allocation = nil if type != "flag" - matched_traffic = datafile_reader.get_matched_traffic(feature[:traffic], context) + matched_traffic = datafile.get_matched_traffic(feature[:traffic], context) if matched_traffic - matched_allocation = datafile_reader.get_matched_allocation(matched_traffic, bucket_value) + matched_allocation = datafile.get_matched_allocation(matched_traffic, bucket_value) end else - matched_traffic = datafile_reader.get_matched_traffic(feature[:traffic], context) + matched_traffic = datafile.get_matched_traffic(feature[:traffic], context) end if matched_traffic @@ -461,7 +462,7 @@ def self.evaluate(options) enabled: false } - logger.debug("matched rule with 0 percentage", evaluation) + diagnostics.debug("matched rule with 0 percentage", evaluation) return evaluation end @@ -487,7 +488,7 @@ def self.evaluate(options) enabled: matched_traffic[:enabled].nil? ? true : matched_traffic[:enabled] } - logger.debug("matched", evaluation) + diagnostics.debug("matched", evaluation) return evaluation end @@ -502,7 +503,7 @@ def self.evaluate(options) enabled: false } - logger.debug("not matched", evaluation) + diagnostics.debug("not matched", evaluation) return evaluation end @@ -520,7 +521,7 @@ def self.evaluate(options) enabled: matched_traffic[:enabled] } - logger.debug("override from rule", evaluation) + diagnostics.debug("override from rule", evaluation) return evaluation end @@ -538,7 +539,7 @@ def self.evaluate(options) enabled: true } - logger.debug("matched traffic", evaluation) + diagnostics.debug("matched traffic", evaluation) return evaluation end @@ -562,7 +563,7 @@ def self.evaluate(options) variation: variation } - logger.debug("override from rule", evaluation) + diagnostics.debug("override from rule", evaluation) return evaluation end @@ -584,7 +585,7 @@ def self.evaluate(options) variation: variation } - logger.debug("allocated variation", evaluation) + diagnostics.debug("allocated variation", evaluation) return evaluation end @@ -603,10 +604,10 @@ def self.evaluate(options) override_index = overrides.find_index do |o| if o[:conditions] conditions = o[:conditions].is_a?(String) && o[:conditions] != "*" ? JSON.parse(o[:conditions]) : o[:conditions] - datafile_reader.all_conditions_are_matched(conditions, context) + datafile.all_conditions_are_matched(conditions, context) elsif o[:segments] - segments = datafile_reader.parse_segments_if_stringified(o[:segments]) - datafile_reader.all_segments_are_matched(segments, context) + segments = datafile.parse_segments_if_stringified(o[:segments]) + datafile.all_segments_are_matched(segments, context) else false end @@ -629,7 +630,7 @@ def self.evaluate(options) variable_override_index: override_index } - logger.debug("variable override from rule", evaluation) + diagnostics.debug("variable override from rule", evaluation) return evaluation end @@ -652,7 +653,7 @@ def self.evaluate(options) variable_value: variable_value } - logger.debug("override from rule", evaluation) + diagnostics.debug("override from rule", evaluation) return evaluation end @@ -677,10 +678,10 @@ def self.evaluate(options) override_index = overrides.find_index do |o| if o[:conditions] conditions = o[:conditions].is_a?(String) && o[:conditions] != "*" ? JSON.parse(o[:conditions]) : o[:conditions] - datafile_reader.all_conditions_are_matched(conditions, context) + datafile.all_conditions_are_matched(conditions, context) elsif o[:segments] - segments = datafile_reader.parse_segments_if_stringified(o[:segments]) - datafile_reader.all_segments_are_matched(segments, context) + segments = datafile.parse_segments_if_stringified(o[:segments]) + datafile.all_segments_are_matched(segments, context) else false end @@ -702,7 +703,7 @@ def self.evaluate(options) variable_override_index: override_index } - logger.debug("variable override from variation", evaluation) + diagnostics.debug("variable override from variation", evaluation) return evaluation end @@ -725,7 +726,7 @@ def self.evaluate(options) variable_value: variable_value } - logger.debug("allocated variable", evaluation) + diagnostics.debug("allocated variable", evaluation) return evaluation end @@ -742,7 +743,7 @@ def self.evaluate(options) bucket_value: bucket_value } - logger.debug("no matched variation", evaluation) + diagnostics.debug("no matched variation", evaluation) return evaluation end @@ -760,7 +761,7 @@ def self.evaluate(options) variable_value: variable_schema[:defaultValue] } - logger.debug("using default value", evaluation) + diagnostics.debug("using default value", evaluation) return evaluation end @@ -774,7 +775,7 @@ def self.evaluate(options) bucket_value: bucket_value } - logger.debug("variable not found", evaluation) + diagnostics.debug("variable not found", evaluation) return evaluation end @@ -788,7 +789,7 @@ def self.evaluate(options) enabled: false } - logger.debug("nothing matched", evaluation) + diagnostics.debug("nothing matched", evaluation) evaluation rescue => e @@ -800,7 +801,7 @@ def self.evaluate(options) error: e } - logger.error("error", evaluation) + diagnostics.error("Error during evaluation", evaluation) evaluation end diff --git a/lib/featurevisor/datafile_reader.rb b/lib/featurevisor/evaluation_data_provider.rb similarity index 89% rename from lib/featurevisor/datafile_reader.rb rename to lib/featurevisor/evaluation_data_provider.rb index 1bede67..e0fcb54 100644 --- a/lib/featurevisor/datafile_reader.rb +++ b/lib/featurevisor/evaluation_data_provider.rb @@ -3,17 +3,17 @@ require "json" module Featurevisor - # DatafileReader class for reading and processing Featurevisor datafiles - class DatafileReader - attr_reader :schema_version, :revision, :featurevisor_version, :segments, :features, :logger, :regex_cache + # Private datafile and matching adapter used by the evaluator. + class InstanceEvaluationDataProvider + attr_reader :schema_version, :revision, :featurevisor_version, :segments, :features, :diagnostics, :regex_cache - # Initialize a new DatafileReader - # @param options [Hash] Options hash containing datafile and logger + # Initialize a new evaluation data provider. + # @param options [Hash] Options hash containing datafile and diagnostics # @option options [Hash] :datafile Datafile content - # @option options [Logger] :logger Logger instance + # @option options [DiagnosticReporter] :diagnostics Diagnostic reporter def initialize(options) datafile = options[:datafile] - @logger = options[:logger] + @diagnostics = options[:diagnostics] @schema_version = datafile[:schemaVersion] @revision = datafile[:revision] @@ -133,7 +133,15 @@ def get_regex(regex_string, regex_flags = "") return @regex_cache[cache_key] if @regex_cache[cache_key] - regex = Regexp.new(regex_string, flags) + ruby_flags = 0 + ruby_flags |= Regexp::IGNORECASE if flags.include?("i") + # Ruby's MULTILINE option controls dot matching newlines, which is the + # JavaScript `s` behavior. Ruby anchors already operate per line. + ruby_flags |= Regexp::MULTILINE if flags.include?("s") + invalid_flags = flags.delete("gimsuy") + raise ArgumentError, "invalid regular expression flags: #{invalid_flags}" unless invalid_flags.empty? + + regex = Regexp.new(regex_string, ruby_flags) @regex_cache[cache_key] = regex @regex_cache[cache_key] end @@ -155,12 +163,11 @@ def all_conditions_are_matched(conditions, context) result = Conditions.condition_is_matched(conditions, context, get_regex_proc) return result rescue => e - @logger.warn("Error in condition evaluation: #{e.message}", { - error: e.class.name, - details: { - condition: conditions, - context: context - } + @diagnostics.warn(e.message, { + code: "condition_match_error", + error: e, + condition: conditions, + context: context }) return false end @@ -183,10 +190,12 @@ def all_conditions_are_matched(conditions, context) end if conditions.is_a?(Hash) && conditions[:not] && conditions[:not].is_a?(Array) + return false if conditions[:not].empty? return !all_conditions_are_matched({ and: conditions[:not] }, context) end if conditions.is_a?(Hash) && conditions["not"] && conditions["not"].is_a?(Array) + return false if conditions["not"].empty? return !all_conditions_are_matched({ "and" => conditions["not"] }, context) end @@ -244,10 +253,12 @@ def all_segments_are_matched(group_segments, context) end if group_segments[:not] && group_segments[:not].is_a?(Array) + return false if group_segments[:not].empty? return !all_segments_are_matched({ and: group_segments[:not] }, context) end if group_segments["not"] && group_segments["not"].is_a?(Array) + return false if group_segments["not"].empty? return !all_segments_are_matched({ "and" => group_segments["not"] }, context) end @@ -333,7 +344,7 @@ def parse_conditions_if_stringified(conditions) begin JSON.parse(conditions) rescue => e - @logger.error("Error parsing conditions", { + @diagnostics.error("Error parsing conditions", { error: e, details: { conditions: conditions @@ -355,5 +366,5 @@ def parse_segments_if_stringified(segments) end end - private_constant :DatafileReader + private_constant :InstanceEvaluationDataProvider end diff --git a/lib/featurevisor/events.rb b/lib/featurevisor/events.rb index 511324b..c3b3258 100644 --- a/lib/featurevisor/events.rb +++ b/lib/featurevisor/events.rb @@ -21,8 +21,8 @@ def self.get_params_for_sticky_set_event(previous_sticky = {}, new_sticky = {}, end # Get parameters for datafile set event - # @param previous_reader [DatafileReader] Previous datafile reader - # @param new_reader [DatafileReader] New datafile reader + # @param previous_reader [InstanceEvaluationDataProvider] Previous datafile + # @param new_reader [InstanceEvaluationDataProvider] New datafile # @return [Hash] Event parameters def self.get_params_for_datafile_set_event(previous_reader, new_reader, replace = false) previous_revision = previous_reader.get_revision diff --git a/lib/featurevisor/instance.rb b/lib/featurevisor/instance.rb index 705a7e2..bbb3559 100644 --- a/lib/featurevisor/instance.rb +++ b/lib/featurevisor/instance.rb @@ -25,9 +25,9 @@ class Instance def initialize(options = {}) # from options @context = options[:context] || {} - @logger = Logger.new( + @diagnostics = DiagnosticReporter.new( level: options[:log_level] || "info", - handler: method(:handle_internal_log) + handler: method(:handle_evaluation_diagnostic) ) @on_diagnostic = options[:on_diagnostic] || options[:onDiagnostic] @emitter = Featurevisor::Emitter.new @@ -36,9 +36,9 @@ def initialize(options = {}) @module_diagnostic_subscriptions = [] # datafile - @datafile_reader = DatafileReader.new( + @datafile = InstanceEvaluationDataProvider.new( datafile: EMPTY_DATAFILE, - logger: @logger + diagnostics: @diagnostics ) @modules_manager = Featurevisor::Modules::ModulesManager.new( @@ -62,21 +62,35 @@ def initialize(options = {}) # Set the log level # @param level [String] Log level def set_log_level(level) - @logger.set_level(level) + @diagnostics.set_level(level) end - def handle_internal_log(level, message, details = nil) + def handle_evaluation_diagnostic(level, message, details = nil) details = (details || {}).dup - code = details[:reason] || details["reason"] || message + code = details.delete(:code) || details.delete("code") || details[:reason] || details["reason"] || message code = "deprecated_feature" if message == "feature is deprecated" code = "deprecated_variable" if message == "variable is deprecated" code = "feature_not_found" if message == "feature not found" code = "variable_not_found" if message == "variable schema not found" code = "no_variations" if message == "no variations" code = "invalid_bucket_by" if message == "invalid bucketBy" - report_diagnostic(level: level, code: code.to_s, message: message, details: details) + code = "evaluation_error" if message == "Error during evaluation" + code = "conditions_parse_error" if message == "Error parsing conditions" + original_error = details.delete(:error) || details.delete("error") + nested_details = details.delete(:details) || details.delete("details") + details.merge!(nested_details) if nested_details.is_a?(Hash) + if details.key?(:feature_key) && details.key?(:reason) + evaluation = details.dup + details = { + featureKey: evaluation[:feature_key], + variableKey: evaluation[:variable_key], + reason: evaluation[:reason], + evaluation: camelize_diagnostic_value(evaluation) + } + end + report_diagnostic(level: level, code: code.to_s, message: message, details: details, originalError: original_error) end - private :handle_internal_log + private :handle_evaluation_diagnostic # Set the datafile # @param datafile [Hash, String] Datafile content or JSON string @@ -97,14 +111,14 @@ def set_datafile(datafile, replace = false) parsed_datafile[:features].is_a?(Hash) raise ArgumentError, "Invalid datafile" end - next_datafile = replace ? parsed_datafile : merge_datafiles(@datafile_reader.get_datafile, parsed_datafile) - new_datafile_reader = DatafileReader.new( + next_datafile = replace ? parsed_datafile : merge_datafiles(@datafile.get_datafile, parsed_datafile) + new_datafile = InstanceEvaluationDataProvider.new( datafile: next_datafile, - logger: @logger + diagnostics: @diagnostics ) - details = Featurevisor::Events.get_params_for_datafile_set_event(@datafile_reader, new_datafile_reader, replace) - @datafile_reader = new_datafile_reader + details = Featurevisor::Events.get_params_for_datafile_set_event(@datafile, new_datafile, replace) + @datafile = new_datafile report_diagnostic( level: "info", @@ -152,34 +166,34 @@ def set_sticky(sticky, replace = false) # Get the revision # @return [String] Revision string def get_revision - @datafile_reader.get_revision + @datafile.get_revision end def get_schema_version - @datafile_reader.get_schema_version + @datafile.get_schema_version end def get_segment(segment_key) - @datafile_reader.get_segment(segment_key) + @datafile.get_segment(segment_key) end def get_feature_keys - @datafile_reader.get_feature_keys + @datafile.get_feature_keys end def get_variable_keys(feature_key) - @datafile_reader.get_variable_keys(feature_key) + @datafile.get_variable_keys(feature_key) end def has_variations?(feature_key) - @datafile_reader.has_variations?(feature_key) + @datafile.has_variations?(feature_key) end # Get a feature by key # @param feature_key [String] Feature key # @return [Hash, nil] Feature data or nil if not found def get_feature(feature_key) - @datafile_reader.get_feature(feature_key) + @datafile.get_feature(feature_key) end # Add a module @@ -189,8 +203,8 @@ def add_module(mod) @modules_manager.add(mod) end - def remove_module(name_or_module) - @modules_manager.remove(name_or_module) + def remove_module(name) + @modules_manager.remove(name) end # Subscribe to an event @@ -198,11 +212,15 @@ def remove_module(name_or_module) # @param callback [Proc] Callback function # @return [Proc] Unsubscribe function def on(event_name, callback) + return -> {} if @closed + @emitter.on(event_name, callback) end # Close the instance def close + return if @closed + @closed = true @modules_manager.close_all @module_diagnostic_subscriptions = [] @@ -285,7 +303,7 @@ def is_enabled(feature_key, context = {}, options = {}) evaluation = evaluate_flag(feature_key, context, options) evaluation[:enabled] == true rescue => e - @logger.error("isEnabled", { feature_key: feature_key, error: e }) + report_diagnostic(level: "error", code: "evaluation_error", message: "isEnabled failed", originalError: e, details: { featureKey: feature_key }) false end end @@ -313,7 +331,7 @@ def get_variation(feature_key, context = {}, options = {}) begin evaluation = evaluate_variation(feature_key, context, options) - if evaluation[:variation_value] + if evaluation.key?(:variation_value) evaluation[:variation_value] elsif evaluation[:variation] evaluation[:variation][:value] @@ -321,7 +339,7 @@ def get_variation(feature_key, context = {}, options = {}) nil end rescue => e - @logger.error("getVariation", { feature_key: feature_key, error: e }) + report_diagnostic(level: "error", code: "evaluation_error", message: "getVariation failed", originalError: e, details: { featureKey: feature_key }) nil end end @@ -352,7 +370,7 @@ def get_variable(feature_key, variable_key, context = {}, options = {}) begin evaluation = evaluate_variable(feature_key, variable_key, context, options) - if !evaluation[:variable_value].nil? + if evaluation.key?(:variable_value) if evaluation[:variable_schema] && evaluation[:variable_schema][:type] == "json" && evaluation[:variable_value].is_a?(String) @@ -364,7 +382,7 @@ def get_variable(feature_key, variable_key, context = {}, options = {}) nil end rescue => e - @logger.error("getVariable", { feature_key: feature_key, variable_key: variable_key, error: e }) + report_diagnostic(level: "error", code: "evaluation_error", message: "getVariable failed", originalError: e, details: { featureKey: feature_key, variableKey: variable_key }) nil end end @@ -454,7 +472,7 @@ def get_variable_json(feature_key, variable_key, context = {}, options = {}) def get_all_evaluations(context = {}, feature_keys = [], options = {}) result = {} - keys = feature_keys.size > 0 ? feature_keys : @datafile_reader.get_feature_keys + keys = feature_keys.size > 0 ? feature_keys : @datafile.get_feature_keys keys.each do |feature_key| # Convert symbol keys to strings for evaluation functions @@ -466,13 +484,13 @@ def get_all_evaluations(context = {}, feature_keys = [], options = {}) } # variation - if @datafile_reader.has_variations?(feature_key_str) + if @datafile.has_variations?(feature_key_str) variation = get_variation(feature_key_str, context, options) - evaluated_feature[:variation] = variation if variation + evaluated_feature[:variation] = variation unless variation.nil? end # variables - variable_keys = @datafile_reader.get_variable_keys(feature_key_str) + variable_keys = @datafile.get_variable_keys(feature_key_str) if variable_keys.size > 0 evaluated_feature[:variables] = {} @@ -501,13 +519,14 @@ def get_all_evaluations(context = {}, feature_keys = [], options = {}) def get_evaluation_dependencies(context, options = {}) { context: get_context(context), - logger: @logger, + diagnostics: @diagnostics, modules_manager: @modules_manager, - datafile_reader: @datafile_reader, + datafile: @datafile, sticky: options[:__featurevisor_child_sticky] || @sticky, - default_variation_value: options[:default_variation_value], - default_variable_value: options[:default_variable_value] - } + }.tap do |dependencies| + dependencies[:default_variation_value] = options[:default_variation_value] if options.key?(:default_variation_value) + dependencies[:default_variable_value] = options[:default_variable_value] if options.key?(:default_variable_value) + end end # Get value by type @@ -584,7 +603,7 @@ def report_diagnostic(diagnostic, source_module = nil) diagnostic = (diagnostic || {}).dup diagnostic[:level] ||= "info" diagnostic[:module] = source_module.name if source_module && source_module.name - details = (diagnostic[:details] || {}).dup + details = camelize_diagnostic_value((diagnostic[:details] || {}).dup) legacy_module_name = diagnostic.delete(:module_name) legacy_original_error = diagnostic.delete(:original_error) diagnostic[:moduleName] = legacy_module_name if !diagnostic.key?(:moduleName) && !legacy_module_name.nil? @@ -609,7 +628,7 @@ def report_diagnostic(diagnostic, source_module = nil) end if @on_diagnostic - if should_report_diagnostic?(diagnostic[:level], @logger.level) + if should_report_diagnostic?(diagnostic[:level], @diagnostics.level) begin @on_diagnostic.call(diagnostic) rescue => e @@ -617,7 +636,7 @@ def report_diagnostic(diagnostic, source_module = nil) end end else - Logger.new(level: @logger.level).log( + DiagnosticReporter.new(level: @diagnostics.level).log( diagnostic[:level], diagnostic[:message], diagnostic @@ -638,6 +657,21 @@ def should_report_diagnostic?(diagnostic_level, subscriber_level) subscriber_index >= diagnostic_index end + + def camelize_diagnostic_value(value) + case value + when Hash + value.each_with_object({}) do |(key, item), result| + normalized_key = key.to_s.gsub(/_([a-z])/) { Regexp.last_match(1).upcase }.to_sym + result[normalized_key] = camelize_diagnostic_value(item) + end + when Array + value.map { |item| camelize_diagnostic_value(item) } + else + value + end + end + private :camelize_diagnostic_value end # Create a new Featurevisor instance diff --git a/lib/featurevisor/version.rb b/lib/featurevisor/version.rb index 722ba7e..2bd30d7 100644 --- a/lib/featurevisor/version.rb +++ b/lib/featurevisor/version.rb @@ -1,3 +1,3 @@ module Featurevisor - VERSION = "1.1.0" + VERSION = "2.0.0" end diff --git a/spec/bucketer_spec.rb b/spec/bucketer_spec.rb index 7caf762..445664d 100644 --- a/spec/bucketer_spec.rb +++ b/spec/bucketer_spec.rb @@ -38,7 +38,7 @@ end describe "get_bucket_key" do - let(:logger) { Featurevisor.const_get(:Logger).new(level: "warn") } + let(:diagnostics) { Featurevisor.const_get(:DiagnosticReporter).new(level: "warn") } it "should be a method" do expect(Featurevisor::Bucketer).to respond_to(:get_bucket_key) @@ -53,7 +53,7 @@ feature_key: feature_key, bucket_by: bucket_by, context: context, - logger: logger + diagnostics: diagnostics ) expect(bucket_key).to eq("123.test-feature") @@ -68,7 +68,7 @@ feature_key: feature_key, bucket_by: bucket_by, context: context, - logger: logger + diagnostics: diagnostics ) expect(bucket_key).to eq("test-feature") @@ -83,7 +83,7 @@ feature_key: feature_key, bucket_by: bucket_by, context: context, - logger: logger + diagnostics: diagnostics ) expect(bucket_key).to eq("123.234.test-feature") @@ -98,7 +98,7 @@ feature_key: feature_key, bucket_by: bucket_by, context: context, - logger: logger + diagnostics: diagnostics ) expect(bucket_key).to eq("123.test-feature") @@ -119,7 +119,7 @@ feature_key: feature_key, bucket_by: bucket_by, context: context, - logger: logger + diagnostics: diagnostics ) expect(bucket_key).to eq("123.234.test-feature") @@ -134,7 +134,7 @@ feature_key: feature_key, bucket_by: bucket_by, context: context, - logger: logger + diagnostics: diagnostics ) expect(bucket_key).to eq("234.test-feature") @@ -149,7 +149,7 @@ feature_key: feature_key, bucket_by: bucket_by, context: context, - logger: logger + diagnostics: diagnostics ) expect(bucket_key).to eq("deviceIdHere.test-feature") @@ -165,10 +165,29 @@ feature_key: feature_key, bucket_by: bucket_by, context: context, - logger: logger + diagnostics: diagnostics ) end.to raise_error(StandardError, "invalid bucketBy") end + + it "stringifies bucket values like JavaScript" do + diagnostics = Featurevisor.const_get(:DiagnosticReporter).new(level: "fatal") + + expect(Featurevisor::Bucketer.get_bucket_key( + feature_key: "feature", bucket_by: %w[missing enabled values object], + context: { missing: nil, enabled: false, values: [1, true, nil], object: { id: 1 } }, + diagnostics: diagnostics + )).to eq(".false.1,true,.[object Object].feature") + end + + it "normalizes whole floating point values and negative zero like JavaScript" do + diagnostics = Featurevisor.const_get(:DiagnosticReporter).new(level: "fatal") + + expect(Featurevisor::Bucketer.get_bucket_key( + feature_key: "feature", bucket_by: %w[whole negative_zero small large], + context: { whole: 1.0, negative_zero: -0.0, small: 1e-6, large: 1e21 }, diagnostics: diagnostics + )).to eq("1.0.0.000001.1e+21.feature") + end end describe "constants" do diff --git a/spec/child_instance_spec.rb b/spec/child_instance_spec.rb index 146a948..d02bed1 100644 --- a/spec/child_instance_spec.rb +++ b/spec/child_instance_spec.rb @@ -218,9 +218,12 @@ expect(child_f.is_enabled("test")).to be true expect(child_f.get_variation("test")).to eq("control") + expect(child_f.evaluate_flag("test")[:enabled]).to be true + expect(child_f.evaluate_variation("test")[:variation][:value]).to eq("control") expect(child_f.get_variable("test", "color")).to eq("black") expect(child_f.get_variable_string("test", "color")).to eq("black") + expect(child_f.evaluate_variable("test", "color")[:variable_value]).to eq("black") expect(child_f.get_variable("test", "showSidebar")).to be false expect(child_f.get_variable_boolean("test", "showSidebar")).to be false @@ -257,6 +260,7 @@ } }) expect(child_f.is_enabled("newFeature")).to be true + expect(child_f.evaluate_flag("newFeature")[:reason]).to eq("sticky") all_evaluations = child_f.get_all_evaluations expect(all_evaluations.keys).to eq([:test, :anotherTest]) @@ -264,3 +268,41 @@ child_f.close end end + +RSpec.describe "child lifecycle parity" do + it "removes delegated subscriptions when closed" do + parent = Featurevisor.create_featurevisor(log_level: "fatal") + child = parent.spawn + events = [] + child.on("datafile_set") { |event| events << event } + + child.close + child.close + parent.set_datafile( + { + schemaVersion: "2", + revision: "after-close", + segments: {}, + features: {} + }, + true + ) + + expect(events).to be_empty + end + + it "matches JavaScript parent context snapshot behavior" do + parent = Featurevisor.create_featurevisor( + context: { country: "nl", plan: "free" }, + log_level: "fatal" + ) + child = parent.spawn(country: "de") + parent.set_context({ plan: "pro", locale: "de-DE" }) + + expect(child.get_context).to eq( + country: "de", + plan: "free", + locale: "de-DE" + ) + end +end diff --git a/spec/cli_spec.rb b/spec/cli_spec.rb index cccd688..0a553d2 100644 --- a/spec/cli_spec.rb +++ b/spec/cli_spec.rb @@ -120,7 +120,15 @@ "features" => { "testFeature" => { "key" => "testFeature", - "variations" => [] + "bucketBy" => "userId", + "variations" => [], + "traffic" => [ + { + "key" => "everyone", + "segments" => "*", + "percentage" => 100_000 + } + ] } } }) diff --git a/spec/conditions_spec.rb b/spec/conditions_spec.rb index 9472e51..5eaf9dc 100644 --- a/spec/conditions_spec.rb +++ b/spec/conditions_spec.rb @@ -1,16 +1,16 @@ require "featurevisor" RSpec.describe Featurevisor::Conditions do - let(:logger) { Featurevisor.const_get(:Logger).new } + let(:diagnostics) { Featurevisor.const_get(:DiagnosticReporter).new } let(:datafile_reader) do - Featurevisor.const_get(:DatafileReader).new( + Featurevisor.const_get(:InstanceEvaluationDataProvider).new( datafile: { schemaVersion: "2.0", revision: "1", segments: {}, features: {} }, - logger: logger + diagnostics: diagnostics ) end @@ -54,6 +54,44 @@ describe "condition operators" do let(:get_regex) { ->(pattern, flags) { Regexp.new(pattern, flags) } } + it "keeps JavaScript primitive equality and presence semantics" do + expect(Featurevisor::Conditions.condition_is_matched( + { attribute: "value", operator: "equals", value: 1 }, + { value: "1" }, get_regex + )).to be false + expect(Featurevisor::Conditions.condition_is_matched( + { attribute: "value", operator: "equals", value: 1 }, + { value: true }, get_regex + )).to be false + expect(Featurevisor::Conditions.condition_is_matched( + { attribute: "value", operator: "exists" }, + { value: nil }, get_regex + )).to be true + expect(Featurevisor::Conditions.condition_is_matched( + { attribute: "value", operator: "notExists" }, + { value: nil }, get_regex + )).to be false + end + + it "uses strict primitive membership for in and includes" do + expect(Featurevisor::Conditions.condition_is_matched( + { attribute: "value", operator: "in", value: [1] }, + { value: "1" }, get_regex + )).to be false + expect(Featurevisor::Conditions.condition_is_matched( + { attribute: "values", operator: "includes", value: 1 }, + { values: [1, true, nil] }, get_regex + )).to be true + expect(Featurevisor::Conditions.condition_is_matched( + { attribute: "values", operator: "includes", value: nil }, + { values: [1, true, nil] }, get_regex + )).to be true + expect(Featurevisor::Conditions.condition_is_matched( + { attribute: "values", operator: "includes", value: 1 }, + { values: ["1"] }, get_regex + )).to be false + end + describe "equals" do it "should match exact values" do condition = { attribute: "browser_type", operator: "equals", value: "chrome" } @@ -350,6 +388,23 @@ Featurevisor::Conditions.condition_is_matched(condition, { version: "1.1.0" }, get_regex) ).to be false end + + it "should compare prereleases and ignore build metadata" do + expect( + Featurevisor::Conditions.condition_is_matched( + { attribute: "version", operator: "semverLessThan", value: "1.2.3" }, + { version: "1.2.3-beta.1" }, + get_regex + ) + ).to be true + expect( + Featurevisor::Conditions.condition_is_matched( + { attribute: "version", operator: "semverEquals", value: "1.2.3+build.9" }, + { version: "1.2.3+build.5" }, + get_regex + ) + ).to be true + end end describe "regex operators" do @@ -381,6 +436,20 @@ context = { date: "2023-05-12T00:00:00Z" } expect(Featurevisor::Conditions.condition_is_matched(condition, context, get_regex)).to be true + expect( + Featurevisor::Conditions.condition_is_matched( + condition, + { date: "2023-05-12T00:00:00" }, + get_regex + ) + ).to be false + expect( + Featurevisor::Conditions.condition_is_matched( + condition, + { date: "2023-05-13T17:23:59+01:00" }, + get_regex + ) + ).to be false end it "should handle after" do diff --git a/spec/conformance_spec.rb b/spec/conformance_spec.rb index 742a5e6..ae9d93e 100644 --- a/spec/conformance_spec.rb +++ b/spec/conformance_spec.rb @@ -6,9 +6,10 @@ RSpec.describe "Featurevisor v3 conformance" do it "uses the shared inclusive allocation contract" do fixture = JSON.parse(File.read(File.expand_path("../conformance/sdk-v3.json", __dir__)), symbolize_names: true) - reader = Featurevisor.const_get(:DatafileReader).new( + expect(fixture[:version]).to eq(2) + reader = Featurevisor.const_get(:InstanceEvaluationDataProvider).new( datafile: { schemaVersion: "2", revision: "conformance", segments: {}, features: {} }, - logger: Featurevisor.const_get(:Logger).new(level: "fatal") + diagnostics: Featurevisor.const_get(:DiagnosticReporter).new(level: "fatal") ) traffic = { allocation: fixture.dig(:bucketing, :allocations) } @@ -16,5 +17,43 @@ allocation = reader.get_matched_allocation(traffic, bucket.to_s.to_i) expect(allocation[:variation]).to eq(expected) end + + fixture[:numericBucketKeys].each do |test_case| + bucket_key = Featurevisor::Bucketer.get_bucket_key( + feature_key: "feature", + bucket_by: "value", + context: { value: test_case[:value] }, + diagnostics: Featurevisor.const_get(:DiagnosticReporter).new + ) + expect(bucket_key).to eq("#{test_case[:expected]}.feature") + end + + fixture.dig(:regularExpressions, :portableCases).each do |test_case| + condition = { + attribute: "value", + operator: "matches", + value: test_case[:pattern], + regexFlags: test_case[:flags] + } + expect( + reader.all_conditions_are_matched(condition, {value: test_case[:value]}) + ).to eq(test_case[:expected]), "pattern #{test_case[:pattern]}, flags #{test_case[:flags]}" + end + + fixture[:conditionCases].each do |test_case| + expect( + reader.all_conditions_are_matched(test_case[:condition], test_case[:context]) + ).to eq(test_case[:expected]), test_case[:name] + end + + aggregate_case = fixture.dig(:defaults, :aggregateCase) + featurevisor = Featurevisor.create_featurevisor(datafile: aggregate_case[:datafile], log_level: "fatal") + evaluated = featurevisor.get_all_evaluations( + {}, + [], + default_variation_value: aggregate_case[:defaultVariationValue] + )[:experiment] + expect(evaluated[:enabled]).to eq(aggregate_case.dig(:expected, :enabled)) + expect(evaluated[:variation]).to eq(aggregate_case.dig(:expected, :variation)) end end diff --git a/spec/evaluate_spec.rb b/spec/evaluate_spec.rb index 3eb251c..a1c424a 100644 --- a/spec/evaluate_spec.rb +++ b/spec/evaluate_spec.rb @@ -38,19 +38,19 @@ end describe "evaluate_with_modules" do - let(:logger) { Featurevisor.const_get(:Logger).new(level: "warn") } + let(:diagnostics) { Featurevisor.const_get(:DiagnosticReporter).new(level: "warn") } let(:datafile_reader) do - Featurevisor.const_get(:DatafileReader).new( + Featurevisor.const_get(:InstanceEvaluationDataProvider).new( datafile: { schemaVersion: "2.0", revision: "1", segments: {}, features: {} }, - logger: logger + diagnostics: diagnostics ) end - let(:modules_manager) { Featurevisor::Modules::ModulesManager.new(logger: logger) } + let(:modules_manager) { Featurevisor::Modules::ModulesManager.new(diagnostics: diagnostics) } it "should be a method" do expect(Featurevisor::Evaluate).to respond_to(:evaluate_with_modules) @@ -61,9 +61,9 @@ type: "flag", feature_key: "test-feature", context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader } # Mock datafile_reader to raise an error @@ -87,9 +87,9 @@ type: "variation", feature_key: "test-feature", context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader, + datafile: datafile_reader, default_variation_value: "default" } @@ -114,9 +114,9 @@ feature_key: "test-feature", variable_key: "test-var", context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader, + datafile: datafile_reader, default_variable_value: "default" } @@ -131,19 +131,19 @@ end describe "evaluate" do - let(:logger) { Featurevisor.const_get(:Logger).new(level: "warn") } + let(:diagnostics) { Featurevisor.const_get(:DiagnosticReporter).new(level: "warn") } let(:datafile_reader) do - Featurevisor.const_get(:DatafileReader).new( + Featurevisor.const_get(:InstanceEvaluationDataProvider).new( datafile: { schemaVersion: "2.0", revision: "1", segments: {}, features: {} }, - logger: logger + diagnostics: diagnostics ) end - let(:modules_manager) { Featurevisor::Modules::ModulesManager.new(logger: logger) } + let(:modules_manager) { Featurevisor::Modules::ModulesManager.new(diagnostics: diagnostics) } it "should be a method" do expect(Featurevisor::Evaluate).to respond_to(:evaluate) @@ -154,9 +154,9 @@ type: "flag", feature_key: "non-existent-feature", context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader } result = Featurevisor::Evaluate.evaluate(options) @@ -176,9 +176,9 @@ type: "flag", feature_key: "test-feature", context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader, + datafile: datafile_reader, sticky: sticky } @@ -200,9 +200,9 @@ type: "variation", feature_key: "test-feature", context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader, + datafile: datafile_reader, sticky: sticky } @@ -226,9 +226,9 @@ feature_key: "test-feature", variable_key: "test-var", context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader, + datafile: datafile_reader, sticky: sticky } @@ -259,9 +259,9 @@ type: "flag", feature_key: feature, context: { userId: "123" }, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader } result = Featurevisor::Evaluate.evaluate(options) @@ -297,9 +297,9 @@ type: "flag", feature_key: feature, context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader } # We need to stub the recursive call to evaluate @@ -354,9 +354,9 @@ type: "flag", feature_key: feature, context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader ) expect(result[:reason]).not_to eq(Featurevisor::EvaluationReason::REQUIRED) @@ -397,9 +397,9 @@ type: "flag", feature_key: feature, context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader ) expect(result[:reason]).not_to eq(Featurevisor::EvaluationReason::REQUIRED) @@ -410,9 +410,9 @@ type: "flag", feature_key: "test-feature", context: {}, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader } # Mock datafile_reader to raise an error @@ -470,9 +470,9 @@ feature_key: feature, variable_key: :test_var, context: { country: "nl" }, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader ) expect(result[:reason]).to eq(Featurevisor::EvaluationReason::VARIABLE_OVERRIDE_RULE) @@ -532,9 +532,9 @@ feature_key: feature, variable_key: :test_var, context: { country: "nl" }, - logger: logger, + diagnostics: diagnostics, modules_manager: modules_manager, - datafile_reader: datafile_reader + datafile: datafile_reader ) expect(result[:reason]).to eq(Featurevisor::EvaluationReason::VARIABLE_OVERRIDE_VARIATION) diff --git a/spec/datafile_reader_spec.rb b/spec/evaluation_data_provider_spec.rb similarity index 95% rename from spec/datafile_reader_spec.rb rename to spec/evaluation_data_provider_spec.rb index 071ae68..a1c1014 100644 --- a/spec/datafile_reader_spec.rb +++ b/spec/evaluation_data_provider_spec.rb @@ -1,18 +1,18 @@ require "featurevisor" -RSpec.describe Featurevisor.const_get(:DatafileReader) do - let(:logger) { Featurevisor.const_get(:Logger).new } +RSpec.describe Featurevisor.const_get(:InstanceEvaluationDataProvider) do + let(:diagnostics) { Featurevisor.const_get(:DiagnosticReporter).new } describe "basic functionality" do it "should be a class" do - expect(Featurevisor.const_get(:DatafileReader)).to be_a(Class) + expect(Featurevisor.const_get(:InstanceEvaluationDataProvider)).to be_a(Class) end it "should create an instance with options" do datafile = { schemaVersion: "2", revision: "1", segments: {}, features: {} } - reader = Featurevisor.const_get(:DatafileReader).new(datafile: datafile, logger: logger) + reader = Featurevisor.const_get(:InstanceEvaluationDataProvider).new(datafile: datafile, diagnostics: diagnostics) - expect(reader).to be_instance_of(Featurevisor.const_get(:DatafileReader)) + expect(reader).to be_instance_of(Featurevisor.const_get(:InstanceEvaluationDataProvider)) end end @@ -72,7 +72,7 @@ } end - let(:reader) { Featurevisor.const_get(:DatafileReader).new(datafile: datafile_json, logger: logger) } + let(:reader) { Featurevisor.const_get(:InstanceEvaluationDataProvider).new(datafile: datafile_json, diagnostics: diagnostics) } it "should return requested entities" do expect(reader.get_revision).to eq("1") @@ -255,7 +255,7 @@ } end - let(:datafile_reader) { Featurevisor.const_get(:DatafileReader).new(datafile: datafile_content, logger: logger) } + let(:datafile_reader) { Featurevisor.const_get(:InstanceEvaluationDataProvider).new(datafile: datafile_content, diagnostics: diagnostics) } it "should match everyone" do group = groups.find { |g| g[:key] == "*" } @@ -441,14 +441,14 @@ describe "conditions" do let(:datafile_reader) do - Featurevisor.const_get(:DatafileReader).new( + Featurevisor.const_get(:InstanceEvaluationDataProvider).new( datafile: { schemaVersion: "2.0", revision: "1", segments: {}, features: {} }, - logger: logger + diagnostics: diagnostics ) end @@ -670,14 +670,14 @@ describe "utility methods" do let(:datafile_reader) do - Featurevisor.const_get(:DatafileReader).new( + Featurevisor.const_get(:InstanceEvaluationDataProvider).new( datafile: { schemaVersion: "2.0", revision: "1", segments: {}, features: {} }, - logger: logger + diagnostics: diagnostics ) end diff --git a/spec/events_spec.rb b/spec/events_spec.rb index 090741f..ba8c89a 100644 --- a/spec/events_spec.rb +++ b/spec/events_spec.rb @@ -1,7 +1,7 @@ require "featurevisor" RSpec.describe Featurevisor::Events do - let(:logger) { Featurevisor.const_get(:Logger).new(level: "error") } + let(:diagnostics) { Featurevisor.const_get(:DiagnosticReporter).new(level: "error") } describe ".get_params_for_sticky_set_event" do it "should get params for sticky set event: empty to new" do @@ -50,14 +50,14 @@ describe ".get_params_for_datafile_set_event" do def build_reader(revision:, features:) - Featurevisor.const_get(:DatafileReader).new( + Featurevisor.const_get(:InstanceEvaluationDataProvider).new( datafile: { schemaVersion: "1.0.0", revision: revision, features: features, segments: {} }, - logger: logger + diagnostics: diagnostics ) end diff --git a/spec/instance_spec.rb b/spec/instance_spec.rb index 2858aad..36de8a6 100644 --- a/spec/instance_spec.rb +++ b/spec/instance_spec.rb @@ -1350,7 +1350,7 @@ ) expect(sdk.get_revision).to eq("2.0") - reader = sdk.instance_variable_get(:@datafile_reader) + reader = sdk.instance_variable_get(:@datafile) expect(reader.featurevisor_version).to eq("3.1.0") expect(sdk.get_feature("firstFeature")).to be_a(Hash) expect(sdk.get_feature("secondFeature")).to be_a(Hash) @@ -1543,6 +1543,69 @@ expect(received_by_module.last).to include(code: "module_warning", module: "reporter") end + it "keeps module diagnostic levels independent from the instance level" do + received = [] + instance = Featurevisor.create_featurevisor( + log_level: "fatal", + modules: [ + { + name: "observer", + setup: lambda do |api| + api[:on_diagnostic].call( + ->(diagnostic) { received << diagnostic }, + log_level: "debug" + ) + end + } + ] + ) + + instance.is_enabled("missing") + + diagnostic = received.find { |item| item[:code] == "feature_not_found" } + expect(diagnostic).not_to be_nil + expect(diagnostic[:details]).to include( + featureKey: "missing", + reason: "feature_not_found" + ) + expect(diagnostic[:details][:evaluation]).to include( + featureKey: "missing", + reason: "feature_not_found" + ) + end + + it "normalizes condition match diagnostics without nesting details" do + diagnostics = [] + instance = Featurevisor.create_featurevisor( + log_level: "debug", + on_diagnostic: ->(diagnostic) { diagnostics << diagnostic } + ) + error = ArgumentError.new("invalid semver") + + instance.send( + :handle_evaluation_diagnostic, + "warn", + "invalid semver", + { + code: "condition_match_error", + error: error, + details: { + condition: { attribute: "version" }, + context: { version: "invalid" } + } + } + ) + + diagnostic = diagnostics.last + expect(diagnostic[:code]).to eq("condition_match_error") + expect(diagnostic[:originalError]).to equal(error) + expect(diagnostic[:details]).to include( + condition: { attribute: "version" }, + context: { version: "invalid" } + ) + expect(diagnostic[:details]).not_to have_key(:details) + end + it "should isolate diagnostic handler failures" do sdk = nil diff --git a/spec/logger_spec.rb b/spec/logger_spec.rb deleted file mode 100644 index d4ceb4e..0000000 --- a/spec/logger_spec.rb +++ /dev/null @@ -1,262 +0,0 @@ -require "featurevisor" - -RSpec.describe Featurevisor.const_get(:Logger) do - let(:console_output) { StringIO.new } - let(:original_stdout) { $stdout } - let(:original_stderr) { $stderr } - - before(:each) do - $stdout = console_output - $stderr = console_output - end - - after(:each) do - $stdout = original_stdout - $stderr = original_stderr - end - - describe "create_logger" do - it "should create a logger with default options" do - logger = Featurevisor.const_get(:Logger).new - expect(logger).to be_instance_of(Featurevisor.const_get(:Logger)) - end - - it "should create a logger with custom level" do - logger = Featurevisor.const_get(:Logger).new(level: "debug") - expect(logger).to be_instance_of(Featurevisor.const_get(:Logger)) - end - - it "should create a logger with custom handler" do - custom_handler = double("custom_handler") - expect(custom_handler).to receive(:call).with("info", "test message", nil) - - logger = Featurevisor.const_get(:Logger).new(handler: custom_handler) - logger.info("test message") - end - end - - describe "Logger" do - describe "constructor" do - it "should use default log level when none provided" do - logger = Featurevisor.const_get(:Logger).new - logger.debug("debug message") - - # Debug should not be logged with default level (info) - expect(console_output.string).not_to include("debug message") - end - - it "should use provided log level" do - logger = Featurevisor.const_get(:Logger).new(level: "debug") - logger.debug("debug message") - - # Debug should be logged with debug level - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("debug message") - end - - it "should use default handler when none provided" do - logger = Featurevisor.const_get(:Logger).new - logger.info("test message") - - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("test message") - end - - it "should use provided handler" do - custom_handler = double("custom_handler") - expect(custom_handler).to receive(:call).with("info", "test message", nil) - - logger = Featurevisor.const_get(:Logger).new(handler: custom_handler) - logger.info("test message") - end - end - - describe "set_level" do - it "should update the log level" do - logger = Featurevisor.const_get(:Logger).new(level: "info") - - # Debug should not be logged initially - logger.debug("debug message") - expect(console_output.string).not_to include("debug message") - - # Set to debug level - logger.set_level("debug") - logger.debug("debug message") - expect(console_output.string).to include("debug message") - end - end - - describe "log level filtering" do - it "should log error messages at all levels" do - levels = %w[debug info warn error] - - levels.each do |level| - console_output.truncate(0) - logger = Featurevisor.const_get(:Logger).new(level: level) - logger.error("error message") - expect(console_output.string).to include("error message") - end - end - - it "should log warn messages at warn level and above" do - logger = Featurevisor.const_get(:Logger).new(level: "warn") - - logger.warn("warn message") - expect(console_output.string).to include("warn message") - - logger.error("error message") - expect(console_output.string).to include("error message") - end - - it "should not log info messages at warn level" do - logger = Featurevisor.const_get(:Logger).new(level: "warn") - - logger.info("info message") - expect(console_output.string).not_to include("info message") - end - - it "should not log debug messages at info level" do - logger = Featurevisor.const_get(:Logger).new(level: "info") - - logger.debug("debug message") - expect(console_output.string).not_to include("debug message") - end - - it "should log all messages at debug level" do - logger = Featurevisor.const_get(:Logger).new(level: "debug") - - logger.debug("debug message") - expect(console_output.string).to include("debug message") - - logger.info("info message") - expect(console_output.string).to include("info message") - - logger.warn("warn message") - expect(console_output.string).to include("warn message") - - logger.error("error message") - expect(console_output.string).to include("error message") - end - end - - describe "convenience methods" do - let(:logger) { Featurevisor.const_get(:Logger).new(level: "debug") } - - it "should call debug method correctly" do - logger.debug("debug message") - expect(console_output.string).to include("debug message") - end - - it "should call info method correctly" do - logger.info("info message") - expect(console_output.string).to include("info message") - end - - it "should call warn method correctly" do - logger.warn("warn message") - expect(console_output.string).to include("warn message") - end - - it "should call error method correctly" do - logger.error("error message") - expect(console_output.string).to include("error message") - end - - it "should call fatal method correctly" do - logger.fatal("fatal message") - expect(console_output.string).to include("fatal message") - end - - it "should handle details parameter" do - details = { key: "value", number: 42 } - - logger.info("message with details", details) - expect(console_output.string).to include("message with details") - expect(console_output.string).to include("key") - expect(console_output.string).to include("value") - expect(console_output.string).to include("42") - end - end - - describe "log method" do - it "should call handler with correct parameters" do - custom_handler = double("custom_handler") - expect(custom_handler).to receive(:call).with("info", "test message", { test: true }) - - logger = Featurevisor.const_get(:Logger).new(handler: custom_handler, level: "debug") - details = { test: true } - - logger.log("info", "test message", details) - end - - it "should not call handler when level is filtered out" do - custom_handler = double("custom_handler") - expect(custom_handler).not_to receive(:call) - - logger = Featurevisor.const_get(:Logger).new(handler: custom_handler, level: "warn") - logger.log("debug", "debug message") - end - end - end - - describe "default_log_handler" do - it "should use puts for debug level" do - Featurevisor.default_log_handler("debug", "debug message") - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("debug message") - end - - it "should use puts for info level" do - Featurevisor.default_log_handler("info", "info message") - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("info message") - end - - it "should use warn for warn level" do - Featurevisor.default_log_handler("warn", "warn message") - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("warn message") - end - - it "should use warn for error level" do - Featurevisor.default_log_handler("error", "error message") - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("error message") - end - - it "should use warn for fatal level" do - Featurevisor.default_log_handler("fatal", "fatal message") - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("fatal message") - end - - it "should handle nil details" do - Featurevisor.default_log_handler("info", "message without details") - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("message without details") - end - - it "should handle provided details" do - details = { key: "value" } - Featurevisor.default_log_handler("info", "message with details", details) - expect(console_output.string).to include("[Featurevisor]") - expect(console_output.string).to include("message with details") - expect(console_output.string).to include("key") - expect(console_output.string).to include("value") - end - end - - describe "constants" do - it "should have correct log levels" do - expect(Featurevisor::LOG_LEVELS).to eq(%w[fatal error warn info debug]) - end - - it "should have correct default log level" do - expect(Featurevisor::DEFAULT_LOG_LEVEL).to eq("info") - end - - it "should have correct logger prefix" do - expect(Featurevisor::LOGGER_PREFIX).to eq("[Featurevisor]") - end - end -end diff --git a/spec/modules_spec.rb b/spec/modules_spec.rb index 990cbd1..b5a0f3f 100644 --- a/spec/modules_spec.rb +++ b/spec/modules_spec.rb @@ -2,7 +2,7 @@ RSpec.describe Featurevisor::Modules do describe "Module" do - let(:logger) { Featurevisor.const_get(:Logger).new(level: "warn") } + let(:diagnostics) { Featurevisor.const_get(:DiagnosticReporter).new(level: "warn") } it "should be a class" do expect(Featurevisor::Modules::FeaturevisorModule).to be_a(Class) @@ -123,11 +123,11 @@ end describe "ModulesManager" do - let(:logger) { Featurevisor.const_get(:Logger).new(level: "warn") } + let(:diagnostics) { Featurevisor.const_get(:DiagnosticReporter).new(level: "warn") } let(:diagnostics) { [] } let(:modules_manager) do Featurevisor::Modules::ModulesManager.new( - logger: logger, + diagnostics: diagnostics, report_diagnostic: ->(diagnostic, _mod = nil) { diagnostics << diagnostic } ) end @@ -281,7 +281,7 @@ it "should initialize with existing modules" do mod = Featurevisor::Modules::FeaturevisorModule.new(name: "test-mod") - manager = Featurevisor::Modules::ModulesManager.new(modules: [mod], logger: logger) + manager = Featurevisor::Modules::ModulesManager.new(modules: [mod], diagnostics: diagnostics) expect(manager.modules).to include(mod) end diff --git a/spec/public_api_spec.rb b/spec/public_api_spec.rb new file mode 100644 index 0000000..b50fa13 --- /dev/null +++ b/spec/public_api_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "featurevisor" + +RSpec.describe "Featurevisor public API" do + it "keeps reader and diagnostic implementation types private" do + expect { Featurevisor::InstanceEvaluationDataProvider }.to raise_error(NameError) + expect { Featurevisor::DiagnosticReporter }.to raise_error(NameError) + expect(Featurevisor.const_defined?(:DatafileReader, false)).to be(false) + expect(Featurevisor.const_defined?(:Logger, false)).to be(false) + end +end