Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3e7f4c79 authored by Bo Majewski's avatar Bo Majewski
Browse files

[DocsUI, Search]: Fix result duplicates. 

When searching everywhere, we were including external storage provider
and media and downloads proviers. Those last two (or more like 4)
replicate results that can be located via external storage provider.
This is now removed, for search everywhere. Various tests are fixed and
expanded.

Bug: 416107009
Test: DocumentsUIUnitTests:com.android.documentsui.base.SearchLoaderTest
Test: DocumentsUIUnitTests:com.android.documentsui.base.SearchViewManagerTest
Flag: com.android.documentsui.flags.use_search_v2_read_only

Change-Id: Id55fc56c318ddbfa06fe953953fbd2f46b092449
parent 25240f01

src/com/android/documentsui/base/FolderInfo.kt

+1 −0
Original line number Diff line number Diff line
@@ -23,5 +23,6 @@ package com.android.documentsui.base 
 * DocumentInfo.

 */

data class FolderInfo(val folderId: String, val authority: String) {

    constructor(folder: DocumentInfo) : this(folder.documentId, folder.authority)

    constructor(rootInfo: RootInfo) : this(rootInfo.rootId, rootInfo.authority)

}

src/com/android/documentsui/queries/SearchViewManager.java

+25 −10
Original line number Diff line number Diff line
@@ -56,6 +56,7 @@ import com.android.documentsui.base.DocumentInfo; 
import com.android.documentsui.base.DocumentStack;

import com.android.documentsui.base.EventHandler;

import com.android.documentsui.base.FolderInfo;

import com.android.documentsui.base.Providers;

import com.android.documentsui.base.RootInfo;

import com.android.documentsui.base.Shared;

import com.android.documentsui.base.State;
@@ -63,6 +64,7 @@ import com.android.modules.utils.build.SdkLevel; 


import java.util.ArrayList;

import java.util.Collection;

import java.util.Collections;

import java.util.Timer;

import java.util.TimerTask;

