From 50b19e5d7733fe27b4e7de96b390a140c8ea9f4d Mon Sep 17 00:00:00 2001 From: Chris Narkiewicz Date: Thu, 28 May 2020 01:23:31 +0100 Subject: [PATCH] Refactored deep link handling Signed-off-by: Chris Narkiewicz --- .../client/files/DeepLinkHandlerTest.kt | 99 +++++++++++-------- .../nextcloud/client/files/DeepLinkHandler.kt | 34 ++++--- .../ui/activity/FileDisplayActivity.java | 71 +++++-------- 3 files changed, 104 insertions(+), 100 deletions(-) diff --git a/src/androidTest/java/com/nextcloud/client/files/DeepLinkHandlerTest.kt b/src/androidTest/java/com/nextcloud/client/files/DeepLinkHandlerTest.kt index 0456eff8fb..b1826f460b 100644 --- a/src/androidTest/java/com/nextcloud/client/files/DeepLinkHandlerTest.kt +++ b/src/androidTest/java/com/nextcloud/client/files/DeepLinkHandlerTest.kt @@ -1,57 +1,74 @@ package com.nextcloud.client.files +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Before import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import org.junit.runners.Suite +@RunWith(Suite::class) +@Suite.SuiteClasses( + DeepLinkHandlerTest.DeepLinkPattern::class +) class DeepLinkHandlerTest { - @Test - fun valid_uri_can_be_handled_by_one_user() { - // GIVEN - // uri matching allowed pattern - // one user can open the file + @RunWith(Parameterized::class) + class DeepLinkPattern { - // WHEN - // deep link is handled + companion object { + val FILE_ID = 1234 + val SERVER_BASE_URLS = listOf( + "http://hostname.net", + "https://hostname.net", + "http://hostname.net/subdir1", + "https://hostname.net/subdir1", + "http://hostname.net/subdir1/subdir2", + "https://hostname.net/subdir1/subdir2", + "http://hostname.net/subdir1/subdir2/subdir3", + "https://hostname.net/subdir1/subdir2/subdir3" + ) + val INDEX_PHP_PATH = listOf( + "", + "/index.php" + ) - // THEN - // file is opened immediately - } + @Parameterized.Parameters + @JvmStatic + fun urls(): Array> { + val testInput = mutableListOf>() + SERVER_BASE_URLS.forEach { baseUrl -> + INDEX_PHP_PATH.forEach { + indexPath -> + val url = "$baseUrl$indexPath/f/$FILE_ID" + testInput.add(arrayOf(baseUrl, indexPath, "$FILE_ID", url)) + } + } + return testInput.toTypedArray() + } + } - @Test - fun valid_uri_can_be_handled_by_multiple_users() { - // GIVEN - // uri matching allowed pattern - // multiple users can open the file + @Parameterized.Parameter(0) + lateinit var baseUrl: String - // WHEN - // deep link is handled + @Parameterized.Parameter(1) + lateinit var indexPath: String - // THEN - // user chooser dialog is opened - } + @Parameterized.Parameter(2) + lateinit var fileId: String - @Test - fun valid_uri_cannot_be_handled_by_any_user() { - // GIVEN - // uri matching allowed pattern - // no user can open given uri + @Parameterized.Parameter(3) + lateinit var url: String - // WHEN - // deep link is handled + @Test + fun matches_deep_link_patterns() { + val match = DeepLinkHandler.DEEP_LINK_PATTERN.matchEntire(url) + assertNotNull("Url [$url] does not match pattern", match) + assertEquals(baseUrl, match?.groupValues?.get(DeepLinkHandler.BASE_URL_GROUP_INDEX)) + assertEquals(indexPath, match?.groupValues?.get(DeepLinkHandler.INDEX_PATH_GROUP_INDEX)) + assertEquals(fileId, match?.groupValues?.get(DeepLinkHandler.FILE_ID_GROUP_INDEX)) + } - // THEN - // deep link is ignored - } - - @Test - fun invalid_uri_is_ignored() { - // GIVEN - // file uri does not match allowed pattern - - // WHEN - // deep link is handled - - // THEN - // deep link is ignored } } diff --git a/src/main/java/com/nextcloud/client/files/DeepLinkHandler.kt b/src/main/java/com/nextcloud/client/files/DeepLinkHandler.kt index e140d9ef0f..14b80227e3 100644 --- a/src/main/java/com/nextcloud/client/files/DeepLinkHandler.kt +++ b/src/main/java/com/nextcloud/client/files/DeepLinkHandler.kt @@ -4,25 +4,31 @@ import android.content.Context import android.net.Uri import com.nextcloud.client.account.User import com.nextcloud.client.account.UserAccountManager +import java.util.regex.Pattern class DeepLinkHandler( private val context: Context, - private val userAccountManager: UserAccountManager, - private val onUserChoiceRequired: (users: List, fileId: String)->Unit + private val userAccountManager: UserAccountManager ) { - /** - * Open deep link. - * - * If deep link can be opened immediately, new activity is launched. - * If link can be handled by multiple users, [onUserChoiceRequired] callback - * is invoked with list of matching users. - * - * @param uri Deep link received in incoming [Intent] - * @return true if deep link can be handled - */ - fun openDeepLink(uri: Uri): Boolean { - throw NotImplementedError() + data class Match(val serverBaseUrl: String, val fileId: String) + + companion object { + val DEEP_LINK_PATTERN = Regex("""(.*?)(/index\.php)?/f/([0-9]+)$""") + val BASE_URL_GROUP_INDEX = 1 + val INDEX_PATH_GROUP_INDEX = 2 + val FILE_ID_GROUP_INDEX = 3 + } + + fun parseDeepLink(uri: Uri): Match? { + val match = DEEP_LINK_PATTERN.matchEntire(uri.toString()) + if (match != null) { + val baseServerUrl = match.groupValues[BASE_URL_GROUP_INDEX] + val fielId = match.groupValues[FILE_ID_GROUP_INDEX] + return Match(baseServerUrl, fielId) + } else { + return null + } } } diff --git a/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java b/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java index d4033e16b5..1fb3b7d22b 100644 --- a/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java +++ b/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java @@ -52,6 +52,7 @@ import com.google.android.material.snackbar.Snackbar; import com.nextcloud.client.account.User; import com.nextcloud.client.appinfo.AppInfo; import com.nextcloud.client.di.Injectable; +import com.nextcloud.client.files.DeepLinkHandler; import com.nextcloud.client.media.PlayerServiceConnection; import com.nextcloud.client.network.ConnectivityService; import com.nextcloud.client.preferences.AppPreferences; @@ -2408,53 +2409,32 @@ public class FileDisplayActivity extends FileActivity String fileId = intent.getStringExtra(KEY_FILE_ID); if (userName == null && fileId == null && intent.getData() != null) { - // Handle intent coming from URI - - Pattern pattern1 = Pattern.compile("(.*)/index\\.php/([f])/([0-9]+)$"); - Pattern pattern2 = Pattern.compile("(.*)/([f])/([0-9]+)$"); - Matcher matcher1 = pattern1.matcher(intent.getData().toString()); - Matcher matcher2 = pattern2.matcher(intent.getData().toString()); - if (matcher1.matches()) { - String uri = matcher1.group(1); - if ("f".equals(matcher1.group(2))) { - fileId = matcher1.group(3); - findAccountAndOpenFile(uri, fileId); - return; - } - } else if (matcher2.matches()) { - String uri = matcher2.group(1); - if ("f".equals(matcher2.group(2))) { - fileId = matcher2.group(3); - findAccountAndOpenFile(uri, fileId); - return; - } + openDeepLink(intent.getData()); + } else { + Optional optionalUser = userName == null ? getUser() : getUserAccountManager().getUser(userName); + if (optionalUser.isPresent()) { + openFile(optionalUser.get(), fileId); } else { dismissLoadingDialog(); - DisplayUtils.showSnackMessage(this, getString(R.string.invalid_url)); - return; + DisplayUtils.showSnackMessage(this, getString(R.string.associated_account_not_found)); } } - openFile(userName, fileId); - } - private void openFile(String userName, String fileId) { - Optional optionalNewUser; - User user; - if (userName == null) { - optionalNewUser = getUser(); - } else { - optionalNewUser = getUserAccountManager().getUser(userName); - } - - if (optionalNewUser.isPresent()) { - user = optionalNewUser.get(); - setUser(user); + private void openDeepLink(Uri uri) { + DeepLinkHandler linkHandler = new DeepLinkHandler(getApplicationContext(), getUserAccountManager()); + DeepLinkHandler.Match match = linkHandler.parseDeepLink(uri); + if (match != null) { + findAccountAndOpenFile(match.getServerBaseUrl(), match.getFileId()); } else { dismissLoadingDialog(); - DisplayUtils.showSnackMessage(this, getString(R.string.associated_account_not_found)); - return; + DisplayUtils.showSnackMessage(this, getString(R.string.invalid_url)); } + } + + private void openFile(User user, String fileId) { + setUser(user); + updateAccountList(); if (fileId == null) { dismissLoadingDialog(); @@ -2493,7 +2473,7 @@ public class FileDisplayActivity extends FileActivity } if (validUsers.size() == 1) { - openFile(validUsers.get(0).getAccountName(), fileId); + openFile(validUsers.get(0), fileId); return; } @@ -2507,12 +2487,13 @@ public class FileDisplayActivity extends FileActivity builder .setTitle(R.string.common_choose_account) .setItems(validUserNames.toArray(new CharSequence[validUserNames.size()]), - new DialogInterface.OnClickListener() { - public void onClick(DialogInterface dialog, int which) { - openFile(validUsers.get(which).getAccountName(), fileId); - showLoadingDialog(getString(R.string.retrieving_file)); - } - }); + (dialog, which) -> { + // TODO: refactor to use User model directly + String accountName = validUsers.get(which).getAccountName(); + User user = getUserAccountManager().getUser(accountName).orElseThrow(RuntimeException::new); + openFile(user, fileId); + showLoadingDialog(getString(R.string.retrieving_file)); + }); AlertDialog dialog = builder.create(); dismissLoadingDialog(); dialog.show();