diff --git a/.eslintignore b/.eslintignore index a2a1ab2097..979e37d35f 100644 --- a/.eslintignore +++ b/.eslintignore @@ -672,6 +672,7 @@ packages/lib/eventManager.js packages/lib/file-api-driver-joplinServer.js packages/lib/file-api-driver-memory.js packages/lib/file-api-driver.test.js +packages/lib/file-api.test.js packages/lib/file-api.js packages/lib/fs-driver-base.js packages/lib/fs-driver-node.js diff --git a/.gitignore b/.gitignore index b75db6180d..1dce8c0a8c 100644 --- a/.gitignore +++ b/.gitignore @@ -652,6 +652,7 @@ packages/lib/eventManager.js packages/lib/file-api-driver-joplinServer.js packages/lib/file-api-driver-memory.js packages/lib/file-api-driver.test.js +packages/lib/file-api.test.js packages/lib/file-api.js packages/lib/fs-driver-base.js packages/lib/fs-driver-node.js diff --git a/packages/lib/Synchronizer.ts b/packages/lib/Synchronizer.ts index fb992d2d5b..e79db6b263 100644 --- a/packages/lib/Synchronizer.ts +++ b/packages/lib/Synchronizer.ts @@ -20,7 +20,7 @@ import JoplinError from './JoplinError'; import ShareService from './services/share/ShareService'; import TaskQueue from './TaskQueue'; import ItemUploader from './services/synchronizer/ItemUploader'; -import { FileApi, RemoteItem } from './file-api'; +import { FileApi, getSupportsDeltaWithItems, PaginatedList, RemoteItem } from './file-api'; import JoplinDatabase from './JoplinDatabase'; import { fetchSyncInfo, getActiveMasterKey, localSyncInfo, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, uploadSyncInfo } from './services/synchronizer/syncInfoUtils'; import { getMasterPassword, setupAndDisableEncryption, setupAndEnableEncryption } from './services/e2ee/utils'; @@ -389,9 +389,6 @@ export default class Synchronizer { this.syncTargetIsLocked_ = false; this.cancelling_ = false; - // const masterKeysBefore = await MasterKey.count(); - // let hasAutoEnabledEncryption = false; - const synchronizationId = time.unixMs().toString(); const outputContext = { ...lastContext }; @@ -401,7 +398,7 @@ export default class Synchronizer { this.dispatch({ type: 'SYNC_STARTED' }); eventManager.emit(EventName.SyncStart); - this.logSyncOperation('starting', null, null, `Starting synchronisation to target ${syncTargetId}... supportsAccurateTimestamp = ${this.api().supportsAccurateTimestamp}; supportsMultiPut = ${this.api().supportsMultiPut}; supportsDeltaWithItems = ${this.api().supportsDeltaWithItems} [${synchronizationId}]`); + this.logSyncOperation('starting', null, null, `Starting synchronisation to target ${syncTargetId}... supportsAccurateTimestamp = ${this.api().supportsAccurateTimestamp}; supportsMultiPut = ${this.api().supportsMultiPut}} [${synchronizationId}]`); const handleCannotSyncItem = async (ItemClass: any, syncTargetId: any, item: any, cannotSyncReason: string, itemLocation: any = null) => { await ItemClass.saveSyncDisabled(syncTargetId, item, cannotSyncReason, itemLocation); @@ -810,7 +807,7 @@ export default class Synchronizer { while (true) { if (this.cancelling() || hasCancelled) break; - const listResult: any = await this.apiCall('delta', '', { + const listResult: PaginatedList = await this.apiCall('delta', '', { context: context, // allItemIdsHandler() provides a way for drivers that don't have a delta API to @@ -827,7 +824,9 @@ export default class Synchronizer { logger: logger, }); - const remotes: RemoteItem[] = listResult.items; + const supportsDeltaWithItems = getSupportsDeltaWithItems(listResult); + + const remotes = listResult.items; this.logSyncOperation('fetchingTotal', null, null, 'Fetching delta items from sync target', remotes.length); @@ -843,7 +842,7 @@ export default class Synchronizer { if (local && local.updated_time === remote.jop_updated_time) needsToDownload = false; } - if (this.api().supportsDeltaWithItems) { + if (supportsDeltaWithItems) { needsToDownload = false; } @@ -866,9 +865,7 @@ export default class Synchronizer { if (!BaseItem.isSystemPath(remote.path)) continue; // The delta API might return things like the .sync, .resource or the root folder const loadContent = async () => { - if (this.api().supportsDeltaWithItems) { - return remote.jopItem; - } + if (supportsDeltaWithItems) return remote.jopItem; const task = await this.downloadQueue_.waitForResult(path); if (task.error) throw task.error; diff --git a/packages/lib/file-api-driver-joplinServer.ts b/packages/lib/file-api-driver-joplinServer.ts index 269d6065ff..f3309bff6f 100644 --- a/packages/lib/file-api-driver-joplinServer.ts +++ b/packages/lib/file-api-driver-joplinServer.ts @@ -45,10 +45,6 @@ export default class FileApiDriverJoplinServer { return true; } - public get supportsDeltaWithItems() { - return true; - } - public requestRepeatCount() { return 3; } diff --git a/packages/lib/file-api.test.ts b/packages/lib/file-api.test.ts new file mode 100644 index 0000000000..7717163df8 --- /dev/null +++ b/packages/lib/file-api.test.ts @@ -0,0 +1,73 @@ +import { PaginatedList, RemoteItem, getSupportsDeltaWithItems } from './file-api'; + +const defaultPaginatedList = (): PaginatedList => { + return { + items: [], + hasMore: false, + context: null, + }; +}; + +const defaultItem = (): RemoteItem => { + return { + id: '', + }; +}; + +describe('file-api', () => { + + test.each([ + [ + { + ...defaultPaginatedList(), + items: [], + }, + false, + ], + + [ + { + ...defaultPaginatedList(), + items: [ + { + ...defaultItem(), + path: 'test', + }, + ], + }, + false, + ], + + [ + { + ...defaultPaginatedList(), + items: [ + { + ...defaultItem(), + path: 'test', + jopItem: null, + }, + ], + }, + true, + ], + + [ + { + ...defaultPaginatedList(), + items: [ + { + ...defaultItem(), + path: 'test', + jopItem: { something: 'abcd' }, + }, + ], + }, + true, + ], + ])('should tell if the sync target supports delta with items', async (deltaResponse: PaginatedList, expected: boolean) => { + const actual = getSupportsDeltaWithItems(deltaResponse); + expect(actual).toBe(expected); + }); + +}); diff --git a/packages/lib/file-api.ts b/packages/lib/file-api.ts index e7919cdd58..b491072cd5 100644 --- a/packages/lib/file-api.ts +++ b/packages/lib/file-api.ts @@ -43,6 +43,14 @@ export interface PaginatedList { context: any; } +// Tells whether the delta call is going to include the items themselves or +// just the metadata (which is the default). If the items are included it +// means less http request and faster processing. +export const getSupportsDeltaWithItems = (deltaResponse: PaginatedList) => { + if (!deltaResponse.items.length) return false; + return 'jopItem' in deltaResponse.items[0]; +}; + function requestCanBeRepeated(error: any) { const errorCode = typeof error === 'object' && error.code ? error.code : null; @@ -139,13 +147,6 @@ class FileApi { return !!this.driver().supportsLocks; } - // Tells whether the delta call is going to include the items themselves or - // just the metadata (which is the default). If the items are included it - // means less http request and faster processing. - public get supportsDeltaWithItems(): boolean { - return !!this.driver().supportsDeltaWithItems; - } - private async fetchRemoteDateOffset_() { const tempFile = `${this.tempDirName()}/timeCheck${Math.round(Math.random() * 1000000)}.txt`; const startTime = Date.now(); @@ -361,7 +362,7 @@ class FileApi { return tryAndRepeat(() => this.driver_.clearRoot(this.baseDir()), this.requestRepeatCount()); } - public delta(path: string, options: any = null) { + public delta(path: string, options: any = null): Promise { logger.debug(`delta ${this.fullPath(path)}`); return tryAndRepeat(() => this.driver_.delta(this.fullPath(path), options), this.requestRepeatCount()); } diff --git a/packages/lib/models/BaseItem.ts b/packages/lib/models/BaseItem.ts index bfe214aafc..3d7b2937fe 100644 --- a/packages/lib/models/BaseItem.ts +++ b/packages/lib/models/BaseItem.ts @@ -207,7 +207,7 @@ export default class BaseItem extends BaseModel { return output; } - public static pathToId(path: string) { + public static pathToId(path: string): string { const p = path.split('/'); const s = p[p.length - 1].split('.'); let name: any = s[0]; diff --git a/packages/server/src/models/ChangeModel.test.ts b/packages/server/src/models/ChangeModel.test.ts index 1aae4199ac..9c37241a61 100644 --- a/packages/server/src/models/ChangeModel.test.ts +++ b/packages/server/src/models/ChangeModel.test.ts @@ -325,4 +325,21 @@ describe('ChangeModel', () => { } }); + test('should not return the whole item if the option is disabled', async () => { + const { user } = await createUserAndSession(1, true); + + const changeModel = await models().change(); + changeModel.deltaIncludesItems_ = false; + + await createItemTree3(user.id, '', '', [ + { + id: '000000000000000000000000000000F1', + title: 'Folder 1', + }, + ]); + + const result = await changeModel.delta(user.id); + expect('jopItem' in result.items[0]).toBe(false); + }); + }); diff --git a/packages/server/src/models/ChangeModel.ts b/packages/server/src/models/ChangeModel.ts index b033451366..4bfb14cecb 100644 --- a/packages/server/src/models/ChangeModel.ts +++ b/packages/server/src/models/ChangeModel.ts @@ -16,7 +16,7 @@ export const defaultChangeTtl = 180 * Day; export interface DeltaChange extends Change { jop_updated_time?: number; - jopItem: any; + jopItem?: any; } export type PaginatedDeltaChanges = PaginatedResults; @@ -53,7 +53,7 @@ export function requestDeltaPagination(query: any): ChangePagination { export default class ChangeModel extends BaseModel { - private deltaIncludesItems_: boolean; + public deltaIncludesItems_: boolean; public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { super(db, modelFactory, config); @@ -266,12 +266,14 @@ export default class ChangeModel extends BaseModel { const finalChanges = processedChanges.map(change => { const item = items.find(item => item.id === change.item_id); - if (!item) return { ...change, jopItem: null }; + if (!item) return this.deltaIncludesItems_ ? { ...change, jopItem: null } : { ...change }; const deltaChange: DeltaChange = { ...change, jop_updated_time: item.jop_updated_time, - jopItem: this.deltaIncludesItems_ && item.jop_type ? this.models().item().itemToJoplinItem(item) : null, }; + if (this.deltaIncludesItems_) { + deltaChange.jopItem = item.jop_type ? this.models().item().itemToJoplinItem(item) : null; + } return deltaChange; });