Merge pull request #6121 from nextcloud/enhance_deep_links

enhance deep linking to allow pretty urls

scripts/deleteOutdatedComments.sh

@@ -13,3 +13,5 @@ oldComments=$(curl 2>/dev/null -u $GITHUB_USER:$GITHUB_PASSWORD -X GET https://a
 echo $oldComments | while read comment ; do
     curl 2>/dev/null -u $GITHUB_USER:$GITHUB_PASSWORD -X DELETE https://api.github.com/repos/nextcloud/android/issues/comments/$comment
 done
+
+exit 0

src/androidTest/java/com/nextcloud/client/files/DeepLinkHandlerTest.kt

@@ -0,0 +1,235 @@
+/**
+ * Nextcloud Android client application
+ *
+ * @author Chris Narkiewicz
+ * Copyright (C) 2020 Chris Narkiewicz <hello@ezaquarii.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package com.nextcloud.client.files
+
+import android.net.Uri
+import com.nextcloud.client.account.Server
+import com.nextcloud.client.account.User
+import com.nextcloud.client.account.UserAccountManager
+import com.nhaarman.mockitokotlin2.mock
+import com.nhaarman.mockitokotlin2.whenever
+import com.owncloud.android.lib.resources.status.OwnCloudVersion
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertSame
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+import org.junit.runners.Suite
+import org.mockito.Mock
+import org.mockito.MockitoAnnotations
+import java.net.URI
+
+@RunWith(Suite::class)
+@Suite.SuiteClasses(
+    DeepLinkHandlerTest.DeepLinkPattern::class,
+    DeepLinkHandlerTest.FileDeepLink::class
+)
+class DeepLinkHandlerTest {
+
+    @RunWith(Parameterized::class)
+    class DeepLinkPattern {
+
+        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"
+            )
+
+            @Parameterized.Parameters
+            @JvmStatic
+            fun urls(): Array<Array<Any>> {
+                val testInput = mutableListOf<Array<Any>>()
+                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()
+            }
+        }
+
+        @Parameterized.Parameter(0)
+        lateinit var baseUrl: String
+
+        @Parameterized.Parameter(1)
+        lateinit var indexPath: String
+
+        @Parameterized.Parameter(2)
+        lateinit var fileId: String
+
+        @Parameterized.Parameter(3)
+        lateinit var url: String
+
+        @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))
+        }
+
+        @Test
+        fun no_trailing_path_allowed_after_file_id() {
+            val invalidUrl = "$url/"
+            val match = DeepLinkHandler.DEEP_LINK_PATTERN.matchEntire(invalidUrl)
+            assertNull(match)
+        }
+    }
+
+    class FileDeepLink {
+
+        companion object {
+            const val OTHER_SERVER_BASE_URL = "https://someotherserver.net"
+            const val SERVER_BASE_URL = "https://server.net"
+            const val FILE_ID = "1234567890"
+            val DEEP_LINK = Uri.parse("$SERVER_BASE_URL/index.php/f/$FILE_ID")
+
+            fun createMockUser(serverBaseUrl: String): User {
+                val user = mock<User>()
+                val uri = URI.create(serverBaseUrl)
+                val server = Server(uri = uri, version = OwnCloudVersion.nextcloud_19)
+                whenever(user.server).thenReturn(server)
+                return user
+            }
+        }
+
+        @Mock
+        lateinit var userAccountManager: UserAccountManager
+        lateinit var allUsers: List<User>
+        lateinit var handler: DeepLinkHandler
+
+        @Before
+        fun setUp() {
+            MockitoAnnotations.initMocks(this)
+            whenever(userAccountManager.allUsers).thenAnswer { allUsers }
+            allUsers = emptyList()
+            handler = DeepLinkHandler(userAccountManager)
+        }
+
+        @Test
+        fun no_user_can_open_file() {
+            // GIVEN
+            //      no user capable of opening the file
+            allUsers = listOf(
+                createMockUser(OTHER_SERVER_BASE_URL),
+                createMockUser(OTHER_SERVER_BASE_URL)
+            )
+
+            // WHEN
+            //      deep link is parsed
+            val match = handler.parseDeepLink(DEEP_LINK)
+
+            // THEN
+            //      link is valid
+            //      no user can open the file
+            assertNotNull(match)
+            assertEquals(0, match?.users?.size)
+        }
+
+        @Test
+        fun single_user_can_open_file() {
+            // GIVEN
+            //      multiple users registered
+            //      one user capable of opening the link
+            val matchingUser = createMockUser(SERVER_BASE_URL)
+            allUsers = listOf(
+                createMockUser(OTHER_SERVER_BASE_URL),
+                matchingUser,
+                createMockUser(OTHER_SERVER_BASE_URL)
+            )
+
+            // WHEN
+            //      deep link is parsed
+            val match = handler.parseDeepLink(DEEP_LINK)
+
+            // THEN
+            //      link can be opened by single user
+            assertNotNull(match)
+            assertSame(matchingUser, match?.users?.get(0))
+        }
+
+        @Test
+        fun multiple_users_can_open_file() {
+            // GIVEN
+            //      mutltiple users registered
+            //      multiple users capable of opening the link
+            val matchingUsers = setOf(
+                createMockUser(SERVER_BASE_URL),
+                createMockUser(SERVER_BASE_URL)
+            )
+            val otherUsers = setOf(
+                createMockUser(OTHER_SERVER_BASE_URL),
+                createMockUser(OTHER_SERVER_BASE_URL)
+            )
+            allUsers = listOf(matchingUsers, otherUsers).flatten()
+
+            // WHEN
+            //      deep link is parsed
+            val match = handler.parseDeepLink(DEEP_LINK)
+
+            // THEN
+            //      link can be opened by multiple matching users
+            assertNotNull(match)
+            assertEquals(matchingUsers, match?.users?.toSet())
+        }
+
+        @Test
+        fun match_contains_extracted_file_id() {
+            // WHEN
+            //      valid deep file link is parsed
+            val match = handler.parseDeepLink(DEEP_LINK)
+
+            // THEN
+            //      file id is returned
+            assertEquals(FILE_ID, match?.fileId)
+        }
+
+        @Test
+        fun no_match_for_invalid_link() {
+            // GIVEN
+            //      invalid deep link
+            val invalidLink = Uri.parse("http://www.dodgylink.com/index.php")
+
+            // WHEN
+            //      deep link is parsed
+            val match = handler.parseDeepLink(invalidLink)
+
+            // THEN
+            //      no match
+            assertNull(match)
+        }
+    }
+}

src/main/AndroidManifest.xml

@@ -121,14 +121,13 @@
                 <action android:name="android.intent.action.VIEW" />
                 <category android:name="android.intent.category.DEFAULT" />
                 <category android:name="android.intent.category.BROWSABLE" />
-                <data android:scheme="http"
-                    android:host="*"
-                    android:pathPattern="/.*index.php/f/.*"
-                    />
-                <data android:scheme="https"
-                      android:host="*"
-                      android:pathPattern="/.*index.php/f/.*"
-                    />
+                <data android:scheme="http" />
+                <data android:scheme="https" />
+                <data android:host="*" />
+                <data android:pathPattern="/f/..*" />
+                <data android:pathPattern="/..*/f/..*" />
+                <data android:pathPattern="/..*/..*/f/..*" />
+                <data android:pathPattern="/..*/..*/..*/f/..*" />
             </intent-filter>
             <meta-data android:name="android.app.searchable"
                 android:resource="@xml/users_and_groups_searchable"/>

src/main/java/com/nextcloud/client/files/DeepLinkHandler.kt

@@ -0,0 +1,71 @@
+/**
+ * Nextcloud Android client application
+ *
+ * @author Chris Narkiewicz
+ * Copyright (C) 2020 Chris Narkiewicz <hello@ezaquarii.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+package com.nextcloud.client.files
+
+import android.net.Uri
+import com.nextcloud.client.account.User
+import com.nextcloud.client.account.UserAccountManager
+
+/**
+ * This component parses and matches deep links.
+ * Result is returned to the UI for further processing.
+ *
+ * TODO: This is intermediate refactring step; this component should be moved into
+ *       [com.nextcloud.client.mixins.ActivityMixin] and handle UI callbacks as well
+ */
+class DeepLinkHandler(
+    private val userAccountManager: UserAccountManager
+) {
+
+    /**
+     * Provide parsed link arguments and context information required
+     * to launch it.
+     */
+    data class Match(val users: List<User>, 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
+    }
+
+    /**
+     * Parse deep link and return a match result.
+     * Matching result may depend on environmental factors, such
+     * as app version or registered users.
+     *
+     * @param uri Deep link as arrived in incoming [android.content.Intent]
+     * @return deep link match result with all context data required for further processing; null if link does not match
+     */
+    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 fileId = match.groupValues[FILE_ID_GROUP_INDEX]
+            return Match(users = getMatchingUsers(baseServerUrl), fileId = fileId)
+        } else {
+            return null
+        }
+    }
+
+    private fun getMatchingUsers(serverBaseUrl: String): List<User> =
+        userAccountManager.allUsers.filter { it.server.uri.toString() == serverBaseUrl }
+}

