From 5eb26f787177abcdb191b87d649096784f57ee30 Mon Sep 17 00:00:00 2001 From: Mohamed Afifi Date: Sat, 16 Dec 2023 16:06:54 -0500 Subject: [PATCH] Ignore autocomplete errors --- .../Search/Searchers/CompositeSearcher.swift | 10 +-- .../Tests/CompositeSearcherTests.swift | 46 +++++----- Features/SearchFeature/SearchTypes.swift | 35 ++++++++ Features/SearchFeature/SearchView.swift | 22 ++--- .../SearchFeature/SearchViewController.swift | 6 +- Features/SearchFeature/SearchViewModel.swift | 89 +++++++------------ 6 files changed, 110 insertions(+), 98 deletions(-) create mode 100644 Features/SearchFeature/SearchTypes.swift diff --git a/Domain/QuranTextKit/Sources/Search/Searchers/CompositeSearcher.swift b/Domain/QuranTextKit/Sources/Search/Searchers/CompositeSearcher.swift index dc892b57..e7363bc8 100644 --- a/Domain/QuranTextKit/Sources/Search/Searchers/CompositeSearcher.swift +++ b/Domain/QuranTextKit/Sources/Search/Searchers/CompositeSearcher.swift @@ -47,19 +47,19 @@ public struct CompositeSearcher { // MARK: Public - public func autocomplete(term: String, quran: Quran) async throws -> [String] { + public func autocomplete(term: String, quran: Quran) async -> [String] { guard let term = SearchTerm(term) else { return [] } logger.info("Autocompleting term: \(term.compactQuery)") - let autocompletions = try await simpleSearchers.asyncMap { searcher in - try await searcher.autocomplete(term: term, quran: quran) + let autocompletions = await simpleSearchers.asyncMap { searcher in + try? await searcher.autocomplete(term: term, quran: quran) } - var results = autocompletions.flatMap { $0 } + var results = autocompletions.compactMap { $0 }.flatMap { $0 } if shouldPerformTranslationSearch(simpleSearchResults: results, term: term.compactQuery) { - results += try await translationsSearcher.autocomplete(term: term, quran: quran) + results += (try? await translationsSearcher.autocomplete(term: term, quran: quran)) ?? [] } if !results.contains(term.compactQuery) { results.insert(term.compactQuery, at: 0) diff --git a/Domain/QuranTextKit/Tests/CompositeSearcherTests.swift b/Domain/QuranTextKit/Tests/CompositeSearcherTests.swift index 73f2b86e..ba10f1ab 100644 --- a/Domain/QuranTextKit/Tests/CompositeSearcherTests.swift +++ b/Domain/QuranTextKit/Tests/CompositeSearcherTests.swift @@ -42,11 +42,11 @@ class CompositeSearcherTests: XCTestCase { } func testNumbers() async throws { - try await autocompleteNumber("4") - try await autocompleteNumber("44") - try await autocompleteNumber("444") - try await autocompleteNumber("4444") - try await autocompleteNumber("3:4") + await autocompleteNumber("4") + await autocompleteNumber("44") + await autocompleteNumber("444") + await autocompleteNumber("4444") + await autocompleteNumber("3:4") try await testSearch(term: "1") try await testSearch(term: "33") @@ -59,19 +59,19 @@ class CompositeSearcherTests: XCTestCase { } func testMatchOneSura() async throws { - try await testAutocomplete(term: "Al-Ahzab") + await testAutocomplete(term: "Al-Ahzab") try await testSearch(term: "Al-Ahzab") } func testMatchMultipleSuras() async throws { - try await testAutocomplete(term: "Yu") + await testAutocomplete(term: "Yu") try await testSearch(term: "Yu") } func testMatchSuraAndQuran() async throws { - try await testAutocomplete(term: "الفات") // Al-fatiha - try await testAutocomplete(term: "الاحۡ") // Al-Ahzab - try await testAutocomplete(term: "الأَعۡلَ") // Al-A'la + await testAutocomplete(term: "الفات") // Al-fatiha + await testAutocomplete(term: "الاحۡ") // Al-Ahzab + await testAutocomplete(term: "الأَعۡلَ") // Al-A'la try await testSearch(term: "الفات") // Al-fatiha try await testSearch(term: "الاحۡ") // Al-Ahzab @@ -79,32 +79,32 @@ class CompositeSearcherTests: XCTestCase { } func testMatchArabicSuraName() async throws { - try await testAutocomplete(term: "النحل") + await testAutocomplete(term: "النحل") try await testSearch(term: "النحل") } func testMatchSuraAndQuranWithIncorrectTashkeel() async throws { - try await testAutocomplete(term: "فُتح") + await testAutocomplete(term: "فُتح") try await testSearch(term: "فُتح") } func testMatchArabicQuran() async throws { let term = "لكنا" - try await testAutocomplete(term: term) + await testAutocomplete(term: term) try await testSearch(term: term) } func testMatchTranslation() async throws { - try await testAutocomplete(term: "All") + await testAutocomplete(term: "All") try await testSearch(term: "All") } - func test_autocomplete_allSuras_prefixed() async throws { - try await enumerateAllSuras { sura, language in + func test_autocomplete_allSuras_prefixed() async { + await enumerateAllSuras { sura, language in // Autocomplete sura name unchanged. let suraName = sura.localizedName(withPrefix: false, language: language) let suraNamePrefix = prefixSuraName(suraName) - let result = try await searcher.autocomplete(term: suraNamePrefix, quran: quran) + let result = await searcher.autocomplete(term: suraNamePrefix, quran: quran) .map { $0.removeInvalidSearchCharacters() } let strippedSuraName = suraName.removeInvalidSearchCharacters() XCTAssertTrue(result.contains(strippedSuraName), "[\(language)] \(result) doesn't contain \(strippedSuraName)") @@ -119,7 +119,7 @@ class CompositeSearcherTests: XCTestCase { let cleanedSuraNamePrefix = prefixSuraName(cleanedSuraName) // Check autocomplete partial pure letters and spaces sura name results contains the full sura name. - let autocompleteResult = try await searcher.autocomplete(term: cleanedSuraNamePrefix, quran: quran) + let autocompleteResult = await searcher.autocomplete(term: cleanedSuraNamePrefix, quran: quran) .map { $0.removeInvalidSearchCharacters() } XCTAssertTrue(autocompleteResult.contains(cleanedSuraName), "[\(language)] \(autocompleteResult) doesn't contain \(cleanedSuraName)") @@ -141,13 +141,13 @@ class CompositeSearcherTests: XCTestCase { TestData.sahihTranslation, ] - private func autocompleteNumber(_ number: String) async throws { - let result = try await searcher.autocomplete(term: number, quran: quran) + private func autocompleteNumber(_ number: String) async { + let result = await searcher.autocomplete(term: number, quran: quran) XCTAssertEqual(result, [number]) } - private func testAutocomplete(term: String, record: Bool = false, testName: String = #function) async throws { - let result = try await searcher.autocomplete(term: term, quran: quran) + private func testAutocomplete(term: String, record: Bool = false, testName: String = #function) async { + let result = await searcher.autocomplete(term: term, quran: quran) // assert the text assertSnapshot(matching: result.sorted(), as: .json, record: record, testName: testName) @@ -160,7 +160,7 @@ class CompositeSearcherTests: XCTestCase { assertSnapshot(matching: result, as: .json, record: record, testName: testName) } - private func enumerateAllSuras(_ block: (Sura, Language) async throws -> Void) async throws { + private func enumerateAllSuras(_ block: (Sura, Language) async throws -> Void) async rethrows { for language in [Language.arabic, Language.english] { for sura in quran.suras { try await block(sura, language) diff --git a/Features/SearchFeature/SearchTypes.swift b/Features/SearchFeature/SearchTypes.swift new file mode 100644 index 00000000..e9883892 --- /dev/null +++ b/Features/SearchFeature/SearchTypes.swift @@ -0,0 +1,35 @@ +// +// SearchTypes.swift +// +// +// Created by Mohamed Afifi on 2023-12-16. +// + +import Foundation + +enum SearchUIState { + case entry + case autocomplete + case loading + case searchResults +} + +enum SearchTerm { + case autocomplete(_ term: String) + case noAction(_ term: String) + + // MARK: Internal + + var term: String { + switch self { + case .autocomplete(let term), .noAction(let term): return term + } + } + + var isAutocomplete: Bool { + switch self { + case .autocomplete: return true + case .noAction: return false + } + } +} diff --git a/Features/SearchFeature/SearchView.swift b/Features/SearchFeature/SearchView.swift index 713560de..6cf68aa3 100644 --- a/Features/SearchFeature/SearchView.swift +++ b/Features/SearchFeature/SearchView.swift @@ -18,14 +18,14 @@ struct SearchView: View { var body: some View { SearchViewUI( error: $viewModel.error, - type: viewModel.searchType, + state: viewModel.uiState, term: viewModel.searchTerm.term, recents: viewModel.recents, populars: viewModel.populars, autocompletions: viewModel.autocompletions, searchResults: viewModel.searchResults, start: { await viewModel.start() }, - search: { await viewModel.search(searchTerm: $0) }, + search: { viewModel.search(for: $0) }, selectSearchResult: { viewModel.select(searchResult: $0.result, source: $0.source) } ) } @@ -34,7 +34,7 @@ struct SearchView: View { private struct SearchViewUI: View { @Binding var error: Error? - let type: SearchUIType + let state: SearchUIState let term: String let recents: [String] @@ -48,7 +48,7 @@ private struct SearchViewUI: View { var body: some View { Group { - switch type { + switch state { case .entry: entry case .autocomplete: @@ -186,14 +186,14 @@ struct SearchView_Previews: PreviewProvider { ]) @State var results = [populatedResults] - @State var type = SearchUIType.searchResults + @State var state = SearchUIState.searchResults @State var error: Error? var body: some View { NavigationView { SearchViewUI( error: $error, - type: type, + state: state, term: "is", recents: ["Recent 1", "Recent 2"], populars: ["Popular 1", "Popular 2"], @@ -207,17 +207,17 @@ struct SearchView_Previews: PreviewProvider { .toolbar { ScrollView(.horizontal) { HStack { - Button("Entry") { type = .entry } - Button("Autocomplete") { type = .autocomplete } + Button("Entry") { state = .entry } + Button("Autocomplete") { state = .autocomplete } Button("Search") { - type = .searchResults + state = .searchResults results = [Self.populatedResults] } Button("No Results") { - type = .searchResults + state = .searchResults results = [] } - Button("Loading") { type = .loading } + Button("Loading") { state = .loading } Button("Error") { error = URLError(.notConnectedToInternet) } } } diff --git a/Features/SearchFeature/SearchViewController.swift b/Features/SearchFeature/SearchViewController.swift index 7c52e4f1..0c5abc93 100644 --- a/Features/SearchFeature/SearchViewController.swift +++ b/Features/SearchFeature/SearchViewController.swift @@ -73,14 +73,12 @@ final class SearchViewController: UIViewController, UISearchResultsUpdating, UIS func updateSearchResults(for searchController: UISearchController) { let term = searchController.searchBar.text ?? "" if viewModel.searchTerm.term != term { - viewModel.searchTerm = .autocomple(term) + viewModel.searchTerm = .autocomplete(term) } } func searchBarSearchButtonClicked(_ searchBar: UISearchBar) { - Task { - await viewModel.search() - } + viewModel.searchForUserTypedTerm() } // MARK: Private diff --git a/Features/SearchFeature/SearchViewModel.swift b/Features/SearchFeature/SearchViewModel.swift index de6d0054..59acc9b6 100644 --- a/Features/SearchFeature/SearchViewModel.swift +++ b/Features/SearchFeature/SearchViewModel.swift @@ -15,26 +15,6 @@ import ReadingService import TranslationService import VLogging -enum SearchUIType { - case entry - case autocomplete - case loading - case searchResults -} - -struct SearchTerm { - let term: String - let autocomplete: Bool - - static func autocomple(_ term: String) -> Self { - SearchTerm(term: term, autocomplete: true) - } - - static func noAction(_ term: String) -> Self { - SearchTerm(term: term, autocomplete: false) - } -} - @MainActor final class SearchViewModel: ObservableObject { // MARK: Lifecycle @@ -49,9 +29,9 @@ final class SearchViewModel: ObservableObject { @Published var error: Error? = nil - @Published var searchType = SearchUIType.entry + @Published var uiState = SearchUIState.entry - @Published var searchTerm = SearchTerm(term: "", autocomplete: false) + @Published var searchTerm = SearchTerm.noAction("") @Published var autocompletions: [String] = [] @Published var searchResults: [SearchResults] = [] @Published var recents: [String] = [] @@ -63,7 +43,7 @@ final class SearchViewModel: ObservableObject { func start() async { async let reading: () = observeReadingChanges() async let autocomplete: () = observeSearchTermChanges() - async let recents: () = observeRecentChanges() + async let recents: () = observeRecentSearchItemsChanges() _ = await [reading, autocomplete, recents] } @@ -86,14 +66,32 @@ final class SearchViewModel: ObservableObject { navigateTo(searchResult.ayah) } - func search(searchTerm term: String) async { + func searchForUserTypedTerm() { + search(for: searchTerm.term) + } + + func search(for term: String) { resignSearchBar.send() searchTerm = .noAction(term) - await search() + + Task { + await search() + } } - func search() async { - searchType = .loading + // MARK: Private + + private let analytics: AnalyticsLibrary + private let searchService: CompositeSearcher + private let navigateTo: (AyahNumber) -> Void + + private let recentsService = SearchRecentsService.shared + private let readingPreferences = ReadingPreferences.shared + private let contentStatePreferences = QuranContentStatePreferences.shared + private let selectedTranslationsPreferences = SelectedTranslationsPreferences.shared + + private func search() async { + uiState = .loading let term = searchTerm.term do { @@ -105,7 +103,7 @@ final class SearchViewModel: ObservableObject { searchResults = results recentsService.addToRecents(term) - searchType = .searchResults + uiState = .searchResults } } catch { logger.error("Error while searching. Error: \(error)") @@ -114,22 +112,11 @@ final class SearchViewModel: ObservableObject { } searchResults = [] self.error = error - searchType = .searchResults + uiState = .searchResults } } - // MARK: Private - - private let analytics: AnalyticsLibrary - private let searchService: CompositeSearcher - private let navigateTo: (AyahNumber) -> Void - - private let recentsService = SearchRecentsService.shared - private let readingPreferences = ReadingPreferences.shared - private let contentStatePreferences = QuranContentStatePreferences.shared - private let selectedTranslationsPreferences = SelectedTranslationsPreferences.shared - - private func observeRecentChanges() async { + private func observeRecentSearchItemsChanges() async { let recentsSequence = recentsService.$recentSearchItems .prepend(recentsService.recentSearchItems) .values() @@ -149,12 +136,12 @@ final class SearchViewModel: ObservableObject { private func observeSearchTermChanges() async { let searchTermSequence = $searchTerm .dropFirst() - .filter(\.autocomplete) + .filter(\.isAutocomplete) .map(\.term) .throttle(for: .milliseconds(300), scheduler: DispatchQueue.main, latest: true) .values() for await term in searchTermSequence { - if searchType == .loading { + if uiState == .loading { continue } @@ -162,25 +149,17 @@ final class SearchViewModel: ObservableObject { if searchTerm.term == term { self.autocompletions = autocompletions if !autocompletions.isEmpty { - searchType = .autocomplete + uiState = .autocomplete } else { - searchType = .entry + uiState = .entry } } } } private func autocomplete(_ term: String) async -> [String] { - if term.trimmingCharacters(in: .whitespaces).isEmpty { - return [] - } - do { - let quran = readingPreferences.reading.quran - return try await searchService.autocomplete(term: term, quran: quran) - } catch { - logger.error("Error while trying to autocomplete. Error: \(error)") - return [] - } + let quran = readingPreferences.reading.quran + return await searchService.autocomplete(term: term, quran: quran) } }