From 2dd1f3ebdd6a98f55082d1ba710acc359b7767a0 Mon Sep 17 00:00:00 2001 From: Justin Mazzocchi <2831158+jzzocc@users.noreply.github.com> Date: Wed, 9 Sep 2020 05:05:43 -0700 Subject: [PATCH] Refactoring --- DB/Sources/DB/Content/AccountResult.swift | 4 +- DB/Sources/DB/Content/StatusRecord.swift | 4 +- DB/Sources/DB/Content/StatusResult.swift | 12 +-- .../DB/Extensions/Identity+Internal.swift | 2 +- .../DB/Extensions/Timeline+Extensions.swift | 2 +- DB/Sources/DB/Identity/IdentityDatabase.swift | 36 ++++---- DB/Sources/DB/Identity/IdentityResult.swift | 9 +- .../Services/AllIdentitiesService.swift | 10 +-- .../Services/IdentityService.swift | 5 +- .../ViewModels/Entities/Identification.swift | 30 +------ .../Sources/ViewModels/RootViewModel.swift | 90 +++++++++++-------- .../AddIdentityViewModelTests.swift | 22 +++-- .../ViewModelsTests/RootViewModelTests.swift | 2 +- Views/AddIdentityView.swift | 2 +- Views/IdentitiesView.swift | 2 +- Views/TabNavigationView.swift | 2 +- 16 files changed, 121 insertions(+), 113 deletions(-) diff --git a/DB/Sources/DB/Content/AccountResult.swift b/DB/Sources/DB/Content/AccountResult.swift index 69dd51c..c8b890b 100644 --- a/DB/Sources/DB/Content/AccountResult.swift +++ b/DB/Sources/DB/Content/AccountResult.swift @@ -9,8 +9,8 @@ struct AccountResult: Codable, Hashable, FetchableRecord { } extension QueryInterfaceRequest where RowDecoder == AccountRecord { - var accountResultRequest: AnyFetchRequest { - AnyFetchRequest(including(optional: AccountRecord.moved)) + var accountResultRequest: QueryInterfaceRequest { + including(optional: AccountRecord.moved) .asRequest(of: AccountResult.self) } } diff --git a/DB/Sources/DB/Content/StatusRecord.swift b/DB/Sources/DB/Content/StatusRecord.swift index d996be0..3bf29ac 100644 --- a/DB/Sources/DB/Content/StatusRecord.swift +++ b/DB/Sources/DB/Content/StatusRecord.swift @@ -74,11 +74,11 @@ extension StatusRecord { through: descendantJoins, using: StatusContextJoin.status) - var ancestors: AnyFetchRequest { + var ancestors: QueryInterfaceRequest { request(for: Self.ancestors).statusResultRequest } - var descendants: AnyFetchRequest { + var descendants: QueryInterfaceRequest { request(for: Self.descendants).statusResultRequest } diff --git a/DB/Sources/DB/Content/StatusResult.swift b/DB/Sources/DB/Content/StatusResult.swift index 73b6d08..1a135b6 100644 --- a/DB/Sources/DB/Content/StatusResult.swift +++ b/DB/Sources/DB/Content/StatusResult.swift @@ -25,12 +25,12 @@ extension StatusResult { } extension QueryInterfaceRequest where RowDecoder == StatusRecord { - var statusResultRequest: AnyFetchRequest { - AnyFetchRequest(including(required: StatusRecord.account) - .including(optional: StatusRecord.accountMoved) - .including(optional: StatusRecord.reblogAccount) - .including(optional: StatusRecord.reblogAccountMoved) - .including(optional: StatusRecord.reblog)) + var statusResultRequest: QueryInterfaceRequest { + including(required: StatusRecord.account) + .including(optional: StatusRecord.accountMoved) + .including(optional: StatusRecord.reblogAccount) + .including(optional: StatusRecord.reblogAccountMoved) + .including(optional: StatusRecord.reblog) .asRequest(of: StatusResult.self) } } diff --git a/DB/Sources/DB/Extensions/Identity+Internal.swift b/DB/Sources/DB/Extensions/Identity+Internal.swift index de8a61d..a18f80d 100644 --- a/DB/Sources/DB/Extensions/Identity+Internal.swift +++ b/DB/Sources/DB/Extensions/Identity+Internal.swift @@ -14,7 +14,7 @@ extension Identity { instance: result.instance, account: result.account, lastRegisteredDeviceToken: result.identity.lastRegisteredDeviceToken, - pushSubscriptionAlerts: result.pushSubscriptionAlerts) + pushSubscriptionAlerts: result.identity.pushSubscriptionAlerts) } } diff --git a/DB/Sources/DB/Extensions/Timeline+Extensions.swift b/DB/Sources/DB/Extensions/Timeline+Extensions.swift index bacfa2a..dab38ca 100644 --- a/DB/Sources/DB/Extensions/Timeline+Extensions.swift +++ b/DB/Sources/DB/Extensions/Timeline+Extensions.swift @@ -41,7 +41,7 @@ extension Timeline { using: TimelineStatusJoin.status) .order(Column("createdAt").desc) - var statuses: AnyFetchRequest { + var statuses: QueryInterfaceRequest { request(for: Self.statuses).statusResultRequest } } diff --git a/DB/Sources/DB/Identity/IdentityDatabase.swift b/DB/Sources/DB/Identity/IdentityDatabase.swift index 986cdae..c805cb8 100644 --- a/DB/Sources/DB/Identity/IdentityDatabase.swift +++ b/DB/Sources/DB/Identity/IdentityDatabase.swift @@ -117,8 +117,8 @@ public extension IdentityDatabase { func updatePreferences(_ preferences: Identity.Preferences, forIdentityID identityID: UUID) -> AnyPublisher { databaseQueue.writePublisher(updates: Self.writePreferences(preferences, id: identityID)) - .ignoreOutput() - .eraseToAnyPublisher() + .ignoreOutput() + .eraseToAnyPublisher() } func updatePushSubscription(alerts: PushSubscription.Alerts, @@ -141,16 +141,14 @@ public extension IdentityDatabase { .eraseToAnyPublisher() } - func identityObservation(id: UUID) -> AnyPublisher { + func identityObservation(id: UUID, immediate: Bool) -> AnyPublisher { ValueObservation.tracking( IdentityRecord .filter(Column("id") == id) - .including(optional: IdentityRecord.instance) - .including(optional: IdentityRecord.account) - .asRequest(of: IdentityResult.self) + .identityResultRequest .fetchOne) .removeDuplicates() - .publisher(in: databaseQueue, scheduling: .immediate) + .publisher(in: databaseQueue, scheduling: immediate ? .immediate : .async(onQueue: .main)) .tryMap { guard let result = $0 else { throw IdentityDatabaseError.identityNotFound } @@ -160,7 +158,11 @@ public extension IdentityDatabase { } func identitiesObservation() -> AnyPublisher<[Identity], Error> { - ValueObservation.tracking(Self.identitiesRequest().fetchAll) + ValueObservation.tracking( + IdentityRecord + .order(Column("lastUsedAt").desc) + .identityResultRequest + .fetchAll) .removeDuplicates() .publisher(in: databaseQueue) .map { $0.map(Identity.init(result:)) } @@ -169,7 +171,9 @@ public extension IdentityDatabase { func recentIdentitiesObservation(excluding: UUID) -> AnyPublisher<[Identity], Error> { ValueObservation.tracking( - Self.identitiesRequest() + IdentityRecord + .order(Column("lastUsedAt").desc) + .identityResultRequest .filter(Column("id") != excluding) .limit(9) .fetchAll) @@ -179,7 +183,7 @@ public extension IdentityDatabase { .eraseToAnyPublisher() } - func mostRecentlyUsedIdentityIDObservation() -> AnyPublisher { + func immediateMostRecentlyUsedIdentityIDObservation() -> AnyPublisher { ValueObservation.tracking(IdentityRecord.select(Column("id")).order(Column("lastUsedAt").desc).fetchOne) .removeDuplicates() .publisher(in: databaseQueue, scheduling: .immediate) @@ -188,7 +192,9 @@ public extension IdentityDatabase { func identitiesWithOutdatedDeviceTokens(deviceToken: Data) -> AnyPublisher<[Identity], Error> { databaseQueue.readPublisher( - value: Self.identitiesRequest() + value: IdentityRecord + .order(Column("lastUsedAt").desc) + .identityResultRequest .filter(Column("lastRegisteredDeviceToken") != deviceToken) .fetchAll) .map { $0.map(Identity.init(result:)) } @@ -199,14 +205,6 @@ public extension IdentityDatabase { private extension IdentityDatabase { private static let name = "Identity" - private static func identitiesRequest() -> QueryInterfaceRequest { - IdentityRecord - .order(Column("lastUsedAt").desc) - .including(optional: IdentityRecord.instance) - .including(optional: IdentityRecord.account) - .asRequest(of: IdentityResult.self) - } - private static func writePreferences(_ preferences: Identity.Preferences, id: UUID) -> (Database) throws -> Void { { let data = try IdentityRecord.databaseJSONEncoder(for: "preferences").encode(preferences) diff --git a/DB/Sources/DB/Identity/IdentityResult.swift b/DB/Sources/DB/Identity/IdentityResult.swift index 398b6d4..f144fa3 100644 --- a/DB/Sources/DB/Identity/IdentityResult.swift +++ b/DB/Sources/DB/Identity/IdentityResult.swift @@ -8,5 +8,12 @@ struct IdentityResult: Codable, Hashable, FetchableRecord { let identity: IdentityRecord let instance: Identity.Instance? let account: Identity.Account? - let pushSubscriptionAlerts: PushSubscription.Alerts +} + +extension QueryInterfaceRequest where RowDecoder == IdentityRecord { + var identityResultRequest: QueryInterfaceRequest { + including(optional: IdentityRecord.instance) + .including(optional: IdentityRecord.account) + .asRequest(of: IdentityResult.self) + } } diff --git a/ServiceLayer/Sources/ServiceLayer/Services/AllIdentitiesService.swift b/ServiceLayer/Sources/ServiceLayer/Services/AllIdentitiesService.swift index 67b63d5..6dff9cf 100644 --- a/ServiceLayer/Sources/ServiceLayer/Services/AllIdentitiesService.swift +++ b/ServiceLayer/Sources/ServiceLayer/Services/AllIdentitiesService.swift @@ -8,8 +8,6 @@ import MastodonAPI import Secrets public struct AllIdentitiesService { - public let mostRecentlyUsedIdentityID: AnyPublisher - private let environment: AppEnvironment private let database: IdentityDatabase @@ -18,10 +16,6 @@ public struct AllIdentitiesService { self.database = try environment.fixtureDatabase ?? IdentityDatabase( inMemory: environment.inMemoryContent, keychain: environment.keychain) - - mostRecentlyUsedIdentityID = database.mostRecentlyUsedIdentityIDObservation() - .replaceError(with: nil) - .eraseToAnyPublisher() } } @@ -30,6 +24,10 @@ public extension AllIdentitiesService { try IdentityService(id: id, database: database, environment: environment) } + func immediateMostRecentlyUsedIdentityIDObservation() -> AnyPublisher { + database.immediateMostRecentlyUsedIdentityIDObservation() + } + func createIdentity(id: UUID, url: URL, authenticated: Bool) -> AnyPublisher { let secrets = Secrets(identityID: id, keychain: environment.keychain) diff --git a/ServiceLayer/Sources/ServiceLayer/Services/IdentityService.swift b/ServiceLayer/Sources/ServiceLayer/Services/IdentityService.swift index f905a9b..5e1fc38 100644 --- a/ServiceLayer/Sources/ServiceLayer/Services/IdentityService.swift +++ b/ServiceLayer/Sources/ServiceLayer/Services/IdentityService.swift @@ -15,7 +15,6 @@ public struct IdentityService { private let environment: AppEnvironment private let mastodonAPIClient: MastodonAPIClient private let secrets: Secrets - private let observationErrorsInput = PassthroughSubject() init(id: UUID, database: IdentityDatabase, environment: AppEnvironment) throws { identityID = id @@ -86,8 +85,8 @@ public extension IdentityService { .eraseToAnyPublisher() } - func observation() -> AnyPublisher { - identityDatabase.identityObservation(id: identityID) + func observation(immediate: Bool) -> AnyPublisher { + identityDatabase.identityObservation(id: identityID, immediate: immediate) } func listsObservation() -> AnyPublisher<[Timeline], Error> { diff --git a/ViewModels/Sources/ViewModels/Entities/Identification.swift b/ViewModels/Sources/ViewModels/Entities/Identification.swift index 454b4c6..72d8372 100644 --- a/ViewModels/Sources/ViewModels/Entities/Identification.swift +++ b/ViewModels/Sources/ViewModels/Entities/Identification.swift @@ -4,38 +4,16 @@ import Combine import Foundation import ServiceLayer -enum IdentificationError: Error { - case initialIdentityValueAbsent -} - public final class Identification: ObservableObject { @Published private(set) var identity: Identity let service: IdentityService - let observationErrors: AnyPublisher - - init(service: IdentityService) throws { - self.service = service - // The scheduling on the observation is immediate so an initial value can be extracted - let sharedObservation = service.observation().share() - var initialIdentity: Identity? - - _ = sharedObservation.first().sink( - receiveCompletion: { _ in }, - receiveValue: { initialIdentity = $0 }) - - guard let identity = initialIdentity else { throw IdentificationError.initialIdentityValueAbsent } + init(identity: Identity, observation: AnyPublisher, service: IdentityService) { self.identity = identity + self.service = service - let observationErrorsSubject = PassthroughSubject() - - observationErrors = observationErrorsSubject.eraseToAnyPublisher() - - sharedObservation.catch { error -> Empty in - observationErrorsSubject.send(error) - - return Empty() + DispatchQueue.main.async { + observation.dropFirst().assign(to: &self.$identity) } - .assign(to: &$identity) } } diff --git a/ViewModels/Sources/ViewModels/RootViewModel.swift b/ViewModels/Sources/ViewModels/RootViewModel.swift index afab8b6..8b90a86 100644 --- a/ViewModels/Sources/ViewModels/RootViewModel.swift +++ b/ViewModels/Sources/ViewModels/RootViewModel.swift @@ -5,7 +5,24 @@ import Foundation import ServiceLayer public final class RootViewModel: ObservableObject { - @Published public private(set) var identification: Identification? + @Published public private(set) var identification: Identification? { + didSet { + guard let identification = identification else { return } + + identification.service.updateLastUse() + .sink { _ in } receiveValue: { _ in } + .store(in: &cancellables) + + userNotificationService.isAuthorized() + .filter { $0 } + .zip(registerForRemoteNotifications()) + .filter { identification.identity.lastRegisteredDeviceToken != $1 } + .map { ($1, identification.identity.pushSubscriptionAlerts) } + .flatMap(identification.service.createPushSubscription(deviceToken:alerts:)) + .sink { _ in } receiveValue: { _ in } + .store(in: &cancellables) + } + } @Published private var mostRecentlyUsedIdentityID: UUID? private let environment: AppEnvironment @@ -21,9 +38,11 @@ public final class RootViewModel: ObservableObject { userNotificationService = UserNotificationService(environment: environment) self.registerForRemoteNotifications = registerForRemoteNotifications - allIdentitiesService.mostRecentlyUsedIdentityID.assign(to: &$mostRecentlyUsedIdentityID) + allIdentitiesService.immediateMostRecentlyUsedIdentityIDObservation() + .replaceError(with: nil) + .assign(to: &$mostRecentlyUsedIdentityID) - newIdentitySelected(id: mostRecentlyUsedIdentityID) + identitySelected(id: mostRecentlyUsedIdentityID, immediate: true) userNotificationService.isAuthorized() .filter { $0 } @@ -36,39 +55,8 @@ public final class RootViewModel: ObservableObject { } public extension RootViewModel { - func newIdentitySelected(id: UUID?) { - guard let id = id else { - identification = nil - - return - } - - let identification: Identification - - do { - identification = try Identification(service: allIdentitiesService.identityService(id: id)) - self.identification = identification - } catch { - return - } - - identification.observationErrors - .receive(on: RunLoop.main) - .sink { [weak self] _ in self?.newIdentitySelected(id: self?.mostRecentlyUsedIdentityID ) } - .store(in: &cancellables) - - identification.service.updateLastUse() - .sink { _ in } receiveValue: { _ in } - .store(in: &cancellables) - - userNotificationService.isAuthorized() - .filter { $0 } - .zip(registerForRemoteNotifications()) - .filter { identification.identity.lastRegisteredDeviceToken != $1 } - .map { ($1, identification.identity.pushSubscriptionAlerts) } - .flatMap(identification.service.createPushSubscription(deviceToken:alerts:)) - .sink { _ in } receiveValue: { _ in } - .store(in: &cancellables) + func identitySelected(id: UUID?) { + identitySelected(id: id, immediate: false) } func deleteIdentity(id: UUID) { @@ -83,3 +71,33 @@ public extension RootViewModel { instanceFilterService: InstanceFilterService(environment: environment)) } } + +private extension RootViewModel { + func identitySelected(id: UUID?, immediate: Bool) { + guard + let id = id, + let identityService = try? allIdentitiesService.identityService(id: id) else { + identification = nil + + return + } + + let observation = identityService.observation(immediate: immediate) + .catch { [weak self] _ -> Empty in + DispatchQueue.main.async { + self?.identitySelected(id: self?.mostRecentlyUsedIdentityID, immediate: false) + } + + return Empty() + } + .share() + + observation.map { + Identification( + identity: $0, + observation: observation.eraseToAnyPublisher(), + service: identityService) + } + .assign(to: &$identification) + } +} diff --git a/ViewModels/Tests/ViewModelsTests/AddIdentityViewModelTests.swift b/ViewModels/Tests/ViewModelsTests/AddIdentityViewModelTests.swift index 2f3db59..54993cb 100644 --- a/ViewModels/Tests/ViewModelsTests/AddIdentityViewModelTests.swift +++ b/ViewModels/Tests/ViewModelsTests/AddIdentityViewModelTests.swift @@ -12,7 +12,10 @@ import XCTest class AddIdentityViewModelTests: XCTestCase { func testAddIdentity() throws { - let sut = AddIdentityViewModel(allIdentitiesService: try AllIdentitiesService(environment: .mock())) + let environment = AppEnvironment.mock() + let sut = AddIdentityViewModel( + allIdentitiesService: try AllIdentitiesService(environment: environment), + instanceFilterService: InstanceFilterService(environment: environment)) let addedIDRecorder = sut.addedIdentityID.record() sut.urlFieldText = "https://mastodon.social" @@ -22,7 +25,10 @@ class AddIdentityViewModelTests: XCTestCase { } func testAddIdentityWithoutScheme() throws { - let sut = AddIdentityViewModel(allIdentitiesService: try AllIdentitiesService(environment: .mock())) + let environment = AppEnvironment.mock() + let sut = AddIdentityViewModel( + allIdentitiesService: try AllIdentitiesService(environment: environment), + instanceFilterService: InstanceFilterService(environment: environment)) let addedIDRecorder = sut.addedIdentityID.record() sut.urlFieldText = "mastodon.social" @@ -32,7 +38,10 @@ class AddIdentityViewModelTests: XCTestCase { } func testInvalidURL() throws { - let sut = AddIdentityViewModel(allIdentitiesService: try AllIdentitiesService(environment: .mock())) + let environment = AppEnvironment.mock() + let sut = AddIdentityViewModel( + allIdentitiesService: try AllIdentitiesService(environment: environment), + instanceFilterService: InstanceFilterService(environment: environment)) let recorder = sut.$alertItem.record() XCTAssertNil(try wait(for: recorder.next(), timeout: 1)) @@ -46,9 +55,10 @@ class AddIdentityViewModelTests: XCTestCase { } func testDoesNotAlertCanceledLogin() throws { - let allIdentitiesService = try AllIdentitiesService( - environment: .mock(webAuthSessionType: CanceledLoginMockWebAuthSession.self)) - let sut = AddIdentityViewModel(allIdentitiesService: allIdentitiesService) + let environment = AppEnvironment.mock(webAuthSessionType: CanceledLoginMockWebAuthSession.self) + let sut = AddIdentityViewModel( + allIdentitiesService: try AllIdentitiesService(environment: environment), + instanceFilterService: InstanceFilterService(environment: environment)) let recorder = sut.$alertItem.record() XCTAssertNil(try wait(for: recorder.next(), timeout: 1)) diff --git a/ViewModels/Tests/ViewModelsTests/RootViewModelTests.swift b/ViewModels/Tests/ViewModelsTests/RootViewModelTests.swift index cd1abac..e8ff3f1 100644 --- a/ViewModels/Tests/ViewModelsTests/RootViewModelTests.swift +++ b/ViewModels/Tests/ViewModelsTests/RootViewModelTests.swift @@ -21,7 +21,7 @@ class RootViewModelTests: XCTestCase { let addIdentityViewModel = sut.addIdentityViewModel() addIdentityViewModel.addedIdentityID - .sink(receiveValue: sut.newIdentitySelected(id:)) + .sink(receiveValue: sut.identitySelected(id:)) .store(in: &cancellables) addIdentityViewModel.urlFieldText = "https://mastodon.social" diff --git a/Views/AddIdentityView.swift b/Views/AddIdentityView.swift index 940b2eb..8b3a817 100644 --- a/Views/AddIdentityView.swift +++ b/Views/AddIdentityView.swift @@ -28,7 +28,7 @@ struct AddIdentityView: View { .alertItem($viewModel.alertItem) .onReceive(viewModel.addedIdentityID) { id in withAnimation { - rootViewModel.newIdentitySelected(id: id) + rootViewModel.identitySelected(id: id) } } .onAppear(perform: viewModel.refreshFilter) diff --git a/Views/IdentitiesView.swift b/Views/IdentitiesView.swift index 4d7b9c0..0dbb26e 100644 --- a/Views/IdentitiesView.swift +++ b/Views/IdentitiesView.swift @@ -41,7 +41,7 @@ private extension IdentitiesView { ForEach(identities) { identity in Button { withAnimation { - rootViewModel.newIdentitySelected(id: identity.id) + rootViewModel.identitySelected(id: identity.id) } } label: { row(identity: identity) diff --git a/Views/TabNavigationView.swift b/Views/TabNavigationView.swift index c0040f2..ca25d5e 100644 --- a/Views/TabNavigationView.swift +++ b/Views/TabNavigationView.swift @@ -88,7 +88,7 @@ private extension TabNavigationView { .contextMenu(ContextMenu { ForEach(viewModel.recentIdentities) { recentIdentity in Button { - rootViewModel.newIdentitySelected(id: recentIdentity.id) + rootViewModel.identitySelected(id: recentIdentity.id) } label: { Label( title: { Text(recentIdentity.handle) },