From e76ca380f733b515c31297a285d8bea44e074a1b Mon Sep 17 00:00:00 2001 From: Shin'ya Ueoka Date: Fri, 10 May 2019 22:27:20 +0900 Subject: [PATCH] Make addon-enabled as a clean architecture --- .../infrastructures/ContentMessageClient.ts | 6 +- src/content/MessageListener.ts | 2 +- src/content/actions/addon.ts | 19 ---- src/content/actions/index.ts | 10 --- src/content/actions/operation.ts | 19 ++-- src/content/client/AddonIndicatorClient.ts | 16 ++++ src/content/components/common/index.ts | 14 ++- src/content/components/common/keymapper.ts | 8 +- src/content/components/top-content/index.ts | 13 ++- src/content/reducers/addon.ts | 22 ----- src/content/reducers/index.ts | 4 +- .../repositories/AddonEnabledRepository.ts | 19 ++++ src/content/usecases/AddonEnabledUseCase.ts | 40 +++++++++ test/content/reducers/addon.test.ts | 17 ---- .../AddonEnabledRepository.test.ts | 15 ++++ .../usecases/AddonEnabledUseCase.test.ts | 90 +++++++++++++++++++ 16 files changed, 218 insertions(+), 96 deletions(-) delete mode 100644 src/content/actions/addon.ts create mode 100644 src/content/client/AddonIndicatorClient.ts delete mode 100644 src/content/reducers/addon.ts create mode 100644 src/content/repositories/AddonEnabledRepository.ts create mode 100644 src/content/usecases/AddonEnabledUseCase.ts delete mode 100644 test/content/reducers/addon.test.ts create mode 100644 test/content/repositories/AddonEnabledRepository.test.ts create mode 100644 test/content/usecases/AddonEnabledUseCase.test.ts diff --git a/src/background/infrastructures/ContentMessageClient.ts b/src/background/infrastructures/ContentMessageClient.ts index d4bc476..2215330 100644 --- a/src/background/infrastructures/ContentMessageClient.ts +++ b/src/background/infrastructures/ContentMessageClient.ts @@ -14,10 +14,10 @@ export default class ContentMessageClient { } async getAddonEnabled(tabId: number): Promise { - let { enabled } = await browser.tabs.sendMessage(tabId, { + let enabled = await browser.tabs.sendMessage(tabId, { type: messages.ADDON_ENABLED_QUERY, - }) as { enabled: boolean }; - return enabled; + }); + return enabled as any as boolean; } toggleAddonEnabled(tabId: number): Promise { diff --git a/src/content/MessageListener.ts b/src/content/MessageListener.ts index 105d028..1d7a479 100644 --- a/src/content/MessageListener.ts +++ b/src/content/MessageListener.ts @@ -25,7 +25,7 @@ export default class MessageListener { ) { browser.runtime.onMessage.addListener( (msg: any, sender: WebExtMessageSender) => { - listener(valueOf(msg), sender); + return listener(valueOf(msg), sender); }, ); } diff --git a/src/content/actions/addon.ts b/src/content/actions/addon.ts deleted file mode 100644 index 8dedae0..0000000 --- a/src/content/actions/addon.ts +++ /dev/null @@ -1,19 +0,0 @@ -import * as messages from '../../shared/messages'; -import * as actions from './index'; - -const enable = (): Promise => setEnabled(true); - -const disable = (): Promise => setEnabled(false); - -const setEnabled = async(enabled: boolean): Promise => { - await browser.runtime.sendMessage({ - type: messages.ADDON_ENABLED_RESPONSE, - enabled, - }); - return { - type: actions.ADDON_SET_ENABLED, - enabled, - }; -}; - -export { enable, disable, setEnabled }; diff --git a/src/content/actions/index.ts b/src/content/actions/index.ts index 8aa9c23..74353fb 100644 --- a/src/content/actions/index.ts +++ b/src/content/actions/index.ts @@ -2,9 +2,6 @@ import Redux from 'redux'; import Settings from '../../shared/Settings'; import * as keyUtils from '../../shared/utils/keys'; -// Enable/disable -export const ADDON_SET_ENABLED = 'addon.set.enabled'; - // Find export const FIND_SET_KEYWORD = 'find.set.keyword'; @@ -34,11 +31,6 @@ export const MARK_SET_LOCAL = 'mark.set.local'; export const NOOP = 'noop'; -export interface AddonSetEnabledAction extends Redux.Action { - type: typeof ADDON_SET_ENABLED; - enabled: boolean; -} - export interface FindSetKeywordAction extends Redux.Action { type: typeof FIND_SET_KEYWORD; keyword: string; @@ -101,7 +93,6 @@ export interface NoopAction extends Redux.Action { type: typeof NOOP; } -export type AddonAction = AddonSetEnabledAction; export type FindAction = FindSetKeywordAction | NoopAction; export type SettingAction = SettingSetAction; export type InputAction = InputKeyPressAction | InputClearKeysAction; @@ -113,7 +104,6 @@ export type MarkAction = MarkCancelAction | MarkSetLocalAction | NoopAction; export type Action = - AddonAction | FindAction | SettingAction | InputAction | diff --git a/src/content/actions/operation.ts b/src/content/actions/operation.ts index 41e080b..949f69f 100644 --- a/src/content/actions/operation.ts +++ b/src/content/actions/operation.ts @@ -6,23 +6,28 @@ import * as navigates from '../navigates'; import * as focuses from '../focuses'; import * as urls from '../urls'; import * as consoleFrames from '../console-frames'; -import * as addonActions from './addon'; import * as markActions from './mark'; +import AddonEnabledUseCase from '../usecases/AddonEnabledUseCase'; + +let addonEnabledUseCase = new AddonEnabledUseCase(); + // eslint-disable-next-line complexity, max-lines-per-function -const exec = ( +const exec = async( operation: operations.Operation, settings: any, - addonEnabled: boolean, -): Promise | actions.Action => { +): Promise => { let smoothscroll = settings.properties.smoothscroll; switch (operation.type) { case operations.ADDON_ENABLE: - return addonActions.enable(); + await addonEnabledUseCase.enable(); + return { type: actions.NOOP }; case operations.ADDON_DISABLE: - return addonActions.disable(); + await addonEnabledUseCase.disable(); + return { type: actions.NOOP }; case operations.ADDON_TOGGLE_ENABLED: - return addonActions.setEnabled(!addonEnabled); + await addonEnabledUseCase.toggle(); + return { type: actions.NOOP }; case operations.FIND_NEXT: window.top.postMessage(JSON.stringify({ type: messages.FIND_NEXT, diff --git a/src/content/client/AddonIndicatorClient.ts b/src/content/client/AddonIndicatorClient.ts new file mode 100644 index 0000000..afb9fa4 --- /dev/null +++ b/src/content/client/AddonIndicatorClient.ts @@ -0,0 +1,16 @@ +import * as messages from '../../shared/messages'; + +export default interface AddonIndicatorClient { + setEnabled(enabled: boolean): Promise; + + // eslint-disable-next-line semi +} + +export class AddonIndicatorClientImpl implements AddonIndicatorClient { + setEnabled(enabled: boolean): Promise { + return browser.runtime.sendMessage({ + type: messages.ADDON_ENABLED_RESPONSE, + enabled, + }); + } +} diff --git a/src/content/components/common/index.ts b/src/content/components/common/index.ts index 5b097b6..be77812 100644 --- a/src/content/components/common/index.ts +++ b/src/content/components/common/index.ts @@ -5,11 +5,14 @@ import KeymapperComponent from './keymapper'; import * as settingActions from '../../actions/setting'; import * as messages from '../../../shared/messages'; import MessageListener from '../../MessageListener'; -import * as addonActions from '../../actions/addon'; import * as blacklists from '../../../shared/blacklists'; import * as keys from '../../../shared/utils/keys'; import * as actions from '../../actions'; +import AddonEnabledUseCase from '../../usecases/AddonEnabledUseCase'; + +let addonEnabledUseCase = new AddonEnabledUseCase(); + export default class Common { private win: Window; @@ -34,12 +37,11 @@ export default class Common { } onMessage(message: messages.Message) { - let { enabled } = this.store.getState().addon; switch (message.type) { case messages.SETTINGS_CHANGED: return this.reloadSettings(); case messages.ADDON_TOGGLE_ENABLED: - this.store.dispatch(addonActions.setEnabled(!enabled)); + addonEnabledUseCase.toggle(); } } @@ -50,7 +52,11 @@ export default class Common { let enabled = !blacklists.includes( action.settings.blacklist, this.win.location.href ); - this.store.dispatch(addonActions.setEnabled(enabled)); + if (enabled) { + addonEnabledUseCase.enable(); + } else { + addonEnabledUseCase.disable(); + } }); } catch (e) { // Sometime sendMessage fails when background script is not ready. diff --git a/src/content/components/common/keymapper.ts b/src/content/components/common/keymapper.ts index c94bae0..02579ec 100644 --- a/src/content/components/common/keymapper.ts +++ b/src/content/components/common/keymapper.ts @@ -3,6 +3,10 @@ import * as operationActions from '../../actions/operation'; import * as operations from '../../../shared/operations'; import * as keyUtils from '../../../shared/utils/keys'; +import AddonEnabledUseCase from '../../usecases/AddonEnabledUseCase'; + +let addonEnabledUseCase = new AddonEnabledUseCase(); + const mapStartsWith = ( mapping: keyUtils.Key[], keys: keyUtils.Key[], @@ -41,7 +45,7 @@ export default class KeymapperComponent { (mapping: keyUtils.Key[]) => { return mapStartsWith(mapping, input.keys); }); - if (!state.addon.enabled) { + if (!addonEnabledUseCase.getEnabled()) { // available keymaps are only ADDON_ENABLE and ADDON_TOGGLE_ENABLED if // the addon disabled matched = matched.filter((keys) => { @@ -59,7 +63,7 @@ export default class KeymapperComponent { } let operation = keymaps.get(matched[0]) as operations.Operation; let act = operationActions.exec( - operation, state.setting, state.addon.enabled + operation, state.setting, ); this.store.dispatch(act); this.store.dispatch(inputActions.clearKeys()); diff --git a/src/content/components/top-content/index.ts b/src/content/components/top-content/index.ts index ac95ea9..101edca 100644 --- a/src/content/components/top-content/index.ts +++ b/src/content/components/top-content/index.ts @@ -5,15 +5,15 @@ import * as consoleFrames from '../../console-frames'; import * as messages from '../../../shared/messages'; import MessageListener from '../../MessageListener'; import * as scrolls from '../../scrolls'; +import AddonEnabledUseCase from '../../usecases/AddonEnabledUseCase'; + +let addonEnabledUseCase = new AddonEnabledUseCase(); export default class TopContent { private win: Window; - private store: any; - constructor(win: Window, store: any) { this.win = win; - this.store = store; new CommonComponent(win, store); // eslint-disable-line no-new new FollowController(win, store); // eslint-disable-line no-new @@ -36,14 +36,11 @@ export default class TopContent { } onBackgroundMessage(message: messages.Message) { - let addonState = this.store.getState().addon; + let addonEnabled = addonEnabledUseCase.getEnabled(); switch (message.type) { case messages.ADDON_ENABLED_QUERY: - return Promise.resolve({ - type: messages.ADDON_ENABLED_RESPONSE, - enabled: addonState.enabled, - }); + return Promise.resolve(addonEnabled); case messages.TAB_SCROLL_TO: return scrolls.scrollTo(message.x, message.y, false); } diff --git a/src/content/reducers/addon.ts b/src/content/reducers/addon.ts deleted file mode 100644 index 2131228..0000000 --- a/src/content/reducers/addon.ts +++ /dev/null @@ -1,22 +0,0 @@ -import * as actions from '../actions'; - -export interface State { - enabled: boolean; -} - -const defaultState: State = { - enabled: true, -}; - -export default function reducer( - state: State = defaultState, - action: actions.AddonAction, -): State { - switch (action.type) { - case actions.ADDON_SET_ENABLED: - return { ...state, - enabled: action.enabled, }; - default: - return state; - } -} diff --git a/src/content/reducers/index.ts b/src/content/reducers/index.ts index fb5eb84..6f11512 100644 --- a/src/content/reducers/index.ts +++ b/src/content/reducers/index.ts @@ -1,5 +1,4 @@ import { combineReducers } from 'redux'; -import addon, { State as AddonState } from './addon'; import find, { State as FindState } from './find'; import setting, { State as SettingState } from './setting'; import input, { State as InputState } from './input'; @@ -8,7 +7,6 @@ import followController, { State as FollowControllerState } import mark, { State as MarkState } from './mark'; export interface State { - addon: AddonState; find: FindState; setting: SettingState; input: InputState; @@ -17,5 +15,5 @@ export interface State { } export default combineReducers({ - addon, find, setting, input, followController, mark, + find, setting, input, followController, mark, }); diff --git a/src/content/repositories/AddonEnabledRepository.ts b/src/content/repositories/AddonEnabledRepository.ts new file mode 100644 index 0000000..4eaabb1 --- /dev/null +++ b/src/content/repositories/AddonEnabledRepository.ts @@ -0,0 +1,19 @@ +let enabled: boolean = false; + +export default interface AddonEnabledRepository { + set(on: boolean): void; + + get(): boolean; + + // eslint-disable-next-line semi +} + +export class AddonEnabledRepositoryImpl implements AddonEnabledRepository { + set(on: boolean): void { + enabled = on; + } + + get(): boolean { + return enabled; + } +} diff --git a/src/content/usecases/AddonEnabledUseCase.ts b/src/content/usecases/AddonEnabledUseCase.ts new file mode 100644 index 0000000..e9ce0a6 --- /dev/null +++ b/src/content/usecases/AddonEnabledUseCase.ts @@ -0,0 +1,40 @@ +import AddonIndicatorClient, { AddonIndicatorClientImpl } + from '../client/AddonIndicatorClient'; +import AddonEnabledRepository, { AddonEnabledRepositoryImpl } + from '../repositories/AddonEnabledRepository'; + +export default class AddonEnabledUseCase { + private indicator: AddonIndicatorClient; + + private repository: AddonEnabledRepository; + + constructor({ + indicator = new AddonIndicatorClientImpl(), + repository = new AddonEnabledRepositoryImpl(), + } = {}) { + this.indicator = indicator; + this.repository = repository; + } + + async enable(): Promise { + await this.setEnabled(true); + } + + async disable(): Promise { + await this.setEnabled(false); + } + + async toggle(): Promise { + let current = this.repository.get(); + await this.setEnabled(!current); + } + + getEnabled(): boolean { + return this.repository.get(); + } + + private async setEnabled(on: boolean): Promise { + this.repository.set(on); + await this.indicator.setEnabled(on); + } +} diff --git a/test/content/reducers/addon.test.ts b/test/content/reducers/addon.test.ts deleted file mode 100644 index fb05244..0000000 --- a/test/content/reducers/addon.test.ts +++ /dev/null @@ -1,17 +0,0 @@ -import * as actions from 'content/actions'; -import addonReducer from 'content/reducers/addon'; - -describe("addon reducer", () => { - it('return the initial state', () => { - let state = addonReducer(undefined, {}); - expect(state).to.have.property('enabled', true); - }); - - it('return next state for ADDON_SET_ENABLED', () => { - let action = { type: actions.ADDON_SET_ENABLED, enabled: true }; - let prev = { enabled: false }; - let state = addonReducer(prev, action); - - expect(state.enabled).is.equal(true); - }); -}); diff --git a/test/content/repositories/AddonEnabledRepository.test.ts b/test/content/repositories/AddonEnabledRepository.test.ts new file mode 100644 index 0000000..3cea897 --- /dev/null +++ b/test/content/repositories/AddonEnabledRepository.test.ts @@ -0,0 +1,15 @@ +import { AddonEnabledRepositoryImpl } from '../../../src/content/repositories/AddonEnabledRepository'; +import { expect } from 'chai'; + +describe('AddonEnabledRepositoryImpl', () => { + it('updates and gets current value', () => { + let sut = new AddonEnabledRepositoryImpl(); + + sut.set(true); + expect(sut.get()).to.be.true; + + sut.set(false); + expect(sut.get()).to.be.false; + }); +}); + diff --git a/test/content/usecases/AddonEnabledUseCase.test.ts b/test/content/usecases/AddonEnabledUseCase.test.ts new file mode 100644 index 0000000..912bddf --- /dev/null +++ b/test/content/usecases/AddonEnabledUseCase.test.ts @@ -0,0 +1,90 @@ +import AddonEnabledRepository from '../../../src/content/repositories/AddonEnabledRepository'; +import AddonEnabledUseCase from '../../../src/content/usecases/AddonEnabledUseCase'; +import AddonIndicatorClient from '../../../src/content/client/AddonIndicatorClient'; +import { expect } from 'chai'; + +class MockAddonEnabledRepository implements AddonEnabledRepository { + private enabled: boolean; + + constructor(init: boolean) { + this.enabled = init; + } + + set(on: boolean): void { + this.enabled = on; + } + + get(): boolean { + return this.enabled; + } +} + +class MockAddonIndicatorClient implements AddonIndicatorClient { + public enabled: boolean; + + constructor(init: boolean) { + this.enabled = init; + } + + async setEnabled(enabled: boolean): Promise { + this.enabled = enabled; + return + } +} + +describe('AddonEnabledUseCase', () => { + let repository: MockAddonEnabledRepository; + let indicator: MockAddonIndicatorClient; + let sut: AddonEnabledUseCase; + + beforeEach(() => { + repository = new MockAddonEnabledRepository(true); + indicator = new MockAddonIndicatorClient(false); + sut = new AddonEnabledUseCase({ repository, indicator }); + }); + + describe('#enable', () => { + it('store and indicate as enabled', async() => { + await sut.enable(); + + expect(repository.get()).to.be.true; + expect(indicator.enabled).to.be.true; + }); + }); + + describe('#disable', async() => { + it('store and indicate as disabled', async() => { + await sut.disable(); + + expect(repository.get()).to.be.false; + expect(indicator.enabled).to.be.false; + }); + }); + + describe('#toggle', () => { + it('toggled enabled and disabled', async() => { + repository.set(true); + await sut.toggle(); + + expect(repository.get()).to.be.false; + expect(indicator.enabled).to.be.false; + + repository.set(false); + + await sut.toggle(); + + expect(repository.get()).to.be.true; + expect(indicator.enabled).to.be.true; + }); + }); + + describe('#getEnabled', () => { + it('returns current addon enabled', () => { + repository.set(true); + expect(sut.getEnabled()).to.be.true; + + repository.set(false); + expect(sut.getEnabled()).to.be.false; + }); + }); +});