Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

fix(iOS): Making permissions switch statements exhaustive & supporting new iOS 14 cases #3160

Merged
merged 7 commits into from
Jul 1, 2020
Merged

fix(iOS): Making permissions switch statements exhaustive & supporting new iOS 14 cases #3160

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2f14c57
Cleaned up permissions enum handling. Added unknown default handlers …
ikeith Jun 25, 2020
7479349
Missed a type declaration
ikeith Jun 25, 2020
78fc230
Update ios/Capacitor/Capacitor/Plugins/Permissions.swift
ikeith Jun 25, 2020
ac4bea8
Changing @unknown default to be a failure case.
ikeith Jun 30, 2020
56ffb24
Merge branch 'permission-enums' of github.com:ionic-team/capacitor in…
ikeith Jun 30, 2020
08310b9
Merge branch 'master' into permission-enums
ikeith Jun 30, 2020
4e1bd25
Merge branch 'master' into permission-enums
jcesarmobile Jul 1, 2020
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
65 changes: 43 additions & 22 deletions ios/Capacitor/Capacitor/Plugins/Permissions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@ public class CAPPermissionsPlugin: CAPPlugin {
func checkCamera(_ call: CAPPluginCall) {
let authStatus = AVCaptureDevice.authorizationStatus(for: .video)

var ret = "prompt"
let ret: String
switch (authStatus) {
case .notDetermined:
ret = "prompt"
case .denied, .restricted:
ret = "denied"
case .authorized:
ret = "granted"
case .notDetermined:
fallthrough
@unknown default:
ret = "prompt"
}

call.resolve([
Expand All @@ -52,31 +54,42 @@ public class CAPPermissionsPlugin: CAPPlugin {

func checkPhotos(_ call: CAPPluginCall) {
let photoAuthorizationStatus = PHPhotoLibrary.authorizationStatus()

var ret = "prompt"
let ret: String
switch (photoAuthorizationStatus) {
case .notDetermined:
ret = "prompt"
case .denied, .restricted:
ret = "denied"
case .authorized:
ret = "granted"
case .denied, .restricted:
ret = "denied"
case .authorized:
ret = "granted"
#if swift(>=5.3)
Copy link
Contributor

Choose a reason for hiding this comment

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

The new enum cases are not bound to Swift 5.3. They are added in iOS 14 and the availability check should check for os version instead.

Copy link
Member

Choose a reason for hiding this comment

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

As far as I know, they are only available on iOS 14 if you use SDK 14 to compile (Xcode 12), if you use SDK 13 (Xcode 11) then they won't be available because it works in a sort of "compatibility mode".

The #if swift(>=5.3) is only true on Xcode 12, it's false in Xcode 11 because it ships with swift 5.1 to 5.2.4.

I agree that using if #available(iOS 14, *) would be more clear, but sadly, it can't be used inside the switch.

Maybe is less confusing if we use #if compiler(>=5.3) instead of #if swift(>=5.3)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is true that language version is not synonymous with the SDK version but in practice it's not an issue. The only way it would be a problem is if you downloaded a newer version of the Swift compiler and replaced the toolchain in Xcode 11. If you did that, you should know what you're doing and can expect side effects in many places.

Not only is #available invalid inside a switch statement (so we'd need to have duplicated switches to use it), it doesn't actually solve our problem because that's a hook to insert a runtime check, meaning the new enum case would fail to compile under Xcode 11. There is no compiler flag that I'm aware of to strip code based on the SDK or OS version being targeted.

At this time, we need to be able to build cleanly with Xcode 11 and 12. Ultimately, this is only temporary for some months until 12 becomes the default.

case .limited:
// TODO: address this new case properly
#warning(".limited != .authorized, authorization status should be revisted for iOS 14")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In addition to the new case, authorizationStatus() is being deprecated in favor of authorizationStatus(for:). The implications of .limited aren't fully clear to me yet so I'm adding this warning to be sure that it gets revisited before the beta period is over since it might require changes across more of Capacitor. But this addresses the immediate problem of compiles breaking with Xcode 12 until that time.

ikeith marked this conversation as resolved.
Show resolved Hide resolved
ret = "granted"
#endif
case .notDetermined:
fallthrough
@unknown default:
ret = "prompt"
}

call.resolve([
"state": ret
])
}

func checkGeolocation(_ call: CAPPluginCall) {
var ret = "prompt"
let ret: String
if CLLocationManager.locationServicesEnabled() {
switch CLLocationManager.authorizationStatus() {
case .notDetermined:
ret = "prompt"
case .denied, .restricted:
ret = "denied"
case .authorizedAlways, .authorizedWhenInUse:
ret = "granted"
case .notDetermined:
fallthrough
@unknown default:
ret = "prompt"
}
} else {
ret = "denied"
Expand All @@ -89,13 +102,19 @@ public class CAPPermissionsPlugin: CAPPlugin {

func checkNotifications(_ call: CAPPluginCall) {
UNUserNotificationCenter.current().getNotificationSettings(completionHandler: { settings in
var ret = "prompt"
let ret: String
switch settings.authorizationStatus {
case .authorized, .provisional:
ret = "granted"
#if swift(>=5.3)
case .ephemeral:
ret = "granted"
#endif
case .denied:
ret = "denied"
case .notDetermined:
fallthrough
@unknown default:
ret = "prompt"
}

Expand All @@ -114,14 +133,16 @@ public class CAPPermissionsPlugin: CAPPlugin {
func checkMicrophone(_ call: CAPPluginCall) {
let microStatus = AVCaptureDevice.authorizationStatus(for: .audio)

var ret = "prompt"
let ret: String
switch (microStatus) {
case .authorized:
ret = "granted"
case .denied, .restricted:
ret = "denied"
case .notDetermined:
ret = "prompt"
case .authorized:
ret = "granted"
case .denied, .restricted:
ret = "denied"
case .notDetermined:
fallthrough
@unknown default:
ret = "prompt"
}

call.resolve([
Expand Down