Merge pull request #981 from forcedotcom/sm/reusable-file-locks

feat: reusable file locks outside of ConfigFile

src/config/configFile.ts

@@ -8,14 +8,13 @@
 import * as fs from 'fs';
 import { constants as fsConstants, Stats as fsStats } from 'fs';
 import { homedir as osHomedir } from 'os';
-import { basename, dirname as pathDirname, join as pathJoin } from 'path';
-import { lock, lockSync } from 'proper-lockfile';
+import { join as pathJoin } from 'path';
 import { parseJsonMap } from '@salesforce/kit';
 import { Global } from '../global';
 import { Logger } from '../logger/logger';
 import { SfError } from '../sfError';
 import { resolveProjectPath, resolveProjectPathSync } from '../util/internal';
-import { lockOptions, lockRetryOptions } from '../util/lockRetryOptions';
+import { lockInit, lockInitSync } from '../util/fileLocking';
 import { BaseConfigStore } from './configStore';
 import { ConfigContents } from './configStackTypes';
 import { stateFromContents } from './lwwMap';
@@ -168,6 +167,7 @@ export class ConfigFile<
             !this.hasRead ? 'hasRead is false' : 'force parameter is true'
           }`
         );
+
         const obj = parseJsonMap<P>(await fs.promises.readFile(this.getPath(), 'utf8'), this.getPath());
         this.setContentsFromFileContents(obj, (await fs.promises.stat(this.getPath(), { bigint: true })).mtimeNs);
       }
@@ -234,42 +234,18 @@ export class ConfigFile<
    * @param newContents The new contents of the file.
    */
   public async write(): Promise<P> {
-    // make sure we can write to the directory
-    try {
-      await fs.promises.mkdir(pathDirname(this.getPath()), { recursive: true });
-    } catch (err) {
-      throw SfError.wrap(err as Error);
-    }
+    const lockResponse = await lockInit(this.getPath());
 
-    let unlockFn: (() => Promise<void>) | undefined;
     // lock the file.  Returns an unlock function to call when done.
     try {
-      if (await this.exists()) {
-        unlockFn = await lock(this.getPath(), lockRetryOptions);
-      } else {
-        // lock the entire directory to keep others from trying to create the file while we are
-        unlockFn = await lock(basename(this.getPath()), lockRetryOptions);
-        this.logger.debug(
-          `No file found at ${this.getPath()}.  Write will create it.  Locking the entire directory until file is written.`
-        );
-      }
-
       const fileTimestamp = (await fs.promises.stat(this.getPath(), { bigint: true })).mtimeNs;
       const fileContents = parseJsonMap<P>(await fs.promises.readFile(this.getPath(), 'utf8'), this.getPath());
       this.logAndMergeContents(fileTimestamp, fileContents);
     } catch (err) {
       this.handleWriteError(err);
     }
     // write the merged LWWMap to file
-    this.logger.debug(`Writing to config file: ${this.getPath()}`);
-    try {
-      await fs.promises.writeFile(this.getPath(), JSON.stringify(this.getContents(), null, 2));
-    } finally {
-      // unlock the file
-      if (typeof unlockFn !== 'undefined') {
-        await unlockFn();
-      }
-    }
+    await lockResponse.writeAndUnlock(JSON.stringify(this.getContents(), null, 2));
 
     return this.getContents();
   }
@@ -281,16 +257,8 @@ export class ConfigFile<
    * @param newContents The new contents of the file.
    */
   public writeSync(): P {
+    const lockResponse = lockInitSync(this.getPath());
     try {
-      fs.mkdirSync(pathDirname(this.getPath()), { recursive: true });
-    } catch (err) {
-      throw SfError.wrap(err as Error);
-    }
-
-    let unlockFn: (() => void) | undefined;
-    try {
-      // lock the file.  Returns an unlock function to call when done.
-      unlockFn = lockSync(this.getPath(), lockOptions);
       // get the file modstamp.  Do this after the lock acquisition in case the file is being written to.
       const fileTimestamp = fs.statSync(this.getPath(), { bigint: true }).mtimeNs;
       const fileContents = parseJsonMap<P>(fs.readFileSync(this.getPath(), 'utf8'), this.getPath());
@@ -300,16 +268,7 @@ export class ConfigFile<
     }
 
     // write the merged LWWMap to file
-
-    this.logger.trace(`Writing to config file: ${this.getPath()}`);
-    try {
-      fs.writeFileSync(this.getPath(), JSON.stringify(this.toObject(), null, 2));
-    } finally {
-      if (typeof unlockFn !== 'undefined') {
-        // unlock the file
-        unlockFn();
-      }
-    }
+    lockResponse.writeAndUnlock(JSON.stringify(this.getContents(), null, 2));
 
     return this.getContents();
   }

src/exported.ts

@@ -89,7 +89,7 @@ export { MyDomainResolver } from './status/myDomainResolver';
 export { DefaultUserFields, REQUIRED_FIELDS, User, UserFields } from './org/user';
 
 export { PermissionSetAssignment, PermissionSetAssignmentFields } from './org/permissionSetAssignment';
-
+export { lockInit } from './util/fileLocking';
 export {
   ScratchOrgCreateOptions,
   ScratchOrgCreateResult,

src/util/fileLocking.ts

@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2023, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+import * as fs from 'node:fs';
+import { dirname } from 'node:path';
+import { lock, lockSync } from 'proper-lockfile';
+import { SfError } from '../sfError';
+import { Logger } from '../logger/logger';
+import { lockRetryOptions } from './lockRetryOptions';
+
+type LockInitResponse = { writeAndUnlock: (data: string) => Promise<void>; unlock: () => Promise<void> };
+type LockInitSyncResponse = { writeAndUnlock: (data: string) => void; unlock: () => void };
+
+/**
+ *
+ *This method exists as a separate function so it can be used by ConfigFile OR outside of ConfigFile.
+ *
+ * @param filePath where to save the file
+ * @returns 2 functions:
+ * - writeAndUnlock: a function that takes the data to write and writes it to the file, then unlocks the file whether write succeeded or not
+ * - unlock: a function that unlocks the file (in case you don't end up calling writeAndUnlock)
+ */
+export const lockInit = async (filePath: string): Promise<LockInitResponse> => {
+  // make sure we can write to the directory
+  try {
+    await fs.promises.mkdir(dirname(filePath), { recursive: true });
+  } catch (err) {
+    throw SfError.wrap(err as Error);
+  }
+
+  const [unlock] = await Promise.all(
+    fs.existsSync(filePath)
+      ? // if the file exists, wait for it to be unlocked
+        [lock(filePath, lockRetryOptions)]
+      : // lock the entire directory to keep others from trying to create the file while we are
+        [
+          lock(dirname(filePath), lockRetryOptions),
+          (
+            await Logger.child('fileLocking.lockInit')
+          ).debug(
+            `No file found at ${filePath}.  Write will create it.  Locking the entire directory until file is written.`
+          ),
+        ]
+  );
+
+  return {
+    writeAndUnlock: async (data: string): Promise<void> => {
+      const logger = await Logger.child('fileLocking.writeAndUnlock');
+      logger.debug(`Writing to file: ${filePath}`);
+      try {
+        await fs.promises.writeFile(filePath, data);
+      } finally {
+        await unlock();
+      }
+    },
+    unlock,
+  };
+};
+
+/**
+ * prefer async {@link lockInit}.
+ * See its documentation for details.
+ */
+export const lockInitSync = (filePath: string): LockInitSyncResponse => {
+  // make sure we can write to the directory
+  try {
+    fs.mkdirSync(dirname(filePath), { recursive: true });
+  } catch (err) {
+    throw SfError.wrap(err as Error);
+  }
+
+  const [unlock] = fs.existsSync(filePath)
+    ? // if the file exists, wait for it to be unlocked
+      [lockSync(filePath, lockRetryOptions)]
+    : // lock the entire directory to keep others from trying to create the file while we are
+      [
+        lockSync(dirname(filePath), lockRetryOptions),
+        Logger.childFromRoot('fileLocking.lockInit').debug(
+          `No file found at ${filePath}.  Write will create it.  Locking the entire directory until file is written.`
+        ),
+      ];
+  return {
+    writeAndUnlock: (data: string): void => {
+      const logger = Logger.childFromRoot('fileLocking.writeAndUnlock');
+      logger.debug(`Writing to file: ${filePath}`);
+      try {
+        fs.writeFileSync(filePath, data);
+      } finally {
+        unlock();
+      }
+    },
+    unlock,
+  };
+};