Skip to content

Commit

Permalink
#40: Add UserSessionStoreProtocol.
Browse files Browse the repository at this point in the history
Only log out of the specific account.
Add tests for the keychain controller.
Expand test coverage.
PR comments
  • Loading branch information
pixlwave committed Jun 16, 2022
1 parent b982122 commit 059c440
Show file tree
Hide file tree
Showing 17 changed files with 261 additions and 78 deletions.
86 changes: 51 additions & 35 deletions ElementX.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions ElementX/Sources/AppCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ class AppCoordinator: AuthenticationCoordinatorDelegate, Coordinator {

private let navigationRouter: NavigationRouter

private let userSessionStore: UserSessionStore
private let userSessionStore: UserSessionStoreProtocol

private var userSession: UserSession!
private var userSession: UserSessionProtocol!

private let memberDetailProviderManager: MemberDetailProviderManager

Expand Down Expand Up @@ -88,7 +88,7 @@ class AppCoordinator: AuthenticationCoordinatorDelegate, Coordinator {
stateMachine.processEvent(.attemptedSignIn)
}

func authenticationCoordinator(_ authenticationCoordinator: AuthenticationCoordinator, didLoginWithSession userSession: UserSession) {
func authenticationCoordinator(_ authenticationCoordinator: AuthenticationCoordinator, didLoginWithSession userSession: UserSessionProtocol) {
self.userSession = userSession
remove(childCoordinator: authenticationCoordinator)
stateMachine.processEvent(.succeededSigningIn)
Expand All @@ -100,14 +100,14 @@ class AppCoordinator: AuthenticationCoordinatorDelegate, Coordinator {

// MARK: - Private

// swiftlint:disable cyclomatic_complexity
// swiftlint:disable cyclomatic_complexity function_body_length
private func setupStateMachine() {
stateMachine.addTransitionHandler { [weak self] context in
guard let self = self else { return }

switch (context.fromState, context.event, context.toState) {
case (.initial, .startWithAuthentication, .signedOut):
self.showAuthentication()
self.startAuthentication()
case (.signedOut, .attemptedSignIn, .signingIn):
self.showLoadingIndicator()
case (.signingIn, .failedSigningIn, .signedOut):
Expand Down Expand Up @@ -151,7 +151,7 @@ class AppCoordinator: AuthenticationCoordinatorDelegate, Coordinator {
fatalError("Failed transition with context: \(context)")
}
}
// swiftlint:enable cyclomatic_complexity
// swiftlint:enable cyclomatic_complexity function_body_length

private func restoreUserSession() {
Task {
Expand All @@ -166,7 +166,7 @@ class AppCoordinator: AuthenticationCoordinatorDelegate, Coordinator {
}
}

private func showAuthentication() {
private func startAuthentication() {
let coordinator = AuthenticationCoordinator(userSessionStore: userSessionStore,
navigationRouter: navigationRouter)
coordinator.delegate = self
Expand All @@ -184,7 +184,7 @@ class AppCoordinator: AuthenticationCoordinatorDelegate, Coordinator {

mainNavigationController.setViewControllers([splashViewController], animated: false)

showAuthentication()
startAuthentication()
}

private func presentHomeScreen() {
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,23 @@ protocol AuthenticationCoordinatorDelegate: AnyObject {
func authenticationCoordinatorDidStartLoading(_ authenticationCoordinator: AuthenticationCoordinator)

func authenticationCoordinator(_ authenticationCoordinator: AuthenticationCoordinator,
didLoginWithSession userSession: UserSession)
didLoginWithSession userSession: UserSessionProtocol)

func authenticationCoordinator(_ authenticationCoordinator: AuthenticationCoordinator,
didFailWithError error: AuthenticationCoordinatorError)
}

class AuthenticationCoordinator: Coordinator {

private let userSessionStore: UserSessionStore
private let userSessionStore: UserSessionStoreProtocol
private let navigationRouter: NavigationRouter

private(set) var clientProxy: ClientProxyProtocol?
var childCoordinators: [Coordinator] = []

weak var delegate: AuthenticationCoordinatorDelegate?

init(userSessionStore: UserSessionStore,
init(userSessionStore: UserSessionStoreProtocol,
navigationRouter: NavigationRouter) {
self.userSessionStore = userSessionStore
self.navigationRouter = navigationRouter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@

import SwiftUI

/// Metrics used across the entire onboarding flow.
struct AuthenticationMetrics {
/// Standard constants used across the app's UI.
struct UIConstants {
static let maxContentWidth: CGFloat = 600
static let maxContentHeight: CGFloat = 750

/// The padding used between the top of the main content and the navigation bar.
static let topPaddingToNavigationBar: CGFloat = 16
/// The width/height used for the main icon shown in most of the screens.
static let iconSize: CGFloat = 90
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@

import SwiftUI

protocol SplashScreenCoordinatorProtocol: Coordinator, Presentable {
var callback: ((SplashScreenCoordinatorAction) -> Void)? { get set }
}

final class SplashScreenCoordinator: SplashScreenCoordinatorProtocol {
final class SplashScreenCoordinator: Coordinator, Presentable {

// MARK: - Properties

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ enum SplashScreenViewModelAction {

// MARK: View

struct SplashScreenViewState: BindableState, CustomDebugStringConvertible {
struct SplashScreenViewState: BindableState {
private enum Constants {
static let gradientColors = [
Color(red: 0.95, green: 0.98, blue: 0.96),
Expand All @@ -55,11 +55,6 @@ struct SplashScreenViewState: BindableState, CustomDebugStringConvertible {
let content: [SplashScreenPageContent]
var bindings: SplashScreenBindings

/// Custom debug description to reduce noise in the logs.
var debugDescription: String {
"SplashScreenViewState at page \(bindings.pageIndex)."
}

init() {
// The pun doesn't translate, so we only use it for English.
let locale = Locale.current
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ struct SplashScreen: View {
overlayHeight: overlayFrame.height + geometry.safeAreaInsets.bottom)
.frame(width: geometry.size.width)
.tag(-1)
.accessibilityIdentifier("hiddenPage")

ForEach(0..<pageCount, id: \.self) { index in
SplashScreenPage(content: viewModel.viewState.content[index],
Expand Down Expand Up @@ -90,7 +91,7 @@ struct SplashScreen: View {

buttons
.padding(.horizontal, 16)
.frame(maxWidth: AuthenticationMetrics.maxContentWidth)
.frame(maxWidth: UIConstants.maxContentWidth)
Spacer()
}
.background(ViewFrameReader(frame: $overlayFrame))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ struct SplashScreenPage: View {
Spacer().frame(maxHeight: overlayHeight)
}
.padding(.horizontal, 16)
.frame(maxWidth: AuthenticationMetrics.maxContentWidth,
maxHeight: AuthenticationMetrics.maxContentHeight)
.frame(maxWidth: UIConstants.maxContentWidth,
maxHeight: UIConstants.maxContentHeight)
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.background(backgroundGradient.ignoresSafeArea())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ class KeychainController: KeychainControllerProtocol {
do {
try keychain.set(accessToken, key: username)
} catch {
MXLog.error("Failed storing user restore token with error: \(error)")
MXLog.error("Failed storing user access token with error: \(error)")
}
}

func accessTokenForUsername(_ username: String) -> String? {
do {
return try keychain.get(username)
} catch {
MXLog.error("Failed retrieving user restore token")
MXLog.error("Failed retrieving user access token")
return nil
}
}
Expand All @@ -43,6 +43,14 @@ class KeychainController: KeychainControllerProtocol {
}
}

func removeAccessTokenForUsername(_ username: String) {
do {
try keychain.remove(username)
} catch {
MXLog.error("Failed removing access token with error: \(error)")
}
}

func removeAllAccessTokens() {
do {
try keychain.removeAll()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ protocol KeychainControllerProtocol {
func setAccessToken(_ accessToken: String, forUsername username: String)
func accessTokenForUsername(_ username: String) -> String?
func accessTokens() -> [(username: String, accessToken: String)]
func removeAccessTokenForUsername(_ username: String)
func removeAllAccessTokens()
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,7 @@ import Foundation
import MatrixRustSDK
import Kingfisher

enum UserSessionStoreError: Error {
case missingCredentials
case failedRestoringLogin
case failedSettingUpSession
}

@MainActor
class UserSessionStore {
class UserSessionStore: UserSessionStoreProtocol {

private let keychainController: KeychainControllerProtocol

Expand Down Expand Up @@ -70,8 +63,9 @@ class UserSessionStore {
}

func logout(userSession: UserSessionProtocol) {
keychainController.removeAllAccessTokens()
deleteBaseDirectory(for: userSession.clientProxy.userIdentifier)
let username = userSession.clientProxy.userIdentifier
keychainController.removeAccessTokenForUsername(username)
deleteBaseDirectory(for: username)
}

private func restorePreviousLogin(_ usernameTokenTuple: (username: String, accessToken: String)) async -> Result<ClientProxyProtocol, UserSessionStoreError> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//
// Copyright 2022 New Vector Ltd
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import MatrixRustSDK

enum UserSessionStoreError: Error {
case missingCredentials
case failedRestoringLogin
case failedSettingUpSession
}

@MainActor
protocol UserSessionStoreProtocol {
/// Whether or not there are sessions in the store.
var hasSessions: Bool { get }

/// Restores an existing user session.
func restoreUserSession() async -> Result<UserSession, UserSessionStoreError>

/// Creates a user session for a new client from the SDK.
func userSession(for client: Client) async -> Result<UserSession, UserSessionStoreError>

/// Logs out of the specified session.
func logout(userSession: UserSessionProtocol)

/// Returns the location to store user data for a particular username.
func baseDirectoryPath(for username: String) -> String
}
4 changes: 3 additions & 1 deletion ElementX/Sources/UITestsAppCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ class UITestsAppCoordinator: Coordinator {
window.tintColor = .element.accent

let screens = mockScreens()
screens.forEach { $0.coordinator.start() }

let rootView = UITestsRootView(mockScreens: screens) { id in
guard let screen = screens.first(where: { $0.id == id }) else {
fatalError()
Expand Down Expand Up @@ -52,5 +54,5 @@ class UITestsAppCoordinator: Coordinator {

struct MockScreen: Identifiable {
let id: String
let coordinator: Presentable
let coordinator: Coordinator & Presentable
}
43 changes: 42 additions & 1 deletion UITests/Sources/SplashScreenUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,52 @@

import XCTest

@MainActor
class SplashScreenUITests: XCTestCase {
func testInitialStateComponents() {
let app = Application.launch()
app.goToScreenWithIdentifier("Splash Screen")

XCTAssert(app.buttons["Get started"].exists)
let getStartedButton = app.buttons["Get started"]
XCTAssertTrue(getStartedButton.exists, "The primary action button should be shown.")
}

func testSwipingBetweenPages() async throws {
let app = Application.launch()
app.goToScreenWithIdentifier("Splash Screen")

// Given the splash screen in its initial state.
let page1TitleText = app.staticTexts["Own your conversations."]
let page2TitleText = app.staticTexts["You're in control."]
let hiddenPageTitleText = app.staticTexts["hiddenPage"].firstMatch

XCTAssertTrue(page1TitleText.isHittable, "The title from the first page of the carousel should be onscreen.")
XCTAssertFalse(page2TitleText.isHittable, "The title from the second page of the carousel should be offscreen.")
XCTAssertFalse(hiddenPageTitleText.isHittable, "The hidden page of the carousel should be offscreen.")

// When swiping to the next screen.
page1TitleText.swipeLeft()
try await Task.sleep(nanoseconds: 200_000_000) // Wait for the animation.

// Then the second screen should be shown.
XCTAssertFalse(page1TitleText.isHittable, "The title from the first page of the carousel should be offscreen.")
XCTAssertTrue(page2TitleText.isHittable, "The title from the second page of the carousel should be onscreen.")

// When swiping back to the previous screen.
page2TitleText.swipeRight()
try await Task.sleep(nanoseconds: 200_000_000) // Wait for the animation.

// Then the first screen should be shown again.
XCTAssertTrue(page1TitleText.isHittable, "The title from the first page of the carousel should be onscreen.")
XCTAssertFalse(page2TitleText.isHittable, "The title from the second page of the carousel should be offscreen.")

// When swiping back to the previous screen.
page1TitleText.swipeRight()
try await Task.sleep(nanoseconds: 200_000_000) // Wait for the animation.

// Then the screen shouldn't change and the hidden screen should be ignored.
XCTAssertTrue(page1TitleText.isHittable, "The title from the first page of the carousel should be still be onscreen.")
XCTAssertFalse(page2TitleText.isHittable, "The title from the second page of the carousel should be offscreen.")
XCTAssertFalse(hiddenPageTitleText.isHittable, "It shouldn't be possible to swipe to the hidden page of the carousel.")
}
}
Loading

0 comments on commit 059c440

Please sign in to comment.