src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.java

@@ -38,6 +38,7 @@ import android.content.IntentFilter;
 import android.content.ServiceConnection;
 import android.content.pm.PackageManager;
 import android.content.res.Resources.NotFoundException;
+import android.net.Uri;
 import android.os.Bundle;
 import android.os.IBinder;
 import android.os.Parcelable;
@@ -52,6 +53,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;
@@ -123,8 +125,6 @@ import java.io.File;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import javax.annotation.Nullable;
 import javax.inject.Inject;
@@ -2291,12 +2291,6 @@ public class FileDisplayActivity extends FileActivity
             transaction.replace(R.id.left_fragment_container, fragment, TAG_LIST_OF_FILES);
             transaction.commit();
         }
-//        else {
-//            Log_OC.d(this, "Switch to OCFileListFragment");
-//
-//            fragment = new OCFileListFragment();
-//        }
-
     }
 
     @Subscribe(threadMode = ThreadMode.MAIN)
@@ -2408,44 +2402,54 @@ 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 pattern  = Pattern.compile("(.*)/index\\.php/([f])/([0-9]+)$");
-            Matcher matcher = pattern.matcher(intent.getData().toString());
-            if (matcher.matches()) {
-                String uri = matcher.group(1);
-                if ("f".equals(matcher.group(2))) {
-                    fileId = matcher.group(3);
-                    findAccountAndOpenFile(uri, fileId);
-                    return;
-                }
+            openDeepLink(intent.getData());
+        } else {
+            Optional<User> 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<User> optionalNewUser;
-        User user;
 
-        if (userName == null) {
-            optionalNewUser = getUser();
+    private void openDeepLink(Uri uri) {
+        DeepLinkHandler linkHandler = new DeepLinkHandler(getUserAccountManager());
+        DeepLinkHandler.Match match = linkHandler.parseDeepLink(uri);
+        if (match == null) {
+            dismissLoadingDialog();
+            DisplayUtils.showSnackMessage(this, getString(R.string.invalid_url));
+        } else if (match.getUsers().isEmpty()) {
+            dismissLoadingDialog();
+            DisplayUtils.showSnackMessage(this, getString(R.string.associated_account_not_found));
+        } else if (match.getUsers().size() == 1) {
+            openFile(match.getUsers().get(0), match.getFileId());
         } else {
-            optionalNewUser = getUserAccountManager().getUser(userName);
+            selectUserAndOpenFile(match.getUsers(), match.getFileId());
         }
+    }
 
-        if (optionalNewUser.isPresent()) {
-            user = optionalNewUser.get();
-            setUser(user);
-        } else {
-            dismissLoadingDialog();
-            DisplayUtils.showSnackMessage(this, getString(R.string.associated_account_not_found));
-            return;
+    private void selectUserAndOpenFile(List<User> users, String fileId) {
+        final CharSequence[] userNames = new CharSequence[users.size()];
+        for (int i = 0; i < userNames.length; i++) {
+            userNames[i] = users.get(i).getAccountName();
         }
+        final AlertDialog.Builder builder = new AlertDialog.Builder(this);
+        builder
+            .setTitle(R.string.common_choose_account)
+            .setItems(userNames, (dialog, which) -> {
+                User user = users.get(which);
+                openFile(user, fileId);
+                showLoadingDialog(getString(R.string.retrieving_file));
+            });
+        final AlertDialog dialog = builder.create();
+        dismissLoadingDialog();
+        dialog.show();
+    }
+
+    private void openFile(User user, String fileId) {
+        setUser(user);
 
         if (fileId == null) {
             dismissLoadingDialog();
@@ -2466,46 +2470,4 @@ public class FileDisplayActivity extends FileActivity
         fetchRemoteFileTask.execute();
 
     }
-
-    private void findAccountAndOpenFile(String uri, String fileId) {
-
-        ArrayList<User> validUsers = new ArrayList<>();
-
-        for (User user : getUserAccountManager().getAllUsers()) {
-            if (user.getServer().getUri().toString().equals(uri)) {
-                validUsers.add(user);
-            }
-        }
-
-        if (validUsers.size() == 0) {
-            dismissLoadingDialog();
-            DisplayUtils.showSnackMessage(this, getString(R.string.associated_account_not_found));
-            return;
-        }
-
-        if (validUsers.size() == 1) {
-            openFile(validUsers.get(0).getAccountName(), fileId);
-            return;
-        }
-
-        ArrayList<String> validUserNames = new ArrayList<>();
-
-        for (User user : validUsers) {
-            validUserNames.add(user.getAccountName());
-        }
-
-        AlertDialog.Builder builder = new AlertDialog.Builder(getActivity());
-        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));
-                }
-            });
-        AlertDialog dialog = builder.create();
-        dismissLoadingDialog();
-        dialog.show();
-    }
 }