Android-SingleSignOn icon indicating copy to clipboard operation
Android-SingleSignOn copied to clipboard

Support Activity Result API

Open bubelov opened this issue 3 years ago • 3 comments

Here is how I integrate NC SSO:

override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
    super.onActivityResult(requestCode, resultCode, data)

    when (resultCode) {
        AppCompatActivity.RESULT_CANCELED -> setButtonsEnabled(true)

        else -> {
            AccountImporter.onActivityResult(
                requestCode,
                resultCode,
                data,
                this,
            ) { onNextcloudAccountAccessGranted(it) }
        }
    }
}

It works fine, except for showing deprecation warnings:

'onActivityResult(Int, Int, Intent?): Unit' is deprecated. Deprecated in Java

This method has been deprecated in favor of using the Activity Result API which brings increased type safety via an ActivityResultContract and the prebuilt contracts for common intents available in androidx.activity.result.contract.ActivityResultContracts, provides hooks for testing, and allow receiving results in separate, testable classes independent from your fragment. Use registerForActivityResult(ActivityResultContract, ActivityResultCallback) with the appropriate ActivityResultContract and handling the result in the callback.

It looks like the modern way to get a SingleSignOnAccount is to create an ActivityResultContract<Nothing, SingleSignOnAccount> or something like that and to ship it with the library. Are there any reasons for not supporting Activity Result API except for the lack of dev time?

bubelov avatar Apr 17 '22 08:04 bubelov

Are there any reasons for not supporting Activity Result API except for the lack of dev time?

In short: Nope. Maybe you want to start a Pull Request? I will happily review it 🙂

stefan-niedermann avatar Apr 17 '22 12:04 stefan-niedermann

@stefan-niedermann I don't have a NC instance at the moment, so it would be hard to test the code. Do you know any reliable hosted services? I tried a couple from the official website, one of them was freezing for hours and another one wasn't able to send me an email confirmation for some reason =)

bubelov avatar Apr 17 '22 16:04 bubelov

Do you know any reliable hosted services?

For testing around just use one of those free hosters.

So, I played a bit around with the idea of ActivityResultContracts. I think, we need three different ActivityResultContracts:

  1. CHOOSE_ACCOUNT_SSOChooseAccountContract
  2. REQUEST_AUTH_TOKEN_SSORequestAuthTokenContract
  3. REQUEST_GET_ACCOUNTS_PERMISSIONRequestGetAccountsPermissionContract

Further thoughts:

  1. Do 3rd party apps really need to control the import aspect this detailed? Wouldn't it be better to provide just one ImportSingleSignOnAccountContract which handles the rest of the stuff under the hood?
  2. While implementing ActivityResultContract I stumbled upon the problem, that one needs to override createIntent and parseResult - both methods do not throw any exception according their signature. This leads to the problem, that we can't check and throw NextcloudFilesAppNotInstalledException for example without wrapping it into a RuntimeException or extending SSOException RuntimeException - I don't like any of those approaches. Another idea is to do the check within a constructor, but this again leads to an ugly static initialization from the 3rd party apps perspective:
private final ActivityResultLauncher<Void> launcher;

{
  try {
      launcher = registerForActivityResult(
              new ImportAccountContract(this),
              (ActivityResultCallback<SingleSignOnAccount>) this::onActivityResult
      );
  } catch (NextcloudFilesAppNotInstalledException e) {
      throw new RuntimeException(e);
  }
}

stefan-niedermann avatar May 20 '23 13:05 stefan-niedermann