From 6b7fb918cce4e8c7badfb99b2a558161bf372609 Mon Sep 17 00:00:00 2001 From: ArnabChatterjee20k Date: Fri, 8 May 2026 19:09:50 +0530 Subject: [PATCH] refactor: streamline presence upsert logic and remove redundant presenceId parameter --- src/Appwrite/Databases/PresenceState.php | 15 ++-- tests/e2e/Services/Presence/PresenceBase.php | 91 -------------------- 2 files changed, 7 insertions(+), 99 deletions(-) diff --git a/src/Appwrite/Databases/PresenceState.php b/src/Appwrite/Databases/PresenceState.php index 981e6ba57d..ce2c51420e 100644 --- a/src/Appwrite/Databases/PresenceState.php +++ b/src/Appwrite/Databases/PresenceState.php @@ -69,13 +69,13 @@ class PresenceState try { if ($dbForProject->getAdapter()->getSupportForUpsertOnUniqueIndex()) { + // $id will not be updated if new id provided. id is always stable $presence = $dbForProject->upsertDocument(self::COLLECTION_ID, $presenceDocument); $presenceCreated = $presence->getCreatedAt() === $presence->getUpdatedAt(); } else { $presence = $this->transactionalUpsertForUser( $dbForProject, $presenceDocument, - $presenceId, $userInternalId, $presenceCreated ); @@ -100,11 +100,10 @@ class PresenceState private function transactionalUpsertForUser( Database $dbForProject, Document $presenceDocument, - string $presenceId, mixed $userInternalId, ?bool &$presenceCreated = null ): Document { - return $dbForProject->withTransaction(function () use ($dbForProject, $presenceDocument, $presenceId, $userInternalId, &$presenceCreated) { + return $dbForProject->withTransaction(function () use ($dbForProject, $presenceDocument, $userInternalId, &$presenceCreated) { $existingPresence = $dbForProject->findOne(self::COLLECTION_ID, [Query::equal('userInternalId', [$userInternalId])]); if ($existingPresence->isEmpty()) { @@ -119,11 +118,11 @@ class PresenceState throw new Exception(Exception::DOCUMENT_NOT_FOUND, params: [$existingPresence->getId()]); } - if ($presenceId !== 'unique()' && $currentPresence->getId() !== $presenceId) { - $presenceDocument->setAttribute('$id', $presenceId); - $dbForProject->deleteDocument(self::COLLECTION_ID, $currentPresence->getId()); - return $dbForProject->createDocument(self::COLLECTION_ID, $presenceDocument); - } + // Mirror the native upsertDocument path: the unique key is userInternalId, so an + // existing row's $id is preserved even if the caller asked for a different one. + // Only the other fields land. Realign the input doc's $id to the existing row so + // updateDocument doesn't trip its own $id-consistency check. + $presenceDocument->setAttribute('$id', $currentPresence->getId()); return $dbForProject->updateDocument(self::COLLECTION_ID, $currentPresence->getId(), $presenceDocument); }); diff --git a/tests/e2e/Services/Presence/PresenceBase.php b/tests/e2e/Services/Presence/PresenceBase.php index f04afbbec1..c70379e1fb 100644 --- a/tests/e2e/Services/Presence/PresenceBase.php +++ b/tests/e2e/Services/Presence/PresenceBase.php @@ -908,95 +908,4 @@ trait PresenceBase $this->assertEquals($userId, $list['body']['presences'][0]['userId']); $this->assertEquals('away', $list['body']['presences'][0]['status']); } - - public function testUpsertSameUserDifferentIdsReplacesRow(): void - { - if ($this->getSide() !== 'server') { - $this->expectNotToPerformAssertions(); - return; - } - - $projectId = $this->getProject()['$id']; - $originalUser = $this->getUser(); - // Fresh user so this test isn't entangled with rows seeded by other tests in the suite. - $targetUserId = $this->getUser(true)['$id']; - // Restore the suite's cached user/session so subsequent tests aren't affected. - self::$user[$projectId] = $originalUser; - - $headers = \array_merge([ - 'content-type' => 'application/json', - 'x-appwrite-project' => $projectId, - ], $this->getPresenceServerHeaders()); - - $firstId = ID::unique(); - $first = $this->client->call( - Client::METHOD_PUT, - '/presences/' . $firstId, - $headers, - [ - 'userId' => $targetUserId, - 'status' => 'online', - 'metadata' => ['source' => 'first'], - ] - ); - $this->assertEquals(200, $first['headers']['status-code']); - $this->assertEquals($firstId, $first['body']['$id']); - $this->assertEquals($targetUserId, $first['body']['userId']); - - $secondId = ID::unique(); - $this->assertNotEquals($firstId, $secondId); - - $second = $this->client->call( - Client::METHOD_PUT, - '/presences/' . $secondId, - $headers, - [ - 'userId' => $targetUserId, - 'status' => 'away', - 'metadata' => ['source' => 'second'], - ] - ); - $this->assertEquals(200, $second['headers']['status-code']); - // Returned $id must be the one we asked for — not silently aliased to the prior row. - $this->assertEquals($secondId, $second['body']['$id']); - $this->assertEquals($targetUserId, $second['body']['userId']); - $this->assertEquals('away', $second['body']['status']); - $this->assertEquals(['source' => 'second'], $second['body']['metadata']); - - // Old presenceId is gone (the unique-key-on-userInternalId guarantees one row per user; - // an ID change must replace the prior row, not coexist with it). - $oldGet = $this->client->call( - Client::METHOD_GET, - '/presences/' . $firstId, - $headers - ); - $this->assertEquals(404, $oldGet['headers']['status-code']); - - // New presenceId is reachable and matches the upsert response. - $newGet = $this->client->call( - Client::METHOD_GET, - '/presences/' . $secondId, - $headers - ); - $this->assertEquals(200, $newGet['headers']['status-code']); - $this->assertEquals($secondId, $newGet['body']['$id']); - $this->assertEquals($targetUserId, $newGet['body']['userId']); - $this->assertEquals('away', $newGet['body']['status']); - - // Listing the user surfaces exactly one row, carrying the new $id. - $list = $this->client->call( - Client::METHOD_GET, - '/presences', - $headers, - [ - 'queries' => [ - Query::equal('userId', [$targetUserId])->toString(), - ], - ] - ); - $this->assertEquals(200, $list['headers']['status-code']); - $this->assertEquals(1, $list['body']['total']); - $this->assertCount(1, $list['body']['presences']); - $this->assertEquals($secondId, $list['body']['presences'][0]['$id']); - } }