ESLint rule for user data lockfile

.eslintrc.json

@@ -20,7 +20,7 @@
 		"projectService": true,
 		"tsconfigRootDir": "./"
 	},
-	"plugins": [ "@typescript-eslint", "prettier", "jest-dom" ],
+	"plugins": [ "@typescript-eslint", "prettier", "jest-dom", "@studio" ],
 	"settings": {
 		"import/resolver": {
 			"typescript": {
@@ -52,7 +52,8 @@
 				},
 				"groups": [ "builtin", "external", "internal", "parent", "sibling", "index", "type" ]
 			}
-		]
+		],
+		"@studio/require-lock-before-save": "error"
 	},
 	"overrides": [
 		{

cli/commands/preview/tests/create.test.ts

@@ -8,6 +8,7 @@ import { validateSiteFolder } from 'cli/lib/validation';
 import { Logger, LoggerError } from 'cli/logger';
 
 jest.mock( 'cli/lib/appdata', () => ( {
+	...jest.requireActual( 'cli/lib/appdata' ),
 	getAppdataDirectory: jest.fn().mockReturnValue( '/test/appdata' ),
 	getAuthToken: jest.fn(),
 } ) );

cli/commands/preview/tests/delete.test.ts

@@ -4,6 +4,7 @@ import { getSnapshotsFromAppdata, deleteSnapshotFromAppdata } from 'cli/lib/snap
 import { Logger, LoggerError } from 'cli/logger';
 
 jest.mock( 'cli/lib/appdata', () => ( {
+	...jest.requireActual( 'cli/lib/appdata' ),
 	getAppdataDirectory: jest.fn().mockReturnValue( '/test/appdata' ),
 	getAuthToken: jest.fn(),
 } ) );

cli/commands/preview/tests/list.test.ts

@@ -4,6 +4,7 @@ import { validateSiteFolder } from 'cli/lib/validation';
 import { Logger } from 'cli/logger';
 
 jest.mock( 'cli/lib/appdata', () => ( {
+	...jest.requireActual( 'cli/lib/appdata' ),
 	getAppdataDirectory: jest.fn().mockReturnValue( '/test/appdata' ),
 	getAuthToken: jest.fn(),
 } ) );

cli/commands/preview/tests/update.test.ts

@@ -9,6 +9,7 @@ import { validateSiteFolder } from 'cli/lib/validation';
 import { Logger, LoggerError } from 'cli/logger';
 
 jest.mock( 'cli/lib/appdata', () => ( {
+	...jest.requireActual( 'cli/lib/appdata' ),
 	getAppdataDirectory: jest.fn().mockReturnValue( '/test/appdata' ),
 	getAuthToken: jest.fn(),
 	getSiteByFolder: jest.fn(),

cli/lib/appdata.ts

@@ -4,7 +4,9 @@ import os from 'os';
 import path from 'path';
 import { __, sprintf } from '@wordpress/i18n';
 import { readFile, writeFile } from 'atomically';
+import { LOCKFILE_NAME } from 'common/constants';
 import { arePathsEqual } from 'common/lib/fs-utils';
+import { lockFileAsync, unlockFileAsync } from 'common/lib/lockfile';
 import { getAuthenticationUrl } from 'common/lib/oauth';
 import { snapshotSchema } from 'common/types/snapshot';
 import { StatsGroup, StatsMetric } from 'common/types/stats';
@@ -43,6 +45,7 @@ const userDataSchema = z
 	.passthrough();
 
 type UserData = z.infer< typeof userDataSchema >;
+type SiteData = z.infer< typeof siteSchema >;
 
 export function getAppdataDirectory(): string {
 	if ( process.platform === 'win32' ) {
@@ -99,13 +102,12 @@ export async function readAppdata(): Promise< UserData > {
 }
 
 export async function saveAppdata( userData: UserData ): Promise< void > {
-	const appDataPath = getAppdataPath();
-
 	try {
 		if ( ! userData.version ) {
 			userData.version = 1;
 		}
 
+		const appDataPath = getAppdataPath();
 		const fileContent = JSON.stringify( userData, null, 2 ) + '\n';
 
 		await writeFile( appDataPath, fileContent, { encoding: 'utf8' } );
@@ -114,6 +116,16 @@ export async function saveAppdata( userData: UserData ): Promise< void > {
 	}
 }
 
+const LOCKFILE_PATH = path.join( getAppdataDirectory(), LOCKFILE_NAME );
+
+export async function lockAppdata(): Promise< void > {
+	await lockFileAsync( LOCKFILE_PATH, { wait: 1000, stale: 1000 } );
+}
+
+export async function unlockAppdata(): Promise< void > {
+	await unlockFileAsync( LOCKFILE_PATH );
+}
+
 export async function getAuthToken(): Promise< NonNullable< UserData[ 'authToken' ] > > {
 	try {
 		const { authToken } = await readAppdata();
@@ -138,9 +150,7 @@ export async function getAuthToken(): Promise< NonNullable< UserData[ 'authToken
 	}
 }
 
-export async function getSiteByFolder(
-	siteFolder: string
-): Promise< z.infer< typeof siteSchema > > {
+export async function getSiteByFolder( siteFolder: string ): Promise< SiteData > {
 	const userData = await readAppdata();
 	const site = [ ...userData.sites, ...userData.newSites ].find( ( site ) =>
 		arePathsEqual( site.path, siteFolder )
@@ -153,7 +163,7 @@ export async function getSiteByFolder(
 	return site;
 }
 
-export function getNewSitePartial( siteFolder: string ): z.infer< typeof siteSchema > {
+export function getNewSitePartial( siteFolder: string ): SiteData {
 	const newSite = {
 		id: crypto.randomUUID(),
 		path: siteFolder,
@@ -163,20 +173,29 @@ export function getNewSitePartial( siteFolder: string ): z.infer< typeof siteSch
 	return newSite;
 }
 
-export async function getOrCreateSiteByFolder(
+const createNewSite = async ( siteFolder: string ): Promise< z.infer< typeof siteSchema > > => {
+	try {
+		await lockAppdata();
+		const userData = await readAppdata();
+		const site = getNewSitePartial( siteFolder );
+		userData.newSites.push( site );
+		await saveAppdata( userData );
+		return site;
+	} finally {
+		await unlockAppdata();
+	}
+};
+
+export const getOrCreateSiteByFolder = async (
 	siteFolder: string
-): Promise< z.infer< typeof siteSchema > > {
-	let site;
+): Promise< z.infer< typeof siteSchema > > => {
 	try {
-		site = await getSiteByFolder( siteFolder );
+		const site = await getSiteByFolder( siteFolder );
+		return site;
 	} catch ( error ) {
 		if ( ! ( error instanceof LoggerError ) ) {
 			throw error;
 		}
-		const userData = await readAppdata();
-		site = getNewSitePartial( siteFolder );
-		userData.newSites.push( site );
-		await saveAppdata( userData );
+		return createNewSite( siteFolder );
 	}
-	return site;
-}
+};

cli/lib/snapshots.ts

@@ -1,5 +1,3 @@
-import path from 'node:path';
-import { promisify } from 'node:util';
 import { __, sprintf } from '@wordpress/i18n';
 // eslint-disable-next-line import/no-named-as-default
 import Table from 'cli-table3';
@@ -13,47 +11,17 @@ import {
 	formatDuration,
 	intervalToDuration,
 } from 'date-fns';
-import lockfile from 'lockfile';
 import {
-	getAppdataDirectory,
 	getAuthToken,
 	getSiteByFolder,
 	readAppdata,
-	saveAppdata,
 	getOrCreateSiteByFolder,
+	lockAppdata,
+	unlockAppdata,
+	saveAppdata,
 } from 'cli/lib/appdata';
 import { LoggerError } from 'cli/logger';
 
-const UPDATE_SNAPSHOTS_LOCKFILE_PATH = path.join(
-	getAppdataDirectory(),
-	'studio-update-snapshots.lock'
-);
-
-function lock( path: string, options: lockfile.Options ) {
-	return new Promise< void >( ( resolve, reject ) => {
-		lockfile.lock( path, options, ( err ) => {
-			if ( err ) {
-				reject( err );
-			} else {
-				resolve();
-			}
-		} );
-	} );
-}
-
-const unlock = promisify( lockfile.unlock );
-
-function withLock< Args extends unknown[], Return >( fn: ( ...args: Args ) => Promise< Return > ) {
-	return async ( ...args: Args ) => {
-		try {
-			await lock( UPDATE_SNAPSHOTS_LOCKFILE_PATH, { wait: 1000 } );
-			return await fn( ...args );
-		} finally {
-			await unlock( UPDATE_SNAPSHOTS_LOCKFILE_PATH );
-		}
-	};
-}
-
 export async function getSnapshotsFromAppdata(
 	userId: number,
 	siteFolder?: string
@@ -74,18 +42,23 @@ export async function updateSnapshotInAppdata(
 	atomicSiteId: number,
 	siteFolder: string
 ): Promise< Snapshot > {
-	const site = await getOrCreateSiteByFolder( siteFolder );
-	const userData = await readAppdata();
-	const snapshot = userData.snapshots.find( ( s ) => s.atomicSiteId === atomicSiteId );
-	if ( ! snapshot ) {
-		throw new LoggerError( __( 'Failed to find existing preview site in appdata' ) );
-	}
+	try {
+		const site = await getOrCreateSiteByFolder( siteFolder );
+		await lockAppdata();
+		const userData = await readAppdata();
+		const snapshot = userData.snapshots.find( ( s ) => s.atomicSiteId === atomicSiteId );
+		if ( ! snapshot ) {
+			throw new LoggerError( __( 'Failed to find existing preview site in appdata' ) );
+		}
 
-	snapshot.localSiteId = site.id;
-	snapshot.date = Date.now();
-	await saveAppdata( userData );
+		snapshot.localSiteId = site.id;
+		snapshot.date = Date.now();
 
-	return snapshot;
+		await saveAppdata( userData );
+		return snapshot;
+	} finally {
+		await unlockAppdata();
+	}
 }
 
 const getNextSequenceNumber = ( siteId: string, snapshots: Snapshot[], userId: number ): number => {
@@ -107,40 +80,50 @@ export async function saveSnapshotToAppdata(
 	atomicSiteId: number,
 	previewUrl: string
 ): Promise< Snapshot > {
-	const site = await getOrCreateSiteByFolder( siteFolder );
-	const userData = await readAppdata();
-	const authToken = await getAuthToken();
-
-	const nextSequenceNumber = getNextSequenceNumber( site.id, userData.snapshots, authToken.id );
-	const snapshot: Snapshot = {
-		url: previewUrl,
-		atomicSiteId,
-		localSiteId: site.id,
-		date: Date.now(),
-		name: sprintf(
-			/* translators: 1: Site name 2: Sequence number (e.g. "My Site Name Preview 1") */
-			__( '%1$s Preview %2$d' ),
-			site.name,
-			nextSequenceNumber
-		),
-		sequence: nextSequenceNumber,
-		userId: authToken.id,
-	};
-
-	userData.snapshots.push( snapshot );
-	await saveAppdata( userData );
-	return snapshot;
+	try {
+		const site = await getOrCreateSiteByFolder( siteFolder );
+		await lockAppdata();
+		const userData = await readAppdata();
+		const authToken = await getAuthToken();
+
+		const nextSequenceNumber = getNextSequenceNumber( site.id, userData.snapshots, authToken.id );
+		const snapshot: Snapshot = {
+			url: previewUrl,
+			atomicSiteId,
+			localSiteId: site.id,
+			date: Date.now(),
+			name: sprintf(
+				/* translators: 1: Site name 2: Sequence number (e.g. "My Site Name Preview 1") */
+				__( '%1$s Preview %2$d' ),
+				site.name,
+				nextSequenceNumber
+			),
+			sequence: nextSequenceNumber,
+			userId: authToken.id,
+		};
+
+		userData.snapshots.push( snapshot );
+		await saveAppdata( userData );
+		return snapshot;
+	} finally {
+		await unlockAppdata();
+	}
 }
 
-export const deleteSnapshotFromAppdata = withLock( async ( snapshotUrl: string ) => {
-	const userData = await readAppdata();
-	const snapshotIndex = userData.snapshots.findIndex( ( s ) => s.url === snapshotUrl );
-	if ( snapshotIndex === -1 ) {
-		return;
+export async function deleteSnapshotFromAppdata( snapshotUrl: string ) {
+	try {
+		await lockAppdata();
+		const userData = await readAppdata();
+		const snapshotIndex = userData.snapshots.findIndex( ( s ) => s.url === snapshotUrl );
+		if ( snapshotIndex === -1 ) {
+			return;
+		}
+		userData.snapshots.splice( snapshotIndex, 1 );
+		await saveAppdata( userData );
+	} finally {
+		await unlockAppdata();
 	}
-	userData.snapshots.splice( snapshotIndex, 1 );
-	await saveAppdata( userData );
-} );
+}
 
 export function isSnapshotExpired( snapshot: Snapshot ) {
 	const now = new Date();

cli/lib/stats.ts

@@ -1,7 +1,7 @@
 import { AggregateInterval, StatsGroup, StatsMetric } from 'common/types/stats';
 import { isSameDay, isSameMonth, isSameWeek } from 'date-fns';
 import fetch from 'node-fetch';
-import { readAppdata, saveAppdata } from './appdata';
+import { lockAppdata, readAppdata, saveAppdata, unlockAppdata } from 'cli/lib/appdata';
 
 // Bumps a stat if it hasn't been bumped within the current aggregate interval.
 // This allows us to approximate a 1-count-per-user stat without recording which
@@ -70,9 +70,14 @@ async function getLastBump( group: StatsGroup, stat: StatsMetric ): Promise< num
 
 // Store this moment as the last time we bumped the state, in UTC time.
 async function updateLastBump( group: StatsGroup, stat: StatsMetric ) {
-	const data = await readAppdata();
-	data.lastBumpStats ??= {};
-	data.lastBumpStats[ group ] ??= {};
-	( data.lastBumpStats[ group ] as Record< StatsMetric, number > )[ stat ] = Date.now();
-	await saveAppdata( data );
+	try {
+		await lockAppdata();
+		const userData = await readAppdata();
+		userData.lastBumpStats ??= {};
+		userData.lastBumpStats[ group ] ??= {};
+		( userData.lastBumpStats[ group ] as Record< StatsMetric, number > )[ stat ] = Date.now();
+		await saveAppdata( userData );
+	} finally {
+		await unlockAppdata();
+	}
 }