From 8f2099c7c5f760ec670a6efc3ed4d7b6964d4753 Mon Sep 17 00:00:00 2001 From: Nanashi Date: Mon, 13 Nov 2023 13:09:47 +0900 Subject: [PATCH] =?UTF-8?q?Change:=20=E3=83=96=E3=83=A9=E3=82=A6=E3=82=B6?= =?UTF-8?q?=E7=89=88=E3=81=AEConfig=E3=82=92BaseConfigManager=E3=83=99?= =?UTF-8?q?=E3=83=BC=E3=82=B9=E3=81=AB=20(#1629)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Hiroshiba --- .github/workflows/test.yml | 6 +- playwright.config.ts | 2 +- src/background.ts | 1 + src/background/electronConfig.ts | 8 +- src/background/engineManager.ts | 15 +- src/browser/contract.ts | 15 +- src/browser/fileImpl.ts | 6 +- src/browser/sandbox.ts | 13 +- src/browser/storeImpl.ts | 141 ++++++++++-------- src/components/SettingDialog.vue | 16 +- src/shared/ConfigManager.ts | 47 +++--- src/type/preload.ts | 11 ++ ...3\202\273\343\203\263\343\203\210.spec.ts" | 4 +- tests/e2e/electron/example.spec.ts | 4 +- tests/e2e/navigators.ts | 2 - 15 files changed, 164 insertions(+), 127 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 631676390e..7e6ecfb964 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -39,6 +39,7 @@ jobs: e2e-test: runs-on: ${{ matrix.os }} strategy: + fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] include: @@ -78,9 +79,10 @@ jobs: chmod +x ${{ steps.download-engine.outputs.run_path }} # .env + sed -i -e 's|"074fc39e-678b-4c13-8916-ffca8d505d1d"|"208cf94d-43d2-4cf5-abc0-9783cac36d29"|' .env.test + sed -i -e 's|"../voicevox_engine/run.exe"|"${{ steps.download-engine.outputs.run_path }}"|' .env.test + sed -i -e 's|"executionArgs": \[\],|"executionArgs": ["--port=50021"],|' .env.test cp .env.test .env - sed -i -e 's|"../voicevox_engine/run.exe"|"${{ steps.download-engine.outputs.run_path }}"|' .env - sed -i -e 's|"executionArgs": \[\],|"executionArgs": ["--port=50021"],|' .env - name: Run npm run test:browser-e2e run: | diff --git a/playwright.config.ts b/playwright.config.ts index d849a2302e..e360ab9050 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -48,7 +48,7 @@ if (isElectron) { const config: PlaywrightTestConfig = { testDir: "./tests/e2e", /* Maximum time one test can run for. */ - timeout: 120 * 1000, + timeout: 60 * 1000, expect: { /** * Maximum time expect() should wait for the condition to be met. diff --git a/src/background.ts b/src/background.ts index 1802994f3d..c73d12d1e7 100644 --- a/src/background.ts +++ b/src/background.ts @@ -482,6 +482,7 @@ async function launchEngines() { // エンジンの追加と削除を反映させるためEngineInfoとAltPortInfoを再生成する。 engineManager.initializeEngineInfosAndAltPortInfo(); + // TODO: デフォルトエンジンの処理をConfigManagerに移してブラウザ版と共通化する const engineInfos = engineManager.fetchEngineInfos(); const engineSettings = configManager.get("engineSettings"); for (const engineInfo of engineInfos) { diff --git a/src/background/electronConfig.ts b/src/background/electronConfig.ts index 0fde5e668d..f534164db0 100644 --- a/src/background/electronConfig.ts +++ b/src/background/electronConfig.ts @@ -5,22 +5,22 @@ import { BaseConfigManager, Metadata } from "@/shared/ConfigManager"; import { ConfigType } from "@/type/preload"; export class ElectronConfigManager extends BaseConfigManager { - getAppVersion() { + protected getAppVersion() { return app.getVersion(); } - public async exists() { + protected async exists() { return await fs.promises .stat(this.configPath) .then(() => true) .catch(() => false); } - public async load(): Promise & Metadata> { + protected async load(): Promise & Metadata> { return JSON.parse(await fs.promises.readFile(this.configPath, "utf-8")); } - public async save(config: ConfigType) { + protected async save(config: ConfigType & Metadata) { await fs.promises.writeFile( this.configPath, JSON.stringify(config, undefined, 2) diff --git a/src/background/engineManager.ts b/src/background/engineManager.ts index 93734d387b..eee03cc6e8 100644 --- a/src/background/engineManager.ts +++ b/src/background/engineManager.ts @@ -7,7 +7,6 @@ import shlex from "shlex"; import { app, dialog } from "electron"; // FIXME: ここでelectronをimportするのは良くない import log from "electron-log/main"; -import { z } from "zod"; import { findAltPort, getPidFromPort, @@ -21,8 +20,8 @@ import { EngineDirValidationResult, MinimumEngineManifest, EngineId, - engineIdSchema, minimumEngineManifestSchema, + envEngineInfoSchema, } from "@/type/preload"; import { AltPortInfos } from "@/store/type"; import { BaseConfigManager } from "@/shared/ConfigManager"; @@ -40,17 +39,7 @@ function createDefaultEngineInfos(defaultEngineDir: string): EngineInfo[] { const defaultEngineInfosEnv = import.meta.env.VITE_DEFAULT_ENGINE_INFOS ?? "[]"; - const envSchema = z - .object({ - uuid: engineIdSchema, - host: z.string(), - name: z.string(), - executionEnabled: z.boolean(), - executionFilePath: z.string(), - executionArgs: z.array(z.string()), - path: z.string().optional(), - }) - .array(); + const envSchema = envEngineInfoSchema.array(); const engines = envSchema.parse(JSON.parse(defaultEngineInfosEnv)); return engines.map((engineInfo) => { diff --git a/src/browser/contract.ts b/src/browser/contract.ts index 486ed0ef65..f649de781f 100644 --- a/src/browser/contract.ts +++ b/src/browser/contract.ts @@ -1,14 +1,11 @@ -import { EngineInfo, EngineId } from "@/type/preload"; +import { EngineInfo, envEngineInfoSchema } from "@/type/preload"; + +const baseEngineInfo = envEngineInfoSchema + .array() + .parse(JSON.parse(import.meta.env.VITE_DEFAULT_ENGINE_INFOS))[0]; export const defaultEngine: EngineInfo = { - uuid: EngineId("074fc39e-678b-4c13-8916-ffca8d505d1d"), - host: "http://127.0.0.1:50021", - name: "VOICEVOX Engine", - path: undefined, - executionEnabled: false, - executionFilePath: "", - executionArgs: [], + ...baseEngineInfo, type: "default", }; - export const directoryHandleStoreKey = "directoryHandle"; diff --git a/src/browser/fileImpl.ts b/src/browser/fileImpl.ts index 608891a452..dc74eb37e3 100644 --- a/src/browser/fileImpl.ts +++ b/src/browser/fileImpl.ts @@ -56,7 +56,7 @@ const showWritableDirectoryPicker = async (): Promise< export const showOpenDirectoryDialogImpl: typeof window[typeof SandboxKey]["showOpenDirectoryDialog"] = async () => { const _directoryHandler = await showWritableDirectoryPicker(); - if (_directoryHandler === undefined) { + if (_directoryHandler == undefined) { return undefined; } @@ -83,7 +83,7 @@ const getDirectoryHandleFromDirectoryPath = async ( ): Promise => { const maybeHandle = directoryHandleMap.get(maybeDirectoryPathKey); - if (maybeHandle !== undefined) { + if (maybeHandle != undefined) { return maybeHandle; } else { // NOTE: fixedDirectoryの場合こっちに落ちる場合がある @@ -91,7 +91,7 @@ const getDirectoryHandleFromDirectoryPath = async ( maybeDirectoryPathKey ); - if (maybeFixedDirectory === undefined) { + if (maybeFixedDirectory == undefined) { throw new Error( `フォルダへのアクセス許可がありません。アクセスしようとしたフォルダ名: ${maybeDirectoryPathKey}` ); diff --git a/src/browser/sandbox.ts b/src/browser/sandbox.ts index ccfda27e04..f61d38cece 100644 --- a/src/browser/sandbox.ts +++ b/src/browser/sandbox.ts @@ -4,7 +4,7 @@ import { showOpenDirectoryDialogImpl, writeFileImpl, } from "./fileImpl"; -import { getSettingEntry, setSettingEntry } from "./storeImpl"; +import { getConfigManager } from "./storeImpl"; import { IpcSOData } from "@/type/ipc"; import { @@ -285,11 +285,14 @@ export const api: Sandbox = { // NOTE: 何もしなくて良さそう return Promise.resolve(); }, - getSetting(key) { - return getSettingEntry(key); + async getSetting(key) { + const configManager = await getConfigManager(); + return configManager.get(key); }, - setSetting(key, newValue) { - return setSettingEntry(key, newValue).then(() => getSettingEntry(key)); + async setSetting(key, newValue) { + const configManager = await getConfigManager(); + configManager.set(key, newValue); + return newValue; }, async setEngineSetting(engineId: EngineId, engineSetting: EngineSetting) { const engineSettings = (await this.getSetting( diff --git a/src/browser/storeImpl.ts b/src/browser/storeImpl.ts index 24792d4e7c..f96044842e 100644 --- a/src/browser/storeImpl.ts +++ b/src/browser/storeImpl.ts @@ -1,19 +1,44 @@ +import AsyncLock from "async-lock"; import { defaultEngine, directoryHandleStoreKey } from "./contract"; -import { - configSchema, - ConfigType, - EngineId, - engineSettingSchema, -} from "@/type/preload"; +import { BaseConfigManager, Metadata } from "@/shared/ConfigManager"; +import { ConfigType, EngineId, engineSettingSchema } from "@/type/preload"; const dbName = `${import.meta.env.VITE_APP_NAME}-web`; -const settingStoreKey = "electronStore"; -// FIXME: DBのschemaを変更したら、dbVersionを上げる -// TODO: 気づけるようにしたい -const dbVersion = 1; +const settingStoreKey = "config"; +const dbVersion = 1; // 固定値。configのmigrationには使用していない。 // NOTE: settingを複数持つことはないと仮定して、keyを固定してしまう -export const entryKey = "value"; +const entryKey = "value"; + +let configManager: BrowserConfigManager | undefined; + +const configManagerLock = new AsyncLock(); +const defaultEngineId = EngineId(defaultEngine.uuid); + +export async function getConfigManager() { + await configManagerLock.acquire("configManager", async () => { + if (!configManager) { + configManager = new BrowserConfigManager(); + await configManager.initialize(); + } + }); + + if (!configManager) { + throw new Error("configManager is undefined"); + } + + return configManager; +} + +const waitRequest = (request: IDBRequest) => + new Promise((resolve, reject) => { + request.onsuccess = () => { + resolve(); + }; + request.onerror = () => { + reject(request.error); + }; + }); export const openDB = () => new Promise((resolve, reject) => { @@ -29,20 +54,15 @@ export const openDB = () => if (ev.oldVersion === 0) { // Initialize const db = request.result; - const baseSchema = configSchema.parse({}); - const defaultVoicevoxEngineId = EngineId(defaultEngine.uuid); - baseSchema.engineSettings = { - [defaultVoicevoxEngineId]: engineSettingSchema.parse({}), - }; - db.createObjectStore(settingStoreKey).add(baseSchema, entryKey); + db.createObjectStore(settingStoreKey); // NOTE: fixedExportDirectoryを使用してファイルの書き出しをする際、 // audio.tsの現在の実装では、ディレクトリを選択するモーダルを表示しないようになっている // ディレクトリへの書き出し権限の要求は、モーダルの表示かディレクトリを指定したファイルの書き出しの時のみで、 // directoryHandleがないと権限の要求が出来ないため、directoryHandleを永続化しておく db.createObjectStore(directoryHandleStoreKey); - } else if (ev.newVersion !== null && ev.newVersion > ev.oldVersion) { + } else if (ev.newVersion != null && ev.newVersion > ev.oldVersion) { // TODO: migrate /* eslint-disable no-console */ // logger みたいなパッケージに切り出して、それに依存する形でもいいかも console.error( @@ -53,49 +73,52 @@ export const openDB = () => }; }); -export const setSettingEntry = async ( - key: Key, - newValue: ConfigType[Key] -) => { - const db = await openDB(); - - // TODO: Schemaに合っているか保存時にvalidationしたい - return new Promise((resolve, reject) => { - const transaction = db.transaction(settingStoreKey, "readwrite"); - const store = transaction.objectStore(settingStoreKey); - const getRequest = store.get(entryKey); - getRequest.onsuccess = () => { - const baseSchema = configSchema.parse(getRequest.result); - baseSchema[key] = newValue; - const validatedSchema = configSchema.parse(baseSchema); - const putRequest = store.put(validatedSchema, entryKey); - putRequest.onsuccess = () => { - resolve(putRequest.result); - }; - putRequest.onerror = () => { - reject(putRequest.error); - }; - }; - getRequest.onerror = () => { - reject(getRequest.error); - }; - }); -}; +class BrowserConfigManager extends BaseConfigManager { + protected getAppVersion() { + return import.meta.env.VITE_APP_VERSION; + } + protected async exists() { + const db = await openDB(); -export const getSettingEntry = async ( - key: Key -): Promise => { - const db = await openDB(); + try { + const transaction = db.transaction(settingStoreKey, "readonly"); + const store = transaction.objectStore(settingStoreKey); + const request = store.get(entryKey); + await waitRequest(request); + const result = request.result; + return result != undefined; + } catch (e) { + return false; + } + } + protected async load(): Promise & Metadata> { + const db = await openDB(); - return new Promise((resolve, reject) => { const transaction = db.transaction(settingStoreKey, "readonly"); const store = transaction.objectStore(settingStoreKey); const request = store.get(entryKey); - request.onsuccess = () => { - resolve(request.result[key]); - }; - request.onerror = () => { - reject(request.error); - }; - }); -}; + await waitRequest(request); + const result = request.result; + if (result == undefined) { + throw new Error("設定ファイルが見つかりません"); + } + return JSON.parse(result); + } + + protected async save(data: ConfigType & Metadata) { + const db = await openDB(); + + const transaction = db.transaction(settingStoreKey, "readwrite"); + const store = transaction.objectStore(settingStoreKey); + const request = store.put(JSON.stringify(data), entryKey); + await waitRequest(request); + } + + protected getDefaultConfig() { + const baseConfig = super.getDefaultConfig(); + baseConfig.engineSettings[defaultEngineId] ??= engineSettingSchema.parse( + {} + ); + return baseConfig; + } +} diff --git a/src/components/SettingDialog.vue b/src/components/SettingDialog.vue index 9f99234d04..d735aff9d7 100644 --- a/src/components/SettingDialog.vue +++ b/src/components/SettingDialog.vue @@ -1087,21 +1087,25 @@ const changeShowAddAudioItemButton = async ( const canSetAudioOutputDevice = computed(() => { return !!HTMLAudioElement.prototype.setSinkId; }); -const currentAudioOutputDeviceComputed = computed<{ - key: string; - label: string; -} | null>({ +const currentAudioOutputDeviceComputed = computed< + | { + key: string; + label: string; + } + | undefined +>({ get: () => { // 再生デバイスが見つからなかったらデフォルト値に戻す + // FIXME: watchなどにしてgetter内で操作しないようにする const device = availableAudioOutputDevices.value?.find( (device) => device.key === store.state.savingSetting.audioOutputDevice ); if (device) { return device; - } else { + } else if (store.state.savingSetting.audioOutputDevice !== "default") { handleSavingSettingChange("audioOutputDevice", "default"); - return null; } + return undefined; }, set: (device) => { if (device) { diff --git a/src/shared/ConfigManager.ts b/src/shared/ConfigManager.ts index bdb7af71de..fd5e61a095 100644 --- a/src/shared/ConfigManager.ts +++ b/src/shared/ConfigManager.ts @@ -1,4 +1,5 @@ import semver from "semver"; +import AsyncLock from "async-lock"; import { AcceptTermsStatus, ConfigType, @@ -9,6 +10,8 @@ import { HotkeySetting, } from "@/type/preload"; +const lockKey = "save"; + const migrations: [string, (store: Record) => unknown][] = [ [ ">=0.13", @@ -118,13 +121,13 @@ export type Metadata = { export abstract class BaseConfigManager { protected config: ConfigType | undefined; - private saveCounter = 0; + private lock = new AsyncLock(); - abstract exists(): Promise; - abstract load(): Promise & Metadata>; - abstract save(config: ConfigType & Metadata): Promise; + protected abstract exists(): Promise; + protected abstract load(): Promise & Metadata>; + protected abstract save(config: ConfigType & Metadata): Promise; - abstract getAppVersion(): string; + protected abstract getAppVersion(): string; public async initialize(): Promise { if (await this.exists()) { @@ -137,10 +140,10 @@ export abstract class BaseConfigManager { } this.config = this.migrateHotkeySettings(configSchema.parse(data)); } else { - const defaultConfig = configSchema.parse({}); - this.config = defaultConfig; + this.config = this.getDefaultConfig(); } this._save(); + await this.ensureSaved(); return this; } @@ -157,23 +160,22 @@ export abstract class BaseConfigManager { } private _save() { - this.saveCounter++; - this.save({ - ...configSchema.parse({ - ...this.config, - }), - __internal__: { - migrations: { - version: this.getAppVersion(), + this.lock.acquire(lockKey, () => { + this.save({ + ...configSchema.parse({ + ...this.config, + }), + __internal__: { + migrations: { + version: this.getAppVersion(), + }, }, - }, - }).finally(() => { - this.saveCounter--; + }); }); } ensureSaved(): Promise | "alreadySaved" { - if (this.saveCounter === 0) { + if (!this.lock.isBusy(lockKey)) { return "alreadySaved"; } @@ -185,10 +187,11 @@ export abstract class BaseConfigManager { for (let i = 0; i < 100; i++) { // 他のスレッドに処理を譲る await new Promise((resolve) => setTimeout(resolve, 100)); - if (this.saveCounter === 0) { + if (!this.lock.isBusy(lockKey)) { return; } } + throw new Error("Failed to save config"); } private migrateHotkeySettings(data: ConfigType): ConfigType { @@ -233,4 +236,8 @@ export abstract class BaseConfigManager { hotkeySettings: migratedHotkeys, }; } + + protected getDefaultConfig(): ConfigType { + return configSchema.parse({}); + } } diff --git a/src/type/preload.ts b/src/type/preload.ts index 38fd98d827..24ab4913ed 100644 --- a/src/type/preload.ts +++ b/src/type/preload.ts @@ -596,6 +596,17 @@ export const configSchema = z.object({ }); export type ConfigType = z.infer; +export const envEngineInfoSchema = z.object({ + uuid: engineIdSchema, + host: z.string(), + name: z.string(), + executionEnabled: z.boolean(), + executionFilePath: z.string(), + executionArgs: z.array(z.string()), + path: z.string().optional(), +}); +export type EnvEngineInfo = z.infer; + // workaround. SystemError(https://nodejs.org/api/errors.html#class-systemerror)が2022/05/19時点ではNodeJSの型定義に記述されていないためこれを追加しています。 export class SystemError extends Error { code?: string | undefined; diff --git "a/tests/e2e/browser/\343\202\242\343\202\257\343\202\273\343\203\263\343\203\210.spec.ts" "b/tests/e2e/browser/\343\202\242\343\202\257\343\202\273\343\203\263\343\203\210.spec.ts" index 45f3386d88..4852e41cc2 100644 --- "a/tests/e2e/browser/\343\202\242\343\202\257\343\202\273\343\203\263\343\203\210.spec.ts" +++ "b/tests/e2e/browser/\343\202\242\343\202\257\343\202\273\343\203\263\343\203\210.spec.ts" @@ -9,9 +9,9 @@ test("アクセント分割したらアクセント区間が増える", async ({ await expect(page.locator(".audio-cell").first()).toBeVisible(); await page.locator(".audio-cell input").first().fill("こんにちは"); await page.locator(".audio-cell input").first().press("Enter"); - await page.waitForTimeout(100); + await page.waitForTimeout(500); expect(await page.locator(".mora-table").count()).toBe(1); await (await page.locator(".splitter-cell").all())[1].click(); - await page.waitForTimeout(100); + await page.waitForTimeout(500); expect(await page.locator(".mora-table").count()).toBe(2); }); diff --git a/tests/e2e/electron/example.spec.ts b/tests/e2e/electron/example.spec.ts index 853b847a24..21e057d363 100644 --- a/tests/e2e/electron/example.spec.ts +++ b/tests/e2e/electron/example.spec.ts @@ -56,6 +56,8 @@ test("起動したら「利用規約に関するお知らせ」が表示され }); // エンジンが起動し「利用規約に関するお知らせ」が表示されるのを待つ - await sut.waitForSelector("text=利用規約に関するお知らせ", { timeout: 0 }); + await sut.waitForSelector("text=利用規約に関するお知らせ", { + timeout: 60000, + }); await app.close(); }); diff --git a/tests/e2e/navigators.ts b/tests/e2e/navigators.ts index f71281fec8..1559538500 100644 --- a/tests/e2e/navigators.ts +++ b/tests/e2e/navigators.ts @@ -32,8 +32,6 @@ export async function navigateToMain(page: Page) { export async function toggleSetting(page: Page, settingName: string) { await page.getByRole("button", { name: "設定" }).click(); await page.waitForTimeout(100); - // FIXME: なぜかariaで取得できない - // await page.getByRole("listitem", { name: "オプション" }).click(); await page.getByText("オプション").click(); await page.waitForTimeout(100); await page