From 02e8b55884fd530dee03613d79adad8aa9180b61 Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Mon, 3 Feb 2020 12:11:17 +0900 Subject: Rename setting repositories and refactor --- src/background/usecases/CommandUseCase.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/background/usecases/CommandUseCase.ts') diff --git a/src/background/usecases/CommandUseCase.ts b/src/background/usecases/CommandUseCase.ts index fcb898d..20ded7e 100644 --- a/src/background/usecases/CommandUseCase.ts +++ b/src/background/usecases/CommandUseCase.ts @@ -5,7 +5,7 @@ import * as urls from '../../shared/urls'; import TabPresenter from '../presenters/TabPresenter'; import WindowPresenter from '../presenters/WindowPresenter'; import HelpPresenter from '../presenters/HelpPresenter'; -import SettingRepository from '../repositories/SettingRepository'; +import CachedSettingRepository from '../repositories/CachedSettingRepository'; import BookmarkRepository from '../repositories/BookmarkRepository'; import ConsoleClient from '../infrastructures/ConsoleClient'; import ContentMessageClient from '../infrastructures/ContentMessageClient'; @@ -17,7 +17,7 @@ export default class CommandIndicator { private tabPresenter: TabPresenter, private windowPresenter: WindowPresenter, private helpPresenter: HelpPresenter, - private settingRepository: SettingRepository, + private settingRepository: CachedSettingRepository, private bookmarkRepository: BookmarkRepository, private consoleClient: ConsoleClient, private contentMessageClient: ContentMessageClient, -- cgit v1.2.3 From b2fc46ebf79ebb1ffa068fb513d1eeb9b50d7b3f Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Sun, 9 Feb 2020 11:13:55 +0900 Subject: Add SettingUseCase tests --- src/background/Application.ts | 6 +- src/background/di.ts | 11 ++ src/background/index.ts | 1 + src/background/presenters/Notifier.ts | 53 +++++++ src/background/presenters/NotifyPresenter.ts | 50 ------- .../repositories/CachedSettingRepository.ts | 21 ++- .../repositories/LocalSettingRepository.ts | 24 --- src/background/repositories/SettingRepository.ts | 49 +++++++ .../repositories/SyncSettingRepository.ts | 25 ---- src/background/usecases/CommandUseCase.ts | 8 +- src/background/usecases/CompletionsUseCase.ts | 8 +- src/background/usecases/SettingUseCase.ts | 21 ++- src/background/usecases/VersionUseCase.ts | 8 +- test/background/usecases/SettingUseCase.test.ts | 161 +++++++++++++++++++++ 14 files changed, 315 insertions(+), 131 deletions(-) create mode 100644 src/background/di.ts create mode 100644 src/background/presenters/Notifier.ts delete mode 100644 src/background/presenters/NotifyPresenter.ts delete mode 100644 src/background/repositories/LocalSettingRepository.ts create mode 100644 src/background/repositories/SettingRepository.ts delete mode 100644 src/background/repositories/SyncSettingRepository.ts create mode 100644 test/background/usecases/SettingUseCase.test.ts (limited to 'src/background/usecases/CommandUseCase.ts') diff --git a/src/background/Application.ts b/src/background/Application.ts index da6bdfc..c2c48b5 100644 --- a/src/background/Application.ts +++ b/src/background/Application.ts @@ -1,8 +1,8 @@ -import { injectable } from 'tsyringe'; +import { injectable, inject } from 'tsyringe'; import ContentMessageListener from './infrastructures/ContentMessageListener'; import SettingController from './controllers/SettingController'; import VersionController from './controllers/VersionController'; -import SyncSettingRepository from "./repositories/SyncSettingRepository"; +import SettingRepository from "./repositories/SettingRepository"; @injectable() export default class Application { @@ -10,7 +10,7 @@ export default class Application { private contentMessageListener: ContentMessageListener, private settingController: SettingController, private versionController: VersionController, - private syncSettingRepository: SyncSettingRepository + @inject("SyncSettingRepository") private syncSettingRepository: SettingRepository, ) { } diff --git a/src/background/di.ts b/src/background/di.ts new file mode 100644 index 0000000..9fc230c --- /dev/null +++ b/src/background/di.ts @@ -0,0 +1,11 @@ +/* eslint-disable max-len */ + +import { LocalSettingRepository, SyncSettingRepository } from "./repositories/SettingRepository"; +import { NotifierImpl } from "./presenters/Notifier"; +import { CachedSettingRepositoryImpl } from "./repositories/CachedSettingRepository"; +import { container } from 'tsyringe'; + +container.register('LocalSettingRepository', { useValue: LocalSettingRepository }); +container.register('SyncSettingRepository', { useClass: SyncSettingRepository }); +container.register('CachedSettingRepository', { useClass: CachedSettingRepositoryImpl }); +container.register('Notifier', { useClass: NotifierImpl }); diff --git a/src/background/index.ts b/src/background/index.ts index 51fde56..f75c53b 100644 --- a/src/background/index.ts +++ b/src/background/index.ts @@ -1,6 +1,7 @@ import 'reflect-metadata'; import { container } from 'tsyringe'; import Application from './Application'; +import './di'; const app = container.resolve(Application); app.run(); diff --git a/src/background/presenters/Notifier.ts b/src/background/presenters/Notifier.ts new file mode 100644 index 0000000..57d58cb --- /dev/null +++ b/src/background/presenters/Notifier.ts @@ -0,0 +1,53 @@ +const NOTIFICATION_ID_UPDATE = 'vimvixen-update'; +const NOTIFICATION_ID_INVALID_SETTINGS = 'vimvixen-update-invalid-settings'; + +export default interface Notifier { + notifyUpdated(version: string, onclick: () => void): Promise; + + notifyInvalidSettings(onclick: () => void): Promise; +} + +export class NotifierImpl implements NotifierImpl { + async notifyUpdated(version: string, onclick: () => void): Promise { + const title = `Vim Vixen ${version} has been installed`; + const message = 'Click here to see release notes'; + + const listener = (id: string) => { + if (id !== NOTIFICATION_ID_UPDATE) { + return; + } + onclick(); + browser.notifications.onClicked.removeListener(listener); + }; + browser.notifications.onClicked.addListener(listener); + + await browser.notifications.create(NOTIFICATION_ID_UPDATE, { + 'type': 'basic', + 'iconUrl': browser.extension.getURL('resources/icon_48x48.png'), + title, + message, + }); + } + + async notifyInvalidSettings(onclick: () => void): Promise { + const title = `Loaded settings is invalid`; + // eslint-disable-next-line max-len + const message = 'The default settings is used due to the last saved settings is invalid. Check your current settings from the add-on preference'; + + const listener = (id: string) => { + if (id !== NOTIFICATION_ID_INVALID_SETTINGS) { + return; + } + onclick(); + browser.notifications.onClicked.removeListener(listener); + }; + browser.notifications.onClicked.addListener(listener); + + await browser.notifications.create(NOTIFICATION_ID_INVALID_SETTINGS, { + 'type': 'basic', + 'iconUrl': browser.extension.getURL('resources/icon_48x48.png'), + title, + message, + }); + } +} diff --git a/src/background/presenters/NotifyPresenter.ts b/src/background/presenters/NotifyPresenter.ts deleted file mode 100644 index b7f4f99..0000000 --- a/src/background/presenters/NotifyPresenter.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { injectable } from 'tsyringe'; - -const NOTIFICATION_ID_UPDATE = 'vimvixen-update'; -const NOTIFICATION_ID_INVALID_SETTINGS = 'vimvixen-update-invalid-settings'; - -@injectable() -export default class NotifyPresenter { - async notifyUpdated(version: string, onclick: () => void): Promise { - const title = `Vim Vixen ${version} has been installed`; - const message = 'Click here to see release notes'; - - const listener = (id: string) => { - if (id !== NOTIFICATION_ID_UPDATE) { - return; - } - onclick(); - browser.notifications.onClicked.removeListener(listener); - }; - browser.notifications.onClicked.addListener(listener); - - await browser.notifications.create(NOTIFICATION_ID_UPDATE, { - 'type': 'basic', - 'iconUrl': browser.extension.getURL('resources/icon_48x48.png'), - title, - message, - }); - } - - async notifyInvalidSettings(onclick: () => void): Promise { - const title = `Loaded settings is invalid`; - // eslint-disable-next-line max-len - const message = 'The default settings is used due to the last saved settings is invalid. Check your current settings from the add-on preference'; - - const listener = (id: string) => { - if (id !== NOTIFICATION_ID_INVALID_SETTINGS) { - return; - } - onclick(); - browser.notifications.onClicked.removeListener(listener); - }; - browser.notifications.onClicked.addListener(listener); - - await browser.notifications.create(NOTIFICATION_ID_INVALID_SETTINGS, { - 'type': 'basic', - 'iconUrl': browser.extension.getURL('resources/icon_48x48.png'), - title, - message, - }); - } -} diff --git a/src/background/repositories/CachedSettingRepository.ts b/src/background/repositories/CachedSettingRepository.ts index b73d94a..1af15d4 100644 --- a/src/background/repositories/CachedSettingRepository.ts +++ b/src/background/repositories/CachedSettingRepository.ts @@ -1,12 +1,20 @@ -import { injectable } from 'tsyringe'; import MemoryStorage from '../infrastructures/MemoryStorage'; import Settings from '../../shared/settings/Settings'; import Properties from '../../shared/settings/Properties'; const CACHED_SETTING_KEY = 'setting'; -@injectable() -export default class CachedSettingRepository { +export default interface CachedSettingRepository { + get(): Promise; + + update(value: Settings): Promise; + + setProperty( + name: string, value: string | number | boolean, + ): Promise; +} + +export class CachedSettingRepositoryImpl implements CachedSettingRepository { private cache: MemoryStorage; constructor() { @@ -18,8 +26,9 @@ export default class CachedSettingRepository { return Promise.resolve(Settings.fromJSON(data)); } - update(value: Settings): void { - return this.cache.set(CACHED_SETTING_KEY, value.toJSON()); + update(value: Settings): Promise { + this.cache.set(CACHED_SETTING_KEY, value.toJSON()); + return Promise.resolve() } async setProperty( @@ -49,6 +58,6 @@ export default class CachedSettingRepository { current.properties.complete = newValue as string; break; } - return this.update(current); + await this.update(current); } } diff --git a/src/background/repositories/LocalSettingRepository.ts b/src/background/repositories/LocalSettingRepository.ts deleted file mode 100644 index 0c9ef3b..0000000 --- a/src/background/repositories/LocalSettingRepository.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { injectable } from 'tsyringe'; -import SettingData from '../../shared/SettingData'; - -@injectable() -export default class LocalSettingRepository { - async load(): Promise { - const {settings} = await browser.storage.local.get('settings'); - if (!settings) { - return null; - } - return SettingData.fromJSON(settings as any); - } - - onChange(callback: () => void) { - browser.storage.onChanged.addListener((changes, area) => { - if (area !== 'local') { - return; - } - if (changes.settings) { - callback(); - } - }); - } -} diff --git a/src/background/repositories/SettingRepository.ts b/src/background/repositories/SettingRepository.ts new file mode 100644 index 0000000..b522045 --- /dev/null +++ b/src/background/repositories/SettingRepository.ts @@ -0,0 +1,49 @@ +import SettingData from '../../shared/SettingData'; + +export default interface SettingRepository { + load(): Promise; + + onChange(callback: () => void): void; +} + +export class LocalSettingRepository implements SettingRepository { + async load(): Promise { + const {settings} = await browser.storage.local.get('settings'); + if (!settings) { + return null; + } + return SettingData.fromJSON(settings as any); + } + + onChange(callback: () => void) { + browser.storage.onChanged.addListener((changes, area) => { + if (area !== 'local') { + return; + } + if (changes.settings) { + callback(); + } + }); + } +} + +export class SyncSettingRepository implements SettingRepository { + async load(): Promise { + const {settings} = await browser.storage.sync.get('settings'); + if (!settings) { + return null; + } + return SettingData.fromJSON(settings as any); + } + + onChange(callback: () => void) { + browser.storage.onChanged.addListener((changes, area) => { + if (area !== 'sync') { + return; + } + if (changes.settings) { + callback(); + } + }); + } +} \ No newline at end of file diff --git a/src/background/repositories/SyncSettingRepository.ts b/src/background/repositories/SyncSettingRepository.ts deleted file mode 100644 index 9f59e61..0000000 --- a/src/background/repositories/SyncSettingRepository.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { injectable } from 'tsyringe'; -import SettingData from '../../shared/SettingData'; - -@injectable() -export default class SyncSettingRepository { - async load(): Promise { - const { settings } = await browser.storage.sync.get('settings'); - if (!settings) { - return null; - } - return SettingData.fromJSON(settings as any); - } - - onChange(callback: () => void) { - browser.storage.onChanged.addListener((changes, area) => { - if (area !== 'sync') { - return; - } - if (changes.settings) { - callback(); - } - }); - } -} - diff --git a/src/background/usecases/CommandUseCase.ts b/src/background/usecases/CommandUseCase.ts index 20ded7e..7dba664 100644 --- a/src/background/usecases/CommandUseCase.ts +++ b/src/background/usecases/CommandUseCase.ts @@ -1,4 +1,4 @@ -import { injectable } from 'tsyringe'; +import { injectable, inject } from 'tsyringe'; import * as operations from '../../shared/operations'; import * as parsers from './parsers'; import * as urls from '../../shared/urls'; @@ -17,7 +17,7 @@ export default class CommandIndicator { private tabPresenter: TabPresenter, private windowPresenter: WindowPresenter, private helpPresenter: HelpPresenter, - private settingRepository: CachedSettingRepository, + @inject("CachedSettingRepository") private cachedSettingRepository: CachedSettingRepository, private bookmarkRepository: BookmarkRepository, private consoleClient: ConsoleClient, private contentMessageClient: ContentMessageClient, @@ -133,7 +133,7 @@ export default class CommandIndicator { return; } const [name, value] = parsers.parseSetOption(keywords); - await this.settingRepository.setProperty(name, value); + await this.cachedSettingRepository.setProperty(name, value); return this.contentMessageClient.broadcastSettingsChanged(); } @@ -143,7 +143,7 @@ export default class CommandIndicator { } private async urlOrSearch(keywords: string): Promise { - const settings = await this.settingRepository.get(); + const settings = await this.cachedSettingRepository.get(); return urls.searchUrl(keywords, settings.search); } } diff --git a/src/background/usecases/CompletionsUseCase.ts b/src/background/usecases/CompletionsUseCase.ts index 72ba929..9874644 100644 --- a/src/background/usecases/CompletionsUseCase.ts +++ b/src/background/usecases/CompletionsUseCase.ts @@ -1,4 +1,4 @@ -import { injectable } from 'tsyringe'; +import { injectable, inject } from 'tsyringe'; import CompletionGroup from '../domains/CompletionGroup'; import CommandDocs from '../domains/CommandDocs'; import CompletionsRepository from '../repositories/CompletionsRepository'; @@ -17,7 +17,7 @@ export default class CompletionsUseCase { constructor( private tabPresenter: TabPresenter, private completionsRepository: CompletionsRepository, - private settingRepository: CachedSettingRepository, + @inject("CachedSettingRepository") private cachedSettingRepository: CachedSettingRepository, ) { } @@ -41,7 +41,7 @@ export default class CompletionsUseCase { // TODO This logic contains view entities. They should be defined on // content script - const settings = await this.settingRepository.get(); + const settings = await this.cachedSettingRepository.get(); const groups: CompletionGroup[] = []; const complete = settings.properties.complete; @@ -180,7 +180,7 @@ export default class CompletionsUseCase { } async querySearchEngineItems(name: string, keywords: string) { - const settings = await this.settingRepository.get(); + const settings = await this.cachedSettingRepository.get(); const engines = Object.keys(settings.search.engines) .filter(key => key.startsWith(keywords)); return engines.map(key => ({ diff --git a/src/background/usecases/SettingUseCase.ts b/src/background/usecases/SettingUseCase.ts index 02e1240..69b4572 100644 --- a/src/background/usecases/SettingUseCase.ts +++ b/src/background/usecases/SettingUseCase.ts @@ -1,19 +1,18 @@ -import { injectable } from 'tsyringe'; -import LocalSettingRepository from '../repositories/LocalSettingRepository'; +import {inject, injectable} from 'tsyringe'; import CachedSettingRepository from '../repositories/CachedSettingRepository'; -import SettingData, { DefaultSettingData } from '../../shared/SettingData'; +import SettingData, {DefaultSettingData} from '../../shared/SettingData'; import Settings from '../../shared/settings/Settings'; -import NotifyPresenter from '../presenters/NotifyPresenter'; -import SyncSettingRepository from "../repositories/SyncSettingRepository"; +import Notifier from '../presenters/Notifier'; +import SettingRepository from "../repositories/SettingRepository"; @injectable() export default class SettingUseCase { constructor( - private localSettingRepository: LocalSettingRepository, - private syncSettingRepository: SyncSettingRepository, - private cachedSettingRepository: CachedSettingRepository, - private notifyPresenter: NotifyPresenter, + @inject("LocalSettingRepository") private localSettingRepository: SettingRepository, + @inject("SyncSettingRepository") private syncSettingRepository: SettingRepository, + @inject("CachedSettingRepository") private cachedSettingRepository: CachedSettingRepository, + @inject("Notifier") private notifier: Notifier, ) { } @@ -36,7 +35,7 @@ export default class SettingUseCase { this.showUnableToLoad(e); value = DefaultSettingData.toSettings(); } - this.cachedSettingRepository.update(value!!); + await this.cachedSettingRepository.update(value!!); return value; } @@ -54,7 +53,7 @@ export default class SettingUseCase { private showUnableToLoad(e: Error) { console.error('unable to load settings', e); - this.notifyPresenter.notifyInvalidSettings(() => { + this.notifier.notifyInvalidSettings(() => { browser.runtime.openOptionsPage(); }); } diff --git a/src/background/usecases/VersionUseCase.ts b/src/background/usecases/VersionUseCase.ts index 645c859..9ea8af9 100644 --- a/src/background/usecases/VersionUseCase.ts +++ b/src/background/usecases/VersionUseCase.ts @@ -1,19 +1,19 @@ -import { injectable } from 'tsyringe'; +import { injectable, inject } from 'tsyringe'; import TabPresenter from '../presenters/TabPresenter'; -import NotifyPresenter from '../presenters/NotifyPresenter'; +import Notifier from '../presenters/Notifier'; @injectable() export default class VersionUseCase { constructor( private tabPresenter: TabPresenter, - private notifyPresenter: NotifyPresenter, + @inject("Notifier") private notifier: Notifier, ) { } notify(): Promise { const manifest = browser.runtime.getManifest(); const url = this.releaseNoteUrl(manifest.version); - return this.notifyPresenter.notifyUpdated(manifest.version, () => { + return this.notifier.notifyUpdated(manifest.version, () => { this.tabPresenter.create(url); }); } diff --git a/test/background/usecases/SettingUseCase.test.ts b/test/background/usecases/SettingUseCase.test.ts new file mode 100644 index 0000000..bfa599c --- /dev/null +++ b/test/background/usecases/SettingUseCase.test.ts @@ -0,0 +1,161 @@ +import "reflect-metadata"; +import SettingUseCase from "../../../src/background/usecases/SettingUseCase"; +import SettingRepository from "../../../src/background/repositories/SettingRepository"; +import SettingData, {JSONTextSettings} from "../../../src/shared/SettingData"; +import CachedSettingRepository from "../../../src/background/repositories/CachedSettingRepository"; +import Settings, {DefaultSetting} from "../../../src/shared/settings/Settings"; +import Notifier from "../../../src/background/presenters/Notifier"; +import {expect} from "chai"; +import Properties from "../../../src/shared/settings/Properties"; +import sinon from 'sinon'; + +class MockSettingRepository implements SettingRepository { + load(): Promise { + throw new Error("not implemented"); + } + + onChange(_: () => void): void { + } +} + +class MockCachedSettingRepository implements CachedSettingRepository { + private current: Settings = DefaultSetting; + + get(): Promise { + return Promise.resolve(this.current); + } + + setProperty(name: string, value: string | number | boolean): Promise { + (this.current.properties as any)[name] = value; + return Promise.resolve(); + } + + update(value: Settings): Promise { + this.current = value; + return Promise.resolve(); + } +} + +class NopNotifier implements Notifier { + notifyInvalidSettings(_onclick: () => void): Promise { + return Promise.resolve(); + } + + notifyUpdated(_version: string, _onclick: () => void): Promise { + return Promise.resolve(); + } +} + +describe('SettingUseCase', () => { + let localSettingRepository : SettingRepository; + let syncSettingRepository : SettingRepository; + let cachedSettingRepository : CachedSettingRepository; + let notifier: Notifier; + let sut : SettingUseCase; + + beforeEach(() => { + localSettingRepository = new MockSettingRepository(); + syncSettingRepository = new MockSettingRepository(); + cachedSettingRepository = new MockCachedSettingRepository(); + notifier = new NopNotifier(); + sut = new SettingUseCase( + localSettingRepository, + syncSettingRepository, + cachedSettingRepository, + notifier + ); + }); + + describe('getCached', () => { + it("returns cached settings", async () => { + const settings = new Settings({ + keymaps: DefaultSetting.keymaps, + search: DefaultSetting.search, + blacklist: DefaultSetting.blacklist, + properties: new Properties({ + hintchars: "abcd1234" + }), + }); + sinon.stub(cachedSettingRepository, "get") + .returns(Promise.resolve(settings)); + + const got = await sut.getCached(); + expect(got.properties.hintchars).to.equal("abcd1234"); + + }); + }); + + describe("reload", () => { + context("when sync is not set", () => { + it("loads settings from local storage", async() => { + const settings = new Settings({ + keymaps: DefaultSetting.keymaps, + search: DefaultSetting.search, + blacklist: DefaultSetting.blacklist, + properties: new Properties({ + hintchars: "abcd1234" + }), + }); + const settingData = SettingData.fromJSON({ + source: "json", + json: JSONTextSettings.fromSettings(settings).toJSONText(), + }); + + sinon.stub(syncSettingRepository, "load") + .returns(Promise.resolve(null)); + sinon.stub(localSettingRepository, "load") + .returns(Promise.resolve(settingData)); + + await sut.reload(); + + const current = await cachedSettingRepository.get(); + expect(current.properties.hintchars).to.equal("abcd1234"); + }); + }); + + context("when local is not set", () => { + it("loads settings from sync storage", async() => { + const settings = new Settings({ + keymaps: DefaultSetting.keymaps, + search: DefaultSetting.search, + blacklist: DefaultSetting.blacklist, + properties: new Properties({ + hintchars: "aaaa1111" + }), + }); + const settingData = SettingData.fromJSON({ + source: "json", + json: JSONTextSettings.fromSettings(settings).toJSONText(), + }); + + sinon.stub(syncSettingRepository, "load") + .returns(Promise.resolve(settingData)); + sinon.stub(localSettingRepository, "load") + .returns(Promise.resolve(null)); + + await sut.reload(); + + const current = await cachedSettingRepository.get(); + expect(current.properties.hintchars).to.equal("aaaa1111"); + }); + }); + + context("neither local nor sync not set", () => { + it("loads default settings", async() => { + it("loads settings from sync storage", async() => { + sinon.stub(syncSettingRepository, "load") + .returns(Promise.resolve(null)); + sinon.stub(localSettingRepository, "load") + .returns(Promise.resolve(null)); + + await sut.reload(); + + const current = await cachedSettingRepository.get(); + expect(current.properties.hintchars).to.equal(DefaultSetting.properties.hintchars); + }); + + }) + }) + }) +}); + -- cgit v1.2.3