Refactoring and tidy up.

Make the preview manager a singleton (passing in the MXSession to functions). Fix tests.

PreviewManager → URLPreviewManager
URLPreviewViewData → URLPreviewData
URLPreviewCache → URLPreviewStore

Riot/Managers/URLPreviews/ClosedURLPreview.swift

@@ -14,9 +14,12 @@
 // limitations under the License.
 //
 
-import Foundation
+import CoreData
 
 extension ClosedURLPreview {
-    // Nothing to extend, however having this file stops Xcode
-    // complaining that it can't find ClosedURLPreview.
+    convenience init(context: NSManagedObjectContext, eventID: String, roomID: String) {
+        self.init(context: context)
+        self.eventID = eventID
+        self.roomID = roomID
+    }
 }

Riot/Managers/URLPreviews/PreviewManager.swift

@@ -1,112 +0,0 @@
-//
-// Copyright 2021 New Vector Ltd
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-import Foundation
-
-@objcMembers
-class PreviewManager: NSObject {
-    private let restClient: MXRestClient
-    private let mediaManager: MXMediaManager
-    
-    // Core Data cache to reduce network requests
-    private let cache = URLPreviewCache()
-    
-    init(restClient: MXRestClient, mediaManager: MXMediaManager) {
-        self.restClient = restClient
-        self.mediaManager = mediaManager
-    }
-    
-    func preview(for url: URL, and event: MXEvent, success: @escaping (URLPreviewViewData) -> Void, failure: @escaping (Error?) -> Void) {
-        // Sanitize the URL before checking cache or performing lookup
-        let sanitizedURL = sanitize(url)
-        
-        if let preview = cache.preview(for: sanitizedURL, and: event) {
-            MXLog.debug("[PreviewManager] Using preview from cache")
-            success(preview)
-            return
-        }
-        
-        restClient.preview(for: sanitizedURL, success: { preview in
-            MXLog.debug("[PreviewManager] Preview not found in cache. Requesting from homeserver.")
-            
-            if let preview = preview {
-                self.makePreviewData(for: sanitizedURL, and: event, from: preview) { previewData in
-                    self.cache.store(previewData)
-                    success(previewData)
-                }
-            }
-            
-        }, failure: failure)
-    }
-    
-    func makePreviewData(for url: URL, and event: MXEvent, from preview: MXURLPreview, completion: @escaping (URLPreviewViewData) -> Void) {
-        let previewData = URLPreviewViewData(url: url,
-                                             eventID: event.eventId,
-                                             roomID: event.roomId,
-                                             siteName: preview.siteName,
-                                             title: preview.title,
-                                             text: preview.text)
-        
-        guard let imageURL = preview.imageURL else {
-            completion(previewData)
-            return
-        }
-        
-        if let cachePath = MXMediaManager.cachePath(forMatrixContentURI: imageURL, andType: preview.imageType, inFolder: nil),
-           let image = MXMediaManager.loadThroughCache(withFilePath: cachePath) {
-            previewData.image = image
-            completion(previewData)
-            return
-        }
-        
-        // Don't de-dupe image downloads as the manager should de-dupe preview generation.
-        
-        mediaManager.downloadMedia(fromMatrixContentURI: imageURL, withType: preview.imageType, inFolder: nil) { path in
-            guard let image = MXMediaManager.loadThroughCache(withFilePath: path) else {
-                completion(previewData)
-                return
-            }
-            previewData.image = image
-            completion(previewData)
-        } failure: { error in
-            completion(previewData)
-        }
-    }
-    
-    func removeExpiredItemsFromCache() {
-        cache.removeExpiredItems()
-    }
-    
-    func clearCache() {
-        cache.clear()
-    }
-    
-    func closePreview(for eventID: String, in roomID: String) {
-        cache.closePreview(for: eventID, in: roomID)
-    }
-    
-    func hasClosedPreview(from event: MXEvent) -> Bool {
-        cache.hasClosedPreview(for: event.eventId, in: event.roomId)
-    }
-    
-    private func sanitize(_ url: URL) -> URL {
-        // Remove the fragment from the URL.
-        var components = URLComponents(url: url, resolvingAgainstBaseURL: false)
-        components?.fragment = nil
-        
-        return components?.url ?? url
-    }
-}

Riot/Managers/URLPreviews/URLPreviewCacheData.swift

