From edef030b30a1157682ff132b0bbe37651d171265 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Sep 2026 20:28:52 +1200 Subject: [PATCH 1/7] fix: decode spatial values with the calling database's adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The point, linestring and polygon decode filters live in a static registry shared by every Database instance, yet they read $this->adapter. PHP binds $this to whichever instance constructed the closure last, so in a long-running worker with pooled per-tenant instances the decode ran against a foreign adapter — and decodePoint differs per adapter (MySQL WKB, Postgres text, Mongo arrays). The filters are already invoked with the calling instance as their third argument, so take it as a parameter rather than relying on the binding. Co-Authored-By: Claude Opus 5 --- src/Database/Database.php | 18 ++++++--- tests/unit/FilterRegistryTest.php | 64 +++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 6 deletions(-) create mode 100644 tests/unit/FilterRegistryTest.php diff --git a/src/Database/Database.php b/src/Database/Database.php index d581529fa..c33a34100 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -581,13 +581,15 @@ function (mixed $value) { }, /** * @param string|null $value + * @param Document $document + * @param Database $database * @return array|null */ - function (?string $value) { + function (?string $value, Document $document, Database $database) { if ($value === null) { return null; } - return $this->adapter->decodePoint($value); + return $database->adapter->decodePoint($value); } ); @@ -609,13 +611,15 @@ function (mixed $value) { }, /** * @param string|null $value + * @param Document $document + * @param Database $database * @return array|null */ - function (?string $value) { + function (?string $value, Document $document, Database $database) { if (is_null($value)) { return null; } - return $this->adapter->decodeLinestring($value); + return $database->adapter->decodeLinestring($value); } ); @@ -637,13 +641,15 @@ function (mixed $value) { }, /** * @param string|null $value + * @param Document $document + * @param Database $database * @return array|null */ - function (?string $value) { + function (?string $value, Document $document, Database $database) { if (is_null($value)) { return null; } - return $this->adapter->decodePolygon($value); + return $database->adapter->decodePolygon($value); } ); diff --git a/tests/unit/FilterRegistryTest.php b/tests/unit/FilterRegistryTest.php new file mode 100644 index 000000000..1bec09fd2 --- /dev/null +++ b/tests/unit/FilterRegistryTest.php @@ -0,0 +1,64 @@ + $point + */ + private function createDatabase(array $point = [0.0, 0.0]): Database + { + $adapter = $this->createMock(Adapter::class); + $adapter->method('getSupportForHostname')->willReturn(false); + $adapter->method('getTenant')->willReturn(null); + $adapter->method('getNamespace')->willReturn('test'); + $adapter->method('getSharedTables')->willReturn(false); + $adapter->method('filter')->willReturnArgument(0); + $adapter->method('decodePoint')->willReturn($point); + + return new Database($adapter, new Cache(new None())); + } + + private function pointCollection(): Document + { + return new Document([ + '$id' => 'places', + 'attributes' => [ + new Document([ + '$id' => 'location', + 'type' => Database::VAR_POINT, + 'array' => false, + 'filters' => [Database::VAR_POINT], + ]), + ], + ]); + } + + public function testSpatialDecodeUsesTheCallingDatabaseAdapter(): void + { + $first = $this->createDatabase([1.0, 2.0]); + $second = $this->createDatabase([9.0, 9.0]); + + $decoded = $first->decode( + $this->pointCollection(), + new Document(['$id' => 'a', 'location' => 'POINT(1 2)']), + ); + + $this->assertSame([1.0, 2.0], $decoded->getAttribute('location')); + + $decoded = $second->decode( + $this->pointCollection(), + new Document(['$id' => 'b', 'location' => 'POINT(9 9)']), + ); + + $this->assertSame([9.0, 9.0], $decoded->getAttribute('location')); + } +} From b1adbdb0c1698172b51dcc1849d0995bb0cc01a5 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Sep 2026 20:29:48 +1200 Subject: [PATCH 2/7] perf: memoize per-request work on the getDocument hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profiling appwrite shows Database::getDocument at 32% of CPU — every read resolves its collection through it as well. Inside that path getCacheKeys accounts for 8.6% and rebuilds identical values on every call: - getActiveFilterSignatures walked both filter maps and ksorted them per call. It is now cached per instance behind a version counter that addFilter bumps. - The cache key payload json_encoded a nested array per call. The filter map is cached pre-encoded and selects are only encoded when present. - The constructor registered the seven default filters on every instantiation, spending 14 ReflectionFunction constructions on signatures that never differ. They register once now, and unbound, so the static registry no longer pins the last Database instance alive. - getInternalAttributes array_filtered a constant twice per document. - PDO::getHostname re-parsed the DSN on every cache key computation. Measured: cache key payload 3.6us -> 1.8us, construction 11.0us -> 6.5us. Document cache keys change shape, so a deploy starts on a cold document cache — as it already does whenever a filter closure moves, since filter signatures are file:line based. Co-Authored-By: Claude Opus 5 --- src/Database/Database.php | 134 +++++++++++++++++++++--------- src/Database/PDO.php | 18 ++-- tests/unit/FilterRegistryTest.php | 25 ++++++ 3 files changed, 130 insertions(+), 47 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index c33a34100..af08adf12 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -387,11 +387,29 @@ class Database */ protected static array $filters = []; + protected static bool $defaultFiltersRegistered = false; + + protected static int $filtersVersion = 0; + + /** + * @var array>|null + */ + private static ?array $tenantlessInternalAttributes = null; + /** * @var array */ protected array $instanceFilters = []; + /** + * @var array + */ + private array $filterSignatures = []; + + private string $filterSignaturesEncoded = ''; + + private int $filterSignaturesVersion = -1; + /** * @var array> */ @@ -494,13 +512,24 @@ public function __construct( $this->setAuthorization(new Authorization()); + self::registerDefaultFilters(); + } + + private static function registerDefaultFilters(): void + { + if (self::$defaultFiltersRegistered) { + return; + } + + self::$defaultFiltersRegistered = true; + self::addFilter( 'json', /** * @param mixed $value * @return mixed */ - function (mixed $value) { + static function (mixed $value) { $value = ($value instanceof Document) ? $value->getArrayCopy() : $value; if (!is_array($value) && !$value instanceof \stdClass) { @@ -514,7 +543,7 @@ function (mixed $value) { * @return mixed * @throws Exception */ - function (mixed $value) { + static function (mixed $value) { if (!is_string($value)) { return $value; } @@ -524,7 +553,7 @@ function (mixed $value) { if (array_key_exists('$id', $value)) { return new Document($value); } else { - $value = array_map(function ($item) { + $value = array_map(static function ($item) { if (is_array($item) && array_key_exists('$id', $item)) { // if `$id` exists, create a Document instance return new Document($item); } @@ -542,7 +571,7 @@ function (mixed $value) { * @param mixed $value * @return mixed */ - function (mixed $value) { + static function (mixed $value) { if (is_null($value)) { return; } @@ -558,7 +587,7 @@ function (mixed $value) { * @param string|null $value * @return string|null */ - function (?string $value) { + static function (?string $value) { return DateTime::formatTz($value); } ); @@ -569,7 +598,7 @@ function (?string $value) { * @param mixed $value * @return mixed */ - function (mixed $value) { + static function (mixed $value) { if (!is_array($value)) { return $value; } @@ -585,7 +614,7 @@ function (mixed $value) { * @param Database $database * @return array|null */ - function (?string $value, Document $document, Database $database) { + static function (?string $value, Document $document, Database $database) { if ($value === null) { return null; } @@ -599,7 +628,7 @@ function (?string $value, Document $document, Database $database) { * @param mixed $value * @return mixed */ - function (mixed $value) { + static function (mixed $value) { if (!is_array($value)) { return $value; } @@ -615,7 +644,7 @@ function (mixed $value) { * @param Database $database * @return array|null */ - function (?string $value, Document $document, Database $database) { + static function (?string $value, Document $document, Database $database) { if (is_null($value)) { return null; } @@ -629,7 +658,7 @@ function (?string $value, Document $document, Database $database) { * @param mixed $value * @return mixed */ - function (mixed $value) { + static function (mixed $value) { if (!is_array($value)) { return $value; } @@ -645,7 +674,7 @@ function (mixed $value) { * @param Database $database * @return array|null */ - function (?string $value, Document $document, Database $database) { + static function (?string $value, Document $document, Database $database) { if (is_null($value)) { return null; } @@ -659,7 +688,7 @@ function (?string $value, Document $document, Database $database) { * @param mixed $value * @return mixed */ - function (mixed $value) { + static function (mixed $value) { if (!\is_array($value)) { return $value; } @@ -678,7 +707,7 @@ function (mixed $value) { * @param string|null $value * @return mixed */ - function (?string $value) { + static function (?string $value) { if (is_null($value)) { return null; } @@ -696,7 +725,7 @@ function (?string $value) { * @param mixed $value * @return mixed */ - function (mixed $value) { + static function (mixed $value) { if (!\is_array($value) && !$value instanceof \stdClass) { return $value; } @@ -707,7 +736,7 @@ function (mixed $value) { * @param mixed $value * @return array|null */ - function (mixed $value) { + static function (mixed $value) { if (is_null($value)) { return; } @@ -9296,6 +9325,8 @@ public static function addFilter(string $name, callable $encode, callable $decod 'decode' => $decode, 'signature' => self::computeCallableSignature($encode) . ':' . self::computeCallableSignature($decode), ]; + + self::$filtersVersion++; } /** @@ -9917,15 +9948,14 @@ public function convertQuery(Document $collection, Query $query): Query */ public function getInternalAttributes(): array { - $attributes = self::INTERNAL_ATTRIBUTES; - - if (!$this->adapter->getSharedTables()) { - $attributes = \array_filter(Database::INTERNAL_ATTRIBUTES, function ($attribute) { - return $attribute['$id'] !== '$tenant'; - }); + if ($this->adapter->getSharedTables()) { + return self::INTERNAL_ATTRIBUTES; } - return $attributes; + return self::$tenantlessInternalAttributes ??= \array_values(\array_filter( + self::INTERNAL_ATTRIBUTES, + fn (array $attribute): bool => $attribute['$id'] !== '$tenant', + )); } /** @@ -9996,11 +10026,10 @@ public function getCacheKeys(string $collectionId, ?string $documentId = null, a $sortedSelects = $selects; \sort($sortedSelects); - $payload = \json_encode([ - 'selects' => $sortedSelects, - 'relationships' => $this->resolveRelationships, - 'filters' => $this->getActiveFilterSignatures(), - ]) ?: ''; + $payload = ($this->resolveRelationships ? '1' : '0') + . ':' . $this->getFilterSignatureKey() + . ':' . ($sortedSelects === [] ? '' : (\json_encode($sortedSelects) ?: '')); + $documentHashKey = $documentKey . ':' . \md5($payload); } @@ -10142,33 +10171,56 @@ private function normalizeQueryCacheQueryValue(mixed $value): mixed */ private function getActiveFilterSignatures(): array { - $filterSignatures = []; if (!$this->filter) { - return $filterSignatures; + return []; } - $disabled = $this->disabledFilters ?? []; + $this->refreshFilterSignatures(); + + return $this->disabledFilters + ? \array_diff_key($this->filterSignatures, $this->disabledFilters) + : $this->filterSignatures; + } + + private function refreshFilterSignatures(): void + { + if ($this->filterSignaturesVersion === self::$filtersVersion) { + return; + } + + $signatures = []; foreach (self::$filters as $name => $callbacks) { - if (isset($disabled[$name])) { - continue; - } if (\array_key_exists($name, $this->instanceFilters)) { continue; } - $filterSignatures[$name] = $callbacks['signature']; + $signatures[$name] = $callbacks['signature']; } foreach ($this->instanceFilters as $name => $callbacks) { - if (isset($disabled[$name])) { - continue; - } - $filterSignatures[$name] = $callbacks['signature']; + $signatures[$name] = $callbacks['signature']; + } + + \ksort($signatures); + + $this->filterSignatures = $signatures; + $this->filterSignaturesEncoded = \json_encode($signatures) ?: ''; + $this->filterSignaturesVersion = self::$filtersVersion; + } + + private function getFilterSignatureKey(): string + { + if (!$this->filter) { + return ''; + } + + if ($this->disabledFilters) { + return \json_encode($this->getActiveFilterSignatures()) ?: ''; } - \ksort($filterSignatures); + $this->refreshFilterSignatures(); - return $filterSignatures; + return $this->filterSignaturesEncoded; } private static function computeCallableSignature(callable $callable): string @@ -10809,7 +10861,7 @@ private function resolveRelationshipGroupToIds( * @return string * @throws DatabaseException */ - protected function encodeSpatialData(mixed $value, string $type): string + protected static function encodeSpatialData(mixed $value, string $type): string { $validator = new Spatial($type); if (!$validator->isValid($value)) { diff --git a/src/Database/PDO.php b/src/Database/PDO.php index 6981ca7cb..b8c98e260 100644 --- a/src/Database/PDO.php +++ b/src/Database/PDO.php @@ -14,6 +14,8 @@ class PDO { protected \PDO $pdo; + private ?string $hostname = null; + /** * @param string $dsn * @param ?string $username @@ -136,14 +138,18 @@ public function reconnect(): void */ public function getHostname(): string { - $parts = $this->parseDsn($this->dsn); + if ($this->hostname === null) { + $parts = $this->parseDsn($this->dsn); + + /** + * @var string $host + */ + $host = $parts['host'] ?? throw new \Exception('No host found in DSN'); - /** - * @var string $host - */ - $host = $parts['host'] ?? throw new \Exception('No host found in DSN'); + $this->hostname = $host; + } - return $host; + return $this->hostname; } /** diff --git a/tests/unit/FilterRegistryTest.php b/tests/unit/FilterRegistryTest.php index 1bec09fd2..fde4d4ece 100644 --- a/tests/unit/FilterRegistryTest.php +++ b/tests/unit/FilterRegistryTest.php @@ -61,4 +61,29 @@ public function testSpatialDecodeUsesTheCallingDatabaseAdapter(): void $this->assertSame([9.0, 9.0], $decoded->getAttribute('location')); } + + public function testDefaultFilterSignaturesSurviveLaterConstruction(): void + { + $first = $this->createDatabase(); + [, , $before] = $first->getCacheKeys('places', 'a'); + + $this->createDatabase(); + + [, , $after] = $first->getCacheKeys('places', 'a'); + + $this->assertSame($before, $after); + } + + public function testRegisteringAGlobalFilterInvalidatesCacheKeys(): void + { + $database = $this->createDatabase(); + [, , $before] = $database->getCacheKeys('places', 'a'); + + $noop = fn (mixed $value) => $value; + Database::addFilter(__FUNCTION__, $noop, $noop); + + [, , $after] = $database->getCacheKeys('places', 'a'); + + $this->assertNotSame($before, $after); + } } From 5e71e878f7c61da3136dc53ae77eca0fba554622 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Sep 2026 21:17:09 +1200 Subject: [PATCH 3/7] refactor: keep encodeSpatialData an instance method Making it static to suit the now-unbound encode closures would have been a breaking change for a library: it is protected on a class downstream repos subclass, and a subclass overriding it non-statically fatals at class load. The encode filters already receive the calling instance as their third argument, same as decode, so take it there instead. Co-Authored-By: Claude Opus 5 --- src/Database/Database.php | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index af08adf12..59859235c 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -596,14 +596,16 @@ static function (?string $value) { Database::VAR_POINT, /** * @param mixed $value + * @param Document $document + * @param Database $database * @return mixed */ - static function (mixed $value) { + static function (mixed $value, Document $document, Database $database) { if (!is_array($value)) { return $value; } try { - return self::encodeSpatialData($value, Database::VAR_POINT); + return $database->encodeSpatialData($value, Database::VAR_POINT); } catch (\Throwable) { return $value; } @@ -626,14 +628,16 @@ static function (?string $value, Document $document, Database $database) { Database::VAR_LINESTRING, /** * @param mixed $value + * @param Document $document + * @param Database $database * @return mixed */ - static function (mixed $value) { + static function (mixed $value, Document $document, Database $database) { if (!is_array($value)) { return $value; } try { - return self::encodeSpatialData($value, Database::VAR_LINESTRING); + return $database->encodeSpatialData($value, Database::VAR_LINESTRING); } catch (\Throwable) { return $value; } @@ -656,14 +660,16 @@ static function (?string $value, Document $document, Database $database) { Database::VAR_POLYGON, /** * @param mixed $value + * @param Document $document + * @param Database $database * @return mixed */ - static function (mixed $value) { + static function (mixed $value, Document $document, Database $database) { if (!is_array($value)) { return $value; } try { - return self::encodeSpatialData($value, Database::VAR_POLYGON); + return $database->encodeSpatialData($value, Database::VAR_POLYGON); } catch (\Throwable) { return $value; } @@ -10861,7 +10867,7 @@ private function resolveRelationshipGroupToIds( * @return string * @throws DatabaseException */ - protected static function encodeSpatialData(mixed $value, string $type): string + protected function encodeSpatialData(mixed $value, string $type): string { $validator = new Spatial($type); if (!$validator->isValid($value)) { From ca3a1818d7e204b8fd4558210407c34dec3967f5 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Sep 2026 21:27:04 +1200 Subject: [PATCH 4/7] test: assert cache behaviour rather than cache key strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The filter memo was guarded by comparing key strings before and after a change, which makes the key format itself the contract: a harmless reshaping breaks the test, while a cache that ignored the key entirely would still pass. Drive getDocument() against a real cache instead and assert what callers observe — a document cached under the previous filter set is not served once that set changes, and a later instance with the same config hits the entry the first one wrote. This needs a cache that honours the hash argument scoping the document key. Only the Redis adapters do; the bundled Memory adapter drops it, so the test doubles their key/field semantics in memory. Co-Authored-By: Claude Opus 5 --- src/Database/Database.php | 3 + tests/unit/FilterRegistryTest.php | 124 +++++++++++++++------------- tests/unit/HashAwareMemoryCache.php | 56 +++++++++++++ tests/unit/SpatialFilterTest.php | 64 ++++++++++++++ 4 files changed, 190 insertions(+), 57 deletions(-) create mode 100644 tests/unit/HashAwareMemoryCache.php create mode 100644 tests/unit/SpatialFilterTest.php diff --git a/src/Database/Database.php b/src/Database/Database.php index 59859235c..64156df6e 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -402,6 +402,9 @@ class Database protected array $instanceFilters = []; /** + * Valid only while $instanceFilters is what the constructor set: nothing + * reassigns it, and self::$filters changing is caught by $filtersVersion. + * * @var array */ private array $filterSignatures = []; diff --git a/tests/unit/FilterRegistryTest.php b/tests/unit/FilterRegistryTest.php index fde4d4ece..97017f9be 100644 --- a/tests/unit/FilterRegistryTest.php +++ b/tests/unit/FilterRegistryTest.php @@ -3,87 +3,97 @@ namespace Tests\Unit; use PHPUnit\Framework\TestCase; -use Utopia\Cache\Adapter\None; use Utopia\Cache\Cache; -use Utopia\Database\Adapter; +use Utopia\Database\Adapter\Memory as DatabaseMemory; use Utopia\Database\Database; use Utopia\Database\Document; +use Utopia\Database\Helpers\Permission; +use Utopia\Database\Helpers\Role; class FilterRegistryTest extends TestCase { - /** - * @param array $point - */ - private function createDatabase(array $point = [0.0, 0.0]): Database - { - $adapter = $this->createMock(Adapter::class); - $adapter->method('getSupportForHostname')->willReturn(false); - $adapter->method('getTenant')->willReturn(null); - $adapter->method('getNamespace')->willReturn('test'); - $adapter->method('getSharedTables')->willReturn(false); - $adapter->method('filter')->willReturnArgument(0); - $adapter->method('decodePoint')->willReturn($point); - - return new Database($adapter, new Cache(new None())); - } + private DatabaseMemory $adapter; + + private Cache $cache; + + private string $namespace; - private function pointCollection(): Document + private Database $database; + + protected function setUp(): void { - return new Document([ - '$id' => 'places', - 'attributes' => [ - new Document([ - '$id' => 'location', - 'type' => Database::VAR_POINT, - 'array' => false, - 'filters' => [Database::VAR_POINT], - ]), - ], - ]); + $this->adapter = new DatabaseMemory(); + $this->cache = new Cache(new HashAwareMemoryCache()); + $this->namespace = 'filter_registry_' . \uniqid(); + + $this->database = $this->createDatabase(); + $this->database->create(); + $this->database->createCollection('projects'); + $this->database->createAttribute('projects', 'name', Database::VAR_STRING, 255, false); + $this->database->createDocument('projects', new Document([ + '$id' => 'project', + '$permissions' => [Permission::read(Role::any())], + 'name' => 'cached', + ])); } - public function testSpatialDecodeUsesTheCallingDatabaseAdapter(): void + private function createDatabase(): Database { - $first = $this->createDatabase([1.0, 2.0]); - $second = $this->createDatabase([9.0, 9.0]); - - $decoded = $first->decode( - $this->pointCollection(), - new Document(['$id' => 'a', 'location' => 'POINT(1 2)']), - ); + $database = new Database($this->adapter, $this->cache); - $this->assertSame([1.0, 2.0], $decoded->getAttribute('location')); + return $database + ->setDatabase('utopiaTests') + ->setNamespace($this->namespace); + } - $decoded = $second->decode( - $this->pointCollection(), - new Document(['$id' => 'b', 'location' => 'POINT(9 9)']), - ); + /** + * Write through the adapter, bypassing Database and therefore the cache + * purge, so the cache holds a copy the source no longer agrees with. A read + * returning 'cached' was served from the cache; one returning 'fresh' missed + * and went to the adapter. + */ + private function writeBehindTheCache(string $value): void + { + $collection = $this->database->getCollection('projects'); + $document = $this->adapter->getDocument($collection, 'project'); + $document->setAttribute('name', $value); + $this->adapter->updateDocument($collection, 'project', $document, true); + } - $this->assertSame([9.0, 9.0], $decoded->getAttribute('location')); + private function read(?Database $database = null): string + { + return ($database ?? $this->database) + ->getDocument('projects', 'project') + ->getAttribute('name'); } - public function testDefaultFilterSignaturesSurviveLaterConstruction(): void + public function testRegisteringAGlobalFilterStopsStaleEntriesBeingServed(): void { - $first = $this->createDatabase(); - [, , $before] = $first->getCacheKeys('places', 'a'); + $this->assertSame('cached', $this->read()); - $this->createDatabase(); + $this->writeBehindTheCache('fresh'); + $this->assertSame('cached', $this->read(), 'read should still be served from cache'); - [, , $after] = $first->getCacheKeys('places', 'a'); + $noop = fn (mixed $value) => $value; + Database::addFilter(__FUNCTION__, $noop, $noop); - $this->assertSame($before, $after); + $this->assertSame( + 'fresh', + $this->read(), + 'a document cached under the previous filter set must not be served after it changes', + ); } - public function testRegisteringAGlobalFilterInvalidatesCacheKeys(): void + public function testInstancesSharingAConfigShareCachedDocuments(): void { - $database = $this->createDatabase(); - [, , $before] = $database->getCacheKeys('places', 'a'); + $this->assertSame('cached', $this->read()); - $noop = fn (mixed $value) => $value; - Database::addFilter(__FUNCTION__, $noop, $noop); + $this->writeBehindTheCache('fresh'); - [, , $after] = $database->getCacheKeys('places', 'a'); - - $this->assertNotSame($before, $after); + $this->assertSame( + 'cached', + $this->read($this->createDatabase()), + 'a later instance with the same config must hit the entry the first one cached', + ); } } diff --git a/tests/unit/HashAwareMemoryCache.php b/tests/unit/HashAwareMemoryCache.php new file mode 100644 index 000000000..7a7218604 --- /dev/null +++ b/tests/unit/HashAwareMemoryCache.php @@ -0,0 +1,56 @@ +field($key, $hash), $ttl); + } + + /** + * @param array|string $data + * @return bool|string|array + */ + public function save(string $key, array|string $data, string $hash = ''): bool|string|array + { + return parent::save($this->field($key, $hash), $data); + } + + public function touch(string $key, string $hash = ''): bool + { + return parent::touch($this->field($key, $hash)); + } + + public function purge(string $key, string $hash = ''): bool + { + if ($hash !== '') { + return parent::purge($this->field($key, $hash)); + } + + $purged = false; + + foreach (\array_keys($this->store) as $stored) { + if ($stored === $key || \str_starts_with($stored, $key . "\0")) { + unset($this->store[$stored]); + $purged = true; + } + } + + return $purged; + } + + private function field(string $key, string $hash): string + { + return $hash === '' ? $key : $key . "\0" . $hash; + } +} diff --git a/tests/unit/SpatialFilterTest.php b/tests/unit/SpatialFilterTest.php new file mode 100644 index 000000000..01ead2082 --- /dev/null +++ b/tests/unit/SpatialFilterTest.php @@ -0,0 +1,64 @@ + $point + */ + private function createDatabase(array $point): Database + { + $adapter = $this->createMock(Adapter::class); + $adapter->method('getSupportForHostname')->willReturn(false); + $adapter->method('getTenant')->willReturn(null); + $adapter->method('getNamespace')->willReturn('test'); + $adapter->method('getSharedTables')->willReturn(false); + $adapter->method('filter')->willReturnArgument(0); + $adapter->method('decodePoint')->willReturn($point); + + return new Database($adapter, new Cache(new None())); + } + + private function pointCollection(): Document + { + return new Document([ + '$id' => 'places', + 'attributes' => [ + new Document([ + '$id' => 'location', + 'type' => Database::VAR_POINT, + 'array' => false, + 'filters' => [Database::VAR_POINT], + ]), + ], + ]); + } + + private function decode(Database $database, string $id): mixed + { + return $database + ->decode($this->pointCollection(), new Document(['$id' => $id, 'location' => 'POINT(0 0)'])) + ->getAttribute('location'); + } + + public function testSpatialDecodeUsesTheCallingDatabaseAdapter(): void + { + $first = $this->createDatabase([1.0, 2.0]); + $second = $this->createDatabase([9.0, 9.0]); + + $this->assertSame([1.0, 2.0], $this->decode($first, 'a')); + $this->assertSame([9.0, 9.0], $this->decode($second, 'b')); + + // The decode filters live in a static registry shared by both instances, + // so the first must still reach its own adapter after the second exists. + $this->assertSame([1.0, 2.0], $this->decode($first, 'c')); + } +} From 0510951671eae73df789e6b9435ecb2951aa6ddf Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Sep 2026 21:46:57 +1200 Subject: [PATCH 5/7] fix: invalidate the filter memo when instance filters change The memo was only versioned against the static registry, so a subclass replacing $instanceFilters after construction kept the old cache key: a read could be served an entry written under the previous callbacks, and a miss would write the new value back under that same stale key. Snapshotting the array and comparing identity closes it. Nothing in the library reassigns the property, so the comparison hits PHP's pointer short-circuit for arrays that share a zend_array and costs nothing on the hot path. Co-Authored-By: Claude Opus 5 --- src/Database/Database.php | 14 ++++++++++---- tests/unit/FilterRegistryTest.php | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index 64156df6e..c635c1307 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -402,9 +402,6 @@ class Database protected array $instanceFilters = []; /** - * Valid only while $instanceFilters is what the constructor set: nothing - * reassigns it, and self::$filters changing is caught by $filtersVersion. - * * @var array */ private array $filterSignatures = []; @@ -413,6 +410,11 @@ class Database private int $filterSignaturesVersion = -1; + /** + * @var array + */ + private array $filterSignaturesSource = []; + /** * @var array> */ @@ -10193,7 +10195,10 @@ private function getActiveFilterSignatures(): array private function refreshFilterSignatures(): void { - if ($this->filterSignaturesVersion === self::$filtersVersion) { + if ( + $this->filterSignaturesVersion === self::$filtersVersion + && $this->filterSignaturesSource === $this->instanceFilters + ) { return; } @@ -10215,6 +10220,7 @@ private function refreshFilterSignatures(): void $this->filterSignatures = $signatures; $this->filterSignaturesEncoded = \json_encode($signatures) ?: ''; $this->filterSignaturesVersion = self::$filtersVersion; + $this->filterSignaturesSource = $this->instanceFilters; } private function getFilterSignatureKey(): string diff --git a/tests/unit/FilterRegistryTest.php b/tests/unit/FilterRegistryTest.php index 97017f9be..e4a235681 100644 --- a/tests/unit/FilterRegistryTest.php +++ b/tests/unit/FilterRegistryTest.php @@ -84,6 +84,34 @@ public function testRegisteringAGlobalFilterStopsStaleEntriesBeingServed(): void ); } + public function testChangingInstanceFiltersStopsStaleEntriesBeingServed(): void + { + $database = new class ($this->adapter, $this->cache) extends Database { + public function swapInstanceFilter(string $signature): void + { + $noop = fn (mixed $value) => $value; + + $this->instanceFilters = [ + 'probe' => ['encode' => $noop, 'decode' => $noop, 'signature' => $signature], + ]; + } + }; + $database->setDatabase('utopiaTests')->setNamespace($this->namespace); + + $this->assertSame('cached', $this->read($database)); + + $this->writeBehindTheCache('fresh'); + $this->assertSame('cached', $this->read($database), 'read should still be served from cache'); + + $database->swapInstanceFilter('v2'); + + $this->assertSame( + 'fresh', + $this->read($database), + 'a subclass replacing its instance filters must not keep serving the previous entry', + ); + } + public function testInstancesSharingAConfigShareCachedDocuments(): void { $this->assertSame('cached', $this->read()); From cf7862c09147d71bfc7690878d3169f8371e4b8d Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Sep 2026 22:37:38 +1200 Subject: [PATCH 6/7] fix: make a built-in filter override win in either order Registering the defaults once left the registry order-dependent: an addFilter() for a built-in name issued before the first constructor was clobbered by it, while the same call after one was permanent. Appwrite Cloud overrides 'datetime' this way from its migration workers, so the rule it lands on matters. Register the defaults on first touch of the registry instead, from addFilter() as well as the constructor, so an explicit registration always wins. The guard is set before the defaults are registered because addFilter() now calls back into the registrar; that guard is what ends the recursion. Also restore the static registry between tests. addFilter() has no removal API, so a test registering a filter otherwise leaked into every test that ran after it. Co-Authored-By: Claude Opus 5 --- src/Database/Database.php | 8 ++++++ tests/unit/FilterRegistryTest.php | 41 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/Database/Database.php b/src/Database/Database.php index c635c1307..9d9542822 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -520,6 +520,12 @@ public function __construct( self::registerDefaultFilters(); } + /** + * Registers the built-in filters on first touch of the registry, so an + * explicit addFilter() always wins regardless of whether it ran before or + * after the first instance. The flag is set first: addFilter() calls back + * into this, and the guard is what terminates that recursion. + */ private static function registerDefaultFilters(): void { if (self::$defaultFiltersRegistered) { @@ -9331,6 +9337,8 @@ public function sum(string $collection, string $attribute, array $queries = [], */ public static function addFilter(string $name, callable $encode, callable $decode): void { + self::registerDefaultFilters(); + self::$filters[$name] = [ 'encode' => $encode, 'decode' => $decode, diff --git a/tests/unit/FilterRegistryTest.php b/tests/unit/FilterRegistryTest.php index e4a235681..454e67105 100644 --- a/tests/unit/FilterRegistryTest.php +++ b/tests/unit/FilterRegistryTest.php @@ -20,6 +20,11 @@ class FilterRegistryTest extends TestCase private Database $database; + /** + * @var array + */ + private array $registry; + protected function setUp(): void { $this->adapter = new DatabaseMemory(); @@ -27,6 +32,12 @@ protected function setUp(): void $this->namespace = 'filter_registry_' . \uniqid(); $this->database = $this->createDatabase(); + + // Snapshot once the constructor has registered the built-ins, so the + // restore in tearDown puts back a populated registry rather than an + // empty one. + $this->registry = (new \ReflectionProperty(Database::class, 'filters'))->getValue(); + $this->database->create(); $this->database->createCollection('projects'); $this->database->createAttribute('projects', 'name', Database::VAR_STRING, 255, false); @@ -37,6 +48,14 @@ protected function setUp(): void ])); } + protected function tearDown(): void + { + // addFilter() writes to a static registry with no removal API, so a test + // registering one would otherwise leak into every later test. + (new \ReflectionProperty(Database::class, 'filters'))->setValue(null, $this->registry); + (new \ReflectionProperty(Database::class, 'defaultFiltersRegistered'))->setValue(null, true); + } + private function createDatabase(): Database { $database = new Database($this->adapter, $this->cache); @@ -112,6 +131,28 @@ public function swapInstanceFilter(string $signature): void ); } + public function testOverridingABuiltInFilterBeforeTheFirstInstanceStillWins(): void + { + $registry = new \ReflectionProperty(Database::class, 'filters'); + + // A fresh process: nothing has constructed a Database yet, so the + // built-ins are not in the registry. + $registry->setValue(null, []); + (new \ReflectionProperty(Database::class, 'defaultFiltersRegistered'))->setValue(null, false); + + $identity = fn (mixed $value) => $value; + Database::addFilter('datetime', $identity, $identity); + $override = $registry->getValue()['datetime']['signature']; + + $this->createDatabase(); + + $this->assertSame( + $override, + $registry->getValue()['datetime']['signature'], + 'constructing a database must not restore a built-in filter the caller replaced before it', + ); + } + public function testInstancesSharingAConfigShareCachedDocuments(): void { $this->assertSame('cached', $this->read()); From 1c4fe4f06e50adbc51d4bce0465cbdc0de41725a Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Mon, 21 Sep 2026 23:40:45 +1200 Subject: [PATCH 7/7] test: assert the override runs, not what the registry holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The boot-order test compared a signature string inside the static filter registry, which passes whether or not the override is the filter that actually executes. Decode a datetime through it instead: the override hands the value back untouched where the built-in would return ISO 8601, so the assertion now fails on the behaviour rather than on a representation. Reflection still resets the statics to stand in for a fresh process — there is no public way to unregister a filter — but it is confined to setup and teardown, not the assertion. Co-Authored-By: Claude Opus 5 --- tests/unit/FilterRegistryTest.php | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/tests/unit/FilterRegistryTest.php b/tests/unit/FilterRegistryTest.php index 454e67105..f817efce7 100644 --- a/tests/unit/FilterRegistryTest.php +++ b/tests/unit/FilterRegistryTest.php @@ -133,23 +133,34 @@ public function swapInstanceFilter(string $signature): void public function testOverridingABuiltInFilterBeforeTheFirstInstanceStillWins(): void { - $registry = new \ReflectionProperty(Database::class, 'filters'); - // A fresh process: nothing has constructed a Database yet, so the // built-ins are not in the registry. - $registry->setValue(null, []); + (new \ReflectionProperty(Database::class, 'filters'))->setValue(null, []); (new \ReflectionProperty(Database::class, 'defaultFiltersRegistered'))->setValue(null, false); $identity = fn (mixed $value) => $value; Database::addFilter('datetime', $identity, $identity); - $override = $registry->getValue()['datetime']['signature']; - $this->createDatabase(); + $decoded = $this->createDatabase()->decode( + new Document([ + '$id' => 'events', + 'attributes' => [ + new Document([ + '$id' => 'occurredAt', + 'type' => Database::VAR_DATETIME, + 'array' => false, + 'filters' => ['datetime'], + ]), + ], + ]), + new Document(['$id' => 'event', 'occurredAt' => '2026-09-21 10:00:00.000']), + ); + // The built-in decode would hand back '2026-09-21T10:00:00.000+00:00'. $this->assertSame( - $override, - $registry->getValue()['datetime']['signature'], - 'constructing a database must not restore a built-in filter the caller replaced before it', + '2026-09-21 10:00:00.000', + $decoded->getAttribute('occurredAt'), + 'the override registered before the first instance must be the filter that runs', ); }