From c58c84636be06eb96e091eb4fdd3acd2e6eabd68 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 19 Aug 2026 18:18:41 -0700 Subject: [PATCH 1/3] fix: Check the deleted marker before decoding items from a persistent store --- .../Integrations/FeatureRequesterBase.php | 20 +-- .../Integrations/FeatureRequesterBaseTest.php | 114 ++++++++++++++++++ 2 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 tests/Impl/Integrations/FeatureRequesterBaseTest.php diff --git a/src/LaunchDarkly/Impl/Integrations/FeatureRequesterBase.php b/src/LaunchDarkly/Impl/Integrations/FeatureRequesterBase.php index 158ede8f9..4006548dc 100644 --- a/src/LaunchDarkly/Impl/Integrations/FeatureRequesterBase.php +++ b/src/LaunchDarkly/Impl/Integrations/FeatureRequesterBase.php @@ -102,12 +102,14 @@ public function getFeature(string $key): ?FeatureFlag { $json = $this->getJsonItem(self::FEATURES_NAMESPACE, $key); if ($json) { - $flag = FeatureFlag::decode($json); - if ($flag->isDeleted()) { + // Check the deleted marker before decoding. Some store writers + // persist tombstones that omit most of the schema, and the model + // decoder requires the full schema. + if ($json['deleted'] ?? false) { $this->_logger->warning("FeatureRequester: Attempted to get deleted feature with key: " . $key); return null; } - return $flag; + return FeatureFlag::decode($json); } else { $this->_logger->warning("FeatureRequester: Attempted to get missing feature with key: " . $key); return null; @@ -124,12 +126,11 @@ public function getSegment(string $key): ?Segment { $json = $this->getJsonItem(self::SEGMENTS_NAMESPACE, $key); if ($json) { - $segment = Segment::decode($json); - if ($segment->isDeleted()) { + if ($json['deleted'] ?? false) { $this->_logger->warning("FeatureRequester: Attempted to get deleted segment with key: " . $key); return null; } - return $segment; + return Segment::decode($json); } else { $this->_logger->warning("FeatureRequester: Attempted to get missing segment with key: " . $key); return null; @@ -146,10 +147,11 @@ public function getAllFeatures(): ?array $jsonList = $this->getJsonItemList(self::FEATURES_NAMESPACE); $itemsOut = []; foreach ($jsonList as $json) { - $flag = FeatureFlag::decode($json); - if (!$flag->isDeleted()) { - $itemsOut[$flag->getKey()] = $flag; + if ($json['deleted'] ?? false) { + continue; } + $flag = FeatureFlag::decode($json); + $itemsOut[$flag->getKey()] = $flag; } return $itemsOut; } diff --git a/tests/Impl/Integrations/FeatureRequesterBaseTest.php b/tests/Impl/Integrations/FeatureRequesterBaseTest.php new file mode 100644 index 000000000..37a614c08 --- /dev/null +++ b/tests/Impl/Integrations/FeatureRequesterBaseTest.php @@ -0,0 +1,114 @@ +> */ + private array $data; + + /** + * @param array> $data map of namespace to (key to raw JSON string) + */ + public function __construct(array $data) + { + parent::__construct('', '', []); + $this->data = $data; + } + + protected function readItemString(string $namespace, string $key): ?string + { + return $this->data[$namespace][$key] ?? null; + } + + protected function readItemStringList(string $namespace): ?array + { + return array_values($this->data[$namespace] ?? []); + } +} + +class FeatureRequesterBaseTest extends TestCase +{ + private const FLAG_JSON = '{"key":"flagkey","version":1,"on":true,"prerequisites":[],"salt":"salty",' . + '"targets":[],"contextTargets":[],"rules":[],"fallthrough":{"variation":0},"offVariation":1,' . + '"variations":["fall","off"],"deleted":false,"trackEvents":false,"trackEventsFallthrough":false,' . + '"debugEventsUntilDate":null,"clientSide":false}'; + + private const SEGMENT_JSON = '{"key":"segkey","version":1,"included":[],"excluded":[],"rules":[],' . + '"salt":"salty","deleted":false}'; + + private const FULL_FLAG_TOMBSTONE_JSON = '{"key":"deletedkey","version":2,"on":false,"prerequisites":[],' . + '"salt":"","targets":[],"contextTargets":[],"rules":[],"fallthrough":{},"offVariation":null,' . + '"variations":[],"deleted":true,"trackEvents":false,"trackEventsFallthrough":false,' . + '"debugEventsUntilDate":null,"clientSide":false}'; + + private const MINIMAL_TOMBSTONE_JSON = '{"version":2,"deleted":true}'; + + private const KEYED_MINIMAL_TOMBSTONE_JSON = '{"key":"deletedkey","version":2,"deleted":true}'; + + public function testGetFeatureReturnsFlag(): void + { + $requester = new FakeStoreFeatureRequester(['features' => ['flagkey' => self::FLAG_JSON]]); + $flag = $requester->getFeature('flagkey'); + $this->assertNotNull($flag); + $this->assertEquals('flagkey', $flag->getKey()); + } + + public function testGetFeatureReturnsNullForFullSchemaTombstone(): void + { + $requester = new FakeStoreFeatureRequester( + ['features' => ['deletedkey' => self::FULL_FLAG_TOMBSTONE_JSON]] + ); + $this->assertNull($requester->getFeature('deletedkey')); + } + + public function testGetFeatureReturnsNullForMinimalTombstone(): void + { + $requester = new FakeStoreFeatureRequester( + ['features' => ['deletedkey' => self::MINIMAL_TOMBSTONE_JSON]] + ); + $this->assertNull($requester->getFeature('deletedkey')); + } + + public function testGetFeatureReturnsNullForKeyedMinimalTombstone(): void + { + $requester = new FakeStoreFeatureRequester( + ['features' => ['deletedkey' => self::KEYED_MINIMAL_TOMBSTONE_JSON]] + ); + $this->assertNull($requester->getFeature('deletedkey')); + } + + public function testGetSegmentReturnsSegment(): void + { + $requester = new FakeStoreFeatureRequester(['segments' => ['segkey' => self::SEGMENT_JSON]]); + $segment = $requester->getSegment('segkey'); + $this->assertNotNull($segment); + $this->assertEquals('segkey', $segment->getKey()); + } + + public function testGetSegmentReturnsNullForMinimalTombstone(): void + { + $requester = new FakeStoreFeatureRequester( + ['segments' => ['segkey' => self::MINIMAL_TOMBSTONE_JSON]] + ); + $this->assertNull($requester->getSegment('segkey')); + } + + public function testGetAllFeaturesSkipsTombstones(): void + { + $requester = new FakeStoreFeatureRequester(['features' => [ + 'flagkey' => self::FLAG_JSON, + 'deleted1' => self::FULL_FLAG_TOMBSTONE_JSON, + 'deleted2' => self::MINIMAL_TOMBSTONE_JSON, + 'deleted3' => self::KEYED_MINIMAL_TOMBSTONE_JSON, + ]]); + $flags = $requester->getAllFeatures(); + $this->assertNotNull($flags); + $this->assertEquals(['flagkey'], array_keys($flags)); + } +} From daa9333ecb30439104527bb74a52bf970652c13f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 19 Aug 2026 18:27:03 -0700 Subject: [PATCH 2/3] fix: Satisfy psalm strict-comparison and unused-method checks --- .../Impl/Integrations/FeatureRequesterBase.php | 6 +++--- src/LaunchDarkly/Impl/Model/FeatureFlag.php | 7 +++++++ src/LaunchDarkly/Impl/Model/Segment.php | 7 +++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/LaunchDarkly/Impl/Integrations/FeatureRequesterBase.php b/src/LaunchDarkly/Impl/Integrations/FeatureRequesterBase.php index 4006548dc..dfc5c6749 100644 --- a/src/LaunchDarkly/Impl/Integrations/FeatureRequesterBase.php +++ b/src/LaunchDarkly/Impl/Integrations/FeatureRequesterBase.php @@ -105,7 +105,7 @@ public function getFeature(string $key): ?FeatureFlag // Check the deleted marker before decoding. Some store writers // persist tombstones that omit most of the schema, and the model // decoder requires the full schema. - if ($json['deleted'] ?? false) { + if (($json['deleted'] ?? false) === true) { $this->_logger->warning("FeatureRequester: Attempted to get deleted feature with key: " . $key); return null; } @@ -126,7 +126,7 @@ public function getSegment(string $key): ?Segment { $json = $this->getJsonItem(self::SEGMENTS_NAMESPACE, $key); if ($json) { - if ($json['deleted'] ?? false) { + if (($json['deleted'] ?? false) === true) { $this->_logger->warning("FeatureRequester: Attempted to get deleted segment with key: " . $key); return null; } @@ -147,7 +147,7 @@ public function getAllFeatures(): ?array $jsonList = $this->getJsonItemList(self::FEATURES_NAMESPACE); $itemsOut = []; foreach ($jsonList as $json) { - if ($json['deleted'] ?? false) { + if (($json['deleted'] ?? false) === true) { continue; } $flag = FeatureFlag::decode($json); diff --git a/src/LaunchDarkly/Impl/Model/FeatureFlag.php b/src/LaunchDarkly/Impl/Model/FeatureFlag.php index 00c60b6aa..07ab61ef0 100644 --- a/src/LaunchDarkly/Impl/Model/FeatureFlag.php +++ b/src/LaunchDarkly/Impl/Model/FeatureFlag.php @@ -144,6 +144,13 @@ public function getDebugEventsUntilDate(): ?int return $this->_debugEventsUntilDate; } + /** + * The requester checks the deleted marker on the raw JSON before it + * decodes, so this accessor has no internal callers. It stays because the + * model mirrors the flag schema. + * + * @psalm-suppress PossiblyUnusedMethod + */ public function isDeleted(): bool { return $this->_deleted; diff --git a/src/LaunchDarkly/Impl/Model/Segment.php b/src/LaunchDarkly/Impl/Model/Segment.php index 688e1aacd..60486b7a1 100644 --- a/src/LaunchDarkly/Impl/Model/Segment.php +++ b/src/LaunchDarkly/Impl/Model/Segment.php @@ -86,6 +86,13 @@ public static function decode(array $v): Segment return static::getDecoder()($v); } + /** + * The requester checks the deleted marker on the raw JSON before it + * decodes, so this accessor has no internal callers. It stays because the + * model mirrors the segment schema. + * + * @psalm-suppress PossiblyUnusedMethod + */ public function isDeleted(): bool { return $this->_deleted; From ca6a9bdcc4428a2f4c7a7a3131f4f31c57ca92ee Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 20 Aug 2026 11:20:19 -0700 Subject: [PATCH 3/3] chore: Drop prose from the isDeleted suppression docblocks --- src/LaunchDarkly/Impl/Model/FeatureFlag.php | 4 ---- src/LaunchDarkly/Impl/Model/Segment.php | 4 ---- 2 files changed, 8 deletions(-) diff --git a/src/LaunchDarkly/Impl/Model/FeatureFlag.php b/src/LaunchDarkly/Impl/Model/FeatureFlag.php index 07ab61ef0..0e6969180 100644 --- a/src/LaunchDarkly/Impl/Model/FeatureFlag.php +++ b/src/LaunchDarkly/Impl/Model/FeatureFlag.php @@ -145,10 +145,6 @@ public function getDebugEventsUntilDate(): ?int } /** - * The requester checks the deleted marker on the raw JSON before it - * decodes, so this accessor has no internal callers. It stays because the - * model mirrors the flag schema. - * * @psalm-suppress PossiblyUnusedMethod */ public function isDeleted(): bool diff --git a/src/LaunchDarkly/Impl/Model/Segment.php b/src/LaunchDarkly/Impl/Model/Segment.php index 60486b7a1..70756b419 100644 --- a/src/LaunchDarkly/Impl/Model/Segment.php +++ b/src/LaunchDarkly/Impl/Model/Segment.php @@ -87,10 +87,6 @@ public static function decode(array $v): Segment } /** - * The requester checks the deleted marker on the raw JSON before it - * decodes, so this accessor has no internal callers. It stays because the - * model mirrors the segment schema. - * * @psalm-suppress PossiblyUnusedMethod */ public function isDeleted(): bool