@@ -17,12 +17,12 @@
 import CoreData
 
 extension URLPreviewCacheData {
-    convenience init(context: NSManagedObjectContext, preview: URLPreviewViewData, creationDate: Date) {
+    convenience init(context: NSManagedObjectContext, preview: URLPreviewData, creationDate: Date) {
         self.init(context: context)
         update(from: preview, on: creationDate)
     }
     
-    func update(from preview: URLPreviewViewData, on date: Date) {
+    func update(from preview: URLPreviewData, on date: Date) {
         url = preview.url
         siteName = preview.siteName
         title = preview.title
@@ -32,15 +32,15 @@ extension URLPreviewCacheData {
         creationDate = date
     }
     
-    func preview(for event: MXEvent) -> URLPreviewViewData? {
+    func preview(for event: MXEvent) -> URLPreviewData? {
         guard let url = url else { return nil }
         
-        let viewData = URLPreviewViewData(url: url,
-                                          eventID: event.eventId,
-                                          roomID: event.roomId,
-                                          siteName: siteName,
-                                          title: title,
-                                          text: text)
+        let viewData = URLPreviewData(url: url,
+                                      eventID: event.eventId,
+                                      roomID: event.roomId,
+                                      siteName: siteName,
+                                      title: title,
+                                      text: text)
         viewData.image = image as? UIImage
         
         return viewData

Riot/Managers/URLPreviews/URLPreviewData.swift

@@ -0,0 +1,52 @@
+// 
+// Copyright 2021 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import Foundation
+
+@objcMembers
+class URLPreviewData: NSObject {
+    /// The URL that's represented by the preview data. This may have been sanitized.
+    /// Note: The original URL, can be found in the bubble components with `eventID` and `roomID`.
+    let url: URL
+    
+    /// The ID of the event that created this preview.
+    let eventID: String
+    
+    /// The ID of the room that this preview is from.
+    let roomID: String
+    
+    /// The OpenGraph site name for the URL.
+    let siteName: String?
+    
+    /// The OpenGraph title for the URL.
+    let title: String?
+    
+    /// The OpenGraph description for the URL.
+    let text: String?
+    
+    /// The OpenGraph image for the URL.
+    var image: UIImage?
+    
+    init(url: URL, eventID: String, roomID: String, siteName: String?, title: String?, text: String?) {
+        self.url = url
+        self.eventID = eventID
+        self.roomID = roomID
+        self.siteName = siteName
+        self.title = title
+        // Remove line breaks from the description text
+        self.text = text?.replacingOccurrences(of: "\n", with: " ")
+    }
+}

Riot/Managers/URLPreviews/URLPreviewManager.swift

@@ -0,0 +1,116 @@
+//
+// Copyright 2021 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import Foundation
+
+@objcMembers
+class URLPreviewManager: NSObject {
+    static let shared = URLPreviewManager()
+    
+    // Core Data store to reduce network requests
+    private let store = URLPreviewStore()
+    
+    private override init() { }
+    
+    func preview(for url: URL,
+                 and event: MXEvent,
+                 with session: MXSession,
+                 success: @escaping (URLPreviewData) -> Void,
+                 failure: @escaping (Error?) -> Void) {
+        // Sanitize the URL before checking the store or performing lookup
+        let sanitizedURL = sanitize(url)
+        
+        if let preview = store.preview(for: sanitizedURL, and: event) {
+            MXLog.debug("[URLPreviewManager] Using cached preview.")
+            success(preview)
+            return
+        }
+        
+        session.matrixRestClient.preview(for: sanitizedURL, success: { previewResponse in
+            MXLog.debug("[URLPreviewManager] Cached preview not found. Requesting from homeserver.")
+            
+            if let previewResponse = previewResponse {
+                self.makePreviewData(from: previewResponse, for: sanitizedURL, and: event, with: session) { previewData in
+                    self.store.store(previewData)
+                    success(previewData)
+                }
+            }
+            
+        }, failure: failure)
+    }
+    
+    func makePreviewData(from previewResponse: MXURLPreview,
+                         for url: URL,
+                         and event: MXEvent,
+                         with session: MXSession,
+                         completion: @escaping (URLPreviewData) -> Void) {
+        let previewData = URLPreviewData(url: url,
+                                         eventID: event.eventId,
+                                         roomID: event.roomId,
+                                         siteName: previewResponse.siteName,
+                                         title: previewResponse.title,
+                                         text: previewResponse.text)
+        
+        guard let imageURL = previewResponse.imageURL else {
+            completion(previewData)
+            return
+        }
+        
+        if let cachePath = MXMediaManager.cachePath(forMatrixContentURI: imageURL, andType: previewResponse.imageType, inFolder: nil),
+           let image = MXMediaManager.loadThroughCache(withFilePath: cachePath) {
+            previewData.image = image
+            completion(previewData)
+            return
+        }
+        
+        // Don't de-dupe image downloads as the manager should de-dupe preview generation.
+        
+        session.mediaManager.downloadMedia(fromMatrixContentURI: imageURL, withType: previewResponse.imageType, inFolder: nil) { path in
+            guard let image = MXMediaManager.loadThroughCache(withFilePath: path) else {
+                completion(previewData)
+                return
+            }
+            previewData.image = image
+            completion(previewData)
+        } failure: { error in
+            completion(previewData)
+        }
+    }
+    
+    func removeExpiredCacheData() {
+        store.removeExpiredItems()
+    }
+    
+    func clearStore() {
+        store.deleteAll()
+    }
+    
+    func closePreview(for eventID: String, in roomID: String) {
+        store.closePreview(for: eventID, in: roomID)
+    }
+    
+    func hasClosedPreview(from event: MXEvent) -> Bool {
+        store.hasClosedPreview(for: event.eventId, in: event.roomId)
+    }
+    
+    private func sanitize(_ url: URL) -> URL {
+        // Remove the fragment from the URL.
+        var components = URLComponents(url: url, resolvingAgainstBaseURL: false)
+        components?.fragment = nil
+        
+        return components?.url ?? url
+    }
+}

Riot/Managers/URLPreviews/URLPreviewStore.swift

@@ -17,7 +17,7 @@
 import CoreData
 
 /// A cache for URL previews backed by Core Data.
-class URLPreviewCache {
+class URLPreviewStore {
     
     // MARK: - Properties
     
@@ -46,7 +46,7 @@ class URLPreviewCache {
         ValueTransformer.setValueTransformer(URLPreviewImageTransformer(), forName: .urlPreviewImageTransformer)
         
         // Create the container, updating it's path if storing the data in memory.
-        container = NSPersistentContainer(name: "URLPreviewCache")
+        container = NSPersistentContainer(name: "URLPreviewStore")
         
         if inMemory {
             container.persistentStoreDescriptions.first?.url = URL(fileURLWithPath: "/dev/null")
@@ -55,7 +55,7 @@ class URLPreviewCache {
         // Load the persistent stores into the container
         container.loadPersistentStores { storeDescription, error in
             if let error = error {
-                MXLog.error("[URLPreviewCache] Core Data container error: \(error.localizedDescription)")
+                MXLog.error("[URLPreviewStore] Core Data container error: \(error.localizedDescription)")
             }
         }
     }
@@ -65,7 +65,7 @@ class URLPreviewCache {
     /// Store a preview in the cache. If a preview already exists with the same URL it will be updated from the new preview.
     /// - Parameter preview: The preview to add to the cache.
     /// - Parameter date: Optional: The date the preview was generated.
-    func store(_ preview: URLPreviewViewData, generatedOn generationDate: Date? = nil) {
+    func store(_ preview: URLPreviewData, generatedOn generationDate: Date? = nil) {
         // Create a fetch request for an existing preview.
         let request: NSFetchRequest<URLPreviewCacheData> = URLPreviewCacheData.fetchRequest()
         request.predicate = NSPredicate(format: "url == %@", preview.url as NSURL)
@@ -87,7 +87,7 @@ class URLPreviewCache {
     /// if the preview is older than the ``dataValidityTime`` the returned value will be nil.
     /// - Parameter url: The URL to fetch the preview for.
     /// - Returns: The preview if found, otherwise nil.
-    func preview(for url: URL, and event: MXEvent) -> URLPreviewViewData? {
+    func preview(for url: URL, and event: MXEvent) -> URLPreviewData? {
         // Create a request for the url excluding any expired items
         let request: NSFetchRequest<URLPreviewCacheData> = URLPreviewCacheData.fetchRequest()
         request.predicate = NSCompoundPredicate(type: .and, subpredicates: [
@@ -104,11 +104,13 @@ class URLPreviewCache {
         return cachedPreview.preview(for: event)
     }
     
-    func count() -> Int {
+    /// Returns the number of URL previews cached in the store.
+    func cacheCount() -> Int {
         let request: NSFetchRequest<NSFetchRequestResult> = URLPreviewCacheData.fetchRequest()
         return (try? context.count(for: request)) ?? 0
     }
     
+    /// Removes any expired cache data from the store.
     func removeExpiredItems() {
         let request: NSFetchRequest<NSFetchRequestResult> = URLPreviewCacheData.fetchRequest()
         request.predicate = NSPredicate(format: "creationDate < %@", expiryDate as NSDate)
@@ -116,23 +118,22 @@ class URLPreviewCache {
         do {
             try context.execute(NSBatchDeleteRequest(fetchRequest: request))
         } catch {
-            MXLog.error("[URLPreviewCache] Error executing batch delete request: \(error.localizedDescription)")
+            MXLog.error("[URLPreviewStore] Error executing batch delete request: \(error.localizedDescription)")
         }
     }
     
-    func clear() {
+    /// Deletes all cache data and all closed previews from the store.
+    func deleteAll() {
         do {
             _ = try context.execute(NSBatchDeleteRequest(fetchRequest: URLPreviewCacheData.fetchRequest()))
             _ = try context.execute(NSBatchDeleteRequest(fetchRequest: ClosedURLPreview.fetchRequest()))
         } catch {
-            MXLog.error("[URLPreviewCache] Error executing batch delete request: \(error.localizedDescription)")
+            MXLog.error("[URLPreviewStore] Error executing batch delete request: \(error.localizedDescription)")
         }
     }
     
     func closePreview(for eventID: String, in roomID: String) {
-        let closedPreview = ClosedURLPreview(context: context)
-        closedPreview.eventID = eventID
-        closedPreview.roomID = roomID
+        _ = ClosedURLPreview(context: context, eventID: eventID, roomID: roomID)
         save()
     }