From 038b35cc43d38d53c3a7f2c90479bb16a18084a6 Mon Sep 17 00:00:00 2001 From: Kristóf Marussy Date: Tue, 25 Jan 2022 17:26:32 +0100 Subject: refactor: Store services in a map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the synchronization of references across the main/renderer process boundary more robust. Signed-off-by: Kristóf Marussy --- packages/main/src/stores/Profile.ts | 10 +-- packages/main/src/stores/Service.ts | 21 ++--- packages/main/src/stores/SharedStore.ts | 130 ++++++++++++++++++++++-------- packages/main/src/utils/SettingsWithId.ts | 25 ------ packages/shared/src/stores/SharedStore.ts | 6 +- 5 files changed, 111 insertions(+), 81 deletions(-) delete mode 100644 packages/main/src/utils/SettingsWithId.ts 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 {} -export type ProfileSettingsSnapshotWithId = - SettingsWithId; +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 { @@ -89,8 +85,7 @@ export const service = overrideProps(originalService, { export interface Service extends Instance {} -export type ServiceSettingsSnapshotWithId = - SettingsWithId; +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(models: { config: T }[]): T[] | undefined { return models.length === 0 ? undefined : models.map((model) => model.config); } -function reconcileSettings( - originalSnapshots: SettingsWithId[], - settingsSnapshotsWithId: SettingsWithId[], -): SettingsWithId[] { - const idToOriginalSnapshots = new Map( - originalSnapshots.map((originalSnapshot) => [ - originalSnapshot.id, - originalSnapshot, - ]), - ); - return settingsSnapshotsWithId.map(({ id, settings }) => ({ - ...idToOriginalSnapshots.get(id), - id, - settings, - })); +type TypeWithSettings = IType< + { id: string; settings: C }, + unknown, + { settings: IStateTreeNode> } +>; + +function deleteStaleModels>( + currentById: IMSTMap, + toApplyById: Map, +): void { + const toDelete = new Set(currentById.keys()); + toApplyById.forEach((_settings, id) => toDelete.delete(id)); + toDelete.forEach((id) => currentById.delete(id)); +} + +function applySettings>( + currentById: IMSTMap, + toApplyById: Map, +): 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>( + list: IMSTArray>, + 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 - * - * 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 . - * - * SPDX-License-Identifier: AGPL-3.0-only - */ - -export default interface SettingsWithId { - 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, }); -- cgit v1.2.3-54-g00ecf