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.

Sign up for GitHub

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

Jump to bottom

Persist visit options if a response is present #155

Merged
merged 5 commits into from
Mar 22, 2024
Merged

Persist visit options if a response is present #155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ded644e
Persist visit options if a response is present
olivaresf Nov 6, 2023
da5fcec
Merge remote-tracking branch 'origin/main' into preserve-visit-option…
olivaresf Feb 12, 2024
7647a48
Merge remote-tracking branch 'origin/main' into preserve-visit-option…
olivaresf Mar 1, 2024
6517e74
Add unit tests
olivaresf Mar 4, 2024
10e5c90
Clarify comments
olivaresf Mar 4, 2024
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
7 changes: 6 additions & 1 deletion Source/Session/Session.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ public class Session: NSObject {
private lazy var bridge = WebViewBridge(webView: webView)
private var initialized = false
private var refreshing = false

/// Options behave differently if a response is provided.
private let visitOptionsHandler = VisitOptionsHandler()

private var isShowingStaleContent = false
private var isSnapshotCacheStale = false

Expand Down Expand Up @@ -63,7 +67,8 @@ public class Session: NSObject {
initialized = false
}

let visit = makeVisit(for: visitable, options: options ?? VisitOptions())
let processedOptions = visitOptionsHandler.process(options)
let visit = makeVisit(for: visitable, options: processedOptions)
currentVisit?.cancel()
currentVisit = visit

Expand Down
24 changes: 24 additions & 0 deletions Source/Session/VisitOptionsHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import Foundation

class VisitOptionsHandler {

private var unhandledVisitOptions: VisitOptions?

/// If a form submission provides a response HTML, save the options and pass them to the next visit proposal.
func process(_ options: VisitOptions?) -> VisitOptions {

if let options, options.response?.responseHTML != nil {
/// A `responseHTML` is provided for the next visit.
unhandledVisitOptions = options
return options
} else if let unhandledVisitOptions {
/// Next visit is happening. Use the previous `responseHTML`.
self.unhandledVisitOptions = nil
Copy link
Member Author

Choose a reason for hiding this comment

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

Make sure URLs match between visit options

Copy link
Member

Choose a reason for hiding this comment

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

Has this been resolved?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not exactly. Would we keep the visit options around indefinitely until the next visit or should the next visit use them? If the former, URL matching makes sense. Otherwise, this implementation works for our purposes.

What do you think? Sorry to say I'm still trying to understand when to expect a responseHTML and I also don't think Android is doing it 🤔.

return unhandledVisitOptions
} else {
/// No options are unhandled.
return options ?? VisitOptions()
}
}

}
40 changes: 38 additions & 2 deletions Tests/VisitOptionsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,51 @@ class VisitOptionsTests: XCTestCase {
}

func test_Decodable_canBeInitializedWithResponse() throws {
_ = try validVisitVisitOptions(responseHTMLString: "<html></html>")
}

func test_visitOptionsArePreserved() throws {
let visitOptionsWithResponse = try validVisitVisitOptions(responseHTMLString: "<html></html>")
let handler = VisitOptionsHandler()

let processedOptions = handler.process(visitOptionsWithResponse)
XCTAssert(processedOptions == visitOptionsWithResponse)

let nextVisitOptions = try validVisitVisitOptions(responseHTMLString: nil)
let savedOptions = handler.process(nextVisitOptions)
XCTAssert(savedOptions == visitOptionsWithResponse)
}
}

extension VisitOptionsTests {
func validVisitVisitOptions(responseHTMLString: String?) throws -> VisitOptions {
var responseJSON = ""
if let responseHTMLString {
responseJSON = ", \"responseHTML\": \"\(responseHTMLString)\""
}

let json = """
{"response": {"statusCode": 200, "responseHTML": "<html></html>"}}
{"response": {"statusCode": 200\(responseJSON)}}
""".data(using: .utf8)!

let options = try JSONDecoder().decode(VisitOptions.self, from: json)
XCTAssertEqual(options.action, .advance)

let response = try XCTUnwrap(options.response)
XCTAssertEqual(response.statusCode, 200)
XCTAssertEqual(response.responseHTML, "<html></html>")
XCTAssertEqual(response.responseHTML, responseHTMLString)
return options
}
}

extension VisitOptions : Equatable {
public static func == (lhs: VisitOptions, rhs: VisitOptions) -> Bool {
lhs.action == rhs.action && lhs.response == rhs.response
}
}

extension VisitResponse : Equatable {
public static func == (lhs: VisitResponse, rhs: VisitResponse) -> Bool {
lhs.responseHTML == rhs.responseHTML && lhs.statusCode == rhs.statusCode
}
}