Reworked to just use a unique id rather than hacky hash overrides

SessionMessagingKit/Sending & Receiving/Attachments/SignalAttachment.swift

@@ -104,7 +104,7 @@ public enum TSImageQuality: UInt {
 // [SignalAttachment hasError] will be true for non-valid attachments.
 //
 // TODO: Perhaps do conversion off the main thread?
-public class SignalAttachment: Equatable, Hashable {
+public class SignalAttachment: Equatable {
 
     // MARK: Properties
 
@@ -1131,20 +1131,4 @@ public class SignalAttachment: Equatable, Hashable {
             lhs.isVoiceMessage == rhs.isVoiceMessage
         )
     }
-    
-    // MARK: - Hashable
-    
-    public func hash(into hasher: inout Hasher) {
-        dataUTI.hash(into: &hasher)
-        captionText.hash(into: &hasher)
-        linkPreviewDraft.hash(into: &hasher)
-        isConvertibleToTextMessage.hash(into: &hasher)
-        isConvertibleToContactShare.hash(into: &hasher)
-        isVoiceMessage.hash(into: &hasher)
-        dataSource.hash(into: &hasher)
-        cachedImage?.size.width.hash(into: &hasher)
-        cachedImage?.size.height.hash(into: &hasher)
-        cachedVideoPreview?.size.width.hash(into: &hasher)
-        cachedVideoPreview?.size.height.hash(into: &hasher)
-    }
 }

SessionUtilitiesKit/Media/DataSource.swift

@@ -4,7 +4,7 @@ import Foundation
 
 // MARK: - DataSource
 
-public protocol DataSource: Equatable, Hashable {
+public protocol DataSource: Equatable {
     var data: Data { get }
     var dataUrl: URL? { get }
 
@@ -140,13 +140,6 @@ public class DataSourceValue: DataSource {
         try data.write(to: URL(fileURLWithPath: path), options: .atomic)
     }
     
-    public func hash(into hasher: inout Hasher) {
-        data.hash(into: &hasher)
-        sourceFilename.hash(into: &hasher)
-        fileExtension.hash(into: &hasher)
-        shouldDeleteOnDeinit.hash(into: &hasher)
-    }
-    
     public static func == (lhs: DataSourceValue, rhs: DataSourceValue) -> Bool {
         return (
             lhs.data == rhs.data &&
@@ -234,12 +227,6 @@ public class DataSourcePath: DataSource {
         try FileManager.default.copyItem(atPath: filePath, toPath: path)
     }
     
-    public func hash(into hasher: inout Hasher) {
-        filePath.hash(into: &hasher)
-        sourceFilename.hash(into: &hasher)
-        shouldDeleteOnDeinit.hash(into: &hasher)
-    }
-    
     public static func == (lhs: DataSourcePath, rhs: DataSourcePath) -> Bool {
         return (
             lhs.filePath == rhs.filePath &&

SignalUtilitiesKit/Media Viewing & Editing/Attachment Approval/AttachmentApprovalViewController.swift

@@ -95,7 +95,7 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC
         set { setCurrentItem(newValue, direction: .forward, animated: false) }
     }
     
-    private var cachedPages: [SignalAttachmentItem: AttachmentPrepViewController] = [:]
+    private var cachedPages: [UUID: AttachmentPrepViewController] = [:]
 
     public var shouldHideControls: Bool {
         guard let pageViewController: AttachmentPrepViewController = pageViewControllers?.first else {
@@ -477,7 +477,7 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC
     }
 
     private func buildPage(item: SignalAttachmentItem) -> AttachmentPrepViewController? {
-        if let cachedPage = cachedPages[item] {
+        if let cachedPage = cachedPages[item.uniqueIdentifier] {
             Log.debug("[AttachmentApprovalViewController] cache hit.")
             return cachedPage
         }
@@ -485,7 +485,7 @@ public class AttachmentApprovalViewController: UIPageViewController, UIPageViewC
         Log.debug("[AttachmentApprovalViewController] cache miss.")
         let viewController = AttachmentPrepViewController(attachmentItem: item, using: dependencies)
         viewController.prepDelegate = self
-        cachedPages[item] = viewController
+        cachedPages[item.uniqueIdentifier] = viewController
 
         return viewController
     }

SignalUtilitiesKit/Media Viewing & Editing/Attachment Approval/AttachmentItemCollection.swift

@@ -24,13 +24,13 @@ class AddMoreRailItem: GalleryRailItem {
     }
 }
 
-class SignalAttachmentItem: Hashable {
+class SignalAttachmentItem: Equatable {
 
     enum SignalAttachmentItemError: Error {
         case noThumbnail
     }
 
-    let uniqueIdentifier: UUID
+    let uniqueIdentifier: UUID = UUID()
     let attachment: SignalAttachment
 
     // This might be nil if the attachment is not a valid image.
@@ -64,25 +64,6 @@ class SignalAttachmentItem: Hashable {
         return attachment.staticThumbnail()
     }
 
-    // MARK: Hashable
-    
-    func hash(into hasher: inout Hasher) {
-        /// There was a crash in `AttachmentApprovalViewController` when trying to generate the hash
-        /// value to store in a dictionary, this crash persisted even after refactoring `DataSource` into Swift and
-        /// using custom `hash(into:)` functions on everything in order to exclude values which might have
-        /// been unsafe.
-        ///
-        /// Since the crash is still occurring the most likely culprit is now that one of the values used to generate the
-        /// hash was mutated after the value was stored (as `SignalAttachment` is a class and it was previously
-        /// used for generating the hash) - in order to avoid this we now generate a `uniqueIdentifier` when
-        /// initialising this type and use _only_ that for the hash (this `SignalAttachmentItem` is only used for
-        /// the `AttachmentApprovalViewController` and based on it's usage we shouldn't run into issues
-        /// with this hash not being deterministic
-        ///
-        /// If the crash still occurs it's likely a red herring and there is some other, larger, issue that is causing it
-        uniqueIdentifier.hash(into: &hasher)
-    }
-
     // MARK: Equatable
 
     static func == (lhs: SignalAttachmentItem, rhs: SignalAttachmentItem) -> Bool {