From dbf8a3f3543d088b731deedc7c3f66c34164c27e Mon Sep 17 00:00:00 2001
From: Kelvin Kwan <kelvinkwan@google.com>
Date: Thu, 13 Feb 2020 16:47:21 +0000
Subject: Make sure GlobalSearch and Recents can load correct data

Bug: 148270755
Test: manual
Test: atest DocumentsUIGoogleTests

Change-Id: I66450aed4a80dbdbc8abc04b5c913f6bad192b2c
---
 src/com/android/documentsui/ModelId.java           | 54 ++++------------------
 .../documentsui/MultiRootDocumentsLoader.java      | 42 ++++++++---------
 .../documentsui/roots/RootCursorWrapper.java       | 11 +++++
 .../com/android/documentsui/testing/TestEnv.java   |  6 +--
 .../com/android/documentsui/testing/TestModel.java |  2 +-
 .../dirlist/DirectoryAddonsAdapterTest.java        |  4 +-
 .../documentsui/files/ActionHandlerTest.java       |  7 +--
 7 files changed, 48 insertions(+), 78 deletions(-)

diff --git a/src/com/android/documentsui/ModelId.java b/src/com/android/documentsui/ModelId.java
index e178c6318..78e059a95 100644
--- a/src/com/android/documentsui/ModelId.java
+++ b/src/com/android/documentsui/ModelId.java
@@ -1,68 +1,30 @@
 package com.android.documentsui;
 
+import static com.android.documentsui.base.DocumentInfo.getCursorInt;
 import static com.android.documentsui.base.DocumentInfo.getCursorString;
 
 import android.database.Cursor;
-import android.net.Uri;
 import android.provider.DocumentsContract;
-import android.util.Log;
 
-import com.android.documentsui.base.DocumentInfo;
+import com.android.documentsui.base.UserId;
 import com.android.documentsui.roots.RootCursorWrapper;
 
-import java.util.ArrayList;
-import java.util.List;
-
 public class ModelId {
-    private final static String TAG = "ModelId";
-
-    public static final String build(Uri uri) {
-        String documentId;
-        try {
-            documentId = DocumentsContract.getDocumentId(uri);
-        } catch (IllegalArgumentException e) {
-            Log.e(TAG, "Failed to get document id.", e);
-            return null;
-        }
-        String authority;
-        authority = uri.getAuthority();
-        return ModelId.build(authority, documentId);
-    }
-
-    public static final String build(DocumentInfo docInfo) {
-        if (docInfo == null) {
-            return null;
-        }
-        return ModelId.build(docInfo.authority, docInfo.documentId);
-    }
 
     public static final String build(Cursor cursor) {
         if (cursor == null) {
             return null;
         }
-        return ModelId.build(getCursorString(cursor, RootCursorWrapper.COLUMN_AUTHORITY),
+        return ModelId.build(UserId.of(getCursorInt(cursor, RootCursorWrapper.COLUMN_USER_ID)),
+                getCursorString(cursor, RootCursorWrapper.COLUMN_AUTHORITY),
                 getCursorString(cursor, DocumentsContract.Document.COLUMN_DOCUMENT_ID));
     }
 
-    public static final ArrayList<String> build(ArrayList<Uri> uris) {
-        if (uris == null || uris.isEmpty()) {
-            return null;
-        }
-        ArrayList<String> ids = new ArrayList<>();
-        String id;
-        for (Uri uri : uris) {
-            id = ModelId.build(uri);
-            if (id != null) {
-                ids.add(id);
-            }
-        }
-        return ids;
-    }
-
-    public static final String build(String authority, String docId) {
-        if (authority == null || authority.isEmpty() || docId == null || docId.isEmpty()) {
+    public static final String build(UserId userId, String authority, String docId) {
+        if (userId == null || authority == null || authority.isEmpty() || docId == null
+                || docId.isEmpty()) {
             return null;
         }
-        return authority + "|" + docId;
+        return userId + "|" + authority + "|" + docId;
     }
 }
diff --git a/src/com/android/documentsui/MultiRootDocumentsLoader.java b/src/com/android/documentsui/MultiRootDocumentsLoader.java
index f93c8a4a6..711a81ca0 100644
--- a/src/com/android/documentsui/MultiRootDocumentsLoader.java
+++ b/src/com/android/documentsui/MultiRootDocumentsLoader.java
@@ -330,10 +330,6 @@ public abstract class MultiRootDocumentsLoader extends AsyncTaskLoader<Directory
         }
         FileUtils.closeQuietly(mResult);
         mResult = null;
-
-        if (mObserver != null) {
-            getContext().getContentResolver().unregisterContentObserver(mObserver);
-        }
     }
 
     // TODO: create better transfer of ownership around cursor to ensure its
@@ -402,17 +398,17 @@ public abstract class MultiRootDocumentsLoader extends AsyncTaskLoader<Directory
                 return;
             }
 
