Show native save dialogs in desktop app #50, #264

This commit introduces native operating system file dialogs in the desktop application replacing the existing web-based dialogs. It lays the foundation for future enhancements such as: - Providing error messages when saving or executing files, addressing #264. - Creating system restore points, addressing #50. Documentation updates: - Update `desktop-vs-web-features.md` with added functionality. - Update `README.md` with security feature highlights. - Update home page documentation to emphasize security features. Other supporting changes include: - Integrate IPC communication channels for secure Electron dialog API interactions. - Refactor `IpcRegistration` for more type-safety and simplicity. - Introduce a Vue hook to encapsulate dialog functionality. - Improve errors during IPC registration for easier troubleshooting. - Move `ClientLoggerFactory` for consistency in hooks organization and remove `LoggerFactory` interface for simplicity. - Add tests for the save file dialog in the browser context. - Add `Blob` polyfill in tests to compensate for the missing `blob.text()` function in `jsdom` (see jsdom/jsdom#2555). Improve environment detection logic: - Treat test environment as browser environments to correctly activate features based on the environment. This resolves issues where the environment is misidentified as desktop, but Electron preloader APIs are missing. - Rename `isDesktop` environment identification variable to `isRunningAsDesktopApplication` for better clarity and to avoid confusion with desktop environments in web/browser/test environments. - Simplify `BrowserRuntimeEnvironment` to consistently detect non-desktop application environments. - Improve environment detection for Electron main process (electron/electron#2288).
2024-01-13 18:04:23 +01:00
parent da4be500da
commit c546a33eff
65 changed files with 1384 additions and 345 deletions
--- a/tests/unit/presentation/bootstrapping/DependencyProvider.spec.ts
+++ b/tests/unit/presentation/bootstrapping/DependencyProvider.spec.ts
@@ -19,6 +19,7 @@ describe('DependencyProvider', () => {
      useUserSelectionState: createTransientTests(),
      useLogger: createTransientTests(),
      useCodeRunner: createTransientTests(),
+      useDialog: createTransientTests(),
    };
    Object.entries(testCases).forEach(([key, runTests]) => {
      const registeredKey = InjectionKeys[key].key;
--- a/tests/unit/presentation/components/Scripts/Slider/UseDragHandler.spec.ts
+++ b/tests/unit/presentation/components/Scripts/Slider/UseDragHandler.spec.ts
@@ -245,7 +245,7 @@ function initializeDragHandlerWithMocks(mocks?: {
 }

 function createMockPointerEvent(...args: ConstructorArguments<typeof PointerEvent>): PointerEvent {
-  return new MouseEvent(...args) as PointerEvent; // JSDom does not support `PointerEvent` constructor, https://github.com/jsdom/jsdom/issues/2527
+  return new MouseEvent(...args) as PointerEvent; // jsdom does not support `PointerEvent` constructor, https://github.com/jsdom/jsdom/issues/2527
 }

 class DragDomModifierMock implements DragDomModifier {
--- a/tests/unit/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.spec.ts
+++ b/tests/unit/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory.spec.ts
@@ -0,0 +1,88 @@
+import { describe, it, expect } from 'vitest';
+import { determineDialogBasedOnEnvironment, WindowDialogCreationFunction, BrowserDialogCreationFunction } from '@/presentation/components/Shared/Hooks/Dialog/ClientDialogFactory';
+import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment';
+import { RuntimeEnvironmentStub } from '@tests/unit/shared/Stubs/RuntimeEnvironmentStub';
+import { DialogStub } from '@tests/unit/shared/Stubs/DialogStub';
+import { collectExceptionMessage } from '@tests/unit/shared/ExceptionCollector';
+
+describe('ClientDialogFactory', () => {
+  describe('determineDialogBasedOnEnvironment', () => {
+    describe('non-desktop environment', () => {
+      it('returns browser dialog', () => {
+        // arrange
+        const expectedDialog = new DialogStub();
+        const context = new DialogCreationTestSetup()
+          .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(false))
+          .withBrowserDialogFactory(() => expectedDialog);
+
+        // act
+        const actualDialog = context.createDialogForTest();
+
+        // assert
+        expect(expectedDialog).to.equal(actualDialog);
+      });
+    });
+    describe('desktop environment', () => {
+      it('returns window-injected dialog', () => {
+        // arrange
+        const expectedDialog = new DialogStub();
+        const context = new DialogCreationTestSetup()
+          .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(true))
+          .withWindowInjectedDialogFactory(() => expectedDialog);
+
+        // act
+        const actualDialog = context.createDialogForTest();
+
+        // assert
+        expect(expectedDialog).to.equal(actualDialog);
+      });
+      it('throws error when window-injected dialog is unavailable', () => {
+        // arrange
+        const expectedError = 'The Dialog API could not be retrieved from the window object.';
+        const context = new DialogCreationTestSetup()
+          .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(true))
+          .withWindowInjectedDialogFactory(() => undefined);
+
+        // act
+        const act = () => context.createDialogForTest();
+
+        // assert
+        const actualError = collectExceptionMessage(act);
+        expect(actualError).to.include(expectedError);
+      });
+    });
+  });
+});
+
+class DialogCreationTestSetup {
+  private environment: RuntimeEnvironment = new RuntimeEnvironmentStub();
+
+  private browserDialogFactory: BrowserDialogCreationFunction = () => new DialogStub();
+
+  private windowInjectedDialogFactory: WindowDialogCreationFunction = () => new DialogStub();
+
+  public withEnvironment(environment: RuntimeEnvironment): this {
+    this.environment = environment;
+    return this;
+  }
+
+  public withBrowserDialogFactory(browserDialogFactory: BrowserDialogCreationFunction): this {
+    this.browserDialogFactory = browserDialogFactory;
+    return this;
+  }
+
+  public withWindowInjectedDialogFactory(
+    windowInjectedDialogFactory: WindowDialogCreationFunction,
+  ): this {
+    this.windowInjectedDialogFactory = windowInjectedDialogFactory;
+    return this;
+  }
+
+  public createDialogForTest() {
+    return determineDialogBasedOnEnvironment(
+      this.environment,
+      this.windowInjectedDialogFactory,
+      this.browserDialogFactory,
+    );
+  }
+}
--- a/tests/unit/presentation/components/Shared/Hooks/Dialog/UseDialog.spec.ts
+++ b/tests/unit/presentation/components/Shared/Hooks/Dialog/UseDialog.spec.ts
@@ -0,0 +1,19 @@
+import { describe, it, expect } from 'vitest';
+import { DialogFactory, useDialog } from '@/presentation/components/Shared/Hooks/Dialog/UseDialog';
+import { DialogStub } from '@tests/unit/shared/Stubs/DialogStub';
+
+describe('UseDialog', () => {
+  describe('useDialog', () => {
+    it('returns provided dialog instance', () => {
+      // arrange
+      const expectedDialog = new DialogStub();
+      const factoryMock: DialogFactory = () => expectedDialog;
+
+      // act
+      const { dialog } = useDialog(factoryMock);
+
+      // assert
+      expect(dialog).to.equal(expectedDialog);
+    });
+  });
+});
--- a/tests/unit/presentation/components/Shared/Hooks/Log/ClientLoggerFactory.spec.ts
+++ b/tests/unit/presentation/components/Shared/Hooks/Log/ClientLoggerFactory.spec.ts
@@ -1,11 +1,11 @@
 // eslint-disable-next-line max-classes-per-file
 import { describe, it } from 'vitest';
 import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment';
-import { ClientLoggerFactory, LoggerCreationFunction, WindowAccessor } from '@/presentation/bootstrapping/ClientLoggerFactory';
 import { Logger } from '@/application/Common/Log/Logger';
 import { RuntimeEnvironmentStub } from '@tests/unit/shared/Stubs/RuntimeEnvironmentStub';
 import { itIsSingleton } from '@tests/unit/shared/TestCases/SingletonTests';
 import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub';
+import { ClientLoggerFactory, LoggerCreationFunction, WindowAccessor } from '@/presentation/components/Shared/Hooks/Log/ClientLoggerFactory';

 describe('ClientLoggerFactory', () => {
  describe('Current', () => {
@@ -29,7 +29,7 @@ describe('ClientLoggerFactory', () => {
        build: (b, expectedLogger) => b
          .withWindowInjectedLoggerFactory(() => expectedLogger)
          .withEnvironment(new RuntimeEnvironmentStub()
-            .withIsDesktop(true)
+            .withIsRunningAsDesktopApplication(true)
            .withIsNonProduction(false))
          .withWindowAccessor(() => createWindowWithLogger())
          .build(),
@@ -39,7 +39,7 @@ describe('ClientLoggerFactory', () => {
        build: (b, expectedLogger) => b
          .withWindowInjectedLoggerFactory(() => expectedLogger)
          .withEnvironment(new RuntimeEnvironmentStub()
-            .withIsDesktop(true)
+            .withIsRunningAsDesktopApplication(true)
            .withIsNonProduction(true))
          .withWindowAccessor(() => createWindowWithLogger())
          .build(),
@@ -49,7 +49,7 @@ describe('ClientLoggerFactory', () => {
        build: (b, expectedLogger) => b
          .withConsoleLoggerFactory(() => expectedLogger)
          .withEnvironment(new RuntimeEnvironmentStub()
-            .withIsDesktop(false)
+            .withIsRunningAsDesktopApplication(false)
            .withIsNonProduction(true))
          .withWindowAccessor(() => createWindowWithLogger())
          .build(),
@@ -59,7 +59,7 @@ describe('ClientLoggerFactory', () => {
        build: (b, expectedLogger) => b
          .withNoopLoggerFactory(() => expectedLogger)
          .withEnvironment(new RuntimeEnvironmentStub()
-            .withIsDesktop(false)
+            .withIsRunningAsDesktopApplication(false)
            .withIsNonProduction(false))
          .withWindowAccessor(() => createWindowWithLogger())
          .build(),
@@ -68,7 +68,7 @@ describe('ClientLoggerFactory', () => {
        description: 'unit/integration tests environment',
        build: (b, expectedLogger) => b
          .withNoopLoggerFactory(() => expectedLogger)
-          .withEnvironment(new RuntimeEnvironmentStub().withIsDesktop(true))
+          .withEnvironment(new RuntimeEnvironmentStub().withIsRunningAsDesktopApplication(true))
          .withWindowAccessor(() => createWindowWithLogger(null))
          .build(),
      },
--- a/tests/unit/presentation/components/Shared/Hooks/Log/UseLogger.spec.ts
+++ b/tests/unit/presentation/components/Shared/Hooks/Log/UseLogger.spec.ts
@@ -1,16 +1,17 @@
 import { describe, it, expect } from 'vitest';
-import { useLogger } from '@/presentation/components/Shared/Hooks/UseLogger';
 import { LoggerStub } from '@tests/unit/shared/Stubs/LoggerStub';
-import { LoggerFactoryStub } from '@tests/unit/shared/Stubs/LoggerFactoryStub';
+import { useLogger } from '@/presentation/components/Shared/Hooks/Log/UseLogger';
+import { LoggerFactory } from '@/presentation/components/Shared/Hooks/Log/LoggerFactory';

 describe('UseLogger', () => {
  it('returns expected logger from factory', () => {
    // arrange
    const expectedLogger = new LoggerStub();
-    const factory = new LoggerFactoryStub()
-      .withLogger(expectedLogger);
+    const factoryMock: LoggerFactory = {
+      logger: expectedLogger,
+    };
    // act
-    const { log: actualLogger } = useLogger(factory);
+    const { log: actualLogger } = useLogger(factoryMock);
    // assert
    expect(actualLogger).to.equal(expectedLogger);
  });
--- a/tests/unit/presentation/electron/main/IpcRegistration.spec.ts
+++ b/tests/unit/presentation/electron/main/IpcRegistration.spec.ts
@@ -1,10 +1,13 @@
 import { describe, it, expect } from 'vitest';
 import { CodeRunnerStub } from '@tests/unit/shared/Stubs/CodeRunnerStub';
-import { IpcChannelDefinitions } from '@/presentation/electron/shared/IpcBridging/IpcChannelDefinitions';
-import { CodeRunnerFactory, IpcRegistrar, registerAllIpcChannels } from '@/presentation/electron/main/IpcRegistration';
+import { ChannelDefinitionKey, IpcChannelDefinitions } from '@/presentation/electron/shared/IpcBridging/IpcChannelDefinitions';
+import {
+  CodeRunnerFactory, DialogFactory, IpcChannelRegistrar, registerAllIpcChannels,
+} from '@/presentation/electron/main/IpcRegistration';
 import { IpcChannel } from '@/presentation/electron/shared/IpcBridging/IpcChannel';
 import { expectExists } from '@tests/shared/Assertions/ExpectExists';
 import { collectExceptionMessage } from '@tests/unit/shared/ExceptionCollector';
+import { DialogStub } from '@tests/unit/shared/Stubs/DialogStub';

 describe('IpcRegistration', () => {
  describe('registerAllIpcChannels', () => {
@@ -23,7 +26,7 @@ describe('IpcRegistration', () => {
      });
    });
    describe('registers expected instances', () => {
-      const testScenarios: Record<keyof typeof IpcChannelDefinitions, {
+      const testScenarios: Record<ChannelDefinitionKey, {
        buildContext: (context: IpcRegistrationTestSetup) => IpcRegistrationTestSetup,
        expectedInstance: object,
      }> = {
@@ -34,6 +37,13 @@ describe('IpcRegistration', () => {
            expectedInstance,
          };
        })(),
+        Dialog: (() => {
+          const expectedInstance = new DialogStub();
+          return {
+            buildContext: (c) => c.withDialogFactory(() => expectedInstance),
+            expectedInstance,
+          };
+        })(),
      };
      Object.entries(testScenarios).forEach(([
        key, { buildContext, expectedInstance },
@@ -46,7 +56,8 @@ describe('IpcRegistration', () => {
          // act
          context.run();
          // assert
-          const actualInstance = getRegisteredInstance(IpcChannelDefinitions.CodeRunner);
+          const channel = IpcChannelDefinitions[key];
+          const actualInstance = getRegisteredInstance(channel);
          expect(actualInstance).to.equal(expectedInstance);
        });
      });
@@ -54,7 +65,7 @@ describe('IpcRegistration', () => {
    it('throws an error if registration fails', () => {
      // arrange
      const expectedError = 'registrar error';
-      const registrarMock: IpcRegistrar = () => {
+      const registrarMock: IpcChannelRegistrar = () => {
        throw new Error(expectedError);
      };
      const context = new IpcRegistrationTestSetup()
@@ -70,9 +81,11 @@ describe('IpcRegistration', () => {
 class IpcRegistrationTestSetup {
  private codeRunnerFactory: CodeRunnerFactory = () => new CodeRunnerStub();

-  private registrar: IpcRegistrar = () => { /* NOOP */ };
+  private dialogFactory: DialogFactory = () => new DialogStub();

-  public withRegistrar(registrar: IpcRegistrar): this {
+  private registrar: IpcChannelRegistrar = () => { /* NOOP */ };
+
+  public withRegistrar(registrar: IpcChannelRegistrar): this {
    this.registrar = registrar;
    return this;
  }
@@ -82,26 +95,37 @@ class IpcRegistrationTestSetup {
    return this;
  }

+  public withDialogFactory(dialogFactory: DialogFactory): this {
+    this.dialogFactory = dialogFactory;
+    return this;
+  }
+
  public run() {
    registerAllIpcChannels(
      this.codeRunnerFactory,
+      this.dialogFactory,
      this.registrar,
    );
  }
 }

+type DefinedIpcChannelTypes = {
+  [K in ChannelDefinitionKey]: (typeof IpcChannelDefinitions)[K]
+}[ChannelDefinitionKey];
+
 function createIpcRegistrarMock() {
-  const registeredChannels = new Array<Parameters<IpcRegistrar>>();
-  const registrarMock: IpcRegistrar = <T>(channel: IpcChannel<T>, obj: T) => {
+  const registeredChannels = new Array<Parameters<IpcChannelRegistrar>>();
+  const registrarMock: IpcChannelRegistrar = <T>(channel: IpcChannel<T>, obj: T) => {
    registeredChannels.push([channel as IpcChannel<unknown>, obj]);
  };
-  const isChannelRegistered = <T>(channel: IpcChannel<T>): boolean => {
+  const isChannelRegistered = (channel: DefinedIpcChannelTypes): boolean => {
    return registeredChannels.some((i) => i[0] === channel);
  };
-  const getRegisteredInstance = <T>(channel: IpcChannel<T>): unknown => {
+  const getRegisteredInstance = <T>(channel: IpcChannel<T>): T => {
    const registration = registeredChannels.find((i) => i[0] === channel);
    expectExists(registration);
-    return registration[1];
+    const [, registeredInstance] = registration;
+    return registeredInstance as T;
  };
  return {
    registrarMock,
--- a/tests/unit/presentation/electron/preload/ContextBridging/RendererApiProvider.spec.ts
+++ b/tests/unit/presentation/electron/preload/ContextBridging/RendererApiProvider.spec.ts
@@ -16,7 +16,7 @@ describe('RendererApiProvider', () => {
      readonly expectedValue: unknown;
    }
    const testScenarios: Record<PropertyKeys<Required<WindowVariables>>, WindowVariableTestCase> = {
-      isDesktop: {
+      isRunningAsDesktopApplication: {
        description: 'returns true',
        setupContext: (context) => context,
        expectedValue: true,
@@ -34,6 +34,7 @@ describe('RendererApiProvider', () => {
        instance: new LoggerStub(),
        setupContext: (c, logger) => c.withLogger(logger),
      }),
+      dialog: expectIpcConsumer(IpcChannelDefinitions.Dialog),
    };
    Object.entries(testScenarios).forEach((
      [property, { description, setupContext, expectedValue }],
--- a/tests/unit/presentation/electron/shared/IpcChannelDefinitions.spec.ts
+++ b/tests/unit/presentation/electron/shared/IpcChannelDefinitions.spec.ts
@@ -1,10 +1,10 @@
 import { describe, it, expect } from 'vitest';
 import { IpcChannel } from '@/presentation/electron/shared/IpcBridging/IpcChannel';
-import { IpcChannelDefinitions } from '@/presentation/electron/shared/IpcBridging/IpcChannelDefinitions';
+import { ChannelDefinitionKey, IpcChannelDefinitions } from '@/presentation/electron/shared/IpcBridging/IpcChannelDefinitions';

 describe('IpcChannelDefinitions', () => {
  it('defines IPC channels correctly', () => {
-    const testScenarios: Record<keyof typeof IpcChannelDefinitions, {
+    const testScenarios: Record<ChannelDefinitionKey, {
      readonly expectedNamespace: string;
      readonly expectedAccessibleMembers: readonly string[];
    }> = {
@@ -12,6 +12,10 @@ describe('IpcChannelDefinitions', () => {
        expectedNamespace: 'code-run',
        expectedAccessibleMembers: ['runCode'],
      },
+      Dialog: {
+        expectedNamespace: 'dialogs',
+        expectedAccessibleMembers: ['saveFile'],
+      },
    };
    Object.entries(testScenarios).forEach((
      [definitionKey, { expectedNamespace, expectedAccessibleMembers }],
@@ -49,7 +53,7 @@ describe('IpcChannelDefinitions', () => {
    it('has unique accessible members within each channel', () => {
      Object.values(IpcChannelDefinitions).forEach((channel) => {
        // arrange
-        const { accessibleMembers: accessibleMembersOfChannel } = channel;
+        const accessibleMembersOfChannel = channel.accessibleMembers as string[];
        // act
        const repeatedAccessibleMembersInChannel = accessibleMembersOfChannel
          .filter((item, index) => accessibleMembersOfChannel.indexOf(item) !== index);
--- a/tests/unit/presentation/electron/shared/IpcProxy.spec.ts
+++ b/tests/unit/presentation/electron/shared/IpcProxy.spec.ts
@@ -215,19 +215,35 @@ describe('IpcProxy', () => {
      expect(actualChannelNames).to.have.lengthOf(expectedChannelNames.length);
      expect(actualChannelNames).to.have.members(expectedChannelNames);
    });
-    it('throws error for non-function members', () => {
-      // arrange
-      const expectedError = 'Non-function members are not yet supported';
-      const propertyName = 'propertyKey';
-      const testObject = { [`${propertyName}`]: 123 };
-      const testIpcChannel: IpcChannel<typeof testObject> = {
-        namespace: 'testNamespace',
-        accessibleMembers: [propertyName] as never,
-      };
-      // act
-      const act = () => registerIpcChannel(testIpcChannel, testObject, mockIpcMain().ipcMainMock);
-      // assert
-      expect(act).to.throw(expectedError);
+    describe('validation', () => {
+      it('throws error for non-function members', () => {
+        // arrange
+        const expectedError = 'Non-function members are not yet supported';
+        const propertyName = 'propertyKey';
+        const testObject = { [`${propertyName}`]: 123 };
+        const testIpcChannel: IpcChannel<typeof testObject> = {
+          namespace: 'testNamespace',
+          accessibleMembers: [propertyName] as never,
+        };
+        // act
+        const act = () => registerIpcChannel(testIpcChannel, testObject, mockIpcMain().ipcMainMock);
+        // assert
+        expect(act).to.throw(expectedError);
+      });
+      it('throws error for undefined members', () => {
+        // arrange
+        const nonExistingFunctionName = 'nonExistingFunction';
+        const expectedError = `The function "${nonExistingFunctionName}" is not found on the target object.`;
+        const testObject = { };
+        const testIpcChannel: IpcChannel<typeof testObject> = {
+          namespace: 'testNamespace',
+          accessibleMembers: [nonExistingFunctionName] as never,
+        };
+        // act
+        const act = () => registerIpcChannel(testIpcChannel, testObject, mockIpcMain().ipcMainMock);
+        // assert
+        expect(act).to.throw(expectedError);
+      });
    });
  });
 });