Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
1d5609f
converting url_launcher from objc to swift
chrisdlangham Aug 18, 2023
d5727dd
converting tests to swift
chrisdlangham Aug 22, 2023
c4e55fb
converting tests to swift
chrisdlangham Aug 22, 2023
050d6b3
fixing issue where pigeon and method channels were not setup right
chrisdlangham Aug 22, 2023
5278199
formatting
chrisdlangham Aug 22, 2023
55a345b
reverting unintentional local changes
chrisdlangham Aug 22, 2023
8356d7a
fixing issues with Ui tests
chrisdlangham Aug 22, 2023
120b2d4
formating
chrisdlangham Aug 22, 2023
1316f3c
Merge branch 'main' into coverting-objc-to-swift
chrisdlangham Aug 22, 2023
7bd3553
updating version and change log
chrisdlangham Aug 22, 2023
2741de4
converting tests to swift
chrisdlangham Aug 22, 2023
495de26
converting unit tests to swift
chrisdlangham Aug 23, 2023
c898698
Merge branch 'main' into converting-url-launcher-ios-tests-to-swift
chrisdlangham Aug 23, 2023
6a6c8d5
updating change log
chrisdlangham Aug 23, 2023
254b9af
resolving merge conflicts
chrisdlangham Aug 23, 2023
869f18f
formatting
chrisdlangham Aug 23, 2023
1120baa
making test class final and private
chrisdlangham Aug 29, 2023
9f591e3
resolving merge conflicts
chrisdlangham Sep 13, 2023
13cd002
updated tests and formated pigeon file
chrisdlangham Sep 13, 2023
f8f5f0b
updates changelog
chrisdlangham Sep 13, 2023
827204a
Update CHANGELOG.md
chrisdlangham Sep 14, 2023
30568fd
Merge branch 'main' into coverting-objc-to-swift
chrisdlangham Sep 14, 2023
6b5e4cd
uses latest version of pigeon
chrisdlangham Sep 15, 2023
3651a97
updates change log
chrisdlangham Sep 15, 2023
257b7cc
moves setting up the pigeon api to the register function instead of t…
chrisdlangham Sep 15, 2023
3ee9d96
resolving merge conflicts
chrisdlangham Sep 28, 2023
d90702f
Merge branch 'main' into coverting-objc-to-swift
chrisdlangham Oct 7, 2023
7241d20
adds in missing throws keyword
chrisdlangham Oct 7, 2023
9d578f2
addresses feedback
chrisdlangham Oct 7, 2023
657c011
changes pigeon api to not throw errors, and let the dart side throw e…
chrisdlangham Oct 11, 2023
7a2fac5
Merge branch 'main' into coverting-objc-to-swift
chrisdlangham Oct 19, 2023
4d4f1bb
addresses feedback
chrisdlangham Oct 19, 2023
7b6272f
addressing feedback
chrisdlangham Oct 26, 2023
b91dca7
Merge branch 'main' into coverting-objc-to-swift
stuartmorgan-g Oct 26, 2023
e7e011a
Replace default launcher implementation with conformance extension
stuartmorgan-g Oct 26, 2023
d955ef8
swift-format
stuartmorgan-g Oct 26, 2023
1297683
Rework return enum to have different versions
stuartmorgan-g Oct 26, 2023
43966a0
Improve invalid URL testing
stuartmorgan-g Oct 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
addresses feedback
  • Loading branch information
