diff --git a/ZShare/DbRequests/CreateBackendItemDbRequest.swift b/ZShare/DbRequests/CreateBackendItemDbRequest.swift index dad3fc1c6..719183bb0 100644 --- a/ZShare/DbRequests/CreateBackendItemDbRequest.swift +++ b/ZShare/DbRequests/CreateBackendItemDbRequest.swift @@ -24,7 +24,14 @@ struct CreateBackendItemDbRequest: DbResponseRequest { throw DbError.objectNotFound } - _ = try StoreItemsDbResponseRequest(responses: [self.item], schemaController: self.schemaController, dateParser: self.dateParser, preferResponseData: true).process(in: database) + _ = try StoreItemsDbResponseRequest( + responses: [self.item], + schemaController: self.schemaController, + dateParser: self.dateParser, + preferResponseData: true, + denyIncorrectCreator: false + ) + .process(in: database) guard let item = database.objects(RItem.self).filter(.key(self.item.key, in: libraryId)).first else { throw DbError.objectNotFound diff --git a/ZShare/DbRequests/CreateItemWithAttachmentDbRequest.swift b/ZShare/DbRequests/CreateItemWithAttachmentDbRequest.swift index 59d518222..d21552d1e 100644 --- a/ZShare/DbRequests/CreateItemWithAttachmentDbRequest.swift +++ b/ZShare/DbRequests/CreateItemWithAttachmentDbRequest.swift @@ -21,10 +21,14 @@ struct CreateItemWithAttachmentDbRequest: DbResponseRequest { var needsWrite: Bool { return true } func process(in database: Realm) throws -> (RItem, RItem) { - _ = try StoreItemsDbResponseRequest(responses: [self.item], - schemaController: self.schemaController, - dateParser: self.dateParser, - preferResponseData: true).process(in: database) + _ = try StoreItemsDbResponseRequest( + responses: [self.item], + schemaController: self.schemaController, + dateParser: self.dateParser, + preferResponseData: true, + denyIncorrectCreator: false + ) + .process(in: database) guard let item = database.objects(RItem.self).filter(.key(self.item.key, in: self.attachment.libraryId)).first else { throw DbError.objectNotFound @@ -35,12 +39,15 @@ struct CreateItemWithAttachmentDbRequest: DbResponseRequest { item.fields.forEach({ $0.changed = true }) let localizedType = self.schemaController.localized(itemType: ItemTypes.attachment) ?? "" - let attachment = try CreateAttachmentDbRequest(attachment: self.attachment, - parentKey: nil, - localizedType: localizedType, - includeAccessDate: self.attachment.hasUrl, - collections: [], - tags: []).process(in: database) + let attachment = try CreateAttachmentDbRequest( + attachment: self.attachment, + parentKey: nil, + localizedType: localizedType, + includeAccessDate: self.attachment.hasUrl, + collections: [], + tags: [] + ) + .process(in: database) attachment.parent = item attachment.changes.append(RObjectChange.create(changes: RItemChanges.parent)) diff --git a/Zotero/Controllers/Database/Requests/CreateTranslatedItemsDbRequest.swift b/Zotero/Controllers/Database/Requests/CreateTranslatedItemsDbRequest.swift index f490b3487..b3687eadd 100644 --- a/Zotero/Controllers/Database/Requests/CreateTranslatedItemsDbRequest.swift +++ b/Zotero/Controllers/Database/Requests/CreateTranslatedItemsDbRequest.swift @@ -20,7 +20,14 @@ struct CreateTranslatedItemsDbRequest: DbRequest { func process(in database: Realm) throws { for response in self.responses { - let (item, _) = try StoreItemDbRequest(response: response, schemaController: self.schemaController, dateParser: self.dateParser, preferRemoteData: true).process(in: database) + let (item, _) = try StoreItemDbRequest( + response: response, + schemaController: self.schemaController, + dateParser: self.dateParser, + preferRemoteData: true, + denyIncorrectCreator: false + ) + .process(in: database) item.changeType = .user for field in item.fields { diff --git a/Zotero/Controllers/Database/Requests/MarkObjectsAsSyncedDbRequest.swift b/Zotero/Controllers/Database/Requests/MarkObjectsAsSyncedDbRequest.swift index 1dab5c259..b39e9be82 100644 --- a/Zotero/Controllers/Database/Requests/MarkObjectsAsSyncedDbRequest.swift +++ b/Zotero/Controllers/Database/Requests/MarkObjectsAsSyncedDbRequest.swift @@ -119,7 +119,7 @@ struct MarkItemAsSyncedAndUpdateDbRequest: DbRequest { guard let item = database.objects(RItem.self).filter(.key(self.response.key, in: self.libraryId)).first else { return } item.deleteChanges(uuids: self.changeUuids, database: database) - self.updateUnchangedData(of: item, with: self.response, database: database) + try self.updateUnchangedData(of: item, with: self.response, database: database) if let parent = item.parent { // This is to mitigate the issue in item detail screen (ItemDetailActionHandler.shouldReloadData) where observing of `children` doesn't report changes between `oldValue` and `newValue`. @@ -127,11 +127,19 @@ struct MarkItemAsSyncedAndUpdateDbRequest: DbRequest { } } - private func updateUnchangedData(of item: RItem, with response: ItemResponse, database: Realm) { + private func updateUnchangedData(of item: RItem, with response: ItemResponse, database: Realm) throws { let localChanges = item.changedFields if localChanges.isEmpty { - _ = StoreItemDbRequest.update(item: item, libraryId: self.libraryId, with: response, schemaController: self.schemaController, dateParser: self.dateParser, database: database) + _ = try StoreItemDbRequest.update( + item: item, + libraryId: self.libraryId, + with: response, + denyIncorrectCreator: false, + schemaController: self.schemaController, + dateParser: self.dateParser, + database: database + ) item.changeType = .syncResponse return } @@ -168,7 +176,7 @@ struct MarkItemAsSyncedAndUpdateDbRequest: DbRequest { } if !localChanges.contains(.creators) { - StoreItemDbRequest.sync(creators: response.creators, item: item, schemaController: self.schemaController, database: database) + try StoreItemDbRequest.sync(creators: response.creators, item: item, denyIncorrectCreator: false, schemaController: self.schemaController, database: database) } if !localChanges.contains(.relations) { diff --git a/Zotero/Controllers/Database/Requests/StoreItemsDbResponseRequest.swift b/Zotero/Controllers/Database/Requests/StoreItemsDbResponseRequest.swift index 6c7c4ec32..def5c44ac 100644 --- a/Zotero/Controllers/Database/Requests/StoreItemsDbResponseRequest.swift +++ b/Zotero/Controllers/Database/Requests/StoreItemsDbResponseRequest.swift @@ -22,6 +22,8 @@ struct StoreItemsResponse { enum Error: Swift.Error { case itemDeleted(ItemResponse) case itemChanged(ItemResponse) + case noValidCreators(key: String, itemType: String) + case invalidCreator(key: String, creatorType: String) } let changedFilenames: [FilenameChange] @@ -35,6 +37,7 @@ struct StoreItemsDbResponseRequest: DbResponseRequest { unowned let schemaController: SchemaController unowned let dateParser: DateParser let preferResponseData: Bool + let denyIncorrectCreator: Bool var needsWrite: Bool { return true } @@ -44,7 +47,14 @@ struct StoreItemsDbResponseRequest: DbResponseRequest { for response in self.responses { do { - let (_, change) = try StoreItemDbRequest(response: response, schemaController: self.schemaController, dateParser: self.dateParser, preferRemoteData: self.preferResponseData).process(in: database) + let (_, change) = try StoreItemDbRequest( + response: response, + schemaController: self.schemaController, + dateParser: self.dateParser, + preferRemoteData: self.preferResponseData, + denyIncorrectCreator: self.denyIncorrectCreator + ) + .process(in: database) if let change = change { filenameChanges.append(change) } @@ -66,6 +76,7 @@ struct StoreItemDbRequest: DbResponseRequest { unowned let schemaController: SchemaController unowned let dateParser: DateParser let preferRemoteData: Bool + let denyIncorrectCreator: Bool var needsWrite: Bool { return true } @@ -96,10 +107,26 @@ struct StoreItemDbRequest: DbResponseRequest { item.attachmentNeedsSync = false } - return StoreItemDbRequest.update(item: item, libraryId: libraryId, with: self.response, schemaController: self.schemaController, dateParser: self.dateParser, database: database) + return try StoreItemDbRequest.update( + item: item, + libraryId: libraryId, + with: self.response, + denyIncorrectCreator: self.denyIncorrectCreator, + schemaController: self.schemaController, + dateParser: self.dateParser, + database: database + ) } - static func update(item: RItem, libraryId: LibraryIdentifier, with response: ItemResponse, schemaController: SchemaController, dateParser: DateParser, database: Realm) -> (RItem, StoreItemsResponse.FilenameChange?) { + static func update( + item: RItem, + libraryId: LibraryIdentifier, + with response: ItemResponse, + denyIncorrectCreator: Bool, + schemaController: SchemaController, + dateParser: DateParser, + database: Realm + ) throws -> (RItem, StoreItemsResponse.FilenameChange?) { item.key = response.key item.rawType = response.rawType item.localizedType = schemaController.localized(itemType: response.rawType) ?? "" @@ -118,7 +145,7 @@ struct StoreItemDbRequest: DbResponseRequest { self.syncParent(key: response.parentKey, libraryId: libraryId, item: item, database: database) self.syncCollections(keys: response.collectionKeys, libraryId: libraryId, item: item, database: database) self.sync(tags: response.tags, libraryId: libraryId, item: item, database: database) - self.sync(creators: response.creators, item: item, schemaController: schemaController, database: database) + try self.sync(creators: response.creators, item: item, denyIncorrectCreator: denyIncorrectCreator, schemaController: schemaController, database: database) self.sync(relations: response.relations, item: item, database: database) self.syncLinks(data: response, item: item, database: database) self.syncUsers(createdBy: response.createdBy, lastModifiedBy: response.lastModifiedBy, item: item, database: database) @@ -406,12 +433,21 @@ struct StoreItemDbRequest: DbResponseRequest { } } - static func sync(creators: [CreatorResponse], item: RItem, schemaController: SchemaController, database: Realm) { + static func sync(creators: [CreatorResponse], item: RItem, denyIncorrectCreator: Bool, schemaController: SchemaController, database: Realm) throws { + switch item.rawType { + case ItemTypes.annotation, ItemTypes.attachment, ItemTypes.note: + // These item types don't support creators, so `validCreators` would always be empty. + return + + default: + break + } + database.delete(item.creators) guard let validCreators = schemaController.creators(for: item.rawType), !validCreators.isEmpty else { DDLogError("StoreItemsDbResponseRequest: can't find valid creators for item type \(item.rawType). Skipping creators.") - return + throw StoreItemsResponse.Error.noValidCreators(key: item.key, itemType: item.rawType) } for (idx, object) in creators.enumerated() { @@ -423,6 +459,8 @@ struct StoreItemDbRequest: DbResponseRequest { if validCreators.contains(where: { $0.creatorType == object.creatorType }) { creator.rawType = object.creatorType + } else if denyIncorrectCreator { + throw StoreItemsResponse.Error.invalidCreator(key: item.key, creatorType: object.creatorType) } else if let primaryCreator = validCreators.first(where: { $0.primary }) { DDLogError("StoreItemsDbResponseRquest: creator type '\(object.creatorType)' isn't valid for \(item.rawType) - changing to primary creator") creator.rawType = primaryCreator.creatorType diff --git a/Zotero/Controllers/Sync/SyncActions/RevertLibraryFilesSyncAction.swift b/Zotero/Controllers/Sync/SyncActions/RevertLibraryFilesSyncAction.swift index 2db6efdae..aed5d6ab1 100644 --- a/Zotero/Controllers/Sync/SyncActions/RevertLibraryFilesSyncAction.swift +++ b/Zotero/Controllers/Sync/SyncActions/RevertLibraryFilesSyncAction.swift @@ -64,7 +64,13 @@ struct RevertLibraryFilesSyncAction: SyncAction { try coordinator.perform(request: DeleteObjectsDbRequest(keys: failedKeys, libraryId: self.libraryId)) // Store cached objects from backend to local database to get rid of local changes. DDLogError("RevertLibraryFilesSyncAction: restore cached objects") - let request = StoreItemsDbResponseRequest(responses: cachedResponses, schemaController: self.schemaController, dateParser: self.dateParser, preferResponseData: true) + let request = StoreItemsDbResponseRequest( + responses: cachedResponses, + schemaController: self.schemaController, + dateParser: self.dateParser, + preferResponseData: true, + denyIncorrectCreator: true + ) changedFilenames = try coordinator.perform(request: request).changedFilenames coordinator.invalidate() } diff --git a/Zotero/Controllers/Sync/SyncActions/RevertLibraryUpdatesSyncAction.swift b/Zotero/Controllers/Sync/SyncActions/RevertLibraryUpdatesSyncAction.swift index 3a68024c6..a5a844601 100644 --- a/Zotero/Controllers/Sync/SyncActions/RevertLibraryUpdatesSyncAction.swift +++ b/Zotero/Controllers/Sync/SyncActions/RevertLibraryUpdatesSyncAction.swift @@ -61,7 +61,13 @@ struct RevertLibraryUpdatesSyncAction: SyncAction { try coordinator.perform(writeRequests: [storeCollectionsRequest, storeSearchesRequest]) // Force response data here, since we're reverting - let storeItemsRequest = StoreItemsDbResponseRequest(responses: items.responses, schemaController: self.schemaController, dateParser: self.dateParser, preferResponseData: true) + let storeItemsRequest = StoreItemsDbResponseRequest( + responses: items.responses, + schemaController: self.schemaController, + dateParser: self.dateParser, + preferResponseData: true, + denyIncorrectCreator: true + ) changes = try coordinator.perform(request: storeItemsRequest).changedFilenames failedCollections = collections.failed diff --git a/Zotero/Controllers/Sync/SyncBatchProcessor.swift b/Zotero/Controllers/Sync/SyncBatchProcessor.swift index 73be4c119..3da85c07e 100644 --- a/Zotero/Controllers/Sync/SyncBatchProcessor.swift +++ b/Zotero/Controllers/Sync/SyncBatchProcessor.swift @@ -162,7 +162,7 @@ final class SyncBatchProcessor { self.storeIndividualObjects(from: objects, type: .item, libraryId: libraryId) // BETA: - forcing preferResponseData to true for beta, it should be false here so that we report conflicts - let request = StoreItemsDbResponseRequest(responses: items, schemaController: self.schemaController, dateParser: self.dateParser, preferResponseData: true) + let request = StoreItemsDbResponseRequest(responses: items, schemaController: self.schemaController, dateParser: self.dateParser, preferResponseData: true, denyIncorrectCreator: true) let response = try self.dbStorage.perform(request: request, on: self.storageQueue, invalidateRealm: true) let failedKeys = self.failedKeys(from: expectedKeys, parsedKeys: items.map({ $0.key }), errors: errors) diff --git a/ZoteroTests/SyncActionsSpec.swift b/ZoteroTests/SyncActionsSpec.swift index b60b52a27..fdc254632 100644 --- a/ZoteroTests/SyncActionsSpec.swift +++ b/ZoteroTests/SyncActionsSpec.swift @@ -84,7 +84,8 @@ final class SyncActionsSpec: QuickSpec { responses: [itemResponse], schemaController: TestControllers.schemaController, dateParser: TestControllers.dateParser, - preferResponseData: true + preferResponseData: true, + denyIncorrectCreator: true ), on: .main ) @@ -229,7 +230,8 @@ final class SyncActionsSpec: QuickSpec { responses: [itemResponse], schemaController: TestControllers.schemaController, dateParser: TestControllers.dateParser, - preferResponseData: true + preferResponseData: true, + denyIncorrectCreator: true ), on: .main ) @@ -346,7 +348,8 @@ final class SyncActionsSpec: QuickSpec { responses: [itemResponse], schemaController: TestControllers.schemaController, dateParser: TestControllers.dateParser, - preferResponseData: true + preferResponseData: true, + denyIncorrectCreator: false ), on: .main ) diff --git a/ZoteroTests/WebDavControllerSpec.swift b/ZoteroTests/WebDavControllerSpec.swift index 1e6bb0158..9354d04fc 100644 --- a/ZoteroTests/WebDavControllerSpec.swift +++ b/ZoteroTests/WebDavControllerSpec.swift @@ -130,7 +130,7 @@ final class WebDavControllerSpec: QuickSpec { switch error { case .sessionTaskFailed(let error): let nsError = error as NSError - if nsError.code == NSURLErrorCannotConnectToHost { + if nsError.code == NSURLErrorCannotConnectToHost || nsError.code == NSURLErrorAppTransportSecurityRequiresSecureConnection { finished() return }