From 29c59e013f0cf25407430bd0d4752e4df8686ab5 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Wed, 8 Nov 2023 15:06:53 -0600 Subject: [PATCH] feat: reusable file locks outside of ConfigFile --- src/config/configFile.ts | 55 +++-------------------- src/exported.ts | 2 +- src/util/fileLocking.ts | 97 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 49 deletions(-) create mode 100644 src/util/fileLocking.ts diff --git a/src/config/configFile.ts b/src/config/configFile.ts index f727af2434..40042b1f04 100644 --- a/src/config/configFile.ts +++ b/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

(await fs.promises.readFile(this.getPath(), 'utf8'), this.getPath()); this.setContentsFromFileContents(obj, (await fs.promises.stat(this.getPath(), { bigint: true })).mtimeNs); } @@ -234,26 +234,10 @@ export class ConfigFile< * @param newContents The new contents of the file. */ public async write(): Promise

{ - // 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) | 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

(await fs.promises.readFile(this.getPath(), 'utf8'), this.getPath()); this.logAndMergeContents(fileTimestamp, fileContents); @@ -261,15 +245,7 @@ export class ConfigFile< 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

(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(); } diff --git a/src/exported.ts b/src/exported.ts index 3ba065e927..c067a98232 100644 --- a/src/exported.ts +++ b/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, diff --git a/src/util/fileLocking.ts b/src/util/fileLocking.ts new file mode 100644 index 0000000000..0a6efb6277 --- /dev/null +++ b/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; unlock: () => Promise }; +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 => { + // 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 => { + 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, + }; +};