chrisdlangham committed Oct 19, 2023
commit 4d4f1bb7938f7206d84525f28c1c9cb6c445cfa7
Original file line number Diff line number Diff line change
Expand Up @@ -20,34 +20,28 @@ final class URLLauncherTests: XCTestCase {

func testCanLaunchSuccess() {
let result = createPlugin().canLaunchUrl(url: "good://url")
XCTAssertEqual(result.result, .success)
XCTAssertEqual(result, .success)
}

func testCanLaunchFailure() {
let result = createPlugin().canLaunchUrl(url: "bad://url")
XCTAssertEqual(result.result, .failure)
XCTAssertEqual(result, .failedToLoad)
}

func testCanLaunchFailureWithInvalidURL() {
let result = createPlugin().canLaunchUrl(url: "urls can't have spaces")
if result.result == .failure {
// When linking against the iOS 17 SDK or later, NSURL uses a lenient parser, and won't
// fail to parse URLs, so the test must allow for either outcome.
XCTAssertNil(result.errorMessage)
XCTAssertNil(result.errorDetails)
} else {
XCTAssertEqual(result.result, .invalidUrl)
XCTAssertEqual(result.errorMessage, "Unable to parse URL")
XCTAssertEqual(result.errorDetails, "Provided URL: urls can't have spaces")
}

// When linking against the iOS 17 SDK or later, NSURL uses a lenient parser, and won't
// fail to parse URLs, so the test must allow for either outcome.
XCTAssertTrue(result == .failedToLoad || result == .invalidUrl)
}

func testLaunchSuccess() {
let expectation = XCTestExpectation(description: "completion called")
createPlugin().launchUrl(url: "good://url", universalLinksOnly: false) { result in
switch result {
case .success(let details):
XCTAssertEqual(details.result, .success)
XCTAssertEqual(details, .success)
case .failure(let error):
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -62,7 +56,7 @@ final class URLLauncherTests: XCTestCase {
createPlugin().launchUrl(url: "bad://url", universalLinksOnly: false) { result in
switch result {
case .success(let details):
XCTAssertEqual(details.result, .failure)
XCTAssertEqual(details, .failedToLoad)
case .failure(let error):
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -77,16 +71,9 @@ final class URLLauncherTests: XCTestCase {
createPlugin().launchUrl(url: "urls can't have spaces", universalLinksOnly: false) { result in
switch result {
case .success(let details):
if details.result == .failure {
// When linking against the iOS 17 SDK or later, NSURL uses a lenient parser, and won't
// fail to parse URLs, so the test must allow for either outcome.
XCTAssertNil(details.errorMessage)
XCTAssertNil(details.errorDetails)
} else {
XCTAssertEqual(details.result, .invalidUrl)
XCTAssertEqual(details.errorMessage, "Unable to parse URL")
XCTAssertEqual(details.errorDetails, "Provided URL: urls can't have spaces")
}
// When linking against the iOS 17 SDK or later, NSURL uses a lenient parser, and won't
// fail to parse URLs, so the test must allow for either outcome.
XCTAssertTrue(details == .failedToLoad || details == .invalidUrl)
Copy link
Contributor

Choose a reason for hiding this comment

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

which case will it actually be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on the iOS version the tests are being run on. I think @stuartmorgan can explain it better, but on iOS 17 we should get .failedToLoad, and on older versions, we should get .invalidUrl

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you do "if iOS 17 else".

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately Swift apparently has no equivalent to Obj-C SDK preprocessor checks, and that's the determining factor. We don't want to switch on whether it's running on iOS 17, we want to switch on whether it was linked against the iOS 17 SDK, and that's apparently not expressible in Swift. (The best I can find is people suggesting mapping from the SDK to the version of Xcode that introduced that SDK, and then from there to the version of Swift introduced by that version of Xcode, but that's extremely gross since this has nothing to do with Swift language version.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I realized there's a better option; I've added a utility method in the test file that checks the NSURL behavior directly, and then switches based on that in the tests.

case .failure(let error):
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -104,7 +91,7 @@ final class URLLauncherTests: XCTestCase {
plugin.launchUrl(url: "good://url", universalLinksOnly: false) { result in
switch result {
case .success(let details):
XCTAssertEqual(details.result, .success)
XCTAssertEqual(details, .success)
case .failure(let error):
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -123,7 +110,7 @@ final class URLLauncherTests: XCTestCase {
plugin.launchUrl(url: "good://url", universalLinksOnly: true) { result in
switch result {
case .success(let details):
XCTAssertEqual(details.result, .success)
XCTAssertEqual(details, .success)
case .failure(let error):
XCTFail("Unexpected error: \(error)")
}
Expand All @@ -144,7 +131,8 @@ final private class FakeLauncher: NSObject, Launcher {
}

func openURL(
_ url: URL, options: [UIApplication.OpenExternalURLOptionsKey: Any],
_ url: URL,
options: [UIApplication.OpenExternalURLOptionsKey: Any],
completionHandler completion: ((Bool) -> Void)?
) {
self.passedOptions = options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@
///
/// This protocol exists to allow injecting an alternate implementation for testing.
protocol Launcher {
/// Returns a Boolean value that indicates whether an app is available to handle a URL scheme.
func canOpenURL(_ url: URL) -> Bool

/// Attempts to asynchronously open the resource at the specified URL.
func openURL(
_ url: URL, options: [UIApplication.OpenExternalURLOptionsKey: Any],
_ url: URL,
options: [UIApplication.OpenExternalURLOptionsKey: Any],
completionHandler completion: ((Bool) -> Void)?)
}

Expand All @@ -19,7 +23,8 @@ final class UIApplicationLauncher: Launcher {
}

func openURL(
_ url: URL, options: [UIApplication.OpenExternalURLOptionsKey: Any],
_ url: URL,
options: [UIApplication.OpenExternalURLOptionsKey: Any],
completionHandler completion: ((Bool) -> Void)?
) {
UIApplication.shared.open(url, options: options, completionHandler: completion)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import Flutter
import SafariServices

typealias OpenInSafariCompletionHandler = (Result<LaunchResultDetails, Error>) -> Void
typealias OpenInSafariCompletionHandler = (Result<LaunchResult, Error>) -> Void

/// A session responsible for launching a URL in Safari and handling its events.
final class URLLaunchSession: NSObject, SFSafariViewControllerDelegate {
Expand Down Expand Up @@ -41,12 +41,9 @@ final class URLLaunchSession: NSObject, SFSafariViewControllerDelegate {
_ controller: SFSafariViewController, didCompleteInitialLoad didLoadSuccessfully: Bool
) {
if didLoadSuccessfully {
completion(.success(LaunchResultDetails(result: .success)))
completion(.success(.success))
} else {
completion(
.success(
LaunchResultDetails(result: .failedToLoad, errorMessage: "Error while launching \(url)")
))
completion(.success(.failedToLoad))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public final class URLLauncherPlugin: NSObject, FlutterPlugin, UrlLauncherApi {
private var currentSession: URLLaunchSession?
private let launcher: Launcher

var topViewController: UIViewController? {
private var topViewController: UIViewController? {
// TODO(stuartmorgan) Provide a non-deprecated codepath. See
// https://github.com/flutter/flutter/issues/104117
UIApplication.shared.keyWindow?.rootViewController?.topViewController
Expand All @@ -25,34 +25,33 @@ public final class URLLauncherPlugin: NSObject, FlutterPlugin, UrlLauncherApi {
self.launcher = launcher
}

func canLaunchUrl(url: String) -> LaunchResultDetails {
func canLaunchUrl(url: String) -> LaunchResult {
guard let url = URL(string: url) else {
return invalidURLError(for: url)
return .failedToLoad
}
let canOpen = launcher.canOpenURL(url)
return LaunchResultDetails(result: canOpen ? .success : .failure)
return canOpen ? .success : .failedToLoad
}

func launchUrl(
url: String, universalLinksOnly: Bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

line break

completion: @escaping (Result<LaunchResultDetails, Error>) -> Void
completion: @escaping (Result<LaunchResult, Error>) -> Void
) {
guard let url = URL(string: url) else {
completion(.success(invalidURLError(for: url)))
completion(.success(.failedToLoad))
return
}
let options = [UIApplication.OpenExternalURLOptionsKey.universalLinksOnly: universalLinksOnly]
launcher.openURL(url, options: options) { success in
let result = LaunchResultDetails(result: success ? .success : .failure)
completion(.success(result))
launcher.openURL(url, options: options) { result in
completion(.success(result ? .success : .failedToLoad))
}
}

func openUrlInSafariViewController(
url: String, completion: @escaping (Result<LaunchResultDetails, Error>) -> Void
url: String, completion: @escaping (Result<LaunchResult, Error>) -> Void
Copy link
Contributor

Choose a reason for hiding this comment

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

line break. please go over all signatures and make sure comma separated items are in separate lines.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've swift-formated the whole plugin.

) {
guard let url = URL(string: url) else {
completion(.success(invalidURLError(for: url)))
completion(.success(.failedToLoad))
return
}

Expand All @@ -68,18 +67,6 @@ public final class URLLauncherPlugin: NSObject, FlutterPlugin, UrlLauncherApi {
func closeSafariViewController() throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this throwing?

currentSession?.close()
}

/**
* Creates an error for an invalid URL string.
*
* @param url The invalid URL string
* @return The error to return
*/
func invalidURLError(for url: String) -> LaunchResultDetails {
LaunchResultDetails(
result: .invalidUrl, errorMessage: "Unable to parse URL", errorDetails: "Provided URL: \(url)"
)
}
}

/// This method recursively iterates through the view hierarchy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,111 +47,43 @@ enum LaunchResult: Int {
/// The URL was successfully launched.
case success = 0
/// The URL could not be launched
case failure = 1
case failedToLoad = 1
/// The URL was not launched because it is not invalid URL
case invalidUrl = 2
/// The URL did not load successfully in the SFSafariViewController.
case failedToLoad = 3
}

/// Generated class from Pigeon that represents data sent in messages.
struct LaunchResultDetails {
/// The result of the launch attempt.
var result: LaunchResult
/// A system-provided error message, if any.
var errorMessage: String? = nil
/// A system-provided error details, if any.
var errorDetails: String? = nil

static func fromList(_ list: [Any?]) -> LaunchResultDetails? {
let result = LaunchResult(rawValue: list[0] as! Int)!
let errorMessage: String? = nilOrValue(list[1])
let errorDetails: String? = nilOrValue(list[2])

return LaunchResultDetails(
result: result,
errorMessage: errorMessage,
errorDetails: errorDetails
)
}
func toList() -> [Any?] {
return [
result.rawValue,
errorMessage,
errorDetails,
]
}
}

private class UrlLauncherApiCodecReader: FlutterStandardReader {
override func readValue(ofType type: UInt8) -> Any? {
switch type {
case 128:
return LaunchResultDetails.fromList(self.readValue() as! [Any?])
default:
return super.readValue(ofType: type)
}
}
}

private class UrlLauncherApiCodecWriter: FlutterStandardWriter {
override func writeValue(_ value: Any) {
if let value = value as? LaunchResultDetails {
super.writeByte(128)
super.writeValue(value.toList())
} else {
super.writeValue(value)
}
}
}

private class UrlLauncherApiCodecReaderWriter: FlutterStandardReaderWriter {
override func reader(with data: Data) -> FlutterStandardReader {
return UrlLauncherApiCodecReader(data: data)
}

override func writer(with data: NSMutableData) -> FlutterStandardWriter {
return UrlLauncherApiCodecWriter(data: data)
}
}

class UrlLauncherApiCodec: FlutterStandardMessageCodec {
static let shared = UrlLauncherApiCodec(readerWriter: UrlLauncherApiCodecReaderWriter())
}

/// Generated protocol from Pigeon that represents a handler of messages from Flutter.
protocol UrlLauncherApi {
/// Returns true if the URL can definitely be launched.
func canLaunchUrl(url: String) throws -> LaunchResultDetails
func canLaunchUrl(url: String) throws -> LaunchResult
/// Opens the URL externally, returning true if successful.
func launchUrl(
url: String, universalLinksOnly: Bool,
completion: @escaping (Result<LaunchResultDetails, Error>) -> Void)
completion: @escaping (Result<LaunchResult, Error>) -> Void)
/// Opens the URL in an in-app SFSafariViewController, returning true
/// when it has loaded successfully.
func openUrlInSafariViewController(
url: String, completion: @escaping (Result<LaunchResultDetails, Error>) -> Void)
url: String, completion: @escaping (Result<LaunchResult, Error>) -> Void)
/// Closes the view controller opened by [openUrlInSafariViewController].
func closeSafariViewController() throws
}

/// Generated setup class from Pigeon to handle messages through the `binaryMessenger`.
class UrlLauncherApiSetup {
/// The codec used by UrlLauncherApi.
static var codec: FlutterStandardMessageCodec { UrlLauncherApiCodec.shared }
/// Sets up an instance of `UrlLauncherApi` to handle messages through the `binaryMessenger`.
static func setUp(binaryMessenger: FlutterBinaryMessenger, api: UrlLauncherApi?) {
/// Returns true if the URL can definitely be launched.
let canLaunchUrlChannel = FlutterBasicMessageChannel(
name: "dev.flutter.pigeon.url_launcher_ios.UrlLauncherApi.canLaunchUrl",
binaryMessenger: binaryMessenger, codec: codec)
binaryMessenger: binaryMessenger)
if let api = api {
canLaunchUrlChannel.setMessageHandler { message, reply in
let args = message as! [Any?]
let urlArg = args[0] as! String
do {
let result = try api.canLaunchUrl(url: urlArg)
reply(wrapResult(result))
reply(wrapResult(result.rawValue))
} catch {
reply(wrapError(error))
}
Expand All @@ -162,7 +94,7 @@ class UrlLauncherApiSetup {
/// Opens the URL externally, returning true if successful.
let launchUrlChannel = FlutterBasicMessageChannel(
name: "dev.flutter.pigeon.url_launcher_ios.UrlLauncherApi.launchUrl",
binaryMessenger: binaryMessenger, codec: codec)
binaryMessenger: binaryMessenger)
if let api = api {
launchUrlChannel.setMessageHandler { message, reply in
let args = message as! [Any?]
Expand All @@ -171,7 +103,7 @@ class UrlLauncherApiSetup {
api.launchUrl(url: urlArg, universalLinksOnly: universalLinksOnlyArg) { result in
switch result {
case .success(let res):
reply(wrapResult(res))
reply(wrapResult(res.rawValue))
case .failure(let error):
reply(wrapError(error))
}
Expand All @@ -184,15 +116,15 @@ class UrlLauncherApiSetup {
/// when it has loaded successfully.
let openUrlInSafariViewControllerChannel = FlutterBasicMessageChannel(
name: "dev.flutter.pigeon.url_launcher_ios.UrlLauncherApi.openUrlInSafariViewController",
binaryMessenger: binaryMessenger, codec: codec)
binaryMessenger: binaryMessenger)
if let api = api {
openUrlInSafariViewControllerChannel.setMessageHandler { message, reply in
let args = message as! [Any?]
let urlArg = args[0] as! String
api.openUrlInSafariViewController(url: urlArg) { result in
switch result {
case .success(let res):
reply(wrapResult(res))
reply(wrapResult(res.rawValue))
case .failure(let error):
reply(wrapError(error))
}
Expand All @@ -204,7 +136,7 @@ class UrlLauncherApiSetup {
/// Closes the view controller opened by [openUrlInSafariViewController].
let closeSafariViewControllerChannel = FlutterBasicMessageChannel(
name: "dev.flutter.pigeon.url_launcher_ios.UrlLauncherApi.closeSafariViewController",
binaryMessenger: binaryMessenger, codec: codec)
binaryMessenger: binaryMessenger)
if let api = api {
closeSafariViewControllerChannel.setMessageHandler { _, reply in
do {
Expand Down
Loading