import java.util.function.Predicate;
@@ -854,21 +856,34 @@ public class SearchViewManager implements 
            folderList = roots.stream().filter(filter).map(FolderInfo::new).collect(

                    Collectors.toList());

        } else if (mLocationOption != null) {

            RootInfo root = stack.getRoot();

            if (root == null) {

                return Collections.emptyList();

            }

            DocumentInfo topFolder = stack.peek();

            switch (mLocationOption) {

                case CURRENT_FOLDER: {

                case CURRENT_FOLDER:

                    // Fall-through.

                    // TODO(b:391232249): Searching with stack.peek().documentId does not work.

                case ROOT_FOLDER: {

                    if (Providers.AUTHORITY_DOWNLOADS.equals(root.authority) && topFolder != null) {

                        folderList.add(new FolderInfo(topFolder));

                    } else {

                        // Here we are searching with rootId, even though we are suppose to search

                        // in the current folder. This needs to be fixed.

                    folderList.add(new FolderInfo(stack.getRoot()));

                    break;

                        folderList.add(new FolderInfo(root));

                    }

                case ROOT_FOLDER:

                    folderList.add(new FolderInfo(stack.getRoot()));

                    break;

                case EVERYWHERE:

                    folderList = roots.stream().filter(baseFilter).map(FolderInfo::new).collect(

                            Collectors.toList());

                }

                case EVERYWHERE: {

                    // When searching locally, to avoid duplicates, do not rely on MediaStore or

                    // DownloadStorageProvider.

                    folderList = roots.stream().filter(baseFilter.and(

                            r -> !Providers.AUTHORITY_MEDIA.equals(r.authority)

                                    && !Providers.AUTHORITY_DOWNLOADS.equals(r.authority))).map(

                            FolderInfo::new).collect(Collectors.toList());

                    break;

                }

                default:

                    throw new IllegalStateException(

                            "Unhandled location option " + mLocationOption.name());

tests/functional/com/android/documentsui/SearchViewUiTest.java

+2 −2
Original line number Diff line number Diff line
@@ -383,7 +383,7 @@ public class SearchViewUiTest extends ActivityTestJunit4<FilesActivity> { 
    public void testSearchV2SearchLocationDropdown() throws Exception {

        // Start search with term "file1", but rather than searching locally, search everywhere.

        bots.search.expand();

        bots.search.setInputText("file");

        bots.search.setInputText("fred-dog.jpg");

        bots.keyboard.pressEnter();

        onView(withId(R.id.search_location_trigger)).perform(click());

        onView(withText(R.string.search_location_everywhere)).inRoot(isPlatformPopup()).perform(
@@ -392,7 +392,7 @@ public class SearchViewUiTest extends ActivityTestJunit4<FilesActivity> { 
        // Silence subsequent warnings about device being potentially null.

        Assert.assertNotNull(device);

        device.waitForIdle();

        bots.directory.assertDocumentsCountOnList(true, 3);

        bots.directory.assertDocumentsCountOnList(true, 1);

    }



    @Test

tests/unit/com/android/documentsui/loaders/SearchLoaderTest.kt

+12 −2
Original line number Diff line number Diff line
@@ -95,7 +95,12 @@ class SearchLoaderTest(private val testParams: LoaderTestParams) : BaseLoaderTes 
        )



        val folderInfo =

            listOf(FolderInfo(TestProvidersAccess.DOWNLOADS.authority, userIds[0].toString()))

            listOf(

                FolderInfo(

                    TestProvidersAccess.DOWNLOADS.rootId,

                    TestProvidersAccess.DOWNLOADS.authority

                )

            )



        val loader = SearchLoader(

            mActivity,
@@ -118,7 +123,12 @@ class SearchLoaderTest(private val testParams: LoaderTestParams) : BaseLoaderTes 
    fun testBlankQueryAndRecency() {

        val userIds = listOf(TestProvidersAccess.DOWNLOADS.userId)

        val folderInfo =

            listOf(FolderInfo(TestProvidersAccess.DOWNLOADS.authority, userIds[0].toString()))

            listOf(

                FolderInfo(

                    TestProvidersAccess.DOWNLOADS.rootId,

                    TestProvidersAccess.DOWNLOADS.authority

                )

            )

        val noLastModifiedQueryOptions =

            QueryOptions(10, null, null, true, arrayOf("*/*"), Bundle())

tests/unit/com/android/documentsui/queries/SearchViewManagerTest.java

+36 −2
Original line number Diff line number Diff line
@@ -53,6 +53,8 @@ import com.android.documentsui.R; 
import com.android.documentsui.base.DocumentInfo;

import com.android.documentsui.base.DocumentStack;

import com.android.documentsui.base.EventHandler;

import com.android.documentsui.base.FolderInfo;

import com.android.documentsui.base.Providers;

import com.android.documentsui.base.RootInfo;

import com.android.documentsui.flags.Flags;

import com.android.documentsui.queries.SearchViewManager.SearchManagerListener;
@@ -70,7 +72,9 @@ import org.junit.runner.RunWith; 


import java.time.LocalDate;

import java.time.ZoneId;

import java.util.Collection;

import java.util.HashSet;

import java.util.List;

import java.util.Set;

import java.util.Timer;

import java.util.TimerTask;
@@ -89,6 +93,7 @@ public final class SearchViewManagerTest { 
    private TestMenuItem mSearchMenuItem;

    private TestableSearchViewManager mSearchViewManager;

    private SearchChipViewManager mSearchChipViewManager;

    private SearchOptionsController mSearchOptionsController;



    private boolean mListenerOnSearchChangedCalled;

    private int mOnSearchStartingCallCount;
@@ -136,8 +141,7 @@ public final class SearchViewManagerTest { 
        ViewGroup chipGroup = mock(ViewGroup.class);

        mSearchChipViewManager = spy(new SearchChipViewManager(chipGroup));

        View searchOptionsView = mock(View.class);

        SearchOptionsController mSearchOptionsController = new SearchOptionsController(

                searchOptionsView);

        mSearchOptionsController = new SearchOptionsController(searchOptionsView);

        mSearchViewManager = new TestableSearchViewManager(

                searchListener,

                mTestEventHandler,
@@ -531,4 +535,34 @@ public final class SearchViewManagerTest { 
                0 /* titleRes */, new String[]{""}));

        return chipDataList;

    }



    @Test

    @RequiresFlagsEnabled({Flags.FLAG_USE_SEARCH_V2_READ_ONLY})

    public void testMediaAndDownloadsHiddenOnSearchEverywhere() {

        RootInfo mediaRoot = spy(new RootInfo());

        mediaRoot.authority = Providers.AUTHORITY_MEDIA;

        mediaRoot.rootId = "images";

        mediaRoot.flags =  DocumentsContract.Root.FLAG_SUPPORTS_SEARCH;

        RootInfo downloadsRoot = spy(new RootInfo());

        downloadsRoot.authority = Providers.AUTHORITY_DOWNLOADS;

        downloadsRoot.rootId = "downloads";

        downloadsRoot.flags =  DocumentsContract.Root.FLAG_SUPPORTS_SEARCH;

        RootInfo externalRoot = spy(new RootInfo());

        externalRoot.authority = Providers.AUTHORITY_STORAGE;

        externalRoot.rootId = "primary";

        externalRoot.flags =  DocumentsContract.Root.FLAG_SUPPORTS_SEARCH;



        Collection<RootInfo> roots = List.of(mediaRoot, downloadsRoot, externalRoot);

        DocumentInfo nestedFolder = new DocumentInfo();

        nestedFolder.authority = Providers.AUTHORITY_DOWNLOADS;

        nestedFolder.documentId = "xyz:Nested";

        DocumentStack stack = new DocumentStack(downloadsRoot, nestedFolder);

        // Force search everywhere in mSearchViewManager. This is a private variable, so we

        // use this round-about method of setting it.

        mSearchOptionsController.onLocationSelected(SearchLocationOption.EVERYWHERE.getValue());

        mSearchOptionsController.notifyOptionsChangeListener();



        assertEquals(List.of(new FolderInfo(externalRoot)),

                mSearchViewManager.getSearchFolders(roots, stack));

    }

}