Studio: Keep the site list from CLI events instead of asking the CLI each time

apps/studio/src/lib/site-list-cache.ts

@@ -0,0 +1,97 @@
+import { SITE_EVENTS, type SiteEvent, type SiteListItem } from '@studio/common/lib/cli-events';
+import { listSites } from '@studio/common/sites/list';
+import { executeCliCommand } from 'src/modules/cli/lib/execute-command';
+
+// Changes no event reports (e.g. a crashed daemon) show up within this long, the
+// interval the renderer polls the site list on.
+export const SITE_LIST_MAX_AGE_MS = 2 * 60 * 1000;
+
+/**
+ * The CLI's site list, kept current by the `_events` stream so that reading it doesn't start a CLI
+ * process each time. It's only used while the stream runs, and `drop()` discards it, so the next
+ * read asks the CLI again.
+ */
+export function createSiteListCache( fetchSites: () => Promise< SiteListItem[] >, now = Date.now ) {
+	let live = false;
+	let sites: Map< string, SiteListItem > | undefined;
+	let fetchedAt = 0;
+	// The fetch that will become the cache, with the events that arrive while it runs.
+	let pending: { promise: Promise< SiteListItem[] >; events: SiteEvent[] } | undefined;
+
+	function drop() {
+		sites = undefined;
+		pending = undefined;
+	}
+
+	// Returns false when the event doesn't carry what's needed to apply it.
+	function apply( list: Map< string, SiteListItem >, event: SiteEvent ): boolean {
+		if ( event.event === SITE_EVENTS.DELETED ) {
+			list.delete( event.siteId );
+			return true;
+		}
+		if ( ! event.site ) {
+			return false;
+		}
+		// The CLI reports `running` the way `site list` does.
+		list.set( event.siteId, { ...event.site, running: event.running } );
+		return true;
+	}
+
+	function fetchAndKeep(): Promise< SiteListItem[] > {
+		const startedAt = now();
+		const fetch = {
+			// With the events that arrived meanwhile, which a reader that joined the fetch after an
+			// event expects to see.
+			promise: fetchSites().then( ( list ) => {
+				const kept = new Map( list.map( ( site ) => [ site.id, site ] ) );
+				if ( ! fetch.events.every( ( event ) => apply( kept, event ) ) ) {
+					return list;
+				}
+				if ( pending === fetch && live ) {
+					pending = undefined;
+					sites = kept;
+					fetchedAt = startedAt;
+				}
+				return [ ...kept.values() ];
+			} ),
+			events: [] as SiteEvent[],
+		};
+		fetch.promise.catch( () => {
+			if ( pending === fetch ) {
+				pending = undefined;
+			}
+		} );
+		pending = fetch;
+		return fetch.promise;
+	}
+
+	return {
+		/** Whether the `_events` stream is running, so that its events keep the cache current. */
+		setLive( value: boolean ) {
+			live = value;
+			drop();
+		},
+
+		drop,
+
+		applyEvent( event: SiteEvent ) {
+			pending?.events.push( event );
+			if ( sites && ! apply( sites, event ) ) {
+				drop();
+			}
+		},
+
+		read(): Promise< SiteListItem[] > {
+			if ( ! live ) {
+				return fetchSites();
+			}
+			if ( sites && now() - fetchedAt < SITE_LIST_MAX_AGE_MS ) {
+				return Promise.resolve( [ ...sites.values() ] );
+			}
+			return pending?.promise ?? fetchAndKeep();
+		},
+	};
+}
+
+// Kept current by the CLI events subscriber.
+export const siteListCache = createSiteListCache( () => listSites( executeCliCommand ) );

apps/studio/src/lib/sites.ts

@@ -1,15 +1,16 @@
 import fs from 'fs';
 import * as Sentry from '@sentry/electron/main';
 import {
-	listSitesWithMetadata,
 	updateSiteMetadata,
+	withSiteMetadata,
 	type SiteMetadata,
 } from '@studio/common/sites/metadata';
 import {
 	WP_CLI_DEFAULT_RESPONSE_TIMEOUT,
 	WP_CLI_IMPORT_EXPORT_RESPONSE_TIMEOUT,
 } from 'src/constants';
 import { isInErrorRecovery, stopErrorRecovery } from 'src/lib/php-error-recovery';
