Skip to content

Fix cache clobber #815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abnegate wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Fix cache clobber #815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
241cd90
Use utopia base
abnegate Feb 16, 2026
0e7c43e
Fix cache clobber
abnegate Feb 16, 2026
e7bd2da
Merge branch 'fix-ghost-perms' into fix-cache-clobber
abnegate Feb 16, 2026
2aa26e6
fix: Revert Dockerfile to use php:8.4-cli-alpine base image
abnegate Feb 16, 2026
1727eaf
Revert "fix: Revert Dockerfile to use php:8.4-cli-alpine base image"
abnegate Feb 16, 2026
0f3aeec
fix: Use correct Docker Hub image name for utopia base
abnegate Feb 16, 2026
b518052
debug: Log attribute differences in updateDocument
abnegate Feb 16, 2026
278151f
debug: Use stdout for attribute difference logging
abnegate Feb 16, 2026
a4d1d34
debug: Include diff info in AuthorizationException message
abnegate Feb 16, 2026
3f6ccc2
fix: Handle int/float type coercion in updateDocument comparison
abnegate Feb 16, 2026
e5d0d3f
fix: Flaky testSingleDocumentDateOperations timing issue
abnegate Feb 16, 2026
0380e4a
fix: Cast merged document types before comparison in updateDocument
abnegate Feb 16, 2026
142b8dd
fix: Cast comparison values by attribute type in updateDocument
abnegate Feb 16, 2026
451d523
fix: Cast both sides of comparison and fix flaky timing test
abnegate Feb 16, 2026
389e166
fix: Replace arrow functions with switch for PSR-12 compliance
abnegate Feb 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 2 additions & 41 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,9 @@ RUN composer install \
--no-scripts \
--prefer-dist

FROM php:8.4.18-cli-alpine3.22 AS compile
FROM appwrite/utopia-base:php-8.4-0.2.1 AS compile

ENV PHP_REDIS_VERSION="6.3.0" \
PHP_SWOOLE_VERSION="v6.1.6" \
PHP_XDEBUG_VERSION="3.4.2" \
PHP_MONGODB_VERSION="2.1.1"
RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone
ENV PHP_MONGODB_VERSION="2.1.1"

