From c9052f5f81069944b32dc840f197ee218f71ffdf Mon Sep 17 00:00:00 2001 From: Satindar Dhillon Date: Mon, 25 Apr 2022 17:50:01 -0700 Subject: [PATCH] use coredata to track highlight merge and updates --- .../App/PDFSupport/PDFViewerViewModel.swift | 33 ++----- .../App/Views/LinkItemDetailView.swift | 2 +- .../Views/WebReader/WebReaderViewModel.swift | 69 +++++++------- .../Sources/Models/DataModels/Highlight.swift | 56 +++++++++++ .../Mutations/CreateHighlight.swift | 36 ++----- .../Mutations/DeleteHighlight.swift | 77 +++++++-------- .../Mutations/MergeHighlight.swift | 95 +++++++++++++------ .../DataService/Mutations/RemoveLink.swift | 1 + .../UpdateArticleReadingProgress.swift | 1 - .../Mutations/UpdateHighlightAttributes.swift | 64 +++++++------ 10 files changed, 243 insertions(+), 191 deletions(-) create mode 100644 apple/OmnivoreKit/Sources/Models/DataModels/Highlight.swift diff --git a/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift b/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift index 7d0b46e52..bf2404ebc 100644 --- a/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/PDFSupport/PDFViewerViewModel.swift @@ -39,32 +39,19 @@ public final class PDFViewerViewModel: ObservableObject { patch: String, overlapHighlightIdList: [String] ) { - services.dataService - .mergeHighlightPublisher( - shortId: shortId, - highlightID: highlightID, - quote: quote, - patch: patch, - articleId: linkedItem.unwrappedID, - overlapHighlightIdList: overlapHighlightIdList - ) - .sink { [weak self] completion in - guard case let .failure(error) = completion else { return } - self?.errorMessage = error.localizedDescription - } receiveValue: { _ in } - .store(in: &subscriptions) + _ = services.dataService.mergeHighlights( + shortId: shortId, + highlightID: highlightID, + quote: quote, + patch: patch, + articleId: linkedItem.unwrappedID, + overlapHighlightIdList: overlapHighlightIdList + ) } public func removeHighlights(highlightIds: [String]) { - highlightIds.forEach { highlightId in - services.dataService.deleteHighlightPublisher(highlightId: highlightId) - .sink { [weak self] completion in - guard case let .failure(error) = completion else { return } - self?.errorMessage = error.localizedDescription - } receiveValue: { value in - print("remove highlight value", value) - } - .store(in: &subscriptions) + highlightIds.forEach { highlightID in + services.dataService.deleteHighlight(highlightID: highlightID) } } diff --git a/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift b/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift index 3d41f40f2..876564b45 100644 --- a/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift +++ b/apple/OmnivoreKit/Sources/App/Views/LinkItemDetailView.swift @@ -82,7 +82,7 @@ enum PDFProvider { rawAuthCookie: rawAuthCookie ) - newWebAppWrapperViewModel.performActionSubject.sink { [weak self] action in + newWebAppWrapperViewModel.performActionSubject.sink { action in switch action { case let .shareHighlight(highlightID): print("show share modal for highlight with id: \(highlightID)") diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift index 4db2620dd..ce0c9171a 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderViewModel.swift @@ -60,16 +60,12 @@ final class WebReaderViewModel: ObservableObject { replyHandler: @escaping WKScriptMessageReplyHandler, dataService: DataService ) { - dataService.deleteHighlightPublisher( - highlightId: messageBody["highlightId"] as? String ?? "" - ) - .sink { completion in - guard case .failure = completion else { return } - replyHandler(["result": false], nil) - } receiveValue: { _ in + if let highlightID = messageBody["highlightId"] as? String { + dataService.deleteHighlight(highlightID: highlightID) replyHandler(["result": true], nil) + } else { + replyHandler(["result": false], nil) } - .store(in: &subscriptions) } func mergeHighlight( @@ -77,25 +73,28 @@ final class WebReaderViewModel: ObservableObject { replyHandler: @escaping WKScriptMessageReplyHandler, dataService: DataService ) { - dataService.mergeHighlightPublisher( - shortId: messageBody["shortId"] as? String ?? "", - highlightID: messageBody["id"] as? String ?? "", - quote: messageBody["quote"] as? String ?? "", - patch: messageBody["patch"] as? String ?? "", - articleId: messageBody["articleId"] as? String ?? "", - overlapHighlightIdList: messageBody["overlapHighlightIdList"] as? [String] ?? [] - ) - .sink { completion in - guard case .failure = completion else { return } - replyHandler([], "mergeHighlight: Error encoding response") - } receiveValue: { result in - if let highlightValue = result { - replyHandler(["result": highlightValue], nil) - } else { - replyHandler([], "createHighlight: Error encoding response") - } + guard + let shortId = messageBody["shortId"] as? String, + let highlightID = messageBody["id"] as? String, + let quote = messageBody["quote"] as? String, + let patch = messageBody["patch"] as? String, + let articleId = messageBody["articleId"] as? String, + let overlapHighlightIdList = messageBody["overlapHighlightIdList"] as? [String] + else { + replyHandler([], "createHighlight: Error encoding response") + return } - .store(in: &subscriptions) + + let jsonHighlight = dataService.mergeHighlights( + shortId: shortId, + highlightID: highlightID, + quote: quote, + patch: patch, + articleId: articleId, + overlapHighlightIdList: overlapHighlightIdList + ) + + replyHandler(["result": jsonHighlight], nil) } func updateHighlight( @@ -103,19 +102,15 @@ final class WebReaderViewModel: ObservableObject { replyHandler: @escaping WKScriptMessageReplyHandler, dataService: DataService ) { - dataService.updateHighlightAttributesPublisher( - highlightID: messageBody["highlightId"] as? String ?? "", - annotation: messageBody["annotation"] as? String ?? "", - sharedAt: nil - ) - .sink { completion in - guard case .failure = completion else { return } - replyHandler([], "updateHighlight: Error encoding response") - } receiveValue: { highlightID in - // Update highlight JS code just expects the highlight ID back + let highlightID = messageBody["highlightId"] as? String + let annotation = messageBody["annotation"] as? String + + if let highlightID = highlightID, let annotation = annotation { + dataService.updateHighlightAttributes(highlightID: highlightID, annotation: annotation) replyHandler(["result": highlightID], nil) + } else { + replyHandler([], "updateHighlight: Error encoding response") } - .store(in: &subscriptions) } func updateReadingProgress( diff --git a/apple/OmnivoreKit/Sources/Models/DataModels/Highlight.swift b/apple/OmnivoreKit/Sources/Models/DataModels/Highlight.swift new file mode 100644 index 000000000..0f78f36ef --- /dev/null +++ b/apple/OmnivoreKit/Sources/Models/DataModels/Highlight.swift @@ -0,0 +1,56 @@ +import CoreData +import Foundation + +public extension Highlight { + var unwrappedID: String { + id ?? "" + } + + static func lookup(byID highlightID: String, inContext context: NSManagedObjectContext) -> Highlight? { + let fetchRequest: NSFetchRequest = Highlight.fetchRequest() + fetchRequest.predicate = NSPredicate( + format: "id == %@", highlightID + ) + + var highlight: Highlight? + + context.performAndWait { + highlight = (try? context.fetch(fetchRequest))?.first + } + + return highlight + } + + func update( + inContext context: NSManagedObjectContext, + newAnnotation: String + ) { + context.perform { + self.annotation = newAnnotation + + guard context.hasChanges else { return } + + do { + try context.save() + logger.debug("Highlight updated succesfully") + } catch { + context.rollback() + logger.debug("Failed to update Highlight: \(error.localizedDescription)") + } + } + } + + func remove(inContext context: NSManagedObjectContext) { + context.perform { + context.delete(self) + + do { + try context.save() + logger.debug("Highlight removed") + } catch { + context.rollback() + logger.debug("Failed to remove Highlight: \(error.localizedDescription)") + } + } + } +} diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/CreateHighlight.swift b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/CreateHighlight.swift index 9afba24a0..9a8203ce0 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/CreateHighlight.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/CreateHighlight.swift @@ -27,26 +27,12 @@ extension DataService { internalHighlight.persist(context: backgroundContext, associatedItemID: articleId) // Send update to server - syncHighlightCreation( - shortId: shortId, - highlightID: highlightID, - quote: quote, - patch: patch, - articleId: articleId, - annotation: annotation - ) + syncHighlightCreation(highlight: internalHighlight, articleId: articleId) return internalHighlight.encoded() } - func syncHighlightCreation( - shortId: String, - highlightID: String, - quote: String, - patch: String, - articleId: String, - annotation: String? - ) { + func syncHighlightCreation(highlight: InternalHighlight, articleId: String) { enum MutationResult { case saved(highlight: InternalHighlight) case error(errorCode: Enums.CreateHighlightErrorCode) @@ -64,12 +50,12 @@ extension DataService { let mutation = Selection.Mutation { try $0.createHighlight( input: InputObjects.CreateHighlightInput( - id: highlightID, - shortId: shortId, + id: highlight.id, + shortId: highlight.shortId, articleId: articleId, - patch: patch, - quote: quote, - annotation: OptionalArgument(annotation) + patch: highlight.patch, + quote: highlight.quote, + annotation: OptionalArgument(highlight.annotation) ), selection: selection ) @@ -85,12 +71,10 @@ extension DataService { context.perform { let fetchRequest: NSFetchRequest = Highlight.fetchRequest() - fetchRequest.predicate = NSPredicate( - format: "id == %@", highlightID - ) + fetchRequest.predicate = NSPredicate(format: "id == %@", highlight.id) - guard let highlight = (try? context.fetch(fetchRequest))?.first else { return } - highlight.serverSyncStatus = Int64(syncStatus.rawValue) + guard let highlightObject = (try? context.fetch(fetchRequest))?.first else { return } + highlightObject.serverSyncStatus = Int64(syncStatus.rawValue) do { try context.save() diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/DeleteHighlight.swift b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/DeleteHighlight.swift index 1461437e6..3050cc471 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/DeleteHighlight.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/DeleteHighlight.swift @@ -1,13 +1,28 @@ -import Combine import CoreData import Foundation import Models import SwiftGraphQL public extension DataService { - func deleteHighlightPublisher( - highlightId: String - ) -> AnyPublisher { + func deleteHighlight(highlightID: String) { + if let highlight = Highlight.lookup(byID: highlightID, inContext: backgroundContext) { + deleteHighlight(objectID: highlight.objectID) + } + } + + private func deleteHighlight(objectID: NSManagedObjectID) { + // Update CoreData + backgroundContext.perform { [weak self] in + guard let self = self else { return } + guard let highlight = self.backgroundContext.object(with: objectID) as? Highlight else { return } + highlight.remove(inContext: self.backgroundContext) + + // Send update to server + self.syncHighlightDeletion(highlightID: highlight.unwrappedID, objectID: objectID) + } + } + + internal func syncHighlightDeletion(highlightID: String, objectID: NSManagedObjectID) { enum MutationResult { case saved(id: String) case error(errorCode: Enums.DeleteHighlightErrorCode) @@ -24,55 +39,31 @@ public extension DataService { let mutation = Selection.Mutation { try $0.deleteHighlight( - highlightId: highlightId.lowercased(), + highlightId: highlightID, selection: selection ) } let path = appEnvironment.graphqlPath let headers = networker.defaultHeaders + let context = backgroundContext - return Deferred { - Future { promise in - send(mutation, to: path, headers: headers) { result in - switch result { - case let .success(payload): - if let graphqlError = payload.errors { - promise(.failure(.message(messageText: "graphql error: \(graphqlError)"))) - } + send(mutation, to: path, headers: headers) { result in + let data = try? result.get() + let syncStatus: ServerSyncStatus = data == nil ? .needsDeletion : .isNSync - switch payload.data { - case let .saved(id: id): - self.deletePersistedHighlight(objectID: id) - promise(.success(id)) - case let .error(errorCode: errorCode): - promise(.failure(.message(messageText: errorCode.rawValue))) - } - case .failure: - promise(.failure(.message(messageText: "graphql error"))) - } + context.perform { + guard let highlight = context.object(with: objectID) as? Highlight else { return } + highlight.serverSyncStatus = Int64(syncStatus.rawValue) + + do { + try context.save() + logger.debug("Highlight deleted succesfully") + } catch { + context.rollback() + logger.debug("Failed to delete Highlight: \(error.localizedDescription)") } } } - .receive(on: DispatchQueue.main) - .eraseToAnyPublisher() - } - - func deletePersistedHighlight(objectID: String) { - backgroundContext.perform { - let fetchRequest: NSFetchRequest = Highlight.fetchRequest() - fetchRequest.predicate = NSPredicate(format: "id == %@", objectID) - for highlight in (try? self.backgroundContext.fetch(fetchRequest)) ?? [] { - self.backgroundContext.delete(highlight) - } - - do { - try self.backgroundContext.save() - print("Highlight deleted succesfully") - } catch { - self.backgroundContext.rollback() - print("Failed to delete Highlight: \(error.localizedDescription)") - } - } } } diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/MergeHighlight.swift b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/MergeHighlight.swift index c892ed01b..d5a0ea97d 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/MergeHighlight.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/MergeHighlight.swift @@ -1,18 +1,44 @@ -import Combine +import CoreData import Foundation import Models import SwiftGraphQL -public extension DataService { +extension DataService { // swiftlint:disable:next function_parameter_count function_body_length - func mergeHighlightPublisher( + public func mergeHighlights( shortId: String, highlightID: String, quote: String, patch: String, articleId: String, - overlapHighlightIdList: [String] - ) -> AnyPublisher<[String: Any]?, BasicError> { + overlapHighlightIdList: [String] // TODO: pass in annotation? + ) -> [String: Any]? { + let internalHighlight = InternalHighlight( + id: highlightID, + shortId: shortId, + quote: quote, + prefix: nil, + suffix: nil, + patch: patch, + annotation: nil, + createdAt: nil, + updatedAt: nil, + createdByMe: true + ) + + internalHighlight.persist( + context: backgroundContext, + associatedItemID: articleId, + oldHighlightsIds: overlapHighlightIdList + ) + + // Send update to server + syncHighlightCreation(highlight: internalHighlight, articleId: articleId) + + return internalHighlight.encoded() + } + + func syncHighlightMerge(highlight: InternalHighlight, articleId: String, overlapHighlightIdList: [String]) { enum MutationResult { case saved(highlight: InternalHighlight) case error(errorCode: Enums.MergeHighlightErrorCode) @@ -30,11 +56,11 @@ public extension DataService { let mutation = Selection.Mutation { try $0.mergeHighlight( input: InputObjects.MergeHighlightInput( - id: highlightID, - shortId: shortId, + id: highlight.id, + shortId: highlight.shortId, articleId: articleId, - patch: patch, - quote: quote, + patch: highlight.patch, + quote: highlight.quote, prefix: .absent(), suffix: .absent(), annotation: .absent(), @@ -46,34 +72,41 @@ public extension DataService { let path = appEnvironment.graphqlPath let headers = networker.defaultHeaders + let context = backgroundContext - return Deferred { - Future { promise in - send(mutation, to: path, headers: headers) { result in - switch result { - case let .success(payload): - if let graphqlError = payload.errors { - promise(.failure(.message(messageText: "graphql error: \(graphqlError)"))) - } + send(mutation, to: path, headers: headers) { result in + let data = try? result.get() + let isSyncSuccess = data != nil - switch payload.data { - case let .saved(highlight: highlight): - highlight.persist( - context: self.backgroundContext, - associatedItemID: articleId, - oldHighlightsIds: overlapHighlightIdList - ) - promise(.success(highlight.encoded())) - case let .error(errorCode: errorCode): - promise(.failure(.message(messageText: errorCode.rawValue))) + context.perform { + let fetchRequest: NSFetchRequest = Highlight.fetchRequest() + fetchRequest.predicate = NSPredicate(format: "id == %@", highlight.id) + + guard let highlightObject = (try? context.fetch(fetchRequest))?.first else { return } + let newHighlightSyncStatus: ServerSyncStatus = data == nil ? .needsCreation : .isNSync + highlightObject.serverSyncStatus = Int64(newHighlightSyncStatus.rawValue) + + for overlapHighlightID in overlapHighlightIdList { + let fetchRequest: NSFetchRequest = Highlight.fetchRequest() + fetchRequest.predicate = NSPredicate(format: "id == %@", overlapHighlightID) + + if let highlightObject = (try? context.fetch(fetchRequest))?.first { + if isSyncSuccess { + highlightObject.remove(inContext: context) + } else { + highlightObject.serverSyncStatus = Int64(ServerSyncStatus.needsDeletion.rawValue) } - case .failure: - promise(.failure(.message(messageText: "graphql error"))) } } + + do { + try context.save() + logger.debug("Highlight merged succesfully") + } catch { + context.rollback() + logger.debug("Failed to create Highlight: \(error.localizedDescription)") + } } } - .receive(on: DispatchQueue.main) - .eraseToAnyPublisher() } } diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/RemoveLink.swift b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/RemoveLink.swift index fb2f7f8da..a4fa75fe4 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/RemoveLink.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/RemoveLink.swift @@ -56,6 +56,7 @@ extension DataService { context.perform { guard let linkedItem = LinkedItem.lookup(byID: itemID, inContext: context) else { return } linkedItem.serverSyncStatus = Int64(syncStatus.rawValue) + // TODO: remove object if network req was sucessful do { try context.save() diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/UpdateArticleReadingProgress.swift b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/UpdateArticleReadingProgress.swift index c9a73b36e..95ba6f308 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/UpdateArticleReadingProgress.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/UpdateArticleReadingProgress.swift @@ -1,4 +1,3 @@ -import Combine import CoreData import Foundation import Models diff --git a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/UpdateHighlightAttributes.swift b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/UpdateHighlightAttributes.swift index f0550bf1a..781306f18 100644 --- a/apple/OmnivoreKit/Sources/Services/DataService/Mutations/UpdateHighlightAttributes.swift +++ b/apple/OmnivoreKit/Sources/Services/DataService/Mutations/UpdateHighlightAttributes.swift @@ -1,15 +1,29 @@ -import Combine import CoreData import Foundation import Models import SwiftGraphQL -public extension DataService { - func updateHighlightAttributesPublisher( +extension DataService { + public func updateHighlightAttributes( highlightID: String, - annotation: String?, - sharedAt: Date? - ) -> AnyPublisher { + annotation: String + ) { + backgroundContext.perform { [weak self] in + guard let self = self else { return } + guard let highlight = Highlight.lookup(byID: highlightID, inContext: self.backgroundContext) else { return } + + highlight.update(inContext: self.backgroundContext, newAnnotation: annotation) + + // Send update to server + self.syncHighlightAttributes( + highlightID: highlightID, + objectID: highlight.objectID, + annotation: annotation + ) + } + } + + func syncHighlightAttributes(highlightID: String, objectID: NSManagedObjectID, annotation: String) { enum MutationResult { case saved(highlight: InternalHighlight) case error(errorCode: Enums.UpdateHighlightErrorCode) @@ -29,7 +43,7 @@ public extension DataService { input: InputObjects.UpdateHighlightInput( highlightId: highlightID, annotation: OptionalArgument(annotation), - sharedAt: OptionalArgument(sharedAt.flatMap { DateTime(from: $0) }) + sharedAt: OptionalArgument(nil) ), selection: selection ) @@ -37,32 +51,24 @@ public extension DataService { let path = appEnvironment.graphqlPath let headers = networker.defaultHeaders + let context = backgroundContext - return Deferred { - Future { promise in - send(mutation, to: path, headers: headers) { result in - switch result { - case let .success(payload): - if let graphqlError = payload.errors { - promise(.failure(.message(messageText: "graphql error: \(graphqlError)"))) - } + send(mutation, to: path, headers: headers) { result in + let data = try? result.get() + let syncStatus: ServerSyncStatus = data == nil ? .needsUpdate : .isNSync - switch payload.data { - case let .saved(highlight: highlight): - self.backgroundContext.perform { - highlight.persist(context: self.backgroundContext, associatedItemID: nil) - } - promise(.success(highlight.id)) - case let .error(errorCode: errorCode): - promise(.failure(.message(messageText: errorCode.rawValue))) - } - case .failure: - promise(.failure(.message(messageText: "graphql error"))) - } + context.perform { + guard let highlight = context.object(with: objectID) as? Highlight else { return } + highlight.serverSyncStatus = Int64(syncStatus.rawValue) + + do { + try context.save() + logger.debug("Highlight updated succesfully") + } catch { + context.rollback() + logger.debug("Failed to update Highlight: \(error.localizedDescription)") } } } - .receive(on: DispatchQueue.main) - .eraseToAnyPublisher() } }