Mhd/camera requester crash

[mhdostal]

#1022

Uses async version of AVCaptureDevice.requestAccess(for:).

[rolson]

I'm confused as to why this method is taking 2 closures. Since it's asynchronous can we not just return a boolean for isAuthorized?

[mhdostal]

I'm confused as to why this method is taking 2 closures. Since it's asynchronous can we not just return a boolean for isAuthorized?

There are code paths in the AVCaptureDevice.authorizationStatus switch that do not specifically set isAuthorized. That's why it can't simply return a boolean.

[rolson]

One thing to consider is that isAuthorized is not a return value but a published property:

@Published private(set) var isAuthorized = false

I think that might simplify code looking at where the closures are used?

[dfeinzimer]

I investigated using the proposed published isAuthorized property. At face value it does seem like we could potentially simplify this:

await cameraRequester.request {
    cameraAccessIsAuthorized = true
} onAccessDenied: {
    isPresented = false
}

to this:

await cameraRequester.request()
if cameraRequester.isAuthorized {
    cameraAccessIsAuthorized = true
} else {
    isPresented = false
}

but as Mark noted there is a third possibility we have to account for (when we get AVAuthorizationStatus.denied or AVAuthorizationStatus.restricted back) and CameraRequester needs to remain presented in order to present this alert:

The current callback structure makes it simple to keep CameraRequester presented so that the user can respond to this alert.

With this, my recommendation would be to keep the PR as is.

[rolson]

So maybe an isPublished Bool is not enough, but an enum might be?

[dfeinzimer]

An enum could work yes, but if we use AVAuthorizationStatus directly then the CameraRequester user has four possible outcomes to handle, rather than the current two.

Alternatively, we could use the proposed boolean property (or create our own simplified enumeration) and just set it false or denied once the user has pressed Cancel but this mean the developer now also needs to install a .task(id: cameraRequester.isAuthorized) { ... } elsewhere to monitor the value, scattering permission handling logic into more locations.

[rolson]

Approving. I feel like the closure api is a bit of a strange metaphor for SwiftUI in this case, but maybe I'm just not understanding the nuances here.

[dfeinzimer]

Approving. I feel like the closure api is a bit of a strange metaphor for SwiftUI in this case, but maybe I'm just not understanding the nuances here.

Agreed, I think the strangeness is rooted in the fact that the class has an async method and it's used in conjunction with a view modifier that may or may not ever need to present an alert. The good news is that this is all internal so I've opened #1077 so we can revisit this later on.