RUN apk update && apk add --no-cache \
libpq \
Expand All @@ -28,8 +24,6 @@ RUN apk update && apk add --no-cache \
autoconf \
gcc \
g++ \
git \
brotli-dev \
linux-headers \
docker-cli \
docker-cli-compose \
Expand All @@ -48,24 +42,6 @@ RUN apk update && apk add --no-cache \
&& apk del libpq-dev \
&& rm -rf /var/cache/apk/*

# Redis Extension
FROM compile AS redis
RUN \
git clone --depth 1 --branch $PHP_REDIS_VERSION https://github.com/phpredis/phpredis.git \
&& cd phpredis \
&& phpize \
&& ./configure \
&& make && make install

## Swoole Extension
FROM compile AS swoole
RUN \
git clone --depth 1 --branch $PHP_SWOOLE_VERSION https://github.com/swoole/swoole-src.git \
&& cd swoole-src \
&& phpize \
&& ./configure --enable-http2 \
&& make && make install

## PCOV Extension
FROM compile AS pcov
RUN \
Expand All @@ -75,15 +51,6 @@ RUN \
&& ./configure --enable-pcov \
&& make && make install

## XDebug Extension
FROM compile AS xdebug
RUN \
git clone --depth 1 --branch $PHP_XDEBUG_VERSION https://github.com/xdebug/xdebug && \
cd xdebug && \
phpize && \
./configure && \
make && make install

FROM compile AS final

LABEL maintainer="team@appwrite.io"
Expand All @@ -93,10 +60,7 @@ ENV DEBUG=$DEBUG

WORKDIR /usr/src/code

RUN echo extension=redis.so >> /usr/local/etc/php/conf.d/redis.ini
RUN echo extension=swoole.so >> /usr/local/etc/php/conf.d/swoole.ini
RUN echo extension=pcov.so >> /usr/local/etc/php/conf.d/pcov.ini
RUN echo extension=xdebug.so >> /usr/local/etc/php/conf.d/xdebug.ini

RUN mv "$PHP_INI_DIR/php.ini-production" "$PHP_INI_DIR/php.ini"

Expand All @@ -105,10 +69,7 @@ RUN echo "opcache.enable_cli=1" >> $PHP_INI_DIR/php.ini
RUN echo "memory_limit=1024M" >> $PHP_INI_DIR/php.ini

COPY --from=composer /usr/local/src/vendor /usr/src/code/vendor
COPY --from=swoole /usr/local/lib/php/extensions/no-debug-non-zts-20240924/swoole.so /usr/local/lib/php/extensions/no-debug-non-zts-20240924/
COPY --from=redis /usr/local/lib/php/extensions/no-debug-non-zts-20240924/redis.so /usr/local/lib/php/extensions/no-debug-non-zts-20240924/
COPY --from=pcov /usr/local/lib/php/extensions/no-debug-non-zts-20240924/pcov.so /usr/local/lib/php/extensions/no-debug-non-zts-20240924/
COPY --from=xdebug /usr/local/lib/php/extensions/no-debug-non-zts-20240924/xdebug.so /usr/local/lib/php/extensions/no-debug-non-zts-20240924/

COPY ./bin /usr/src/code/bin
COPY ./src /usr/src/code/src
Expand Down
42 changes: 36 additions & 6 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4271,11 +4271,14 @@ public function getDocument(string $collection, string $id, array $queries = [],
$selections
);

try {
$cached = $this->cache->load($documentKey, self::TTL, $hashKey);
} catch (Exception $e) {
Console::warning('Warning: Failed to get document from cache: ' . $e->getMessage());
$cached = null;
$cached = null;
if (!$forUpdate) {
try {
$cached = $this->cache->load($documentKey, self::TTL, $hashKey);
} catch (Exception $e) {
Console::warning('Warning: Failed to get document from cache: ' . $e->getMessage());
$cached = null;
}
}

if ($cached) {
Expand Down Expand Up @@ -4348,7 +4351,7 @@ public function getDocument(string $collection, string $id, array $queries = [],
);

// Don't save to cache if it's part of a relationship
if (empty($relationships)) {
if (!$forUpdate && empty($relationships)) {
try {
$this->cache->save($documentKey, $document->getArrayCopy(), $hashKey);
$this->cache->save($collectionKey, 'empty', $documentKey);
Expand Down Expand Up @@ -5598,6 +5601,12 @@ public function updateDocument(string $collection, string $id, Document $documen
return $attribute['type'] === Database::VAR_RELATIONSHIP;
});

// Build attribute type map for type-safe comparison
$attributeTypes = [];
foreach ($collection->getAttribute('attributes', []) as $attr) {
$attributeTypes[$attr['$id'] ?? ''] = $attr['type'] ?? '';
}

$shouldUpdate = false;

if ($collection->getId() !== self::METADATA) {
Expand Down Expand Up @@ -5695,6 +5704,27 @@ public function updateDocument(string $collection, string $id, Document $documen

$oldValue = $old->getAttribute($key);

// Cast both values to attribute type for consistent comparison
// (e.g. cache JSON round-trip turns float 1.0 into int 1,
// and some adapters may not cast floats on read)
$attrType = $attributeTypes[$key] ?? null;
if ($attrType !== null && !($value instanceof Operator)) {
switch ($attrType) {
case self::VAR_FLOAT:
$value = \is_null($value) ? null : (float)$value;
$oldValue = \is_null($oldValue) ? null : (float)$oldValue;
break;
case self::VAR_INTEGER:
$value = \is_null($value) ? null : (int)$value;
$oldValue = \is_null($oldValue) ? null : (int)$oldValue;
break;
case self::VAR_BOOLEAN:
$value = \is_null($value) ? null : (bool)$value;
$oldValue = \is_null($oldValue) ? null : (bool)$oldValue;
break;
}
}

// If values are not equal we need to update document.
if ($value !== $oldValue) {
$shouldUpdate = true;
Expand Down
51 changes: 51 additions & 0 deletions tests/e2e/Adapter/Scopes/DocumentTests.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5834,6 +5834,7 @@ public function testSingleDocumentDateOperations(): void

$doc4->setAttribute('$updatedAt', null);
$doc4->setAttribute('$createdAt', null);
\usleep(2000); // Ensure updatedAt timestamp differs from creation time
$updatedDoc4 = $database->updateDocument($collection, 'doc4', document: $doc4);

$this->assertEquals($originalCreatedAt4, $updatedDoc4->getAttribute('$createdAt'));
Expand Down Expand Up @@ -5921,6 +5922,8 @@ public function testSingleDocumentDateOperations(): void

$newUpdatedAt = $doc11->getUpdatedAt();

\usleep(2000); // Ensure updatedAt timestamp differs from creation time
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid flaky timestamp comparisons across adapters.

A 2ms sleep may not advance $updatedAt on adapters with second/millisecond precision, making the subsequent assertion non-deterministic. Consider sleeping at least a second (or looping until the timestamp changes).

🛠️ Suggested fix 
-        \usleep(2000); // Ensure updatedAt timestamp differs from creation time
+        // Some adapters only store timestamps with second precision.
+        \usleep(1_100_000); // Ensure updatedAt timestamp differs from creation time
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
\usleep(2000); // Ensure updatedAt timestamp differs from creation time
// Some adapters only store timestamps with second precision.
\usleep(1_100_000); // Ensure updatedAt timestamp differs from creation time
🤖 Prompt for AI Agents 
In `@tests/e2e/Adapter/Scopes/DocumentTests.php` at line 5924, The test currently
calls \usleep(2000) to try to force an updatedAt change which is flaky across
adapters; replace this single microsecond sleep by waiting until the timestamp
actually changes (either use sleep(1) to wait at least one second or implement a
short loop that reloads the document and breaks when updatedAt !== createdAt,
with a reasonable timeout) in the test that contains the \usleep(2000) call so
the assertion comparing createdAt and updatedAt is deterministic.


$newDoc11 = new Document([
'string' => 'no_dates_update',
]);
Expand Down Expand Up @@ -7456,6 +7459,54 @@ public function testRegexInjection(): void
$database->deleteCollection($collectionName);
}

public function testUpdateDocumentUsesFreshForUpdateReadWhenCacheIsStale(): void
{
/** @var Database $database */
$database = $this->getDatabase();

