diff --git a/.sampo/changesets/roguish-lady-louhi.md b/.sampo/changesets/roguish-lady-louhi.md new file mode 100644 index 0000000..00af211 --- /dev/null +++ b/.sampo/changesets/roguish-lady-louhi.md @@ -0,0 +1,5 @@ +--- +hex/posthog: patch +--- + +Honor the definitions snapshot property_matching_version in local feature flag evaluation and shared definition caches. Missing or version 1 now intentionally matches released service legacy boolean truthiness rather than the former SDK behavior; version 2 uses explicit equality, with recursive truthiness for empty filters. Correct known-null complements and composite equality normalization. diff --git a/lib/posthog/feature_flags/definition_loader.ex b/lib/posthog/feature_flags/definition_loader.ex index 381af4c..08ea830 100644 --- a/lib/posthog/feature_flags/definition_loader.ex +++ b/lib/posthog/feature_flags/definition_loader.ex @@ -166,6 +166,7 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do flags_by_key: %{String.t() => map()}, group_type_mapping: map(), cohorts: map(), + property_matching_version: term(), minimal_flag_called_events: boolean() } @@ -534,6 +535,7 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do if is_list(flags) and is_map(mapping) and is_map(cohorts) do minimal = value(body, "minimal_flag_called_events") == true + property_matching_version = value(body, "property_matching_version") snapshot = %{ flags: flags, @@ -543,6 +545,7 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do end), group_type_mapping: mapping, cohorts: cohorts, + property_matching_version: property_matching_version, minimal_flag_called_events: minimal } @@ -550,6 +553,7 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do "flags" => flags, "group_type_mapping" => mapping, "cohorts" => cohorts, + "property_matching_version" => property_matching_version, "minimal_flag_called_events" => minimal } @@ -571,6 +575,9 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do defp value(map, "minimal_flag_called_events"), do: Map.get(map, "minimal_flag_called_events", Map.get(map, :minimal_flag_called_events)) + defp value(map, "property_matching_version"), + do: Map.get(map, "property_matching_version", Map.get(map, :property_matching_version, 1)) + defp value(map, "key"), do: Map.get(map, "key", Map.get(map, :key)) defp response_etag(%{headers: headers}), do: header_value(headers, "etag") diff --git a/lib/posthog/feature_flags/flag_definition_cache_provider.ex b/lib/posthog/feature_flags/flag_definition_cache_provider.ex index 20a9ba0..e0ee09e 100644 --- a/lib/posthog/feature_flags/flag_definition_cache_provider.ex +++ b/lib/posthog/feature_flags/flag_definition_cache_provider.ex @@ -6,6 +6,9 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProvider do `:flag_definition_cache_provider`. The definition loader bounds and isolates every callback. Cached values must be maps containing `flags`, `group_type_mapping`, and `cohorts` (string or atom keys are accepted). + Preserve the complete envelope, including `property_matching_version`, with + the definitions. Only version 2 selects explicit property matching; older + cached envelopes omitting the version use released service legacy matching. A minimal provider can coordinate fetching and keep the complete envelope in an application-owned cache: diff --git a/lib/posthog/feature_flags/local_evaluator.ex b/lib/posthog/feature_flags/local_evaluator.ex index 477e312..dd33702 100644 --- a/lib/posthog/feature_flags/local_evaluator.ex +++ b/lib/posthog/feature_flags/local_evaluator.ex @@ -434,9 +434,19 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do when is_map(property) do {result, cache} = case value(property, "type") do - "cohort" -> {match_cohort(property, property_values, definitions, context, cache), cache} - "flag" -> match_dependency(property, definitions, context, cache) - _ -> {match_property(property, property_values, context.now), cache} + "cohort" -> + {match_cohort(property, property_values, definitions, context, cache), cache} + + "flag" -> + match_dependency(property, definitions, context, cache) + + _ -> + {match_property( + property, + property_values, + context.now, + Map.get(definitions, :property_matching_version, 1) + ), cache} end {apply_negation(result, value(property, "negation") == true), cache} @@ -601,7 +611,7 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do end end - defp match_property(property, property_values, now) do + defp match_property(property, property_values, now, property_matching_version) do key = value(property, "key") operator = value(property, "operator") || "exact" filter_value = value(property, "value") @@ -610,6 +620,10 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do :error -> :inconclusive + {:ok, property_value} when operator in ["exact", "is_not"] -> + match = exact_match?(property_value, filter_value, property_matching_version) + boolean_result(if(operator == "exact", do: match, else: not match)) + {:ok, property_value} -> apply_operator(operator, property_value, filter_value, now) end @@ -618,24 +632,8 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do defp apply_operator("is_set", _property, _filter, _now), do: :match defp apply_operator("is_not_set", _property, _filter, _now), do: :no_match - defp apply_operator("is_not", nil, filter, _now) do - match = if is_list(filter), do: nil in filter, else: is_nil(filter) - boolean_result(not match) - end - defp apply_operator(_operator, nil, _filter, _now), do: :no_match - defp apply_operator(operator, property, filter, _now) when operator in ["exact", "is_not"] do - match = - if is_list(filter) do - Enum.any?(filter, &case_insensitive_equal?(property, &1)) - else - case_insensitive_equal?(property, filter) - end - - boolean_result(if(operator == "exact", do: match, else: not match)) - end - defp apply_operator(operator, property, filter, _now) when operator in [ "icontains", @@ -775,8 +773,104 @@ defmodule PostHog.FeatureFlags.LocalEvaluator do defp boolean_result(true), do: :match defp boolean_result(false), do: :no_match - defp case_insensitive_equal?(left, right), - do: String.downcase(to_string(left)) == String.downcase(to_string(right)) + defp exact_match?(property, filter, property_matching_version) do + cond do + # The service treats an empty filter as recursive ALL truthiness in both modes. + filter == [] -> + truthy?(property) + + property_matching_version != 2 and boolean_like?(filter) -> + truthy?(property) == truthy?(filter) + + is_list(filter) -> + Enum.any?(filter, &case_insensitive_equal?(property, &1)) + + true -> + case_insensitive_equal?(property, filter) + end + end + + defp boolean_like?(value) when is_boolean(value), do: true + defp boolean_like?(value) when is_binary(value), do: String.downcase(value) in ["true", "false"] + defp boolean_like?(value) when is_list(value), do: Enum.all?(value, &boolean_like?/1) + defp boolean_like?(_value), do: false + + defp truthy?(value) when is_boolean(value), do: value + defp truthy?(value) when is_binary(value), do: String.downcase(value) == "true" + defp truthy?(value) when is_list(value), do: Enum.all?(value, &truthy?/1) + + # Jason sends non-boolean atoms as strings, but a struct's encoder is opaque. + defp truthy?(value) when is_atom(value) and not is_nil(value), + do: value |> Atom.to_string() |> truthy?() + + defp truthy?(value) when is_struct(value) do + raise ArgumentError, "opaque JSON struct truthiness" + end + + defp truthy?(_value), do: false + + defp case_insensitive_equal?(left, right) do + left_string = exact_string(left) + right_string = exact_string(right) + + # Rust lowercases sigma contextually; String.downcase/1 does not. Keep newly + # supported composite comparisons inconclusive if either side could differ. + if (composite_json?(left) or composite_json?(right)) and + (String.contains?(left_string, "Σ") or String.contains?(right_string, "Σ")) do + raise ArgumentError, "ambiguous composite Unicode casing" + end + + String.downcase(left_string) == String.downcase(right_string) + end + + defp composite_json?(value), do: is_list(value) or (is_map(value) and not is_struct(value)) + + defp exact_string(value) when is_struct(value), do: to_string(value) + + # Lists must remain whole JSON values, not Elixir charlists; known null is not missing. + defp exact_string(value) when is_nil(value) or is_list(value) or is_map(value), + do: value |> sort_json_objects() |> Jason.encode!() + + defp exact_string(value), do: to_string(value) + + # Nested encoders can hide unsorted keys and ambiguous numbers from normalization. + # Top-level scalar structs use exact_string/1 instead. OrderedObjects generated + # below contain already-normalized members and are never passed back through here. + defp sort_json_objects(value) when is_struct(value) do + raise ArgumentError, "opaque composite JSON struct" + end + + # Map iteration order is not JSON key order, including for mixed atom/string keys. + defp sort_json_objects(value) when is_map(value) and not is_struct(value) do + members = + Enum.map(value, fn {key, member} -> + # Jason uses atom names for keys, including nil as "nil", not "". + key = if is_atom(key), do: Atom.to_string(key), else: to_string(key) + {key, sort_json_objects(member)} + end) + + # JSON parsing on the service drops duplicate keys, unlike Jason.OrderedObject. + if length(Enum.uniq_by(members, &elem(&1, 0))) != map_size(value) do + raise ArgumentError, "ambiguous composite JSON keys" + end + + members + |> Enum.sort_by(&elem(&1, 0)) + |> Jason.OrderedObject.new() + end + + defp sort_json_objects(value) when is_list(value), do: Enum.map(value, &sort_json_objects/1) + + # Jason and serde_json differ for floats and integers outside the service's i64/u64 range. + # Leave these composite comparisons inconclusive through the existing evaluation boundary. + defp sort_json_objects(value) + when is_float(value) or + (is_integer(value) and + (value < -9_223_372_036_854_775_808 or value > 18_446_744_073_709_551_615)) do + raise ArgumentError, "ambiguous composite JSON number" + end + + defp sort_json_objects(value), do: value defp ascii_downcase(value) do for <>, into: "" do diff --git a/test/posthog/feature_flags/flag_definition_cache_provider_test.exs b/test/posthog/feature_flags/flag_definition_cache_provider_test.exs index f37e6cd..93e1e8b 100644 --- a/test/posthog/feature_flags/flag_definition_cache_provider_test.exs +++ b/test/posthog/feature_flags/flag_definition_cache_provider_test.exs @@ -71,6 +71,113 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do start_supervised!({PostHog.Supervisor, cfg}) end + test "matching version round trips through providers and version-only hydration replaces results" do + owner = self() + # No API request stub: only the owned definitions GET below is permitted. + stub(PostHog.API.Mock, :client, fn _key, _host -> + %PostHog.API.Client{client: :stub_client, module: PostHog.API.Mock} + end) + + property = %{"key" => "prop", "value" => false} + cohort = %{"type" => "AND", "values" => [%{"type" => "OR", "values" => [property]}]} + + flags = + for {key, properties, extra} <- [ + {"person", [property], %{}}, + {"group", [property], %{"aggregation_group_type_index" => 0}}, + {"cohort", [%{"type" => "cohort", "value" => 1}], %{}} + ] do + %{ + "key" => key, + "active" => true, + "filters" => Map.merge(%{"groups" => [%{"properties" => properties}]}, extra) + } + end + + wire = + envelope("unused") + |> Map.put("flags", flags) + |> Map.put("cohorts", %{"1" => cohort}) + |> Map.put("property_matching_version", 2) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: wire, headers: %{}}} + end) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: true, read: nil, store: :ok, shutdown: :ok} + end) + + start_instance(__MODULE__.VersionOwner, provider) + assert DefinitionLoader.ready?(__MODULE__.VersionOwner) + assert_receive {:stored, stored} + assert stored == wire + + Agent.update(provider, &%{&1 | decision: false, read: stored}) + name = __MODULE__.VersionReader + start_instance(name, provider) + + context = %{ + distinct_id: "user", + person_properties: %{prop: "banana"}, + groups: %{organization: "org"}, + group_properties: %{organization: %{prop: "banana"}}, + only_evaluate_locally: true + } + + assert {:ok, frozen} = PostHog.FeatureFlags.evaluate_flags(name, context) + + assert Enum.all?(frozen.flags, fn {_key, result} -> + not result.enabled and result.locally_evaluated + end) + + initial = DefinitionLoader.definitions(name) + assert initial.property_matching_version == 2 + + for read <- [nil, {:raise, "unavailable"}, %{"flags" => []}] do + Agent.update(provider, &%{&1 | read: read}) + capture_log(fn -> DefinitionLoader.refresh(name) end) + assert DefinitionLoader.definitions(name) == initial + end + + for version <- [1, 2, 1, 2, :missing] do + # Atom-key cache documents are supported alongside JSON string keys. + cached = %{ + flags: flags, + cohorts: %{"1" => cohort}, + group_type_mapping: %{"0" => "organization"}, + minimal_flag_called_events: true + } + + cached = + if version == :missing, + do: cached, + else: Map.put(cached, :property_matching_version, version) + + Agent.update(provider, &%{&1 | read: cached}) + assert :ok = DefinitionLoader.refresh(name) + current = DefinitionLoader.definitions(name) + assert current.flags == initial.flags + assert current.cohorts == initial.cohorts + assert current.group_type_mapping == initial.group_type_mapping + assert current.property_matching_version == if(version == :missing, do: 1, else: version) + assert {:ok, result} = PostHog.FeatureFlags.evaluate_flags(name, context) + assert map_size(result.flags) == 3 + + for {_key, flag} <- result.flags do + assert flag.enabled == (version != 2) + assert flag.locally_evaluated + end + + assert Enum.all?(frozen.flags, fn {_key, result} -> not result.enabled end) + end + + assert DefinitionLoader.definitions(__MODULE__.VersionOwner).property_matching_version == 2 + stop_supervised(name) + stop_supervised(__MODULE__.VersionOwner) + end + test "negative decision reads complete cached definitions without an API request" do stub_with(PostHog.API.Mock, PostHog.API.Stub) diff --git a/test/posthog/feature_flags/local_evaluation_integration_test.exs b/test/posthog/feature_flags/local_evaluation_integration_test.exs index 2d58c27..4bd62b1 100644 --- a/test/posthog/feature_flags/local_evaluation_integration_test.exs +++ b/test/posthog/feature_flags/local_evaluation_integration_test.exs @@ -50,6 +50,77 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do start_supervised!({PostHog.Supervisor, config}) end + test "HTTP version-only refreshes replace matching atomically and preserve frozen results" do + definitions = envelope([flag("versioned", [%{"key" => "prop", "value" => false}])]) + + context = %{ + distinct_id: "user", + person_properties: %{prop: "banana"}, + only_evaluate_locally: true + } + + name = __MODULE__.Versioned + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: definitions, headers: %{}}} + end) + + start_instance(name) + assert {:ok, frozen} = FeatureFlags.evaluate_flags(name, context) + assert frozen.flags["versioned"].enabled + assert FeatureFlags.DefinitionLoader.definitions(name).property_matching_version == 1 + + for {response, version} <- [ + {{:ok, %{status: 200, body: Map.put(definitions, "property_matching_version", 1)}}, 1}, + {{:ok, + %{ + status: 200, + body: Map.put(definitions, "property_matching_version", 2), + headers: %{"etag" => "v2"} + }}, 2}, + {{:ok, %{status: 304, body: nil}}, 2}, + {{:ok, %{status: 503, body: %{}}}, 2}, + {{:error, :timeout}, 2}, + {{:ok, %{status: 200, body: %{"flags" => []}}}, 2}, + {{:ok, %{status: 200, body: Map.put(definitions, "property_matching_version", 1)}}, 1}, + {{:ok, %{status: 200, body: Map.put(definitions, "property_matching_version", 2)}}, 2}, + {{:ok, %{status: 200, body: definitions}}, 1}, + {{:ok, %{status: 200, body: Map.put(definitions, "property_matching_version", 2)}}, 2}, + {{:ok, %{status: 401, body: %{}}}, nil}, + {{:ok, %{status: 200, body: definitions}}, 1} + ] do + before = FeatureFlags.DefinitionLoader.definitions(name) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", opts -> + if match?({:ok, %{status: 304}}, response), + do: assert({"if-none-match", "v2"} in opts[:headers]) + + response + end) + + ExUnit.CaptureLog.capture_log(fn -> FeatureFlags.DefinitionLoader.refresh(name) end) + current = FeatureFlags.DefinitionLoader.definitions(name) + assert frozen.flags["versioned"].enabled + + if is_nil(version) do + assert current == nil + else + assert current.property_matching_version == version + assert current.flags == definitions["flags"] + + if match?({:ok, %{status: 304}}, response) or match?({:ok, %{status: 503}}, response) or + match?({:error, _}, response) or match?({:ok, %{body: %{"flags" => []}}}, response), + do: assert(current == before) + + expected = version != 2 + assert {:ok, result} = FeatureFlags.evaluate_flags(name, context) + assert result.flags["versioned"].enabled == expected + assert result.flags["versioned"].locally_evaluated + assert {:ok, ^expected} = FeatureFlags.check(name, "versioned", context) + end + end + end + test "matching local boolean and variant payload produce a frozen snapshot without /flags" do variant = flag("variant", [], %{ @@ -142,6 +213,197 @@ defmodule PostHog.FeatureFlags.LocalEvaluationIntegrationTest do assert snapshot.flags["extra"].enabled end + for version <- [1, 2], operator <- ["exact", "is_not"] do + @matching_version version + @matching_operator operator + test "version #{version} #{operator} falls back for composite numeric serialization" do + condition = %{ + "key" => "prop", + "operator" => @matching_operator, + "value" => ~s({"n":0.00001}) + } + + expected = @matching_operator == "exact" + + definitions = + [flag("local"), flag("numeric-composite", [condition])] + |> envelope() + |> Map.put("property_matching_version", @matching_version) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: definitions, headers: %{}}} + end) + + name = __MODULE__.CompositeNumbers + start_instance(name) + context = %{distinct_id: "user", person_properties: %{prop: %{"n" => 0.00001}}} + + assert {:ok, local_only} = + FeatureFlags.evaluate_flags(name, Map.put(context, :only_evaluate_locally, true)) + + assert Evaluations.keys(local_only) == ["local"] + + expect(PostHog.API.Mock, :request, fn :stub_client, :post, "/flags", opts -> + assert opts[:json].person_properties == context.person_properties + + {:ok, + %{ + status: 200, + body: %{"flags" => %{"numeric-composite" => %{"enabled" => expected}}} + }} + end) + + assert {:ok, result} = FeatureFlags.evaluate_flags(name, context) + assert result.flags["local"].locally_evaluated + assert result.flags["numeric-composite"].enabled == expected + refute result.flags["numeric-composite"].locally_evaluated + end + end + + for version <- [1, 2], operator <- ["exact", "is_not"] do + @matching_version version + @matching_operator operator + test "version #{version} #{operator} falls back for composite sigma casing" do + condition = %{ + "key" => "prop", + "operator" => @matching_operator, + "value" => %{"name" => "ος"} + } + + expected = @matching_operator == "exact" + + definitions = + [flag("local"), flag("unicode", [condition])] + |> envelope() + |> Map.put("property_matching_version", @matching_version) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: definitions, headers: %{}}} + end) + + name = __MODULE__.CompositeUnicode + start_instance(name) + context = %{distinct_id: "user", person_properties: %{prop: %{"name" => "ΟΣ"}}} + + assert {:ok, local_only} = + FeatureFlags.evaluate_flags(name, Map.put(context, :only_evaluate_locally, true)) + + assert Evaluations.keys(local_only) == ["local"] + + expect(PostHog.API.Mock, :request, fn :stub_client, :post, "/flags", opts -> + assert opts[:json].person_properties == context.person_properties + + {:ok, %{status: 200, body: %{"flags" => %{"unicode" => %{"enabled" => expected}}}}} + end) + + assert {:ok, result} = FeatureFlags.evaluate_flags(name, context) + assert result.flags["local"].locally_evaluated + assert result.flags["unicode"].enabled == expected + refute result.flags["unicode"].locally_evaluated + end + end + + for version <- [:missing, 1, 2], operator <- ["exact", "is_not"] do + @matching_version version + @matching_operator operator + test "version #{version} #{operator} omits nested structs locally and falls back to /flags" do + wire = %{"nested" => [%{"a" => 2, "z" => 1}]} + condition = %{"key" => "prop", "operator" => @matching_operator, "value" => wire} + definitions = envelope([flag("local"), flag("opaque", [condition])]) + + definitions = + if @matching_version == :missing, + do: definitions, + else: Map.put(definitions, "property_matching_version", @matching_version) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: definitions, headers: %{}}} + end) + + name = __MODULE__.NestedStructs + start_instance(name) + expected = @matching_operator == "exact" + + for value <- [ + Jason.OrderedObject.new([{"z", 1}, {"a", 2}]), + %PostHog.Test.LocalEvaluatorStructs.CustomValue{payload: %{:z => 1, "a" => 2}} + ] do + context = %{distinct_id: "user", person_properties: %{prop: %{"nested" => [value]}}} + + assert {:ok, local_only} = + FeatureFlags.evaluate_flags(name, Map.put(context, :only_evaluate_locally, true)) + + assert Evaluations.keys(local_only) == ["local"] + + expect(PostHog.API.Mock, :request, fn :stub_client, :post, "/flags", opts -> + assert opts[:json].person_properties == context.person_properties + + assert opts[:json].person_properties |> Jason.encode!() |> Jason.decode!() == + %{"prop" => wire} + + {:ok, %{status: 200, body: %{"flags" => %{"opaque" => %{"enabled" => expected}}}}} + end) + + assert {:ok, result} = FeatureFlags.evaluate_flags(name, context) + assert result.flags["local"].locally_evaluated + assert result.flags["opaque"].enabled == expected + refute result.flags["opaque"].locally_evaluated + end + + # Decoded JSON still resolves locally, including normalized internal OrderedObjects. + context = %{distinct_id: "user", person_properties: %{prop: wire}} + assert {:ok, result} = FeatureFlags.evaluate_flags(name, context) + assert result.flags["opaque"].enabled == expected + assert result.flags["opaque"].locally_evaluated + end + end + + for version <- [:missing, 1, 2], operator <- ["exact", "is_not"] do + @matching_version version + @matching_operator operator + test "version #{version} #{operator} falls back for opaque scalar truthiness" do + filter = if @matching_version == 2, do: [], else: true + condition = %{"key" => "prop", "operator" => @matching_operator, "value" => filter} + definitions = envelope([flag("local"), flag("opaque-truthiness", [condition])]) + + definitions = + if @matching_version == :missing, + do: definitions, + else: Map.put(definitions, "property_matching_version", @matching_version) + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: definitions, headers: %{}}} + end) + + name = __MODULE__.OpaqueTruthiness + start_instance(name) + expected = @matching_operator == "exact" + scalar = %PostHog.Test.LocalEvaluatorStructs.CustomValue{payload: "true"} + + for {property, wire} <- [{scalar, "true"}, {[true, [scalar]], [true, ["true"]]}] do + context = %{distinct_id: "user", person_properties: %{prop: property}} + + assert {:ok, local_only} = + FeatureFlags.evaluate_flags(name, Map.put(context, :only_evaluate_locally, true)) + + assert Evaluations.keys(local_only) == ["local"] + + expect(PostHog.API.Mock, :request, fn :stub_client, :post, "/flags", opts -> + assert opts[:json].person_properties |> Jason.encode!() |> Jason.decode!() == + %{"prop" => wire} + + {:ok, + %{status: 200, body: %{"flags" => %{"opaque-truthiness" => %{"enabled" => expected}}}}} + end) + + assert {:ok, result} = FeatureFlags.evaluate_flags(name, context) + assert result.flags["local"].locally_evaluated + assert result.flags["opaque-truthiness"].enabled == expected + refute result.flags["opaque-truthiness"].locally_evaluated + end + end + end + test "snapshot-level remote errors are logged for locally resolved flags" do unknown = flag("unknown", [%{"key" => "x", "operator" => "future", "value" => 1}]) diff --git a/test/posthog/feature_flags/local_evaluator_test.exs b/test/posthog/feature_flags/local_evaluator_test.exs index cc1f488..4d8510f 100644 --- a/test/posthog/feature_flags/local_evaluator_test.exs +++ b/test/posthog/feature_flags/local_evaluator_test.exs @@ -3,6 +3,8 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do alias PostHog.FeatureFlags.LocalEvaluator alias PostHog.FeatureFlags.Result + alias PostHog.Test.LocalEvaluatorStructs.CustomValue + alias PostHog.Test.LocalEvaluatorStructs.DerivedObject defp snapshot(flags, extras \\ %{}) do Map.merge( @@ -34,6 +36,475 @@ defmodule PostHog.FeatureFlags.LocalEvaluatorTest do LocalEvaluator.evaluate(snapshot([flag]), context, [flag["key"]]) end + # Expected values are the released service v1 and explicit v2 results, not + # Elixir truthiness or the SDK's former unversioned array membership behavior. + for version <- [:missing, 1, 2, 0, 3, "2", nil] do + @matching_version version + test "version #{inspect(version)} selects service exact matching and is_not complements" do + rows = [ + {false, "banana", true, false}, + {false, 0, true, false}, + {false, 1, true, false}, + {"FaLsE", "banana", true, false}, + {["FALSE"], "banana", true, false}, + {["true", "false"], "true", false, true}, + {["true", "false"], "pro", true, false}, + {[], true, true, true}, + {[], [], true, true}, + {true, [true], true, false}, + {"TrUe", [true], true, false}, + {[true], [true], true, false}, + {true, [], true, false}, + {false, "FALSE", true, true}, + {false, nil, true, false}, + {false, "", true, false}, + {false, %{}, true, false}, + {[], [true, ["TRUE", []]], true, true}, + {[], [true, [false]], false, false}, + {[], false, false, false}, + {[], nil, false, false}, + {[], 0, false, false}, + {[], 1, false, false}, + {[], "banana", false, false}, + {["FREE", "PRO"], "pro", true, true}, + {[false, "PRO"], "pro", true, true}, + {[false, "PRO"], "banana", false, false}, + {[[true], "PRO"], [true], true, true}, + {[["ÉLITE", true], "PRO"], ["élite", true], true, true}, + {[[true], ["FALSE"]], "banana", true, false}, + {["TrUe", "FALSE"], true, false, true}, + {["TrUe", "FALSE"], false, true, true}, + {nil, nil, true, true}, + {[nil], nil, true, true}, + {nil, "", false, false}, + {%{"plan" => "PRO"}, %{"plan" => "pro"}, true, true}, + {"ÉLITE", "élite", true, true}, + {[1, "pro"], "1", true, true} + ] + + for {filter, property, legacy, explicit} <- rows, + operator <- ["exact", "is_not"] do + condition = %{"key" => "prop", "operator" => operator, "value" => filter} + definitions = versioned_snapshot([flag("versioned", [condition])], @matching_version) + context = %{distinct_id: "user", person_properties: %{"prop" => property}} + result = LocalEvaluator.evaluate(definitions, context) + expected = if @matching_version == 2, do: explicit, else: legacy + expected = if operator == "is_not", do: not expected, else: expected + + assert %Result{enabled: ^expected, locally_evaluated: true} = result.results["versioned"], + inspect({@matching_version, operator, filter, property, expected, result}) + + assert MapSet.size(result.unresolved) == 0 + end + end + end + + for {name, property, canonical} <- [ + {"mixed atom and string keys", %{"a" => true, :z => true}, ~s({"a":true,"z":true})}, + {"nested objects and arrays", %{"a" => [%{"b" => 1, :z => 2}], :z => %{a: true}}, + ~s({"a":[{"b":1,"z":2}],"z":{"a":true}})}, + {"large string-key maps", + Map.new(1..40, &{"key#{String.pad_leading("#{&1}", 2, "0")}", &1}), + "{" <> + Enum.map_join(1..40, ",", &~s("key#{String.pad_leading("#{&1}", 2, "0")}":#{&1})) <> + "}"} + ] do + @composite_property property + @canonical_json canonical + test "exact and is_not use recursively sorted JSON for #{name}" do + for version <- [:missing, 1, 2], + operator <- ["exact", "is_not"], + {filter, property} <- [ + {@canonical_json, @composite_property}, + {@composite_property, @canonical_json}, + {[@composite_property], @canonical_json} + ] do + condition = %{"key" => "prop", "operator" => operator, "value" => filter} + definitions = versioned_snapshot([flag("canonical", [condition])], version) + context = %{distinct_id: "user", person_properties: %{"prop" => property}} + result = LocalEvaluator.evaluate(definitions, context) + expected = operator == "exact" + + assert %Result{enabled: ^expected, locally_evaluated: true} = result.results["canonical"], + inspect({version, operator, filter, property, result}) + + assert MapSet.size(result.unresolved) == 0 + end + end + end + + for version <- [:missing, 1, 2], operator <- ["exact", "is_not"] do + @matching_version version + @matching_operator operator + test "version #{version} #{@matching_operator} leaves composite sigma casing inconclusive" do + for {upper, lower} <- [ + {%{"name" => "ΟΣ"}, %{"name" => "ος"}}, + {%{"name" => "ΟΣ"}, %{"name" => "οσ"}}, + {%{"ΟΣ" => [%{"name" => "ΟΣ"}]}, %{"ος" => [%{"name" => "ος"}]}}, + {[%{"name" => "ΟΣ"}], [%{"name" => "ος"}]} + ], + {filter, property} <- [ + {lower, upper}, + {upper, lower}, + {Jason.encode!(upper), lower}, + {lower, Jason.encode!(upper)}, + {[upper], lower}, + {[lower], upper} + ] do + condition = %{"key" => "prop", "operator" => @matching_operator, "value" => filter} + definitions = versioned_snapshot([flag("unicode", [condition])], @matching_version) + context = %{distinct_id: "user", person_properties: %{"prop" => property}} + result = LocalEvaluator.evaluate(definitions, context) + + assert result.results == %{}, inspect({filter, property, result}) + assert result.unresolved == MapSet.new(["unicode"]) + end + end + end + + for version <- [:missing, 1, 2], operator <- ["exact", "is_not"] do + @matching_version version + @matching_operator operator + test "version #{version} #{operator} normalizes nil atom keys like Jason" do + for {composite, wire} <- [ + {%{nil => 1}, %{"nil" => 1}}, + {%{"nested" => [%{nil => 1}]}, %{"nested" => [%{"nil" => 1}]}}, + {[%{nil => 1}], [%{"nil" => 1}]}, + {%{nil => 1, "" => 2, true => 3, false => 4, :a => 5}, + %{"nil" => 1, "" => 2, "true" => 3, "false" => 4, "a" => 5}} + ] do + assert composite |> Jason.encode!() |> Jason.decode!() == wire + + for {filter, property} <- [{[wire], composite}, {[composite], wire}] do + condition = %{"key" => "prop", "operator" => @matching_operator, "value" => filter} + definitions = versioned_snapshot([flag("nil-keys", [condition])], @matching_version) + context = %{distinct_id: "user", person_properties: %{"prop" => property}} + result = LocalEvaluator.evaluate(definitions, context) + expected = @matching_operator == "exact" + + assert( + %Result{enabled: ^expected, locally_evaluated: true} = result.results["nil-keys"], + inspect({filter, property, result}) + ) + + assert MapSet.size(result.unresolved) == 0 + end + end + end + + test "version #{version} #{operator} leaves colliding composite keys inconclusive" do + for {composite, candidates} <- [ + {%{nil => 1, "nil" => 2}, [%{"nil" => 1}, %{"nil" => 2}]}, + {%{"nested" => [%{nil => 1, "nil" => 2}]}, + [%{"nested" => [%{"nil" => 1}]}, %{"nested" => [%{"nil" => 2}]}]}, + {%{:a => 1, "a" => 2}, [%{"a" => 1}, %{"a" => 2}]}, + {%{"nested" => [%{:a => 1, "a" => 2}]}, + [%{"nested" => [%{"a" => 1}]}, %{"nested" => [%{"a" => 2}]}]} + ], + {filter, property} <- [ + {candidates, composite}, + {[composite], hd(candidates)} + ] do + condition = %{"key" => "prop", "operator" => @matching_operator, "value" => filter} + definitions = versioned_snapshot([flag("colliding-keys", [condition])], @matching_version) + context = %{distinct_id: "user", person_properties: %{"prop" => property}} + result = LocalEvaluator.evaluate(definitions, context) + + assert result.results == %{}, inspect({filter, property, result}) + assert result.unresolved == MapSet.new(["colliding-keys"]) + end + end + end + + for version <- [:missing, 1, 2], + operator <- ["exact", "is_not"], + family <- [:ordered, :derived_ordering, :derived_keys, :derived_number, :custom] do + @matching_version version + @matching_operator operator + @encoder_family family + test "version #{version} #{operator} leaves nested #{family} encoders inconclusive" do + values = + case @encoder_family do + :ordered -> + [Jason.OrderedObject.new([{"z", 1}, {"a", 2}])] + + :derived_ordering -> + [%DerivedObject{payload: %{:z => 1, "a" => 2}}] + + :derived_keys -> + [%DerivedObject{payload: %{:a => 1, "a" => 2}}] + + :derived_number -> + [%DerivedObject{payload: 0.00001}] + + :custom -> + Enum.map( + [%{:z => 1, "a" => 2}, [%{n: 0.00001}], "plain"], + &%CustomValue{payload: &1} + ) + end + + for value <- values do + refute Jason.Encoder.impl_for(value) == Jason.Encoder.Any + + for composite <- [%{"nested" => value}, [%{"nested" => [value]}]] do + wire = composite |> Jason.encode!() |> Jason.decode!() + service_string = composite |> Jason.encode!() |> String.replace("1.0e-5", "0.00001") + + for {filter, property} <- [ + {[service_string], composite}, + {[composite], service_string}, + {[wire], composite}, + {[composite], wire} + ] do + condition = %{"key" => "prop", "operator" => @matching_operator, "value" => filter} + definitions = versioned_snapshot([flag("opaque", [condition])], @matching_version) + context = %{distinct_id: "user", person_properties: %{"prop" => property}} + result = LocalEvaluator.evaluate(definitions, context) + + assert result.results == %{}, inspect({filter, property, result}) + assert result.unresolved == MapSet.new(["opaque"]) + end + end + end + end + end + + test "nested scalar structs fall back without changing top-level scalar matching" do + for version <- [:missing, 1, 2], + operator <- ["exact", "is_not"], + value <- [~D[2025-01-01], ~T[12:34:56], %CustomValue{payload: "plain"}], + {filter, property} <- [{[to_string(value)], value}, {[value], to_string(value)}] do + condition = %{"key" => "prop", "operator" => operator, "value" => filter} + definitions = versioned_snapshot([flag("scalar", [condition])], version) + context = %{distinct_id: "user", person_properties: %{prop: property}} + result = LocalEvaluator.evaluate(definitions, context) + expected = operator == "exact" + assert %Result{enabled: ^expected} = result.results["scalar"] + assert result.unresolved == MapSet.new() + + nested_condition = %{condition | "value" => [%{"nested" => hd(filter)}]} + definitions = versioned_snapshot([flag("scalar", [nested_condition])], version) + context = %{context | person_properties: %{prop: %{"nested" => property}}} + result = LocalEvaluator.evaluate(definitions, context) + assert result.results == %{} + assert result.unresolved == MapSet.new(["scalar"]) + end + end + + for version <- [:missing, 1, 2], operator <- ["exact", "is_not"] do + @matching_version version + @matching_operator operator + test "version #{version} #{operator} leaves opaque property truthiness inconclusive" do + filters = if @matching_version == 2, do: [[]], else: [true, false, [true], []] + scalar = %CustomValue{payload: "true"} + assert scalar |> Jason.encode!() |> Jason.decode!() == "true" + + for property <- [scalar, [scalar], [true, [scalar]]], filter <- filters do + condition = %{"key" => "prop", "operator" => @matching_operator, "value" => filter} + + definitions = + versioned_snapshot([flag("opaque-truthiness", [condition])], @matching_version) + + context = %{distinct_id: "user", person_properties: %{prop: property}} + result = LocalEvaluator.evaluate(definitions, context) + assert result.results == %{}, inspect({filter, property, result}) + assert result.unresolved == MapSet.new(["opaque-truthiness"]) + end + end + + test "version #{version} #{operator} uses wire-string truthiness for native atoms" do + filters = if @matching_version == 2, do: [[]], else: [true, [true], []] + + for {property, truthy} <- [ + {:TRUE, true}, + {[:TRUE, [:TrUe]], true}, + {:FALSE, false}, + {:banana, false}, + {nil, false}, + {false, false}, + {true, true}, + {%{nested: :TRUE}, false} + ], + filter <- filters do + wire = property |> Jason.encode!() |> Jason.decode!() + condition = %{"key" => "prop", "operator" => @matching_operator, "value" => filter} + + definitions = + versioned_snapshot([flag("atom-truthiness", [condition])], @matching_version) + + expected = if @matching_operator == "exact", do: truthy, else: not truthy + + for value <- [property, wire] do + context = %{distinct_id: "user", person_properties: %{prop: value}} + result = LocalEvaluator.evaluate(definitions, context) + assert %Result{enabled: ^expected} = result.results["atom-truthiness"] + assert result.unresolved == MapSet.new() + end + end + end + end + + test "opaque structs do not change legacy or empty-filter truthiness" do + property = %{"nested" => %DerivedObject{payload: 0.00001}} + + for {version, filter} <- [{:missing, false}, {1, [false]}, {1, false}, {2, []}], + operator <- ["exact", "is_not"] do + condition = %{"key" => "prop", "operator" => operator, "value" => filter} + definitions = versioned_snapshot([flag("truthiness", [condition])], version) + + result = + LocalEvaluator.evaluate(definitions, %{ + distinct_id: "user", + person_properties: %{prop: property} + }) + + expected = version != 2 == (operator == "exact") + assert %Result{enabled: ^expected} = result.results["truthiness"] + assert result.unresolved == MapSet.new() + end + end + + test "composite numeric serialization ambiguity stays inconclusive" do + for version <- [:missing, 1, 2], + operator <- ["exact", "is_not"], + {number, service_json} <- [ + {0.00001, "0.00001"}, + {1.0e-7, "1e-7"}, + {1.0e20, "1e20"}, + {18_446_744_073_709_551_616, "1.8446744073709552e19"}, + {-9_223_372_036_854_775_809, "-9.223372036854776e18"} + ], + {composite, canonical} <- [ + {%{"n" => number}, ~s({"n":#{service_json}})}, + {[%{"n" => [number]}], ~s([{"n":[#{service_json}]}])} + ], + {filter, property} <- [ + {canonical, composite}, + {[composite], canonical} + ] do + condition = %{"key" => "prop", "operator" => operator, "value" => filter} + definitions = versioned_snapshot([flag("numeric-composite", [condition])], version) + context = %{distinct_id: "user", person_properties: %{"prop" => property}} + result = LocalEvaluator.evaluate(definitions, context) + + assert result.results == %{}, inspect({version, operator, filter, property, result}) + assert result.unresolved == MapSet.new(["numeric-composite"]) + end + end + + test "composite integers within the service range still match locally" do + property = %{"min" => -9_223_372_036_854_775_808, "max" => 18_446_744_073_709_551_615} + canonical = ~s({"max":18446744073709551615,"min":-9223372036854775808}) + + for version <- [:missing, 1, 2], operator <- ["exact", "is_not"] do + condition = %{"key" => "prop", "operator" => operator, "value" => canonical} + definitions = versioned_snapshot([flag("integer-composite", [condition])], version) + context = %{distinct_id: "user", person_properties: %{"prop" => property}} + result = LocalEvaluator.evaluate(definitions, context) + expected = operator == "exact" + + assert %Result{enabled: ^expected, locally_evaluated: true} = + result.results["integer-composite"] + + assert MapSet.size(result.unresolved) == 0 + end + end + + for {name, property, string} <- [ + {"Date", ~D[2025-01-01], "2025-01-01"}, + {"Time", ~T[12:34:56], "12:34:56"} + ] do + @scalar_struct property + @scalar_string string + test "exact and is_not preserve ordinary string matching for #{name} properties" do + for version <- [:missing, 1, 2], + operator <- ["exact", "is_not"], + {filter, matches} <- [ + {@scalar_string, true}, + {[@scalar_string], true}, + {"different", false} + ] do + condition = %{"key" => "prop", "operator" => operator, "value" => filter} + definitions = versioned_snapshot([flag("scalar-struct", [condition])], version) + context = %{distinct_id: "user", person_properties: %{"prop" => @scalar_struct}} + result = LocalEvaluator.evaluate(definitions, context) + expected = if operator == "exact", do: matches, else: not matches + + assert %Result{enabled: ^expected, locally_evaluated: true} = + result.results["scalar-struct"], + inspect({version, operator, filter, @scalar_struct, result}) + + assert MapSet.size(result.unresolved) == 0 + end + end + end + + test "missing properties stay inconclusive for both operators and matching versions" do + for version <- [:missing, 1, 2], operator <- ["exact", "is_not"] do + condition = %{"key" => "prop", "operator" => operator, "value" => false} + definitions = versioned_snapshot([flag("missing", [condition])], version) + result = LocalEvaluator.evaluate(definitions, %{distinct_id: "user"}) + assert result.results == %{} + assert MapSet.equal?(result.unresolved, MapSet.new(["missing"])) + end + end + + test "person, group, recursive cohort and dependency share one snapshot version" do + for version <- [:missing, 1, 2], operator <- ["exact", "is_not"] do + condition = %{"key" => "prop", "operator" => operator, "value" => false} + person = flag("person", [condition]) + group = put_in(flag("group", [condition]), ["filters", "aggregation_group_type_index"], 0) + cohort = flag("cohort", [%{"type" => "cohort", "value" => 1}]) + + dependency = + flag("dependency", [ + %{ + "type" => "flag", + "key" => "person", + "operator" => "flag_evaluates_to", + "value" => true, + "dependency_chain" => ["person"] + } + ]) + + definitions = + versioned_snapshot([person, group, cohort, dependency], version) + |> Map.put(:group_type_mapping, %{"0" => "organization"}) + |> Map.put(:cohorts, %{ + "1" => %{ + "type" => "AND", + "values" => [ + %{"type" => "OR", "values" => [condition]} + ] + } + }) + + context = %{ + distinct_id: "user", + person_properties: %{prop: "banana"}, + groups: %{organization: "org"}, + group_properties: %{organization: %{prop: "banana"}} + } + + expected = if operator == "exact", do: version != 2, else: version == 2 + + for keys <- [nil, ["dependency"], ["person"], ["group"], ["cohort"]] do + result = LocalEvaluator.evaluate(definitions, context, keys) + assert MapSet.size(result.unresolved) == 0 + + for key <- keys || ["person", "group", "cohort", "dependency"] do + assert result.results[key].enabled == expected, inspect({version, operator, key}) + end + end + end + end + + defp versioned_snapshot(flags, :missing), do: snapshot(flags) + + defp versioned_snapshot(flags, version), + do: snapshot(flags, %{property_matching_version: version}) + test "canonical hash vectors and fractional rollout boundaries" do assert_in_delta LocalEvaluator.hash("flag", "user"), 0.4357368498163313, 1.0e-15 assert_in_delta LocalEvaluator.hash("flag", "user", "variant"), 0.4727021985667222, 1.0e-15 diff --git a/test/support/local_evaluator_structs.ex b/test/support/local_evaluator_structs.ex new file mode 100644 index 0000000..4c1275d --- /dev/null +++ b/test/support/local_evaluator_structs.ex @@ -0,0 +1,18 @@ +defmodule PostHog.Test.LocalEvaluatorStructs.DerivedObject do + @moduledoc false + @derive Jason.Encoder + defstruct [:payload] +end + +defmodule PostHog.Test.LocalEvaluatorStructs.CustomValue do + @moduledoc false + defstruct [:payload] + + defimpl Jason.Encoder do + def encode(value, opts), do: Jason.Encode.value(value.payload, opts) + end + + defimpl String.Chars do + def to_string(value), do: Kernel.to_string(value.payload) + end +end