diff --git a/js/background.js b/js/background.js index 54079c6fc..f266fb6c8 100644 --- a/js/background.js +++ b/js/background.js @@ -439,7 +439,7 @@ try { await ConversationController.load(); - BlockedNumberController.load(); + BlockedNumberController.refresh(); } catch (error) { window.log.error( 'background.js: ConversationController failed to load:', diff --git a/js/blocked_number_controller.js b/js/blocked_number_controller.js index e390961d2..7f4b3792c 100644 --- a/js/blocked_number_controller.js +++ b/js/blocked_number_controller.js @@ -1,4 +1,4 @@ -/* global , Whisper, storage, ConversationController */ +/* global , Whisper, storage */ /* global textsecure: false */ /* eslint-disable more/no-then */ @@ -13,33 +13,20 @@ window.getBlockedNumbers = () => blockedNumbers; window.BlockedNumberController = { - getAll() { - try { - this.load(); - } catch (e) { - window.log.warn(e); - } - return blockedNumbers; - }, reset() { + this.unblockAll(); blockedNumbers.reset([]); }, - load() { + refresh() { window.log.info('BlockedNumberController: starting initial fetch'); - if (blockedNumbers.length) { - throw new Error('BlockedNumberController: Already loaded!'); - } - if (!storage) { throw new Error('BlockedNumberController: Could not load blocked numbers'); } // Add the numbers to the collection const numbers = storage.getBlockedNumbers(); - blockedNumbers.add( - numbers.map(number => ({ number })) - ); + blockedNumbers.reset(numbers.map(number => ({ number }))); }, block(number) { const ourNumber = textsecure.storage.user.getNumber(); @@ -53,7 +40,7 @@ storage.addBlockedNumber(number); // Make sure we don't add duplicates - if (blockedNumbers.getNumber(number)) + if (blockedNumbers.getModel(number)) return; blockedNumbers.add({ number }); @@ -61,18 +48,15 @@ unblock(number) { storage.removeBlockedNumber(number); - // Make sure we don't add duplicates - const model = blockedNumbers.getNumber(number); + // Remove the model from our collection + const model = blockedNumbers.getModel(number); if (model) { blockedNumbers.remove(model); } }, unblockAll() { - const all = blockedNumbers.models; - all.forEach(number => { - storage.removeBlockedNumber(number); - blockedNumbers.remove(number); - }); + const numbers = blockedNumbers.map(m => m.get('number')); + numbers.forEach(n => this.unblock(n)); }, isBlocked(number) { return storage.isBlocked(number); diff --git a/js/models/blockedNumbers.js b/js/models/blockedNumbers.js index 93cf697d8..f61716d68 100644 --- a/js/models/blockedNumbers.js +++ b/js/models/blockedNumbers.js @@ -85,7 +85,7 @@ comparator(m) { return m.get('number'); }, - getNumber(number) { + getModel(number) { return this.models.find(m => m.get('number') === number); }, }); diff --git a/js/models/profile.js b/js/models/profile.js index cd8b6ec9b..c20d49bbb 100644 --- a/js/models/profile.js +++ b/js/models/profile.js @@ -17,6 +17,10 @@ } storage.setProfileName = async (newName) => { + if (typeof newName !== 'string' && newName !== null) { + throw Error('Name must be a string!'); + } + // Update our profiles accordingly' const trimmed = newName && newName.trim(); diff --git a/js/signal_protocol_store.js b/js/signal_protocol_store.js index a51656810..76f001aa6 100644 --- a/js/signal_protocol_store.js +++ b/js/signal_protocol_store.js @@ -864,7 +864,7 @@ ConversationController.reset(); BlockedNumberController.reset(); await ConversationController.load(); - BlockedNumberController.load(); + BlockedNumberController.refresh(); }, async removeAllConfiguration() { await window.Signal.Data.removeAllConfiguration(); diff --git a/js/views/blocked_number_view.js b/js/views/blocked_number_view.js index e5202e7d5..51f53dd03 100644 --- a/js/views/blocked_number_view.js +++ b/js/views/blocked_number_view.js @@ -1,4 +1,5 @@ /* global BlockedNumberController: false */ +/* global getBlockedNumbers: false */ /* global Whisper: false */ /* global storage: false */ /* global i18n: false */ @@ -19,7 +20,8 @@ }, initialize() { storage.onready(() => { - this.collection = BlockedNumberController.getAll(); + BlockedNumberController.refresh(); + this.collection = getBlockedNumbers(); this.listView = new Whisper.BlockedNumberListView({ collection: this.collection, }); diff --git a/test/blocked_number_controller_test.js b/test/blocked_number_controller_test.js new file mode 100644 index 000000000..8df157d31 --- /dev/null +++ b/test/blocked_number_controller_test.js @@ -0,0 +1,157 @@ +/* global textsecure, BlockedNumberController, storage */ + +'use strict'; + +describe('Blocked Number Controller', () => { + + beforeEach(async () => { + // Purge everything manually + const numbers = storage.getBlockedNumbers(); + numbers.forEach(storage.removeBlockedNumber); + window.getBlockedNumbers().reset([]); + }); + + describe('reset', () => { + it('clears blocked numbers', () => { + BlockedNumberController.block('1'); + assert.isNotEmpty(storage.getBlockedNumbers()); + assert.isNotEmpty(window.getBlockedNumbers().models); + + BlockedNumberController.reset(); + assert.isEmpty(storage.getBlockedNumbers()); + assert.isEmpty(window.getBlockedNumbers().models); + }); + }); + + describe('refresh', () => { + it('loads blocked numbers from storage', () => { + BlockedNumberController.refresh(); + assert.isEmpty(window.getBlockedNumbers().models); + + storage.addBlockedNumber('1'); + storage.addBlockedNumber('2'); + BlockedNumberController.refresh(); + + const blocked = window.getBlockedNumbers().map(m => m.get('number')); + assert.lengthOf(blocked, 2); + assert.deepEqual(['1', '2'], blocked.sort()); + }); + + it('overrides old numbers if we refresh again', () => { + BlockedNumberController.refresh(); + assert.isEmpty(window.getBlockedNumbers().models); + + storage.addBlockedNumber('1'); + BlockedNumberController.refresh(); + assert.isNotEmpty(window.getBlockedNumbers().find(m => m.get('number') === '1')); + + storage.removeBlockedNumber('1'); + storage.addBlockedNumber('2'); + BlockedNumberController.refresh(); + assert.isNotEmpty(window.getBlockedNumbers().find(m => m.get('number') === '2')); + }); + + it('throws if storage is invalid', () => { + const _storage = window.storage; + window.storage = null; + assert.throws(() => BlockedNumberController.refresh(), 'BlockedNumberController: Could not load blocked numbers'); + window.storage = _storage; + }); + }); + + describe('block', () => { + beforeEach(() => { + BlockedNumberController.refresh(); + assert.isEmpty(storage.getBlockedNumbers()); + assert.isEmpty(window.getBlockedNumbers().models); + }); + + it('adds number to the blocked list', () => { + BlockedNumberController.block('1'); + + const numbers = window.getBlockedNumbers().models; + assert.lengthOf(numbers, 1); + assert.strictEqual('1', numbers[0].get('number')) + assert.deepEqual(['1'], storage.getBlockedNumbers()); + }); + + it('only blocks the same number once', () => { + BlockedNumberController.block('2'); + BlockedNumberController.block('2'); + assert.lengthOf(window.getBlockedNumbers().models, 1); + assert.deepEqual(['2'], storage.getBlockedNumbers()); + }); + + it('does not block our own number', () => { + BlockedNumberController.block(textsecure.storage.user.getNumber()); + assert.isEmpty(window.getBlockedNumbers().models); + assert.isEmpty(storage.getBlockedNumbers()); + }); + }); + + describe('unblock', () => { + beforeEach(() => { + BlockedNumberController.refresh(); + assert.isEmpty(storage.getBlockedNumbers()); + assert.isEmpty(window.getBlockedNumbers().models); + }); + + it('removes number from the blocked list', () => { + BlockedNumberController.block('1'); + BlockedNumberController.block('2'); + + assert.lengthOf(window.getBlockedNumbers().models, 2); + assert.lengthOf(storage.getBlockedNumbers(), 2); + + BlockedNumberController.unblock('1'); + + const numbers = window.getBlockedNumbers().models; + assert.lengthOf(numbers, 1); + assert.isEmpty(numbers.filter(n => n.get('number') === '1')); + assert.deepEqual(['2'], storage.getBlockedNumbers()); + }); + + it('removes number from the blocked list even if it is not present in the collection', () => { + BlockedNumberController.block('1'); + BlockedNumberController.block('2'); + window.getBlockedNumbers().reset([]); + + assert.isEmpty(window.getBlockedNumbers().models); + assert.lengthOf(storage.getBlockedNumbers(), 2); + + BlockedNumberController.unblock('1'); + assert.deepEqual(['2'], storage.getBlockedNumbers()); + }); + }); + + describe('unblockAll', () => { + it('removes all our blocked numbers', () => { + BlockedNumberController.refresh(); + + BlockedNumberController.block('1'); + BlockedNumberController.block('2'); + BlockedNumberController.block('3'); + + assert.lengthOf(window.getBlockedNumbers().models, 3); + assert.lengthOf(storage.getBlockedNumbers(), 3); + + BlockedNumberController.unblockAll(); + + assert.lengthOf(window.getBlockedNumbers().models, 0); + assert.lengthOf(storage.getBlockedNumbers(), 0); + }); + }); + + describe('isBlocked', () => { + it('returns whether a number is blocked', () => { + BlockedNumberController.refresh(); + + BlockedNumberController.block('1'); + assert.isOk(BlockedNumberController.isBlocked('1')); + assert.isNotOk(BlockedNumberController.isBlocked('2')); + + BlockedNumberController.unblock('1'); + assert.isNotOk(BlockedNumberController.isBlocked('1')); + }); + }); +}); diff --git a/test/index.html b/test/index.html index 192c6f556..80e223b1c 100644 --- a/test/index.html +++ b/test/index.html @@ -426,8 +426,10 @@ + + diff --git a/test/models/conversations_test.js b/test/models/conversations_test.js index 62190fec7..de8798a97 100644 --- a/test/models/conversations_test.js +++ b/test/models/conversations_test.js @@ -192,4 +192,4 @@ describe('Conversation', () => { assert.isUndefined(collection.get(convo.id), 'got result for "+"'); }); }); -})(); +}); diff --git a/test/models/profile_test.js b/test/models/profile_test.js new file mode 100644 index 000000000..b30d8dfc8 --- /dev/null +++ b/test/models/profile_test.js @@ -0,0 +1,116 @@ +/* global storage */ + +/* eslint no-await-in-loop: 0 */ + +'use strict'; + +const PROFILE_ID = 'local-profile'; + +describe('Profile', () => { + beforeEach(async () => { + await clearDatabase(); + await storage.remove(PROFILE_ID); + }); + + describe('getLocalProfile', () => { + it('returns the local profile', async () => { + const values = [null, 'hello', { a: 'b' }]; + for(let i = 0; i < values.length; i += 1) { + await storage.put(PROFILE_ID, values[i]); + assert.strictEqual(values[i], storage.getLocalProfile()); + } + }); + }); + + describe('saveLocalProfile', () => { + it('saves a profile', async () => { + const values = [null, 'hello', { a: 'b' }]; + for(let i = 0; i < values.length; i += 1) { + await storage.saveLocalProfile(values[i]); + assert.strictEqual(values[i], storage.get(PROFILE_ID)); + } + }); + }); + + describe('removeLocalProfile', () => { + it('removes a profile', async () => { + await storage.saveLocalProfile('a'); + assert.strictEqual('a', storage.getLocalProfile()); + + await storage.removeLocalProfile(); + assert.strictEqual(null, storage.getLocalProfile()); + }); + }); + + describe('setProfileName', () => { + it('throws if a name is not a string', async () => { + const values = [0, { a: 'b'}, [1, 2]]; + for(let i = 0; i < values.length; i += 1) { + try { + await storage.setProfileName(values[i]); + assert.fail(`setProfileName did not throw an error for ${typeof values[i]}`); + } catch (e) { + assert.throws(() => { throw e; }, 'Name must be a string!'); + } + } + }); + + it('does not throw if we pass a string or null', async () => { + const values = [null, '1']; + for(let i = 0; i < values.length; i += 1) { + try { + await storage.setProfileName(values[i]); + } catch (e) { + assert.fail('setProfileName threw an error'); + } + } + }); + + it('saves the display name', async () => { + await storage.setProfileName('hi there!'); + + const expected = { + displayName: 'hi there!', + }; + const profile = storage.getLocalProfile(); + assert.exists(profile.name); + assert.deepEqual(expected, profile.name); + }); + + it('saves the display name without overwriting the other profile properties', async () => { + const profile = { title: 'hello' }; + await storage.put(PROFILE_ID, profile); + await storage.setProfileName('hi there!'); + + const expected = { + ...profile, + name: { + displayName: 'hi there!', + }, + }; + assert.deepEqual(expected, storage.getLocalProfile()); + }); + + it('trims the display name', async () => { + await storage.setProfileName(' in middle '); + const profile = storage.getLocalProfile(); + const name = { + displayName: 'in middle', + }; + assert.deepEqual(name, profile.name); + }); + + it('unsets the name property if it is empty', async () => { + const profile = { + name: { + displayName: 'HI THERE!', + }, + }; + await storage.put(PROFILE_ID, profile); + assert.exists(storage.getLocalProfile().name); + + await storage.setProfileName(''); + assert.notExists(storage.getLocalProfile().name); + }); + }); +});