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

Update LockCallback and AuthenticationCallback [SDK-2480] #621

Merged
merged 4 commits into from
May 3, 2021

Conversation

lbalmaceda
Copy link
Contributor

Changes

With this change, the AuthenticationCallback is reduced from 3 methods down to 2. Before, the "canceled" event would trigger a onCanceled method call. This method no longer exists and instead, an exception now is raised with the code "a0.authentication_canceled". This is easily testable checking the AuthenticationException#isCanceled() method.

The immediate benefit is accessing the AuthenticationException directly, without having to cast the cause wrapped in the (previously existing) LockException. This helps to consume different error codes and messages that are now going to be received after #618

// Before
val callback: LockCallback = object : AuthenticationCallback() {
    override fun onAuthentication(credentials: Credentials) {
        // Authenticated
    }
    override fun onCanceled() {
        // Canceled
    }
    override fun onError(error: LockException) {
        // Another error. Check code & description.
    }
}
// After
val callback: LockCallback = object : AuthenticationCallback() {
    override fun onAuthentication(credentials: Credentials) {
        // Authenticated
    }
    override fun onError(error: AuthenticationException) {
        if (error.isCanceled) {
            // Canceled
        } else {
            // Another error. Check code & description.
        }
    }
}

This PR includes a breaking change.

References

See SDK-2480

@lbalmaceda lbalmaceda added this to the Major - v3 milestone Apr 29, 2021
@lbalmaceda lbalmaceda requested a review from a team as a code owner April 29, 2021 18:51
/**
* Called when the user goes back and closes the activity, without using an Authentication flow.
*/
public abstract void onCanceled();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AuthenticationCallback is a subclass of LockCallback. The onCanceled is now merged into onError for those developers passing an AuthenticationCallback implementation (what we document and recommend).

case LockEvent.SIGN_UP:
break;
if (data.hasExtra(Constants.EXCEPTION_EXTRA)) {
onError((AuthenticationException) data.getSerializableExtra(Constants.EXCEPTION_EXTRA));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

any exception available would be raised through onError immediately

Credentials credentials = extractCredentials(data);
onAuthentication(credentials);
} else if (event == LockEvent.CANCELED) {
onError(new AuthenticationException("a0.authentication_canceled", "The user pressed back"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the event is "canceled", then raise an exception with the specific code that represents this. This is only for AuthenticationException. LockCallback would still receive this event and is up to the dev to handle (this advanced scenario remains available)

@@ -168,7 +167,7 @@ private void processEvent(@NonNull Intent data) {
break;
case Constants.INVALID_CONFIGURATION_ACTION:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have a "public" event type for this. So we raise an exception instead.

@@ -68,5 +69,5 @@
*
* @param error describing what happened.
*/
void onError(@NonNull LockException error);
void onError(@NonNull AuthenticationException error);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes the type of exception. Helps to deliver the actual exception and not wrapping everything again.

Widcket
Widcket previously approved these changes Apr 29, 2021
Copy link
Contributor

@Widcket Widcket left a comment

Choose a reason for hiding this comment

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

LGTM, just have a question regarding the Readme.


### Removed methods

#### From class `AuthenticationCallback`
- Removed `public void onCanceled()`. Instead, an exception will be raised with this code. Check the property `AuthenticationException#isCanceled()` to handle it.
Copy link
Contributor

Choose a reason for hiding this comment

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

Says "with this code" but doesn't include the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Rephrased this and also noted that this is a method and not a property. Ready for review.

@lbalmaceda lbalmaceda merged commit 1969e59 into v3 May 3, 2021
@lbalmaceda lbalmaceda deleted the bc-lock-callback branch May 3, 2021 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants