Add some extra logic for using hardcoded favionURLs as cache keys. Ad…

…d some additional unit tests.

BrowserKit/Sources/SiteImageView/Entities/SiteImageModel.swift

@@ -32,7 +32,14 @@ public struct SiteImageModel {
         self.id = id
         self.imageType = imageType
         self.siteURL = siteURL
-        self.cacheKey = SiteImageModel.generateCacheKey(siteURL: siteURL, type: imageType)
+        if case .favicon = imageType, let faviconURL = resourceURL {
+            // If we already have a favicon url, use the url as the cache key.
+            // This is a special case where we want to use the exact URL that's provided (e.g. sponsored site, default
+            // top sites, etc.).
+            self.cacheKey = faviconURL.absoluteString
+        } else {
+            self.cacheKey = SiteImageModel.generateCacheKey(siteURL: siteURL, type: imageType)
+        }
         self.resourceURL = resourceURL
         self.image = image
     }
@@ -48,9 +55,6 @@ public struct SiteImageModel {
             // Always use the full site URL as the cache key for hero images
             return siteURL.absoluteString
         case .favicon:
-            // FIXME Why would we ever want to use a hard-coded faviconURL (sponsored tile, suggested tile, etc...)
-            // as the cache key for either a URL or Image?
-
             // Use the domain as the key to avoid caching and fetching unnecessary duplicates
             return siteURL.shortDomain ?? siteURL.shortDisplayString
         }

BrowserKit/Tests/SiteImageViewTests/FaviconURLProcessing/FaviconURLHandlerTests.swift

@@ -25,7 +25,7 @@ class FaviconURLHandlerTests: XCTestCase {
                                                urlCache: mockCache)
         do {
             let url = try await subject.getFaviconURL(model: model)
-            
+
             XCTAssertEqual(url, faviconURL)
             let getURLCount = await mockCache.getURLFromCacheCalledCount
             let cacheURLCount = await mockCache.cacheURLCalledCount

BrowserKit/Tests/SiteImageViewTests/SiteImageModelTests.swift

@@ -0,0 +1,49 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/
+
+import XCTest
+@testable import SiteImageView
+
+class SiteImageModelTests: XCTestCase {
+    let siteURL = URL(string: "https://www.mozilla.org")!
+    let faviconURL = URL(string: "https://www.mozilla.org/media/img/favicons/mozilla/apple-touch-icon.8cbe9c835c00.png")!
+
+    func testFaviconURL_isCacheKey_whenProvidedForFavicon() async {
+        let model = SiteImageModel(id: UUID(), imageType: .favicon, siteURL: siteURL, resourceURL: faviconURL)
+
+        XCTAssertEqual(model.cacheKey, faviconURL.absoluteString)
+    }
+
+    func testShortDomain_isCacheKey_forFavicon() async {
+        let model = SiteImageModel(id: UUID(), imageType: .favicon, siteURL: siteURL)
+
+        XCTAssertEqual(model.cacheKey, siteURL.shortDomain)
+    }
+
+    func testAbsolutePath_isCacheKey_forHeroImage() async {
+        let model = SiteImageModel(id: UUID(), imageType: .heroImage, siteURL: siteURL)
+
+        XCTAssertEqual(model.cacheKey, siteURL.absoluteString)
+    }
+
+    func testShortDomain_isCacheKey_whenResourceURLProvidedForHeroImage() async {
+        let model = SiteImageModel(id: UUID(), imageType: .heroImage, siteURL: siteURL, resourceURL: faviconURL)
+
+        XCTAssertEqual(model.cacheKey, siteURL.absoluteString)
+    }
+
+    // MARK: - Test generateCacheKey
+
+    func testGenerateCacheKey_returnsShortDomain_forFavicon() async {
+        let cacheKey = SiteImageModel.generateCacheKey(siteURL: siteURL, type: .favicon)
+
+        XCTAssertEqual(cacheKey, siteURL.shortDomain)
+    }
+
+    func testGenerateCacheKey_returnsAbsolutePath_forHeroImage() async {
+        let cacheKey = SiteImageModel.generateCacheKey(siteURL: siteURL, type: .heroImage)
+
+        XCTAssertEqual(cacheKey, siteURL.absoluteString)
+    }
+}