Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[ios_platform_images] migrate objC to swift #4847

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
ad9ed35
[ios_platform_images] migrate objC to swift
Mairramer Sep 5, 2023
e5545bc
pump version
Mairramer Sep 5, 2023
cf21182
added some tests
Mairramer Sep 6, 2023
008d5d3
update tests
Mairramer Sep 8, 2023
e422e78
Merge branch 'main' into ios_platform_images_swift_migration
Mairramer Sep 8, 2023
b0122cf
fix format
Mairramer Sep 8, 2023
54b9b92
Merge branch 'main' into ios_platform_images_swift_migration
Mairramer Sep 9, 2023
6c29139
Merge branch 'main' into ios_platform_images_swift_migration
Mairramer Sep 10, 2023
f3ce031
improve code
Mairramer Sep 11, 2023
100202a
rollback comment
Mairramer Sep 11, 2023
97dc083
pump version
Mairramer Sep 11, 2023
1a28d20
Merge branch 'main' into ios_platform_images_swift_migration
Mairramer Sep 13, 2023
8da12bd
rollback implementation
Mairramer Sep 13, 2023
50ce448
rollback original logic
Mairramer Sep 14, 2023
59d7b6d
fix logic
Mairramer Sep 14, 2023
17a369e
rollback logic
Mairramer Sep 14, 2023
cfbf55f
Merge branch 'main' into ios_platform_images_swift_migration
Mairramer Sep 14, 2023
101c190
fix test
Mairramer Sep 14, 2023
1d2969b
fix test
Mairramer Sep 14, 2023
372ce11
fix scaled image and migrate tests
Mairramer Oct 13, 2023
523a277
Merge remote-tracking branch 'origin/main' into ios_platform_images_s…
Mairramer Oct 13, 2023
aef7985
migrate pigeon to swift
Mairramer Oct 13, 2023
78cbc82
format code
Mairramer Oct 13, 2023
45d185d
fix build script
Mairramer Oct 13, 2023
a9a4879
improves
Mairramer Oct 13, 2023
364be83
update readme
Mairramer Oct 13, 2023
aae7a29
rollback objC test
Mairramer Oct 17, 2023
cbbc208
fix test import
Mairramer Oct 17, 2023
ac54592
improves
Mairramer Oct 17, 2023
3e66718
improve test
Mairramer Oct 17, 2023
71f2589
fix test
Mairramer Oct 17, 2023
e2d9a9a
Merge branch 'main' into ios_platform_images_swift_migration
Mairramer Oct 17, 2023
3324ccd
rollback
Mairramer Oct 17, 2023
e370d61
update readme
Mairramer Oct 17, 2023
b25d574
Revert whitespace change in podfile
stuartmorgan Oct 20, 2023
5d03976
TODO style fix
stuartmorgan Oct 20, 2023
eaf7c90
Re-add DEFINES_MODULE
stuartmorgan Oct 20, 2023
139e125
Merge branch 'main' into ios_platform_images_swift_migration
stuartmorgan Oct 20, 2023
6ea1fc0
Merge branch 'main' into ios_platform_images_swift_migration
Mairramer Oct 20, 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
1 change: 1 addition & 0 deletions packages/ios_platform_images/AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,4 @@ Aleksandr Yurkovskiy <[email protected]>
Anton Borries <[email protected]>
Alex Li <[email protected]>
Rahul Raj <[email protected]>
Mairramer <[email protected]>
4 changes: 4 additions & 0 deletions packages/ios_platform_images/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.3.0
Mairramer marked this conversation as resolved.
Show resolved Hide resolved

* Migrate objC to Swift.
Mairramer marked this conversation as resolved.
Show resolved Hide resolved

## 0.2.2+2