+import { siteListCache } from 'src/lib/site-list-cache';
 import { executeCliCommand } from 'src/modules/cli/lib/execute-command';
 import { getSiteThumbnailPath } from 'src/storage/paths';
 import type { SiteListItem } from '@studio/common/lib/cli-events';
@@ -22,7 +23,7 @@ export type Site = SiteListItem & SiteMetadata;
 
 /** The user's sites as the CLI reports them, with Studio's own metadata merged in. */
 export async function getSites(): Promise< Site[] > {
-	const sites = await listSitesWithMetadata( executeCliCommand );
+	const sites = await withSiteMetadata( siteListCache.read() );
 	// A site serving a PHP-error page counts as running even though the CLI reports it stopped.
 	return sites.map( ( site ) =>
 		isInErrorRecovery( site.id )

apps/studio/src/lib/tests/site-list-cache.test.ts

@@ -0,0 +1,129 @@
+import { SITE_EVENTS, type SiteEvent, type SiteListItem } from '@studio/common/lib/cli-events';
+import { vi } from 'vitest';
+import { createSiteListCache, SITE_LIST_MAX_AGE_MS } from 'src/lib/site-list-cache';
+
+vi.mock( 'src/modules/cli/lib/execute-command' );
+
+function site( id: string, running = false ): SiteListItem {
+	return {
+		id,
+		name: id,
+		path: `/sites/${ id }`,
+		port: 8881,
+		url: 'http://localhost:8881',
+		phpVersion: '8.3',
+		running,
+	};
+}
+
+function event( type: SITE_EVENTS, siteId: string, running = false, name = siteId ): SiteEvent {
+	const { running: _, ...details } = site( siteId );
+	return { event: type, siteId, running, site: { ...details, name } };
+}
+
+function setup( initial: SiteListItem[] = [ site( 'a' ) ] ) {
+	let clock = 0;
+	const fetchSites = vi.fn( async () => initial );
+	const cache = createSiteListCache( fetchSites, () => clock );
+	cache.setLive( true );
+	return { cache, fetchSites, advance: ( ms: number ) => ( clock += ms ) };
+}
+
+describe( 'site list cache', () => {
+	it( 'asks the CLI once while the events stream runs', async () => {
+		const { cache, fetchSites } = setup();
+		await cache.read();
+		await cache.read();
+		expect( fetchSites ).toHaveBeenCalledTimes( 1 );
+	} );
+
+	it( 'asks the CLI every time without the events stream', async () => {
+		const { cache, fetchSites } = setup();
+		cache.setLive( false );
+		await cache.read();
+		await cache.read();
+		expect( fetchSites ).toHaveBeenCalledTimes( 2 );
+	} );
+
+	it( 'shares one CLI call between reads that overlap', async () => {
+		const { cache, fetchSites } = setup();
+		await Promise.all( [ cache.read(), cache.read() ] );
+		expect( fetchSites ).toHaveBeenCalledTimes( 1 );
+	} );
+
+	it( 'applies site events', async () => {
+		const { cache, fetchSites } = setup();
+		await cache.read();
+		cache.applyEvent( event( SITE_EVENTS.UPDATED, 'a', true, 'renamed' ) );
+		cache.applyEvent( event( SITE_EVENTS.CREATED, 'b' ) );
+		expect( await cache.read() ).toEqual( [
+			{ ...site( 'a', true ), name: 'renamed' },
+			site( 'b' ),
+		] );
+
+		cache.applyEvent( { event: SITE_EVENTS.DELETED, siteId: 'a', running: false } );
+		expect( await cache.read() ).toEqual( [ site( 'b' ) ] );
+		expect( fetchSites ).toHaveBeenCalledTimes( 1 );
+	} );
+
+	it( 'applies operation events to a site it has yet to list', async () => {
+		const { cache, fetchSites } = setup();
+		await cache.read();
+		cache.applyEvent( event( SITE_EVENTS.OPERATIONS_CHANGED, 'b' ) );
+		expect( await cache.read() ).toEqual( [ site( 'a' ), site( 'b' ) ] );
+		expect( fetchSites ).toHaveBeenCalledTimes( 1 );
+	} );
+
+	it( 'applies the events that arrive while the CLI is answering', async () => {
+		const { cache, fetchSites } = setup();
+		const reading = cache.read();
+		cache.applyEvent( event( SITE_EVENTS.UPDATED, 'a', true ) );
+		// A reader that heard of the event joins the answer that predates it.
+		const joining = cache.read();
+		expect( await reading ).toEqual( [ site( 'a', true ) ] );
+		expect( await joining ).toEqual( [ site( 'a', true ) ] );
+		expect( await cache.read() ).toEqual( [ site( 'a', true ) ] );
+		expect( fetchSites ).toHaveBeenCalledTimes( 1 );
+	} );
+
+	it( 'asks the CLI again after an event it cannot apply', async () => {
+		const { cache, fetchSites } = setup();
+		await cache.read();
+		cache.applyEvent( { event: SITE_EVENTS.UPDATED, siteId: 'a', running: true } );
+		await cache.read();
+		expect( fetchSites ).toHaveBeenCalledTimes( 2 );
+	} );
+
+	it( 'asks the CLI again once dropped', async () => {
+		const { cache, fetchSites } = setup();
+		await cache.read();
+		cache.drop();
+		await cache.read();
+		expect( fetchSites ).toHaveBeenCalledTimes( 2 );
+	} );
+
+	it( 'keeps no answer that was dropped while the CLI was answering', async () => {
+		const { cache, fetchSites } = setup();
+		const reading = cache.read();
+		cache.drop();
+		await reading;
+		await cache.read();
+		expect( fetchSites ).toHaveBeenCalledTimes( 2 );
+	} );
+
+	it( 'asks the CLI again once the list is old', async () => {
+		const { cache, fetchSites, advance } = setup();
+		await cache.read();
+		advance( SITE_LIST_MAX_AGE_MS );
+		await cache.read();
+		expect( fetchSites ).toHaveBeenCalledTimes( 2 );
+	} );
+
+	it( 'asks the CLI again after a failed answer', async () => {
+		const { cache, fetchSites } = setup();
+		fetchSites.mockRejectedValueOnce( new Error( 'CLI failed' ) );
+		await expect( cache.read() ).rejects.toThrow( 'CLI failed' );
+		await cache.read();
+		expect( fetchSites ).toHaveBeenCalledTimes( 2 );
+	} );
+} );

apps/studio/src/modules/cli/lib/cli-events-subscriber.ts

@@ -12,11 +12,12 @@ import { sendIpcEventToRenderer } from 'src/ipc-utils';
 import { trackSyncActivity } from 'src/lib/active-sync-operations';
 import { captureSiteThumbnail } from 'src/lib/capture-site-thumbnail';
 import { isInErrorRecovery } from 'src/lib/php-error-recovery';
+import { siteListCache } from 'src/lib/site-list-cache';
 import { forgetSite, getSites } from 'src/lib/sites';
-import { executeCliCommand } from 'src/modules/cli/lib/execute-command';
+import { executeCliCommand, onCliCommandFinished } from 'src/modules/cli/lib/execute-command';
 
 // The last running state of each site, so a site's thumbnail is captured when it starts only.
-// Seeded from the site list when the subscriber starts; a site missing from it is new, so stopped.
+// Seeded from the site list once the events stream listens; a site missing from it is new, so stopped.
 const lastRunning = new Map< string, boolean >();
 let seeded: Promise< void > = Promise.resolve();
 
@@ -54,18 +55,27 @@ const handleSiteEvent = sequential( async ( event: SiteEvent ): Promise< void >
 } );
 
 let subscriber: ReturnType< typeof executeCliCommand > | null = null;
+let watchingCommands = false;
+
+// A command's changes can reach the events stream after it exits, so a site list read after a
+// command asks the CLI again.
+function dropSiteListAfterCommands() {
+	if ( watchingCommands ) {
+		return;
+	}
+	watchingCommands = true;
+	onCliCommandFinished( ( [ command, subcommand ] ) => {
+		if ( command !== '_events' && ! ( command === 'site' && subcommand === 'list' ) ) {
+			siteListCache.drop();
+		}
+	} );
+}
 
 export async function startCliEventsSubscriber(): Promise< void > {
 	if ( subscriber ) {
 		return;
 	}
-	seeded = getSites()
-		.then( ( sites ) => {
-			for ( const site of sites ) {
-				lastRunning.set( site.id, site.running );
-			}
-		} )
-		.catch( () => undefined );
+	dropSiteListAfterCommands();
 
 	return new Promise( ( resolve, reject ) => {
 		if ( subscriber ) {
@@ -82,7 +92,21 @@ export async function startCliEventsSubscriber(): Promise< void > {
 			resolve();
 		} );
 
+		let live = false;
 		eventEmitter.on( 'data', ( { data } ) => {
+			// Its first message comes once it listens for changes, so its events from then on are complete.
+			if ( ! live ) {
+				live = true;
+				siteListCache.setLive( true );
+				seeded = getSites()
+					.then( ( sites ) => {
+						for ( const site of sites ) {
+							lastRunning.set( site.id, site.running );
+						}
+					} )
+					.catch( () => undefined );
+			}
+
 			const authParsed = cliAuthEventSchema.safeParse( data );
 			if ( authParsed.success ) {
 				const { event, token } = authParsed.data.value;
@@ -113,23 +137,28 @@ export async function startCliEventsSubscriber(): Promise< void > {
 			}
 
 			const siteEvent = parsed.data.value;
+			// Before the renderer hears of the event, so the site list it then reads includes it.
+			siteListCache.applyEvent( siteEvent );
 			void handleSiteEvent( siteEvent );
 		} );
 
 		eventEmitter.on( 'error', ( { error } ) => {
+			siteListCache.setLive( false );
 			reject( error );
 			console.error( 'CLI events subscriber error:', error );
 			subscriber = null;
 		} );
 
 		eventEmitter.on( 'failure', () => {
+			siteListCache.setLive( false );
 			console.warn( 'CLI events subscriber exited unexpectedly' );
 			subscriber = null;
 		} );
 	} );
 }
 
 export function stopCliEventsSubscriber(): void {
+	siteListCache.setLive( false );
 	if ( subscriber ) {
 		const [ eventEmitter, childProcess ] = subscriber;
 		eventEmitter.removeAllListeners();

apps/studio/src/modules/cli/lib/execute-command.ts

@@ -28,8 +28,20 @@ function getRunner(): CliRunner {
 	return runner;
 }
 
-export const executeCliCommand = ( ( args, options ) =>
-	getRunner().executeCliCommand( args, options ) ) as ExecuteCliCommand;
+const finishedListeners = new Set< ( args: string[] ) => void >();
+
+/** Call `listener` with each command's arguments when the command exits. */
+export function onCliCommandFinished( listener: ( args: string[] ) => void ): void {
+	finishedListeners.add( listener );
+}
+
+export const executeCliCommand = ( ( args, options ) => {
+	const command = getRunner().executeCliCommand( args, options );
+	const notify = () => finishedListeners.forEach( ( listener ) => listener( args ) );
+	command[ 0 ].on( 'success', notify );
+	command[ 0 ].on( 'failure', notify );
+	return command;
+} ) as ExecuteCliCommand;
 
 // Called on quit before the quit flow starts its own commands, which must not be stopped. Only asks,
 // so a sync past the point where stopping is safe finishes (see `exitOnCancel` in the CLI).