From 8af3967745bd808d711e748b89ae46ac08ccabfc Mon Sep 17 00:00:00 2001 From: Fernando Olivares Date: Thu, 16 Nov 2023 21:39:23 -0600 Subject: [PATCH 01/25] Use a private encoder to compare messages, if needed --- Source/Message.swift | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/Source/Message.swift b/Source/Message.swift index 104eaf1..db36234 100644 --- a/Source/Message.swift +++ b/Source/Message.swift @@ -31,6 +31,9 @@ public struct Message: Equatable { self.metadata = metadata self.jsonData = jsonData } + + /// Used to compare `jsonData` Strings. + private static let equalityJSONEncoder = JSONEncoder() } extension Message { @@ -94,3 +97,37 @@ extension Message { } } } + +extension Message { + + /// Using `Equatable`'s default implementation is bound to give us false positives since two `Message`s may have semantically equal, but textually different, `jsonData`. + /// + /// For example, the following `jsonData` should be considered equal. + /// + /// ``` + /// lhs.jsonData = "{\"title\":\"Page-title\",\"subtitle\":\"Page-subtitle\",\"action_name\":\"go\"}")" + /// + /// rhs.jsonData = "{\"action_name\":\"go\",\"title\":\"Page-title\",\"subtitle\":\"Page-subtitle\"}")" + /// ``` + /// + /// - Parameters: + /// - lhs: a message + /// - rhs: another message + /// - Returns: true if they're semantically equal + public static func == (lhs: Self, rhs: Self) -> Bool { + + if lhs.jsonData != rhs.jsonData { + guard let lhsJSONData = lhs.jsonData.data(using: .utf8), + let rhsJSONData = rhs.jsonData.data(using: .utf8), + let lhsJSONObject = try? JSONSerialization.jsonObject(with: lhsJSONData, options: []) as? NSObject, + let rhsJSONObject = try? JSONSerialization.jsonObject(with: rhsJSONData, options: []) as? NSObject, + lhsJSONObject == rhsJSONObject + else { return false } + } + + return lhs.id == rhs.id && + lhs.component == rhs.component && + lhs.event == rhs.event && + lhs.metadata == rhs.metadata + } +} From e3c4ae88740ead8584c3350b946828737e5768c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Thu, 7 Dec 2023 18:37:04 +0100 Subject: [PATCH 02/25] Remove not necessary encoder and simplify equality function. --- Source/Message.swift | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/Source/Message.swift b/Source/Message.swift index db36234..9e14e76 100644 --- a/Source/Message.swift +++ b/Source/Message.swift @@ -31,9 +31,6 @@ public struct Message: Equatable { self.metadata = metadata self.jsonData = jsonData } - - /// Used to compare `jsonData` Strings. - private static let equalityJSONEncoder = JSONEncoder() } extension Message { @@ -99,8 +96,8 @@ extension Message { } extension Message { - - /// Using `Equatable`'s default implementation is bound to give us false positives since two `Message`s may have semantically equal, but textually different, `jsonData`. + /// Using `Equatable`'s default implementation is bound to give us false positives + /// since two `Message`s may have semantically equal, but textually different, `jsonData`. /// /// For example, the following `jsonData` should be considered equal. /// @@ -115,19 +112,10 @@ extension Message { /// - rhs: another message /// - Returns: true if they're semantically equal public static func == (lhs: Self, rhs: Self) -> Bool { - - if lhs.jsonData != rhs.jsonData { - guard let lhsJSONData = lhs.jsonData.data(using: .utf8), - let rhsJSONData = rhs.jsonData.data(using: .utf8), - let lhsJSONObject = try? JSONSerialization.jsonObject(with: lhsJSONData, options: []) as? NSObject, - let rhsJSONObject = try? JSONSerialization.jsonObject(with: rhsJSONData, options: []) as? NSObject, - lhsJSONObject == rhsJSONObject - else { return false } - } - return lhs.id == rhs.id && lhs.component == rhs.component && lhs.event == rhs.event && - lhs.metadata == rhs.metadata + lhs.metadata == rhs.metadata && + lhs.jsonData.jsonObject() as? [String: AnyHashable] == rhs.jsonData.jsonObject() as? [String: AnyHashable] } } From bdafa3f921b4d3a4eac4660c0fca8d4bd1be39f9 Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Thu, 7 Dec 2023 11:33:57 -0800 Subject: [PATCH 03/25] Convert all JavaScript calls to async functions --- Source/Bridge.swift | 80 +++++++++++-------- Source/BridgeComponent.swift | 16 ++-- Source/BridgeDelegate.swift | 14 ++-- Source/ScriptMessageHandler.swift | 4 +- Tests/BridgeComponentTests.swift | 36 ++++----- Tests/BridgeDelegateTests.swift | 16 ++-- Tests/BridgeTests.swift | 55 +++++-------- .../ComposerComponent.swift | 4 +- .../ComposerComponentTests.swift | 18 ++--- 9 files changed, 121 insertions(+), 122 deletions(-) diff --git a/Source/Bridge.swift b/Source/Bridge.swift index 3d1d5ad..09f6ca0 100644 --- a/Source/Bridge.swift +++ b/Source/Bridge.swift @@ -9,17 +9,17 @@ protocol Bridgable: AnyObject { var delegate: BridgeDelegate? { get set } var webView: WKWebView? { get } - func register(component: String) - func register(components: [String]) - func unregister(component: String) - func reply(with message: Message) + func register(component: String) async + func register(components: [String]) async + func unregister(component: String) async + func reply(with message: Message) async } /// `Bridge` is the object for configuring a web view and /// the channel for sending/receiving messages public final class Bridge: Bridgable { typealias CompletionHandler = (_ result: Any?, _ error: Error?) -> Void - + weak var delegate: BridgeDelegate? weak var webView: WKWebView? @@ -42,28 +42,28 @@ public final class Bridge: Bridgable { /// Register a single component /// - Parameter component: Name of a component to register support for - func register(component: String) { - callBridgeFunction(.register, arguments: [component]) + func register(component: String) async { + await callBridgeFunction(.register, arguments: [component]) } /// Register multiple components /// - Parameter components: Array of component names to register - func register(components: [String]) { - callBridgeFunction(.register, arguments: [components]) + func register(components: [String]) async { + await callBridgeFunction(.register, arguments: [components]) } /// Unregister support for a single component /// - Parameter component: Component name - func unregister(component: String) { - callBridgeFunction(.unregister, arguments: [component]) + func unregister(component: String) async { + await callBridgeFunction(.unregister, arguments: [component]) } /// Send a message through the bridge to the web application /// - Parameter message: Message to send - func reply(with message: Message) { + func reply(with message: Message) async { logger.debug("bridgeWillReplyWithMessage: \(String(describing: message))") let internalMessage = InternalMessage(from: message) - callBridgeFunction(.replyWith, arguments: [internalMessage.toJSON()]) + await callBridgeFunction(.replyWith, arguments: [internalMessage.toJSON()]) } // /// Convenience method to reply to a previously received message. Data will be replaced, @@ -75,27 +75,29 @@ public final class Bridge: Bridgable { // callBridgeFunction("send", arguments: [replyMessage.toJSON()]) // } - /// Evaluates javaScript string directly as passed in sending through the web view - func evaluate(javaScript: String, completion: CompletionHandler? = nil) { + @discardableResult + func evaluate(javaScript: String) async -> JavaScriptResult { guard let webView = webView else { - completion?(nil, BridgeError.missingWebView) - return + return JavaScriptResult(result: nil, error: BridgeError.missingWebView) } - - webView.evaluateJavaScript(javaScript) { result, error in - if let error = error { - logger.error("Error evaluating JavaScript: \(error)") - } - - completion?(result, error) + + var result: Any? = nil + var error: Error? = nil + do { + result = try await webView.evaluateJavaScript(javaScript) + } catch let e { + logger.error("Error evaluating JavaScript: \(e)") + error = e } + + return JavaScriptResult(result: result, error: error) } - + /// Evaluates a JavaScript function with optional arguments by encoding the arguments /// Function should not include the parens /// Usage: evaluate(function: "console.log", arguments: ["test"]) - func evaluate(function: String, arguments: [Any] = [], completion: CompletionHandler? = nil) { - evaluate(javaScript: JavaScript(functionName: function, arguments: arguments), completion: completion) + func evaluate(function: String, arguments: [Any] = []) async -> JavaScriptResult { + await evaluate(javaScript: JavaScript(functionName: function, arguments: arguments)) } static func initialize(_ bridge: Bridge) { @@ -116,9 +118,9 @@ public final class Bridge: Bridgable { /// The webkit.messageHandlers name private let scriptHandlerName = "strada" - private func callBridgeFunction(_ function: JavaScriptBridgeFunction, arguments: [Any]) { + private func callBridgeFunction(_ function: JavaScriptBridgeFunction, arguments: [Any]) async { let js = JavaScript(object: bridgeGlobal, functionName: function.rawValue, arguments: arguments) - evaluate(javaScript: js) + await evaluate(javaScript: js) } // MARK: - Configuration @@ -153,15 +155,16 @@ public final class Bridge: Bridgable { // MARK: - JavaScript Evaluation - private func evaluate(javaScript: JavaScript, completion: CompletionHandler? = nil) { + @discardableResult + private func evaluate(javaScript: JavaScript) async -> JavaScriptResult { do { - evaluate(javaScript: try javaScript.toString(), completion: completion) + return await evaluate(javaScript: try javaScript.toString()) } catch { logger.error("Error evaluating JavaScript: \(String(describing: javaScript)), error: \(error)") - completion?(nil, error) + return JavaScriptResult(result: nil, error: error) } } - + private enum JavaScriptBridgeFunction: String { case register case unregister @@ -170,10 +173,10 @@ public final class Bridge: Bridgable { } extension Bridge: ScriptMessageHandlerDelegate { - func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) { + func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async { if let event = scriptMessage.body as? String, event == "ready" { - delegate?.bridgeDidInitialize() + await delegate?.bridgeDidInitialize() return } @@ -185,3 +188,10 @@ extension Bridge: ScriptMessageHandlerDelegate { logger.warning("Unhandled message received: \(String(describing: scriptMessage.body))") } } + +extension Bridge { + struct JavaScriptResult { + let result: Any? + let error: Error? + } +} diff --git a/Source/BridgeComponent.swift b/Source/BridgeComponent.swift index de9343e..e91df5d 100644 --- a/Source/BridgeComponent.swift +++ b/Source/BridgeComponent.swift @@ -50,8 +50,8 @@ open class BridgeComponent: BridgingComponent { /// /// - Parameter message: The message to be replied with. /// - Returns: `true` if the reply was successful, `false` if the bridge is not available. - public func reply(with message: Message) -> Bool { - return delegate.reply(with: message) + public func reply(with message: Message) async -> Bool { + await delegate.reply(with: message) } @discardableResult @@ -61,13 +61,13 @@ open class BridgeComponent: BridgingComponent { /// /// - Parameter event: The `event` for which a reply should be sent. /// - Returns: `true` if the reply was successful, `false` if the event message was not received. - public func reply(to event: String) -> Bool { + public func reply(to event: String) async -> Bool { guard let message = receivedMessage(for: event) else { logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received") return false } - return reply(with: message) + return await reply(with: message) } @discardableResult @@ -79,7 +79,7 @@ open class BridgeComponent: BridgingComponent { /// - event: The `event` for which a reply should be sent. /// - jsonData: The `jsonData` to be included in the reply message. /// - Returns: `true` if the reply was successful, `false` if the event message was not received. - public func reply(to event: String, with jsonData: String) -> Bool { + public func reply(to event: String, with jsonData: String) async -> Bool { guard let message = receivedMessage(for: event) else { logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received") return false @@ -87,7 +87,7 @@ open class BridgeComponent: BridgingComponent { let messageReply = message.replacing(jsonData: jsonData) - return reply(with: messageReply) + return await reply(with: messageReply) } @discardableResult @@ -100,7 +100,7 @@ open class BridgeComponent: BridgingComponent { /// - event: The `event` for which a reply should be sent. /// - data: An instance conforming to `Encodable` to be included as `jsonData` in the reply message. /// - Returns: `true` if the reply was successful, `false` if the event message was not received. - public func reply(to event: String, with data: T) -> Bool { + public func reply(to event: String, with data: T) async -> Bool { guard let message = receivedMessage(for: event) else { logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received") return false @@ -108,7 +108,7 @@ open class BridgeComponent: BridgingComponent { let messageReply = message.replacing(data: data) - return reply(with: messageReply) + return await reply(with: messageReply) } /// Returns the last received message for a given `event`, if available. diff --git a/Source/BridgeDelegate.swift b/Source/BridgeDelegate.swift index 97c861e..2efb3b9 100644 --- a/Source/BridgeDelegate.swift +++ b/Source/BridgeDelegate.swift @@ -10,8 +10,8 @@ public protocol BridgingDelegate: AnyObject { func webViewDidBecomeActive(_ webView: WKWebView) func webViewDidBecomeDeactivated() - func reply(with message: Message) -> Bool - + func reply(with message: Message) async -> Bool + func onViewDidLoad() func onViewWillAppear() func onViewDidAppear() @@ -20,7 +20,7 @@ public protocol BridgingDelegate: AnyObject { func component() -> C? - func bridgeDidInitialize() + func bridgeDidInitialize() async func bridgeDidReceiveMessage(_ message: Message) -> Bool } @@ -60,13 +60,13 @@ public final class BridgeDelegate: BridgingDelegate { /// /// - Parameter message: The message to be replied with. /// - Returns: `true` if the reply was successful, `false` if the bridge is not available. - public func reply(with message: Message) -> Bool { + public func reply(with message: Message) async -> Bool { guard let bridge else { logger.warning("bridgeMessageFailedToReply: bridge is not available") return false } - bridge.reply(with: message) + await bridge.reply(with: message) return true } @@ -109,9 +109,9 @@ public final class BridgeDelegate: BridgingDelegate { // MARK: Internal use - public func bridgeDidInitialize() { + public func bridgeDidInitialize() async { let componentNames = componentTypes.map { $0.name } - bridge?.register(components: componentNames) + await bridge?.register(components: componentNames) } @discardableResult diff --git a/Source/ScriptMessageHandler.swift b/Source/ScriptMessageHandler.swift index 1b21d18..63720b7 100644 --- a/Source/ScriptMessageHandler.swift +++ b/Source/ScriptMessageHandler.swift @@ -1,7 +1,7 @@ import WebKit protocol ScriptMessageHandlerDelegate: AnyObject { - func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) + func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async } // Avoids retain cycle caused by WKUserContentController @@ -13,6 +13,6 @@ final class ScriptMessageHandler: NSObject, WKScriptMessageHandler { } func userContentController(_ userContentController: WKUserContentController, didReceive scriptMessage: WKScriptMessage) { - delegate?.scriptMessageHandlerDidReceiveMessage(scriptMessage) + Task { await delegate?.scriptMessageHandlerDidReceiveMessage(scriptMessage) } } } diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index 91dd5d9..e0cfe12 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -44,47 +44,47 @@ class BridgeComponentTest: XCTestCase { // MARK: reply(to:) - func test_replyToReceivedMessageSucceeds() { - let success = component.reply(to: "connect") - + func test_replyToReceivedMessageSucceeds() async { + let success = await component.reply(to: "connect") + XCTAssertTrue(success) XCTAssertTrue(delegate.replyWithMessageWasCalled) XCTAssertEqual(delegate.replyWithMessageArg, message) } - func test_replyToReceivedMessageWithACodableObjectSucceeds() { + func test_replyToReceivedMessageWithACodableObjectSucceeds() async { let messageData = MessageData(title: "hey", subtitle: "", actionName: "tap") let newJsonData = "{\"title\":\"hey\",\"subtitle\":\"\",\"actionName\":\"tap\"}" let newMessage = message.replacing(jsonData: newJsonData) - let success = component.reply(to: "connect", with: messageData) - + let success = await component.reply(to: "connect", with: messageData) + XCTAssertTrue(success) XCTAssertTrue(delegate.replyWithMessageWasCalled) XCTAssertEqual(delegate.replyWithMessageArg, newMessage) } - func test_replyToMessageNotReceivedWithACodableObjectIgnoresTheReply() { + func test_replyToMessageNotReceivedWithACodableObjectIgnoresTheReply() async { let messageData = MessageData(title: "hey", subtitle: "", actionName: "tap") - let success = component.reply(to: "disconnect", with: messageData) - + let success = await component.reply(to: "disconnect", with: messageData) + XCTAssertFalse(success) XCTAssertFalse(delegate.replyWithMessageWasCalled) XCTAssertNil(delegate.replyWithMessageArg) } - func test_replyToMessageNotReceivedIgnoresTheReply() { - let success = component.reply(to: "disconnect") - + func test_replyToMessageNotReceivedIgnoresTheReply() async { + let success = await component.reply(to: "disconnect") + XCTAssertFalse(success) XCTAssertFalse(delegate.replyWithMessageWasCalled) XCTAssertNil(delegate.replyWithMessageArg) } - func test_replyToMessageNotReceivedWithJsonDataIgnoresTheReply() { - let success = component.reply(to: "disconnect", with: "{\"title\":\"Page-title\"}") - + func test_replyToMessageNotReceivedWithJsonDataIgnoresTheReply() async { + let success = await component.reply(to: "disconnect", with: "{\"title\":\"Page-title\"}") + XCTAssertFalse(success) XCTAssertFalse(delegate.replyWithMessageWasCalled) XCTAssertNil(delegate.replyWithMessageArg) @@ -92,12 +92,12 @@ class BridgeComponentTest: XCTestCase { // MARK: reply(with:) - func test_replyWithSucceedsWhenBridgeIsSet() { + func test_replyWithSucceedsWhenBridgeIsSet() async { let newJsonData = "{\"title\":\"Page-title\"}" let newMessage = message.replacing(jsonData: newJsonData) - let success = component.reply(with: newMessage) - + let success = await component.reply(with: newMessage) + XCTAssertTrue(success) XCTAssertTrue(delegate.replyWithMessageWasCalled) XCTAssertEqual(delegate.replyWithMessageArg, newMessage) diff --git a/Tests/BridgeDelegateTests.swift b/Tests/BridgeDelegateTests.swift index b16940f..2f075e6 100644 --- a/Tests/BridgeDelegateTests.swift +++ b/Tests/BridgeDelegateTests.swift @@ -22,9 +22,9 @@ class BridgeDelegateTests: XCTestCase { delegate.onViewDidLoad() } - func testBridgeDidInitialize() { - delegate.bridgeDidInitialize() - + func testBridgeDidInitialize() async { + await delegate.bridgeDidInitialize() + XCTAssertTrue(bridge.registerComponentsWasCalled) XCTAssertEqual(bridge.registerComponentsArg, ["one", "two"]) @@ -143,20 +143,20 @@ class BridgeDelegateTests: XCTestCase { // MARK: reply(with:) - func test_replyWithSucceedsWhenBridgeIsSet() { + func test_replyWithSucceedsWhenBridgeIsSet() async { let message = testMessage() - let success = delegate.reply(with: message) - + let success = await delegate.reply(with: message) + XCTAssertTrue(success) XCTAssertTrue(bridge.replyWithMessageWasCalled) XCTAssertEqual(bridge.replyWithMessageArg, message) } - func test_replyWithFailsWhenBridgeNotSet() { + func test_replyWithFailsWhenBridgeNotSet() async { delegate.bridge = nil let message = testMessage() - let success = delegate.reply(with: message) + let success = await delegate.reply(with: message) XCTAssertFalse(success) XCTAssertFalse(bridge.replyWithMessageWasCalled) diff --git a/Tests/BridgeTests.swift b/Tests/BridgeTests.swift index c0e6b17..c1e5f18 100644 --- a/Tests/BridgeTests.swift +++ b/Tests/BridgeTests.swift @@ -24,34 +24,38 @@ class BridgeTests: XCTestCase { XCTAssertEqual(userContentController.userScripts.count, 1) } - func testRegisterComponentCallsJavaScriptFunction() { + @MainActor + func testRegisterComponentCallsJavaScriptFunction() async { let webView = TestWebView() let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - bridge.register(component: "test") + await bridge.register(component: "test") XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.register(\"test\")") } - func testRegisterComponentsCallsJavaScriptFunction() { + @MainActor + func testRegisterComponentsCallsJavaScriptFunction() async { let webView = TestWebView() let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - bridge.register(components: ["one", "two"]) + await bridge.register(components: ["one", "two"]) XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.register([\"one\",\"two\"])") } - func testUnregisterComponentCallsJavaScriptFunction() { + @MainActor + func testUnregisterComponentCallsJavaScriptFunction() async { let webView = TestWebView() let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - bridge.unregister(component: "test") + await bridge.unregister(component: "test") XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.unregister(\"test\")") } - func testSendCallsJavaScriptFunction() { + @MainActor + func testSendCallsJavaScriptFunction() async { let webView = TestWebView() let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) @@ -67,53 +71,38 @@ class BridgeTests: XCTestCase { jsonData: data) - bridge.reply(with: message) + await bridge.reply(with: message) XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.replyWith({\"component\":\"page\",\"event\":\"connect\",\"data\":{\"title\":\"Page-title\"},\"id\":\"1\"})") } - func testEvaluateJavaScript() { + @MainActor + func testEvaluateJavaScript() async { let webView = TestWebView() let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - bridge.evaluate(javaScript: "test(1,2,3)") + await bridge.evaluate(javaScript: "test(1,2,3)") XCTAssertEqual(webView.lastEvaluatedJavaScript, "test(1,2,3)") } - func testEvaluateJavaScriptReturnsErrorForNoWebView() { + @MainActor + func testEvaluateJavaScriptReturnsErrorForNoWebView() async { let bridge = Bridge(webView: WKWebView()) bridge.webView = nil - let expectation = self.expectation(description: "error handler") - bridge.evaluate(function: "test", arguments: []) { (result, error) in - XCTAssertEqual(error! as! BridgeError, BridgeError.missingWebView) - expectation.fulfill() - } - - waitForExpectations(timeout: 2) + let result = await bridge.evaluate(function: "test", arguments: []) + XCTAssertEqual(result.error! as! BridgeError, BridgeError.missingWebView) } - func testEvaluateFunction() { + @MainActor + func testEvaluateFunction() async { let webView = TestWebView() let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - bridge.evaluate(function: "test", arguments: [1, 2, 3]) + await _ = bridge.evaluate(function: "test", arguments: [1, 2, 3]) XCTAssertEqual(webView.lastEvaluatedJavaScript, "test(1,2,3)") } - - func testEvaluateFunctionCallsCompletionHandler() { - let webView = TestWebView() - let bridge = Bridge(webView: webView) - - let expectation = self.expectation(description: "completion handler") - - bridge.evaluate(function: "test", arguments: []) { (result, error) in - expectation.fulfill() - } - - waitForExpectations(timeout: 2) - } } private final class TestWebView: WKWebView { diff --git a/Tests/ComponentTestExample/ComposerComponent.swift b/Tests/ComponentTestExample/ComposerComponent.swift index a58901f..1a6b9b8 100644 --- a/Tests/ComponentTestExample/ComposerComponent.swift +++ b/Tests/ComponentTestExample/ComposerComponent.swift @@ -17,7 +17,7 @@ final class ComposerComponent: BridgeComponent { } } - func selectSender(emailAddress: String) { + func selectSender(emailAddress: String) async { guard let message = receivedMessage(for: InboundEvent.connect.rawValue), let senders: [Sender] = message.data() else { return @@ -29,7 +29,7 @@ final class ComposerComponent: BridgeComponent { let newMessage = message.replacing(event: OutboundEvent.selectSender.rawValue, data: SelectSenderMessageData(selectedIndex: sender.index)) - reply(with: newMessage) + await reply(with: newMessage) } func selectedSender() -> String? { diff --git a/Tests/ComponentTestExample/ComposerComponentTests.swift b/Tests/ComponentTestExample/ComposerComponentTests.swift index 4a81d09..849758c 100644 --- a/Tests/ComponentTestExample/ComposerComponentTests.swift +++ b/Tests/ComponentTestExample/ComposerComponentTests.swift @@ -47,29 +47,29 @@ final class ComposerComponentTests: XCTestCase { // MARK: Select sender tests - func test_selectSender_emailFound_sendsTheCorrectMessageReply() { + func test_selectSender_emailFound_sendsTheCorrectMessageReply() async { component.didReceive(message: connectMessage) - component.selectSender(emailAddress: "user1@37signals.com") - + await component.selectSender(emailAddress: "user1@37signals.com") + let expectedMessage = connectMessage.replacing(event: "select-sender", jsonData: "{\"selectedIndex\":1}") XCTAssertTrue(delegate.replyWithMessageWasCalled) XCTAssertEqual(delegate.replyWithMessageArg, expectedMessage) } - func test_selectSender_emailNotFound_doesNotSendAnyMessage() { + func test_selectSender_emailNotFound_doesNotSendAnyMessage() async { component.didReceive(message: connectMessage) - component.selectSender(emailAddress: "test@37signals.com") - + await component.selectSender(emailAddress: "test@37signals.com") + XCTAssertFalse(delegate.replyWithMessageWasCalled) XCTAssertNil(delegate.replyWithMessageArg) } - func test_selectSender_beforeConnectMessage_doesNotSendAnyMessage() { - component.selectSender(emailAddress: "user1@37signals.com") - + func test_selectSender_beforeConnectMessage_doesNotSendAnyMessage() async { + await component.selectSender(emailAddress: "user1@37signals.com") + XCTAssertFalse(delegate.replyWithMessageWasCalled) XCTAssertNil(delegate.replyWithMessageArg) } From 8b4e7f7682a95d3c85616abe1c1dd78f0dbe170b Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Sun, 10 Dec 2023 06:26:32 -0800 Subject: [PATCH 04/25] evaluateJavaScript must be called from main thread --- Source/Bridge.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/Bridge.swift b/Source/Bridge.swift index 09f6ca0..394ab18 100644 --- a/Source/Bridge.swift +++ b/Source/Bridge.swift @@ -74,7 +74,7 @@ public final class Bridge: Bridgable { // let replyMessage = message.replacing(data: data) // callBridgeFunction("send", arguments: [replyMessage.toJSON()]) // } - + @MainActor @discardableResult func evaluate(javaScript: String) async -> JavaScriptResult { guard let webView = webView else { @@ -173,6 +173,7 @@ public final class Bridge: Bridgable { } extension Bridge: ScriptMessageHandlerDelegate { + @MainActor func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async { if let event = scriptMessage.body as? String, event == "ready" { From 28dd478225d97e4419b87d098dc80cfdf5463793 Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Sun, 10 Dec 2023 06:34:16 -0800 Subject: [PATCH 05/25] Only import public API when testing public API --- Tests/BridgeComponentTests.swift | 2 +- Tests/ComponentTestExample/ComposerComponent.swift | 2 +- Tests/ComponentTestExample/ComposerComponentTests.swift | 2 +- Tests/MessageTests.swift | 2 +- Tests/Spies/BridgeComponentSpy.swift | 2 +- Tests/Spies/BridgeDelegateSpy.swift | 2 +- Tests/UserAgentTests.swift | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index e0cfe12..02da133 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -1,7 +1,7 @@ import Foundation import XCTest import WebKit -@testable import Strada +import Strada class BridgeComponentTest: XCTestCase { private var delegate: BridgeDelegateSpy! diff --git a/Tests/ComponentTestExample/ComposerComponent.swift b/Tests/ComponentTestExample/ComposerComponent.swift index 1a6b9b8..6ebd873 100644 --- a/Tests/ComponentTestExample/ComposerComponent.swift +++ b/Tests/ComponentTestExample/ComposerComponent.swift @@ -1,6 +1,6 @@ import Foundation import XCTest -@testable import Strada +import Strada final class ComposerComponent: BridgeComponent { static override var name: String { "composer" } diff --git a/Tests/ComponentTestExample/ComposerComponentTests.swift b/Tests/ComponentTestExample/ComposerComponentTests.swift index 849758c..4a4347d 100644 --- a/Tests/ComponentTestExample/ComposerComponentTests.swift +++ b/Tests/ComponentTestExample/ComposerComponentTests.swift @@ -1,6 +1,6 @@ import XCTest import WebKit -@testable import Strada +import Strada final class ComposerComponentTests: XCTestCase { private var delegate: BridgeDelegateSpy! diff --git a/Tests/MessageTests.swift b/Tests/MessageTests.swift index 54fb786..06dc965 100644 --- a/Tests/MessageTests.swift +++ b/Tests/MessageTests.swift @@ -1,5 +1,5 @@ import XCTest -@testable import Strada +import Strada class MessageTests: XCTestCase { diff --git a/Tests/Spies/BridgeComponentSpy.swift b/Tests/Spies/BridgeComponentSpy.swift index bf3b379..bf65282 100644 --- a/Tests/Spies/BridgeComponentSpy.swift +++ b/Tests/Spies/BridgeComponentSpy.swift @@ -1,5 +1,5 @@ import Foundation -@testable import Strada +import Strada final class BridgeComponentSpy: BridgeComponent { static override var name: String { "two" } diff --git a/Tests/Spies/BridgeDelegateSpy.swift b/Tests/Spies/BridgeDelegateSpy.swift index d3c0690..cec3a4f 100644 --- a/Tests/Spies/BridgeDelegateSpy.swift +++ b/Tests/Spies/BridgeDelegateSpy.swift @@ -1,6 +1,6 @@ import Foundation import WebKit -@testable import Strada +import Strada final class BridgeDelegateSpy: BridgingDelegate { let location: String = "" diff --git a/Tests/UserAgentTests.swift b/Tests/UserAgentTests.swift index 6a60016..29696b3 100644 --- a/Tests/UserAgentTests.swift +++ b/Tests/UserAgentTests.swift @@ -1,6 +1,6 @@ import Foundation import XCTest -@testable import Strada +import Strada class UserAgentTests: XCTestCase { func testUserAgentSubstringWithTwoComponents() { From 8c6872adcc78c574595ab4617e11c2f4b4dc53cc Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Mon, 26 Feb 2024 09:39:00 -0800 Subject: [PATCH 06/25] Ignore .swiftpm directory --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 419f575..ebcfab2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,6 @@ build node_modules *.log - *.xcuserstate - *.xcbkptlist +.swiftpm From 5c7b8b54185c80089c182e1669ba234add3d3330 Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Mon, 26 Feb 2024 09:39:09 -0800 Subject: [PATCH 07/25] Expose error entirely up stack --- Source/Bridge.swift | 113 ++++++++---------- Source/BridgeComponent.swift | 18 ++- Source/BridgeDelegate.swift | 12 +- Source/ScriptMessageHandler.swift | 4 +- Strada.xcodeproj/project.pbxproj | 18 ++- .../xcshareddata/xcschemes/Strada.xcscheme | 2 +- Tests/BridgeComponentTests.swift | 24 ++-- Tests/BridgeDelegateTests.swift | 12 +- Tests/BridgeTests.swift | 34 +++--- .../ComposerComponent.swift | 4 +- .../ComposerComponentTests.swift | 12 +- 11 files changed, 125 insertions(+), 128 deletions(-) diff --git a/Source/Bridge.swift b/Source/Bridge.swift index 394ab18..efce598 100644 --- a/Source/Bridge.swift +++ b/Source/Bridge.swift @@ -8,11 +8,11 @@ public enum BridgeError: Error { protocol Bridgable: AnyObject { var delegate: BridgeDelegate? { get set } var webView: WKWebView? { get } - - func register(component: String) async - func register(components: [String]) async - func unregister(component: String) async - func reply(with message: Message) async + + func register(component: String) async throws + func register(components: [String]) async throws + func unregister(component: String) async throws + func reply(with message: Message) async throws } /// `Bridge` is the object for configuring a web view and @@ -22,50 +22,50 @@ public final class Bridge: Bridgable { weak var delegate: BridgeDelegate? weak var webView: WKWebView? - + public static func initialize(_ webView: WKWebView) { if getBridgeFor(webView) == nil { initialize(Bridge(webView: webView)) } } - + init(webView: WKWebView) { self.webView = webView loadIntoWebView() } - + deinit { webView?.configuration.userContentController.removeScriptMessageHandler(forName: scriptHandlerName) } - + // MARK: - Internal API - + /// Register a single component /// - Parameter component: Name of a component to register support for - func register(component: String) async { - await callBridgeFunction(.register, arguments: [component]) + func register(component: String) async throws { + try await callBridgeFunction(.register, arguments: [component]) } - + /// Register multiple components /// - Parameter components: Array of component names to register - func register(components: [String]) async { - await callBridgeFunction(.register, arguments: [components]) + func register(components: [String]) async throws { + try await callBridgeFunction(.register, arguments: [components]) } - + /// Unregister support for a single component /// - Parameter component: Component name - func unregister(component: String) async { - await callBridgeFunction(.unregister, arguments: [component]) + func unregister(component: String) async throws { + try await callBridgeFunction(.unregister, arguments: [component]) } - + /// Send a message through the bridge to the web application /// - Parameter message: Message to send - func reply(with message: Message) async { + func reply(with message: Message) async throws { logger.debug("bridgeWillReplyWithMessage: \(String(describing: message))") let internalMessage = InternalMessage(from: message) - await callBridgeFunction(.replyWith, arguments: [internalMessage.toJSON()]) + try await callBridgeFunction(.replyWith, arguments: [internalMessage.toJSON()]) } - + // /// Convenience method to reply to a previously received message. Data will be replaced, // /// while id, component, and event will remain the same // /// - Parameter message: Message to reply to @@ -76,28 +76,24 @@ public final class Bridge: Bridgable { // } @MainActor @discardableResult - func evaluate(javaScript: String) async -> JavaScriptResult { + func evaluate(javaScript: String) async throws -> Any? { guard let webView = webView else { - return JavaScriptResult(result: nil, error: BridgeError.missingWebView) + throw BridgeError.missingWebView } - var result: Any? = nil - var error: Error? = nil do { - result = try await webView.evaluateJavaScript(javaScript) - } catch let e { - logger.error("Error evaluating JavaScript: \(e)") - error = e + return try await webView.evaluateJavaScript(javaScript) + } catch { + logger.error("Error evaluating JavaScript: \(error)") + throw error } - - return JavaScriptResult(result: result, error: error) } /// Evaluates a JavaScript function with optional arguments by encoding the arguments /// Function should not include the parens /// Usage: evaluate(function: "console.log", arguments: ["test"]) - func evaluate(function: String, arguments: [Any] = []) async -> JavaScriptResult { - await evaluate(javaScript: JavaScript(functionName: function, arguments: arguments)) + func evaluate(function: String, arguments: [Any] = []) async throws -> Any? { + try await evaluate(javaScript: JavaScript(functionName: function, arguments: arguments).toString()) } static func initialize(_ bridge: Bridge) { @@ -108,23 +104,23 @@ public final class Bridge: Bridgable { static func getBridgeFor(_ webView: WKWebView) -> Bridge? { return instances.first { $0.webView == webView } } - + // MARK: Private private static var instances: [Bridge] = [] /// This needs to match whatever the JavaScript file uses private let bridgeGlobal = "window.nativeBridge" - + /// The webkit.messageHandlers name private let scriptHandlerName = "strada" - - private func callBridgeFunction(_ function: JavaScriptBridgeFunction, arguments: [Any]) async { + + private func callBridgeFunction(_ function: JavaScriptBridgeFunction, arguments: [Any]) async throws { let js = JavaScript(object: bridgeGlobal, functionName: function.rawValue, arguments: arguments) - await evaluate(javaScript: js) + try await evaluate(javaScript: js) } // MARK: - Configuration - + /// Configure the bridge in the provided web view private func loadIntoWebView() { guard let configuration = webView?.configuration else { return } @@ -133,17 +129,18 @@ public final class Bridge: Bridgable { if let userScript = makeUserScript() { configuration.userContentController.addUserScript(userScript) } - + let scriptMessageHandler = ScriptMessageHandler(delegate: self) configuration.userContentController.add(scriptMessageHandler, name: scriptHandlerName) } private func makeUserScript() -> WKUserScript? { guard - let path = PathLoader().pathFor(name: "strada", fileType: "js") else { - return nil + let path = PathLoader().pathFor(name: "strada", fileType: "js") + else { + return nil } - + do { let source = try String(contentsOfFile: path) return WKUserScript(source: source, injectionTime: .atDocumentStart, forMainFrameOnly: true) @@ -152,16 +149,16 @@ public final class Bridge: Bridgable { return nil } } - + // MARK: - JavaScript Evaluation - + @discardableResult - private func evaluate(javaScript: JavaScript) async -> JavaScriptResult { + private func evaluate(javaScript: JavaScript) async throws -> Any? { do { - return await evaluate(javaScript: try javaScript.toString()) + return try await evaluate(javaScript: javaScript.toString()) } catch { logger.error("Error evaluating JavaScript: \(String(describing: javaScript)), error: \(error)") - return JavaScriptResult(result: nil, error: error) + throw error } } @@ -174,25 +171,17 @@ public final class Bridge: Bridgable { extension Bridge: ScriptMessageHandlerDelegate { @MainActor - func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async { - if let event = scriptMessage.body as? String, - event == "ready" { - await delegate?.bridgeDidInitialize() + func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async throws { + if let event = scriptMessage.body as? String, event == "ready" { + try await delegate?.bridgeDidInitialize() return } - + if let message = InternalMessage(scriptMessage: scriptMessage) { delegate?.bridgeDidReceiveMessage(message.toMessage()) return } - - logger.warning("Unhandled message received: \(String(describing: scriptMessage.body))") - } -} -extension Bridge { - struct JavaScriptResult { - let result: Any? - let error: Error? + logger.warning("Unhandled message received: \(String(describing: scriptMessage.body))") } } diff --git a/Source/BridgeComponent.swift b/Source/BridgeComponent.swift index e91df5d..8d75596 100644 --- a/Source/BridgeComponent.swift +++ b/Source/BridgeComponent.swift @@ -50,8 +50,8 @@ open class BridgeComponent: BridgingComponent { /// /// - Parameter message: The message to be replied with. /// - Returns: `true` if the reply was successful, `false` if the bridge is not available. - public func reply(with message: Message) async -> Bool { - await delegate.reply(with: message) + public func reply(with message: Message) async throws -> Bool { + try await delegate.reply(with: message) } @discardableResult @@ -61,13 +61,13 @@ open class BridgeComponent: BridgingComponent { /// /// - Parameter event: The `event` for which a reply should be sent. /// - Returns: `true` if the reply was successful, `false` if the event message was not received. - public func reply(to event: String) async -> Bool { + public func reply(to event: String) async throws -> Bool { guard let message = receivedMessage(for: event) else { logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received") return false } - return await reply(with: message) + return try await reply(with: message) } @discardableResult @@ -79,15 +79,14 @@ open class BridgeComponent: BridgingComponent { /// - event: The `event` for which a reply should be sent. /// - jsonData: The `jsonData` to be included in the reply message. /// - Returns: `true` if the reply was successful, `false` if the event message was not received. - public func reply(to event: String, with jsonData: String) async -> Bool { + public func reply(to event: String, with jsonData: String) async throws -> Bool { guard let message = receivedMessage(for: event) else { logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received") return false } let messageReply = message.replacing(jsonData: jsonData) - - return await reply(with: messageReply) + return try await reply(with: messageReply) } @discardableResult @@ -100,15 +99,14 @@ open class BridgeComponent: BridgingComponent { /// - event: The `event` for which a reply should be sent. /// - data: An instance conforming to `Encodable` to be included as `jsonData` in the reply message. /// - Returns: `true` if the reply was successful, `false` if the event message was not received. - public func reply(to event: String, with data: T) async -> Bool { + public func reply(to event: String, with data: T) async throws -> Bool { guard let message = receivedMessage(for: event) else { logger.warning("bridgeMessageFailedToReply: message for event \(event) was not received") return false } let messageReply = message.replacing(data: data) - - return await reply(with: messageReply) + return try await reply(with: messageReply) } /// Returns the last received message for a given `event`, if available. diff --git a/Source/BridgeDelegate.swift b/Source/BridgeDelegate.swift index 2efb3b9..19e3470 100644 --- a/Source/BridgeDelegate.swift +++ b/Source/BridgeDelegate.swift @@ -10,7 +10,7 @@ public protocol BridgingDelegate: AnyObject { func webViewDidBecomeActive(_ webView: WKWebView) func webViewDidBecomeDeactivated() - func reply(with message: Message) async -> Bool + func reply(with message: Message) async throws -> Bool func onViewDidLoad() func onViewWillAppear() @@ -20,7 +20,7 @@ public protocol BridgingDelegate: AnyObject { func component() -> C? - func bridgeDidInitialize() async + func bridgeDidInitialize() async throws func bridgeDidReceiveMessage(_ message: Message) -> Bool } @@ -60,13 +60,13 @@ public final class BridgeDelegate: BridgingDelegate { /// /// - Parameter message: The message to be replied with. /// - Returns: `true` if the reply was successful, `false` if the bridge is not available. - public func reply(with message: Message) async -> Bool { + public func reply(with message: Message) async throws -> Bool { guard let bridge else { logger.warning("bridgeMessageFailedToReply: bridge is not available") return false } - await bridge.reply(with: message) + try await bridge.reply(with: message) return true } @@ -109,9 +109,9 @@ public final class BridgeDelegate: BridgingDelegate { // MARK: Internal use - public func bridgeDidInitialize() async { + public func bridgeDidInitialize() async throws { let componentNames = componentTypes.map { $0.name } - await bridge?.register(components: componentNames) + try await bridge?.register(components: componentNames) } @discardableResult diff --git a/Source/ScriptMessageHandler.swift b/Source/ScriptMessageHandler.swift index 63720b7..222cd2a 100644 --- a/Source/ScriptMessageHandler.swift +++ b/Source/ScriptMessageHandler.swift @@ -1,7 +1,7 @@ import WebKit protocol ScriptMessageHandlerDelegate: AnyObject { - func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async + func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async throws } // Avoids retain cycle caused by WKUserContentController @@ -13,6 +13,6 @@ final class ScriptMessageHandler: NSObject, WKScriptMessageHandler { } func userContentController(_ userContentController: WKUserContentController, didReceive scriptMessage: WKScriptMessage) { - Task { await delegate?.scriptMessageHandlerDidReceiveMessage(scriptMessage) } + Task { try await delegate?.scriptMessageHandlerDidReceiveMessage(scriptMessage) } } } diff --git a/Strada.xcodeproj/project.pbxproj b/Strada.xcodeproj/project.pbxproj index 12ca970..a478a53 100644 --- a/Strada.xcodeproj/project.pbxproj +++ b/Strada.xcodeproj/project.pbxproj @@ -3,7 +3,7 @@ archiveVersion = 1; classes = { }; - objectVersion = 53; + objectVersion = 54; objects = { /* Begin PBXBuildFile section */ @@ -248,7 +248,7 @@ attributes = { BuildIndependentTargetsInParallel = YES; LastSwiftUpdateCheck = 1010; - LastUpgradeCheck = 1430; + LastUpgradeCheck = 1520; ORGANIZATIONNAME = Basecamp; TargetAttributes = { 9274F1E42229963B003E85F4 = { @@ -356,6 +356,7 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_SEARCH_USER_PATHS = NO; + ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE; CLANG_CXX_LANGUAGE_STANDARD = "gnu++14"; @@ -391,6 +392,7 @@ DEBUG_INFORMATION_FORMAT = dwarf; ENABLE_STRICT_OBJC_MSGSEND = YES; ENABLE_TESTABILITY = YES; + ENABLE_USER_SCRIPT_SANDBOXING = YES; GCC_C_LANGUAGE_STANDARD = gnu11; GCC_DYNAMIC_NO_PIC = NO; GCC_NO_COMMON_BLOCKS = YES; @@ -421,6 +423,7 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_SEARCH_USER_PATHS = NO; + ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE; CLANG_CXX_LANGUAGE_STANDARD = "gnu++14"; @@ -456,6 +459,7 @@ DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym"; ENABLE_NS_ASSERTIONS = NO; ENABLE_STRICT_OBJC_MSGSEND = YES; + ENABLE_USER_SCRIPT_SANDBOXING = YES; GCC_C_LANGUAGE_STANDARD = gnu11; GCC_NO_COMMON_BLOCKS = YES; GCC_WARN_64_TO_32_BIT_CONVERSION = YES; @@ -539,8 +543,8 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES; - CODE_SIGN_STYLE = Automatic; - DEVELOPMENT_TEAM = 2WNYUYRS7G; + CODE_SIGN_STYLE = Manual; + DEVELOPMENT_TEAM = ""; INFOPLIST_FILE = Tests/Info.plist; LD_RUNPATH_SEARCH_PATHS = ( "$(inherited)", @@ -549,6 +553,7 @@ ); PRODUCT_BUNDLE_IDENTIFIER = dev.hotwire.strada.tests; PRODUCT_NAME = "$(TARGET_NAME)"; + PROVISIONING_PROFILE_SPECIFIER = ""; SWIFT_VERSION = 5.0; TARGETED_DEVICE_FAMILY = "1,2"; }; @@ -558,8 +563,8 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES; - CODE_SIGN_STYLE = Automatic; - DEVELOPMENT_TEAM = 2WNYUYRS7G; + CODE_SIGN_STYLE = Manual; + DEVELOPMENT_TEAM = ""; INFOPLIST_FILE = Tests/Info.plist; LD_RUNPATH_SEARCH_PATHS = ( "$(inherited)", @@ -568,6 +573,7 @@ ); PRODUCT_BUNDLE_IDENTIFIER = dev.hotwire.strada.tests; PRODUCT_NAME = "$(TARGET_NAME)"; + PROVISIONING_PROFILE_SPECIFIER = ""; SWIFT_VERSION = 5.0; TARGETED_DEVICE_FAMILY = "1,2"; }; diff --git a/Strada.xcodeproj/xcshareddata/xcschemes/Strada.xcscheme b/Strada.xcodeproj/xcshareddata/xcschemes/Strada.xcscheme index a43b636..61c2ae8 100644 --- a/Strada.xcodeproj/xcshareddata/xcschemes/Strada.xcscheme +++ b/Strada.xcodeproj/xcshareddata/xcschemes/Strada.xcscheme @@ -1,6 +1,6 @@ String? { diff --git a/Tests/ComponentTestExample/ComposerComponentTests.swift b/Tests/ComponentTestExample/ComposerComponentTests.swift index 4a4347d..1201c40 100644 --- a/Tests/ComponentTestExample/ComposerComponentTests.swift +++ b/Tests/ComponentTestExample/ComposerComponentTests.swift @@ -47,10 +47,10 @@ final class ComposerComponentTests: XCTestCase { // MARK: Select sender tests - func test_selectSender_emailFound_sendsTheCorrectMessageReply() async { + func test_selectSender_emailFound_sendsTheCorrectMessageReply() async throws { component.didReceive(message: connectMessage) - await component.selectSender(emailAddress: "user1@37signals.com") + try await component.selectSender(emailAddress: "user1@37signals.com") let expectedMessage = connectMessage.replacing(event: "select-sender", jsonData: "{\"selectedIndex\":1}") @@ -58,17 +58,17 @@ final class ComposerComponentTests: XCTestCase { XCTAssertEqual(delegate.replyWithMessageArg, expectedMessage) } - func test_selectSender_emailNotFound_doesNotSendAnyMessage() async { + func test_selectSender_emailNotFound_doesNotSendAnyMessage() async throws { component.didReceive(message: connectMessage) - await component.selectSender(emailAddress: "test@37signals.com") + try await component.selectSender(emailAddress: "test@37signals.com") XCTAssertFalse(delegate.replyWithMessageWasCalled) XCTAssertNil(delegate.replyWithMessageArg) } - func test_selectSender_beforeConnectMessage_doesNotSendAnyMessage() async { - await component.selectSender(emailAddress: "user1@37signals.com") + func test_selectSender_beforeConnectMessage_doesNotSendAnyMessage() async throws { + try await component.selectSender(emailAddress: "user1@37signals.com") XCTAssertFalse(delegate.replyWithMessageWasCalled) XCTAssertNil(delegate.replyWithMessageArg) From 77ea02550712a6dcfdb9f1fb839368f2dcab675f Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Mon, 26 Feb 2024 09:39:51 -0800 Subject: [PATCH 08/25] Ignore xcuserdata --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ebcfab2..45985c1 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ node_modules *.xcuserstate *.xcbkptlist .swiftpm +xcuserdata From 2a9aa02283968ce7fc8871f43ade768bb10dd291 Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Mon, 26 Feb 2024 09:56:28 -0800 Subject: [PATCH 09/25] Expose non-async functions to public API --- Source/BridgeComponent.swift | 20 +++++++++++++++-- Tests/BridgeComponentTests.swift | 33 +++++++++++++++++++++++++++++ Tests/Spies/BridgeDelegateSpy.swift | 2 +- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/Source/BridgeComponent.swift b/Source/BridgeComponent.swift index 8d75596..19f4225 100644 --- a/Source/BridgeComponent.swift +++ b/Source/BridgeComponent.swift @@ -53,7 +53,14 @@ open class BridgeComponent: BridgingComponent { public func reply(with message: Message) async throws -> Bool { try await delegate.reply(with: message) } - + + /// Replies to the web with a received message, optionally replacing its `event` or `jsonData`. + /// + /// - Parameter message: The message to be replied with. + public func reply(with message: Message) { + Task { _ = try await delegate.reply(with: message) } + } + @discardableResult /// Replies to the web with the last received message for a given `event` with its original `jsonData`. /// @@ -69,7 +76,16 @@ open class BridgeComponent: BridgingComponent { return try await reply(with: message) } - + + /// Replies to the web with the last received message for a given `event` with its original `jsonData`. + /// + /// NOTE: If a message has not been received for the given `event`, the reply will be ignored. + /// + /// - Parameter event: The `event` for which a reply should be sent. + public func reply(to event: String) { + Task { _ = try await reply(to: event) } + } + @discardableResult /// Replies to the web with the last received message for a given `event`, replacing its `jsonData`. /// diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index 6fcbb1d..abe6c4e 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -90,6 +90,21 @@ class BridgeComponentTest: XCTestCase { XCTAssertNil(delegate.replyWithMessageArg) } + // MARK: reply(to:) non-async + + func test_replyToReceivedMessageSucceeds() { + component.reply(to: "connect") + + wait(for: [expectation( + that: \.replyWithMessageWasCalled, + on: delegate, + willEqual: true + )], timeout: 1) + + XCTAssertTrue(delegate.replyWithMessageWasCalled) + XCTAssertEqual(delegate.replyWithMessageArg, message) + } + // MARK: reply(with:) func test_replyWithSucceedsWhenBridgeIsSet() async throws { @@ -102,4 +117,22 @@ class BridgeComponentTest: XCTestCase { XCTAssertTrue(delegate.replyWithMessageWasCalled) XCTAssertEqual(delegate.replyWithMessageArg, newMessage) } + + // MARK: reply(with:) non-async + + func test_replyWithSucceedsWhenBridgeIsSet() { + let newJsonData = "{\"title\":\"Page-title\"}" + let newMessage = message.replacing(jsonData: newJsonData) + + component.reply(with: newMessage) + + wait(for: [expectation( + that: \.replyWithMessageWasCalled, + on: delegate, + willEqual: true + )], timeout: 1) + + XCTAssertTrue(delegate.replyWithMessageWasCalled) + XCTAssertEqual(delegate.replyWithMessageArg, newMessage) + } } diff --git a/Tests/Spies/BridgeDelegateSpy.swift b/Tests/Spies/BridgeDelegateSpy.swift index cec3a4f..b8b4c02 100644 --- a/Tests/Spies/BridgeDelegateSpy.swift +++ b/Tests/Spies/BridgeDelegateSpy.swift @@ -2,7 +2,7 @@ import Foundation import WebKit import Strada -final class BridgeDelegateSpy: BridgingDelegate { +final class BridgeDelegateSpy: NSObject, BridgingDelegate { let location: String = "" let destination: BridgeDestination = AppBridgeDestination() var webView: WKWebView? = nil From 52de2a35fee7c2347739b757756420b32b4aa4d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Tue, 27 Feb 2024 11:40:20 +0100 Subject: [PATCH 10/25] Return non optional value from `evaluate(javaScript: String)`. --- Source/Bridge.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/Bridge.swift b/Source/Bridge.swift index efce598..0de998c 100644 --- a/Source/Bridge.swift +++ b/Source/Bridge.swift @@ -76,8 +76,8 @@ public final class Bridge: Bridgable { // } @MainActor @discardableResult - func evaluate(javaScript: String) async throws -> Any? { - guard let webView = webView else { + func evaluate(javaScript: String) async throws -> Any { + guard let webView else { throw BridgeError.missingWebView } @@ -92,7 +92,7 @@ public final class Bridge: Bridgable { /// Evaluates a JavaScript function with optional arguments by encoding the arguments /// Function should not include the parens /// Usage: evaluate(function: "console.log", arguments: ["test"]) - func evaluate(function: String, arguments: [Any] = []) async throws -> Any? { + func evaluate(function: String, arguments: [Any] = []) async throws -> Any { try await evaluate(javaScript: JavaScript(functionName: function, arguments: arguments).toString()) } @@ -153,7 +153,7 @@ public final class Bridge: Bridgable { // MARK: - JavaScript Evaluation @discardableResult - private func evaluate(javaScript: JavaScript) async throws -> Any? { + private func evaluate(javaScript: JavaScript) async throws -> Any { do { return try await evaluate(javaScript: javaScript.toString()) } catch { From e9ce397ad8fdfac9dfe94a1538984230f4100920 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Tue, 27 Feb 2024 11:40:50 +0100 Subject: [PATCH 11/25] Remove obsolete completion handler. --- Source/Bridge.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Source/Bridge.swift b/Source/Bridge.swift index 0de998c..a7cb9b5 100644 --- a/Source/Bridge.swift +++ b/Source/Bridge.swift @@ -18,8 +18,6 @@ protocol Bridgable: AnyObject { /// `Bridge` is the object for configuring a web view and /// the channel for sending/receiving messages public final class Bridge: Bridgable { - typealias CompletionHandler = (_ result: Any?, _ error: Error?) -> Void - weak var delegate: BridgeDelegate? weak var webView: WKWebView? From 36f9b17b07b749056483961823c185efa33ae4c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Tue, 27 Feb 2024 14:04:45 +0100 Subject: [PATCH 12/25] Fix failing BridgeTests. --- Tests/BridgeTests.swift | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/Tests/BridgeTests.swift b/Tests/BridgeTests.swift index 279b38c..661badf 100644 --- a/Tests/BridgeTests.swift +++ b/Tests/BridgeTests.swift @@ -23,14 +23,17 @@ class BridgeTests: XCTestCase { Bridge.initialize(webView) XCTAssertEqual(userContentController.userScripts.count, 1) } - + + /// NOTE: Each call to `webView.evaluateJavaScript(String)` will throw an error. + /// We intentionally disregard any thrown errors (`try? await bridge...`) + /// because we validate the evaluated JavaScript string ourselves. @MainActor func testRegisterComponentCallsJavaScriptFunction() async throws { let webView = TestWebView() let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - try await bridge.register(component: "test") + try? await bridge.register(component: "test") XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.register(\"test\")") } @@ -40,7 +43,7 @@ class BridgeTests: XCTestCase { let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - try await bridge.register(components: ["one", "two"]) + try? await bridge.register(components: ["one", "two"]) XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.register([\"one\",\"two\"])") } @@ -50,7 +53,7 @@ class BridgeTests: XCTestCase { let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - try await bridge.unregister(component: "test") + try? await bridge.unregister(component: "test") XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.unregister(\"test\")") } @@ -71,7 +74,7 @@ class BridgeTests: XCTestCase { jsonData: data) - try await bridge.reply(with: message) + try? await bridge.reply(with: message) XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.replyWith({\"component\":\"page\",\"event\":\"connect\",\"data\":{\"title\":\"Page-title\"},\"id\":\"1\"})") } @@ -81,7 +84,7 @@ class BridgeTests: XCTestCase { let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - try await bridge.evaluate(javaScript: "test(1,2,3)") + _ = try? await bridge.evaluate(javaScript: "test(1,2,3)") XCTAssertEqual(webView.lastEvaluatedJavaScript, "test(1,2,3)") } @@ -104,16 +107,16 @@ class BridgeTests: XCTestCase { let bridge = Bridge(webView: webView) XCTAssertNil(webView.lastEvaluatedJavaScript) - _ = try await bridge.evaluate(function: "test", arguments: [1, 2, 3]) + _ = try? await bridge.evaluate(function: "test", arguments: [1, 2, 3]) XCTAssertEqual(webView.lastEvaluatedJavaScript, "test(1,2,3)") } } private final class TestWebView: WKWebView { var lastEvaluatedJavaScript: String? - - override func evaluateJavaScript(_ javaScriptString: String, completionHandler: ((Any?, Error?) -> Void)? = nil) { + + override func evaluateJavaScript(_ javaScriptString: String) async throws -> Any { lastEvaluatedJavaScript = javaScriptString - super.evaluateJavaScript(javaScriptString, completionHandler: completionHandler) + return try await super.evaluateJavaScript(javaScriptString) } } From b2fbbfac2a170f50040689fa838615e89d55eedf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Tue, 27 Feb 2024 14:28:35 +0100 Subject: [PATCH 13/25] Update `BridgeDelegateSpy` to conform to `BridgingDelegate` protocol. --- Tests/Spies/BridgeDelegateSpy.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Spies/BridgeDelegateSpy.swift b/Tests/Spies/BridgeDelegateSpy.swift index b8b4c02..2ffb74f 100644 --- a/Tests/Spies/BridgeDelegateSpy.swift +++ b/Tests/Spies/BridgeDelegateSpy.swift @@ -18,7 +18,7 @@ final class BridgeDelegateSpy: NSObject, BridgingDelegate { } - func reply(with message: Message) -> Bool { + func reply(with message: Message) async throws -> Bool { replyWithMessageWasCalled = true replyWithMessageArg = message @@ -49,7 +49,7 @@ final class BridgeDelegateSpy: NSObject, BridgingDelegate { return nil } - func bridgeDidInitialize() { + func bridgeDidInitialize() async throws { } From 814213ae2843a28d5b14b973e5431698504adf1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Tue, 27 Feb 2024 16:03:53 +0100 Subject: [PATCH 14/25] Increase expectations timeout interval to 5s. --- Tests/BridgeComponentTests.swift | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index abe6c4e..bdc9ff9 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -99,7 +99,7 @@ class BridgeComponentTest: XCTestCase { that: \.replyWithMessageWasCalled, on: delegate, willEqual: true - )], timeout: 1) + )], timeout: .expectationTimeout) XCTAssertTrue(delegate.replyWithMessageWasCalled) XCTAssertEqual(delegate.replyWithMessageArg, message) @@ -130,9 +130,13 @@ class BridgeComponentTest: XCTestCase { that: \.replyWithMessageWasCalled, on: delegate, willEqual: true - )], timeout: 1) + )], timeout: .expectationTimeout) XCTAssertTrue(delegate.replyWithMessageWasCalled) XCTAssertEqual(delegate.replyWithMessageArg, newMessage) } } + +private extension TimeInterval { + static let expectationTimeout: TimeInterval = 5 +} From 63ca1f0723ed769a6d66b657409c614cf01bf2b4 Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Tue, 27 Feb 2024 11:49:44 -0800 Subject: [PATCH 15/25] Improve reliability of flaky tests --- Tests/BridgeComponentTests.swift | 2 ++ Tests/Spies/BridgeDelegateSpy.swift | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index bdc9ff9..39e4af0 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -92,6 +92,7 @@ class BridgeComponentTest: XCTestCase { // MARK: reply(to:) non-async + @MainActor func test_replyToReceivedMessageSucceeds() { component.reply(to: "connect") @@ -120,6 +121,7 @@ class BridgeComponentTest: XCTestCase { // MARK: reply(with:) non-async + @MainActor func test_replyWithSucceedsWhenBridgeIsSet() { let newJsonData = "{\"title\":\"Page-title\"}" let newMessage = message.replacing(jsonData: newJsonData) diff --git a/Tests/Spies/BridgeDelegateSpy.swift b/Tests/Spies/BridgeDelegateSpy.swift index 2ffb74f..ba5c01b 100644 --- a/Tests/Spies/BridgeDelegateSpy.swift +++ b/Tests/Spies/BridgeDelegateSpy.swift @@ -7,7 +7,7 @@ final class BridgeDelegateSpy: NSObject, BridgingDelegate { let destination: BridgeDestination = AppBridgeDestination() var webView: WKWebView? = nil - var replyWithMessageWasCalled = false + @objc dynamic var replyWithMessageWasCalled = false var replyWithMessageArg: Message? func webViewDidBecomeActive(_ webView: WKWebView) { From 3b083b02389277d2c0415d22dda7fb0743b01fd5 Mon Sep 17 00:00:00 2001 From: Joe Masilotti Date: Tue, 27 Feb 2024 11:59:02 -0800 Subject: [PATCH 16/25] Completion blocks and no more flaky tests! --- Source/BridgeComponent.swift | 24 ++++++++++--- Tests/BridgeComponentTests.swift | 52 +++++++++++++++++------------ Tests/Spies/BridgeDelegateSpy.swift | 4 +-- 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/Source/BridgeComponent.swift b/Source/BridgeComponent.swift index 19f4225..1351398 100644 --- a/Source/BridgeComponent.swift +++ b/Source/BridgeComponent.swift @@ -23,6 +23,8 @@ protocol BridgingComponent: AnyObject { } open class BridgeComponent: BridgingComponent { + public typealias ReplyCompletionHandler = (Result) -> Void + /// A unique name representing the `BridgeComponent` type. /// /// Subclasses must provide their own implementation of this property. @@ -57,8 +59,15 @@ open class BridgeComponent: BridgingComponent { /// Replies to the web with a received message, optionally replacing its `event` or `jsonData`. /// /// - Parameter message: The message to be replied with. - public func reply(with message: Message) { - Task { _ = try await delegate.reply(with: message) } + public func reply(with message: Message, completion: ReplyCompletionHandler? = nil) { + Task { + do { + let result = try await delegate.reply(with: message) + completion?(.success((result))) + } catch { + completion?(.failure(error)) + } + } } @discardableResult @@ -82,8 +91,15 @@ open class BridgeComponent: BridgingComponent { /// NOTE: If a message has not been received for the given `event`, the reply will be ignored. /// /// - Parameter event: The `event` for which a reply should be sent. - public func reply(to event: String) { - Task { _ = try await reply(to: event) } + public func reply(to event: String, completion: ReplyCompletionHandler? = nil) { + Task { + do { + let result = try await reply(to: event) + completion?(.success((result))) + } catch { + completion?(.failure(error)) + } + } } @discardableResult diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index 39e4af0..0f77ed5 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -92,18 +92,22 @@ class BridgeComponentTest: XCTestCase { // MARK: reply(to:) non-async - @MainActor func test_replyToReceivedMessageSucceeds() { - component.reply(to: "connect") - - wait(for: [expectation( - that: \.replyWithMessageWasCalled, - on: delegate, - willEqual: true - )], timeout: .expectationTimeout) - - XCTAssertTrue(delegate.replyWithMessageWasCalled) - XCTAssertEqual(delegate.replyWithMessageArg, message) + let expectation = expectation(description: "Wait for completion.") + + component.reply(to: "connect") { [unowned self] result in + switch result { + case .success(let success): + XCTAssert(success) + XCTAssertTrue(delegate.replyWithMessageWasCalled) + XCTAssertEqual(delegate.replyWithMessageArg, message) + case .failure(let error): + XCTFail("Failed with error: \(error)") + } + expectation.fulfill() + } + + wait(for: [expectation], timeout: .expectationTimeout) } // MARK: reply(with:) @@ -121,21 +125,25 @@ class BridgeComponentTest: XCTestCase { // MARK: reply(with:) non-async - @MainActor func test_replyWithSucceedsWhenBridgeIsSet() { + let expectation = expectation(description: "Wait for completion.") + let newJsonData = "{\"title\":\"Page-title\"}" let newMessage = message.replacing(jsonData: newJsonData) - component.reply(with: newMessage) - - wait(for: [expectation( - that: \.replyWithMessageWasCalled, - on: delegate, - willEqual: true - )], timeout: .expectationTimeout) - - XCTAssertTrue(delegate.replyWithMessageWasCalled) - XCTAssertEqual(delegate.replyWithMessageArg, newMessage) + component.reply(with: newMessage) { [unowned self] result in + switch result { + case .success(let success): + XCTAssert(success) + XCTAssertTrue(delegate.replyWithMessageWasCalled) + XCTAssertEqual(delegate.replyWithMessageArg, newMessage) + case .failure(let error): + XCTFail("Failed with error: \(error)") + } + expectation.fulfill() + } + + wait(for: [expectation], timeout: .expectationTimeout) } } diff --git a/Tests/Spies/BridgeDelegateSpy.swift b/Tests/Spies/BridgeDelegateSpy.swift index ba5c01b..99790a8 100644 --- a/Tests/Spies/BridgeDelegateSpy.swift +++ b/Tests/Spies/BridgeDelegateSpy.swift @@ -2,12 +2,12 @@ import Foundation import WebKit import Strada -final class BridgeDelegateSpy: NSObject, BridgingDelegate { +final class BridgeDelegateSpy: BridgingDelegate { let location: String = "" let destination: BridgeDestination = AppBridgeDestination() var webView: WKWebView? = nil - @objc dynamic var replyWithMessageWasCalled = false + var replyWithMessageWasCalled = false var replyWithMessageArg: Message? func webViewDidBecomeActive(_ webView: WKWebView) { From 64b17d7398ad16c693f89fd5fdd7cf2cefd8d593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Wed, 28 Feb 2024 11:17:20 +0100 Subject: [PATCH 17/25] Add all missing non async versions of `reply(to:,with:)`. --- Source/BridgeComponent.swift | 51 ++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/Source/BridgeComponent.swift b/Source/BridgeComponent.swift index 1351398..1e48c33 100644 --- a/Source/BridgeComponent.swift +++ b/Source/BridgeComponent.swift @@ -58,7 +58,10 @@ open class BridgeComponent: BridgingComponent { /// Replies to the web with a received message, optionally replacing its `event` or `jsonData`. /// - /// - Parameter message: The message to be replied with. + /// - Parameters: + /// - message: The message to be replied with. + /// - completion: An optional completion handler to be called when the reply attempt completes. + /// It includes a result indicating whether the reply was successful or not. public func reply(with message: Message, completion: ReplyCompletionHandler? = nil) { Task { do { @@ -90,7 +93,10 @@ open class BridgeComponent: BridgingComponent { /// /// NOTE: If a message has not been received for the given `event`, the reply will be ignored. /// - /// - Parameter event: The `event` for which a reply should be sent. + /// - Parameters: + /// - event: The `event` for which a reply should be sent. + /// - completion: An optional completion handler to be called when the reply attempt completes. + /// It includes a result indicating whether the reply was successful or not. public func reply(to event: String, completion: ReplyCompletionHandler? = nil) { Task { do { @@ -121,6 +127,26 @@ open class BridgeComponent: BridgingComponent { return try await reply(with: messageReply) } + /// Replies to the web with the last received message for a given `event`, replacing its `jsonData`. + /// + /// NOTE: If a message has not been received for the given `event`, the reply will be ignored. + /// + /// - Parameters: + /// - event: The `event` for which a reply should be sent. + /// - jsonData: The `jsonData` to be included in the reply message. + /// - completion: An optional completion handler to be called when the reply attempt completes. + /// It includes a result indicating whether the reply was successful or not. + public func reply(to event: String, with jsonData: String, completion: ReplyCompletionHandler? = nil) { + Task { + do { + let result = try await reply(to: event, with: jsonData) + completion?(.success((result))) + } catch { + completion?(.failure(error)) + } + } + } + @discardableResult /// Replies to the web with the last received message for a given `event`, replacing its `jsonData` /// with the provided `Encodable` object. @@ -141,6 +167,27 @@ open class BridgeComponent: BridgingComponent { return try await reply(with: messageReply) } + /// Replies to the web with the last received message for a given `event`, replacing its `jsonData` + /// with the provided `Encodable` object. + /// + /// NOTE: If a message has not been received for the given `event`, the reply will be ignored. + /// + /// - Parameters: + /// - event: The `event` for which a reply should be sent. + /// - data: An instance conforming to `Encodable` to be included as `jsonData` in the reply message. + /// - completion: An optional completion handler to be called when the reply attempt completes. + /// It includes a result indicating whether the reply was successful or not. + public func reply(to event: String, with data: T, completion: ReplyCompletionHandler? = nil) { + Task { + do { + let result = try await reply(to: event, with: data) + completion?(.success((result))) + } catch { + completion?(.failure(error)) + } + } + } + /// Returns the last received message for a given `event`, if available. /// - Parameter event: The event name. /// - Returns: The last received message, or nil. From 86ea6f5d77ff5211c46576ef8b8fb5cd96b61d05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Wed, 28 Feb 2024 11:18:40 +0100 Subject: [PATCH 18/25] Add tests for non async version of `reply(to:, with:)`.` --- Tests/BridgeComponentTests.swift | 76 ++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index 0f77ed5..3460827 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -109,6 +109,82 @@ class BridgeComponentTest: XCTestCase { wait(for: [expectation], timeout: .expectationTimeout) } + + func test_replyToReceivedMessageWithACodableObjectSucceeds() { + let messageData = MessageData(title: "hey", subtitle: "", actionName: "tap") + let newJsonData = "{\"title\":\"hey\",\"subtitle\":\"\",\"actionName\":\"tap\"}" + let newMessage = message.replacing(jsonData: newJsonData) + let expectation = expectation(description: "Wait for completion.") + + component.reply(to: "connect", with: messageData) { [unowned self] result in + switch result { + case .success(let success): + XCTAssertTrue(success) + XCTAssertTrue(delegate.replyWithMessageWasCalled) + XCTAssertEqual(delegate.replyWithMessageArg, newMessage) + case .failure(let error): + XCTFail("Failed with error: \(error)") + } + expectation.fulfill() + } + + wait(for: [expectation], timeout: .expectationTimeout) + } + + func test_replyToMessageNotReceivedWithACodableObjectIgnoresTheReply() { + let messageData = MessageData(title: "hey", subtitle: "", actionName: "tap") + let expectation = expectation(description: "Wait for completion.") + + component.reply(to: "disconnect", with: messageData) { [unowned self] result in + switch result { + case .success(let success): + XCTAssertFalse(success) + XCTAssertFalse(delegate.replyWithMessageWasCalled) + XCTAssertNil(delegate.replyWithMessageArg) + case .failure(let error): + XCTFail("Failed with error: \(error)") + } + expectation.fulfill() + } + + wait(for: [expectation], timeout: .expectationTimeout) + } + + func test_replyToMessageNotReceivedIgnoresTheReply() { + let expectation = expectation(description: "Wait for completion.") + + component.reply(to: "disconnect") { [unowned self] result in + switch result { + case .success(let success): + XCTAssertFalse(success) + XCTAssertFalse(delegate.replyWithMessageWasCalled) + XCTAssertNil(delegate.replyWithMessageArg) + case .failure(let error): + XCTFail("Failed with error: \(error)") + } + expectation.fulfill() + } + + wait(for: [expectation], timeout: .expectationTimeout) + } + + func test_replyToMessageNotReceivedWithJsonDataIgnoresTheReply() { + let expectation = expectation(description: "Wait for completion.") + + component.reply(to: "disconnect", with: "{\"title\":\"Page-title\"}") { [unowned self] result in + switch result { + case .success(let success): + XCTAssertFalse(success) + XCTAssertFalse(delegate.replyWithMessageWasCalled) + XCTAssertNil(delegate.replyWithMessageArg) + case .failure(let error): + XCTFail("Failed with error: \(error)") + } + expectation.fulfill() + } + + wait(for: [expectation], timeout: .expectationTimeout) + } // MARK: reply(with:) From 390aad4fcde270e92e67105250253969b6d35abd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Wed, 28 Feb 2024 19:58:50 +0100 Subject: [PATCH 19/25] Make `BridgeComponent` run on main thread by adopting @MainActor attribute. --- Source/BridgeComponent.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Source/BridgeComponent.swift b/Source/BridgeComponent.swift index 1e48c33..1348add 100644 --- a/Source/BridgeComponent.swift +++ b/Source/BridgeComponent.swift @@ -1,5 +1,6 @@ import Foundation +@MainActor protocol BridgingComponent: AnyObject { static var name: String { get } var delegate: BridgingDelegate { get } @@ -22,6 +23,7 @@ protocol BridgingComponent: AnyObject { func viewDidDisappear() } +@MainActor open class BridgeComponent: BridgingComponent { public typealias ReplyCompletionHandler = (Result) -> Void @@ -30,7 +32,7 @@ open class BridgeComponent: BridgingComponent { /// Subclasses must provide their own implementation of this property. /// /// - Note: This property is used for identifying the component. - open class var name: String { + nonisolated open class var name: String { fatalError("BridgeComponent subclass must provide a unique 'name'") } @@ -149,7 +151,7 @@ open class BridgeComponent: BridgingComponent { @discardableResult /// Replies to the web with the last received message for a given `event`, replacing its `jsonData` - /// with the provided `Encodable` object. + /// with the provided `Encodable` object. /// /// NOTE: If a message has not been received for the given `event`, the reply will be ignored. /// @@ -275,3 +277,4 @@ open class BridgeComponent: BridgingComponent { private var receivedMessages = [String: Message]() } + From 5d58ad4381716df45a310f4cd56ccd982347b933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Wed, 28 Feb 2024 19:59:00 +0100 Subject: [PATCH 20/25] Make `BridgeDelegate` run on main thread by adopting @MainActor attribute. --- Source/BridgeDelegate.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/BridgeDelegate.swift b/Source/BridgeDelegate.swift index 19e3470..2d42dbc 100644 --- a/Source/BridgeDelegate.swift +++ b/Source/BridgeDelegate.swift @@ -3,6 +3,7 @@ import WebKit public protocol BridgeDestination: AnyObject {} +@MainActor public protocol BridgingDelegate: AnyObject { var location: String { get } var destination: BridgeDestination { get } @@ -24,6 +25,7 @@ public protocol BridgingDelegate: AnyObject { func bridgeDidReceiveMessage(_ message: Message) -> Bool } +@MainActor public final class BridgeDelegate: BridgingDelegate { public let location: String public unowned let destination: BridgeDestination @@ -153,4 +155,3 @@ public final class BridgeDelegate: BridgingDelegate { return component } } - From 2364983c8b9fadf8475e587633f0b011967b32d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Wed, 28 Feb 2024 20:00:04 +0100 Subject: [PATCH 21/25] Make `Bridge` run on main thread by adopting @MainActor attribute. Fix a crash when using `evaluateJavaScript` function. --- Source/Bridge.swift | 48 +++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/Source/Bridge.swift b/Source/Bridge.swift index a7cb9b5..c8daa67 100644 --- a/Source/Bridge.swift +++ b/Source/Bridge.swift @@ -5,6 +5,7 @@ public enum BridgeError: Error { case missingWebView } +@MainActor protocol Bridgable: AnyObject { var delegate: BridgeDelegate? { get set } var webView: WKWebView? { get } @@ -17,25 +18,30 @@ protocol Bridgable: AnyObject { /// `Bridge` is the object for configuring a web view and /// the channel for sending/receiving messages +@MainActor public final class Bridge: Bridgable { + public typealias InitializationCompletionHandler = () -> Void weak var delegate: BridgeDelegate? weak var webView: WKWebView? - public static func initialize(_ webView: WKWebView) { + nonisolated public static func initialize(_ webView: WKWebView, completion: InitializationCompletionHandler?) { + Task { @MainActor in + await initialize(webView) + completion?() + } + } + + public static func initialize(_ webView: WKWebView) async { if getBridgeFor(webView) == nil { initialize(Bridge(webView: webView)) } } - + init(webView: WKWebView) { self.webView = webView loadIntoWebView() } - deinit { - webView?.configuration.userContentController.removeScriptMessageHandler(forName: scriptHandlerName) - } - // MARK: - Internal API /// Register a single component @@ -72,15 +78,14 @@ public final class Bridge: Bridgable { // let replyMessage = message.replacing(data: data) // callBridgeFunction("send", arguments: [replyMessage.toJSON()]) // } - @MainActor @discardableResult - func evaluate(javaScript: String) async throws -> Any { + func evaluate(javaScript: String) async throws -> Any? { guard let webView else { throw BridgeError.missingWebView } do { - return try await webView.evaluateJavaScript(javaScript) + return try await webView.evaluateJavaScriptAsync(javaScript) } catch { logger.error("Error evaluating JavaScript: \(error)") throw error @@ -90,7 +95,7 @@ public final class Bridge: Bridgable { /// Evaluates a JavaScript function with optional arguments by encoding the arguments /// Function should not include the parens /// Usage: evaluate(function: "console.log", arguments: ["test"]) - func evaluate(function: String, arguments: [Any] = []) async throws -> Any { + func evaluate(function: String, arguments: [Any] = []) async throws -> Any? { try await evaluate(javaScript: JavaScript(functionName: function, arguments: arguments).toString()) } @@ -151,7 +156,7 @@ public final class Bridge: Bridgable { // MARK: - JavaScript Evaluation @discardableResult - private func evaluate(javaScript: JavaScript) async throws -> Any { + private func evaluate(javaScript: JavaScript) async throws -> Any? { do { return try await evaluate(javaScript: javaScript.toString()) } catch { @@ -168,7 +173,6 @@ public final class Bridge: Bridgable { } extension Bridge: ScriptMessageHandlerDelegate { - @MainActor func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async throws { if let event = scriptMessage.body as? String, event == "ready" { try await delegate?.bridgeDidInitialize() @@ -183,3 +187,23 @@ extension Bridge: ScriptMessageHandlerDelegate { logger.warning("Unhandled message received: \(String(describing: scriptMessage.body))") } } + +private extension WKWebView { + /// NOTE: The async version crashes the app with `Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value` + /// in case the function doesn't return anything. + /// This is a workaround. See https://forums.developer.apple.com/forums/thread/701553 for more details. + @discardableResult + func evaluateJavaScriptAsync(_ str: String) async throws -> Any? { + return try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in + DispatchQueue.main.async { + self.evaluateJavaScript(str) { data, error in + if let error = error { + continuation.resume(throwing: error) + } else { + continuation.resume(returning: data) + } + } + } + } + } +} From 894a8f1eb8b5d73a2acb96d6c66ae038ef1a833e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Wed, 28 Feb 2024 20:05:54 +0100 Subject: [PATCH 22/25] Update tests to run on @MainActor. --- Tests/BridgeComponentTests.swift | 3 +- Tests/BridgeDelegateTests.swift | 1 + Tests/BridgeTests.swift | 53 ++++++++++++++----- .../ComposerComponentTests.swift | 1 + 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index 3460827..8a02804 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -3,6 +3,7 @@ import XCTest import WebKit import Strada +@MainActor class BridgeComponentTest: XCTestCase { private var delegate: BridgeDelegateSpy! private var destination: AppBridgeDestination! @@ -223,6 +224,6 @@ class BridgeComponentTest: XCTestCase { } } -private extension TimeInterval { +extension TimeInterval { static let expectationTimeout: TimeInterval = 5 } diff --git a/Tests/BridgeDelegateTests.swift b/Tests/BridgeDelegateTests.swift index bb8e56e..78a2ee8 100644 --- a/Tests/BridgeDelegateTests.swift +++ b/Tests/BridgeDelegateTests.swift @@ -3,6 +3,7 @@ import XCTest import WebKit @testable import Strada +@MainActor class BridgeDelegateTests: XCTestCase { private var delegate: BridgeDelegate! private var destination: AppBridgeDestination! diff --git a/Tests/BridgeTests.swift b/Tests/BridgeTests.swift index 661badf..6e00a98 100644 --- a/Tests/BridgeTests.swift +++ b/Tests/BridgeTests.swift @@ -2,32 +2,67 @@ import XCTest import WebKit @testable import Strada +@MainActor class BridgeTests: XCTestCase { - func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() { + func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() async { let webView = WKWebView() let userContentController = webView.configuration.userContentController XCTAssertTrue(userContentController.userScripts.isEmpty) - Bridge.initialize(webView) + await Bridge.initialize(webView) XCTAssertEqual(userContentController.userScripts.count, 1) } - func testInitWithTheSameWebViewDoesNotLoadTwice() { + func testInitWithTheSameWebViewDoesNotLoadTwice() async { let webView = WKWebView() let userContentController = webView.configuration.userContentController XCTAssertTrue(userContentController.userScripts.isEmpty) - Bridge.initialize(webView) + await Bridge.initialize(webView) XCTAssertEqual(userContentController.userScripts.count, 1) - Bridge.initialize(webView) + await Bridge.initialize(webView) XCTAssertEqual(userContentController.userScripts.count, 1) } + + func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() { + let webView = WKWebView() + let userContentController = webView.configuration.userContentController + XCTAssertTrue(userContentController.userScripts.isEmpty) + + let expectation = expectation(description: "Wait for completion.") + Bridge.initialize(webView) { + XCTAssertEqual(userContentController.userScripts.count, 1) + expectation.fulfill() + } + + wait(for: [expectation], timeout: .expectationTimeout) + } + + func testInitWithTheSameWebViewDoesNotLoadTwice() { + let webView = WKWebView() + let userContentController = webView.configuration.userContentController + XCTAssertTrue(userContentController.userScripts.isEmpty) + + let expectation1 = expectation(description: "Wait for completion.") + Bridge.initialize(webView) { + XCTAssertEqual(userContentController.userScripts.count, 1) + expectation1.fulfill() + } + + let expectation2 = expectation(description: "Wait for completion.") + + Bridge.initialize(webView) { + XCTAssertEqual(userContentController.userScripts.count, 1) + expectation2.fulfill() + } + + wait(for: [expectation1, expectation2], timeout: .expectationTimeout) + } /// NOTE: Each call to `webView.evaluateJavaScript(String)` will throw an error. /// We intentionally disregard any thrown errors (`try? await bridge...`) /// because we validate the evaluated JavaScript string ourselves. - @MainActor func testRegisterComponentCallsJavaScriptFunction() async throws { let webView = TestWebView() let bridge = Bridge(webView: webView) @@ -37,7 +72,6 @@ class BridgeTests: XCTestCase { XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.register(\"test\")") } - @MainActor func testRegisterComponentsCallsJavaScriptFunction() async throws { let webView = TestWebView() let bridge = Bridge(webView: webView) @@ -47,7 +81,6 @@ class BridgeTests: XCTestCase { XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.register([\"one\",\"two\"])") } - @MainActor func testUnregisterComponentCallsJavaScriptFunction() async throws { let webView = TestWebView() let bridge = Bridge(webView: webView) @@ -57,7 +90,6 @@ class BridgeTests: XCTestCase { XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.unregister(\"test\")") } - @MainActor func testSendCallsJavaScriptFunction() async throws { let webView = TestWebView() let bridge = Bridge(webView: webView) @@ -78,7 +110,6 @@ class BridgeTests: XCTestCase { XCTAssertEqual(webView.lastEvaluatedJavaScript, "window.nativeBridge.replyWith({\"component\":\"page\",\"event\":\"connect\",\"data\":{\"title\":\"Page-title\"},\"id\":\"1\"})") } - @MainActor func testEvaluateJavaScript() async throws { let webView = TestWebView() let bridge = Bridge(webView: webView) @@ -88,7 +119,6 @@ class BridgeTests: XCTestCase { XCTAssertEqual(webView.lastEvaluatedJavaScript, "test(1,2,3)") } - @MainActor func testEvaluateJavaScriptReturnsErrorForNoWebView() async throws { let bridge = Bridge(webView: WKWebView()) bridge.webView = nil @@ -101,7 +131,6 @@ class BridgeTests: XCTestCase { XCTAssertEqual(bridgeError, BridgeError.missingWebView) } - @MainActor func testEvaluateFunction() async throws { let webView = TestWebView() let bridge = Bridge(webView: webView) diff --git a/Tests/ComponentTestExample/ComposerComponentTests.swift b/Tests/ComponentTestExample/ComposerComponentTests.swift index 1201c40..f25289c 100644 --- a/Tests/ComponentTestExample/ComposerComponentTests.swift +++ b/Tests/ComponentTestExample/ComposerComponentTests.swift @@ -2,6 +2,7 @@ import XCTest import WebKit import Strada +@MainActor final class ComposerComponentTests: XCTestCase { private var delegate: BridgeDelegateSpy! private var destination: AppBridgeDestination! From e83e7b40acb6ff340b6cf4dde546bbe63089fd94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Thu, 29 Feb 2024 11:28:17 +0100 Subject: [PATCH 23/25] Don't dispatch script messages in an async context. --- Source/Bridge.swift | 19 +++++++++---------- Source/BridgeDelegate.swift | 12 +++++++++--- Source/ScriptMessageHandler.swift | 4 ++-- Tests/BridgeDelegateTests.swift | 7 +++++-- Tests/Spies/BridgeDelegateSpy.swift | 2 +- Tests/Spies/BridgeSpy.swift | 10 +++++++++- 6 files changed, 35 insertions(+), 19 deletions(-) diff --git a/Source/Bridge.swift b/Source/Bridge.swift index c8daa67..2c0a7f3 100644 --- a/Source/Bridge.swift +++ b/Source/Bridge.swift @@ -173,9 +173,9 @@ public final class Bridge: Bridgable { } extension Bridge: ScriptMessageHandlerDelegate { - func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async throws { + func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) { if let event = scriptMessage.body as? String, event == "ready" { - try await delegate?.bridgeDidInitialize() + delegate?.bridgeDidInitialize() return } @@ -193,15 +193,14 @@ private extension WKWebView { /// in case the function doesn't return anything. /// This is a workaround. See https://forums.developer.apple.com/forums/thread/701553 for more details. @discardableResult - func evaluateJavaScriptAsync(_ str: String) async throws -> Any? { + @MainActor + func evaluateJavaScriptAsync(_ javaScriptString: String) async throws -> Any? { return try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in - DispatchQueue.main.async { - self.evaluateJavaScript(str) { data, error in - if let error = error { - continuation.resume(throwing: error) - } else { - continuation.resume(returning: data) - } + evaluateJavaScript(javaScriptString) { data, error in + if let error { + continuation.resume(throwing: error) + } else { + continuation.resume(returning: data) } } } diff --git a/Source/BridgeDelegate.swift b/Source/BridgeDelegate.swift index 2d42dbc..2743a86 100644 --- a/Source/BridgeDelegate.swift +++ b/Source/BridgeDelegate.swift @@ -21,7 +21,7 @@ public protocol BridgingDelegate: AnyObject { func component() -> C? - func bridgeDidInitialize() async throws + func bridgeDidInitialize() func bridgeDidReceiveMessage(_ message: Message) -> Bool } @@ -111,9 +111,15 @@ public final class BridgeDelegate: BridgingDelegate { // MARK: Internal use - public func bridgeDidInitialize() async throws { + public func bridgeDidInitialize() { let componentNames = componentTypes.map { $0.name } - try await bridge?.register(components: componentNames) + Task { + do { + try await bridge?.register(components: componentNames) + } catch { + logger.error("bridgeDidFailToRegisterComponents: \(error)") + } + } } @discardableResult diff --git a/Source/ScriptMessageHandler.swift b/Source/ScriptMessageHandler.swift index 222cd2a..1b21d18 100644 --- a/Source/ScriptMessageHandler.swift +++ b/Source/ScriptMessageHandler.swift @@ -1,7 +1,7 @@ import WebKit protocol ScriptMessageHandlerDelegate: AnyObject { - func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) async throws + func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) } // Avoids retain cycle caused by WKUserContentController @@ -13,6 +13,6 @@ final class ScriptMessageHandler: NSObject, WKScriptMessageHandler { } func userContentController(_ userContentController: WKUserContentController, didReceive scriptMessage: WKScriptMessage) { - Task { try await delegate?.scriptMessageHandlerDidReceiveMessage(scriptMessage) } + delegate?.scriptMessageHandlerDidReceiveMessage(scriptMessage) } } diff --git a/Tests/BridgeDelegateTests.swift b/Tests/BridgeDelegateTests.swift index 78a2ee8..daf0638 100644 --- a/Tests/BridgeDelegateTests.swift +++ b/Tests/BridgeDelegateTests.swift @@ -24,8 +24,11 @@ class BridgeDelegateTests: XCTestCase { } func testBridgeDidInitialize() async throws { - try await delegate.bridgeDidInitialize() - + await withCheckedContinuation { continuation in + bridge.registerComponentsContinuation = continuation + delegate.bridgeDidInitialize() + } + XCTAssertTrue(bridge.registerComponentsWasCalled) XCTAssertEqual(bridge.registerComponentsArg, ["one", "two"]) diff --git a/Tests/Spies/BridgeDelegateSpy.swift b/Tests/Spies/BridgeDelegateSpy.swift index 99790a8..d8029f4 100644 --- a/Tests/Spies/BridgeDelegateSpy.swift +++ b/Tests/Spies/BridgeDelegateSpy.swift @@ -49,7 +49,7 @@ final class BridgeDelegateSpy: BridgingDelegate { return nil } - func bridgeDidInitialize() async throws { + func bridgeDidInitialize() { } diff --git a/Tests/Spies/BridgeSpy.swift b/Tests/Spies/BridgeSpy.swift index 9771e34..a8f4371 100644 --- a/Tests/Spies/BridgeSpy.swift +++ b/Tests/Spies/BridgeSpy.swift @@ -9,7 +9,15 @@ final class BridgeSpy: Bridgable { var registerComponentWasCalled = false var registerComponentArg: String? = nil - var registerComponentsWasCalled = false + var registerComponentsWasCalled = false { + didSet { + if registerComponentsWasCalled { + registerComponentsContinuation?.resume() + registerComponentsContinuation = nil + } + } + } + var registerComponentsContinuation: CheckedContinuation? var registerComponentsArg: [String]? = nil var unregisterComponentWasCalled = false From 454d6c067d946d57915ee918f271ac03dfde316c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Thu, 29 Feb 2024 11:38:33 +0100 Subject: [PATCH 24/25] Move expectation timeout interval to its own file. --- Strada.xcodeproj/project.pbxproj | 12 ++++++++++++ Tests/BridgeComponentTests.swift | 4 ---- .../Extensions/TimeInterval+ExpectationTimeout.swift | 5 +++++ 3 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 Tests/Extensions/TimeInterval+ExpectationTimeout.swift diff --git a/Strada.xcodeproj/project.pbxproj b/Strada.xcodeproj/project.pbxproj index a478a53..5c3ab56 100644 --- a/Strada.xcodeproj/project.pbxproj +++ b/Strada.xcodeproj/project.pbxproj @@ -33,6 +33,7 @@ E2DB15912A7163B0001EE08C /* BridgeDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2DB15902A7163B0001EE08C /* BridgeDelegate.swift */; }; E2DB15932A7282CF001EE08C /* BridgeComponent.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2DB15922A7282CF001EE08C /* BridgeComponent.swift */; }; E2DB15952A72B0A8001EE08C /* BridgeDelegateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2DB15942A72B0A8001EE08C /* BridgeDelegateTests.swift */; }; + E2F4E06B2B9095BC000A3A24 /* TimeInterval+ExpectationTimeout.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2F4E06A2B9095BC000A3A24 /* TimeInterval+ExpectationTimeout.swift */; }; E2FDCF982A8297DA003D27AE /* BridgeComponentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FDCF972A8297DA003D27AE /* BridgeComponentTests.swift */; }; E2FDCF9B2A829AEE003D27AE /* BridgeSpy.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FDCF9A2A829AEE003D27AE /* BridgeSpy.swift */; }; E2FDCF9D2A829C6F003D27AE /* TestData.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2FDCF9C2A829C6F003D27AE /* TestData.swift */; }; @@ -79,6 +80,7 @@ E2DB15902A7163B0001EE08C /* BridgeDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BridgeDelegate.swift; sourceTree = ""; }; E2DB15922A7282CF001EE08C /* BridgeComponent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BridgeComponent.swift; sourceTree = ""; }; E2DB15942A72B0A8001EE08C /* BridgeDelegateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BridgeDelegateTests.swift; sourceTree = ""; }; + E2F4E06A2B9095BC000A3A24 /* TimeInterval+ExpectationTimeout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TimeInterval+ExpectationTimeout.swift"; sourceTree = ""; }; E2FDCF972A8297DA003D27AE /* BridgeComponentTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BridgeComponentTests.swift; sourceTree = ""; }; E2FDCF9A2A829AEE003D27AE /* BridgeSpy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BridgeSpy.swift; sourceTree = ""; }; E2FDCF9C2A829C6F003D27AE /* TestData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestData.swift; sourceTree = ""; }; @@ -146,6 +148,7 @@ 9274F1F22229963B003E85F4 /* Tests */ = { isa = PBXGroup; children = ( + E2F4E0692B9095A5000A3A24 /* Extensions */, E227FAF12A94D48C00A645E4 /* ComponentTestExample */, E2FDCF9C2A829C6F003D27AE /* TestData.swift */, E2FDCF992A829AD5003D27AE /* Spies */, @@ -181,6 +184,14 @@ path = ComponentTestExample; sourceTree = ""; }; + E2F4E0692B9095A5000A3A24 /* Extensions */ = { + isa = PBXGroup; + children = ( + E2F4E06A2B9095BC000A3A24 /* TimeInterval+ExpectationTimeout.swift */, + ); + path = Extensions; + sourceTree = ""; + }; E2FDCF992A829AD5003D27AE /* Spies */ = { isa = PBXGroup; children = ( @@ -327,6 +338,7 @@ files = ( E2DB15952A72B0A8001EE08C /* BridgeDelegateTests.swift in Sources */, E227FAF02A94D34E00A645E4 /* ComposerComponent.swift in Sources */, + E2F4E06B2B9095BC000A3A24 /* TimeInterval+ExpectationTimeout.swift in Sources */, E227FAEE2A94B35900A645E4 /* BridgeDelegateSpy.swift in Sources */, C11349C2258801F6000A6E56 /* JavaScriptTests.swift in Sources */, E227FAF32A94D57300A645E4 /* ComposerComponentTests.swift in Sources */, diff --git a/Tests/BridgeComponentTests.swift b/Tests/BridgeComponentTests.swift index 8a02804..a6aa1b0 100644 --- a/Tests/BridgeComponentTests.swift +++ b/Tests/BridgeComponentTests.swift @@ -223,7 +223,3 @@ class BridgeComponentTest: XCTestCase { wait(for: [expectation], timeout: .expectationTimeout) } } - -extension TimeInterval { - static let expectationTimeout: TimeInterval = 5 -} diff --git a/Tests/Extensions/TimeInterval+ExpectationTimeout.swift b/Tests/Extensions/TimeInterval+ExpectationTimeout.swift new file mode 100644 index 0000000..101e7ea --- /dev/null +++ b/Tests/Extensions/TimeInterval+ExpectationTimeout.swift @@ -0,0 +1,5 @@ +import Foundation + +extension TimeInterval { + static let expectationTimeout: TimeInterval = 5 +} From 5d1cffb305fc56387ed11e492bc47d8dd7b18b78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20=C5=A0vara?= Date: Thu, 29 Feb 2024 15:57:01 +0100 Subject: [PATCH 25/25] Remove @MainActor attribute from the Bridge and add it to relevant functions only. --- Source/Bridge.swift | 20 +++++++++--------- Tests/BridgeTests.swift | 45 +++++------------------------------------ 2 files changed, 15 insertions(+), 50 deletions(-) diff --git a/Source/Bridge.swift b/Source/Bridge.swift index 2c0a7f3..23ccd73 100644 --- a/Source/Bridge.swift +++ b/Source/Bridge.swift @@ -5,7 +5,6 @@ public enum BridgeError: Error { case missingWebView } -@MainActor protocol Bridgable: AnyObject { var delegate: BridgeDelegate? { get set } var webView: WKWebView? { get } @@ -18,20 +17,12 @@ protocol Bridgable: AnyObject { /// `Bridge` is the object for configuring a web view and /// the channel for sending/receiving messages -@MainActor public final class Bridge: Bridgable { public typealias InitializationCompletionHandler = () -> Void weak var delegate: BridgeDelegate? weak var webView: WKWebView? - nonisolated public static func initialize(_ webView: WKWebView, completion: InitializationCompletionHandler?) { - Task { @MainActor in - await initialize(webView) - completion?() - } - } - - public static func initialize(_ webView: WKWebView) async { + public static func initialize(_ webView: WKWebView) { if getBridgeFor(webView) == nil { initialize(Bridge(webView: webView)) } @@ -46,24 +37,28 @@ public final class Bridge: Bridgable { /// Register a single component /// - Parameter component: Name of a component to register support for + @MainActor func register(component: String) async throws { try await callBridgeFunction(.register, arguments: [component]) } /// Register multiple components /// - Parameter components: Array of component names to register + @MainActor func register(components: [String]) async throws { try await callBridgeFunction(.register, arguments: [components]) } /// Unregister support for a single component /// - Parameter component: Component name + @MainActor func unregister(component: String) async throws { try await callBridgeFunction(.unregister, arguments: [component]) } /// Send a message through the bridge to the web application /// - Parameter message: Message to send + @MainActor func reply(with message: Message) async throws { logger.debug("bridgeWillReplyWithMessage: \(String(describing: message))") let internalMessage = InternalMessage(from: message) @@ -79,6 +74,7 @@ public final class Bridge: Bridgable { // callBridgeFunction("send", arguments: [replyMessage.toJSON()]) // } @discardableResult + @MainActor func evaluate(javaScript: String) async throws -> Any? { guard let webView else { throw BridgeError.missingWebView @@ -95,6 +91,7 @@ public final class Bridge: Bridgable { /// Evaluates a JavaScript function with optional arguments by encoding the arguments /// Function should not include the parens /// Usage: evaluate(function: "console.log", arguments: ["test"]) + @MainActor func evaluate(function: String, arguments: [Any] = []) async throws -> Any? { try await evaluate(javaScript: JavaScript(functionName: function, arguments: arguments).toString()) } @@ -117,6 +114,7 @@ public final class Bridge: Bridgable { /// The webkit.messageHandlers name private let scriptHandlerName = "strada" + @MainActor private func callBridgeFunction(_ function: JavaScriptBridgeFunction, arguments: [Any]) async throws { let js = JavaScript(object: bridgeGlobal, functionName: function.rawValue, arguments: arguments) try await evaluate(javaScript: js) @@ -156,6 +154,7 @@ public final class Bridge: Bridgable { // MARK: - JavaScript Evaluation @discardableResult + @MainActor private func evaluate(javaScript: JavaScript) async throws -> Any? { do { return try await evaluate(javaScript: javaScript.toString()) @@ -173,6 +172,7 @@ public final class Bridge: Bridgable { } extension Bridge: ScriptMessageHandlerDelegate { + @MainActor func scriptMessageHandlerDidReceiveMessage(_ scriptMessage: WKScriptMessage) { if let event = scriptMessage.body as? String, event == "ready" { delegate?.bridgeDidInitialize() diff --git a/Tests/BridgeTests.swift b/Tests/BridgeTests.swift index 6e00a98..be6fcfc 100644 --- a/Tests/BridgeTests.swift +++ b/Tests/BridgeTests.swift @@ -4,61 +4,26 @@ import WebKit @MainActor class BridgeTests: XCTestCase { - func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() async { + func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() { let webView = WKWebView() let userContentController = webView.configuration.userContentController XCTAssertTrue(userContentController.userScripts.isEmpty) - await Bridge.initialize(webView) + Bridge.initialize(webView) XCTAssertEqual(userContentController.userScripts.count, 1) } - func testInitWithTheSameWebViewDoesNotLoadTwice() async { + func testInitWithTheSameWebViewDoesNotLoadTwice() { let webView = WKWebView() let userContentController = webView.configuration.userContentController XCTAssertTrue(userContentController.userScripts.isEmpty) - await Bridge.initialize(webView) + Bridge.initialize(webView) XCTAssertEqual(userContentController.userScripts.count, 1) - await Bridge.initialize(webView) + Bridge.initialize(webView) XCTAssertEqual(userContentController.userScripts.count, 1) } - - func testInitWithANewWebViewAutomaticallyLoadsIntoWebView() { - let webView = WKWebView() - let userContentController = webView.configuration.userContentController - XCTAssertTrue(userContentController.userScripts.isEmpty) - - let expectation = expectation(description: "Wait for completion.") - Bridge.initialize(webView) { - XCTAssertEqual(userContentController.userScripts.count, 1) - expectation.fulfill() - } - - wait(for: [expectation], timeout: .expectationTimeout) - } - - func testInitWithTheSameWebViewDoesNotLoadTwice() { - let webView = WKWebView() - let userContentController = webView.configuration.userContentController - XCTAssertTrue(userContentController.userScripts.isEmpty) - - let expectation1 = expectation(description: "Wait for completion.") - Bridge.initialize(webView) { - XCTAssertEqual(userContentController.userScripts.count, 1) - expectation1.fulfill() - } - - let expectation2 = expectation(description: "Wait for completion.") - - Bridge.initialize(webView) { - XCTAssertEqual(userContentController.userScripts.count, 1) - expectation2.fulfill() - } - - wait(for: [expectation1, expectation2], timeout: .expectationTimeout) - } /// NOTE: Each call to `webView.evaluateJavaScript(String)` will throw an error. /// We intentionally disregard any thrown errors (`try? await bridge...`)