diff --git a/.changeset/busy-buttons-kiss.md b/.changeset/busy-buttons-kiss.md new file mode 100644 index 0000000..4c994fb --- /dev/null +++ b/.changeset/busy-buttons-kiss.md @@ -0,0 +1,5 @@ +--- +"posthog-php": patch +--- + +Allow shared flag definition cache providers to load and refresh local evaluation definitions without a secret key. diff --git a/lib/Client.php b/lib/Client.php index 84c6b6b..9eae0fd 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -259,8 +259,9 @@ class Client implements FeatureFlagEvaluationsHost * @param HttpClient|null $httpClient Custom HTTP client, primarily for tests and advanced integrations. * @param string|null $personalAPIKey Deprecated: use $secretKey instead. Kept as a * backwards-compatible alias for $secretKey. - * @param bool $loadFeatureFlags Whether to load local feature flag definitions during construction. - * @param string|null $secretKey Credential for local feature flag evaluation and remote config. + * @param bool $loadFeatureFlags Whether to load local feature flag definitions during construction + * from a configured cache provider or the PostHog API. + * @param string|null $secretKey Credential for fetching local feature flag definitions and remote config. * Accepts either a Personal API Key (`phx_...`) or a Project Secret API Key (`phs_...`). * Defaults to null. When both $secretKey and $personalAPIKey are provided, $secretKey wins. */ @@ -323,7 +324,7 @@ public function __construct( if ( $this->enabled && count($this->featureFlags) == 0 - && !is_null($this->secretKey) + && ($this->secretKey !== null || $this->flagDefinitionCacheProvider !== null) && $loadFeatureFlags ) { $this->loadFlags(); @@ -1490,7 +1491,9 @@ private function fetchFlagsResponse( } /** - * Load local feature flag definitions using the configured secret key. + * Load local feature flag definitions from the configured cache provider or PostHog API. + * Without a secret key, provider-backed clients only read the shared cache. + * Direct API requests require a secret key. * * @return void * @throws Exception @@ -1523,8 +1526,6 @@ public function loadFlags() } elseif ($this->hasFlagDefinitionsLoaded()) { return; } - - $shouldFetch = !is_null($this->secretKey); } catch (Throwable $throwable) { $this->logFlagDefinitionCacheWarning( 'Cache provider read error: ' . $throwable->getMessage() @@ -1532,8 +1533,9 @@ public function loadFlags() if ($this->hasFlagDefinitionsLoaded()) { return; } - $shouldFetch = !is_null($this->secretKey); } + + $shouldFetch = $this->secretKey !== null; } if ($shouldFetch) { @@ -1552,13 +1554,18 @@ private function shouldFetchFlagDefinitionsFromApi(): bool return true; } + // Cache-only readers must not acquire fetch leadership through the provider. + if ($this->secretKey === null) { + return false; + } + try { return $this->flagDefinitionCacheProvider->shouldFetchFlagDefinitions(); } catch (Throwable $throwable) { $this->logFlagDefinitionCacheWarning( 'Cache provider fetch-decision error: ' . $throwable->getMessage() ); - return !is_null($this->secretKey); + return true; } } @@ -1765,6 +1772,7 @@ private function logFlagDefinitionCacheWarning(string $message): void * Fetch local feature flag definitions from the PostHog API. * * @return HttpResponse Raw HTTP response, including ETag metadata when available. + * Returns a 401 response without making a request when no secret key is configured. */ public function localFlags(): HttpResponse { @@ -1779,6 +1787,12 @@ public function localFlags(): HttpResponse ); } + if ($this->secretKey === null) { + $message = 'A secret key is required to fetch flag definitions from PostHog'; + $this->logWarning($message); + return new HttpResponse($message, 401); + } + $headers = [ // Send user agent in the form of {library_name}/{library_version} as per RFC 7231. "User-Agent: " . PostHog::LIBRARY . "/" . PostHog::VERSION, diff --git a/lib/FlagDefinitionCacheProvider.php b/lib/FlagDefinitionCacheProvider.php index f209a39..33de847 100644 --- a/lib/FlagDefinitionCacheProvider.php +++ b/lib/FlagDefinitionCacheProvider.php @@ -8,6 +8,11 @@ * Implement this interface to share downloaded flag definitions across PHP workers, serverless * invocations, or other distributed SDK instances. Provider methods are called synchronously by the * SDK and any thrown error is logged as a warning without crashing application code. + * + * A project API key is required. Without a secret key, the SDK only reads shared definitions and + * never calls shouldFetchFlagDefinitions() or publishes definitions. With a secret key, the SDK + * consults the provider to decide whether to fetch and publish or read the cache. Definitions load + * during construction unless loadFeatureFlags is false; call Client::loadFlags() to refresh. */ interface FlagDefinitionCacheProvider { @@ -28,8 +33,11 @@ public function getFlagDefinitions(): ?array; /** * Decide whether this SDK instance should fetch fresh definitions from PostHog. * - * Return false when another worker is responsible for fetching and this instance should read the - * latest definitions from getFlagDefinitions() instead. + * Called only when a secret key is configured. Return false when another worker is responsible + * for fetching and this instance should read the latest definitions from getFlagDefinitions() + * instead. Return true to fetch from the API and publish the result. Empty or unavailable caches + * preserve previously loaded definitions; with no loaded definitions, a keyed client may fetch + * directly to recover from a cache miss or read failure. * * @return bool */ diff --git a/test/FlagDefinitionCacheProviderTest.php b/test/FlagDefinitionCacheProviderTest.php index 944d23c..707d500 100644 --- a/test/FlagDefinitionCacheProviderTest.php +++ b/test/FlagDefinitionCacheProviderTest.php @@ -95,6 +95,218 @@ public function testUsesCachedDataWhenProviderSaysNotToFetch(): void $this->assertArrayHasKey('beta-ui', $client->featureFlagsByKey); } + public function testProviderOnlyClientLoadsAndEvaluatesCachedDefinitions(): void + { + foreach ([null, '', " \n\t "] as $credential) { + $provider = new MockFlagDefinitionCacheProvider(); + $provider->cachedData = $this->versionedDefinitions(2); + $httpClient = new MockedHttpClient(host: 'app.posthog.com'); + $client = new Client( + self::FAKE_API_KEY, + ['flag_definition_cache_provider' => $provider], + $httpClient, + personalAPIKey: $credential, + secretKey: $credential + ); + + $this->assertSame(0, $provider->shouldFetchCallCount); + $this->assertSame(1, $provider->getCallCount); + $this->assertSame(['0' => 'company'], $client->groupTypeMapping); + $this->assertSame($provider->cachedData['cohorts'], $client->cohorts); + $this->assertVersionedResults($client, false); + $this->assertSame([], $httpClient->calls ?? []); + $this->assertSame(0, $provider->onReceivedCallCount); + } + } + + public function testProviderOnlyClientRespectsConstructionOptOutAndSupportsManualLoad(): void + { + $provider = new MockFlagDefinitionCacheProvider(); + $provider->shouldFetch = false; + $provider->cachedData = $this->sampleFlagDefinitionData(); + $httpClient = new MockedHttpClient(host: 'app.posthog.com'); + $client = new Client( + self::FAKE_API_KEY, + ['flag_definition_cache_provider' => $provider], + $httpClient, + loadFeatureFlags: false + ); + + $this->assertSame([], $client->featureFlags); + $this->assertSame([], $client->evaluateFlags('user', onlyEvaluateLocally: true)->getKeys()); + $this->assertSame(0, $provider->shouldFetchCallCount); + $this->assertSame(0, $provider->getCallCount); + + $client->loadFlags(); + + $this->assertTrue($client->evaluateFlags('user')->getFlag('beta-ui')); + $this->assertSame(0, $provider->shouldFetchCallCount); + $this->assertSame(1, $provider->getCallCount); + $this->assertSame(0, $provider->onReceivedCallCount); + $this->assertSame([], $httpClient->calls ?? []); + } + + public function testProviderOnlyClientSkipsPositiveFetchDecisionAndReadsCacheOnReload(): void + { + global $errorMessages; + $provider = new MockFlagDefinitionCacheProvider(); + $provider->cachedData = $this->versionedDefinitions(2); + $httpClient = new MockedHttpClient(host: 'app.posthog.com'); + $client = new Client( + self::FAKE_API_KEY, + ['flag_definition_cache_provider' => $provider], + $httpClient + ); + + $this->assertVersionedResults($client, false); + $provider->cachedData = $this->versionedDefinitions(1); + $client->loadFlags(); + $this->assertVersionedResults($client, true); + $provider->cachedData = $this->versionedDefinitions(2); + $client->loadFlags(); + + $this->assertVersionedResults($client, false); + $this->assertSame(0, $provider->shouldFetchCallCount); + $this->assertSame(3, $provider->getCallCount); + $this->assertSame(0, $provider->onReceivedCallCount); + $this->assertSame([], $httpClient->calls ?? []); + $this->assertSame([], $errorMessages); + } + + public function testProviderOnlyReaderCannotClaimLeadershipOrBlockKeyedPublisher(): void + { + $shared = (object) ['owner' => null, 'data' => $this->versionedDefinitions(1)]; + $providers = []; + foreach (['reader', 'publisher'] as $worker) { + $providers[$worker] = new class ($shared, $worker) extends MockFlagDefinitionCacheProvider { + public function __construct(private object $shared, private string $worker) + { + } + + public function shouldFetchFlagDefinitions(): bool + { + parent::shouldFetchFlagDefinitions(); + if ($this->shared->owner === null || $this->shared->owner === $this->worker) { + $this->shared->owner = $this->worker; + return true; + } + return false; + } + + public function getFlagDefinitions(): ?array + { + $this->cachedData = $this->shared->data; + return parent::getFlagDefinitions(); + } + + public function onFlagDefinitionsReceived(array $data): void + { + parent::onFlagDefinitionsReceived($data); + $this->shared->data = $data; + } + }; + } + $readerHttp = new MockedHttpClient(host: 'app.posthog.com'); + $reader = new Client( + self::FAKE_API_KEY, + ['flag_definition_cache_provider' => $providers['reader']], + $readerHttp + ); + $this->assertNull($shared->owner); + $this->assertVersionedResults($reader, true); + $publisherHttp = new MockedHttpClient(host: 'app.posthog.com'); + $publisherHttp->setFlagEndpointResponseQueue([ + ['response' => $this->versionedDefinitions(2)], + ['response' => $this->versionedDefinitions(1)], + ]); + $publisher = $this->createClient($providers['publisher'], $publisherHttp); + $this->assertSame('publisher', $shared->owner); + $this->assertVersionedResults($publisher, false); + $reader->loadFlags(); + $this->assertVersionedResults($reader, false); + + $publisher->loadFlags(); + $reader->loadFlags(); + + $this->assertVersionedResults($publisher, true); + $this->assertVersionedResults($reader, true); + $this->assertSame('publisher', $shared->owner); + $this->assertSame(0, $providers['reader']->shouldFetchCallCount); + $this->assertSame(3, $providers['reader']->getCallCount); + $this->assertSame(0, $providers['reader']->onReceivedCallCount); + $this->assertSame(2, $providers['publisher']->shouldFetchCallCount); + $this->assertSame(0, $providers['publisher']->getCallCount); + $this->assertSame(2, $providers['publisher']->onReceivedCallCount); + $this->assertCount(2, $publisherHttp->calls); + $this->assertOnlyDefinitionsRequests($publisherHttp); + $this->assertSame([], $readerHttp->calls ?? []); + } + + public function testProviderOnlyCacheMissOrFailureDoesNotRequestApi(): void + { + global $errorMessages; + foreach (['empty', 'malformed', 'read-error'] as $scenario) { + $errorMessages = []; + $provider = new MockFlagDefinitionCacheProvider(); + $provider->shouldFetch = false; + if ($scenario === 'malformed') { + $provider->cachedData = ['flags' => 'invalid']; + } elseif ($scenario === 'read-error') { + $provider->getError = new \RuntimeException('Redis read failed'); + } + $httpClient = new MockedHttpClient(host: 'app.posthog.com'); + $client = new Client( + self::FAKE_API_KEY, + ['flag_definition_cache_provider' => $provider], + $httpClient + ); + + $this->assertSame([], $client->featureFlags); + $this->assertSame(0, $provider->shouldFetchCallCount); + $this->assertSame(1, $provider->getCallCount); + $this->assertSame(0, $provider->onReceivedCallCount); + $this->assertSame([], $httpClient->calls ?? []); + $this->assertCount($scenario === 'empty' ? 0 : 1, $errorMessages); + if ($scenario === 'malformed') { + $this->assertWarningContains('Cache provider returned malformed flag definitions'); + } elseif ($scenario === 'read-error') { + $this->assertWarningContains('Cache provider read error: Redis read failed'); + } + } + } + + public function testProviderOnlyClientRequiresProjectApiKey(): void + { + $provider = new MockFlagDefinitionCacheProvider(); + $provider->shouldFetch = false; + $provider->cachedData = $this->sampleFlagDefinitionData(); + $httpClient = new MockedHttpClient(host: 'app.posthog.com'); + $client = new Client( + null, + ['consumer' => 'noop', 'flag_definition_cache_provider' => $provider], + $httpClient + ); + + $client->loadFlags(); + + $this->assertSame([], $client->featureFlags); + $this->assertSame(0, $provider->shouldFetchCallCount); + $this->assertSame(0, $provider->getCallCount); + $this->assertSame([], $httpClient->calls ?? []); + } + + public function testDirectDefinitionFetchWithoutSecretKeyDoesNotRequestApi(): void + { + $httpClient = new MockedHttpClient(host: 'app.posthog.com'); + $client = new Client(self::FAKE_API_KEY, [], $httpClient); + + $response = $client->localFlags(); + + $this->assertSame(401, $response->getResponseCode()); + $this->assertSame([], $httpClient->calls ?? []); + $this->assertWarningContains('A secret key is required to fetch flag definitions from PostHog'); + } + public function testStoresDefinitionsAfterProviderAllowsApiFetch(): void { $provider = new MockFlagDefinitionCacheProvider(); @@ -173,25 +385,31 @@ public function testProviderFetchDecisionFailureFailsSafeToDirectFetch(): void $this->assertWarningContains('Cache provider fetch-decision error: Lock acquisition failed'); } - public function testProviderFetchDecisionFailureWithoutPersonalApiKeyDoesNotFetchDirectly(): void + public function testProviderOnlyClientSkipsThrowingDecisionAndLoadsValidCache(): void { - $provider = new MockFlagDefinitionCacheProvider(); - $provider->shouldFetchError = new \RuntimeException('Lock acquisition failed'); - $httpClient = new MockedHttpClient(host: "app.posthog.com"); - $client = new Client( - self::FAKE_API_KEY, - ['flag_definition_cache_provider' => $provider], - $httpClient, - null, - false - ); - - $client->loadFlags(); + global $errorMessages; + foreach ([true, false] as $loadOnConstruction) { + $provider = new MockFlagDefinitionCacheProvider(); + $provider->shouldFetchError = new \RuntimeException('Lock acquisition failed'); + $provider->cachedData = $this->sampleFlagDefinitionData(); + $httpClient = new MockedHttpClient(host: 'app.posthog.com'); + $client = new Client( + self::FAKE_API_KEY, + ['flag_definition_cache_provider' => $provider], + $httpClient, + loadFeatureFlags: $loadOnConstruction + ); + if (!$loadOnConstruction) { + $client->loadFlags(); + } - $this->assertSame(1, $provider->shouldFetchCallCount); - $this->assertSame(1, $provider->getCallCount); - $this->assertSame([], $httpClient->calls ?? []); - $this->assertWarningContains('Cache provider fetch-decision error: Lock acquisition failed'); + $this->assertTrue($client->evaluateFlags('user', onlyEvaluateLocally: true)->getFlag('beta-ui')); + $this->assertSame(0, $provider->shouldFetchCallCount); + $this->assertSame(1, $provider->getCallCount); + $this->assertSame(0, $provider->onReceivedCallCount); + $this->assertSame([], $httpClient->calls ?? []); + $this->assertSame([], $errorMessages); + } } public function testProviderStoreFailureKeepsFetchedDefinitionsUsable(): void @@ -382,13 +600,17 @@ public function testVersionOnlyApiReloadAnd304OrFailurePreserveSnapshot(): void $this->assertOnlyDefinitionsRequests($httpClient); } - public function testVersionOnlyCacheReloadAndReadFailure(): void + public function testProviderOnlyVersionCacheReloadAndReadFailure(): void { $provider = new MockFlagDefinitionCacheProvider(); $provider->shouldFetch = false; $provider->cachedData = $this->versionedDefinitions(1); $httpClient = new MockedHttpClient(host: 'app.posthog.com'); - $client = $this->createClient($provider, $httpClient); + $client = new Client( + self::FAKE_API_KEY, + ['flag_definition_cache_provider' => $provider], + $httpClient + ); foreach ([1, 2, 1, 2, null, 3] as $version) { $provider->cachedData = $this->versionedDefinitions($version); // Also cover the provider's supported camelCase projection. @@ -409,6 +631,8 @@ public function testVersionOnlyCacheReloadAndReadFailure(): void $this->assertVersionedResults($client, false); } } + $this->assertSame(0, $provider->shouldFetchCallCount); + $this->assertSame(0, $provider->onReceivedCallCount); $this->assertSame([], $httpClient->calls ?? []); }