From a0c9a843902c8fdda095cf65e9fa10a0938571a6 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Sun, 1 Oct 2017 14:54:39 -0400 Subject: [PATCH] Clean up ahead of PR. // FREEBIE --- Signal.xcodeproj/project.pbxproj | 16 +++--- .../GifPicker/GifPickerCell.swift | 7 ++- .../GifPicker/GifPickerViewController.swift | 10 ++-- .../{GifManager.swift => GiphyAPI.swift} | 52 +++++++++---------- ...Downloader.swift => GiphyDownloader.swift} | 37 +++++++------ 5 files changed, 65 insertions(+), 57 deletions(-) rename Signal/src/network/{GifManager.swift => GiphyAPI.swift} (88%) rename Signal/src/network/{GifDownloader.swift => GiphyDownloader.swift} (91%) diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index cb4ef9e0c..387675b5a 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -16,7 +16,7 @@ 341BB7491DB727EE001E2975 /* JSQMediaItem+OWS.m in Sources */ = {isa = PBXBuildFile; fileRef = 341BB7481DB727EE001E2975 /* JSQMediaItem+OWS.m */; }; 341F2C0F1F2B8AE700D07D6B /* DebugUIMisc.m in Sources */ = {isa = PBXBuildFile; fileRef = 341F2C0E1F2B8AE700D07D6B /* DebugUIMisc.m */; }; 342FCE6B1EF9C375002690AD /* OWS105AttachmentFilePaths.m in Sources */ = {isa = PBXBuildFile; fileRef = 342FCE6A1EF9C375002690AD /* OWS105AttachmentFilePaths.m */; }; - 3430FE181F7751D4000EC51B /* GifManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3430FE171F7751D4000EC51B /* GifManager.swift */; }; + 3430FE181F7751D4000EC51B /* GiphyAPI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3430FE171F7751D4000EC51B /* GiphyAPI.swift */; }; 34330A5A1E7875FB00DF2FB9 /* fontawesome-webfont.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 34330A591E7875FB00DF2FB9 /* fontawesome-webfont.ttf */; }; 34330A5C1E787A9800DF2FB9 /* dripicons-v2.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 34330A5B1E787A9800DF2FB9 /* dripicons-v2.ttf */; }; 34330A5E1E787BD800DF2FB9 /* ElegantIcons.ttf in Resources */ = {isa = PBXBuildFile; fileRef = 34330A5D1E787BD800DF2FB9 /* ElegantIcons.ttf */; }; @@ -92,7 +92,7 @@ 34CE88EC1F3237260098030F /* OWSProfileManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 34CE88EA1F3237260098030F /* OWSProfileManager.m */; }; 34CE88ED1F3237260098030F /* ProfileFetcherJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = 34CE88EB1F3237260098030F /* ProfileFetcherJob.swift */; }; 34D1F0501F7D45A60066283D /* GifPickerCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 34D1F04F1F7D45A60066283D /* GifPickerCell.swift */; }; - 34D1F0521F7E8EA30066283D /* GifDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 34D1F0511F7E8EA30066283D /* GifDownloader.swift */; }; + 34D1F0521F7E8EA30066283D /* GiphyDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 34D1F0511F7E8EA30066283D /* GiphyDownloader.swift */; }; 34D5CC961EA6AFAD005515DB /* OWSContactsSyncing.m in Sources */ = {isa = PBXBuildFile; fileRef = 34D5CC951EA6AFAD005515DB /* OWSContactsSyncing.m */; }; 34D5CCA91EAE3D30005515DB /* AvatarViewHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = 34D5CCA81EAE3D30005515DB /* AvatarViewHelper.m */; }; 34D5CCB11EAE7E7F005515DB /* SelectRecipientViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 34D5CCB01EAE7E7F005515DB /* SelectRecipientViewController.m */; }; @@ -423,7 +423,7 @@ 341F2C0E1F2B8AE700D07D6B /* DebugUIMisc.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = DebugUIMisc.m; sourceTree = ""; }; 342FCE691EF9C375002690AD /* OWS105AttachmentFilePaths.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = OWS105AttachmentFilePaths.h; path = Migrations/OWS105AttachmentFilePaths.h; sourceTree = ""; }; 342FCE6A1EF9C375002690AD /* OWS105AttachmentFilePaths.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = OWS105AttachmentFilePaths.m; path = Migrations/OWS105AttachmentFilePaths.m; sourceTree = ""; }; - 3430FE171F7751D4000EC51B /* GifManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GifManager.swift; sourceTree = ""; }; + 3430FE171F7751D4000EC51B /* GiphyAPI.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GiphyAPI.swift; sourceTree = ""; }; 34330A591E7875FB00DF2FB9 /* fontawesome-webfont.ttf */ = {isa = PBXFileReference; lastKnownFileType = file; path = "fontawesome-webfont.ttf"; sourceTree = ""; }; 34330A5B1E787A9800DF2FB9 /* dripicons-v2.ttf */ = {isa = PBXFileReference; lastKnownFileType = file; path = "dripicons-v2.ttf"; sourceTree = ""; }; 34330A5D1E787BD800DF2FB9 /* ElegantIcons.ttf */ = {isa = PBXFileReference; lastKnownFileType = file; path = ElegantIcons.ttf; sourceTree = ""; }; @@ -552,7 +552,7 @@ 34CE88EA1F3237260098030F /* OWSProfileManager.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OWSProfileManager.m; sourceTree = ""; }; 34CE88EB1F3237260098030F /* ProfileFetcherJob.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProfileFetcherJob.swift; sourceTree = ""; }; 34D1F04F1F7D45A60066283D /* GifPickerCell.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GifPickerCell.swift; sourceTree = ""; }; - 34D1F0511F7E8EA30066283D /* GifDownloader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GifDownloader.swift; sourceTree = ""; }; + 34D1F0511F7E8EA30066283D /* GiphyDownloader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GiphyDownloader.swift; sourceTree = ""; }; 34D5CC941EA6AFAD005515DB /* OWSContactsSyncing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSContactsSyncing.h; sourceTree = ""; }; 34D5CC951EA6AFAD005515DB /* OWSContactsSyncing.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OWSContactsSyncing.m; sourceTree = ""; }; 34D5CC981EA6EB79005515DB /* OWSMessageCollectionViewCell.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSMessageCollectionViewCell.h; sourceTree = ""; }; @@ -1427,8 +1427,8 @@ 76EB041D18170B33006006FC /* network */ = { isa = PBXGroup; children = ( - 34D1F0511F7E8EA30066283D /* GifDownloader.swift */, - 3430FE171F7751D4000EC51B /* GifManager.swift */, + 3430FE171F7751D4000EC51B /* GiphyAPI.swift */, + 34D1F0511F7E8EA30066283D /* GiphyDownloader.swift */, B6B9ECFA198B31BA00C620D3 /* PushManager.h */, B6B9ECFB198B31BA00C620D3 /* PushManager.m */, ); @@ -2271,7 +2271,7 @@ 3400C7961EAF99F4008A8584 /* SelectThreadViewController.m in Sources */, 34D5CCB11EAE7E7F005515DB /* SelectRecipientViewController.m in Sources */, 34B3F88F1E8DF1710035BE1A /* RegistrationViewController.m in Sources */, - 3430FE181F7751D4000EC51B /* GifManager.swift in Sources */, + 3430FE181F7751D4000EC51B /* GiphyAPI.swift in Sources */, 3448BFCD1EDF0EA7005B2D69 /* OWSMessagesInputToolbar.m in Sources */, 34B3F8901E8DF1710035BE1A /* AppSettingsViewController.m in Sources */, 34FD93701E3BD43A00109093 /* OWSAnyTouchGestureRecognizer.m in Sources */, @@ -2312,7 +2312,7 @@ 45464DBC1DFA041F001D3FD6 /* DataChannelMessage.swift in Sources */, 34E3E5681EC4B19400495BAC /* AudioProgressView.swift in Sources */, 3448BFCF1EDF0EA7005B2D69 /* OWSMessagesComposerTextView.m in Sources */, - 34D1F0521F7E8EA30066283D /* GifDownloader.swift in Sources */, + 34D1F0521F7E8EA30066283D /* GiphyDownloader.swift in Sources */, 450DF2051E0D74AC003D14BE /* Platform.swift in Sources */, 34BECE301F7ABCF800D7438D /* GifPickerLayout.swift in Sources */, 3472229F1EB22FFE00E53955 /* AddToGroupViewController.m in Sources */, diff --git a/Signal/src/ViewControllers/GifPicker/GifPickerCell.swift b/Signal/src/ViewControllers/GifPicker/GifPickerCell.swift index 6d6973655..637005a30 100644 --- a/Signal/src/ViewControllers/GifPicker/GifPickerCell.swift +++ b/Signal/src/ViewControllers/GifPicker/GifPickerCell.swift @@ -111,8 +111,9 @@ class GifPickerCell: UICollectionViewCell { return } + // Start still asset request if necessary. if stillAsset == nil && fullAsset == nil && stillAssetRequest == nil { - stillAssetRequest = GifDownloader.sharedInstance.requestAsset(rendition:stillRendition, + stillAssetRequest = GiphyDownloader.sharedInstance.requestAsset(rendition:stillRendition, priority:.high, success: { [weak self] assetRequest, asset in guard let strongSelf = self else { return } @@ -133,8 +134,10 @@ class GifPickerCell: UICollectionViewCell { strongSelf.clearStillAssetRequest() }) } + + // Start full asset request if necessary. if fullAsset == nil && fullAssetRequest == nil { - fullAssetRequest = GifDownloader.sharedInstance.requestAsset(rendition:fullRendition, + fullAssetRequest = GiphyDownloader.sharedInstance.requestAsset(rendition:fullRendition, priority:.low, success: { [weak self] assetRequest, asset in guard let strongSelf = self else { return } diff --git a/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift b/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift index 5ad82daf5..b1dd2386d 100644 --- a/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift +++ b/Signal/src/ViewControllers/GifPicker/GifPickerViewController.swift @@ -188,8 +188,12 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect public func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell { let imageInfo = imageInfos[indexPath.row] - let cell = collectionView.dequeueReusableCell(withReuseIdentifier:kCellReuseIdentifier, for: indexPath) as! GifPickerCell - cell.imageInfo = imageInfo + let cell = collectionView.dequeueReusableCell(withReuseIdentifier:kCellReuseIdentifier, for: indexPath) + guard let gifCell = cell as? GifPickerCell else { + owsFail("\(TAG) Unexpected cell type.") + return cell + } + gifCell.imageInfo = imageInfo return cell } @@ -274,7 +278,7 @@ class GifPickerViewController: OWSViewController, UISearchBarDelegate, UICollect updateContents() self.collectionView.contentOffset = CGPoint.zero - GifManager.sharedInstance.search(query: query, success: { [weak self] imageInfos in + GiphyAPI.sharedInstance.search(query: query, success: { [weak self] imageInfos in guard let strongSelf = self else { return } Logger.info("\(strongSelf.TAG) search complete") strongSelf.imageInfos = imageInfos diff --git a/Signal/src/network/GifManager.swift b/Signal/src/network/GiphyAPI.swift similarity index 88% rename from Signal/src/network/GifManager.swift rename to Signal/src/network/GiphyAPI.swift index d1274b066..b3f5ca963 100644 --- a/Signal/src/network/GifManager.swift +++ b/Signal/src/network/GiphyAPI.swift @@ -195,13 +195,13 @@ enum GiphyFormat { } } -@objc class GifManager: NSObject { +@objc class GiphyAPI: NSObject { // MARK: - Properties - static let TAG = "[GifManager]" + static let TAG = "[GiphyAPI]" - static let sharedInstance = GifManager() + static let sharedInstance = GiphyAPI() // Force usage as a singleton override private init() {} @@ -214,7 +214,7 @@ enum GiphyFormat { private func giphyAPISessionManager() -> AFHTTPSessionManager? { guard let baseUrl = NSURL(string:kGiphyBaseURL) else { - Logger.error("\(GifManager.TAG) Invalid base URL.") + Logger.error("\(GiphyAPI.TAG) Invalid base URL.") return nil } // TODO: We need to verify that this session configuration properly @@ -238,12 +238,12 @@ enum GiphyFormat { public func search(query: String, success: @escaping (([GiphyImageInfo]) -> Void), failure: @escaping (() -> Void)) { guard let sessionManager = giphyAPISessionManager() else { - Logger.error("\(GifManager.TAG) Couldn't create session manager.") + Logger.error("\(GiphyAPI.TAG) Couldn't create session manager.") failure() return } guard NSURL(string:kGiphyBaseURL) != nil else { - Logger.error("\(GifManager.TAG) Invalid base URL.") + Logger.error("\(GiphyAPI.TAG) Invalid base URL.") failure() return } @@ -255,7 +255,7 @@ enum GiphyFormat { let kGiphyPageSize = 200 let kGiphyPageOffset = 0 guard let queryEncoded = query.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else { - Logger.error("\(GifManager.TAG) Could not URL encode query: \(query).") + Logger.error("\(GiphyAPI.TAG) Could not URL encode query: \(query).") failure() return } @@ -265,7 +265,7 @@ enum GiphyFormat { parameters: {}, progress:nil, success: { _, value in - Logger.error("\(GifManager.TAG) search request succeeded") + Logger.error("\(GiphyAPI.TAG) search request succeeded") guard let imageInfos = self.parseGiphyImages(responseJson:value) else { failure() return @@ -273,7 +273,7 @@ enum GiphyFormat { success(imageInfos) }, failure: { _, error in - Logger.error("\(GifManager.TAG) search request failed: \(error)") + Logger.error("\(GiphyAPI.TAG) search request failed: \(error)") failure() }) } @@ -282,15 +282,15 @@ enum GiphyFormat { private func parseGiphyImages(responseJson:Any?) -> [GiphyImageInfo]? { guard let responseJson = responseJson else { - Logger.error("\(GifManager.TAG) Missing response.") + Logger.error("\(GiphyAPI.TAG) Missing response.") return nil } guard let responseDict = responseJson as? [String:Any] else { - Logger.error("\(GifManager.TAG) Invalid response.") + Logger.error("\(GiphyAPI.TAG) Invalid response.") return nil } guard let imageDicts = responseDict["data"] as? [[String:Any]] else { - Logger.error("\(GifManager.TAG) Invalid response data.") + Logger.error("\(GiphyAPI.TAG) Invalid response data.") return nil } var result = [GiphyImageInfo]() @@ -306,21 +306,21 @@ enum GiphyFormat { // Giphy API results are often incomplete or malformed, so we need to be defensive. private func parseGiphyImage(imageDict: [String:Any]) -> GiphyImageInfo? { guard let giphyId = imageDict["id"] as? String else { - Logger.warn("\(GifManager.TAG) Image dict missing id.") + Logger.warn("\(GiphyAPI.TAG) Image dict missing id.") return nil } guard giphyId.characters.count > 0 else { - Logger.warn("\(GifManager.TAG) Image dict has invalid id.") + Logger.warn("\(GiphyAPI.TAG) Image dict has invalid id.") return nil } guard let renditionDicts = imageDict["images"] as? [String:Any] else { - Logger.warn("\(GifManager.TAG) Image dict missing renditions.") + Logger.warn("\(GiphyAPI.TAG) Image dict missing renditions.") return nil } var renditions = [GiphyRendition]() for (renditionName, renditionDict) in renditionDicts { guard let renditionDict = renditionDict as? [String:Any] else { - Logger.warn("\(GifManager.TAG) Invalid rendition dict.") + Logger.warn("\(GiphyAPI.TAG) Invalid rendition dict.") continue } guard let rendition = parseGiphyRendition(renditionName:renditionName, @@ -330,12 +330,12 @@ enum GiphyFormat { renditions.append(rendition) } guard renditions.count > 0 else { - Logger.warn("\(GifManager.TAG) Image has no valid renditions.") + Logger.warn("\(GiphyAPI.TAG) Image has no valid renditions.") return nil } guard let originalRendition = findOriginalRendition(renditions:renditions) else { - Logger.warn("\(GifManager.TAG) Image has no original rendition.") + Logger.warn("\(GiphyAPI.TAG) Image has no original rendition.") return nil } @@ -345,10 +345,8 @@ enum GiphyFormat { } private func findOriginalRendition(renditions: [GiphyRendition]) -> GiphyRendition? { - for rendition in renditions { - if rendition.name == "original" { - return rendition - } + for rendition in renditions where rendition.name == "original" { + return rendition } return nil } @@ -370,15 +368,15 @@ enum GiphyFormat { return nil } guard urlString.characters.count > 0 else { - Logger.warn("\(GifManager.TAG) Rendition has invalid url.") + Logger.warn("\(GiphyAPI.TAG) Rendition has invalid url.") return nil } guard let url = NSURL(string:urlString) else { - Logger.warn("\(GifManager.TAG) Rendition url could not be parsed.") + Logger.warn("\(GiphyAPI.TAG) Rendition url could not be parsed.") return nil } guard let fileExtension = url.pathExtension else { - Logger.warn("\(GifManager.TAG) Rendition url missing file extension.") + Logger.warn("\(GiphyAPI.TAG) Rendition url missing file extension.") return nil } var format = GiphyFormat.gif @@ -391,7 +389,7 @@ enum GiphyFormat { } else if fileExtension.lowercased() == "webp" { return nil } else { - Logger.warn("\(GifManager.TAG) Invalid file extension: \(fileExtension).") + Logger.warn("\(GiphyAPI.TAG) Invalid file extension: \(fileExtension).") return nil } @@ -416,7 +414,7 @@ enum GiphyFormat { return nil } guard parsedValue > 0 else { - Logger.verbose("\(GifManager.TAG) \(typeName) has non-positive \(key): \(parsedValue).") + Logger.verbose("\(GiphyAPI.TAG) \(typeName) has non-positive \(key): \(parsedValue).") return nil } return parsedValue diff --git a/Signal/src/network/GifDownloader.swift b/Signal/src/network/GiphyDownloader.swift similarity index 91% rename from Signal/src/network/GifDownloader.swift rename to Signal/src/network/GiphyDownloader.swift index e0a64529d..8a651a1d4 100644 --- a/Signal/src/network/GifDownloader.swift +++ b/Signal/src/network/GiphyDownloader.swift @@ -160,13 +160,13 @@ extension URLSessionTask { } } -@objc class GifDownloader: NSObject, URLSessionTaskDelegate, URLSessionDownloadDelegate { +@objc class GiphyDownloader: NSObject, URLSessionTaskDelegate, URLSessionDownloadDelegate { // MARK: - Properties - static let TAG = "[GifDownloader]" + static let TAG = "[GiphyDownloader]" - static let sharedInstance = GifDownloader() + static let sharedInstance = GiphyDownloader() // A private queue used for download task callbacks. private let operationQueue = OperationQueue() @@ -287,7 +287,7 @@ extension URLSessionTask { } guard let downloadSession = self.giphyDownloadSession() else { - owsFail("\(GifDownloader.TAG) Couldn't create session manager.") + owsFail("\(GiphyDownloader.TAG) Couldn't create session manager.") self.assetRequestDidFail(assetRequest:assetRequest) return } @@ -302,18 +302,21 @@ extension URLSessionTask { private func popNextAssetRequest() -> GiphyAssetRequest? { AssertIsOnMainThread() - // Prefer the first "high" priority request, + var activeAssetRequestURLs = Set() + for assetRequest in activeAssetRequests { + activeAssetRequestURLs.insert(assetRequest.rendition.url) + } + + // Prefer the first "high" priority request; // fall back to the first "low" priority request. - // - // TODO: We could refine this logic to defer requests if - // there is already an active asset request with the - // same URL. for priority in [GiphyRequestPriority.high, GiphyRequestPriority.low] { - for (assetRequestIndex, assetRequest) in assetRequestQueue.enumerated() { - if assetRequest.priority == priority { - assetRequestQueue.remove(at:assetRequestIndex) - return assetRequest + for (assetRequestIndex, assetRequest) in assetRequestQueue.enumerated() where assetRequest.priority == priority { + guard !activeAssetRequestURLs.contains(assetRequest.rendition.url) else { + // Defer requests if there is already an active asset request with the same URL. + continue } + assetRequestQueue.remove(at:assetRequestIndex) + return assetRequest } } @@ -338,23 +341,23 @@ extension URLSessionTask { return } if let error = error { - Logger.error("\(GifDownloader.TAG) download failed with error: \(error)") + Logger.error("\(GiphyDownloader.TAG) download failed with error: \(error)") assetRequestDidFail(assetRequest:assetRequest) return } guard let httpResponse = task.response as? HTTPURLResponse else { - Logger.error("\(GifDownloader.TAG) missing or unexpected response: \(task.response)") + Logger.error("\(GiphyDownloader.TAG) missing or unexpected response: \(task.response)") assetRequestDidFail(assetRequest:assetRequest) return } let statusCode = httpResponse.statusCode guard statusCode >= 200 && statusCode < 400 else { - Logger.error("\(GifDownloader.TAG) response has invalid status code: \(statusCode)") + Logger.error("\(GiphyDownloader.TAG) response has invalid status code: \(statusCode)") assetRequestDidFail(assetRequest:assetRequest) return } guard let assetFilePath = assetRequest.assetFilePath else { - Logger.error("\(GifDownloader.TAG) task is missing asset file") + Logger.error("\(GiphyDownloader.TAG) task is missing asset file") assetRequestDidFail(assetRequest:assetRequest) return }