From 980a85543d9e3bc8c468621564afee0f47f21d6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EF=BD=81=EF=BD=99=EF=BD=95=EF=BD=8D=EF=BD=89=C2=A0=20?= =?UTF-8?q?=EF=BD=99=EF=BD=95?= Date: Tue, 16 May 2017 21:46:45 +0000 Subject: [PATCH] Add locationSiteKeyCache and use for siteUtil.isBookmarked Fix #8703 --- app/browser/menu.js | 4 +- app/browser/reducers/sitesReducer.js | 48 ++-- app/common/state/tabState.js | 6 +- .../components/navigation/navigationBar.js | 7 +- app/sessionStore.js | 1 + docs/state.md | 3 + js/about/newtab.js | 1 + js/lib/urlutil.js | 15 +- js/state/siteCache.js | 90 ++++++ js/state/siteUtil.js | 132 ++++++--- .../app/browser/reducers/sitesReducerTest.js | 4 +- test/unit/lib/urlutilTest.js | 4 + test/unit/state/siteCacheTest.js | 145 ++++++++++ test/unit/state/siteUtilTest.js | 260 +++++++++--------- 14 files changed, 524 insertions(+), 196 deletions(-) create mode 100644 js/state/siteCache.js create mode 100644 test/unit/state/siteCacheTest.js diff --git a/app/browser/menu.js b/app/browser/menu.js index 431970e681..eb38bb0473 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -26,7 +26,7 @@ const menuUtil = require('../common/lib/menuUtil') const {getByTabId} = require('../common/state/tabState') const getSetting = require('../../js/settings').getSetting const locale = require('../locale') -const {isSiteBookmarked, siteSort} = require('../../js/state/siteUtil') +const {isLocationBookmarked, siteSort} = require('../../js/state/siteUtil') const tabState = require('../../app/common/state/tabState') const isDarwin = process.platform === 'darwin' const isLinux = process.platform === 'linux' @@ -351,7 +351,7 @@ const createHistorySubmenu = () => { } const isCurrentLocationBookmarked = () => { - return isSiteBookmarked(appStore.getState().get('sites'), Immutable.fromJS({location: currentLocation})) + return isLocationBookmarked(appStore.getState(), currentLocation) } const createBookmarksSubmenu = () => { diff --git a/app/browser/reducers/sitesReducer.js b/app/browser/reducers/sitesReducer.js index 7a5bb8a20d..8088fe09f0 100644 --- a/app/browser/reducers/sitesReducer.js +++ b/app/browser/reducers/sitesReducer.js @@ -6,6 +6,7 @@ const appConstants = require('../../../js/constants/appConstants') const filtering = require('../../filtering') +const siteCache = require('../../../js/state/siteCache') const siteTags = require('../../../js/constants/siteTags') const siteUtil = require('../../../js/state/siteUtil') const syncActions = require('../../../js/actions/syncActions') @@ -14,13 +15,26 @@ const Immutable = require('immutable') const settings = require('../../../js/constants/settings') const {getSetting} = require('../../../js/settings') const writeActions = require('../../../js/constants/sync/proto').actions +const tabState = require('../../common/state/tabState') const syncEnabled = () => { return getSetting(settings.SYNC_ENABLED) === true } +const updateActiveTabBookmarked = (state) => { + const tab = tabState.getActiveTab(state) + if (!tab) { + return state + } + const bookmarked = siteUtil.isLocationBookmarked(state, tab.get('url')) + return tabState.updateTabValue(state, tab.set('bookmarked', bookmarked)) +} + const sitesReducer = (state, action, immutableAction) => { switch (action.actionType) { + case appConstants.APP_SET_STATE: + state = siteCache.loadLocationSiteKeysCache(state) + break case appConstants.APP_ON_CLEAR_BROWSING_DATA: if (immutableAction.getIn(['clearDataDetail', 'browserHistory'])) { state = state.set('sites', siteUtil.clearHistory(state.get('sites'))) @@ -30,39 +44,41 @@ const sitesReducer = (state, action, immutableAction) => { case appConstants.APP_ADD_SITE: if (Immutable.List.isList(action.siteDetail)) { action.siteDetail.forEach((s) => { - state = state.set('sites', siteUtil.addSite(state.get('sites'), s, action.tag, undefined, action.skipSync)) + state = siteUtil.addSite(state, s, action.tag, undefined, action.skipSync) }) } else { let sites = state.get('sites') if (!action.siteDetail.get('folderId') && siteUtil.isFolder(action.siteDetail)) { action.siteDetail = action.siteDetail.set('folderId', siteUtil.getNextFolderId(sites)) } - state = state.set('sites', siteUtil.addSite(sites, action.siteDetail, action.tag, action.originalSiteDetail, action.skipSync)) + state = siteUtil.addSite(state, action.siteDetail, action.tag, action.originalSiteDetail, action.skipSync) } if (action.destinationDetail) { const sourceKey = siteUtil.getSiteKey(action.siteDetail) const destinationKey = siteUtil.getSiteKey(action.destinationDetail) if (sourceKey != null) { - state = state.set('sites', siteUtil.moveSite(state.get('sites'), - sourceKey, destinationKey, false, false, true)) + state = siteUtil.moveSite(state, + sourceKey, destinationKey, false, false, true) } } if (syncEnabled()) { state = syncUtil.updateSiteCache(state, action.destinationDetail || action.siteDetail) } + state = updateActiveTabBookmarked(state) break case appConstants.APP_REMOVE_SITE: const removeSiteSyncCallback = action.skipSync ? undefined : syncActions.removeSite - state = state.set('sites', siteUtil.removeSite(state.get('sites'), action.siteDetail, action.tag, true, removeSiteSyncCallback)) + state = siteUtil.removeSite(state, action.siteDetail, action.tag, true, removeSiteSyncCallback) if (syncEnabled()) { state = syncUtil.updateSiteCache(state, action.siteDetail) } + state = updateActiveTabBookmarked(state) break case appConstants.APP_MOVE_SITE: - state = state.set('sites', siteUtil.moveSite(state.get('sites'), + state = siteUtil.moveSite(state, action.sourceKey, action.destinationKey, action.prepend, - action.destinationIsParent, false)) + action.destinationIsParent, false) if (syncEnabled()) { const destinationDetail = state.getIn(['sites', action.destinationKey]) state = syncUtil.updateSiteCache(state, destinationDetail) @@ -85,19 +101,15 @@ const sitesReducer = (state, action, immutableAction) => { const siteData = syncUtil.getSiteDataFromRecord(record, state, action.records) const tag = siteData.tag let siteDetail = siteData.siteDetail - const sites = state.get('sites') switch (record.action) { case writeActions.CREATE: - state = state.set('sites', - siteUtil.addSite(sites, siteDetail, tag, undefined, true)) + state = siteUtil.addSite(state, siteDetail, tag, undefined, true) break case writeActions.UPDATE: - state = state.set('sites', - siteUtil.addSite(sites, siteDetail, tag, siteData.existingObjectData, true)) + state = siteUtil.addSite(state, siteDetail, tag, siteData.existingObjectData, true) break case writeActions.DELETE: - state = state.set('sites', - siteUtil.removeSite(sites, siteDetail, tag)) + state = siteUtil.removeSite(state, siteDetail, tag) break } state = syncUtil.updateSiteCache(state, siteDetail) @@ -115,9 +127,9 @@ const sitesReducer = (state, action, immutableAction) => { const sites = state.get('sites') const siteDetail = siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites) if (pinned) { - state = state.set('sites', siteUtil.addSite(sites, siteDetail, siteTags.PINNED)) + state = siteUtil.addSite(state, siteDetail, siteTags.PINNED) } else { - state = state.set('sites', siteUtil.removeSite(sites, siteDetail, siteTags.PINNED)) + state = siteUtil.removeSite(state, siteDetail, siteTags.PINNED) } if (syncEnabled()) { state = syncUtil.updateSiteCache(state, siteDetail) @@ -127,8 +139,8 @@ const sitesReducer = (state, action, immutableAction) => { case appConstants.APP_CREATE_TAB_REQUESTED: { const createProperties = immutableAction.get('createProperties') if (createProperties.get('pinned')) { - state = state.set('sites', siteUtil.addSite(state.get('sites'), - siteUtil.getDetailFromCreateProperties(createProperties), siteTags.PINNED)) + state = siteUtil.addSite(state, + siteUtil.getDetailFromCreateProperties(createProperties), siteTags.PINNED) } break } diff --git a/app/common/state/tabState.js b/app/common/state/tabState.js index 802aa5843b..8e93e80226 100644 --- a/app/common/state/tabState.js +++ b/app/common/state/tabState.js @@ -9,6 +9,7 @@ const frameState = require('./frameState') const windowState = require('./windowState') // this file should eventually replace frameStateUtil const frameStateUtil = require('../../../js/state/frameStateUtil') +const {isLocationBookmarked} = require('../../../js/state/siteUtil') const validateId = function (propName, id) { assert.ok(id, `${propName} cannot be null`) @@ -283,7 +284,10 @@ const tabState = { return state } - tabValue = tabValue.set('frame', makeImmutable(action.get('frame'))) + const frameLocation = action.getIn(['frame', 'location']) + const frameBookmarked = isLocationBookmarked(state, frameLocation) + const frameValue = action.get('frame').set('bookmarked', frameBookmarked) + tabValue = tabValue.set('frame', makeImmutable(frameValue)) return tabState.updateTabValue(state, tabValue) }, diff --git a/app/renderer/components/navigation/navigationBar.js b/app/renderer/components/navigation/navigationBar.js index 4ed735b2e6..a63a53fb49 100644 --- a/app/renderer/components/navigation/navigationBar.js +++ b/app/renderer/components/navigation/navigationBar.js @@ -110,10 +110,7 @@ class NavigationBar extends React.Component { get bookmarked () { return this.props.activeFrameKey !== undefined && - siteUtil.isSiteBookmarked(this.props.sites, Immutable.fromJS({ - location: this.props.location, - partitionNumber: this.props.partitionNumber - })) + this.props.bookmarked } componentDidMount () { @@ -126,6 +123,7 @@ class NavigationBar extends React.Component { const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map() const activeFrameKey = activeFrame.get('key') const activeTabId = activeFrame.get('tabId') || tabState.TAB_ID_NONE + const activeTab = tabState.getByTabId(state, activeTabId) const activeTabShowingMessageBox = tabState.isShowingMessageBox(state, activeTabId) const bookmarkDetail = currentWindow.get('bookmarkDetail') @@ -155,6 +153,7 @@ class NavigationBar extends React.Component { props.activeFrameKey = activeFrameKey props.location = location props.title = title + props.bookmarked = activeTab && activeTab.get('bookmarked') props.partitionNumber = activeFrame.get('partitionNumber') || 0 props.isSecure = activeFrame.getIn(['security', 'isSecure']) props.loading = loading diff --git a/app/sessionStore.js b/app/sessionStore.js index 5deb3aee88..b7aae20c1c 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -616,6 +616,7 @@ module.exports.defaultAppState = () => { lastFetchTimestamp: 0, objectsById: {} }, + locationSiteKeysCache: undefined, sites: getTopSiteMap(), tabs: [], windows: [], diff --git a/docs/state.md b/docs/state.md index 9c4307bab9..9d7ed6392b 100644 --- a/docs/state.md +++ b/docs/state.md @@ -249,6 +249,9 @@ AppStore title: string } // folder: folderId; bookmark/history: location + partitionNumber + parentFolderId }, + locationSiteKeyCache: { + [location]: Array. // location -> site keys + }, siteSettings: { [hostPattern]: { adControl: string, // (showBraveAds | blockAds | allowAdsAndTracking) diff --git a/js/about/newtab.js b/js/about/newtab.js index 7dded4ae3d..9fcb196d2c 100644 --- a/js/about/newtab.js +++ b/js/about/newtab.js @@ -96,6 +96,7 @@ class NewTabPage extends React.Component { }).size > 0 } isBookmarked (siteProps) { + // XXX: Fixme, not passing state in! return siteUtil.isSiteBookmarked(this.topSites, siteProps) } get gridLayout () { diff --git a/js/lib/urlutil.js b/js/lib/urlutil.js index 814d405630..b1d0dafdd0 100644 --- a/js/lib/urlutil.js +++ b/js/lib/urlutil.js @@ -306,10 +306,19 @@ const UrlUtil = { * @return {string} */ getLocationIfPDF: function (url) { - if (url && url.startsWith(`chrome-extension://${pdfjsExtensionId}/`)) { - return url.replace(`chrome-extension://${pdfjsExtensionId}/`, '') + if (!url || url.indexOf(`chrome-extension://${pdfjsExtensionId}/`) === -1) { + return url } - return url + + if (url.indexOf('content/web/viewer.html?file=') !== -1) { + const querystring = require('querystring') + const parsedUrl = urlParse(url) + const query = querystring.parse(parsedUrl.query) + if (query && query.file) { + return query.file + } + } + return url.replace(`chrome-extension://${pdfjsExtensionId}/`, '') }, /** diff --git a/js/state/siteCache.js b/js/state/siteCache.js new file mode 100644 index 0000000000..adb25da867 --- /dev/null +++ b/js/state/siteCache.js @@ -0,0 +1,90 @@ +/* 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/. */ + +'use strict' +const Immutable = require('immutable') +const siteUtil = require('./siteUtil') +const UrlUtil = require('../lib/urlutil') + +const createLocationSiteKeysCache = (state) => { + state = state.set('locationSiteKeysCache', new Immutable.Map()) + state.get('sites').forEach((site, siteKey) => { + const location = siteUtil.getLocationFromSiteKey(siteKey) + if (!location) { + return + } + state = addLocationSiteKey(state, location, siteKey) + }) + return state +} + +module.exports.loadLocationSiteKeysCache = (state) => { + const cache = state.get('locationSiteKeysCache') + if (cache) { + return state + } + return createLocationSiteKeysCache(state) +} + +/** + * Given a location, get matching appState siteKeys based on cache. + * Loads cache from appState if it hasn't been loaded yet. + * @param state Application state + * @param location {string} + * @return {Immutable.List|null} siteKeys including this location. + */ +module.exports.getLocationSiteKeys = (state, location) => { + const normalLocation = UrlUtil.getLocationIfPDF(location) + return state.getIn(['locationSiteKeysCache', normalLocation]) +} + +/** + * Given a location, add appState siteKey to cached siteKeys list. + * Returns new state. + * @param state Application state + * @param location {string} + * @param siteKey {string} + */ +const addLocationSiteKey = (state, location, siteKey) => { + if (!siteKey || !location) { + return state + } + const normalLocation = UrlUtil.getLocationIfPDF(location) + const cacheKey = ['locationSiteKeysCache', normalLocation] + const siteKeys = state.getIn(cacheKey) + if (!siteKeys) { + return state.setIn(cacheKey, new Immutable.List([siteKey])) + } else { + if (siteKeys.includes(siteKey)) { + return state + } + return state.setIn(cacheKey, siteKeys.push(siteKey)) + } +} +module.exports.addLocationSiteKey = addLocationSiteKey + +/** + * Given a location, remove matching appState siteKeys in cache. + * Loads cache from appState if it hasn't been loaded yet. + * @param state Application state + * @param location {string} + * @param siteKey {string} + */ +const removeLocationSiteKey = (state, location, siteKey) => { + if (!siteKey || !location) { + return state + } + const normalLocation = UrlUtil.getLocationIfPDF(location) + const cacheKey = ['locationSiteKeysCache', normalLocation] + let siteKeys = state.getIn(cacheKey) + if (!siteKeys) { + return state + } + siteKeys = siteKeys.filter(key => key !== siteKey) + if (siteKeys.size > 0) { + return state.setIn(cacheKey, siteKeys) + } else { + return state.deleteIn(cacheKey) + } +} +module.exports.removeLocationSiteKey = removeLocationSiteKey diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index ee3a2b2b82..09d42312ea 100644 --- a/js/state/siteUtil.js +++ b/js/state/siteUtil.js @@ -4,6 +4,7 @@ 'use strict' const Immutable = require('immutable') const normalizeUrl = require('normalize-url') +const siteCache = require('./siteCache') const siteTags = require('../constants/siteTags') const settings = require('../constants/settings') const getSetting = require('../settings').getSetting @@ -84,27 +85,57 @@ module.exports.getSiteKey = function (siteDetail) { } /** - * Checks if a siteDetail has the specified tag + * Calculate location for siteKey + * + * @param siteKey The site key to to be calculated + * @return {string|null} + */ +module.exports.getLocationFromSiteKey = function (siteKey) { + if (!siteKey) { + return null + } + + const splitKey = siteKey.split('|', 2) + if (typeof splitKey[0] === 'string' && typeof splitKey[1] === 'string') { + return splitKey[0] + } + return null +} + +/** + * Checks if a siteDetail has the specified tag. + * Depends on siteDeatil siteKey being accurate. * * @param sites The application state's Immutable sites map * @param siteDetail The site to check if it's in the specified tag * @return true if the location is already bookmarked */ module.exports.isSiteBookmarked = function (sites, siteDetail) { - if (!sites) { - return false - } - - const site = sites.find((site) => - isBookmark(site.get('tags')) && - site.get('location') === UrlUtil.getLocationIfPDF(siteDetail.get('location')) && - (site.get('partitionNumber') || 0) === (siteDetail.get('partitionNumber') || 0) - ) + const siteKey = module.exports.getSiteKey(siteDetail) + const siteTags = sites.getIn([siteKey, 'tags']) + return isBookmark(siteTags) +} - if (site) { - return true +/** + * Checks if a location is bookmarked. + * + * @param state The application state Immutable map + * @param {string} location + * @return {boolean} + */ +module.exports.isLocationBookmarked = function (state, location) { + const sites = state.get('sites') + const siteKeys = siteCache.getLocationSiteKeys(state, location) + if (!siteKeys || siteKeys.length === 0) { + return false } - return false + return siteKeys.some(key => { + const site = sites.get(key) + if (!site) { + return false + } + return isBookmark(site.get('tags')) + }) } const getNextFolderIdItem = (sites) => @@ -227,7 +258,7 @@ const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId, order) => } /** - * Adds or updates the specified siteDetail in sites. + * Adds or updates the specified siteDetail in appState.sites. * * Examples of updating: * - editing bookmark in add/edit modal @@ -241,9 +272,10 @@ const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId, order) => * @param originalSiteDetail If specified, use when searching site list * @param {boolean=} skipSync - True if this site was downloaded by sync and * does not to be re-uploaded - * @return The new sites Immutable object + * @return The new state Immutable object */ -module.exports.addSite = function (sites, siteDetail, tag, originalSiteDetail, skipSync) { +module.exports.addSite = function (state, siteDetail, tag, originalSiteDetail, skipSync) { + let sites = state.get('sites') // Get tag from siteDetail object if not passed via tag param if (tag === undefined) { tag = siteDetail.getIn(['tags', 0]) @@ -278,35 +310,43 @@ module.exports.addSite = function (sites, siteDetail, tag, originalSiteDetail, s sites = sites.delete(oldKey) } + let location if (site.has('location')) { - site = site.set('location', UrlUtil.getLocationIfPDF(site.get('location'))) + location = UrlUtil.getLocationIfPDF(site.get('location')) + site = site.set('location', location) } + const oldLocation = (oldSite && oldSite.get('location')) || site.get('location') + state = siteCache.removeLocationSiteKey(state, oldLocation, oldKey) if (skipSync) { site = site.set('skipSync', true) } + state = state.set('sites', sites) const key = module.exports.getSiteKey(site) if (key === null) { - return sites + return state } - return sites.set(key, site) + state = state.setIn(['sites', key], site) + state = siteCache.addLocationSiteKey(state, location, key) + return state } /** * Removes the specified tag from a siteDetail * - * @param {Immutable.Map} sites The application state's Immutable sites map + * @param {Immutable.Map} state The application state Immutable map * @param {Immutable.Map} siteDetail The siteDetail to remove a tag from * @param {string} tag * @param {boolean} reorder whether to reorder sites (default with reorder) * @param {Function=} syncCallback - * @return {Immutable.Map} The new sites Immutable object + * @return {Immutable.Map} The new state Immutable object */ -module.exports.removeSite = function (sites, siteDetail, tag, reorder = true, syncCallback) { +module.exports.removeSite = function (state, siteDetail, tag, reorder = true, syncCallback) { + let sites = state.get('sites') const key = module.exports.getSiteKey(siteDetail) if (!key) { - return sites + return state } if (getSetting(settings.SYNC_ENABLED) === true && syncCallback) { syncCallback(sites.getIn([key])) @@ -319,32 +359,37 @@ module.exports.removeSite = function (sites, siteDetail, tag, reorder = true, sy childSites.forEach((site) => { const tags = site.get('tags') tags.forEach((tag) => { - sites = module.exports.removeSite(sites, site, tag, false, syncCallback) + state = module.exports.removeSite(state, site, tag, false, syncCallback) }) }) } - let site = sites.get(key) + + const location = siteDetail.get('location') + state = siteCache.removeLocationSiteKey(state, location, key) + + const stateKey = ['sites', key] + let site = state.getIn(stateKey) if (!site) { - return sites + return state } if (isBookmark(tag)) { if (isPinnedTab(tags)) { const tags = site.get('tags').filterNot((tag) => tag === siteTags.BOOKMARK) site = site.set('tags', tags) - return sites.set(key, site) + return state.setIn(stateKey, site) } - if (sites.size && reorder) { - const order = sites.getIn([key, 'order']) - sites = reorderSite(sites, order) + if (state.get('sites').size && reorder) { + const order = state.getIn(stateKey.concat(['order'])) + state = state.set('sites', reorderSite(state.get('sites'), order)) } - return sites.delete(key) + return state.deleteIn(['sites', key]) } else if (isPinnedTab(tag)) { const tags = site.get('tags').filterNot((tag) => tag === siteTags.PINNED) site = site.set('tags', tags) - return sites.set(key, site) + return state.setIn(stateKey, site) } else { site = site.set('lastAccessedTime', undefined) - return sites.set(key, site) + return state.setIn(stateKey, site) } } @@ -388,21 +433,22 @@ module.exports.isMoveAllowed = (sites, sourceDetail, destinationDetail) => { /** * Moves the specified site from one location to another * - * @param sites The application state's Immutable sites map + * @param state The application state Immutable map * @param sourceKey The site key to move * @param destinationKey The site key to move to * @param prepend Whether the destination detail should be prepended or not, not used if destinationIsParent is true * @param destinationIsParent Whether the item should be moved inside of the destinationDetail. * @param disallowReparent If set to true, parent folder will not be set - * @return The new sites Immutable object + * @return The new state Immutable object */ -module.exports.moveSite = function (sites, sourceKey, destinationKey, prepend, +module.exports.moveSite = function (state, sourceKey, destinationKey, prepend, destinationIsParent, disallowReparent) { + let sites = state.get('sites') let sourceSite = sites.get(sourceKey) || Immutable.Map() const destinationSite = sites.get(destinationKey) || Immutable.Map() if (sourceSite.isEmpty() || !module.exports.isMoveAllowed(sites, sourceSite, destinationSite)) { - return sites + return state } const sourceSiteIndex = sourceSite.get('order') @@ -420,8 +466,10 @@ module.exports.moveSite = function (sites, sourceKey, destinationKey, prepend, --newIndex } - sites = sites.delete(sourceKey) - sites = sites.map((site) => { + const location = sourceSite.get('location') + state = siteCache.removeLocationSiteKey(state, location, sourceKey) + state = state.deleteIn(['sites', sourceKey]) + state = state.set('sites', state.get('sites').map((site) => { const siteOrder = site.get('order') if (siteOrder >= newIndex && siteOrder < sourceSiteIndex) { return site.set('order', siteOrder + 1) @@ -429,7 +477,7 @@ module.exports.moveSite = function (sites, sourceKey, destinationKey, prepend, return site.set('order', siteOrder - 1) } return site - }) + })) sourceSite = sourceSite.set('order', newIndex) if (!disallowReparent) { @@ -441,7 +489,9 @@ module.exports.moveSite = function (sites, sourceKey, destinationKey, prepend, sourceSite = sourceSite.set('parentFolderId', destinationSite.get('parentFolderId')) } } - return sites.set(module.exports.getSiteKey(sourceSite), sourceSite) + const destinationSiteKey = module.exports.getSiteKey(sourceSite) + state = siteCache.addLocationSiteKey(state, location, destinationSiteKey) + return state.setIn(['sites', destinationSiteKey], sourceSite) } module.exports.getDetailFromFrame = function (frame, tag) { diff --git a/test/unit/app/browser/reducers/sitesReducerTest.js b/test/unit/app/browser/reducers/sitesReducerTest.js index 4212ae678b..244abe0ea6 100644 --- a/test/unit/app/browser/reducers/sitesReducerTest.js +++ b/test/unit/app/browser/reducers/sitesReducerTest.js @@ -10,7 +10,9 @@ const { makeImmutable } = require('../../../../../app/common/state/immutableUtil require('../../../braveUnit') const initState = Immutable.fromJS({ - sites: {} + sites: {}, + windows: [], + tabs: [] }) /** diff --git a/test/unit/lib/urlutilTest.js b/test/unit/lib/urlutilTest.js index f0d3e63a67..98b98a4426 100644 --- a/test/unit/lib/urlutilTest.js +++ b/test/unit/lib/urlutilTest.js @@ -244,6 +244,10 @@ describe('urlutil', function () { assert.equal(UrlUtil.getLocationIfPDF('chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/https://www.blackhat.co…king-Kernel-Address-Space-Layout-Randomization-KASLR-With-Intel-TSX-wp.pdf'), 'https://www.blackhat.co…king-Kernel-Address-Space-Layout-Randomization-KASLR-With-Intel-TSX-wp.pdf') }) + it('gets location for PDF JS viewer URL', function () { + assert.equal(UrlUtil.getLocationIfPDF('chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/content/web/viewer.html?file=http%3A%2F%2Funec.edu.az%2Fapplication%2Fuploads%2F2014%2F12%2Fpdf-sample.pdf'), + 'http://unec.edu.az/application/uploads/2014/12/pdf-sample.pdf') + }) it('does not remove wayback machine url location for PDF JS URL', function () { assert.equal(UrlUtil.getLocationIfPDF('chrome-extension://jdbefljfgobbmcidnmpjamcbhnbphjnb/https://web.archive.org/web/20160106152308/http://stlab.adobe.com/wiki/images/d/d3/Test.pdf'), 'https://web.archive.org/web/20160106152308/http://stlab.adobe.com/wiki/images/d/d3/Test.pdf') diff --git a/test/unit/state/siteCacheTest.js b/test/unit/state/siteCacheTest.js new file mode 100644 index 0000000000..e61b25f3c6 --- /dev/null +++ b/test/unit/state/siteCacheTest.js @@ -0,0 +1,145 @@ +/* global describe, it */ + +const siteTags = require('../../../js/constants/siteTags') +const siteCache = require('../../../js/state/siteCache') +const siteUtil = require('../../../js/state/siteUtil') +const assert = require('assert') +const Immutable = require('immutable') +// const mockery = require('mockery') +// const settings = require('../../../js/constants/settings') + +describe('siteCache', function () { + const testUrl1 = 'https://brave.com/' + const testUrl2 = 'http://example.com/' + const bookmark = Immutable.fromJS({ + lastAccessedTime: 123, + objectId: [210, 115, 31, 176, 57, 212, 167, 120, 104, 88, 88, 27, 141, 36, 235, 226], + tags: [siteTags.BOOKMARK], + location: testUrl1, + title: 'sample', + parentFolderId: 0, + partitionNumber: 0 + }) + const bookmarkLocation = bookmark.get('location') + const bookmarkKey = siteUtil.getSiteKey(bookmark) + const folder = Immutable.fromJS({ + customTitle: 'folder1', + folderId: 1, + parentFolderId: 0, + tags: [siteTags.BOOKMARK_FOLDER] + }) + const folderKey = siteUtil.getSiteKey(folder) + const historySite = Immutable.fromJS({ + lastAccessedTime: 1477944718876, + location: testUrl2, + title: 'sample 1', + tags: [] + }) + const historySiteKey = siteUtil.getSiteKey(historySite) + const baseState = Immutable.fromJS({ + sites: { + [bookmarkKey]: bookmark, + [historySiteKey]: historySite, + [folderKey]: folder + } + }) + + describe('loadLocationSiteKeysCache', function () { + it('creates cache based on sites with location', function () { + const expectedCache = { + [bookmark.get('location')]: [bookmarkKey], + [historySite.get('location')]: [historySiteKey] + } + const state = siteCache.loadLocationSiteKeysCache(baseState) + assert.deepEqual(state.get('locationSiteKeysCache').toJS(), expectedCache) + }) + }) + + describe('getLocationSiteKeys', function () { + it('returns cached siteKeys', function () { + const state = siteCache.loadLocationSiteKeysCache(baseState) + const cachedKeys = siteCache.getLocationSiteKeys(state, bookmark.get('location')) + assert.deepEqual(cachedKeys.toJS(), [bookmarkKey]) + }) + it('returns null when location is not cached', function () { + const state = siteCache.loadLocationSiteKeysCache(baseState) + const cachedKeys = siteCache.getLocationSiteKeys(state, 'https://archive.org') + assert.equal(cachedKeys, null) + }) + it('returns null when location is undefined', function () { + const state = siteCache.loadLocationSiteKeysCache(baseState) + const cachedKeys = siteCache.getLocationSiteKeys(state, undefined) + assert.equal(cachedKeys, null) + }) + }) + + describe('addLocationSiteKey', function () { + it('when location is already cached, it appends', function () { + const site = Immutable.fromJS({ + lastAccessedTime: 1477944718877, + location: bookmarkLocation, + title: 'different', + parentFolderId: folder.get('folderId'), + tags: [siteTags.BOOKMARK] + }) + const siteKey = siteUtil.getSiteKey(site) + let state = siteCache.loadLocationSiteKeysCache(baseState) + state = siteCache.addLocationSiteKey(state, bookmarkLocation, siteKey) + const cachedKeys = siteCache.getLocationSiteKeys(state, bookmarkLocation) + assert.deepEqual(cachedKeys.toJS(), [bookmarkKey, siteKey]) + }) + it('when location is new, it creates a list with the key', function () { + const location = 'https://archive.org' + const site = Immutable.fromJS({ + lastAccessedTime: 1477944718877, + location, + title: 'different', + tags: [siteTags.BOOKMARK] + }) + const siteKey = siteUtil.getSiteKey(site) + let state = siteCache.loadLocationSiteKeysCache(baseState) + state = siteCache.addLocationSiteKey(state, location, siteKey) + const cachedKeys = siteCache.getLocationSiteKeys(state, location) + assert.deepEqual(cachedKeys.toJS(), [siteKey]) + }) + it('when location is undefined, it no-ops', function () { + const state = siteCache.addLocationSiteKey(baseState, undefined, '1') + assert.deepEqual(state.toJS(), baseState.toJS()) + }) + it('when siteKey is undefined, it no-ops', function () { + const state = siteCache.addLocationSiteKey(baseState, testUrl1, undefined) + assert.deepEqual(state.toJS(), baseState.toJS()) + }) + }) + + describe('removeLocationSiteKey', function () { + it('removes cached siteKeys', function () { + // Same location, different siteKey + const site = Immutable.fromJS({ + lastAccessedTime: 1477944718877, + location: bookmarkLocation, + title: 'different', + parentFolderId: folder.get('folderId'), + tags: [siteTags.BOOKMARK] + }) + const siteKey = siteUtil.getSiteKey(site) + let state = baseState.setIn(['sites', siteKey], site) + state = siteCache.loadLocationSiteKeysCache(state) + + // Sanity + let cachedKeys = siteCache.getLocationSiteKeys(state, bookmarkLocation) + assert.deepEqual(cachedKeys.toJS(), [bookmarkKey, siteKey]) + + state = siteCache.removeLocationSiteKey(state, bookmarkLocation, bookmarkKey) + cachedKeys = siteCache.getLocationSiteKeys(state, bookmarkLocation) + assert.deepEqual(cachedKeys.toJS(), [siteKey]) + }) + + it('when removing the last siteKey, removes location', function () { + let state = siteCache.loadLocationSiteKeysCache(baseState) + state = siteCache.removeLocationSiteKey(state, bookmarkLocation, bookmarkKey) + const cachedKeys = siteCache.getLocationSiteKeys(state, bookmarkLocation) + assert.deepEqual(cachedKeys, undefined) + }) + }) +}) diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js index 1e31ac6367..b50549e634 100644 --- a/test/unit/state/siteUtilTest.js +++ b/test/unit/state/siteUtilTest.js @@ -11,7 +11,7 @@ describe('siteUtil', function () { const testUrl1 = 'https://brave.com/' const testUrl2 = 'http://example.com/' const testFavicon1 = 'https://brave.com/favicon.ico' - const emptySites = Immutable.fromJS({}) + const emptyState = Immutable.fromJS({sites: {}}) const bookmarkAllFields = Immutable.fromJS({ lastAccessedTime: 123, objectId: [210, 115, 31, 176, 57, 212, 167, 120, 104, 88, 88, 27, 141, 36, 235, 226], @@ -130,7 +130,7 @@ describe('siteUtil', function () { const key = siteUtil.getSiteKey(Immutable.fromJS(site)) const sites = {} sites[key] = site - const result = siteUtil.isSiteBookmarked(Immutable.fromJS(sites), Immutable.fromJS({ + const result = siteUtil.isSiteBookmarked(Immutable.fromJS({sites}), Immutable.fromJS({ location: testUrl1, tags: [siteTags.BOOKMARK] })) @@ -145,7 +145,7 @@ describe('siteUtil', function () { const key = siteUtil.getSiteKey(siteDetail) const sites = {} sites[key] = site - const result = siteUtil.isSiteBookmarked(Immutable.fromJS(sites), siteDetail) + const result = siteUtil.isSiteBookmarked(Immutable.fromJS({sites}), siteDetail) assert.equal(result, false) }) }) @@ -227,54 +227,54 @@ describe('siteUtil', function () { describe('addSite', function () { it('gets the tag from siteDetail if not provided', function () { - const processedSites = siteUtil.addSite(emptySites, bookmarkAllFields) + const state = siteUtil.addSite(emptyState, bookmarkAllFields) const processedKey = siteUtil.getSiteKey(bookmarkAllFields) - assert.deepEqual(processedSites.getIn([processedKey, 'tags']), bookmarkAllFields.get('tags')) + assert.deepEqual(state.getIn(['sites', processedKey, 'tags']), bookmarkAllFields.get('tags')) }) describe('record count', function () { - var processedSites + var state it('create history record with count', function () { - processedSites = siteUtil.addSite(emptySites, siteMinFields) + state = siteUtil.addSite(emptyState, siteMinFields) const processedKey = siteUtil.getSiteKey(siteMinFields) - assert.deepEqual(processedSites.getIn([processedKey, 'count']), 1) + assert.deepEqual(state.getIn(['sites', processedKey, 'count']), 1) }) it('increments count for history item', function () { - processedSites = siteUtil.addSite(processedSites, siteMinFields) + state = siteUtil.addSite(state, siteMinFields) const processedKey = siteUtil.getSiteKey(siteMinFields) - assert.deepEqual(processedSites.getIn([processedKey, 'count']), 2) + assert.deepEqual(state.getIn(['sites', processedKey, 'count']), 2) }) }) describe('for new entries (oldSite is null)', function () { describe('when adding bookmark', function () { it('preserves existing siteDetail fields', function () { - const processedSites = siteUtil.addSite(emptySites, bookmarkAllFields, siteTags.BOOKMARK) + const state = siteUtil.addSite(emptyState, bookmarkAllFields, siteTags.BOOKMARK) const processedKey = siteUtil.getSiteKey(bookmarkAllFields) - let sites = {} - sites[processedKey] = bookmarkAllFields.set('order', 0).toJS() - const expectedSites = Immutable.fromJS(sites) - assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) + const expectedSites = Immutable.fromJS({ + [processedKey]: bookmarkAllFields.set('order', 0).toJS() + }) + assert.deepEqual(state.get('sites').toJS(), expectedSites.toJS()) }) it('sets 0 for lastAccessedTime if not specified', function () { - const processedSites = siteUtil.addSite(emptySites, bookmarkMinFields, siteTags.BOOKMARK) + const state = siteUtil.addSite(emptyState, bookmarkMinFields, siteTags.BOOKMARK) const processedKey = siteUtil.getSiteKey(bookmarkMinFields) - assert.equal(processedSites.getIn([processedKey, 'lastAccessedTime']), 0) - assert.deepEqual(processedSites.getIn([processedKey, 'tags']).toJS(), [siteTags.BOOKMARK]) + assert.equal(state.getIn(['sites', processedKey, 'lastAccessedTime']), 0) + assert.deepEqual(state.getIn(['sites', processedKey, 'tags']).toJS(), [siteTags.BOOKMARK]) }) }) describe('when adding bookmark folder', function () { it('assigns a folderId', function () { - const processedSites = siteUtil.addSite(emptySites, folderMinFields) + const state = siteUtil.addSite(emptyState, folderMinFields) const folderMinFieldsWithId = folderMinFields.set('folderId', 1) const processedKey = siteUtil.getSiteKey(folderMinFieldsWithId) - const folderId = processedSites.getIn([processedKey, 'folderId']) + const folderId = state.getIn(['sites', processedKey, 'folderId']) assert.equal(folderId, 1) }) it('allows for new folders to use the same customTitle as an existing folder', function () { // Add a new bookmark folder - let processedSites = siteUtil.addSite(emptySites, folderMinFields) + let state = siteUtil.addSite(emptyState, folderMinFields) const folderMinFieldsWithId1 = folderMinFields.set('folderId', 1) const processedKey1 = siteUtil.getSiteKey(folderMinFieldsWithId1) - const folderId = processedSites.getIn([processedKey1, 'folderId']) + const folderId = state.getIn([processedKey1, 'folderId']) const bookmark = Immutable.fromJS({ lastAccessedTime: 123, title: 'bookmark1', @@ -283,23 +283,25 @@ describe('siteUtil', function () { tags: [siteTags.BOOKMARK] }) // Add a bookmark into that folder - processedSites = siteUtil.addSite(processedSites, bookmark) + state = siteUtil.addSite(state, bookmark) const processedKey2 = siteUtil.getSiteKey(bookmark) - assert.equal(processedSites.size, 2) - assert.equal(processedSites.getIn([processedKey2, 'parentFolderId']), folderId) + assert.equal(state.get('sites').size, 2) + assert.equal(state.getIn([processedKey2, 'parentFolderId']), folderId) // Add another bookmark folder with the same name / parentFolderId - processedSites = siteUtil.addSite(processedSites, folderMinFields) + state = siteUtil.addSite(state, folderMinFields) const folderMinFieldsWithId2 = folderMinFields.set('folderId', 2) const processedKey3 = siteUtil.getSiteKey(folderMinFieldsWithId2) - assert.equal(processedSites.size, 3) - const folderId2 = processedSites.getIn([processedKey3, 'folderId']) + assert.equal(state.get('sites').size, 3) + const folderId2 = state.getIn(['sites', processedKey3, 'folderId']) assert.equal(folderId === folderId2, false) // Ensure fields for both folders are still in sites array - assert.equal(processedSites.getIn([processedKey1, 'customTitle']), - processedSites.getIn([processedKey3, 'customTitle'])) - assert.deepEqual(processedSites.getIn([processedKey1, 'tags']), processedSites.getIn([processedKey3, 'tags'])) + assert.equal( + state.getIn(['sites', processedKey1, 'customTitle']), + state.getIn(['sites', processedKey3, 'customTitle'])) + assert.deepEqual( + state.getIn(['sites', processedKey1, 'tags']), state.getIn(['sites', processedKey3, 'tags'])) }) it('calls removeSite on bookmark folders which have the same customTitle/parentFolderId', function () { let sites = {} @@ -345,28 +347,30 @@ describe('siteUtil', function () { sites[siteKey2] = site2 sites[siteKey3] = site3 sites[siteKey4] = site4 - let processedSites = Immutable.fromJS(sites) + let state = Immutable.fromJS({sites}) Immutable.fromJS(sites).forEach((site) => { - processedSites = siteUtil.addSite(processedSites, site) + state = siteUtil.addSite(state, site) }) const expectedSites = Immutable.fromJS(sites).map((site) => { return site.set('objectId', undefined) }) - assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) + assert.deepEqual(state.get('sites').toJS(), expectedSites.toJS()) }) }) describe('when adding history', function () { it('sets default values for lastAccessedTime and tag when they are missing', function () { - const processedSites = siteUtil.addSite(emptySites, bookmarkMinFields) + const state = siteUtil.addSite(emptyState, bookmarkMinFields) const processedKey = siteUtil.getSiteKey(bookmarkMinFields) - assert.equal(!!processedSites.getIn([processedKey, 'lastAccessedTime']), true) - assert.deepEqual(processedSites.getIn([processedKey, 'tags']).toJS(), []) + assert.equal(!!state.getIn(['sites', processedKey, 'lastAccessedTime']), true) + assert.deepEqual(state.getIn(['sites', processedKey, 'tags']).toJS(), []) }) it('returns newSiteDetail value for lastAccessedTime when oldSite value is undefined', function () { - const processedSites = siteUtil.addSite(emptySites, bookmarkAllFields) + const state = siteUtil.addSite(emptyState, bookmarkAllFields) const processedKey = siteUtil.getSiteKey(bookmarkAllFields) const expectedSites = Immutable.fromJS([bookmarkAllFields]) - assert.deepEqual(processedSites.getIn([processedKey, 'lastAccessedTime']), expectedSites.getIn([0, 'lastAccessedTime'])) + assert.deepEqual( + state.getIn(['sites', processedKey, 'lastAccessedTime']), + expectedSites.getIn([0, 'lastAccessedTime'])) }) }) }) @@ -405,11 +409,11 @@ describe('siteUtil', function () { }) let sites = {} sites[oldSiteKey] = oldSiteDetail.toJS() - const processedSites = siteUtil.addSite(Immutable.fromJS(sites), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) const expectedSiteKey = siteUtil.getSiteKey(expectedSiteDetail) let expectedSites = {} expectedSites[expectedSiteKey] = expectedSiteDetail.toJS() - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('overrides the old title with the new title', function () { const oldSiteDetail = Immutable.fromJS({ @@ -430,11 +434,11 @@ describe('siteUtil', function () { }) let sites = {} sites[oldSiteKey] = oldSiteDetail.toJS() - const processedSites = siteUtil.addSite(Immutable.fromJS(sites), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) const expectedSites = {} const expectedSiteKey = siteUtil.getSiteKey(newSiteDetail) expectedSites[expectedSiteKey] = newSiteDetail.set('order', 0).set('objectId', undefined).toJS() - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('returns oldSiteDetail value for lastAccessedTime when newSite value is undefined', function () { const oldSiteDetail = Immutable.fromJS({ @@ -449,9 +453,11 @@ describe('siteUtil', function () { }) const sites = Immutable.fromJS([oldSiteDetail]) - const processedSites = siteUtil.addSite(sites, newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) const expectedSites = sites - assert.deepEqual(processedSites.getIn([0, 'lastAccessedTime']), expectedSites.getIn([0, 'lastAccessedTime'])) + assert.deepEqual( + state.getIn(['sites', 0, 'lastAccessedTime']), + expectedSites.getIn([0, 'lastAccessedTime'])) }) it('move oldSiteDetail to new folder', function () { const oldSiteDetail = Immutable.fromJS({ @@ -486,11 +492,11 @@ describe('siteUtil', function () { }) let sites = {} sites[oldSiteKey] = oldSiteDetail.toJS() - const processedSites = siteUtil.addSite(Immutable.fromJS(sites), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail) const expectedSiteKey = siteUtil.getSiteKey(expectedSiteDetail) let expectedSites = {} expectedSites[expectedSiteKey] = expectedSiteDetail.set('objectId', undefined).toJS() - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('sets skipSync when skipSync is provided', function () { mockery.enable({ @@ -522,13 +528,13 @@ describe('siteUtil', function () { customTitle: 'new customTitle' }) const siteKey = siteUtil.getSiteKey(oldSiteDetail) - const sites = Immutable.fromJS({ + const sites = { [siteKey]: oldSiteDetail - }) - const processedSites = siteUtil.addSite(sites, newSiteDetail, siteTags.BOOKMARK, oldSiteDetail, true) + } + const state = siteUtil.addSite(Immutable.fromJS({sites}), newSiteDetail, siteTags.BOOKMARK, oldSiteDetail, true) mockery.deregisterMock('./stores/appStoreRenderer') mockery.disable() - assert.equal(processedSites.getIn([siteKey, 'skipSync']), true) + assert.equal(state.getIn(['sites', siteKey, 'skipSync']), true) }) }) }) @@ -543,9 +549,9 @@ describe('siteUtil', function () { const siteKey = siteUtil.getSiteKey(Immutable.fromJS(siteDetail)) let sites = {} sites[siteKey] = siteDetail - const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail), siteTags.BOOKMARK) + const state = siteUtil.removeSite(Immutable.fromJS({sites}), Immutable.fromJS(siteDetail), siteTags.BOOKMARK) const expectedSites = new Immutable.Map() - assert.deepEqual(processedSites, expectedSites) + assert.deepEqual(state.get('sites'), expectedSites) }) it('removes the bookmark tag when it is pinned', function () { const siteDetail = { @@ -561,8 +567,8 @@ describe('siteUtil', function () { const siteKey = siteUtil.getSiteKey(Immutable.fromJS(siteDetail)) let sites = {} sites[siteKey] = siteDetail - const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail), siteTags.BOOKMARK) - assert.deepEqual(processedSites.toJS(), expectedSites) + const state = siteUtil.removeSite(Immutable.fromJS({sites}), Immutable.fromJS(siteDetail), siteTags.BOOKMARK) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('removes the pinned tag when it is bookmarked', function () { const siteDetail = { @@ -578,8 +584,8 @@ describe('siteUtil', function () { const siteKey = siteUtil.getSiteKey(Immutable.fromJS(siteDetail)) let sites = {} sites[siteKey] = siteDetail - const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail), siteTags.PINNED) - assert.deepEqual(processedSites.toJS(), expectedSites) + const state = siteUtil.removeSite(Immutable.fromJS({sites}), Immutable.fromJS(siteDetail), siteTags.PINNED) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('removes folder and its children', function () { let sites = {} @@ -620,9 +626,9 @@ describe('siteUtil', function () { parentFolderId: 0, tags: [siteTags.BOOKMARK_FOLDER] } - const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail), siteTags.BOOKMARK_FOLDER) + const state = siteUtil.removeSite(Immutable.fromJS({sites}), Immutable.fromJS(siteDetail), siteTags.BOOKMARK_FOLDER) const expectedSites = new Immutable.Map() - assert.deepEqual(processedSites, expectedSites) + assert.deepEqual(state.get('sites'), expectedSites) }) it('removes with reorder', function () { let sites = {} @@ -673,9 +679,9 @@ describe('siteUtil', function () { location: testUrl1, tags: [siteTags.BOOKMARK] } - const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail), + const state = siteUtil.removeSite(Immutable.fromJS({sites}), Immutable.fromJS(siteDetail), siteTags.BOOKMARK_FOLDER) - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) }) describe('tag=falsey', function () { @@ -695,8 +701,8 @@ describe('siteUtil', function () { const siteKey = siteUtil.getSiteKey(Immutable.fromJS(siteDetail)) let sites = {} sites[siteKey] = siteDetail - const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail)) - assert.deepEqual(processedSites.toJS(), expectedSites) + const state = siteUtil.removeSite(Immutable.fromJS({sites}), Immutable.fromJS(siteDetail)) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('remove pinned tag when unpinning', function () { const siteDetail = { @@ -714,8 +720,8 @@ describe('siteUtil', function () { const siteKey = siteUtil.getSiteKey(Immutable.fromJS(siteDetail)) let sites = {} sites[siteKey] = siteDetail - const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail), siteTags.PINNED) - assert.deepEqual(processedSites.toJS(), expectedSites) + const state = siteUtil.removeSite(Immutable.fromJS({sites}), Immutable.fromJS(siteDetail), siteTags.PINNED) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('remove a non exist site', function () { const siteDetail = { @@ -734,8 +740,8 @@ describe('siteUtil', function () { const siteKey = siteUtil.getSiteKey(Immutable.fromJS(addedSite)) let sites = {} sites[siteKey] = addedSite - const processedSites = siteUtil.removeSite(Immutable.fromJS(sites), Immutable.fromJS(siteDetail)) - assert.deepEqual(processedSites.toJS(), expectedSites) + const state = siteUtil.removeSite(Immutable.fromJS({sites}), Immutable.fromJS(siteDetail)) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) }) }) @@ -744,35 +750,35 @@ describe('siteUtil', function () { // NOTE: usage taken from Bookmark Manager, which calls aboutActions.moveSite it('does not allow you to move a bookmark folder into itself', function () { // Add a new bookmark folder - let processedSites = siteUtil.addSite(emptySites, folderMinFields) + let state = siteUtil.addSite(emptyState, folderMinFields) const folderMinFieldsWithId = folderMinFields.set('folderId', 1) const processedKey = siteUtil.getSiteKey(folderMinFieldsWithId) - const folderId = processedSites.getIn([processedKey, 'folderId']) + const folderId = state.getIn(['sites', processedKey, 'folderId']) // Add a bookmark into that folder - processedSites = siteUtil.addSite(processedSites, bookmarkAllFields.set('parentFolderId', folderId)) - const bookmarkFolder = processedSites.get(processedKey) + state = siteUtil.addSite(state, bookmarkAllFields.set('parentFolderId', folderId)) + const bookmarkFolder = state.getIn(['sites', processedKey]) // Should NOT be able to move bookmark folder into itself - assert.equal(false, siteUtil.isMoveAllowed(processedSites, bookmarkFolder, bookmarkFolder)) + assert.equal(false, siteUtil.isMoveAllowed(state.get('sites'), bookmarkFolder, bookmarkFolder)) }) it('does not allow you to move an ancestor folder into a descendant folder', function () { // Add a new bookmark folder - let processedSites = siteUtil.addSite(emptySites, folderMinFields) + let state = siteUtil.addSite(emptyState, folderMinFields) const folderMinFieldsWithId1 = folderMinFields.set('folderId', 1) const processedKey1 = siteUtil.getSiteKey(folderMinFieldsWithId1) - const folderId1 = processedSites.getIn([processedKey1, 'folderId']) + const folderId1 = state.getIn(['sites', processedKey1, 'folderId']) // Add a child below that folder - processedSites = siteUtil.addSite(processedSites, folderMinFields.set('parentFolderId', folderId1)) + state = siteUtil.addSite(state, folderMinFields.set('parentFolderId', folderId1)) const folderMinFieldsWithId2 = folderMinFields.set('folderId', 2) const processedKey2 = siteUtil.getSiteKey(folderMinFieldsWithId2) - const folderId2 = processedSites.getIn([processedKey2, 'folderId']) + const folderId2 = state.getIn(['sites', processedKey2, 'folderId']) // Add a folder below the previous child - processedSites = siteUtil.addSite(processedSites, folderMinFields.set('parentFolderId', folderId2)) + state = siteUtil.addSite(state, folderMinFields.set('parentFolderId', folderId2)) const folderMinFieldsWithId3 = folderMinFields.set('folderId', 3) const processedKey3 = siteUtil.getSiteKey(folderMinFieldsWithId3) - const bookmarkFolder1 = processedSites.get(processedKey1) - const bookmarkFolder3 = processedSites.get(processedKey3) + const bookmarkFolder1 = state.getIn(['sites', processedKey1]) + const bookmarkFolder3 = state.getIn(['sites', processedKey3]) // Should NOT be able to move grandparent folder into its grandchild - assert.equal(false, siteUtil.isMoveAllowed(processedSites, bookmarkFolder1, bookmarkFolder3)) + assert.equal(false, siteUtil.isMoveAllowed(state.get('sites'), bookmarkFolder1, bookmarkFolder3)) }) }) @@ -835,10 +841,10 @@ describe('siteUtil', function () { order: 3 } } - const processedSites = siteUtil.moveSite(Immutable.fromJS(sites), + const state = siteUtil.moveSite(Immutable.fromJS({sites}), sourceKey, destinationKey, true, false, true) - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('not prepend target', function () { const expectedSites = { @@ -867,10 +873,10 @@ describe('siteUtil', function () { order: 3 } } - const processedSites = siteUtil.moveSite(Immutable.fromJS(sites), + const state = siteUtil.moveSite(Immutable.fromJS({sites}), sourceKey, destinationKey, false, false, true) - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) }) describe('front to back', function () { @@ -930,10 +936,10 @@ describe('siteUtil', function () { order: 3 } } - const processedSites = siteUtil.moveSite(Immutable.fromJS(sites), + const state = siteUtil.moveSite(Immutable.fromJS({sites}), sourceKey, destinationKey, true, false, true) - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('not prepend target', function () { const expectedSites = { @@ -962,10 +968,10 @@ describe('siteUtil', function () { order: 3 } } - const processedSites = siteUtil.moveSite(Immutable.fromJS(sites), + const state = siteUtil.moveSite(Immutable.fromJS({sites}), sourceKey, destinationKey, false, false, true) - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) }) }) @@ -996,10 +1002,10 @@ describe('siteUtil', function () { order: 1 } } - const processedSites = siteUtil.moveSite(Immutable.fromJS(sites), + const state = siteUtil.moveSite(Immutable.fromJS({sites}), sourceKey, '1', false, true, false) - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) it('reparent', function () { const sourceDetail = { @@ -1027,10 +1033,10 @@ describe('siteUtil', function () { order: 1 } } - const processedSites = siteUtil.moveSite(Immutable.fromJS(sites), + const state = siteUtil.moveSite(Immutable.fromJS({sites}), 'https://brave.com/|0|1', '1', false, false, false) - assert.deepEqual(processedSites.toJS(), expectedSites) + assert.deepEqual(state.get('sites').toJS(), expectedSites) }) }) @@ -1048,18 +1054,18 @@ describe('siteUtil', function () { title: 'history entry', lastAccessedTime: 456 }) - let sites = siteUtil.addSite(emptySites, siteDetail1, siteTags.BOOKMARK) - sites = siteUtil.addSite(sites, siteDetail2) - const processedSites = siteUtil.updateSiteFavicon(sites, testUrl1, testFavicon1) + let state = siteUtil.addSite(emptyState, siteDetail1, siteTags.BOOKMARK) + state = siteUtil.addSite(state, siteDetail2) + const processedSites = siteUtil.updateSiteFavicon(state.get('sites'), testUrl1, testFavicon1) const updatedSiteDetail1 = siteDetail1.set('favicon', testFavicon1) const updatedSiteDetail2 = siteDetail2.set('favicon', testFavicon1) - let expectedSites = siteUtil.addSite(emptySites, updatedSiteDetail1, siteTags.BOOKMARK) - expectedSites = siteUtil.addSite(expectedSites, updatedSiteDetail2) + let expectedState = siteUtil.addSite(emptyState, updatedSiteDetail1, siteTags.BOOKMARK) + expectedState = siteUtil.addSite(expectedState, updatedSiteDetail2) - assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) + assert.deepEqual(processedSites.toJS(), expectedState.get('sites').toJS()) }) it('returns the object unchanged if location is not a URL', function () { - const sites = siteUtil.addSite(emptySites, bookmarkMinFields, siteTags.BOOKMARK) + const sites = siteUtil.addSite(emptyState, bookmarkMinFields, siteTags.BOOKMARK) const processedSites = siteUtil.updateSiteFavicon(sites, 'not-a-url', 'https://brave.com/favicon.ico') assert.deepEqual(processedSites, sites) }) @@ -1069,15 +1075,17 @@ describe('siteUtil', function () { assert.deepEqual(processedSites, emptyLegacySites) }) it('works even if null/undefined entries are present', function () { - const hasInvalidEntries = Immutable.fromJS({ - 'null': null, - 'undefined': undefined + const stateWithInvalidEntries = Immutable.fromJS({ + sites: { + 'null': null, + 'undefined': undefined + } }) - const sites = siteUtil.addSite(hasInvalidEntries, bookmarkMinFields, siteTags.BOOKMARK) - const processedSites = siteUtil.updateSiteFavicon(sites, testUrl1, 'https://brave.com/favicon.ico') + const state = siteUtil.addSite(stateWithInvalidEntries, bookmarkMinFields, siteTags.BOOKMARK) + const processedSites = siteUtil.updateSiteFavicon(state.get('sites'), testUrl1, 'https://brave.com/favicon.ico') const updatedSiteDetail = bookmarkMinFields.set('favicon', 'https://brave.com/favicon.ico') - const expectedSites = siteUtil.addSite(hasInvalidEntries, updatedSiteDetail, siteTags.BOOKMARK) - assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) + const expectedState = siteUtil.addSite(stateWithInvalidEntries, updatedSiteDetail, siteTags.BOOKMARK) + assert.deepEqual(processedSites.toJS(), expectedState.get('sites').toJS()) }) }) @@ -1163,7 +1171,7 @@ describe('siteUtil', function () { const newPinned = newNotPinned.set('tags', [siteTags.PINNED]) it('returns `url` if both it and `provisionalLocation` are pinned', function () { - let sites = siteUtil.addSite(emptySites, oldPinned, siteTags.PINNED) + let sites = siteUtil.addSite(emptyState, oldPinned, siteTags.PINNED) sites = siteUtil.addSite(sites, newPinned, siteTags.PINNED) assert.deepEqual( siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites).toJS(), @@ -1176,7 +1184,7 @@ describe('siteUtil', function () { ) }) it('returns `url` if that was pinned (and `provisionalLocation` is not pinned)', function () { - let sites = siteUtil.addSite(emptySites, oldNotPinned) + let sites = siteUtil.addSite(emptyState, oldNotPinned) sites = siteUtil.addSite(sites, newPinned, siteTags.PINNED) assert.deepEqual( siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites).toJS(), @@ -1189,10 +1197,10 @@ describe('siteUtil', function () { ) }) it('returns `provisionalLocation` if it was pinned (and `url` (redirected) is not pinned)', function () { - let sites = siteUtil.addSite(emptySites, oldPinned, siteTags.PINNED) - sites = siteUtil.addSite(sites, newNotPinned) + let state = siteUtil.addSite(emptyState, oldPinned, siteTags.PINNED) + state = siteUtil.addSite(state, newNotPinned) assert.deepEqual( - siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites).toJS(), + siteUtil.getDetailFromTab(tab, siteTags.PINNED, state.get('sites')).toJS(), { location: originalUrl, title: tab.get('title'), @@ -1203,10 +1211,10 @@ describe('siteUtil', function () { }) it('returns `url` if `provisionalLocation` is falsey', function () { const tab2 = tab.setIn(['frame', 'provisionalLocation'], undefined) - let sites = siteUtil.addSite(emptySites, oldPinned, siteTags.PINNED) - sites = siteUtil.addSite(sites, newNotPinned) + let state = siteUtil.addSite(emptyState, oldPinned, siteTags.PINNED) + state = siteUtil.addSite(state, newNotPinned) assert.deepEqual( - siteUtil.getDetailFromTab(tab2, siteTags.PINNED, sites).toJS(), + siteUtil.getDetailFromTab(tab2, siteTags.PINNED, state.get('sites')).toJS(), { location: newUrl, title: tab2.get('title'), @@ -1244,9 +1252,9 @@ describe('siteUtil', function () { parentFolderId: 10 }) it('returns parentFolderId', function () { - const sites = siteUtil.addSite(emptySites, siteWithFolder, siteTags.PINNED) + const state = siteUtil.addSite(emptyState, siteWithFolder, siteTags.PINNED) assert.deepEqual( - siteUtil.getDetailFromTab(tab, siteTags.PINNED, sites).toJS(), + siteUtil.getDetailFromTab(tab, siteTags.PINNED, state.get('sites')).toJS(), { location: siteUrl, title: tab.get('title'), @@ -1554,8 +1562,8 @@ describe('siteUtil', function () { 'key1': { tags: [siteTags.BOOKMARK_FOLDER] }, 'key2': { tags: [siteTags.BOOKMARK] } }) - const processedSites = siteUtil.clearHistory(sites) - assert.deepEqual(processedSites.toJS(), sites.toJS()) + const processedState = siteUtil.clearHistory(sites) + assert.deepEqual(processedState.toJS(), sites.toJS()) }) it('sets the lastAccessedTime for all entries to null', function () { const sites = Immutable.fromJS({ @@ -1577,8 +1585,8 @@ describe('siteUtil', function () { lastAccessedTime: null } }) - const processedSites = siteUtil.clearHistory(sites) - assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) + const processedState = siteUtil.clearHistory(sites) + assert.deepEqual(processedState.toJS(), expectedSites.toJS()) }) }) @@ -1592,8 +1600,8 @@ describe('siteUtil', function () { tags: [siteTags.BOOKMARK] } }) - const processedSites = siteUtil.getBookmarks(sites) - assert.deepEqual(sites, processedSites) + const processedState = siteUtil.getBookmarks(sites) + assert.deepEqual(sites, processedState) }) it('excludes items which are NOT tagged `BOOKMARK_FOLDER` or `BOOKMARK`', function () { const sites = Immutable.fromJS({ @@ -1605,13 +1613,13 @@ describe('siteUtil', function () { } }) const expectedSites = Immutable.fromJS({}) - const processedSites = siteUtil.getBookmarks(sites) - assert.deepEqual(expectedSites.toJS(), processedSites.toJS()) + const processedState = siteUtil.getBookmarks(sites) + assert.deepEqual(expectedSites.toJS(), processedState.toJS()) }) it('returns empty map if input was falsey', function () { - const processedSites = siteUtil.getBookmarks(null) + const processedState = siteUtil.getBookmarks(null) const expectedSites = Immutable.fromJS({}) - assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) + assert.deepEqual(processedState.toJS(), expectedSites.toJS()) }) })