diff --git a/app/importer.js b/app/importer.js index 635fc3f6d57..27b6e2ea16b 100644 --- a/app/importer.js +++ b/app/importer.js @@ -32,6 +32,7 @@ const locale = require('./locale') const tabMessageBox = require('./browser/tabMessageBox') const {makeImmutable} = require('./common/state/immutableUtil') const bookmarkFoldersUtil = require('./common/lib/bookmarkFoldersUtil') +const FunctionBuffer = require('../js/lib/functionBuffer') let isImportingBookmarks = false let hasBookmarks @@ -92,7 +93,7 @@ importer.on('add-history-page', (e, history) => { importer.on('add-homepage', (e, detail) => { }) -const getParentFolderId = (path, pathMap, folders, topLevelFolderId, nextFolderIdObject) => { +const getParentFolderId = (path, pathMap, addFolderFunction, topLevelFolderId, nextFolderIdObject) => { const pathLen = path.length if (!pathLen) { return topLevelFolderId @@ -103,10 +104,10 @@ const getParentFolderId = (path, pathMap, folders, topLevelFolderId, nextFolderI if (parentFolderId === undefined) { parentFolderId = nextFolderIdObject.id++ pathMap[parentFolder] = parentFolderId - folders.push({ + addFolderFunction({ title: parentFolder, folderId: parentFolderId, - parentFolderId: getParentFolderId(path, pathMap, folders, topLevelFolderId, nextFolderIdObject) + parentFolderId: getParentFolderId(path, pathMap, addFolderFunction, topLevelFolderId, nextFolderIdObject) }) } return parentFolderId @@ -121,36 +122,47 @@ importer.on('add-bookmarks', (e, importedBookmarks, topLevelFolder) => { let folders = [] let bookmarks = [] let topLevelFolderId = nextFolderIdObject.id++ + const functionBuffer = new FunctionBuffer((args) => makeImmutable(args), this) + const bufferedAddFolder = (folder) => { + functionBuffer.buffer(appActions.addBookmarkFolder, folder) + folders.push(folder) + } - folders.push({ + const importTopLevelFolder = { title: bookmarkFoldersUtil.getNextFolderName(bookmarkFolders, topLevelFolder), folderId: topLevelFolderId, parentFolderId: 0 - }) + } + bufferedAddFolder(importTopLevelFolder) for (let i = 0; i < importedBookmarks.length; ++i) { - let path = importedBookmarks[i].path - let parentFolderId = getParentFolderId(path, pathMap, folders, topLevelFolderId, nextFolderIdObject) - if (importedBookmarks[i].is_folder) { + const importedBookmark = importedBookmarks[i] + const path = importedBookmark.path + const title = importedBookmark.title + const parentFolderId = getParentFolderId(path, pathMap, bufferedAddFolder, topLevelFolderId, nextFolderIdObject) + if (importedBookmark.is_folder) { const folderId = nextFolderIdObject.id++ - pathMap[importedBookmarks[i].title] = folderId - folders.push({ - title: importedBookmarks[i].title, - folderId: folderId, - parentFolderId: parentFolderId - }) + pathMap[title] = folderId + const folder = { + title, + folderId, + parentFolderId + } + functionBuffer.buffer(appActions.addBookmarkFolder, folder) + folders.push(folder) } else { - bookmarks.push({ - title: importedBookmarks[i].title, - location: importedBookmarks[i].url, - parentFolderId: parentFolderId - }) + const location = importedBookmark.url + const bookmark = { + title, + location, + parentFolderId + } + functionBuffer.buffer(appActions.addBookmark, bookmark) + bookmarks.push(bookmark) } } - - bookmarkList = bookmarks - appActions.addBookmarkFolder(makeImmutable(folders)) - appActions.addBookmark(makeImmutable(bookmarks)) + functionBuffer.flush() + bookmarkList = makeImmutable(bookmarks) }) importer.on('add-favicons', (e, detail) => { diff --git a/js/lib/functionBuffer.js b/js/lib/functionBuffer.js new file mode 100644 index 00000000000..647cd708573 --- /dev/null +++ b/js/lib/functionBuffer.js @@ -0,0 +1,42 @@ +/* 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/. */ + +/** + * Perform a sequence of function calls, grouping together contiguous function + * calls for the same function different args. + * See syncUtil.applySyncRecords() for a usage example. + */ +module.exports = class FunctionBuffer { + /** + * @param {function=} prepareArguments Before calling a function, run this function to prepare the buffered arguments. + */ + constructor (prepareArguments) { + this.argumentsBuffer = [] + this.previousFunction = () => {} + this.prepareArguments = prepareArguments || ((args) => args) + } + + /** + * @param {function} fn Function to call. NOTE: this supports only calling with the first argument. + * @param {any} arg Arg to buffer + */ + buffer (fn, arg) { + if (fn !== this.previousFunction) { + this.flush() + this.previousFunction = fn + } + this.argumentsBuffer.push(arg) + } + + /** + * Call previousFunction with buffered arguments and empty buffer. + */ + flush () { + if (!this.argumentsBuffer.length) { return } + const preparedArgs = this.prepareArguments(this.argumentsBuffer) + const returnValue = this.previousFunction(preparedArgs) + this.argumentsBuffer = [] + return returnValue + } +} diff --git a/js/state/syncUtil.js b/js/state/syncUtil.js index 9bf4fe9fc05..1b3811a18b9 100644 --- a/js/state/syncUtil.js +++ b/js/state/syncUtil.js @@ -9,6 +9,7 @@ const crypto = require('crypto') const writeActions = require('../constants/sync/proto').actions const {getSetting} = require('../settings') const {isDataUrl} = require('../lib/urlutil') +const FunctionBuffer = require('../lib/functionBuffer') const bookmarkUtil = require('../../app/common/lib/bookmarkUtil') const bookmarkFoldersState = require('../../app/common/state/bookmarkFoldersState') const bookmarkFoldersUtil = require('../../app/common/lib/bookmarkFoldersUtil') @@ -282,21 +283,7 @@ module.exports.applySyncRecords = (records) => { * bookmark sites), then when the apply action changes (e.g. to folders) we * first flush the buffer (e.g. apply the first sequence of sites). */ - let buffer = [] - let previousAppAction = () => {} - const flushBufferedAppActions = () => { - if (!buffer.length) { return } - previousAppAction(new Immutable.List(buffer)) - buffer = [] - } - const bufferAppAction = (appAction, item) => { - if (previousAppAction !== appAction) { - flushBufferedAppActions() - previousAppAction = appAction - } - buffer.push(item) - } - + const functionBuffer = new FunctionBuffer((args) => new Immutable.List(args)) records.forEach((record) => { if (!record) { return true @@ -305,32 +292,32 @@ module.exports.applySyncRecords = (records) => { const siteData = module.exports.getSiteDataFromRecord(record, appState, records).set('skipSync', true) if (shouldAddRecord(record)) { if (record.bookmark.isFolder) { - bufferAppAction(appActions.addBookmarkFolder, siteData) + functionBuffer.buffer(appActions.addBookmarkFolder, siteData) } else { - bufferAppAction(appActions.addBookmark, siteData) + functionBuffer.buffer(appActions.addBookmark, siteData) } } else if (shouldRemoveRecord(record)) { if (record.bookmark.isFolder) { const folderKey = bookmarkFoldersUtil.getKey(siteData) - bufferAppAction(appActions.removeBookmarkFolder, folderKey) + functionBuffer.buffer(appActions.removeBookmarkFolder, folderKey) } else { const siteKey = bookmarkUtil.getKey(siteData) - bufferAppAction(appActions.removeBookmark, siteKey) + functionBuffer.buffer(appActions.removeBookmark, siteKey) } } } else if (record.objectData === 'historySite') { const siteData = module.exports.getSiteDataFromRecord(record, appState, records) if (shouldAddRecord(record)) { - bufferAppAction(appActions.addHistorySite, siteData) + functionBuffer.buffer(appActions.addHistorySite, siteData) } else if (shouldRemoveRecord(record)) { const siteKey = historyUtil.getKey(siteData) - bufferAppAction(appActions.removeHistorySite, siteKey) + functionBuffer.buffer(appActions.removeHistorySite, siteKey) } } else { otherRecords.push(record) } }) - flushBufferedAppActions() + functionBuffer.flush() applyNonBatchedRecords(otherRecords) }