From 3706c898411891ae6a7ef9d3c1c0b8388acff6c8 Mon Sep 17 00:00:00 2001 From: Dustin Byrne Date: Fri, 9 Oct 2026 11:32:19 -0400 Subject: [PATCH 1/2] fix(flags): load shared definitions without a secret key --- .../cantankerous-duchess-loviatar.md | 5 + lib/posthog/config_schema.ex | 7 +- lib/posthog/feature_flags.ex | 3 +- .../feature_flags/definition_loader.ex | 8 + .../flag_definition_cache_provider.ex | 9 +- lib/posthog/supervisor.ex | 5 +- .../flag_definition_cache_provider_test.exs | 208 +++++++++++++++++- 7 files changed, 236 insertions(+), 9 deletions(-) create mode 100644 .sampo/changesets/cantankerous-duchess-loviatar.md diff --git a/.sampo/changesets/cantankerous-duchess-loviatar.md b/.sampo/changesets/cantankerous-duchess-loviatar.md new file mode 100644 index 0000000..6b967d3 --- /dev/null +++ b/.sampo/changesets/cantankerous-duchess-loviatar.md @@ -0,0 +1,5 @@ +--- +hex/posthog: patch +--- + +Allow shared flag definition cache providers to support local evaluation without a secret key. diff --git a/lib/posthog/config_schema.ex b/lib/posthog/config_schema.ex index b602c63..a1c742b 100644 --- a/lib/posthog/config_schema.ex +++ b/lib/posthog/config_schema.ex @@ -44,14 +44,15 @@ defmodule PostHog.Config.Schema do default: nil, doc: """ A privileged project secret (`phs_`) or appropriately scoped personal API key (`phx_`) - used only to load definitions for local feature flag evaluation. Local evaluation is inert - and starts no poller when this value is absent or blank. + required only to fetch local feature flag definitions directly from PostHog. A configured + `flag_definition_cache_provider` can supply definitions without this key. """ ], enable_local_evaluation: [ type: :boolean, default: true, - doc: "Enable local feature flag evaluation when a non-empty `secret_key` is configured." + doc: + "Enable local feature flag evaluation when a non-empty `secret_key` or a `flag_definition_cache_provider` is configured." ], feature_flags_poll_interval_ms: [ type: :pos_integer, diff --git a/lib/posthog/feature_flags.ex b/lib/posthog/feature_flags.ex index 5074de2..90375ff 100644 --- a/lib/posthog/feature_flags.ex +++ b/lib/posthog/feature_flags.ex @@ -108,7 +108,8 @@ defmodule PostHog.FeatureFlags do Evaluates feature flags for a `distinct_id` and returns a snapshot. Returns `{:ok, %PostHog.FeatureFlags.Evaluations{}}` on success. Definitions - are evaluated locally first when privileged local evaluation is configured; + are evaluated locally first when local evaluation is configured with a + privileged key or a shared definition cache provider; unresolved flags are filled by at most one `/flags` call. If that fallback fails after some flags resolved locally, the successful local subset is returned as a partial snapshot. If nothing resolved, the remote error is diff --git a/lib/posthog/feature_flags/definition_loader.ex b/lib/posthog/feature_flags/definition_loader.ex index 08ea830..d16df23 100644 --- a/lib/posthog/feature_flags/definition_loader.ex +++ b/lib/posthog/feature_flags/definition_loader.ex @@ -410,6 +410,14 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do end end + defp fetch_direct(%{config: %{secret_key: nil}} = state, _store?) do + warning( + "definition refresh requires a secret_key to fetch from PostHog; keeping stale definitions" + ) + + state + end + defp fetch_direct(state, store?) do config = state.config diff --git a/lib/posthog/feature_flags/flag_definition_cache_provider.ex b/lib/posthog/feature_flags/flag_definition_cache_provider.ex index e0ee09e..87dc174 100644 --- a/lib/posthog/feature_flags/flag_definition_cache_provider.ex +++ b/lib/posthog/feature_flags/flag_definition_cache_provider.ex @@ -10,6 +10,13 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProvider do the definitions. Only version 2 selects explicit property matching; older cached envelopes omitting the version use released service legacy matching. + With a project `:api_key` and local evaluation enabled, a provider can supply + definitions without a `:secret_key`. Each refresh consults the fetch decision + first and reads cached definitions only when it returns false. A secret key is + required to fetch definitions directly from PostHog; without one, a requested + fetch is skipped and any loaded definitions are preserved. Provider-backed + instances use the same startup, polling, and refresh lifecycle. + A minimal provider can coordinate fetching and keep the complete envelope in an application-owned cache: @@ -23,7 +30,7 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProvider do end config :posthog, - secret_key: System.fetch_env!("POSTHOG_SECRET_KEY"), + api_key: System.fetch_env!("POSTHOG_PROJECT_TOKEN"), flag_definition_cache_provider: {MyApp.PostHogDefinitionCache, provider_state} """ diff --git a/lib/posthog/supervisor.ex b/lib/posthog/supervisor.ex index 78e43f1..47bc267 100644 --- a/lib/posthog/supervisor.ex +++ b/lib/posthog/supervisor.ex @@ -72,11 +72,12 @@ defmodule PostHog.Supervisor do %{ enabled: true, enable_local_evaluation: true, - secret_key: %PostHog.Config.Secret{value: secret_key} + secret_key: secret_key, + flag_definition_cache_provider: provider } = config, callers ) - when is_binary(secret_key) and secret_key != "" do + when not is_nil(secret_key) or not is_nil(provider) do [ {PostHog.FeatureFlags.DefinitionLoader.NegativeKnowledge, supervisor_name: config.supervisor_name}, 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 6455144..0e771aa 100644 --- a/test/posthog/feature_flags/flag_definition_cache_provider_test.exs +++ b/test/posthog/feature_flags/flag_definition_cache_provider_test.exs @@ -26,7 +26,10 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do end defp run(agent, key) do - case Agent.get(agent, &Map.fetch!(&1, key)) do + {owner, result} = Agent.get(agent, &{&1.owner, Map.fetch!(&1, key)}) + send(owner, {:provider_call, key}) + + case result do {:sleep, milliseconds, result} -> Process.sleep(milliseconds) result @@ -71,6 +74,15 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do start_supervised!({PostHog.Supervisor, cfg}) end + defp stub_unexpected_api_requests(owner) do + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + stub(PostHog.API.Mock, :request, fn _client, _method, _path, _opts -> + send(owner, :unexpected_api_fetch) + {:error, :unexpected_api_fetch} + end) + 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. @@ -116,7 +128,7 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do Agent.update(provider, &%{&1 | decision: false, read: stored}) name = __MODULE__.VersionReader - start_instance(name, provider) + start_instance(name, provider, secret_key: nil) context = %{ distinct_id: "user", @@ -204,6 +216,198 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do refute_receive :unexpected_api_fetch end + test "provider-only instances evaluate cached definitions on first use and manual refresh" do + owner = self() + stub_unexpected_api_requests(owner) + + cached = + envelope("cached") + |> Map.put("property_matching_version", 2) + |> Map.put("flags", [ + %{ + "key" => "cached", + "active" => true, + "filters" => %{"groups" => [%{"properties" => [], "rollout_percentage" => 100}]} + } + ]) + + for {name, secret_key} <- [ + {__MODULE__.ProviderOnly, nil}, + {__MODULE__.BlankSecret, " \t "} + ] do + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: false, read: cached, store: :ok, shutdown: :ok} + end) + + start_instance(name, provider, secret_key: secret_key) + assert {:ok, result} = PostHog.FeatureFlags.evaluate_flags(name, "user") + assert result.flags["cached"].enabled + assert result.flags["cached"].locally_evaluated + assert_receive {:provider_call, :decision} + assert_receive {:provider_call, :read} + definitions = DefinitionLoader.definitions(name) + assert definitions.flags == cached["flags"] + assert definitions.group_type_mapping == cached["group_type_mapping"] + assert definitions.cohorts == cached["cohorts"] + assert definitions.property_matching_version == 2 + assert definitions.minimal_flag_called_events + + Agent.update(provider, &%{&1 | read: envelope("refreshed")}) + assert :ok = DefinitionLoader.refresh(name) + assert {:ok, refreshed} = PostHog.FeatureFlags.evaluate_flags(name, "user") + assert Map.keys(refreshed.flags) == ["refreshed"] + assert refreshed.flags["refreshed"].locally_evaluated + assert DefinitionLoader.definitions(name).property_matching_version == 1 + assert_receive {:provider_call, :decision} + assert_receive {:provider_call, :read} + refute_received {:provider_call, :store} + assert :ok = stop_supervised(name) + assert_receive {:provider_call, :shutdown} + assert_receive :shutdown + refute_received :unexpected_api_fetch + end + end + + test "provider-only polling hydrates updated shared definitions" do + owner = self() + stub_unexpected_api_requests(owner) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: false, read: envelope("initial"), store: :ok, shutdown: :ok} + end) + + name = __MODULE__.ProviderOnlyPolling + start_instance(name, provider, secret_key: nil, feature_flags_poll_interval_ms: 50) + assert {:ok, initial} = PostHog.FeatureFlags.evaluate_flags(name, "user") + assert initial.flags["initial"].locally_evaluated + assert_receive {:provider_call, :decision} + assert_receive {:provider_call, :read} + + Agent.update(provider, &%{&1 | read: envelope("polled")}) + assert_receive {:provider_call, :decision}, 500 + assert_receive {:provider_call, :read}, 500 + # Wait for the in-flight poll to finish without initiating a manual refresh. + GenServer.call(PostHog.Registry.via(name, DefinitionLoader), :definitions) + + assert {:ok, polled} = PostHog.FeatureFlags.evaluate_flags(name, "user") + assert Map.keys(polled.flags) == ["polled"] + assert polled.flags["polled"].locally_evaluated + assert :ok = stop_supervised(name) + assert_receive :shutdown + refute_received :unexpected_api_fetch + end + + test "provider-only API fallback warns without fetching or reading on a positive decision" do + owner = self() + stub_unexpected_api_requests(owner) + + for {suffix, decision, read} <- [ + {"Empty", false, nil}, + {"FailedRead", false, {:raise, "unavailable"}}, + {"TimedOutRead", false, {:sleep, 100, nil}}, + {"MalformedRead", false, %{"flags" => []}}, + {"FetchOwner", true, envelope("not-read")}, + {"FailedDecision", {:raise, "unavailable"}, envelope("not-read")} + ] do + name = Module.concat(__MODULE__, suffix) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: decision, read: read, store: :ok, shutdown: :ok} + end) + + log = + capture_log(fn -> + start_instance(name, provider, secret_key: nil) + refute DefinitionLoader.ready?(name) + end) + + assert log =~ "requires a secret_key to fetch from PostHog" + assert_receive {:provider_call, :decision} + + if decision == false do + assert_receive {:provider_call, :read} + else + refute_received {:provider_call, :read} + end + + assert :ok = stop_supervised(name) + assert_receive :shutdown + refute_received :unexpected_api_fetch + refute_received {:stored, _definitions} + end + end + + test "provider-only refresh preserves the last snapshot when reads or fetch decisions fail" do + owner = self() + stub_unexpected_api_requests(owner) + + cached = Map.put(envelope("stale"), "property_matching_version", 2) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: false, read: cached, store: :ok, shutdown: :ok} + end) + + name = __MODULE__.ProviderOnlyStale + start_instance(name, provider, secret_key: nil) + initial = DefinitionLoader.definitions(name) + assert initial.property_matching_version == 2 + + for {decision, read} <- [ + {false, nil}, + {false, {:raise, "unavailable"}}, + {false, {:sleep, 100, nil}}, + {false, %{"flags" => []}}, + {true, envelope("not-read")}, + {{:raise, "unavailable"}, envelope("not-read")} + ] do + Agent.update(provider, &%{&1 | decision: decision, read: read}) + capture_log(fn -> assert :ok = DefinitionLoader.refresh(name) end) + assert DefinitionLoader.definitions(name) == initial + assert {:ok, result} = PostHog.FeatureFlags.evaluate_flags(name, "user") + assert result.flags["stale"].locally_evaluated + refute_received :unexpected_api_fetch + refute_received {:stored, _definitions} + end + + assert :ok = stop_supervised(name) + assert_receive :shutdown + end + + test "provider-only instances respect local evaluation and project-token startup controls" do + owner = self() + stub_with(PostHog.API.Mock, PostHog.API.Stub) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, decision: false, read: envelope("cached"), store: :ok, shutdown: :ok} + end) + + for {name, overrides} <- [ + {__MODULE__.ProviderLocalDisabled, [enable_local_evaluation: false]}, + {__MODULE__.ProviderDisabled, [api_key: ""]} + ] do + capture_log(fn -> start_instance(name, provider, [secret_key: nil] ++ overrides) end) + assert GenServer.whereis(PostHog.Registry.via(name, DefinitionLoader)) == nil + + assert GenServer.whereis(PostHog.Registry.via(name, DefinitionLoader.NegativeKnowledge)) == + nil + + assert {:ok, result} = + PostHog.FeatureFlags.evaluate_flags(name, %{ + distinct_id: "user", + only_evaluate_locally: true + }) + + assert result.flags == %{} + assert :ok = stop_supervised(name) + refute_received {:provider_call, _callback} + end + end + test "positive decision fetches, publishes, and stores the complete envelope" do stub_with(PostHog.API.Mock, PostHog.API.Stub) From 95ca2daf4ba937ebb0f98e720f3132f72d35bd24 Mon Sep 17 00:00:00 2001 From: Dustin Byrne Date: Sat, 10 Oct 2026 00:12:54 -0400 Subject: [PATCH 2/2] fix(flags): make keyless cache consumers read-only --- .../feature_flags/definition_loader.ex | 8 + .../flag_definition_cache_provider.ex | 14 +- .../flag_definition_cache_provider_test.exs | 150 +++++++++++++++--- 3 files changed, 145 insertions(+), 27 deletions(-) diff --git a/lib/posthog/feature_flags/definition_loader.ex b/lib/posthog/feature_flags/definition_loader.ex index d16df23..d400c58 100644 --- a/lib/posthog/feature_flags/definition_loader.ex +++ b/lib/posthog/feature_flags/definition_loader.ex @@ -363,6 +363,14 @@ defmodule PostHog.FeatureFlags.DefinitionLoader do defp refresh_state(%{config: %{flag_definition_cache_provider: nil}} = state), do: fetch_direct(state, false) + # Cache-only readers must not acquire fetch leadership from the provider. + defp refresh_state( + %{ + config: %{secret_key: nil, flag_definition_cache_provider: {module, provider_state}} + } = state + ), + do: read_provider_without_fetch_ownership(state, module, provider_state) + defp refresh_state( %{ config: %{flag_definition_cache_provider: {module, provider_state}} diff --git a/lib/posthog/feature_flags/flag_definition_cache_provider.ex b/lib/posthog/feature_flags/flag_definition_cache_provider.ex index 87dc174..4033f61 100644 --- a/lib/posthog/feature_flags/flag_definition_cache_provider.ex +++ b/lib/posthog/feature_flags/flag_definition_cache_provider.ex @@ -11,11 +11,13 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProvider do cached envelopes omitting the version use released service legacy matching. With a project `:api_key` and local evaluation enabled, a provider can supply - definitions without a `:secret_key`. Each refresh consults the fetch decision - first and reads cached definitions only when it returns false. A secret key is - required to fetch definitions directly from PostHog; without one, a requested - fetch is skipped and any loaded definitions are preserved. Provider-backed - instances use the same startup, polling, and refresh lifecycle. + definitions without a `:secret_key`. These cache-only readers read the provider + on each refresh without invoking the fetch decision or publishing definitions. + Instances with a secret key consult the fetch decision: true fetches and + publishes fresh definitions, while false reads the cache. An empty or failed + cache preserves loaded definitions; a keyed instance without definitions can + recover by fetching directly. Provider-backed instances use the same startup, + polling, and refresh lifecycle. A minimal provider can coordinate fetching and keep the complete envelope in an application-owned cache: @@ -37,7 +39,7 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProvider do @type provider_state :: any() @type definitions :: map() - @doc "Returns whether this SDK instance should fetch fresh definitions from PostHog." + @doc "Returns whether a secret-key-backed SDK instance should fetch fresh definitions from PostHog." @callback should_fetch_flag_definitions(provider_state()) :: boolean() @doc "Reads definitions from the shared cache, or returns nil when it is empty." 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 0e771aa..4ca7ad8 100644 --- a/test/posthog/feature_flags/flag_definition_cache_provider_test.exs +++ b/test/posthog/feature_flags/flag_definition_cache_provider_test.exs @@ -37,12 +37,47 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do {:raise, message} -> raise message + {:throw, reason} -> + throw(reason) + value -> value end end end + defmodule LeaseProvider do + @behaviour PostHog.FeatureFlags.FlagDefinitionCacheProvider + + @impl true + def should_fetch_flag_definitions({agent, identity}) do + Agent.get_and_update(agent, fn state -> + send(state.owner, {:lease_decision, identity}) + leader = state.leader || identity + {leader == identity, %{state | leader: leader}} + end) + end + + @impl true + def get_flag_definitions({agent, identity}) do + Agent.get(agent, fn state -> + send(state.owner, {:lease_read, identity}) + state.cached + end) + end + + @impl true + def on_flag_definitions_received({agent, identity}, definitions) do + Agent.update(agent, fn state -> + send(state.owner, {:lease_store, identity}) + %{state | cached: definitions} + end) + end + + @impl true + def shutdown(_state), do: :ok + end + setup :set_mox_from_context setup :verify_on_exit! @@ -208,6 +243,9 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do start_instance(__MODULE__.Cached, provider) definitions = DefinitionLoader.definitions(__MODULE__.Cached) + assert_receive {:provider_call, :decision} + assert_receive {:provider_call, :read} + refute_received {:provider_call, :store} assert definitions.flags_by_key["cached"] assert definitions.group_type_mapping == %{"0" => "organization"} assert definitions.minimal_flag_called_events @@ -231,20 +269,23 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do } ]) - for {name, secret_key} <- [ - {__MODULE__.ProviderOnly, nil}, - {__MODULE__.BlankSecret, " \t "} + for {name, secret_key, decision} <- [ + {__MODULE__.ProviderOnly, nil, false}, + {__MODULE__.BlankSecret, " \t ", true}, + {__MODULE__.RaisingDecision, nil, {:raise, "unavailable"}}, + {__MODULE__.ThrowingDecision, nil, {:throw, :unavailable}}, + {__MODULE__.HangingDecision, nil, {:sleep, 10_000, true}} ] do {:ok, provider} = Agent.start_link(fn -> - %{owner: owner, decision: false, read: cached, store: :ok, shutdown: :ok} + %{owner: owner, decision: decision, read: cached, store: :ok, shutdown: :ok} end) start_instance(name, provider, secret_key: secret_key) assert {:ok, result} = PostHog.FeatureFlags.evaluate_flags(name, "user") assert result.flags["cached"].enabled assert result.flags["cached"].locally_evaluated - assert_receive {:provider_call, :decision} + refute_received {:provider_call, :decision} assert_receive {:provider_call, :read} definitions = DefinitionLoader.definitions(name) assert definitions.flags == cached["flags"] @@ -259,7 +300,7 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do assert Map.keys(refreshed.flags) == ["refreshed"] assert refreshed.flags["refreshed"].locally_evaluated assert DefinitionLoader.definitions(name).property_matching_version == 1 - assert_receive {:provider_call, :decision} + refute_received {:provider_call, :decision} assert_receive {:provider_call, :read} refute_received {:provider_call, :store} assert :ok = stop_supervised(name) @@ -282,12 +323,12 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do start_instance(name, provider, secret_key: nil, feature_flags_poll_interval_ms: 50) assert {:ok, initial} = PostHog.FeatureFlags.evaluate_flags(name, "user") assert initial.flags["initial"].locally_evaluated - assert_receive {:provider_call, :decision} + refute_received {:provider_call, :decision} assert_receive {:provider_call, :read} Agent.update(provider, &%{&1 | read: envelope("polled")}) - assert_receive {:provider_call, :decision}, 500 assert_receive {:provider_call, :read}, 500 + refute_received {:provider_call, :decision} # Wait for the in-flight poll to finish without initiating a manual refresh. GenServer.call(PostHog.Registry.via(name, DefinitionLoader), :definitions) @@ -296,10 +337,12 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do assert polled.flags["polled"].locally_evaluated assert :ok = stop_supervised(name) assert_receive :shutdown + refute_received {:provider_call, :decision} + refute_received {:provider_call, :store} refute_received :unexpected_api_fetch end - test "provider-only API fallback warns without fetching or reading on a positive decision" do + test "provider-only cache misses and read failures warn without fetching or publishing" do owner = self() stub_unexpected_api_requests(owner) @@ -308,8 +351,8 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do {"FailedRead", false, {:raise, "unavailable"}}, {"TimedOutRead", false, {:sleep, 100, nil}}, {"MalformedRead", false, %{"flags" => []}}, - {"FetchOwner", true, envelope("not-read")}, - {"FailedDecision", {:raise, "unavailable"}, envelope("not-read")} + {"FetchOwner", true, nil}, + {"FailedDecision", {:raise, "unavailable"}, nil} ] do name = Module.concat(__MODULE__, suffix) @@ -325,13 +368,8 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do end) assert log =~ "requires a secret_key to fetch from PostHog" - assert_receive {:provider_call, :decision} - - if decision == false do - assert_receive {:provider_call, :read} - else - refute_received {:provider_call, :read} - end + refute_received {:provider_call, :decision} + assert_receive {:provider_call, :read} assert :ok = stop_supervised(name) assert_receive :shutdown @@ -340,7 +378,7 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do end end - test "provider-only refresh preserves the last snapshot when reads or fetch decisions fail" do + test "provider-only refresh preserves the last snapshot when cache reads fail" do owner = self() stub_unexpected_api_requests(owner) @@ -361,14 +399,15 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do {false, {:raise, "unavailable"}}, {false, {:sleep, 100, nil}}, {false, %{"flags" => []}}, - {true, envelope("not-read")}, - {{:raise, "unavailable"}, envelope("not-read")} + {true, nil}, + {{:raise, "unavailable"}, nil} ] do Agent.update(provider, &%{&1 | decision: decision, read: read}) capture_log(fn -> assert :ok = DefinitionLoader.refresh(name) end) assert DefinitionLoader.definitions(name) == initial assert {:ok, result} = PostHog.FeatureFlags.evaluate_flags(name, "user") assert result.flags["stale"].locally_evaluated + refute_received {:provider_call, :decision} refute_received :unexpected_api_fetch refute_received {:stored, _definitions} end @@ -377,6 +416,72 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do assert_receive :shutdown end + test "cache-only readers cannot claim shared leadership before a keyed publisher refreshes" do + owner = self() + stub_unexpected_api_requests(owner) + + {:ok, provider} = + Agent.start_link(fn -> + %{owner: owner, leader: nil, cached: envelope("initial")} + end) + + reader = __MODULE__.LeaseReader + publisher = __MODULE__.LeasePublisher + + start_instance(reader, provider, + secret_key: nil, + flag_definition_cache_provider: {LeaseProvider, {provider, :reader}} + ) + + initial = DefinitionLoader.definitions(reader) + assert Agent.get(provider, & &1.leader) == nil + assert initial.flags_by_key["initial"] + assert_receive {:lease_read, :reader} + refute_received {:lease_decision, :reader} + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("published"), headers: %{}}} + end) + + start_instance(publisher, provider, + flag_definition_cache_provider: {LeaseProvider, {provider, :publisher}} + ) + + assert DefinitionLoader.definitions(publisher).flags_by_key["published"] + assert_receive {:lease_decision, :publisher} + assert_receive {:lease_store, :publisher} + assert Agent.get(provider, & &1.leader) == :publisher + + # Expire the lease, then refresh the reader before the publisher can reacquire it. + Agent.update(provider, &%{&1 | leader: nil}) + assert :ok = DefinitionLoader.refresh(reader) + assert DefinitionLoader.definitions(reader).flags_by_key["published"] + assert_receive {:lease_read, :reader} + + loader = GenServer.whereis(PostHog.Registry.via(reader, DefinitionLoader)) + send(loader, {:refresh, :sys.get_state(loader).timer_generation}) + GenServer.call(loader, :definitions) + assert_receive {:lease_read, :reader} + assert Agent.get(provider, & &1.leader) == nil + + expect(PostHog.API.Mock, :request, fn :stub_client, :get, "/flags/definitions", _opts -> + {:ok, %{status: 200, body: envelope("updated"), headers: %{}}} + end) + + assert :ok = DefinitionLoader.refresh(publisher) + assert DefinitionLoader.definitions(publisher).flags_by_key["updated"] + assert_receive {:lease_decision, :publisher} + assert_receive {:lease_store, :publisher} + assert :ok = DefinitionLoader.refresh(reader) + assert DefinitionLoader.definitions(reader).flags_by_key["updated"] + assert_receive {:lease_read, :reader} + refute_received {:lease_decision, :reader} + refute_received {:lease_store, :reader} + refute_received :unexpected_api_fetch + assert :ok = stop_supervised(reader) + assert :ok = stop_supervised(publisher) + end + test "provider-only instances respect local evaluation and project-token startup controls" do owner = self() stub_with(PostHog.API.Mock, PostHog.API.Stub) @@ -425,6 +530,9 @@ defmodule PostHog.FeatureFlags.FlagDefinitionCacheProviderTest do start_instance(__MODULE__.Fresh, provider) assert DefinitionLoader.definitions(__MODULE__.Fresh).flags_by_key["fresh"] + assert_receive {:provider_call, :decision} + refute_received {:provider_call, :read} + assert_receive {:provider_call, :store} assert_receive {:stored, stored} assert stored["flags"] != [] assert stored["group_type_mapping"] == %{"0" => "organization"}