-            ContentProviderClient client = null;
-            try {
-                client = DocumentsApplication.acquireUnstableProviderOrThrow(
-                        getContext().getContentResolver(), authority);
-
-                final int rootInfoCount = rootInfos.size();
-                final Cursor[] res = new Cursor[rootInfoCount];
-                mCursors = new Cursor[rootInfoCount];
-
-                for (int i = 0; i < rootInfoCount; i++) {
-                    final Uri uri = getQueryUri(rootInfos.get(i));
+            final int rootInfoCount = rootInfos.size();
+            final Cursor[] res = new Cursor[rootInfoCount];
+            mCursors = new Cursor[rootInfoCount];
+
+            for (int i = 0; i < rootInfoCount; i++) {
+                final RootInfo rootInfo = rootInfos.get(i);
+                try (ContentProviderClient client =
+                             DocumentsApplication.acquireUnstableProviderOrThrow(
+                                     rootInfo.userId.getContentResolver(getContext()),
+                                     authority)) {
+                    final Uri uri = getQueryUri(rootInfo);
                     try {
                         final Bundle queryArgs = new Bundle();
                         mState.sortModel.addQuerySortArgs(queryArgs);
@@ -421,17 +417,14 @@ public abstract class MultiRootDocumentsLoader extends AsyncTaskLoader<Directory
                         if (mObserver != null) {
                             res[i].registerContentObserver(mObserver);
                         }
-                        mCursors[i] = generateResultCursor(rootInfos.get(i), res[i]);
+                        mCursors[i] = generateResultCursor(rootInfo, res[i]);
                     } catch (Exception e) {
-                        Log.w(TAG, "Failed to load " + authority + ", " + rootInfos.get(i).rootId,
-                                e);
+                        Log.w(TAG, "Failed to load " + authority + ", " + rootInfo.rootId, e);
                     }
-                }
 
-            } catch (Exception e) {
-                Log.w(TAG, "Failed to acquire content resolver for authority: " + authority);
-            } finally {
-                FileUtils.closeQuietly(client);
+                } catch (Exception e) {
+                    Log.w(TAG, "Failed to acquire content resolver for authority: " + authority);
+                }
             }
 
             set(mCursors);
@@ -449,6 +442,9 @@ public abstract class MultiRootDocumentsLoader extends AsyncTaskLoader<Directory
             }
 
             for (Cursor cursor : mCursors) {
+                if (mObserver != null && cursor != null) {
+                    cursor.unregisterContentObserver(mObserver);
+                }
                 FileUtils.closeQuietly(cursor);
             }
 
diff --git a/src/com/android/documentsui/roots/RootCursorWrapper.java b/src/com/android/documentsui/roots/RootCursorWrapper.java
index f11452d7e..b40972e3a 100644
--- a/src/com/android/documentsui/roots/RootCursorWrapper.java
+++ b/src/com/android/documentsui/roots/RootCursorWrapper.java
@@ -17,6 +17,7 @@
 package com.android.documentsui.roots;
 
 import android.database.AbstractCursor;
+import android.database.ContentObserver;
 import android.database.Cursor;
 import android.os.Bundle;
 import android.util.Log;
@@ -161,4 +162,14 @@ public class RootCursorWrapper extends AbstractCursor {
     public boolean isNull(int column) {
         return mCursor.isNull(column);
     }
+
+    @Override
+    public void registerContentObserver(ContentObserver observer) {
+        mCursor.registerContentObserver(observer);
+    }
+
+    @Override
+    public void unregisterContentObserver(ContentObserver observer) {
+        mCursor.unregisterContentObserver(observer);
+    }
 }
