From d84e0eead98bb69070bf3d4400349dd6e5da61a8 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 11 Mar 2019 10:44:46 -0400 Subject: [PATCH 1/2] Respond to TIOLI feedback from https://trello.com/c/ntO5hBbl/4161-prs-for-michael-to-review --- .../Interactions/OWSLinkPreview.swift | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift index 556505ccc..ec0a54ba6 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift +++ b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift @@ -531,11 +531,9 @@ public class OWSLinkPreview: MTLModel { private static var linkPreviewDraftCache: NSCache = NSCache() private class func cachedLinkPreview(forPreviewUrl previewUrl: String) -> OWSLinkPreviewDraft? { - var result: OWSLinkPreviewDraft? - serialQueue.sync { - result = linkPreviewDraftCache.object(forKey: previewUrl as NSString) + return serialQueue.sync { + return linkPreviewDraftCache.object(forKey: previewUrl as NSString) } - return result } private class func setCachedLinkPreview(_ linkPreviewDraft: OWSLinkPreviewDraft, @@ -561,7 +559,6 @@ public class OWSLinkPreview: MTLModel { } public class func tryToBuildPreviewInfo(previewUrl: String?) -> Promise { - guard OWSLinkPreview.featureEnabled else { return Promise(error: LinkPreviewError.featureDisabled) } @@ -578,14 +575,13 @@ public class OWSLinkPreview: MTLModel { return downloadLink(url: previewUrl) .then(on: DispatchQueue.global()) { (data) -> Promise in return parseLinkDataAndBuildDraft(linkData: data, linkUrlString: previewUrl) - .then(on: DispatchQueue.global()) { (linkPreviewDraft) -> Promise in - guard linkPreviewDraft.isValid() else { - return Promise(error: LinkPreviewError.noPreview) - } - setCachedLinkPreview(linkPreviewDraft, forPreviewUrl: previewUrl) - - return Promise.value(linkPreviewDraft) + }.then(on: DispatchQueue.global()) { (linkPreviewDraft) -> Promise in + guard linkPreviewDraft.isValid() else { + return Promise(error: LinkPreviewError.noPreview) } + setCachedLinkPreview(linkPreviewDraft, forPreviewUrl: previewUrl) + + return Promise.value(linkPreviewDraft) } } @@ -761,18 +757,18 @@ public class OWSLinkPreview: MTLModel { } return downloadImage(url: imageUrl, imageMimeType: imageMimeType) - .then(on: DispatchQueue.global()) { (imageData: Data) -> Promise in + .map(on: DispatchQueue.global()) { (imageData: Data) -> OWSLinkPreviewDraft in // We always recompress images to Jpeg. let imageFilePath = OWSFileSystem.temporaryFilePath(withFileExtension: "jpg") do { try imageData.write(to: NSURL.fileURL(withPath: imageFilePath), options: .atomicWrite) } catch let error as NSError { owsFailDebug("file write failed: \(imageFilePath), \(error)") - return Promise(error: LinkPreviewError.assertionFailure) + throw LinkPreviewError.assertionFailure } let linkPreviewDraft = OWSLinkPreviewDraft(urlString: linkUrlString, title: title, imageFilePath: imageFilePath) - return Promise.value(linkPreviewDraft) + return linkPreviewDraft } .recover(on: DispatchQueue.global()) { (_) -> Promise in return Promise.value(OWSLinkPreviewDraft(urlString: linkUrlString, title: title)) From 10383783e3beeda1c0d94c04452cb8a06a0de39a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 11 Mar 2019 23:33:25 -0400 Subject: [PATCH 2/2] Respond to CR. --- SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift index ec0a54ba6..9bb11b4de 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift +++ b/SignalServiceKit/src/Messages/Interactions/OWSLinkPreview.swift @@ -577,7 +577,7 @@ public class OWSLinkPreview: MTLModel { return parseLinkDataAndBuildDraft(linkData: data, linkUrlString: previewUrl) }.then(on: DispatchQueue.global()) { (linkPreviewDraft) -> Promise in guard linkPreviewDraft.isValid() else { - return Promise(error: LinkPreviewError.noPreview) + throw LinkPreviewError.noPreview } setCachedLinkPreview(linkPreviewDraft, forPreviewUrl: previewUrl)