Filtering URLs for Top Hits section of suggestions (#16)

* Generation of a root URL
* Download URLs and URLs that failed to load are not pushed to Top Hits section
* Fetching of remote suggestions dismissed for non root URLs
* Number of navigational suggestions adjusted
* Matches with query found for full URLs not just hosts
* Autocompletion allowed just for suggestions from history or bookmarks
* Update autofill to 3.2.0

Sources/BrowserServicesKit/Common/Extensions/URLExtension.swift

@@ -32,10 +32,26 @@ extension URL {
         return components.url
     }
 
-    var isRoot: Bool {
+    var nakedString: String? {
+        naked?.absoluteString.dropping(prefix: "//")
+    }
+
+    public var root: URL? {
+        guard var components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { return self }
+        components.path = "/"
+        components.query = nil
+        components.fragment = nil
+        components.user = nil
+        components.password = nil
+        return components.url
+    }
+
+    public var isRoot: Bool {
         return (path.isEmpty || path == "/") &&
             query == nil &&
-            fragment == nil
+            fragment == nil &&
+            user == nil &&
+            password == nil
     }
 
 }

Sources/BrowserServicesKit/Suggestions/HistoryEntry.swift

@@ -25,5 +25,7 @@ public protocol HistoryEntry {
     var title: String? { get }
     var numberOfVisits: Int { get }
     var lastVisit: Date { get }
+    var failedToLoad: Bool { get }
+    var isDownload: Bool { get }
 
 }

Sources/BrowserServicesKit/Suggestions/Score.swift

@@ -38,13 +38,14 @@ extension Score {
         }
 
         let domain = url.host?.droppingWwwPrefix() ?? ""
+        let nakedUrl = url.nakedString ?? ""
 
         // Tokenized matches
         if queryTokens.count > 1 {
             var matchesAllTokens = true
             for token in queryTokens {
                 // Match only from the begining of the word to avoid unintuitive matches.
-                if !lowercasedTitle.starts(with: token) && !lowercasedTitle.contains(" \(token)") && !domain.starts(with: token) {
+                if !lowercasedTitle.starts(with: token) && !lowercasedTitle.contains(" \(token)") && !nakedUrl.starts(with: token) {
                     matchesAllTokens = false
                     break
                 }
@@ -55,18 +56,18 @@ extension Score {
                 score += 10
 
                 // Boost score if first token matches:
-                if let firstToken = queryTokens.first { // domain - high score boost
-                    if domain.starts(with: firstToken) {
+                if let firstToken = queryTokens.first { // nakedUrlString - high score boost
+                    if nakedUrl.starts(with: firstToken) {
                         score += 300
                     } else if lowercasedTitle.starts(with: firstToken) { // begining of the title - moderate score boost
                         score += 50
                     }
                 }
             }
         } else {
-            // High score for matching domain in the URL
+            // High score for matching URL
             if let firstToken = queryTokens.first {
-                if domain.starts(with: firstToken) {
+                if nakedUrl.starts(with: firstToken) {
                     score += 300
 
                     // Prioritize root URLs most

Sources/BrowserServicesKit/Suggestions/Suggestion.swift

@@ -23,14 +23,14 @@ public enum Suggestion: Equatable {
     case phrase(phrase: String)
     case website(url: URL)
     case bookmark(title: String, url: URL, isFavorite: Bool)
-    case historyEntry(title: String?, url: URL)
+    case historyEntry(title: String?, url: URL, allowedInTopHits: Bool)
     case unknown(value: String)
 
     var url: URL? {
         get {
             switch self {
             case .website(url: let url),
-                 .historyEntry(title: _, url: let url),
+                 .historyEntry(title: _, url: let url, allowedInTopHits: _),
                  .bookmark(title: _, url: let url, isFavorite: _):
                 return url
             case .phrase, .unknown:
@@ -42,7 +42,7 @@ public enum Suggestion: Equatable {
     var title: String? {
         get {
             switch self {
-            case .historyEntry(title: let title, url: _):
+            case .historyEntry(title: let title, url: _, allowedInTopHits: _):
                 return title
             case .bookmark(title: let title, url: _, isFavorite: _):
                 return title
@@ -52,6 +52,15 @@ public enum Suggestion: Equatable {
         }
     }
 
+    public var allowedForAutocompletion: Bool {
+        switch self {
+        case .historyEntry, .bookmark:
+            return true
+        case .phrase, .website,.unknown:
+            return false
+        }
+    }
+
 }
 
 extension Suggestion {
@@ -61,7 +70,8 @@ extension Suggestion {
     }
 
     init(historyEntry: HistoryEntry) {
-        self = .historyEntry(title: historyEntry.title, url: historyEntry.url)
+        let allowedInTopHits = !(historyEntry.failedToLoad || historyEntry.isDownload)
+        self = .historyEntry(title: historyEntry.title, url: historyEntry.url, allowedInTopHits: allowedInTopHits)
     }
 
     init(url: URL) {

Sources/BrowserServicesKit/Suggestions/SuggestionLoading.swift

@@ -40,9 +40,11 @@ public class SuggestionLoader: SuggestionLoading {
 
     public weak var dataSource: SuggestionLoadingDataSource?
     private let processing: SuggestionProcessing
+    private let urlFactory: (String) -> URL?
 
     public init(dataSource: SuggestionLoadingDataSource? = nil, urlFactory: @escaping (String) -> URL?) {
         self.dataSource = dataSource
+        self.urlFactory = urlFactory
         self.processing = SuggestionProcessing(urlFactory: urlFactory)
     }
 
@@ -64,21 +66,27 @@ public class SuggestionLoader: SuggestionLoading {
         var apiResult: APIResult?
         var apiError: Error?
 
+        let url = urlFactory(query)
+
         let group = DispatchGroup()
-        group.enter()
-        dataSource.suggestionLoading(self,
-                                     suggestionDataFromUrl: Self.remoteSuggestionsUrl,
-                                     withParameters: [Self.searchParameter: query]) { data, error in
-            defer { group.leave() }
-            guard let data = data else {
-                apiError = error
-                return
-            }
-            guard let result = try? JSONDecoder().decode(APIResult.self, from: data) else {
-                apiError = SuggestionLoaderError.parsingFailed
-                return
+        if url == nil || url!.isRoot && url!.path.last != "/" {
+            group.enter()
+            dataSource.suggestionLoading(self,
+                                         suggestionDataFromUrl: Self.remoteSuggestionsUrl,
+                                         withParameters: [Self.searchParameter: query]) { data, error in
+                defer { group.leave() }
+                guard let data = data else {
+                    apiError = error
+                    return
+                }
+                guard let result = try? JSONDecoder().decode(APIResult.self, from: data) else {
+                    apiError = SuggestionLoaderError.parsingFailed
+                    return
+                }
+                apiResult = result
             }
-            apiResult = result
+        } else {
+            apiResult = nil
         }
 
         // 2) Processing it

Sources/BrowserServicesKit/Suggestions/SuggestionProcessing.swift

@@ -56,13 +56,13 @@ final class SuggestionProcessing {
 
         let maximumOfNavigationalSuggestions = min(
             Self.maximumNumberOfSuggestions - Self.minimumNumberInSuggestionGroup,
-            query.count * 2)
+            query.count + 1)
         navigationalSuggestions = removeDuplicates(from: navigationalSuggestions,
                                                    maximum: maximumOfNavigationalSuggestions)
 
         // Split the Top Hits and the History and Bookmarks section
-        let topHits = Array(navigationalSuggestions.prefix(2))
-        let historyAndBookmarkSuggestions = Array(navigationalSuggestions.dropFirst(2).filter { suggestion in
+        let topHits = topHits(from: navigationalSuggestions)
+        let historyAndBookmarkSuggestions = Array(navigationalSuggestions.dropFirst(topHits.count).filter { suggestion in
             switch suggestion {
             case .bookmark, .historyEntry:
                 return true
@@ -171,6 +171,32 @@ final class SuggestionProcessing {
         return newSuggestions
     }
 
+    // MARK: - Top Hits
+
+    private func topHits(from suggestions: [Suggestion]) -> [Suggestion] {
+        var topHits = [Suggestion]()
+
+        for (i, suggestion) in suggestions.enumerated() {
+            guard i <= Self.maximumNumberOfTopHits else { break }
+
+            if case let .historyEntry(title: _, url: _, allowedInTopHits: allowedInTopHits) = suggestion {
+                if allowedInTopHits {
+                    topHits.append(suggestion)
+                } else {
+                    break
+                }
+            } else if case .website = suggestion {
+                topHits.append(suggestion)
+            } else if case .bookmark = suggestion {
+                topHits.append(suggestion)
+            } else {
+                break
+            }
+        }
+
+        return topHits
+    }
+
     // MARK: - Cutting off and making the result
 
     static let maximumNumberOfSuggestions = 12

Tests/BrowserServicesKitTests/Common/Extensions/URLExtensionTests.swift

@@ -28,6 +28,14 @@ final class URLExtensionTests: XCTestCase {
         XCTAssertEqual(url.naked, duplicate.naked)
     }
 
+    func testWhenRootIsCalled_ThenURLWithNoPathQueryFragmentUserAndPasswordIsReturned() {
+        let url = URL(string: "https://dax:[email protected]/test.php?test=S&info=test#fragment")!
+
+        let rootUrl = url.root!
+        XCTAssertEqual(rootUrl, URL(string: "https://www.duckduckgo.com/")!)
+        XCTAssert(rootUrl.isRoot)
+    }
+
     func testIsRoot() {
         let url = URL(string: "https://www.server.com:8080/path?query=string#fragment")!
         let rootUrl = URL(string: "https://www.server.com:8080/")!

Tests/BrowserServicesKitTests/Suggestions/HistoryEntryMock.swift

@@ -27,5 +27,7 @@ struct HistoryEntryMock: HistoryEntry {
     var title: String?
     var numberOfVisits: Int
     var lastVisit: Date
+    var failedToLoad: Bool
+    var isDownload: Bool
 
 }

Tests/BrowserServicesKitTests/Suggestions/SuggestionProcessingTests.swift

@@ -43,7 +43,9 @@ extension HistoryEntryMock {
                            url: URL(string: "http://www.duckduckgo.com")!,
                            title: nil,
                            numberOfVisits: 1000,
-                           lastVisit: Date())
+                           lastVisit: Date(),
+                           failedToLoad: false,
+                           isDownload: false)
         ]
     }
 

Tests/BrowserServicesKitTests/Suggestions/SuggestionTests.swift

@@ -53,7 +53,7 @@ final class SuggestionTests: XCTestCase {
         let phraseSuggestion = Suggestion.phrase(phrase: "phrase")
         let websiteSuggestion = Suggestion.website(url: url)
         let bookmarkSuggestion = Suggestion.bookmark(title: "Title", url: url, isFavorite: true)
-        let historyEntrySuggestion = Suggestion.historyEntry(title: "Title", url: url)
+        let historyEntrySuggestion = Suggestion.historyEntry(title: "Title", url: url, allowedInTopHits: true)
         _ = Suggestion.unknown(value: "phrase")
 
         XCTAssertNil(phraseSuggestion.url)
@@ -70,7 +70,7 @@ final class SuggestionTests: XCTestCase {
         let phraseSuggestion = Suggestion.phrase(phrase: "phrase")
         let websiteSuggestion = Suggestion.website(url: url)
         let bookmarkSuggestion = Suggestion.bookmark(title: title, url: url, isFavorite: true)
-        let historyEntrySuggestion = Suggestion.historyEntry(title: title, url: url)
+        let historyEntrySuggestion = Suggestion.historyEntry(title: title, url: url, allowedInTopHits: true)
         _ = Suggestion.unknown(value: "phrase")
 
         XCTAssertNil(phraseSuggestion.title)
@@ -85,7 +85,7 @@ final class SuggestionTests: XCTestCase {
         let title = "Title"
 
 
-        let historyEntry = HistoryEntryMock(identifier: UUID(), url: url, title: title, numberOfVisits: 1, lastVisit: Date())
+        let historyEntry = HistoryEntryMock(identifier: UUID(), url: url, title: title, numberOfVisits: 1, lastVisit: Date(), failedToLoad: false, isDownload: false)
         let suggestion = Suggestion(historyEntry: historyEntry)
 
         guard case .historyEntry = suggestion else {