Skip to content

Commit

Permalink
chore: using simple objects in rpc api (to avoid snapshots)
Browse files Browse the repository at this point in the history
Moving to svelte 5, state objects are proxied. Normally that's an implementation detail,
but when you try to pass the proxies through rpc they can't be cloned and result
in errors.

We could use $state.snapshot(build) every time we pass objects from the UI to the API,
but we really shouldn't be passing whole objects to the backend and blindly trusting
that the objects (including all properties) match what the backend knows anyway.

This changes the three affected API functions to accept ids instead of objects, and the
backend looks up the objects before acting. Although it's slightly more work on the
backend, this is a better/more robust API, and also avoids the svelte 5 proxy issue.

Fixes podman-desktop#1304.

Signed-off-by: Tim deBoer <[email protected]>
  • Loading branch information
deboer-tim committed Feb 10, 2025
1 parent 0b8d634 commit b6f3e66
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 12 deletions.
27 changes: 21 additions & 6 deletions packages/backend/src/api-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ export class BootcApiImpl implements BootcApi {
return checkPrereqs(await getContainerEngine());
}

async checkVMLaunchPrereqs(build: BootcBuildInfo): Promise<string | undefined> {
async checkVMLaunchPrereqs(buildId: string): Promise<string | undefined> {
const build = this.history.getHistory().find(build => build.id === buildId);
if (!build) {
return Promise.reject(new Error(`Could not find build: ${buildId}`));
}
return createVMManager(build).checkVMLaunchPrereqs();
}

Expand All @@ -66,11 +70,16 @@ export class BootcApiImpl implements BootcApi {
return buildDiskImage(build, this.history, overwrite);
}

async launchVM(build: BootcBuildInfo): Promise<void> {
async launchVM(buildId: string): Promise<void> {
try {
await createVMManager(build).launchVM();
// Notify it has successfully launched
await this.notify(Messages.MSG_VM_LAUNCH_ERROR, { success: 'Launched!', error: '' });
const build = this.history.getHistory().find(build => build.id === buildId);
if (!build) {
await this.notify(Messages.MSG_VM_LAUNCH_ERROR, { success: '', error: 'Could not find build to launch' });
} else {
await createVMManager(build).launchVM();
// Notify it has successfully launched
await this.notify(Messages.MSG_VM_LAUNCH_ERROR, { success: 'Launched!', error: '' });
}
} catch (e) {
// Make sure that we are able to display the "stderr" information if it exists as that actually shows
// the error when running the command.
Expand All @@ -90,13 +99,18 @@ export class BootcApiImpl implements BootcApi {
return stopCurrentVM();
}

async deleteBuilds(builds: BootcBuildInfo[]): Promise<void> {
async deleteBuilds(buildIds: string[]): Promise<void> {
const response = await podmanDesktopApi.window.showWarningMessage(
`Are you sure you want to remove the selected disk images from the build history? This will remove the history of the build as well as remove any lingering build containers.`,
'Yes',
'No',
);
if (response === 'Yes') {
// create an array of builds. invalid build ids are ignored
const builds = buildIds
.map(id => this.history.getHistory().find(build => build.id === id))
.filter(build => build) as BootcBuildInfo[];

// Map each build to a delete operation promise
const deletePromises = builds.map(build => this.deleteBuildContainer(build));

Expand Down Expand Up @@ -330,6 +344,7 @@ export class BootcApiImpl implements BootcApi {
// this method is internal and meant to be used by the API implementation
protected async notify(id: string, body: unknown = {}): Promise<void> {
// Must pass in an empty body, if it is undefined this fails
console.log('notify frontend: ' + id);
await this.webview.postMessage({
id,
body,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ test('Test clicking on delete button', async () => {
deleteButton.click();

expect(spyOnDelete).toHaveBeenCalled();
expect(spyOnDelete).toHaveBeenCalledWith(['name1']);
});

test('Test clicking on logs button', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ let isWindows = $state(false);
// Delete the build
async function deleteBuild(): Promise<void> {
await bootcClient.deleteBuilds([object]);
await bootcClient.deleteBuilds([object.id]);
}
// Navigate to the build
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ test('Render virtual machine terminal window', async () => {
await waitFor(() => {
expect(bootcClient.launchVM).toHaveBeenCalled();
});
expect(bootcClient.checkVMLaunchPrereqs).toHaveBeenCalledWith('id1');
expect(bootcClient.launchVM).toHaveBeenCalledWith('id1');
});

test('Show prereqs message if prereq check fails (returns ANY string)', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ async function launchVM(build: BootcBuildInfo): Promise<void> {
// This is launched IN THE BACKGROUND. We do not wait for the VM to boot before showing the terminal.
// we instead are notified by subscribing to Messages.MSG_VM_LAUNCH_ERROR messages from RPC
bootcClient.launchVM(build).catch((e: unknown) => console.error('error launching VM', e));
bootcClient.launchVM(build.id).catch((e: unknown) => console.error('error launching VM', e));
// Initialize the terminal so it awaits the websocket connection.
await initTerminal();
Expand Down Expand Up @@ -261,7 +261,7 @@ onMount(async () => {
if (build) {
// Launch the VM if we pass all the prerequisites, otherwise we will show the empty screen with content / information checks.
vmLaunchPrereqs = await bootcClient.checkVMLaunchPrereqs(build);
vmLaunchPrereqs = await bootcClient.checkVMLaunchPrereqs(build.id);
if (!vmLaunchPrereqs) {
await launchVM(build);
Expand Down
6 changes: 3 additions & 3 deletions packages/shared/src/BootcAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ import type { ExamplesList } from './models/examples';
export abstract class BootcApi {
static readonly CHANNEL: string = 'BootcApi';
abstract checkPrereqs(): Promise<string | undefined>;
abstract checkVMLaunchPrereqs(build: BootcBuildInfo): Promise<string | undefined>;
abstract launchVM(build: BootcBuildInfo): Promise<void>;
abstract checkVMLaunchPrereqs(buildId: string): Promise<string | undefined>;
abstract launchVM(buildId: string): Promise<void>;
abstract buildExists(folder: string, types: BuildType[]): Promise<boolean>;
abstract buildImage(build: BootcBuildInfo, overwrite?: boolean): Promise<void>;
abstract pullImage(image: string, arch?: string): Promise<void>;
abstract inspectImage(image: ImageInfo): Promise<ImageInspectInfo>;
abstract inspectManifest(image: ImageInfo): Promise<ManifestInspectInfo>;
abstract deleteBuilds(builds: BootcBuildInfo[]): Promise<void>;
abstract deleteBuilds(buildIds: string[]): Promise<void>;
abstract selectOutputFolder(): Promise<string>;
abstract selectBuildConfigFile(): Promise<string>;
abstract listBootcImages(): Promise<ImageInfo[]>;
Expand Down

0 comments on commit b6f3e66

Please sign in to comment.