fix(mc): Rework CONFIGs to observe branches not individual prefs (#2757)

This commit is contained in:
Ed Lee 2017-06-23 11:10:59 -07:00
Родитель 3054fa458f
Коммит e2a6b49c1e
9 изменённых файлов: 233 добавлений и 205 удалений

Просмотреть файл

@ -19,83 +19,75 @@ const {TopSitesFeed} = Cu.import("resource://activity-stream/lib/TopSitesFeed.js
const REASON_ADDON_UNINSTALL = 6;
const PREFS_CONFIG = [
{
name: "default.sites",
const PREFS_CONFIG = new Map([
["default.sites", {
title: "Comma-separated list of default top sites to fill in behind visited sites",
value: "https://www.facebook.com/,https://www.youtube.com/,https://www.amazon.com/,https://www.yahoo.com/,https://www.ebay.com/,https://twitter.com/"
},
// When you add a feed pref here:
// 1. The pref should be prefixed with "feeds."
// 2. The init property should be a function that instantiates your Feed
{
name: "feeds.localization",
title: "Initialize strings and detect locale for Activity Stream",
value: true,
init: () => new LocalizationFeed()
},
{
name: "feeds.newtabinit",
title: "Sends a copy of the state to each new tab that is opened",
value: true,
init: () => new NewTabInit()
},
{
name: "feeds.places",
title: "Listens for and relays various Places-related events",
value: true,
init: () => new PlacesFeed()
},
{
name: "feeds.prefs",
title: "Preferences",
value: true,
init: () => new PrefsFeed(PREFS_CONFIG.map(pref => pref.name))
},
{
name: "feeds.telemetry",
title: "Relays telemetry-related actions to TelemetrySender",
value: true,
init: () => new TelemetryFeed()
},
{
name: "feeds.topsites",
title: "Queries places and gets metadata for Top Sites section",
value: true,
init: () => new TopSitesFeed()
},
{
name: "showSearch",
}],
["showSearch", {
title: "Show the Search bar on the New Tab page",
value: true
}],
["showTopSites", {
title: "Show the Top Sites section on the New Tab page",
value: true
}],
["telemetry", {
title: "Enable system error and usage data collection",
value: false
}],
["telemetry.log", {
title: "Log telemetry events in the console",
value: false
}],
["telemetry.ping.endpoint", {
title: "Telemetry server endpoint",
value: "https://tiles.services.mozilla.com/v3/links/activity-stream"
}]
]);
const FEEDS_CONFIG = new Map();
for (const {name, factory, title, value} of [
{
name: "localization",
factory: () => new LocalizationFeed(),
title: "Initialize strings and detect locale for Activity Stream",
value: true
},
{
name: "showTopSites",
title: "Show the Top Sites section on the New Tab page",
name: "newtabinit",
factory: () => new NewTabInit(),
title: "Sends a copy of the state to each new tab that is opened",
value: true
},
{
name: "places",
factory: () => new PlacesFeed(),
title: "Listens for and relays various Places-related events",
value: true
},
{
name: "prefs",
factory: () => new PrefsFeed(PREFS_CONFIG),
title: "Preferences",
value: true
},
{
name: "telemetry",
title: "Enable system error and usage data collection",
value: false
factory: () => new TelemetryFeed(),
title: "Relays telemetry-related actions to TelemetrySender",
value: true
},
{
name: "telemetry.log",
title: "Log telemetry events in the console",
value: false
},
{
name: "telemetry.ping.endpoint",
title: "Telemetry server endpoint",
value: "https://tiles.services.mozilla.com/v3/links/activity-stream"
}
];
const feeds = {};
for (const pref of PREFS_CONFIG) {
if (pref.name.startsWith("feeds.")) {
feeds[pref.name] = pref.init;
name: "topsites",
factory: () => new TopSitesFeed(),
title: "Queries places and gets metadata for Top Sites section",
value: true
}
]) {
const pref = `feeds.${name}`;
FEEDS_CONFIG.set(pref, factory);
PREFS_CONFIG.set(pref, {title, value});
}
this.ActivityStream = class ActivityStream {
@ -112,7 +104,7 @@ this.ActivityStream = class ActivityStream {
this.initialized = false;
this.options = options;
this.store = new Store();
this.feeds = feeds;
this.feeds = FEEDS_CONFIG;
this._defaultPrefs = new DefaultPrefs(PREFS_CONFIG);
}
init() {

Просмотреть файл

@ -18,10 +18,23 @@ this.Prefs = class Prefs extends Preferences {
constructor(branch = ACTIVITY_STREAM_PREF_BRANCH) {
super({branch});
this._branchName = branch;
this._branchObservers = new Map();
}
get branchName() {
return this._branchName;
}
ignoreBranch(listener) {
const observer = this._branchObservers.get(listener);
this._prefBranch.removeObserver(this._branchName, observer);
this._branchObservers.delete(listener);
}
observeBranch(listener) {
const observer = (subject, topic, pref) => {
listener.onPrefChanged(pref, this.get(pref));
};
this._prefBranch.addObserver("", observer);
this._branchObservers.set(listener, observer);
}
};
this.DefaultPrefs = class DefaultPrefs {
@ -29,8 +42,8 @@ this.DefaultPrefs = class DefaultPrefs {
/**
* DefaultPrefs - A helper for setting and resetting default prefs for the add-on
*
* @param {Array} config An array of configuration objects with the following properties:
* {string} .name The name of the pref
* @param {Map} config A Map with {string} key of the pref name and {object}
* value with the following pref properties:
* {string} .title (optional) A description of the pref
* {bool|string|number} .value The default value for the pref
* @param {string} branch (optional) The pref branch (defaults to ACTIVITY_STREAM_PREF_BRANCH)
@ -64,8 +77,8 @@ this.DefaultPrefs = class DefaultPrefs {
* init - Set default prefs for all prefs in the config
*/
init() {
for (const pref of this._config) {
this._setDefaultPref(pref.name, pref.value);
for (const pref of this._config.keys()) {
this._setDefaultPref(pref, this._config.get(pref).value);
}
}
@ -73,8 +86,8 @@ this.DefaultPrefs = class DefaultPrefs {
* reset - Resets all user-defined prefs for prefs in ._config to their defaults
*/
reset() {
for (const pref of this._config) {
this.branch.clearUserPref(pref.name);
for (const name of this._config.keys()) {
this.branch.clearUserPref(name);
}
}
};

Просмотреть файл

@ -9,24 +9,21 @@ const {actionCreators: ac, actionTypes: at} = Cu.import("resource://activity-str
const {Prefs} = Cu.import("resource://activity-stream/lib/ActivityStreamPrefs.jsm", {});
this.PrefsFeed = class PrefsFeed {
constructor(prefNames) {
this._prefNames = prefNames;
constructor(prefMap) {
this._prefMap = prefMap;
this._prefs = new Prefs();
this._observers = new Map();
}
onPrefChanged(name, value) {
this.store.dispatch(ac.BroadcastToContent({type: at.PREF_CHANGED, data: {name, value}}));
if (this._prefMap.has(name)) {
this.store.dispatch(ac.BroadcastToContent({type: at.PREF_CHANGED, data: {name, value}}));
}
}
init() {
const values = {};
this._prefs.observeBranch(this);
// Set up listeners for each activity stream pref
for (const name of this._prefNames) {
const handler = value => {
this.onPrefChanged(name, value);
};
this._observers.set(name, handler, this);
this._prefs.observe(name, handler);
// Get the initial value of each activity stream pref
const values = {};
for (const name of this._prefMap.keys()) {
values[name] = this._prefs.get(name);
}
@ -34,10 +31,7 @@ this.PrefsFeed = class PrefsFeed {
this.store.dispatch(ac.BroadcastToContent({type: at.PREFS_INITIAL_VALUES, data: values}));
}
removeListeners() {
for (const name of this._prefNames) {
this._prefs.ignore(name, this._observers.get(name));
}
this._observers.clear();
this._prefs.ignoreBranch(this);
}
onAction(action) {
switch (action.type) {

Просмотреть файл

@ -31,9 +31,7 @@ this.Store = class Store {
this[method] = (...args) => this._store[method](...args);
}
this.feeds = new Map();
this._feedFactories = null;
this._prefs = new Prefs();
this._prefHandlers = new Map();
this._messageChannel = new ActivityStreamMessageChannel({dispatch: this.dispatch});
this._store = redux.createStore(
redux.combineReducers(reducers),
@ -64,7 +62,7 @@ this.Store = class Store {
* passed to Store.init
*/
initFeed(feedName) {
const feed = this._feedFactories[feedName]();
const feed = this._feedFactories.get(feedName)();
feed.store = this;
this.feeds.set(feedName, feed);
}
@ -87,41 +85,34 @@ this.Store = class Store {
}
/**
* maybeStartFeedAndListenForPrefChanges - Listen for pref changes that turn a
* feed off/on, and as long as that pref was not explicitly set to
* false, initialize the feed immediately.
*
* @param {string} name The name of a feed, as defined in the object passed
* to Store.init
* onPrefChanged - Listener for handling feed changes.
*/
maybeStartFeedAndListenForPrefChanges(prefName) {
// Create a listener that turns the feed off/on based on changes
// to the pref, and cache it so we can unlisten on shut-down.
const onPrefChanged = isEnabled => (isEnabled ? this.initFeed(prefName) : this.uninitFeed(prefName));
this._prefHandlers.set(prefName, onPrefChanged);
this._prefs.observe(prefName, onPrefChanged);
// TODO: This should propbably be done in a generic pref manager for Activity Stream.
// If the pref is true, start the feed immediately.
if (this._prefs.get(prefName)) {
this.initFeed(prefName);
onPrefChanged(name, value) {
if (this._feedFactories.has(name)) {
if (value) {
this.initFeed(name);
} else {
this.uninitFeed(name);
}
}
}
/**
* init - Initializes the ActivityStreamMessageChannel channel, and adds feeds.
*
* @param {array} feedConstructors An array of configuration objects for feeds
* each with .name (the name of the pref for the feed) and .init,
* a function that returns an instance of the feed
* @param {Map} feedFactories A Map of feeds with the name of the pref for
* the feed as the key and a function that
* constructs an instance of the feed.
*/
init(feedConstructors) {
if (feedConstructors) {
this._feedFactories = feedConstructors;
for (const pref of Object.keys(feedConstructors)) {
this.maybeStartFeedAndListenForPrefChanges(pref);
init(feedFactories) {
this._feedFactories = feedFactories;
for (const pref of feedFactories.keys()) {
if (this._prefs.get(pref)) {
this.initFeed(pref);
}
}
this._prefs.observeBranch(this);
this._messageChannel.createChannel();
}
@ -133,10 +124,9 @@ this.Store = class Store {
*/
uninit() {
this.feeds.forEach(feed => this.uninitFeed(feed));
this._prefHandlers.forEach((handler, pref) => this._prefs.ignore(pref, handler));
this._prefHandlers.clear();
this._feedFactories = null;
this.feeds.clear();
this._feedFactories = null;
this._prefs.ignoreBranch(this);
this._messageChannel.destroyChannel();
}
};

Просмотреть файл

@ -83,27 +83,27 @@ describe("ActivityStream", () => {
});
describe("feeds", () => {
it("should create a Localization feed", () => {
const feed = as.feeds["feeds.localization"]();
const feed = as.feeds.get("feeds.localization")();
assert.instanceOf(feed, Fake);
});
it("should create a NewTabInit feed", () => {
const feed = as.feeds["feeds.newtabinit"]();
const feed = as.feeds.get("feeds.newtabinit")();
assert.instanceOf(feed, Fake);
});
it("should create a Places feed", () => {
const feed = as.feeds["feeds.places"]();
const feed = as.feeds.get("feeds.places")();
assert.instanceOf(feed, Fake);
});
it("should create a TopSites feed", () => {
const feed = as.feeds["feeds.topsites"]();
const feed = as.feeds.get("feeds.topsites")();
assert.instanceOf(feed, Fake);
});
it("should create a Telemetry feed", () => {
const feed = as.feeds["feeds.telemetry"]();
const feed = as.feeds.get("feeds.telemetry")();
assert.instanceOf(feed, Fake);
});
it("should create a Prefs feed", () => {
const feed = as.feeds["feeds.prefs"]();
const feed = as.feeds.get("feeds.prefs")();
assert.instanceOf(feed, Fake);
});
});

Просмотреть файл

@ -1,30 +1,75 @@
const ACTIVITY_STREAM_PREF_BRANCH = "browser.newtabpage.activity-stream.";
const {Prefs, DefaultPrefs} = require("lib/ActivityStreamPrefs.jsm");
const TEST_PREF_CONFIG = [
{name: "foo", value: true},
{name: "bar", value: "BAR"},
{name: "baz", value: 1}
];
const TEST_PREF_CONFIG = new Map([
["foo", {value: true}],
["bar", {value: "BAR"}],
["baz", {value: 1}]
]);
describe("ActivityStreamPrefs", () => {
describe("Prefs", () => {
let p;
beforeEach(() => {
p = new Prefs();
});
it("should have get, set, and observe methods", () => {
const p = new Prefs();
assert.property(p, "get");
assert.property(p, "set");
assert.property(p, "observe");
});
describe(".branchName", () => {
it("should return the activity stream branch by default", () => {
const p = new Prefs();
assert.equal(p.branchName, ACTIVITY_STREAM_PREF_BRANCH);
});
it("should return the custom branch name if it was passed to the constructor", () => {
const p = new Prefs("foo");
p = new Prefs("foo");
assert.equal(p.branchName, "foo");
});
});
describe("#observeBranch", () => {
let listener;
beforeEach(() => {
p._prefBranch = {addObserver: sinon.stub()};
listener = {onPrefChanged: sinon.stub()};
p.observeBranch(listener);
});
it("should add an observer", () => {
assert.calledOnce(p._prefBranch.addObserver);
});
it("should store the listener", () => {
assert.equal(p._branchObservers.size, 1);
assert.ok(p._branchObservers.has(listener));
});
it("should call listener's onPrefChanged", () => {
p._branchObservers.get(listener)();
assert.calledOnce(listener.onPrefChanged);
});
});
describe("#ignoreBranch", () => {
let listener;
beforeEach(() => {
p._prefBranch = {
addObserver: sinon.stub(),
removeObserver: sinon.stub()
};
listener = {};
p.observeBranch(listener);
});
it("should remove the observer", () => {
p.ignoreBranch(listener);
assert.calledOnce(p._prefBranch.addObserver);
});
it("should remove the listener", () => {
assert.equal(p._branchObservers.size, 1);
p.ignoreBranch(listener);
assert.equal(p._branchObservers.size, 0);
});
});
});
describe("DefaultPrefs", () => {
@ -54,8 +99,8 @@ describe("ActivityStreamPrefs", () => {
const defaultPrefs = new DefaultPrefs(TEST_PREF_CONFIG);
sinon.spy(defaultPrefs.branch, "clearUserPref");
defaultPrefs.reset();
for (const pref of TEST_PREF_CONFIG) {
assert.calledWith(defaultPrefs.branch.clearUserPref, pref.name);
for (const name of TEST_PREF_CONFIG.keys()) {
assert.calledWith(defaultPrefs.branch.clearUserPref, name);
}
});
});

Просмотреть файл

@ -1,18 +1,20 @@
const {PrefsFeed} = require("lib/PrefsFeed.jsm");
const {actionTypes: at, actionCreators: ac} = require("common/Actions.jsm");
const FAKE_PREFS = [{name: "foo", value: 1}, {name: "bar", value: 2}];
const FAKE_PREFS = new Map([["foo", {value: 1}], ["bar", {value: 2}]]);
describe("PrefsFeed", () => {
let feed;
beforeEach(() => {
feed = new PrefsFeed(FAKE_PREFS.map(p => p.name));
feed = new PrefsFeed(FAKE_PREFS);
feed.store = {dispatch: sinon.spy()};
feed._prefs = {
get: sinon.spy(item => FAKE_PREFS.filter(p => p.name === item)[0].value),
get: sinon.spy(item => FAKE_PREFS.get(item).value),
set: sinon.spy(),
observe: sinon.spy(),
ignore: sinon.spy()
observeBranch: sinon.spy(),
ignore: sinon.spy(),
ignoreBranch: sinon.spy()
};
});
it("should set a pref when a SET_PREF action is received", () => {
@ -25,27 +27,15 @@ describe("PrefsFeed", () => {
assert.equal(feed.store.dispatch.firstCall.args[0].type, at.PREFS_INITIAL_VALUES);
assert.deepEqual(feed.store.dispatch.firstCall.args[0].data, {foo: 1, bar: 2});
});
it("should add one observer per pref on init", () => {
it("should add one branch observer on init", () => {
feed.onAction({type: at.INIT});
FAKE_PREFS.forEach(pref => {
assert.calledWith(feed._prefs.observe, pref.name);
assert.isTrue(feed._observers.has(pref.name));
});
assert.calledOnce(feed._prefs.observeBranch);
assert.calledWith(feed._prefs.observeBranch, feed);
});
it("should call onPrefChanged when an observer is called", () => {
sinon.stub(feed, "onPrefChanged");
feed.onAction({type: at.INIT});
const handlerForFoo = feed._observers.get("foo");
handlerForFoo(true);
assert.calledWith(feed.onPrefChanged, "foo", true);
});
it("should remove all observers on uninit", () => {
it("should remove the branch observer on uninit", () => {
feed.onAction({type: at.UNINIT});
FAKE_PREFS.forEach(pref => {
assert.calledWith(feed._prefs.ignore, pref.name);
});
assert.calledOnce(feed._prefs.ignoreBranch);
assert.calledWith(feed._prefs.ignoreBranch, feed);
});
it("should send a PREF_CHANGED action when onPrefChanged is called", () => {
feed.onPrefChanged("foo", 2);

Просмотреть файл

@ -43,8 +43,8 @@ describe("Store", () => {
describe("#initFeed", () => {
it("should add an instance of the feed to .feeds", () => {
class Foo {}
store._prefs.set("foo", false);
store.init({foo: () => new Foo()});
store._prefs.set("foo", true);
store.init(new Map([["foo", () => new Foo()]]));
store.initFeed("foo");
assert.isTrue(store.feeds.has("foo"), "foo is set");
@ -52,7 +52,7 @@ describe("Store", () => {
});
it("should add a .store property to the feed", () => {
class Foo {}
store._feedFactories = {foo: () => new Foo()};
store._feedFactories = new Map([["foo", () => new Foo()]]);
store.initFeed("foo");
assert.propertyVal(store.feeds.get("foo"), "store", store);
@ -70,7 +70,7 @@ describe("Store", () => {
feed = {uninit: sinon.spy()};
return feed;
}
store._feedFactories = {foo: createFeed};
store._feedFactories = new Map([["foo", createFeed]]);
store.initFeed("foo");
store.uninitFeed("foo");
@ -79,7 +79,7 @@ describe("Store", () => {
});
it("should remove the feed from .feeds", () => {
class Foo {}
store._feedFactories = {foo: () => new Foo()};
store._feedFactories = new Map([["foo", () => new Foo()]]);
store.initFeed("foo");
store.uninitFeed("foo");
@ -87,69 +87,70 @@ describe("Store", () => {
assert.isFalse(store.feeds.has("foo"), "foo is not in .feeds");
});
});
describe("maybeStartFeedAndListenForPrefChanges", () => {
describe("onPrefChanged", () => {
beforeEach(() => {
sinon.stub(store, "initFeed");
sinon.stub(store, "uninitFeed");
});
it("should initialize the feed if the Pref is set to true", () => {
store._prefs.set("foo", true);
store.maybeStartFeedAndListenForPrefChanges("foo");
assert.calledWith(store.initFeed, "foo");
});
it("should not initialize the feed if the Pref is set to false", () => {
store._prefs.set("foo", false);
store.maybeStartFeedAndListenForPrefChanges("foo");
store.init(new Map([["foo", () => ({})]]));
});
it("should initialize the feed if called with true", () => {
store.onPrefChanged("foo", true);
assert.calledWith(store.initFeed, "foo");
assert.notCalled(store.uninitFeed);
});
it("should uninitialize the feed if called with false", () => {
store.onPrefChanged("foo", false);
assert.calledWith(store.uninitFeed, "foo");
assert.notCalled(store.initFeed);
});
it("should observe the pref", () => {
sinon.stub(store._prefs, "observe");
store.maybeStartFeedAndListenForPrefChanges("foo");
assert.calledWith(store._prefs.observe, "foo", store._prefHandlers.get("foo"));
});
describe("handler", () => {
let handler;
beforeEach(() => {
store.maybeStartFeedAndListenForPrefChanges("foo");
handler = store._prefHandlers.get("foo");
});
it("should initialize the feed if called with true", () => {
handler(true);
assert.calledWith(store.initFeed, "foo");
});
it("should uninitialize the feed if called with false", () => {
handler(false);
assert.calledWith(store.uninitFeed, "foo");
});
it("should do nothing if not an expected feed", () => {
store.onPrefChanged("bar", false);
assert.notCalled(store.initFeed);
assert.notCalled(store.uninitFeed);
});
});
describe("#init", () => {
it("should call .maybeStartFeedAndListenForPrefChanges with each key", () => {
sinon.stub(store, "maybeStartFeedAndListenForPrefChanges");
store.init({foo: () => {}, bar: () => {}});
assert.calledWith(store.maybeStartFeedAndListenForPrefChanges, "foo");
assert.calledWith(store.maybeStartFeedAndListenForPrefChanges, "bar");
it("should call .initFeed with each key", () => {
sinon.stub(store, "initFeed");
store._prefs.set("foo", true);
store._prefs.set("bar", true);
store.init(new Map([["foo", () => {}], ["bar", () => {}]]));
assert.calledWith(store.initFeed, "foo");
assert.calledWith(store.initFeed, "bar");
});
it("should not initialize the feed if the Pref is set to false", () => {
sinon.stub(store, "initFeed");
store._prefs.set("foo", false);
store.init(new Map([["foo", () => {}]]));
assert.notCalled(store.initFeed);
});
it("should observe the pref branch", () => {
sinon.stub(store._prefs, "observeBranch");
store.init(new Map());
assert.calledOnce(store._prefs.observeBranch);
assert.calledWith(store._prefs.observeBranch, store);
});
it("should initialize the ActivityStreamMessageChannel channel", () => {
store.init();
store.init(new Map());
assert.calledOnce(store._messageChannel.createChannel);
});
});
describe("#uninit", () => {
it("should clear .feeds, ._prefHandlers, and ._feedFactories", () => {
it("should clear .feeds and ._feedFactories", () => {
store._prefs.set("a", true);
store._prefs.set("b", true);
store._prefs.set("c", true);
store.init({
a: () => ({}),
b: () => ({}),
c: () => ({})
});
store.init(new Map([
["a", () => ({})],
["b", () => ({})],
["c", () => ({})]
]));
store.uninit();
assert.equal(store.feeds.size, 0);
assert.equal(store._prefHandlers.size, 0);
assert.isNull(store._feedFactories);
});
it("should destroy the ActivityStreamMessageChannel channel", () => {
@ -171,7 +172,7 @@ describe("Store", () => {
const action = {type: "FOO"};
store._prefs.set("sub", true);
store.init({sub: () => sub});
store.init(new Map([["sub", () => sub]]));
dispatch(action);

Просмотреть файл

@ -104,6 +104,9 @@ FakePrefs.prototype = {
delete this.observers[prefName];
}
},
_prefBranch: {},
observeBranch(listener) {},
ignoreBranch(listener) {},
prefs: {},
get(prefName) { return this.prefs[prefName]; },