diff --git a/tests/common/com/android/documentsui/testing/TestEnv.java b/tests/common/com/android/documentsui/testing/TestEnv.java
index a75c0034c..d36a74aec 100644
--- a/tests/common/com/android/documentsui/testing/TestEnv.java
+++ b/tests/common/com/android/documentsui/testing/TestEnv.java
@@ -176,7 +176,7 @@ public class TestEnv {
 
     public void populateStack() {
         DocumentInfo rootDoc = model.getDocument(
-                ModelId.build(TestProvidersAccess.HOME.authority, "1"));
+                ModelId.build(model.mUserId, TestProvidersAccess.HOME.authority, "1"));
 
         // These are test setup sanity checks, not test assertions.
         assert rootDoc != null;
@@ -197,7 +197,7 @@ public class TestEnv {
 
     public void selectDocument(DocumentInfo info) {
         List<String> ids = new ArrayList<>(1);
-        ids.add(ModelId.build(info.authority, info.documentId));
+        ids.add(ModelId.build(info.userId, info.authority, info.documentId));
         selectionMgr.setItemsSelected(ids, true);
     }
 
@@ -205,7 +205,7 @@ public class TestEnv {
         List<String> ids = new ArrayList<>(count);
         for (int i = 0; i < count; i++) {
             DocumentInfo info = model.createFile(String.valueOf(i));
-            ids.add(ModelId.build(info.authority, info.documentId));
+            ids.add(ModelId.build(info.userId, info.authority, info.documentId));
         }
         selectionMgr.setItemsSelected(ids, true);
     }
diff --git a/tests/common/com/android/documentsui/testing/TestModel.java b/tests/common/com/android/documentsui/testing/TestModel.java
index 70a836923..2fc012e5f 100644
--- a/tests/common/com/android/documentsui/testing/TestModel.java
+++ b/tests/common/com/android/documentsui/testing/TestModel.java
@@ -43,7 +43,7 @@ public class TestModel extends Model {
         Document.COLUMN_MIME_TYPE
     };
 
-    private final UserId mUserId;
+    public final UserId mUserId;
     private final String mAuthority;
     private int mLastId = 0;
     private Random mRand = new Random();
diff --git a/tests/unit/com/android/documentsui/dirlist/DirectoryAddonsAdapterTest.java b/tests/unit/com/android/documentsui/dirlist/DirectoryAddonsAdapterTest.java
index c2c24eaa9..030c8b147 100644
--- a/tests/unit/com/android/documentsui/dirlist/DirectoryAddonsAdapterTest.java
+++ b/tests/unit/com/android/documentsui/dirlist/DirectoryAddonsAdapterTest.java
@@ -74,10 +74,10 @@ public class DirectoryAddonsAdapterTest extends AndroidTestCase {
         mEnv.model.createFile("b");  // id will be "2"
         mEnv.model.update();
 
-        assertEquals(0, mAdapter.getPosition(ModelId.build(AUTHORITY, "1")));
+        assertEquals(0, mAdapter.getPosition(ModelId.build(mEnv.model.mUserId, AUTHORITY, "1")));
         // adapter inserts a view between item 0 and 1 to force layout
         // break between folders and files. This is reflected by an offset position.
-        assertEquals(2, mAdapter.getPosition(ModelId.build(AUTHORITY, "2")));
+        assertEquals(2, mAdapter.getPosition(ModelId.build(mEnv.model.mUserId, AUTHORITY, "2")));
     }
 
     // Tests that the item count is correct for a directory containing only subdirs.
diff --git a/tests/unit/com/android/documentsui/files/ActionHandlerTest.java b/tests/unit/com/android/documentsui/files/ActionHandlerTest.java
index 1951ff6f9..45a0740f2 100644
--- a/tests/unit/com/android/documentsui/files/ActionHandlerTest.java
+++ b/tests/unit/com/android/documentsui/files/ActionHandlerTest.java
@@ -567,9 +567,10 @@ public class ActionHandlerTest {
         refreshAnswer = false;
         mEnv.populateStack();
         mHandler.refreshDocument(mEnv.model.getDocument(
-                ModelId.build(TestProvidersAccess.HOME.authority, "1")), (boolean answer) -> {
-            refreshAnswer = answer;
-        });
+                ModelId.build(mEnv.model.mUserId, TestProvidersAccess.HOME.authority, "1")),
+                (boolean answer) -> {
+                    refreshAnswer = answer;
+                });
 
         mEnv.beforeAsserts();
         if (mEnv.features.isContentRefreshEnabled()) {
-- 
cgit v1.2.3-59-g8ed1b