From d4c43a8b677ff6a75cbd0dd833a334804c8c5a04 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Mon, 15 May 2023 17:49:26 +0100 Subject: [PATCH] Desktop, Server: Improved handling of items with duplicate IDs --- packages/lib/JoplinServerApi.ts | 11 ++++++----- packages/lib/file-api-driver-joplinServer.ts | 16 ++++++++++++++-- .../lib/services/synchronizer/ItemUploader.ts | 4 +++- packages/server/src/models/ItemModel.ts | 17 ++++++++++++++--- 4 files changed, 37 insertions(+), 11 deletions(-) diff --git a/packages/lib/JoplinServerApi.ts b/packages/lib/JoplinServerApi.ts index ffb89fe6b72..3cad5560859 100644 --- a/packages/lib/JoplinServerApi.ts +++ b/packages/lib/JoplinServerApi.ts @@ -45,12 +45,13 @@ export default class JoplinServerApi { private options_: Options; private session_: Session; private debugRequests_: boolean = false; + private debugRequestsShowPasswords_: boolean = false; public constructor(options: Options) { this.options_ = options; - if (options.env === Env.Dev) { - // this.debugRequests_ = true; + if (options.env !== Env.Dev) { + this.debugRequestsShowPasswords_ = false; } } @@ -97,15 +98,15 @@ export default class JoplinServerApi { try { const output = JSON.parse(o); if (!output) return o; - if (output.password) output.password = '******'; + if (output.password && !this.debugRequestsShowPasswords_) output.password = '******'; return JSON.stringify(output); } catch (error) { return o; } } else { const output = { ...o }; - if (output.password) output.password = '******'; - if (output['X-API-AUTH']) output['X-API-AUTH'] = '******'; + if (output.password && !this.debugRequestsShowPasswords_) output.password = '******'; + if (output['X-API-AUTH'] && !this.debugRequestsShowPasswords_) output['X-API-AUTH'] = '******'; return output; } } diff --git a/packages/lib/file-api-driver-joplinServer.ts b/packages/lib/file-api-driver-joplinServer.ts index fa3fddbd420..43a43eaa026 100644 --- a/packages/lib/file-api-driver-joplinServer.ts +++ b/packages/lib/file-api-driver-joplinServer.ts @@ -175,6 +175,10 @@ export default class FileApiDriverJoplinServer { // they can have names such as ".resources/xxxxxxxxxx' } + private isRejectedBySyncTargetError(error: any) { + return error.code === 413 || error.code === 409 || error.httpCode === 413 || error.httpCode === 409; + } + public async put(path: string, content: any, options: any = null) { try { const output = await this.api().exec('PUT', `${this.apiFilePath_(path)}/content`, options && options.shareId ? { share_id: options.shareId } : null, content, { @@ -182,7 +186,7 @@ export default class FileApiDriverJoplinServer { }, options); return output; } catch (error) { - if (error.code === 413) { + if (this.isRejectedBySyncTargetError(error)) { throw new JoplinError(error.message, 'rejectedByTarget'); } throw error; @@ -190,7 +194,15 @@ export default class FileApiDriverJoplinServer { } public async multiPut(items: MultiPutItem[], options: any = null) { - return this.api().exec('PUT', 'api/batch_items', null, { items: items }, null, options); + const output = await this.api().exec('PUT', 'api/batch_items', null, { items: items }, null, options); + + for (const [, response] of Object.entries(output.items)) { + if (response.error && this.isRejectedBySyncTargetError(response.error)) { + response.error.code = 'rejectedByTarget'; + } + } + + return output; } public async delete(path: string) { diff --git a/packages/lib/services/synchronizer/ItemUploader.ts b/packages/lib/services/synchronizer/ItemUploader.ts index 5581c55857f..644a888275a 100644 --- a/packages/lib/services/synchronizer/ItemUploader.ts +++ b/packages/lib/services/synchronizer/ItemUploader.ts @@ -1,5 +1,6 @@ import { ModelType } from '../../BaseModel'; import { FileApi, MultiPutItem } from '../../file-api'; +import JoplinError from '../../JoplinError'; import Logger from '../../Logger'; import BaseItem from '../../models/BaseItem'; import { BaseItemEntity } from '../database/types'; @@ -45,7 +46,8 @@ export default class ItemUploader { // the regular upload. logger.warn(`Pre-uploaded item updated_time has changed. It is going to be re-uploaded again: ${path} (From ${this.preUploadedItemUpdatedTimes_[path]} to ${local.updated_time})`); } else { - if (preUploadItem.error) throw new Error(preUploadItem.error.message ? preUploadItem.error.message : 'Unknown pre-upload error'); + const error = preUploadItem.error; + if (error) throw new JoplinError(error.message ? error.message : 'Unknown pre-upload error', error.code); return; } } diff --git a/packages/server/src/models/ItemModel.ts b/packages/server/src/models/ItemModel.ts index 108c022312b..ea085d5633d 100644 --- a/packages/server/src/models/ItemModel.ts +++ b/packages/server/src/models/ItemModel.ts @@ -3,12 +3,12 @@ import { ItemType, databaseSchema, Uuid, Item, ShareType, Share, ChangeType, Use import { defaultPagination, paginateDbQuery, PaginatedResults, Pagination } from './utils/pagination'; import { isJoplinItemName, isJoplinResourceBlobPath, linkedResourceIds, serializeJoplinItem, unserializeJoplinItem } from '../utils/joplinUtils'; import { ModelType } from '@joplin/lib/BaseModel'; -import { ApiError, ErrorCode, ErrorForbidden, ErrorPayloadTooLarge, ErrorUnprocessableEntity } from '../utils/errors'; +import { ApiError, ErrorCode, ErrorConflict, ErrorForbidden, ErrorPayloadTooLarge, ErrorUnprocessableEntity } from '../utils/errors'; import { Knex } from 'knex'; import { ChangePreviousItem } from './ChangeModel'; import { unique } from '../utils/array'; import StorageDriverBase, { Context } from './items/storage/StorageDriverBase'; -import { DbConnection, returningSupported } from '../db'; +import { DbConnection, isUniqueConstraintError, returningSupported } from '../db'; import { Config, StorageDriverConfig, StorageDriverMode } from '../utils/types'; import { NewModelFactoryHandler } from './factory'; import loadStorageDriver from './items/storage/loadStorageDriver'; @@ -21,6 +21,8 @@ const mimeUtils = require('@joplin/lib/mime-utils.js').mime; // Converts "root:/myfile.txt:" to "myfile.txt" const extractNameRegex = /^root:\/(.*):$/; +const modelLogger = Logger.create('ItemModel'); + export interface DeleteOptions extends BaseDeleteOptions { deleteChanges?: boolean; } @@ -928,7 +930,16 @@ export default class ItemModel extends BaseModel { } return this.withTransaction(async () => { - item = await super.save(item, options); + try { + item = await super.save(item, options); + } catch (error) { + if (isUniqueConstraintError(error)) { + modelLogger.error(`Unique constraint error on item: ${JSON.stringify({ id: item.id, name: item.name, jop_id: item.jop_id, owner_id: item.owner_id })}`, error); + throw new ErrorConflict(`This item is already present and cannot be added again: ${item.jop_id}`); + } else { + throw error; + } + } if (isNew) await this.models().userItem().add(userId, item.id);