Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include default server route rules by default #76

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,8 @@ public extension PathProperties {
var animated: Bool {
self["animated"] as? Bool ?? true
}

internal var historicalLocation: Bool {
self["historical_location"] as? Bool ?? false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,8 @@ public extension VisitProposal {
var animated: Bool {
properties.animated
}

internal var isHistoricalLocation: Bool {
properties.historicalLocation
}
}
25 changes: 22 additions & 3 deletions Source/Turbo/Navigator/NavigationHierarchyController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class NavigationHierarchyController {
visitable.visitableView.allowsPullToRefresh = proposal.pullToRefreshEnabled
}

dismissModalIfNeeded(for: proposal)

switch proposal.presentation {
case .default:
navigate(with: controller, via: proposal)
Expand Down Expand Up @@ -175,6 +177,11 @@ class NavigationHierarchyController {
}

private func refresh(via proposal: VisitProposal) {
if proposal.isHistoricalLocation {
refreshIfTopViewControllerIsVisitable(from: .main)
return
}

if navigationController.presentedViewController != nil {
if modalNavigationController.viewControllers.count == 1 {
navigationController.dismiss(animated: proposal.animated)
Expand All @@ -183,10 +190,11 @@ class NavigationHierarchyController {
modalNavigationController.popViewController(animated: proposal.animated)
refreshIfTopViewControllerIsVisitable(from: .modal)
}
} else {
navigationController.popViewController(animated: proposal.animated)
refreshIfTopViewControllerIsVisitable(from: .main)
return
}

navigationController.popViewController(animated: proposal.animated)
refreshIfTopViewControllerIsVisitable(from: .main)
}

private func replaceRoot(with controller: UIViewController, via proposal: VisitProposal) {
Expand All @@ -204,4 +212,15 @@ class NavigationHierarchyController {
newTopmostVisitable: navControllerTopmostVisitable)
}
}

private func dismissModalIfNeeded(for visit: VisitProposal) {
// The desired behaviour for historical location visits is
// to always dismiss the "modal" stack.
guard visit.isHistoricalLocation,
navigationController.presentedViewController != nil else {
return
}

navigationController.dismiss(animated: visit.animated)
joemasilotti marked this conversation as resolved.
Show resolved Hide resolved
}
}
5 changes: 4 additions & 1 deletion Source/Turbo/Path Configuration/PathConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public final class PathConfiguration {
public private(set) var settings: [String: AnyHashable] = [:]

/// The list of rules from the configuration: `{ rules: [] }`
public private(set) var rules: [PathRule] = []
/// Default server route rules are included by default.
public private(set) var rules: [PathRule] = PathRule.defaultServerRoutes

/// Sources for this configuration, setting it will
/// cause the configuration to be loaded from the new sources
Expand Down Expand Up @@ -93,6 +94,8 @@ public final class PathConfiguration {
// Update our internal state with the config from the loader
settings = config.settings
rules = config.rules
// Always include the default server route rules.
rules.append(contentsOf: PathRule.defaultServerRoutes)
delegate?.pathConfigurationDidUpdate()
}
}
Expand Down
36 changes: 36 additions & 0 deletions Source/Turbo/Path Configuration/PathRule+ServerRoutes.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import Foundation

extension PathRule {
static let defaultServerRoutes: [PathRule] = [
.recedeHistoricalLocation,
.resumeHistoricalLocation,
.refreshHistoricalLocation
]

static let recedeHistoricalLocation = PathRule(
patterns: ["/recede_historical_location"],
properties: [
"presentation": "pop",
"visitable": false,
svara marked this conversation as resolved.
Show resolved Hide resolved
"historical_location": true
svara marked this conversation as resolved.
Show resolved Hide resolved
]
)

static let resumeHistoricalLocation = PathRule(
patterns: ["/resume_historical_location"],
properties: [
"presentation": "none",
"visitable": false,
"historical_location": true
]
)

static let refreshHistoricalLocation = PathRule(
patterns: ["/refresh_historical_location"],
properties: [
"presentation": "refresh",
"visitable": false,
"historical_location": true
]
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
@testable import HotwireNative
import XCTest

@MainActor
final class NavigationHierarchyControllerHistoricalLocationTests: XCTestCase {
override func setUp() {
navigationController = TestableNavigationController()
modalNavigationController = TestableNavigationController()

navigator = Navigator(session: session, modalSession: modalSession)
hierarchyController = NavigationHierarchyController(
delegate: navigator,
navigationController: navigationController,
modalNavigationController: modalNavigationController
)
navigator.hierarchyController = hierarchyController

loadNavigationControllerInWindow()
}

// Resume behaviour:
// 1. Dismiss the modal view controller.
// 2. Arrive back at the view controller on the "default" stack.
func test_resumeHistoricalLocation() async throws {
let defaultOne = VisitProposal(path: "/default_one", context: .default)
navigator.route(defaultOne)
try await delay()

let defaultTwo = VisitProposal(path: "/default_two", context: .default)
navigator.route(defaultTwo)
try await delay()

XCTAssertEqual(navigationController.viewControllers.count, 2)

let modalOne = VisitProposal(path: "/modal_one", context: .modal)
navigator.route(modalOne)

try await delay()
XCTAssertEqual(modalNavigationController.viewControllers.count, 1)

// Reset spy's properties.
session.visitWasCalled = false
session.visitAction = nil

let resumeHistoricalLocationProposal = VisitProposal(
path: PathRule.resumeHistoricalLocation.patterns.first!,
additionalProperties: PathRule.resumeHistoricalLocation.properties
)
navigator.route(resumeHistoricalLocationProposal)
try await delay()

XCTAssertNil(navigationController.presentedViewController)
XCTAssertEqual(navigationController.viewControllers.count, 2)
XCTAssertEqual(navigator.session.activeVisitable?.visitableURL, defaultTwo.url)
XCTAssertFalse(session.visitWasCalled)
}

// Refresh behaviour:
// 1. Dismiss the modal view controller.
// 2. Arrive back at the view controller on the "default" stack.
// 3. Refresh the view controller on the "default" stack by revisiting the location.
func test_refreshHistoricalLocation() async throws {
let defaultOne = VisitProposal(path: "/default_one", context: .default)
navigator.route(defaultOne)
try await delay()

XCTAssertEqual(navigationController.viewControllers.count, 1)

let modalOne = VisitProposal(path: "/modal_one", context: .modal)
navigator.route(modalOne)
try await delay()

XCTAssertEqual(modalNavigationController.viewControllers.count, 1)

// Reset spy's properties.
session.visitWasCalled = false
session.visitAction = nil

let refreshHistoricalLocationProposal = VisitProposal(
path: PathRule.refreshHistoricalLocation.patterns.first!,
additionalProperties: PathRule.refreshHistoricalLocation.properties
)
navigator.route(refreshHistoricalLocationProposal)
try await delay()

XCTAssertNil(navigationController.presentedViewController)
XCTAssertEqual(navigationController.viewControllers.count, 1)
XCTAssertEqual(navigator.session.activeVisitable?.visitableURL, defaultOne.url)
XCTAssertTrue(session.visitWasCalled)
XCTAssertEqual(session.visitAction, .restore)
}

// Recede behaviour:
// 1. Dismiss the modal view controller.
// 2. Arrive back at the view controller on the "default" stack.
// 3. Pop the view controller on the "default" stack (unless it's already at the beginning of the backstack).
// 4. This will trigger a refresh on the appeared view controller if the snapshot cache has been cleared by a form submission.
@MainActor
func test_recedeHistoricalLocation() async throws {
let defaultOne = VisitProposal(path: "/default_one", context: .default)
navigator.route(defaultOne)
try await delay()

let defaultTwo = VisitProposal(path: "/default_two", context: .default)
navigator.route(defaultTwo)
try await delay()

XCTAssertEqual(navigationController.viewControllers.count, 2)

let modalOne = VisitProposal(path: "/modal_one", context: .modal)
navigator.route(modalOne)
try await delay()

XCTAssertEqual(modalNavigationController.viewControllers.count, 1)

let recedeHistoricalLocationProposal = VisitProposal(
path: PathRule.recedeHistoricalLocation.patterns.first!,
additionalProperties: PathRule.recedeHistoricalLocation.properties
)
navigator.route(recedeHistoricalLocationProposal)
try await delay()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The delays are far from ideal, but they are necessary to ensure the view lifecycle methods execute properly and the tests pass. I’ll explore architectural improvements in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised these are needed even with the TestableNavigationController presenting everything without animation. Does pushing, popping, or dismissing require the delay?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the delay the recede historical location test fails. Even though the presentations are not animated, I doubt the view lifecycle methods are all called synchronously. I'll investigate.


XCTAssertNil(navigationController.presentedViewController)
XCTAssertEqual(navigationController.viewControllers.count, 1)
XCTAssertEqual(navigator.session.activeVisitable?.visitableURL, defaultOne.url)
}

func delay() async throws {
try await Task.sleep(nanoseconds: 1_000_000_000) // 1 second
}

private let session = SessionSpy(webView: Hotwire.config.makeWebView())
private let modalSession = Session(webView: Hotwire.config.makeWebView())

private var navigator: Navigator!
private var hierarchyController: NavigationHierarchyController!
private var navigationController: TestableNavigationController!
private var modalNavigationController: TestableNavigationController!

private let window = UIWindow()

// Simulate a "real" app so presenting view controllers works under test.
private func loadNavigationControllerInWindow() {
window.rootViewController = navigationController
window.makeKeyAndVisible()
navigationController.loadViewIfNeeded()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ private class EmptyNavigationDelegate: NavigationHierarchyControllerDelegate {

// MARK: - VisitProposal extension

private extension VisitProposal {
extension VisitProposal {
init(path: String = "",
action: VisitAction = .advance,
context: Navigation.Context = .default,
Expand All @@ -441,7 +441,7 @@ private extension VisitProposal {
"context": context.rawValue,
"presentation": presentation.rawValue
]
let properties = defaultProperties.merging(additionalProperties) { (current, _) in current }
let properties = defaultProperties.merging(additionalProperties) { (_, new) in new }

self.init(url: url, options: options, properties: properties)
}
Expand Down
3 changes: 2 additions & 1 deletion Tests/Turbo/Navigator/TestableNavigationController.swift
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import UIKit
@testable import HotwireNative

/// Manipulate a navigation controller under test.
/// Ensures `viewControllers` is updated synchronously.
/// Manages `presentedViewController` directly because it isn't updated on the same thread.
class TestableNavigationController: UINavigationController {
class TestableNavigationController: HotwireNavigationController {
override var presentedViewController: UIViewController? {
get { _presentedViewController }
set { _presentedViewController = newValue }
Expand Down
37 changes: 34 additions & 3 deletions Tests/Turbo/PathConfigurationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,50 @@ class PathConfigurationTests: XCTestCase {
var configuration: PathConfiguration!

override func setUp() {
configuration = PathConfiguration(sources: [.file(fileURL)])
XCTAssertGreaterThan(configuration.rules.count, 0)
configuration = PathConfiguration()
}

func test_initWithNoSourcesSetsDefaultRules() {
// Default three historical location rules are always added by default.
XCTAssertEqual(configuration.rules.count, PathRule.defaultServerRoutes.count)

for rule in PathRule.defaultServerRoutes {
XCTAssertNotNil(configuration.properties(for: rule.patterns.first!))
}
}

func test_init_automaticallyLoadsTheConfigurationFromTheSpecifiedLocation() {
loadConfigurationFromFile()

XCTAssertEqual(configuration.settings.count, 2)
XCTAssertEqual(configuration.rules.count, 5)
// Default three historical location rules are always added by default.
XCTAssertEqual(configuration.rules.count, 5 + PathRule.defaultServerRoutes.count)

for rule in PathRule.defaultServerRoutes {
XCTAssertNotNil(configuration.properties(for: rule.patterns.first!))
}
}

func test_settings_returnsCurrentSettings() {
loadConfigurationFromFile()

XCTAssertEqual(configuration.settings, [
"some-feature-enabled": true,
"server": "beta"
])
}

func test_propertiesForPath_whenPathMatches_returnsProperties() {
loadConfigurationFromFile()

XCTAssertEqual(configuration.properties(for: "/"), [
"page": "root"
])
}

func test_propertiesForPath_whenPathMatchesMultipleRules_mergesProperties() {
loadConfigurationFromFile()

XCTAssertEqual(configuration.properties(for: "/new"), [
"context": "modal",
"background_color": "black"
Expand All @@ -41,6 +62,8 @@ class PathConfigurationTests: XCTestCase {
}

func test_propertiesForURL_withParams() {
loadConfigurationFromFile()

let url = URL(string: "http://turbo.test/sample.pdf?open_in_external_browser=true")!

Hotwire.config.pathConfiguration.matchQueryStrings = false
Expand All @@ -53,16 +76,24 @@ class PathConfigurationTests: XCTestCase {
}

func test_propertiesForPath_whenNoMatch_returnsEmptyProperties() {
loadConfigurationFromFile()

XCTAssertEqual(configuration.properties(for: "/missing"), [:])
}

func test_subscript_isAConvenienceMethodForPropertiesForPath() {
loadConfigurationFromFile()

XCTAssertEqual(configuration.properties(for: "/new"), configuration["/new"])
XCTAssertEqual(configuration.properties(for: "/edit"), configuration["/edit"])
XCTAssertEqual(configuration.properties(for: "/"), configuration["/"])
XCTAssertEqual(configuration.properties(for: "/missing"), configuration["/missing"])
XCTAssertEqual(configuration.properties(for: "/sample.pdf?open_in_external_browser=true"), configuration["/sample.pdf?open_in_external_browser=true"])
}

func loadConfigurationFromFile() {
configuration.sources = [.file(fileURL)]
}
}

class PathConfigTests: XCTestCase {
Expand Down
Loading