refactor: Store services in a map

Makes the synchronization of references across the main/renderer process
boundary more robust.

Signed-off-by: Kristóf Marussy <kristof@marussy.com>

5 files changed, 111 insertions, 81 deletions

diff --git a/packages/main/src/stores/Profile.ts b/packages/main/src/stores/Profile.ts
index eaf23c4..5f77fe4 100644
--- a/packages/main/src/stores/Profile.ts
+++ b/packages/main/src/stores/Profile.ts
@@ -24,7 +24,6 @@ import {
 } from '@sophie/shared';
 import { getSnapshot, Instance } from 'mobx-state-tree';
 
-import SettingsWithId from '../utils/SettingsWithId';
 import generateId from '../utils/generateId';
 
 export interface ProfileConfig extends ProfileSettingsSnapshotIn {
@@ -40,17 +39,16 @@ export const profile = originalProfile.views((self) => ({
 
 export interface Profile extends Instance<typeof profile> {}
 
-export type ProfileSettingsSnapshotWithId =
-  SettingsWithId<ProfileSettingsSnapshotIn>;
+export type ProfileSettingsSnapshotWithId = [string, ProfileSettingsSnapshotIn];
 
 export function addMissingProfileIds(
   profileConfigs: ProfileConfig[] | undefined,
 ): ProfileSettingsSnapshotWithId[] {
   return (profileConfigs ?? []).map((profileConfig) => {
     const { id, ...settings } = profileConfig;
-    return {
-      id: typeof id === 'undefined' ? generateId(settings.name) : id,
+    return [
+      typeof id === 'undefined' ? generateId(settings.name) : id,
       settings,
-    };
+    ];
   });
 }
diff --git a/packages/main/src/stores/Service.ts b/packages/main/src/stores/Service.ts
index 78c57cb..331805b 100644
--- a/packages/main/src/stores/Service.ts
+++ b/packages/main/src/stores/Service.ts
@@ -19,18 +19,14 @@
  */
 
 import type { UnreadCount } from '@sophie/service-shared';
-import {
-  service as originalService,
-  ServiceSettingsSnapshotIn,
-} from '@sophie/shared';
+import { service as originalService } from '@sophie/shared';
 import { Instance, getSnapshot, ReferenceIdentifier } from 'mobx-state-tree';
 
-import SettingsWithId from '../utils/SettingsWithId';
 import generateId from '../utils/generateId';
 import overrideProps from '../utils/overrideProps';
 
 import { ProfileSettingsSnapshotWithId } from './Profile';
-import { serviceSettings } from './ServiceSettings';
+import { serviceSettings, ServiceSettingsSnapshotIn } from './ServiceSettings';
 
 export interface ServiceConfig
   extends Omit<ServiceSettingsSnapshotIn, 'profile'> {
@@ -89,8 +85,7 @@ export const service = overrideProps(originalService, {
 
 export interface Service extends Instance<typeof service> {}
 
-export type ServiceSettingsSnapshotWithId =
-  SettingsWithId<ServiceSettingsSnapshotIn>;
+export type ServiceSettingsSnapshotWithId = [string, ServiceSettingsSnapshotIn];
 
 export function addMissingServiceIdsAndProfiles(
   serviceConfigs: ServiceConfig[] | undefined,
@@ -102,11 +97,11 @@ export function addMissingServiceIdsAndProfiles(
     let { profile: profileId } = settings;
     if (profileId === undefined) {
       profileId = generateId(name);
-      profiles.push({ id: profileId, settings: { name } });
+      profiles.push([profileId, { name }]);
     }
-    return {
-      id: id === undefined ? generateId(name) : id,
-      settings: { ...settings, profile: profileId },
-    };
+    return [
+      id === undefined ? generateId(name) : id,
+      { ...settings, profile: profileId },
+    ];
   });
 }
diff --git a/packages/main/src/stores/SharedStore.ts b/packages/main/src/stores/SharedStore.ts
index 861c8ce..9ec7963 100644
--- a/packages/main/src/stores/SharedStore.ts
+++ b/packages/main/src/stores/SharedStore.ts
@@ -22,12 +22,16 @@ import { sharedStore as originalSharedStore } from '@sophie/shared';
 import {
   applySnapshot,
   getSnapshot,
+  IMSTArray,
+  IMSTMap,
   Instance,
+  IReferenceType,
+  IStateTreeNode,
+  IType,
   resolveIdentifier,
   types,
 } from 'mobx-state-tree';
 
-import SettingsWithId from '../utils/SettingsWithId';
 import { getLogger } from '../utils/log';
 import overrideProps from '../utils/overrideProps';
 
@@ -51,27 +55,51 @@ function getConfigs<T>(models: { config: T }[]): T[] | undefined {
   return models.length === 0 ? undefined : models.map((model) => model.config);
 }
 
-function reconcileSettings<T>(
-  originalSnapshots: SettingsWithId<T>[],
-  settingsSnapshotsWithId: SettingsWithId<T>[],
-): SettingsWithId<T>[] {
-  const idToOriginalSnapshots = new Map(
-    originalSnapshots.map((originalSnapshot) => [
-      originalSnapshot.id,
-      originalSnapshot,
-    ]),
-  );
-  return settingsSnapshotsWithId.map(({ id, settings }) => ({
-    ...idToOriginalSnapshots.get(id),
-    id,
-    settings,
-  }));
+type TypeWithSettings<C> = IType<
+  { id: string; settings: C },
+  unknown,
+  { settings: IStateTreeNode<IType<C, unknown, unknown>> }
+>;
+
+function deleteStaleModels<C, D extends TypeWithSettings<C>>(
+  currentById: IMSTMap<D>,
+  toApplyById: Map<string, C>,
+): void {
+  const toDelete = new Set(currentById.keys());
+  toApplyById.forEach((_settings, id) => toDelete.delete(id));
+  toDelete.forEach((id) => currentById.delete(id));
+}
+
+function applySettings<C, D extends TypeWithSettings<C>>(
+  currentById: IMSTMap<D>,
+  toApplyById: Map<string, C>,
+): void {
+  toApplyById.forEach((settingsSnapshot, id) => {
+    const model = currentById.get(id);
+    if (model === undefined) {
+      currentById.set(id, {
+        id,
+        settings: settingsSnapshot,
+      });
+    } else {
+      applySnapshot(model.settings, settingsSnapshot);
+    }
+  });
+}
+
+function pushReferences<C, D extends TypeWithSettings<C>>(
+  list: IMSTArray<IReferenceType<D>>,
+  toApply: [string, C][],
+): void {
+  list.push(...toApply.map(([id]) => id));
 }
 
 export const sharedStore = overrideProps(originalSharedStore, {
   settings: types.optional(globalSettings, {}),
-  profiles: types.array(profile),
-  services: types.array(service),
+  profilesById: types.map(profile),
+  profiles: types.array(types.reference(profile)),
+  servicesById: types.map(service),
+  services: types.array(types.reference(service)),
   selectedService: types.safeReference(service),
 })
   .views((self) => ({
@@ -87,25 +115,57 @@ export const sharedStore = overrideProps(originalSharedStore, {
   }))
   .actions((self) => ({
     loadConfig(config: Config): void {
-      const snapshot = getSnapshot(self);
-      const { profiles, services, ...settings } = config;
-      const profileSettingsSnapshots = addMissingProfileIds(profiles);
-      const serviceSettingsSnapshots = addMissingServiceIdsAndProfiles(
+      // `onPatch` will send store changes piecemeal without any attention to
+      // transaction boundaries. We must make sure that any state communicated to the
+      // renderer process is actually valid.
+      const {
+        profiles,
+        profilesById,
+        selectedService,
         services,
-        profileSettingsSnapshots,
-      );
-      applySnapshot(self, {
-        ...snapshot,
+        servicesById,
         settings,
-        profiles: reconcileSettings(
-          snapshot.profiles,
-          profileSettingsSnapshots,
-        ),
-        services: reconcileSettings(
-          snapshot.services,
-          serviceSettingsSnapshots,
-        ),
-      });
+      } = self;
+      const { id: selectedServiceId } = selectedService ?? { id: undefined };
+      const {
+        profiles: profilesConfig,
+        services: servicesConfig,
+        ...settingsToApply
+      } = config;
+      const profilesToApply = addMissingProfileIds(profilesConfig);
+      const servicesToApply = addMissingServiceIdsAndProfiles(
+        servicesConfig,
+        profilesToApply,
+      );
+      const profilesToApplyById = new Map(profilesToApply);
+      const servicesToApplyById = new Map(servicesToApply);
+      // First remove any references to profiles and services that might be deleted.
+      self.selectedService = undefined;
+      services.clear();
+      profiles.clear();
+      // Delete all services that may depend on profiles that will be delete.
+      deleteStaleModels(servicesById, servicesToApplyById);
+      // Update existing profiles and add new profiles.
+      applySettings(profilesById, profilesToApplyById);
+      // Update existing services and add new services. This will make sure that no service
+      // depends on a profile that will be deleted.
+      applySettings(servicesById, servicesToApplyById);
+      // Now it's safe to delete stale profiles.
+      deleteStaleModels(profilesById, profilesToApplyById);
+      // We are ready to build new profile and service lists from the new models.
+      pushReferences(profiles, profilesToApply);
+      pushReferences(services, servicesToApply);
+      // Global settings may refer to particular profiles or services.
+      applySnapshot(settings, settingsToApply);
+      // Restore service selection (if applicable).
+      let newSelectedService;
+      if (selectedServiceId !== undefined) {
+        newSelectedService = servicesById.get(selectedServiceId);
+      }
+      if (newSelectedService === undefined && services.length > 0) {
+        [newSelectedService] = services;
+      }
+      self.selectedService = newSelectedService;
     },
     setShouldUseDarkColors(shouldUseDarkColors: boolean): void {
       self.shouldUseDarkColors = shouldUseDarkColors;
diff --git a/packages/main/src/utils/SettingsWithId.ts b/packages/main/src/utils/SettingsWithId.ts
deleted file mode 100644
index fde3e86..0000000
--- a/packages/main/src/utils/SettingsWithId.ts
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * Copyright (C)  2022 Kristóf Marussy <kristof@marussy.com>
- *
- * This file is part of Sophie.
- *
- * Sophie is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License as
- * published by the Free Software Foundation, version 3.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License
- * along with this program.  If not, see <https://www.gnu.org/licenses/>.
- *
- * SPDX-License-Identifier: AGPL-3.0-only
- */
-
-export default interface SettingsWithId<T> {
-  id: string;
-
-  settings: T;
-}
diff --git a/packages/shared/src/stores/SharedStore.ts b/packages/shared/src/stores/SharedStore.ts
index a04f4bf..fc8372e 100644
--- a/packages/shared/src/stores/SharedStore.ts
+++ b/packages/shared/src/stores/SharedStore.ts
@@ -32,8 +32,10 @@ import { service } from './Service';
 
 export const sharedStore = types.model('SharedStore', {
   settings: types.optional(globalSettings, {}),
-  profiles: types.array(profile),
-  services: types.array(service),
+  profilesById: types.map(profile),
+  profiles: types.array(types.reference(profile)),
+  servicesById: types.map(service),
+  services: types.array(types.reference(service)),
   selectedService: types.safeReference(service),
   shouldUseDarkColors: false,
 });