* Adds pub topics to package metadata.
Expand Down
5 changes: 1 addition & 4 deletions packages/ios_platform_images/example/ios/Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ require File.expand_path(File.join('packages', 'flutter_tools', 'bin', 'podhelpe
flutter_ios_podfile_setup

target 'Runner' do
use_frameworks!
use_modular_headers!
Mairramer marked this conversation as resolved.
Show resolved Hide resolved

flutter_install_all_ios_pods File.dirname(File.realpath(__FILE__))
target 'RunnerTests' do
inherit! :search_paths
Expand All @@ -41,4 +38,4 @@ post_install do |installer|
installer.pods_project.targets.each do |target|
flutter_additional_ios_build_settings(target)
end
end
end
Mairramer marked this conversation as resolved.
Show resolved Hide resolved

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>IDEDidComputeMac32BitWarning</key>
Mairramer marked this conversation as resolved.
Show resolved Hide resolved
<true/>
</dict>
</plist>
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@
ReferencedContainer = "container:Runner.xcodeproj">
</BuildableReference>
</BuildActionEntry>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "NO"
buildForProfiling = "NO"
buildForArchiving = "NO"
buildForAnalyzing = "NO">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "F76AC1BD266713D00040C8BC"
BuildableName = "RunnerTests.xctest"
BlueprintName = "RunnerTests"
ReferencedContainer = "container:Runner.xcodeproj">
</BuildableReference>
</BuildActionEntry>
</BuildActionEntries>
</BuildAction>
<TestAction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import UIKit
import Flutter
import UIKit

@UIApplicationMain
@objc class AppDelegate: FlutterAppDelegate {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import Flutter
import XCTest

@testable import ios_platform_images

class IosPlatformImagesTests: XCTestCase {

var mockChannel: MockMethodChannel!
Mairramer marked this conversation as resolved.
Show resolved Hide resolved

override func setUp() {
super.setUp()
mockChannel = MockMethodChannel()
}

func testHandleMethodCall_loadImage() {
let name = "flutter"
Mairramer marked this conversation as resolved.
Show resolved Hide resolved

Mairramer marked this conversation as resolved.
Show resolved Hide resolved
let call = FlutterMethodCall(methodName: "loadImage", arguments: [name])

let mockChannel = MockMethodChannel()

let plugin = IosPlatformImagesPlugin(channel: mockChannel)

let resultExpectation = expectation(description: "result block must be called.")

plugin.handle(call) { result in
let result = result as? [String: Any]
XCTAssertNotNil(result)

let scale = result?["scale"] as? CGFloat
let data = result?["data"] as? FlutterStandardTypedData

XCTAssertNotNil(scale)
XCTAssertNotNil(data)

resultExpectation.fulfill()
}

waitForExpectations(timeout: 2, handler: nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Flutter style prefers not using timeouts in tests, as they contribute to flake. This should just wait until the expectation is fulfilled.

Same in all of the tests below.

}

func testHandleMethodCall_loadImage_notFound() {
let name = "flutterNotFound"

let call = FlutterMethodCall(methodName: "loadImage", arguments: [name])

let mockChannel = MockMethodChannel()

let plugin = IosPlatformImagesPlugin(channel: mockChannel)

let resultExpectation = expectation(description: "result block must be called.")

plugin.handle(call) { result in
let result = result as? [String: Any]

XCTAssertNil(result)
resultExpectation.fulfill()
}

waitForExpectations(timeout: 2, handler: nil)
}

func testHandleMethodCall_resolveURL() {
let name = "textfile"

let call = FlutterMethodCall(methodName: "resolveURL", arguments: [name])

let mockChannel = MockMethodChannel()

let plugin = IosPlatformImagesPlugin(channel: mockChannel)

let resultExpectation = expectation(description: "result block must be called.")

plugin.handle(call) { result in
let result = result as? String
XCTAssertNotNil(result)
XCTAssertEqual(result?.components(separatedBy: "/").last, name)
resultExpectation.fulfill()
}

waitForExpectations(timeout: 1, handler: nil)
}

func testHandleMethodCall_resolveURL_notFound() {
let name = "notFound"

let call = FlutterMethodCall(methodName: "resolveURL", arguments: [name])

let mockChannel = MockMethodChannel()

let plugin = IosPlatformImagesPlugin(channel: mockChannel)

let resultExpectation = expectation(description: "result block must be called.")

plugin.handle(call) { result in
XCTAssertNil(result)
resultExpectation.fulfill()
}

waitForExpectations(timeout: 1, handler: nil)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import Foundation

@testable import ios_platform_images

final class MockMethodChannel: MethodChannel {
var invokeMethodStub: ((_ methods: String, _ arguments: Any?) -> Void)? = nil
func invokeMethod(_ method: String, arguments: Any?) {
invokeMethodStub?(method, arguments)
}
}

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import Flutter
import Foundation

public final class IosPlatformImagesPlugin: NSObject, FlutterPlugin {

private let channel: MethodChannel
Mairramer marked this conversation as resolved.
Show resolved Hide resolved

init(channel: MethodChannel) {
self.channel = channel
}

public static func register(with registrar: FlutterPluginRegistrar) {
let channel = FlutterMethodChannel(
name: "plugins.flutter.io/ios_platform_images",
binaryMessenger: registrar.messenger())

let instance = IosPlatformImagesPlugin(channel: channel)
registrar.addMethodCallDelegate(instance, channel: channel)
}

public func handle(_ call: FlutterMethodCall, result: @escaping FlutterResult) {
switch call.method {
case "loadImage":
loadImage(call, result)
case "resolveURL":
resolveURL(call, result)
default:
result(FlutterMethodNotImplemented)
}
}

private func loadImage(_ call: FlutterMethodCall, _ result: @escaping FlutterResult) {
guard let arguments = call.arguments as? [Any],
Mairramer marked this conversation as resolved.
Show resolved Hide resolved
let name = arguments.first as? String,
Mairramer marked this conversation as resolved.
Show resolved Hide resolved
let image = UIImage.flutterImage(withName: name)
else {
result(nil)
return
}

let data = image.pngData()

let imageResult: [String: Any] = [
"scale": image.scale,
"data": FlutterStandardTypedData(bytes: data!),
Mairramer marked this conversation as resolved.
Show resolved Hide resolved
]

result(imageResult)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of the intermediate imageResult variable, rather than inlining the result?

}

private func resolveURL(_ call: FlutterMethodCall, _ result: @escaping FlutterResult) {
if let args = call.arguments as? [String?] {
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, this should not fail silently.

let name = args[0]
let extensionName = args.count == 2 ? args[1] : nil
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the same logic as the Obj-C version, and is not correct. There will always be two arguments, but the second may be NSNull.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I change to avoid Fatal error: Index out of range

Copy link
Contributor

Choose a reason for hiding this comment

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

In what context? This is the call, and it passes two arguments.

If you are referring to the unit tests you added, the solution is to make the unit tests match actual usage. Having unit tests that call the method in a way that production code never will, and then changing the implementation to work in the unit test but break production, makes the unit tests actively harmful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw here in another test the reason for your comment, I will make the appropriate change


if let url = Bundle.main.url(forResource: name, withExtension: extensionName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the explicit branching, rather than optional chaining like the Obj-C version? The more result calls there are, the harder it is to make sure the logic is correctly calling it exactly one time.

result(url.absoluteString)
} else {
result(nil)
}
} else {
result(nil)
}
}
}
16 changes: 16 additions & 0 deletions packages/ios_platform_images/ios/Classes/MethodChannel.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import Flutter

/// A channel for platform code to communicate with the Dart code.
protocol MethodChannel {
Mairramer marked this conversation as resolved.
Show resolved Hide resolved
/// Invokes a method in Dart code.
/// - Parameter method the method name.
/// - Parameter arguments the method arguments.
func invokeMethod(_ method: String, arguments: Any?)
}

/// A default implementation of the `MethodChannel` protocol.
extension FlutterMethodChannel: MethodChannel {}

This file was deleted.

Loading