$collectionId = 'for_update_cache';
$database->createCollection($collectionId);

if ($database->getAdapter()->getSupportForAttributes()) {
$this->assertEquals(true, $database->createAttribute($collectionId, 'a', Database::VAR_STRING, 255, false));
$this->assertEquals(true, $database->createAttribute($collectionId, 'b', Database::VAR_STRING, 255, false));
}

$database->createDocument($collectionId, new Document([
'$id' => 'doc1',
'$permissions' => [
Permission::read(Role::any()),
Permission::update(Role::any()),
],
'a' => 'A1',
'b' => 'B1',
]));

// Prime cache with initial values.
$cached = $database->getDocument($collectionId, 'doc1');
$this->assertEquals('B1', $cached->getAttribute('b'));

$collection = $database->getCollection($collectionId);

// Simulate an out-of-band write that bypasses cache invalidation.
$outOfBand = $database->getAdapter()->getDocument($collection, 'doc1');
$outOfBand->setAttribute('b', 'B2');
$database->getAdapter()->updateDocument($collection, 'doc1', $outOfBand, true);

// Partial update should not overwrite untouched fields with stale cached values.
$updated = $database->updateDocument($collectionId, 'doc1', new Document([
'a' => 'A2',
]));

$this->assertEquals('A2', $updated->getAttribute('a'));
$this->assertEquals('B2', $updated->getAttribute('b'));

$fresh = $database->getDocument($collectionId, 'doc1');
$this->assertEquals('B2', $fresh->getAttribute('b'));

$database->deleteCollection($collectionId);
}

/**
* Test ReDoS (Regular Expression Denial of Service) with timeout protection
* This test verifies that ReDoS patterns either timeout properly or complete quickly,
Expand Down