refresh write lock expiry time when calling acquireWriteLock as the s…

…ame user (#5918)

common/api/core-backend.api.md

@@ -919,6 +919,7 @@ export namespace CloudSqlite {
         releaseWriteLock(): void;
         get storageType(): string;
         uploadChanges(): Promise<void>;
+        get writeLockExpires(): string;
     }
     // (undocumented)
     export interface CloudHttpProps {

core/backend/src/CloudSqlite.ts

@@ -47,7 +47,6 @@ export namespace CloudSqlite {
     refreshPromise?: Promise<void>;
     lockExpireSeconds: number;
     writeLockHeldBy?: string;
-    writeLockExpires?: string;
   }
 
   /**
@@ -346,6 +345,10 @@ export namespace CloudSqlite {
     get alias(): string;
     /** The logId. */
     get logId(): string;
+    /** The time that the write lock expires. Of the form 'YYYY-MM-DDTHH:MM:SS.000Z' in UTC.
+     *  Returns empty string if write lock is not held.
+     */
+    get writeLockExpires(): string;
     /** true if this CloudContainer is currently connected to a CloudCache via the `connect` method. */
     get isConnected(): boolean;
     /** true if this CloudContainer was created with the `writeable` flag (and its `accessToken` supplies write access). */
@@ -581,7 +584,7 @@ export namespace CloudSqlite {
   /**
     * Attempt to acquire the write lock for a container, with retries.
     * If write lock is held by another user, call busyHandler if supplied. If no busyHandler, or handler returns "stop", throw. Otherwise try again.
-    * @note if write lock is already held, this function does nothing.
+    * @note if write lock is already held by the same user, this function will refresh the write lock's expiry time.
     * @param user the name to be displayed to other users in the event they attempt to obtain the lock while it is held by us
     * @param container the CloudContainer for which the lock is to be acquired
     * @param busyHandler if present, function called when the write lock is currently held by another user.
@@ -592,9 +595,9 @@ export namespace CloudSqlite {
     while (true) {
       try {
         if (container.hasWriteLock) {
-          if (container.writeLockHeldBy === args.user)
-            return; // current user already has the lock
-
+          if (container.writeLockHeldBy === args.user) {
+            return container.acquireWriteLock(args.user); // refresh the write lock's expiry time.
+          }
           const err = new Error() as any; // lock held by another user within this process
           err.errorNumber = 5;
           err.lockedBy = container.writeLockHeldBy;
@@ -613,7 +616,7 @@ export namespace CloudSqlite {
 
   /**
  * Perform an asynchronous write operation on a CloudContainer with the write lock held.
- * 1. if write lock is already held by the current user, call operation and return.
+ * 1. if write lock is already held by the current user, refresh write lock's expiry time, call operation and return.
  * 2. attempt to acquire the write lock, with retries. Throw if unable to obtain write lock.
  * 3. perform the operation
  * 3.a if the operation throws, abandon all changes and re-throw
@@ -632,17 +635,14 @@ export namespace CloudSqlite {
       if (containerInternal.writeLockHeldBy === args.user) // If the user already had the write lock, then don't release it.
         return await operation();
       containerInternal.writeLockHeldBy = args.user;
-      containerInternal.writeLockExpires = new Date(Date.now() + 1000 * containerInternal.lockExpireSeconds).toLocaleString();
       // eslint-disable-next-line @typescript-eslint/await-thenable
       const val = await operation(); // wait for work to finish or fail
       containerInternal.releaseWriteLock();
       containerInternal.writeLockHeldBy = undefined;
-      containerInternal.writeLockExpires = undefined;
       return val;
     } catch (e) {
       args.container.abandonChanges();  // if operation threw, abandon all changes
       containerInternal.writeLockHeldBy = undefined;
-      containerInternal.writeLockExpires = undefined;
       throw e;
     }
   }

full-stack-tests/backend/src/integration/Checkpoints.test.ts

@@ -166,6 +166,7 @@ describe("Checkpoints", () => {
       return {} as SnapshotDb;
     });
     sinon.stub(CheckpointManager, "validateCheckpointGuids").callsFake(() => { });
+
     await SnapshotDb.openCheckpointV2({
       accessToken,
       iTwinId: testITwinId,
@@ -277,6 +278,7 @@ describe("Checkpoints", () => {
         return {} as SnapshotDb;
       });
       sinon.stub(CheckpointManager, "validateCheckpointGuids").callsFake(() => { });
+
       await SnapshotDb.openCheckpointV2({
         accessToken,
         iTwinId: testITwinId,

full-stack-tests/backend/src/integration/CloudSqlite.test.ts

@@ -163,6 +163,28 @@ describe("CloudSqlite", () => {
     db.close();
     container.disconnect({ detach: true });
   });
+  it("Should refresh write lock expiry time if withWriteLock called twice with same user", async () => {
+    const container = testContainers[0];
+    container.connect(caches[1]);
+    let writeLockExpiryTimeNoWriteLock = container.writeLockExpires; // Should be empty string when no write lock.
+    expect(writeLockExpiryTimeNoWriteLock).to.equal("");
+    await CloudSqlite.withWriteLock({user: "testuser", container}, async () => {
+      const firstWriteLockExpiryTime = Date.parse(container.writeLockExpires);
+      await BeDuration.wait(500); // sleep 500ms so we get a new write lock expiry time.
+      await CloudSqlite.withWriteLock({user: "testuser", container}, async () => {
+        const secondWriteLockExpiryTime = Date.parse(container.writeLockExpires);
+        expect(secondWriteLockExpiryTime).to.be.greaterThan(firstWriteLockExpiryTime);
+        // subtract 30 minutes and make sure its less than the first write lock expiry time.
+        // This tests that the secondWriteLockExpiryTime is a 'refresh' of the default expiry time of 1 hour.
+        // and not extending the expiry time already present by another hour.
+        // If it were extending the default expiry time of 1 hour, then second writelockexpirytime would be over 1 hour in the future
+        // and the below assert would fail.
+        expect(secondWriteLockExpiryTime - (30 * 60 * 1000)).to.be.lessThan(firstWriteLockExpiryTime);
+      });
+    });
+    writeLockExpiryTimeNoWriteLock = container.writeLockExpires; // Should be empty string when no write lock.
+    expect(writeLockExpiryTimeNoWriteLock).to.equal("");
+  });
 
   it("cloud containers", async () => {
     expect(undefined !== caches[0]);