From a199abd5ab063b99cf09fb5d2d15752250f94d45 Mon Sep 17 00:00:00 2001 From: Eric Andrews Date: Sun, 1 Oct 2023 18:20:08 -0400 Subject: [PATCH 1/7] made recent searches be stored per-account --- Mlem/API/APIClient/APIClient.swift | 5 +++ .../Trackers/RecentSearchesTracker.swift | 40 +++++++++++-------- Mlem/Repositories/PersistenceRepository.swift | 12 ++++-- Mlem/Views/Tabs/Search/SearchView.swift | 25 ++++++------ 4 files changed, 49 insertions(+), 33 deletions(-) diff --git a/Mlem/API/APIClient/APIClient.swift b/Mlem/API/APIClient/APIClient.swift index 4201934de..c95e37fd4 100644 --- a/Mlem/API/APIClient/APIClient.swift +++ b/Mlem/API/APIClient/APIClient.swift @@ -41,6 +41,8 @@ class APIClient { let decoder: JSONDecoder let transport: (URLSession, URLRequest) async throws -> (Data, URLResponse) + /// Stores the hash value of the current account + private(set) var accountHash: Int? private(set) var session: APISession = .undefined // MARK: - Initialisation @@ -48,11 +50,13 @@ class APIClient { init( urlSession: URLSession = .init(configuration: .default), decoder: JSONDecoder = .defaultDecoder, + accountHash: Int? = nil, transport: @escaping (URLSession, URLRequest) async throws -> (Data, URLResponse) ) { self.urlSession = urlSession self.decoder = decoder self.transport = transport + self.accountHash = accountHash } // MARK: - Public methods @@ -62,6 +66,7 @@ class APIClient { func configure(for flow: AppFlow) { switch flow { case let .account(account): + accountHash = account.hashValue session = .authenticated(account.instanceLink, account.accessToken) case .onboarding: // no calls to our `APIClient` should be made during onboarding diff --git a/Mlem/Models/Trackers/RecentSearchesTracker.swift b/Mlem/Models/Trackers/RecentSearchesTracker.swift index 55a1ad666..009622f71 100644 --- a/Mlem/Models/Trackers/RecentSearchesTracker.swift +++ b/Mlem/Models/Trackers/RecentSearchesTracker.swift @@ -18,20 +18,26 @@ class RecentSearchesTracker: ObservableObject { var hasLoaded: Bool = false @Published var recentSearches: [AnyContentModel] = .init() - func loadRecentSearches() async throws { - hasLoaded = true - let identifiers = persistenceRepository.loadRecentSearches() - for id in identifiers { - print(id.contentType, id.contentId) - switch id.contentType { - case .post: - break - case .community: - let community: CommunityModel = try await communityRepository.loadDetails(for: id.contentId) - recentSearches.append(AnyContentModel(community)) - case .user: - let user = try await personRepository.loadDetails(for: id.contentId) - recentSearches.append(AnyContentModel(user)) + /// clears recentSearches and loads new values based on the current account + func reloadRecentSearches() async throws { + defer { hasLoaded = true } + + recentSearches = .init() + if let accountHash = apiClient.accountHash { + let identifiers = persistenceRepository.loadRecentSearches(accountHash: accountHash) + + for id in identifiers { + print(id.contentType, id.contentId) + switch id.contentType { + case .post: + break + case .community: + let community: CommunityModel = try await communityRepository.loadDetails(for: id.contentId) + recentSearches.append(AnyContentModel(community)) + case .user: + let user = try await personRepository.loadDetails(for: id.contentId) + recentSearches.append(AnyContentModel(user)) + } } } } @@ -58,8 +64,10 @@ class RecentSearchesTracker: ObservableObject { } private func saveRecentSearches() { - Task(priority: .background) { - try await persistenceRepository.saveRecentSearches(recentSearches.map { $0.uid }) + if let accountHash = apiClient.accountHash { + Task(priority: .background) { + try await persistenceRepository.saveRecentSearches(for: accountHash, with: recentSearches.map(\.uid)) + } } } } diff --git a/Mlem/Repositories/PersistenceRepository.swift b/Mlem/Repositories/PersistenceRepository.swift index 91d43a222..ea585c833 100644 --- a/Mlem/Repositories/PersistenceRepository.swift +++ b/Mlem/Repositories/PersistenceRepository.swift @@ -83,12 +83,16 @@ class PersistenceRepository { try await save(value, to: Path.savedAccounts) } - func loadRecentSearches() -> [ContentModelIdentifier] { - load([ContentModelIdentifier].self, from: Path.recentSearches) ?? [] + func loadRecentSearches(accountHash: Int) -> [ContentModelIdentifier] { + let searches = load([Int: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] + return searches[accountHash] ?? [] } - func saveRecentSearches(_ value: [ContentModelIdentifier]) async throws { - try await save(value, to: Path.recentSearches) + func saveRecentSearches(for accountHash: Int, with searches: [ContentModelIdentifier]) async throws { + // get recent searches + var extant = load([Int: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] + extant[accountHash] = searches + try await save(extant, to: Path.recentSearches) } func loadFavoriteCommunities() -> [FavoriteCommunity] { diff --git a/Mlem/Views/Tabs/Search/SearchView.swift b/Mlem/Views/Tabs/Search/SearchView.swift index c7d8d8379..ed031ea9c 100644 --- a/Mlem/Views/Tabs/Search/SearchView.swift +++ b/Mlem/Views/Tabs/Search/SearchView.swift @@ -5,11 +5,11 @@ // Created by Jake Shirley on 7/5/23. // -import Dependencies import Combine +import Dependencies import Foundation -import UIKit import SwiftUI +import UIKit private struct ViewOffsetKey: PreferenceKey { typealias Value = CGFloat @@ -54,20 +54,19 @@ struct SearchView: View { page = .home searchModel.searchText = "" } - - } + } .autocorrectionDisabled(true) .textInputAutocapitalization(.never) .onAppear { Task(priority: .background) { - if !recentSearchesTracker.hasLoaded { - do { - try await recentSearchesTracker.loadRecentSearches() - } catch { - print("Error while loading recent searches: \(error.localizedDescription)") - errorHandler.handle(error) - } + // if !recentSearchesTracker.hasLoaded { + do { + try await recentSearchesTracker.reloadRecentSearches() + } catch { + print("Error while loading recent searches: \(error.localizedDescription)") + errorHandler.handle(error) } + // } } } } @@ -93,8 +92,8 @@ struct SearchView: View { .animation(.default, value: page) } .onChange(of: isSearching) { newValue in - if newValue && searchModel.searchText.isEmpty { - page = .recents + if newValue, searchModel.searchText.isEmpty { + page = .recents } } .onChange(of: searchModel.searchText) { newValue in From 561c6b2361eefa94765ece288a3e68501afecc4b Mon Sep 17 00:00:00 2001 From: Eric Andrews Date: Sun, 1 Oct 2023 18:25:17 -0400 Subject: [PATCH 2/7] tidied --- Mlem/Repositories/PersistenceRepository.swift | 1 - Mlem/Views/Tabs/Search/SearchView.swift | 2 -- 2 files changed, 3 deletions(-) diff --git a/Mlem/Repositories/PersistenceRepository.swift b/Mlem/Repositories/PersistenceRepository.swift index ea585c833..ebe2c37d2 100644 --- a/Mlem/Repositories/PersistenceRepository.swift +++ b/Mlem/Repositories/PersistenceRepository.swift @@ -89,7 +89,6 @@ class PersistenceRepository { } func saveRecentSearches(for accountHash: Int, with searches: [ContentModelIdentifier]) async throws { - // get recent searches var extant = load([Int: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] extant[accountHash] = searches try await save(extant, to: Path.recentSearches) diff --git a/Mlem/Views/Tabs/Search/SearchView.swift b/Mlem/Views/Tabs/Search/SearchView.swift index ed031ea9c..e23a5b500 100644 --- a/Mlem/Views/Tabs/Search/SearchView.swift +++ b/Mlem/Views/Tabs/Search/SearchView.swift @@ -59,14 +59,12 @@ struct SearchView: View { .textInputAutocapitalization(.never) .onAppear { Task(priority: .background) { - // if !recentSearchesTracker.hasLoaded { do { try await recentSearchesTracker.reloadRecentSearches() } catch { print("Error while loading recent searches: \(error.localizedDescription)") errorHandler.handle(error) } - // } } } } From 3e1b2575649a3b18930c26f5602ca59f94835739 Mon Sep 17 00:00:00 2001 From: Eric Andrews Date: Sun, 1 Oct 2023 18:38:03 -0400 Subject: [PATCH 3/7] added unit tests --- .../Trackers/RecentSearchesTracker.swift | 2 +- Mlem/Repositories/PersistenceRepository.swift | 2 +- .../PersistenceRepositoryTests.swift | 22 ++++++++++++------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/Mlem/Models/Trackers/RecentSearchesTracker.swift b/Mlem/Models/Trackers/RecentSearchesTracker.swift index 009622f71..3fa8f1518 100644 --- a/Mlem/Models/Trackers/RecentSearchesTracker.swift +++ b/Mlem/Models/Trackers/RecentSearchesTracker.swift @@ -24,7 +24,7 @@ class RecentSearchesTracker: ObservableObject { recentSearches = .init() if let accountHash = apiClient.accountHash { - let identifiers = persistenceRepository.loadRecentSearches(accountHash: accountHash) + let identifiers = persistenceRepository.loadRecentSearches(for: accountHash) for id in identifiers { print(id.contentType, id.contentId) diff --git a/Mlem/Repositories/PersistenceRepository.swift b/Mlem/Repositories/PersistenceRepository.swift index ebe2c37d2..156030d91 100644 --- a/Mlem/Repositories/PersistenceRepository.swift +++ b/Mlem/Repositories/PersistenceRepository.swift @@ -83,7 +83,7 @@ class PersistenceRepository { try await save(value, to: Path.savedAccounts) } - func loadRecentSearches(accountHash: Int) -> [ContentModelIdentifier] { + func loadRecentSearches(for accountHash: Int) -> [ContentModelIdentifier] { let searches = load([Int: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] return searches[accountHash] ?? [] } diff --git a/MlemTests/Persistence/PersistenceRepositoryTests.swift b/MlemTests/Persistence/PersistenceRepositoryTests.swift index 9219cfd29..0b4387b36 100644 --- a/MlemTests/Persistence/PersistenceRepositoryTests.swift +++ b/MlemTests/Persistence/PersistenceRepositoryTests.swift @@ -122,24 +122,30 @@ final class PersistenceRepositoryTests: XCTestCase { func testSaveRecentSearches() async throws { let searches: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] - try await repository.saveRecentSearches(searches) // write the examples to disk - let searchesFromDisk = try load([ContentModelIdentifier].self) // load them from the disk _without_ using the repository + try await repository.saveRecentSearches(for: 1, with: searches) // write the examples to disk + let searchesFromDisk = try load([Int: [ContentModelIdentifier]].self) // load them from the disk _without_ using the repository - XCTAssertEqual(searches, searchesFromDisk) // confirm what was written to disk matches what we sent in + let expected: [Int: [ContentModelIdentifier]] = [1: searches] + XCTAssertEqual(expected, searchesFromDisk) // confirm what was written to disk matches what we sent in } func testLoadRecentSearchesWithValues() async throws { - let searches: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] + let searches1: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] + let searches2: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 2), .init(contentType: .community, contentId: 3)] + + try await repository.saveRecentSearches(for: 1, with: searches1) + try await repository.saveRecentSearches(for: 2, with: searches2) - try await repository.saveRecentSearches(searches) // write the example terms to the disk - let loadedSearches = repository.loadRecentSearches() // read them back + let loadedSearches1 = repository.loadRecentSearches(accountHash: 1) // read them back + let loadedSearches2 = repository.loadRecentSearches(accountHash: 2) - XCTAssertEqual(loadedSearches, searches) // assert we were given the same values back + XCTAssertEqual(loadedSearches1, searches1) // assert we were given the same values back + XCTAssertEqual(loadedSearches2, searches2) } func testLoadRecentSearchesWithoutValues() async throws { XCTAssert(disk.isEmpty) // assert that our mock disk has nothing in it - let loadedSearches = repository.loadRecentSearches() // perform a load knowing the disk is empty + let loadedSearches = repository.loadRecentSearches(accountHash: 1) // perform a load knowing the disk is empty XCTAssert(loadedSearches.isEmpty) // assert we were returned an empty list } From e7c1484c7afbb7daf81233103b264a492bc042b3 Mon Sep 17 00:00:00 2001 From: Eric Andrews Date: Sun, 1 Oct 2023 19:48:40 -0400 Subject: [PATCH 4/7] changed source of truth for current account to appState --- Mlem/API/APIClient/APIClient.swift | 5 ----- Mlem/Models/Trackers/RecentSearchesTracker.swift | 16 ++++++++-------- Mlem/Views/Tabs/Search/RecentSearchesView.swift | 10 +++++----- .../Views/Tabs/Search/SearchResultListView.swift | 5 +++-- Mlem/Views/Tabs/Search/SearchView.swift | 3 ++- MlemTests/Model/LemmyURLTests.swift | 13 +++++++------ .../Persistence/PersistenceRepositoryTests.swift | 6 +++--- 7 files changed, 28 insertions(+), 30 deletions(-) diff --git a/Mlem/API/APIClient/APIClient.swift b/Mlem/API/APIClient/APIClient.swift index c95e37fd4..4201934de 100644 --- a/Mlem/API/APIClient/APIClient.swift +++ b/Mlem/API/APIClient/APIClient.swift @@ -41,8 +41,6 @@ class APIClient { let decoder: JSONDecoder let transport: (URLSession, URLRequest) async throws -> (Data, URLResponse) - /// Stores the hash value of the current account - private(set) var accountHash: Int? private(set) var session: APISession = .undefined // MARK: - Initialisation @@ -50,13 +48,11 @@ class APIClient { init( urlSession: URLSession = .init(configuration: .default), decoder: JSONDecoder = .defaultDecoder, - accountHash: Int? = nil, transport: @escaping (URLSession, URLRequest) async throws -> (Data, URLResponse) ) { self.urlSession = urlSession self.decoder = decoder self.transport = transport - self.accountHash = accountHash } // MARK: - Public methods @@ -66,7 +62,6 @@ class APIClient { func configure(for flow: AppFlow) { switch flow { case let .account(account): - accountHash = account.hashValue session = .authenticated(account.instanceLink, account.accessToken) case .onboarding: // no calls to our `APIClient` should be made during onboarding diff --git a/Mlem/Models/Trackers/RecentSearchesTracker.swift b/Mlem/Models/Trackers/RecentSearchesTracker.swift index 3fa8f1518..bf1e16ef9 100644 --- a/Mlem/Models/Trackers/RecentSearchesTracker.swift +++ b/Mlem/Models/Trackers/RecentSearchesTracker.swift @@ -19,11 +19,11 @@ class RecentSearchesTracker: ObservableObject { @Published var recentSearches: [AnyContentModel] = .init() /// clears recentSearches and loads new values based on the current account - func reloadRecentSearches() async throws { + func reloadRecentSearches(accountHash: Int?) async throws { defer { hasLoaded = true } recentSearches = .init() - if let accountHash = apiClient.accountHash { + if let accountHash { let identifiers = persistenceRepository.loadRecentSearches(for: accountHash) for id in identifiers { @@ -42,7 +42,7 @@ class RecentSearchesTracker: ObservableObject { } } - func addRecentSearch(_ item: AnyContentModel) { + func addRecentSearch(_ item: AnyContentModel, accountHash: Int?) { // if the item is already in the recent list, move it to the top if let index = recentSearches.firstIndex(of: item) { recentSearches.remove(at: index) @@ -55,16 +55,16 @@ class RecentSearchesTracker: ObservableObject { recentSearches = recentSearches.dropLast(1) } } - saveRecentSearches() + saveRecentSearches(accountHash: accountHash) } - func clearRecentSearches() { + func clearRecentSearches(accountHash: Int?) { recentSearches.removeAll() - saveRecentSearches() + saveRecentSearches(accountHash: accountHash) } - private func saveRecentSearches() { - if let accountHash = apiClient.accountHash { + private func saveRecentSearches(accountHash: Int?) { + if let accountHash { Task(priority: .background) { try await persistenceRepository.saveRecentSearches(for: accountHash, with: recentSearches.map(\.uid)) } diff --git a/Mlem/Views/Tabs/Search/RecentSearchesView.swift b/Mlem/Views/Tabs/Search/RecentSearchesView.swift index 2c4f5ff23..b2844b20e 100644 --- a/Mlem/Views/Tabs/Search/RecentSearchesView.swift +++ b/Mlem/Views/Tabs/Search/RecentSearchesView.swift @@ -8,7 +8,7 @@ import SwiftUI struct RecentSearchesView: View { - + @EnvironmentObject var appState: AppState @EnvironmentObject var recentSearchesTracker: RecentSearchesTracker @StateObject var contentTracker: ContentTracker = .init() @@ -17,8 +17,8 @@ struct RecentSearchesView: View { if !recentSearchesTracker.recentSearches.isEmpty { VStack(alignment: .leading, spacing: 0) { headerView - .padding(.top, 15) - .padding(.bottom, 6) + .padding(.top, 15) + .padding(.bottom, 6) Divider() itemsView } @@ -47,7 +47,7 @@ struct RecentSearchesView: View { Spacer() Button { - recentSearchesTracker.clearRecentSearches() + recentSearchesTracker.clearRecentSearches(accountHash: appState.currentActiveAccount?.hashValue) } label: { Text("Clear") .font(.subheadline) @@ -68,7 +68,7 @@ struct RecentSearchesView: View { } } .simultaneousGesture(TapGesture().onEnded { - recentSearchesTracker.addRecentSearch(contentModel) + recentSearchesTracker.addRecentSearch(contentModel, accountHash: appState.currentActiveAccount?.hashValue) }) Divider() } diff --git a/Mlem/Views/Tabs/Search/SearchResultListView.swift b/Mlem/Views/Tabs/Search/SearchResultListView.swift index 498278d87..8fa14e330 100644 --- a/Mlem/Views/Tabs/Search/SearchResultListView.swift +++ b/Mlem/Views/Tabs/Search/SearchResultListView.swift @@ -8,6 +8,7 @@ import SwiftUI struct SearchResultListView: View { + @EnvironmentObject var appState: AppState @EnvironmentObject var recentSearchesTracker: RecentSearchesTracker @EnvironmentObject var contentTracker: ContentTracker @@ -26,7 +27,7 @@ struct SearchResultListView: View { } } .simultaneousGesture(TapGesture().onEnded { - recentSearchesTracker.addRecentSearch(contentModel) + recentSearchesTracker.addRecentSearch(contentModel, accountHash: appState.currentActiveAccount?.hashValue) }) Divider() .onAppear { @@ -54,7 +55,7 @@ struct SearchResultListView: View { } else if contentTracker.items.isEmpty { Text("No results found.") .foregroundStyle(.secondary) - } else if contentTracker.hasReachedEnd && contentTracker.items.count > 10 { + } else if contentTracker.hasReachedEnd, contentTracker.items.count > 10 { HStack { Image(systemName: "figure.climbing") Text("I think I've found the bottom!") diff --git a/Mlem/Views/Tabs/Search/SearchView.swift b/Mlem/Views/Tabs/Search/SearchView.swift index e23a5b500..ec633fd4b 100644 --- a/Mlem/Views/Tabs/Search/SearchView.swift +++ b/Mlem/Views/Tabs/Search/SearchView.swift @@ -27,6 +27,7 @@ struct SearchView: View { } // environment + @EnvironmentObject var appState: AppState @EnvironmentObject private var recentSearchesTracker: RecentSearchesTracker @StateObject var searchModel: SearchModel @@ -60,7 +61,7 @@ struct SearchView: View { .onAppear { Task(priority: .background) { do { - try await recentSearchesTracker.reloadRecentSearches() + try await recentSearchesTracker.reloadRecentSearches(accountHash: appState.currentActiveAccount?.hashValue) } catch { print("Error while loading recent searches: \(error.localizedDescription)") errorHandler.handle(error) diff --git a/MlemTests/Model/LemmyURLTests.swift b/MlemTests/Model/LemmyURLTests.swift index 864597df5..6571113e1 100644 --- a/MlemTests/Model/LemmyURLTests.swift +++ b/MlemTests/Model/LemmyURLTests.swift @@ -17,12 +17,13 @@ final class LemmyURLTests: XCTestCase { XCTAssertEqual(lemmyUrl?.url.absoluteString, validUrl) } - func testHandlesUnencodedURL() throws { - let unencodedUrl = "https://matrix.to/#/#space:lemmy.world" - let lemmyUrl = LemmyURL(string: unencodedUrl) - // expectation is that the # character will be encoded to %23 - XCTAssertEqual(lemmyUrl?.url.absoluteString, "https://matrix.to/%23/%23space:lemmy.world") - } + // NOTE: this test is failing because LemmyURL successfully creates a URL from the given string. Commented for now because OOS for this PR +// func testHandlesUnencodedURL() throws { +// let unencodedUrl = "https://matrix.to/#/#space:lemmy.world" +// let lemmyUrl = LemmyURL(string: unencodedUrl) +// // expectation is that the # character will be encoded to %23 +// XCTAssertEqual(lemmyUrl?.url.absoluteString, "https://matrix.to/%23/%23space:lemmy.world") +// } func testHandlesEncodedURL() throws { let encodedUrl = "https://matrix.to/%23/%23space:lemmy.world" diff --git a/MlemTests/Persistence/PersistenceRepositoryTests.swift b/MlemTests/Persistence/PersistenceRepositoryTests.swift index 0b4387b36..debf3483a 100644 --- a/MlemTests/Persistence/PersistenceRepositoryTests.swift +++ b/MlemTests/Persistence/PersistenceRepositoryTests.swift @@ -136,8 +136,8 @@ final class PersistenceRepositoryTests: XCTestCase { try await repository.saveRecentSearches(for: 1, with: searches1) try await repository.saveRecentSearches(for: 2, with: searches2) - let loadedSearches1 = repository.loadRecentSearches(accountHash: 1) // read them back - let loadedSearches2 = repository.loadRecentSearches(accountHash: 2) + let loadedSearches1 = repository.loadRecentSearches(for: 1) // read them back + let loadedSearches2 = repository.loadRecentSearches(for: 2) XCTAssertEqual(loadedSearches1, searches1) // assert we were given the same values back XCTAssertEqual(loadedSearches2, searches2) @@ -145,7 +145,7 @@ final class PersistenceRepositoryTests: XCTestCase { func testLoadRecentSearchesWithoutValues() async throws { XCTAssert(disk.isEmpty) // assert that our mock disk has nothing in it - let loadedSearches = repository.loadRecentSearches(accountHash: 1) // perform a load knowing the disk is empty + let loadedSearches = repository.loadRecentSearches(for: 1) // perform a load knowing the disk is empty XCTAssert(loadedSearches.isEmpty) // assert we were returned an empty list } From afa1a7ef54c79b455d14e56f21a0e11b4d72d3d5 Mon Sep 17 00:00:00 2001 From: Eric Andrews Date: Tue, 3 Oct 2023 13:00:17 -0400 Subject: [PATCH 5/7] using stable id string instead of hash value --- Mlem/Models/Saved Account.swift | 5 +++++ .../Trackers/RecentSearchesTracker.swift | 21 ++++++++++--------- Mlem/Repositories/PersistenceRepository.swift | 12 +++++------ .../Tabs/Search/RecentSearchesView.swift | 4 ++-- .../Tabs/Search/SearchResultListView.swift | 2 +- Mlem/Views/Tabs/Search/SearchView.swift | 2 +- MlemTests/Model/LemmyURLTests.swift | 14 ++++++------- 7 files changed, 33 insertions(+), 27 deletions(-) diff --git a/Mlem/Models/Saved Account.swift b/Mlem/Models/Saved Account.swift index 324639e36..664c1567c 100644 --- a/Mlem/Models/Saved Account.swift +++ b/Mlem/Models/Saved Account.swift @@ -14,6 +14,7 @@ struct SavedAccount: Identifiable, Codable, Equatable, Hashable { let username: String let storedNickname: String? let avatarUrl: URL? + let stableIdString: String init( id: Int, @@ -29,6 +30,9 @@ struct SavedAccount: Identifiable, Codable, Equatable, Hashable { self.username = username self.storedNickname = storedNickname self.avatarUrl = avatarUrl + + assert(instanceLink.host() != nil, "nil instance link host!") + self.stableIdString = "\(username)@\(instanceLink.host() ?? "unknown")" } // Convenience initializer to create an equal copy with different non-identifying properties. @@ -44,6 +48,7 @@ struct SavedAccount: Identifiable, Codable, Equatable, Hashable { self.username = account.username self.storedNickname = storedNickname ?? account.storedNickname self.avatarUrl = avatarUrl + self.stableIdString = account.stableIdString } // convenience diff --git a/Mlem/Models/Trackers/RecentSearchesTracker.swift b/Mlem/Models/Trackers/RecentSearchesTracker.swift index bf1e16ef9..07d19f977 100644 --- a/Mlem/Models/Trackers/RecentSearchesTracker.swift +++ b/Mlem/Models/Trackers/RecentSearchesTracker.swift @@ -19,12 +19,12 @@ class RecentSearchesTracker: ObservableObject { @Published var recentSearches: [AnyContentModel] = .init() /// clears recentSearches and loads new values based on the current account - func reloadRecentSearches(accountHash: Int?) async throws { + func reloadRecentSearches(accountId: String?) async throws { defer { hasLoaded = true } recentSearches = .init() - if let accountHash { - let identifiers = persistenceRepository.loadRecentSearches(for: accountHash) + if let accountId { + let identifiers = persistenceRepository.loadRecentSearches(for: accountId) for id in identifiers { print(id.contentType, id.contentId) @@ -42,7 +42,7 @@ class RecentSearchesTracker: ObservableObject { } } - func addRecentSearch(_ item: AnyContentModel, accountHash: Int?) { + func addRecentSearch(_ item: AnyContentModel, accountId: String?) { // if the item is already in the recent list, move it to the top if let index = recentSearches.firstIndex(of: item) { recentSearches.remove(at: index) @@ -55,18 +55,19 @@ class RecentSearchesTracker: ObservableObject { recentSearches = recentSearches.dropLast(1) } } - saveRecentSearches(accountHash: accountHash) + saveRecentSearches(accountId: accountId) } - func clearRecentSearches(accountHash: Int?) { + func clearRecentSearches(accountId: String?) { recentSearches.removeAll() - saveRecentSearches(accountHash: accountHash) + saveRecentSearches(accountId: accountId) } - private func saveRecentSearches(accountHash: Int?) { - if let accountHash { + private func saveRecentSearches(accountId: String?) { + if let accountId { + print("saving searches for \(accountId)") Task(priority: .background) { - try await persistenceRepository.saveRecentSearches(for: accountHash, with: recentSearches.map(\.uid)) + try await persistenceRepository.saveRecentSearches(for: accountId, with: recentSearches.map(\.uid)) } } } diff --git a/Mlem/Repositories/PersistenceRepository.swift b/Mlem/Repositories/PersistenceRepository.swift index 156030d91..ff31048e3 100644 --- a/Mlem/Repositories/PersistenceRepository.swift +++ b/Mlem/Repositories/PersistenceRepository.swift @@ -83,14 +83,14 @@ class PersistenceRepository { try await save(value, to: Path.savedAccounts) } - func loadRecentSearches(for accountHash: Int) -> [ContentModelIdentifier] { - let searches = load([Int: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] - return searches[accountHash] ?? [] + func loadRecentSearches(for accountId: String) -> [ContentModelIdentifier] { + let searches = load([String: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] + return searches[accountId] ?? [] } - func saveRecentSearches(for accountHash: Int, with searches: [ContentModelIdentifier]) async throws { - var extant = load([Int: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] - extant[accountHash] = searches + func saveRecentSearches(for accountId: String, with searches: [ContentModelIdentifier]) async throws { + var extant = load([String: [ContentModelIdentifier]].self, from: Path.recentSearches) ?? [:] + extant[accountId] = searches try await save(extant, to: Path.recentSearches) } diff --git a/Mlem/Views/Tabs/Search/RecentSearchesView.swift b/Mlem/Views/Tabs/Search/RecentSearchesView.swift index b2844b20e..d14dd36ab 100644 --- a/Mlem/Views/Tabs/Search/RecentSearchesView.swift +++ b/Mlem/Views/Tabs/Search/RecentSearchesView.swift @@ -47,7 +47,7 @@ struct RecentSearchesView: View { Spacer() Button { - recentSearchesTracker.clearRecentSearches(accountHash: appState.currentActiveAccount?.hashValue) + recentSearchesTracker.clearRecentSearches(accountId: appState.currentActiveAccount?.stableIdString) } label: { Text("Clear") .font(.subheadline) @@ -68,7 +68,7 @@ struct RecentSearchesView: View { } } .simultaneousGesture(TapGesture().onEnded { - recentSearchesTracker.addRecentSearch(contentModel, accountHash: appState.currentActiveAccount?.hashValue) + recentSearchesTracker.addRecentSearch(contentModel, accountId: appState.currentActiveAccount?.stableIdString) }) Divider() } diff --git a/Mlem/Views/Tabs/Search/SearchResultListView.swift b/Mlem/Views/Tabs/Search/SearchResultListView.swift index 8fa14e330..7c83c3b96 100644 --- a/Mlem/Views/Tabs/Search/SearchResultListView.swift +++ b/Mlem/Views/Tabs/Search/SearchResultListView.swift @@ -27,7 +27,7 @@ struct SearchResultListView: View { } } .simultaneousGesture(TapGesture().onEnded { - recentSearchesTracker.addRecentSearch(contentModel, accountHash: appState.currentActiveAccount?.hashValue) + recentSearchesTracker.addRecentSearch(contentModel, accountId: appState.currentActiveAccount?.stableIdString) }) Divider() .onAppear { diff --git a/Mlem/Views/Tabs/Search/SearchView.swift b/Mlem/Views/Tabs/Search/SearchView.swift index ec633fd4b..287471ea6 100644 --- a/Mlem/Views/Tabs/Search/SearchView.swift +++ b/Mlem/Views/Tabs/Search/SearchView.swift @@ -61,7 +61,7 @@ struct SearchView: View { .onAppear { Task(priority: .background) { do { - try await recentSearchesTracker.reloadRecentSearches(accountHash: appState.currentActiveAccount?.hashValue) + try await recentSearchesTracker.reloadRecentSearches(accountId: appState.currentActiveAccount?.stableIdString) } catch { print("Error while loading recent searches: \(error.localizedDescription)") errorHandler.handle(error) diff --git a/MlemTests/Model/LemmyURLTests.swift b/MlemTests/Model/LemmyURLTests.swift index 6571113e1..693bf685e 100644 --- a/MlemTests/Model/LemmyURLTests.swift +++ b/MlemTests/Model/LemmyURLTests.swift @@ -17,13 +17,13 @@ final class LemmyURLTests: XCTestCase { XCTAssertEqual(lemmyUrl?.url.absoluteString, validUrl) } - // NOTE: this test is failing because LemmyURL successfully creates a URL from the given string. Commented for now because OOS for this PR -// func testHandlesUnencodedURL() throws { -// let unencodedUrl = "https://matrix.to/#/#space:lemmy.world" -// let lemmyUrl = LemmyURL(string: unencodedUrl) -// // expectation is that the # character will be encoded to %23 -// XCTAssertEqual(lemmyUrl?.url.absoluteString, "https://matrix.to/%23/%23space:lemmy.world") -// } + // NOTE: this test fails on XCode 15+ + func testHandlesUnencodedURL() throws { + let unencodedUrl = "https://matrix.to/#/#space:lemmy.world" + let lemmyUrl = LemmyURL(string: unencodedUrl) + // expectation is that the # character will be encoded to %23 + XCTAssertEqual(lemmyUrl?.url.absoluteString, "https://matrix.to/%23/%23space:lemmy.world") + } func testHandlesEncodedURL() throws { let encodedUrl = "https://matrix.to/%23/%23space:lemmy.world" From 0721b52963eb114647452542942ee9e051f54349 Mon Sep 17 00:00:00 2001 From: Eric Andrews Date: Tue, 3 Oct 2023 13:20:13 -0400 Subject: [PATCH 6/7] unit tests --- Mlem/Models/Saved Account.swift | 10 +++++----- .../Persistence/PersistenceRepositoryTests.swift | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Mlem/Models/Saved Account.swift b/Mlem/Models/Saved Account.swift index 664c1567c..705b518a8 100644 --- a/Mlem/Models/Saved Account.swift +++ b/Mlem/Models/Saved Account.swift @@ -14,7 +14,11 @@ struct SavedAccount: Identifiable, Codable, Equatable, Hashable { let username: String let storedNickname: String? let avatarUrl: URL? - let stableIdString: String + + var stableIdString: String { + assert(instanceLink.host() != nil, "nil instance link host!") + return "\(username)@\(instanceLink.host() ?? "unknown")" + } init( id: Int, @@ -30,9 +34,6 @@ struct SavedAccount: Identifiable, Codable, Equatable, Hashable { self.username = username self.storedNickname = storedNickname self.avatarUrl = avatarUrl - - assert(instanceLink.host() != nil, "nil instance link host!") - self.stableIdString = "\(username)@\(instanceLink.host() ?? "unknown")" } // Convenience initializer to create an equal copy with different non-identifying properties. @@ -48,7 +49,6 @@ struct SavedAccount: Identifiable, Codable, Equatable, Hashable { self.username = account.username self.storedNickname = storedNickname ?? account.storedNickname self.avatarUrl = avatarUrl - self.stableIdString = account.stableIdString } // convenience diff --git a/MlemTests/Persistence/PersistenceRepositoryTests.swift b/MlemTests/Persistence/PersistenceRepositoryTests.swift index debf3483a..684496791 100644 --- a/MlemTests/Persistence/PersistenceRepositoryTests.swift +++ b/MlemTests/Persistence/PersistenceRepositoryTests.swift @@ -122,7 +122,7 @@ final class PersistenceRepositoryTests: XCTestCase { func testSaveRecentSearches() async throws { let searches: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] - try await repository.saveRecentSearches(for: 1, with: searches) // write the examples to disk + try await repository.saveRecentSearches(for: "one@test", with: searches) // write the examples to disk let searchesFromDisk = try load([Int: [ContentModelIdentifier]].self) // load them from the disk _without_ using the repository let expected: [Int: [ContentModelIdentifier]] = [1: searches] @@ -133,11 +133,11 @@ final class PersistenceRepositoryTests: XCTestCase { let searches1: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] let searches2: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 2), .init(contentType: .community, contentId: 3)] - try await repository.saveRecentSearches(for: 1, with: searches1) - try await repository.saveRecentSearches(for: 2, with: searches2) + try await repository.saveRecentSearches(for: "one@test", with: searches1) + try await repository.saveRecentSearches(for: "two@test", with: searches2) - let loadedSearches1 = repository.loadRecentSearches(for: 1) // read them back - let loadedSearches2 = repository.loadRecentSearches(for: 2) + let loadedSearches1 = repository.loadRecentSearches(for: "one@test") // read them back + let loadedSearches2 = repository.loadRecentSearches(for: "two@test") XCTAssertEqual(loadedSearches1, searches1) // assert we were given the same values back XCTAssertEqual(loadedSearches2, searches2) @@ -145,7 +145,7 @@ final class PersistenceRepositoryTests: XCTestCase { func testLoadRecentSearchesWithoutValues() async throws { XCTAssert(disk.isEmpty) // assert that our mock disk has nothing in it - let loadedSearches = repository.loadRecentSearches(for: 1) // perform a load knowing the disk is empty + let loadedSearches = repository.loadRecentSearches(for: "one@test") // perform a load knowing the disk is empty XCTAssert(loadedSearches.isEmpty) // assert we were returned an empty list } From bb32aca8290bb9780b2b06b70c0b7c857a3ca1be Mon Sep 17 00:00:00 2001 From: Eric Andrews Date: Tue, 3 Oct 2023 19:41:01 -0400 Subject: [PATCH 7/7] more CI --- MlemTests/Persistence/PersistenceRepositoryTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MlemTests/Persistence/PersistenceRepositoryTests.swift b/MlemTests/Persistence/PersistenceRepositoryTests.swift index 684496791..eebc54156 100644 --- a/MlemTests/Persistence/PersistenceRepositoryTests.swift +++ b/MlemTests/Persistence/PersistenceRepositoryTests.swift @@ -123,9 +123,9 @@ final class PersistenceRepositoryTests: XCTestCase { let searches: [ContentModelIdentifier] = [.init(contentType: .user, contentId: 1), .init(contentType: .community, contentId: 2)] try await repository.saveRecentSearches(for: "one@test", with: searches) // write the examples to disk - let searchesFromDisk = try load([Int: [ContentModelIdentifier]].self) // load them from the disk _without_ using the repository + let searchesFromDisk = try load([String: [ContentModelIdentifier]].self) // load them from the disk _without_ using the repository - let expected: [Int: [ContentModelIdentifier]] = [1: searches] + let expected: [String: [ContentModelIdentifier]] = ["one@test": searches] XCTAssertEqual(expected, searchesFromDisk) // confirm what was written to disk matches what we sent in }