Add ability to obtain result of Authentication process #12
Conversation
Define new closure callback to get result of authentication operation
Update test case for AuthenticationResult
|
This is awesome, thank you! I will look more closely soon. |
| @@ -20,7 +20,8 @@ public enum AuthenticatorError: Error { | |||
| /// Manage state required to executed authenticated URLRequests. | |||
| public final class Authenticator { | |||
| public typealias UserAuthenticator = (URL, String) async throws -> URL | |||
|
|
|||
| public typealias AuthenticationResult = (Login?, AuthenticatorError?) -> Void | |||
There was a problem hiding this comment.
Two questions for this type.
First, what do you think about making the parameter Result<Login, AuthenticatorError>? This would help to ease the optional juggling.
Second, I keep coming back to the name AuthenticationResult. What do you think about something like DidLogin or LoginHandler?
There was a problem hiding this comment.
-
Not sure why but I never use the
Resulttype. However I think this is much better. Will make the change. -
I feel that there are a lot of
Provider/Handlerlike types in the package. Do you thinkDidLoginwould go against the naming convention? AlsoLoginHandlerfeels that you need to handle the login which is not the case. It's only a reporting mechanism.
What aboutAuthenticationStatusHandleror something like that?
There was a problem hiding this comment.
-
Great! It took me a while to warm up to
Resulttoo. And, I often now prefer throwing, since it fits in much better with concurrency. But in this case, it does seem like a good fit. -
Yeah, I didn't think too hard, but you are making a good point. That name works for me!
…StatusHandler Update README.md to showcase new usage Add new test to validate that error in AuthenticationStatusHandler is properly propagated
Closes #11.