Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Add settingObserver module
Browse files Browse the repository at this point in the history
Auditors: @bsclifton

Fix #4452

Test Plan: None, it'll be tested implicitly when testing regional adblock settings.  Covered by automated.
  • Loading branch information
bbondy committed Oct 3, 2016
1 parent 6353cd2 commit b27f6ad
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 11 deletions.
45 changes: 45 additions & 0 deletions app/browser/settingObserver.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const appStore = require('../../js/stores/appStore')
const Immutable = require('Immutable')
const getSetting = require('../../js/settings').getSetting
let lastSettings = Immutable.Map()
let watchedPrefs = []

/**
* Registers to get notifications when a preference changes.
* @param prefKey - The preference key to watch
* @param cb - The callback to call when changes occur
* @return a function which can be called by the caller to unregister itself
*/
const registerForPref = (prefKey, cb) => {
watchedPrefs.push({ prefKey, cb })
// Return a function which removes itself when called
return () => {
watchedPrefs = watchedPrefs.filter((pref) => prefKey !== pref.prefKey || cb !== pref.cb)
}
}

const init = () =>
appStore.addChangeListener(() => {
// Determine if we should check for changes
if (lastSettings !== appStore.getState().get('settings')) {
watchedPrefs.forEach((pref) => {
const prefValue = getSetting(pref.prefKey)
if (prefValue !== getSetting(pref.prefKey, lastSettings)) {
pref.cb(pref.prefKey, prefValue)
}
})
lastSettings = appStore.getState().get('settings')
}
})

const getWatchedPrefCount = () => watchedPrefs.length

module.exports = {
init,
registerForPref,
getWatchedPrefCount
}
46 changes: 46 additions & 0 deletions test/unit/app/browser/settingObserverTest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/* global describe, it, before, after, beforeEach */
const mockery = require('mockery')
const Immutable = require('immutable')
const assert = require('assert')

require('../../braveUnit')

describe('setting observer unit test', function () {
let appActions
let settingObserver
before(function () {
mockery.enable({
warnOnReplace: false,
warnOnUnregistered: false,
useCleanCache: true
})
mockery.registerMock('electron', require('../../lib/fakeElectron'))
process.type = 'browser'
appActions = require('../../../../js/actions/appActions')
settingObserver = require('../../../../app/browser/settingObserver')
settingObserver.init()
})

beforeEach(function () {
appActions.setState(Immutable.fromJS({}))
})

after(function () {
mockery.disable()
})

it('observes setting changes and unregisters', function (cb) {
const prefKey = 'test.pref'
const prefValue = 'value1'
assert.equal(settingObserver.getWatchedPrefCount(), 0)
const unregister = settingObserver.registerForPref(prefKey, (pref, val) => {
assert.equal(prefKey, pref)
assert.equal(prefValue, val)
unregister()
assert.equal(settingObserver.getWatchedPrefCount(), 0)
cb()
})
assert.equal(settingObserver.getWatchedPrefCount(), 1)
appActions.changeSetting(prefKey, 'value1')
})
})
16 changes: 16 additions & 0 deletions test/unit/lib/fakeElectron.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const fakeElectron = {
ipcMain: {
on: function () { }
},
remote: {
app: {
on: function () {
}
}
},
app: {
on: function () {
}
}
}
module.exports = fakeElectron
12 changes: 1 addition & 11 deletions test/unit/lib/menuUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,7 @@ describe('menuUtil', function () {
useCleanCache: true
})

const fakeElectron = {
ipcMain: {
on: function () { }
},
remote: {
app: { }
},
app: { }
}

mockery.registerMock('electron', fakeElectron)
mockery.registerMock('electron', require('./fakeElectron'))
menuUtil = require('../../../app/browser/lib/menuUtil')
})

Expand Down

2 comments on commit b27f6ad

@bbondy
Copy link
Member Author

@bbondy bbondy commented on b27f6ad Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw we can move this to common eventually but I only need it for browser for now so it was easier to write that way.

@bsclifton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bbondy this new settings observer looks like a great way to clean up this code 😄
https://github.com/brave/browser-laptop/blob/master/js/stores/appStore.js#L307

Also, GJ with pulling the electron mock out to a file- love seeing tests added or updated 👍

Please sign in to comment.