diff --git a/CliClient/tests/synchronizer.js b/CliClient/tests/synchronizer.js index 9f8220885..4ee20b58f 100644 --- a/CliClient/tests/synchronizer.js +++ b/CliClient/tests/synchronizer.js @@ -1252,4 +1252,22 @@ describe('Synchronizer', function() { expect((await revisionService().revisionNote(revisions, 1)).title).toBe('note REV2'); })); + it("should not download resources over the limit", asyncTest(async () => { + const note1 = await Note.save({ title: 'note' }); + await shim.attachFileToNote(note1, __dirname + '/../tests/support/photo.jpg'); + await synchronizer().start(); + + await switchClient(2); + + const previousMax = synchronizer().maxResourceSize_; + synchronizer().maxResourceSize_ = 1; + await synchronizer().start(); + synchronizer().maxResourceSize_ = previousMax; + + const syncItems = await BaseItem.allSyncItems(syncTargetId()); + expect(syncItems.length).toBe(2); + expect(syncItems[1].item_location).toBe(BaseItem.SYNC_ITEM_LOCATION_REMOTE); + expect(syncItems[1].sync_disabled).toBe(1); + })); + }); diff --git a/ReactNativeClient/lib/components/screens/note.js b/ReactNativeClient/lib/components/screens/note.js index 5660c5c17..29890bbbd 100644 --- a/ReactNativeClient/lib/components/screens/note.js +++ b/ReactNativeClient/lib/components/screens/note.js @@ -424,6 +424,9 @@ class NoteScreenComponent extends BaseScreenComponent { return; } + const itDoes = await shim.fsDriver().waitTillExists(targetPath); + if (!itDoes) throw new Error('Resource file was not created: ' + targetPath); + const fileStat = await shim.fsDriver().stat(targetPath); resource.size = fileStat.size; diff --git a/ReactNativeClient/lib/fs-driver-base.js b/ReactNativeClient/lib/fs-driver-base.js index 03e811a60..45047cd01 100644 --- a/ReactNativeClient/lib/fs-driver-base.js +++ b/ReactNativeClient/lib/fs-driver-base.js @@ -1,4 +1,5 @@ const { filename, fileExtension } = require('lib/path-utils'); +const { time } = require('lib/time-utils.js'); class FsDriverBase { @@ -50,6 +51,17 @@ class FsDriverBase { } } + async waitTillExists(path, timeout = 10000) { + const startTime = Date.now(); + + while (true) { + const e = await this.exists(path); + if (e) return true; + if (Date.now() - startTime > timeout) return false; + await time.msleep(100); + } + } + } module.exports = FsDriverBase; \ No newline at end of file diff --git a/ReactNativeClient/lib/joplin-database.js b/ReactNativeClient/lib/joplin-database.js index 2f4ae588f..9b60defd2 100644 --- a/ReactNativeClient/lib/joplin-database.js +++ b/ReactNativeClient/lib/joplin-database.js @@ -263,7 +263,7 @@ class JoplinDatabase extends Database { // must be set in the synchronizer too. // Note: v16 and v17 don't do anything. They were used to debug an issue. - const existingDatabaseVersions = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20]; + const existingDatabaseVersions = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]; let currentVersionIndex = existingDatabaseVersions.indexOf(fromVersion); @@ -560,6 +560,10 @@ class JoplinDatabase extends Database { queries.push({ sql: 'INSERT INTO migrations (number, created_time, updated_time) VALUES (20, ?, ?)', params: [timestamp, timestamp] }); } + if (targetVersion == 21) { + queries.push('ALTER TABLE sync_items ADD COLUMN item_location INT NOT NULL DEFAULT 1'); + } + queries.push({ sql: 'UPDATE version SET version = ?', params: [targetVersion] }); try { diff --git a/ReactNativeClient/lib/models/BaseItem.js b/ReactNativeClient/lib/models/BaseItem.js index 872f833d3..db0517196 100644 --- a/ReactNativeClient/lib/models/BaseItem.js +++ b/ReactNativeClient/lib/models/BaseItem.js @@ -121,6 +121,11 @@ class BaseItem extends BaseModel { return output; } + static async allSyncItems(syncTarget) { + const output = await this.db().selectAll('SELECT * FROM sync_items WHERE sync_target = ?', [syncTarget]); + return output; + } + static pathToId(path) { let p = path.split('/'); let s = p[p.length - 1].split('.'); @@ -591,29 +596,34 @@ class BaseItem extends BaseModel { const rows = await this.db().selectAll('SELECT * FROM sync_items WHERE sync_disabled = 1 AND sync_target = ?', [syncTargetId]); let output = []; for (let i = 0; i < rows.length; i++) { - const item = await this.loadItem(rows[i].item_type, rows[i].item_id); - if (!item) continue; // The referenced item no longer exist + const row = rows[i]; + const item = await this.loadItem(row.item_type, row.item_id); + if (row.item_location === BaseItem.SYNC_ITEM_LOCATION_LOCAL && !item) continue; // The referenced item no longer exist + output.push({ - syncInfo: rows[i], + syncInfo: row, + location: row.item_location, item: item, }); } return output; } - static updateSyncTimeQueries(syncTarget, item, syncTime, syncDisabled = false, syncDisabledReason = '') { + static updateSyncTimeQueries(syncTarget, item, syncTime, syncDisabled = false, syncDisabledReason = '', itemLocation = null) { const itemType = item.type_; const itemId = item.id; if (!itemType || !itemId || syncTime === undefined) throw new Error(sprintf('Invalid parameters in updateSyncTimeQueries(): %d, %s, %d', syncTarget, JSON.stringify(item), syncTime)); + if (itemLocation === null) itemLocation = BaseItem.SYNC_ITEM_LOCATION_LOCAL; + return [ { sql: 'DELETE FROM sync_items WHERE sync_target = ? AND item_type = ? AND item_id = ?', params: [syncTarget, itemType, itemId], }, { - sql: 'INSERT INTO sync_items (sync_target, item_type, item_id, sync_time, sync_disabled, sync_disabled_reason) VALUES (?, ?, ?, ?, ?, ?)', - params: [syncTarget, itemType, itemId, syncTime, syncDisabled ? 1 : 0, syncDisabledReason + ''], + sql: 'INSERT INTO sync_items (sync_target, item_type, item_id, item_location, sync_time, sync_disabled, sync_disabled_reason) VALUES (?, ?, ?, ?, ?, ?, ?)', + params: [syncTarget, itemType, itemId, itemLocation, syncTime, syncDisabled ? 1 : 0, syncDisabledReason + ''], } ]; } @@ -623,9 +633,9 @@ class BaseItem extends BaseModel { return this.db().transactionExecBatch(queries); } - static async saveSyncDisabled(syncTargetId, item, syncDisabledReason) { + static async saveSyncDisabled(syncTargetId, item, syncDisabledReason, itemLocation = null) { const syncTime = 'sync_time' in item ? item.sync_time : 0; - const queries = this.updateSyncTimeQueries(syncTargetId, item, syncTime, true, syncDisabledReason); + const queries = this.updateSyncTimeQueries(syncTargetId, item, syncTime, true, syncDisabledReason, itemLocation); return this.db().transactionExecBatch(queries); } @@ -643,7 +653,7 @@ class BaseItem extends BaseModel { let selectSql = 'SELECT id FROM ' + ItemClass.tableName(); if (ItemClass.modelType() == this.TYPE_NOTE) selectSql += ' WHERE is_conflict = 0'; - queries.push('DELETE FROM sync_items WHERE item_type = ' + ItemClass.modelType() + ' AND item_id NOT IN (' + selectSql + ')'); + queries.push('DELETE FROM sync_items WHERE item_location = ' + BaseItem.SYNC_ITEM_LOCATION_LOCAL + ' AND item_type = ' + ItemClass.modelType() + ' AND item_id NOT IN (' + selectSql + ')'); } await this.db().transactionExecBatch(queries); @@ -728,4 +738,7 @@ BaseItem.syncItemDefinitions_ = [ { type: BaseModel.TYPE_REVISION, className: 'Revision' }, ]; +BaseItem.SYNC_ITEM_LOCATION_LOCAL = 1; +BaseItem.SYNC_ITEM_LOCATION_REMOTE = 2; + module.exports = BaseItem; \ No newline at end of file diff --git a/ReactNativeClient/lib/services/report.js b/ReactNativeClient/lib/services/report.js index 8bbb227b2..804526a09 100644 --- a/ReactNativeClient/lib/services/report.js +++ b/ReactNativeClient/lib/services/report.js @@ -118,7 +118,11 @@ class ReportService { for (let i = 0; i < disabledItems.length; i++) { const row = disabledItems[i]; - section.body.push(_('%s (%s): %s', row.item.title, row.item.id, row.syncInfo.sync_disabled_reason)); + if (row.location === BaseItem.SYNC_ITEM_LOCATION_LOCAL) { + section.body.push(_('%s (%s) could not be uploaded: %s', row.item.title, row.item.id, row.syncInfo.sync_disabled_reason)); + } else { + section.body.push(_('Item "%s" could not be downloaded: %s', row.syncInfo.item_id, row.syncInfo.sync_disabled_reason)); + } } section.body.push(''); diff --git a/ReactNativeClient/lib/shim-init-node.js b/ReactNativeClient/lib/shim-init-node.js index d65227c94..2efeb9ecc 100644 --- a/ReactNativeClient/lib/shim-init-node.js +++ b/ReactNativeClient/lib/shim-init-node.js @@ -142,8 +142,8 @@ function shimInit() { if (resource.mime == 'image/jpeg' || resource.mime == 'image/jpg' || resource.mime == 'image/png') { const result = await resizeImage_(filePath, targetPath, resource.mime); } else { - const stat = await shim.fsDriver().stat(filePath); - if (stat.size >= 10000000) throw new Error('Resources larger than 10 MB are not currently supported as they may crash the mobile applications. The issue is being investigated and will be fixed at a later time.'); + // const stat = await shim.fsDriver().stat(filePath); + // if (stat.size >= 10000000) throw new Error('Resources larger than 10 MB are not currently supported as they may crash the mobile applications. The issue is being investigated and will be fixed at a later time.'); await fs.copy(filePath, targetPath, { overwrite: true }); } @@ -152,6 +152,9 @@ function shimInit() { resource = Object.assign({}, resource, defaultProps); } + const itDoes = await shim.fsDriver().waitTillExists(targetPath); + if (!itDoes) throw new Error('Resource file was not created: ' + targetPath); + const fileStat = await shim.fsDriver().stat(targetPath); resource.size = fileStat.size; diff --git a/ReactNativeClient/lib/shim-init-react.js b/ReactNativeClient/lib/shim-init-react.js index 7e9ecee5f..d6ca869a1 100644 --- a/ReactNativeClient/lib/shim-init-react.js +++ b/ReactNativeClient/lib/shim-init-react.js @@ -166,6 +166,9 @@ function shimInit() { resource = Object.assign({}, resource, defaultProps); } + const itDoes = await shim.fsDriver().waitTillExists(targetPath); + if (!itDoes) throw new Error('Resource file was not created: ' + targetPath); + const fileStat = await shim.fsDriver().stat(targetPath); resource.size = fileStat.size; diff --git a/ReactNativeClient/lib/synchronizer.js b/ReactNativeClient/lib/synchronizer.js index a567723af..47486be69 100644 --- a/ReactNativeClient/lib/synchronizer.js +++ b/ReactNativeClient/lib/synchronizer.js @@ -27,6 +27,7 @@ class Synchronizer { this.appType_ = appType; this.cancelling_ = false; this.autoStartDecryptionWorker_ = true; + this.maxResourceSize_ = null; // Debug flags are used to test certain hard-to-test conditions // such as cancelling in the middle of a loop. @@ -58,6 +59,11 @@ class Synchronizer { return this.logger_; } + maxResourceSize() { + if (this.maxResourceSize_ !== null) return this.maxResourceSize_; + return this.appType_ === 'mobile' ? 10 * 1000 * 1000 : Infinity; + } + setEncryptionService(v) { this.encryptionService_ = v; } @@ -206,6 +212,11 @@ class Synchronizer { this.logSyncOperation('starting', null, null, 'Starting synchronisation to target ' + syncTargetId + '... [' + synchronizationId + ']'); + const handleCannotSyncItem = async (ItemClass, syncTargetId, item, cannotSyncReason, itemLocation = null) => { + await ItemClass.saveSyncDisabled(syncTargetId, item, cannotSyncReason, itemLocation); + this.dispatch({ type: "SYNC_HAS_DISABLED_SYNC_ITEMS" }); + } + try { await this.api().mkdir(this.syncDirName_); this.api().setTempDirName(this.syncDirName_); @@ -301,11 +312,6 @@ class Synchronizer { this.logSyncOperation(action, local, remote, reason); - const handleCannotSyncItem = async (syncTargetId, item, cannotSyncReason) => { - await ItemClass.saveSyncDisabled(syncTargetId, item, cannotSyncReason); - this.dispatch({ type: "SYNC_HAS_DISABLED_SYNC_ITEMS" }); - }; - if (local.type_ == BaseModel.TYPE_RESOURCE && (action == "createRemote" || action === "updateRemote" || (action == "itemConflict" && remote))) { try { const remoteContentPath = this.resourceDirName_ + "/" + local.id; @@ -315,7 +321,7 @@ class Synchronizer { await this.api().put(remoteContentPath, null, { path: localResourceContentPath, source: "file" }); } catch (error) { if (error && ["rejectedByTarget", "fileNotFound"].indexOf(error.code) >= 0) { - await handleCannotSyncItem(syncTargetId, local, error.message); + await handleCannotSyncItem(ItemClass, syncTargetId, local, error.message); action = null; } else { throw error; @@ -331,7 +337,7 @@ class Synchronizer { await this.api().put(path, content); } catch (error) { if (error && error.code === "rejectedByTarget") { - await handleCannotSyncItem(syncTargetId, local, error.message); + await handleCannotSyncItem(ItemClass, syncTargetId, local, error.message); canSync = false; } else { throw error; @@ -568,6 +574,11 @@ class Synchronizer { const creatingNewResource = content.type_ == BaseModel.TYPE_RESOURCE && action == "createLocal"; if (creatingNewResource) { + if (content.size >= this.maxResourceSize()) { + await handleCannotSyncItem(ItemClass, syncTargetId, content, 'File "' + content.title + '" is larger than allowed ' + this.maxResourceSize() + ' bytes. Beyond this limit, the mobile app would crash.', BaseItem.SYNC_ITEM_LOCATION_REMOTE); + continue; + } + await ResourceLocalState.save({ resource_id: content.id, fetch_status: Resource.FETCH_STATUS_IDLE }); }