Fixed replaceOne implementation

event-driven-io · Sep 29, 2024 · 73d36fd · 73d36fd
1 parent 4b10770
commit 73d36fd
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 76 deletions.
diff --git a/src/packages/pongo/src/e2e/postgres.optimistic-concurrency.spec.ts b/src/packages/pongo/src/e2e/postgres.optimistic-concurrency.spec.ts
@@ -259,7 +259,7 @@ void describe('MongoDB Compatibility Tests', () => {
 
   void describe('Update Operations', () => {
     void describe('updateOne', () => {
-      void it('updates a document WITHOUT correct expected version', async () => {
+      void it('updates a document WITHOUT passing expected version', async () => {
         // Given
         await users.insertOne(user);
 
@@ -342,29 +342,90 @@ void describe('MongoDB Compatibility Tests', () => {
     });
   });
 
-  // void describe('Replace Operations', () => {
-  //   void it('should replace a document', async () => {
-  //     const pongoCollection = pongoDb.collection<User>('updateOne');
-  //     const doc = { name: 'Roger', age: 30 };
+  void describe('Replace Operations', () => {
+    void describe('replaceOne', () => {
+      void it('replaces a document WITHOUT passing expected version', async () => {
+        // Given
+        await users.insertOne(user);
 
-  //     const pongoInsertResult = await pongoCollection.insertOne(doc);
+        // When
+        const updateResult = await users.replaceOne(
+          { _id: user._id! },
+          { ...user, age: 31 },
+        );
 
-  //     const replacement = { name: 'Not Roger', age: 100, tags: ['tag2'] };
+        //Then
+        assert(updateResult.successful);
+        assert(updateResult.modifiedCount === 1);
+        assert(updateResult.matchedCount === 1);
 
-  //     await pongoCollection.replaceOne(
-  //       { _id: pongoInsertResult.insertedId! },
-  //       replacement,
-  //     );
+        const pongoDoc = await users.findOne({
+          _id: user._id!,
+        });
 
-  //     const pongoDoc = await pongoCollection.findOne({
-  //       _id: pongoInsertResult.insertedId!,
-  //     });
+        assert.deepStrictEqual(pongoDoc, {
+          ...user,
+          age: 31,
+          _version: 2n,
+        });
+      });
 
-  //     assert.strictEqual(pongoDoc?.name, replacement.name);
-  //     assert.deepEqual(pongoDoc?.age, replacement.age);
-  //     assert.deepEqual(pongoDoc?.tags, replacement.tags);
-  //   });
-  // });
+      void it('replaces a document with correct expected version', async () => {
+        // Given
+        await users.insertOne(user);
+
+        // When
+        const updateResult = await users.replaceOne(
+          { _id: user._id! },
+          { ...user, age: 31 },
+          { expectedVersion: 1n },
+        );
+
+        //Then
+        assert(updateResult.successful);
+        assert(updateResult.modifiedCount === 1);
+        assert(updateResult.matchedCount === 1);
+
+        const pongoDoc = await users.findOne({
+          _id: user._id!,
+        });
+
+        assert.deepStrictEqual(pongoDoc, {
+          ...user,
+          age: 31,
+          _version: 2n,
+        });
+      });
+
+      [0n, 2n, -1n, 3n].map((incorrectVersion) => {
+        void it(`does NOT replace a document with incorrect ${incorrectVersion} expected version`, async () => {
+          // Given
+          await users.insertOne(user);
+
+          // When
+          const updateResult = await users.replaceOne(
+            { _id: user._id! },
+            { ...user, age: 31 },
+            { expectedVersion: incorrectVersion },
+          );
+
+          //Then
+          assert(updateResult.successful === false);
+          assert(updateResult.modifiedCount === 0);
+          assert(updateResult.matchedCount === 1);
+
+          const pongoDoc = await users.findOne({
+            _id: user._id!,
+          });
+
+          assert.deepStrictEqual(pongoDoc, {
+            ...user,
+            _version: 1n,
+          });
+        });
+      });
+    });
+  });
 
   // void describe('Delete Operations', () => {
   //   void it('should delete a document from both PostgreSQL and MongoDB', async () => {

diff --git a/src/packages/pongo/src/postgres/sqlBuilder/index.ts b/src/packages/pongo/src/postgres/sqlBuilder/index.ts
@@ -98,7 +98,9 @@ export const postgresSQLBuilder = (
       ),
       updated AS (
         UPDATE %I 
-        SET data = %s, _version = _version + 1
+        SET 
+          data = %s || jsonb_build_object('_version', (_version + 1)::text),
+          _version = _version + 1
         FROM existing 
         WHERE %I._id = existing._id ${expectedVersionUpdate}
         RETURNING %I._id
@@ -109,8 +111,7 @@ export const postgresSQLBuilder = (
         COUNT(updated._id) over() AS modified
       FROM existing
       LEFT JOIN updated 
-      ON existing._id = updated._id;  
-      `,
+      ON existing._id = updated._id;`,
       collectionName,
       where(filterQuery),
       collectionName,
@@ -152,60 +153,46 @@ export const postgresSQLBuilder = (
     document: WithoutId<T>,
     options?: ReplaceOneOptions,
   ): SQL => {
-    const expectedVersion = options?.expectedVersion
-      ? expectedVersionValue(options.expectedVersion)
-      : null;
-
-    const expectedVersionUpdate = expectedVersion
-      ? { _version: expectedVersionValue(expectedVersion) }
-      : {};
+    const expectedVersion = expectedVersionValue(options?.expectedVersion);
+    const expectedVersionUpdate =
+      expectedVersion != null ? 'AND %I._version = %L' : '';
+    const expectedVersionParams =
+      expectedVersion != null ? [collectionName, expectedVersion] : [];
 
-    const filterQuery = constructFilterQuery<T>({
-      ...expectedVersionUpdate,
-      ...filter,
-    });
+    const filterQuery = constructFilterQuery<T>(filter);
 
-    return expectedVersion
-      ? sql(
-          `WITH cte AS (
-        SELECT 
-          _id, 
-          CASE WHEN _version = %L THEN 1 ELSE 0 END AS matched,  
-          1 as modified, 
-        FROM %I %s LIMIT 1
+    const query = sql(
+      `WITH existing AS (
+        SELECT _id
+        FROM %I %s 
+        LIMIT 1
+      ),
+      updated AS (
+        UPDATE %I        
+        SET 
+          data = %L || jsonb_build_object('_id', data->>'_id') || jsonb_build_object('_version', (_version + 1)::text),
+          _version = _version + 1
+        FROM existing 
+        WHERE %I._id = existing._id ${expectedVersionUpdate}
+        RETURNING %I._id
       )
-      UPDATE %I 
-      SET data = %L || jsonb_build_object('_id', data->>'_id')
-      FROM cte 
-      WHERE %I._id = cte._id AND %I._version = %L
-      RETURNING cte.matched, cte.modified;`,
-          expectedVersion,
-          collectionName,
-          where(filterQuery),
-          collectionName,
-          JSONSerializer.serialize(document),
-          collectionName,
-          expectedVersion,
-        )
-      : sql(
-          `WITH cte AS (
       SELECT 
-        _id, 
-        1 as matched, 
-        1 as modified 
-      FROM %I %s LIMIT 1
-    )
-    UPDATE %I 
-    SET data = %L || jsonb_build_object('_id', data->>'_id')
-    FROM cte 
-    WHERE %I._id = cte._id
-    RETURNING cte.matched, cte.modified;`,
-          collectionName,
-          where(filterQuery),
-          collectionName,
-          JSONSerializer.serialize(document),
-          collectionName,
-        );
+        existing._id,
+        COUNT(existing._id) over() AS matched,
+        COUNT(updated._id) over() AS modified
+      FROM existing
+      LEFT JOIN updated 
+      ON existing._id = updated._id;`,
+      collectionName,
+      where(filterQuery),
+      collectionName,
+      JSONSerializer.serialize(document),
+      collectionName,
+      ...expectedVersionParams,
+      collectionName,
+    );
+
+    return query;
   },
   updateMany: <T>(filter: PongoFilter<T>, update: PongoUpdate<T>): SQL => {
     const filterQuery = constructFilterQuery(filter);

diff --git a/src/packages/pongo/src/postgres/sqlBuilder/update/index.ts b/src/packages/pongo/src/postgres/sqlBuilder/update/index.ts
@@ -9,10 +9,7 @@ import {
 } from '../../../core';
 
 export const buildUpdateQuery = <T>(update: PongoUpdate<T>): SQL =>
-  objectEntries({
-    ...update,
-    $inc: { _version: 1n, ...(update.$inc ?? {}) },
-  }).reduce((currentUpdateQuery, [op, value]) => {
+  objectEntries(update).reduce((currentUpdateQuery, [op, value]) => {
     switch (op) {
       case '$set':
         return buildSetQuery(value, currentUpdateQuery);