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

Commit e1aee57d authored by Terry Wang's avatar Terry Wang Committed by Automerger Merge Worker
Browse files

Merge "Fix validation crash for nextPageToken." into sc-dev am: f4b8e827 

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15345292

Change-Id: Ia2b6bac986f304bf9b17e3539f6a201b2aeb19cc
parents 08a463f1 f4b8e827

apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java

+22 −0
Original line number Diff line number Diff line
@@ -146,6 +146,9 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; 
public final class AppSearchImpl implements Closeable {

    private static final String TAG = "AppSearchImpl";



    /** A value 0 means that there're no more pages in the search results. */

    private static final long EMPTY_PAGE_TOKEN = 0;



    @VisibleForTesting static final int CHECK_OPTIMIZE_INTERVAL = 100;



    private final ReadWriteLock mReadWriteLock = new ReentrantReadWriteLock();
@@ -1139,6 +1142,16 @@ public final class AppSearchImpl implements Closeable { 
                    searchResultProto.getResultsCount(),

                    searchResultProto);

            checkSuccess(searchResultProto.getStatus());

            if (nextPageToken != EMPTY_PAGE_TOKEN

                    && searchResultProto.getNextPageToken() == EMPTY_PAGE_TOKEN) {

                // At this point, we're guaranteed that this nextPageToken exists for this package,

                // otherwise checkNextPageToken would've thrown an exception.

                // Since the new token is 0, this is the last page. We should remove the old token

                // from our cache since it no longer refers to this query.

                synchronized (mNextPageTokensLocked) {

                    mNextPageTokensLocked.get(packageName).remove(nextPageToken);

                }

            }

            return rewriteSearchResultProto(searchResultProto, mSchemaMapLocked);

        } finally {

            mReadWriteLock.readLock().unlock();
@@ -2059,6 +2072,10 @@ public final class AppSearchImpl implements Closeable { 
    }



    private void addNextPageToken(String packageName, long nextPageToken) {

        if (nextPageToken == EMPTY_PAGE_TOKEN) {

            // There is no more pages. No need to add it.

            return;

        }

        synchronized (mNextPageTokensLocked) {

            Set<Long> tokens = mNextPageTokensLocked.get(packageName);

            if (tokens == null) {
@@ -2071,6 +2088,11 @@ public final class AppSearchImpl implements Closeable { 


    private void checkNextPageToken(String packageName, long nextPageToken)

            throws AppSearchException {

        if (nextPageToken == EMPTY_PAGE_TOKEN) {

            // Swallow the check for empty page token, token = 0 means there is no more page and it

            // won't return anything from Icing.

            return;

        }

        synchronized (mNextPageTokensLocked) {

            Set<Long> nextPageTokens = mNextPageTokensLocked.get(packageName);

            if (nextPageTokens == null || !nextPageTokens.contains(nextPageToken)) {

apex/appsearch/synced_jetpack_changeid.txt

+1 −1
Original line number Diff line number Diff line 
c7387d9b58726a23a0608a9365fb3a1b57b7b8a1
 
Ie04f1ecc033faae8085afcb51eb9e40a298998d5

core/tests/coretests/src/android/app/appsearch/AppSearchSessionUnitTest.java

+127 −0
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package android.app.appsearch;



import static android.app.appsearch.SearchSpec.TERM_MATCH_PREFIX;



import static com.google.common.truth.Truth.assertThat;



import static org.testng.Assert.expectThrows;
@@ -30,6 +32,8 @@ import com.android.server.appsearch.testing.AppSearchEmail; 
import org.junit.Before;

import org.junit.Test;



import java.util.ArrayList;

import java.util.List;

import java.util.Objects;

import java.util.concurrent.CompletableFuture;

import java.util.concurrent.ExecutionException;
@@ -100,4 +104,127 @@ public class AppSearchSessionUnitTest { 
                .isEqualTo(AppSearchResult.RESULT_INTERNAL_ERROR);

        assertThat(appSearchException.getMessage()).startsWith("NullPointerException");

    }



    @Test

    public void testGetEmptyNextPage() throws Exception {

        // Set the schema.

        CompletableFuture<AppSearchResult<SetSchemaResponse>> schemaFuture =

                new CompletableFuture<>();

        mSearchSession.setSchema(

                new SetSchemaRequest.Builder()

                        .addSchemas(new AppSearchSchema.Builder("schema1").build())

                        .setForceOverride(true).build(),

                mExecutor, mExecutor, schemaFuture::complete);

        schemaFuture.get().getResultValue();



        // Create a document and index it.

        GenericDocument document1 = new GenericDocument.Builder<>("namespace", "id1",

                "schema1").build();

        CompletableFuture<AppSearchBatchResult<String, Void>> putDocumentsFuture =

                new CompletableFuture<>();

        mSearchSession.put(

                new PutDocumentsRequest.Builder().addGenericDocuments(document1).build(),

                mExecutor, new BatchResultCallback<String, Void>() {

                    @Override

                    public void onResult(AppSearchBatchResult<String, Void> result) {

                        putDocumentsFuture.complete(result);

                    }



                    @Override

                    public void onSystemError(Throwable throwable) {

                        putDocumentsFuture.completeExceptionally(throwable);

                    }

                });

        putDocumentsFuture.get();



        // Search and get the first page.

        SearchSpec searchSpec = new SearchSpec.Builder()

                .setTermMatch(TERM_MATCH_PREFIX)

                .setResultCountPerPage(1)

                .build();

        SearchResults searchResults = mSearchSession.search("", searchSpec);



        CompletableFuture<AppSearchResult<List<SearchResult>>> getNextPageFuture =

                new CompletableFuture<>();

        searchResults.getNextPage(mExecutor, getNextPageFuture::complete);

        List<SearchResult> results = getNextPageFuture.get().getResultValue();

        assertThat(results).hasSize(1);

        assertThat(results.get(0).getGenericDocument()).isEqualTo(document1);



        // We get all documents, and it shouldn't fail if we keep calling getNextPage().

        getNextPageFuture = new CompletableFuture<>();

        searchResults.getNextPage(mExecutor, getNextPageFuture::complete);

        results = getNextPageFuture.get().getResultValue();

        assertThat(results).isEmpty();

    }



    @Test

    public void testGetEmptyNextPage_multiPages() throws Exception {

        // Set the schema.

        CompletableFuture<AppSearchResult<SetSchemaResponse>> schemaFuture =

                new CompletableFuture<>();

        mSearchSession.setSchema(

                new SetSchemaRequest.Builder()

                        .addSchemas(new AppSearchSchema.Builder("schema1").build())

                        .setForceOverride(true).build(),

                mExecutor, mExecutor, schemaFuture::complete);

        schemaFuture.get().getResultValue();



        // Create a document and insert 3 package1 documents

        GenericDocument document1 = new GenericDocument.Builder<>("namespace", "id1",

                "schema1").build();

        GenericDocument document2 = new GenericDocument.Builder<>("namespace", "id2",

                "schema1").build();

        GenericDocument document3 = new GenericDocument.Builder<>("namespace", "id3",

                "schema1").build();

        CompletableFuture<AppSearchBatchResult<String, Void>> putDocumentsFuture =

                new CompletableFuture<>();

        mSearchSession.put(

                new PutDocumentsRequest.Builder()

                        .addGenericDocuments(document1, document2, document3).build(),

                mExecutor, new BatchResultCallback<String, Void>() {

                    @Override

                    public void onResult(AppSearchBatchResult<String, Void> result) {

                        putDocumentsFuture.complete(result);

                    }



                    @Override

                    public void onSystemError(Throwable throwable) {

                        putDocumentsFuture.completeExceptionally(throwable);

                    }

                });

        putDocumentsFuture.get();



        // Search for only 2 result per page

        SearchSpec searchSpec = new SearchSpec.Builder()

                .setTermMatch(TERM_MATCH_PREFIX)

                .setResultCountPerPage(2)

                .build();

        SearchResults searchResults = mSearchSession.search("", searchSpec);



        // Get the first page, it contains 2 results.

        List<GenericDocument> outDocs = new ArrayList<>();

        CompletableFuture<AppSearchResult<List<SearchResult>>> getNextPageFuture =

                new CompletableFuture<>();

        searchResults.getNextPage(mExecutor, getNextPageFuture::complete);

        List<SearchResult> results = getNextPageFuture.get().getResultValue();

        assertThat(results).hasSize(2);

        outDocs.add(results.get(0).getGenericDocument());

        outDocs.add(results.get(1).getGenericDocument());



        // Get the second page, it contains only 1 result.

        getNextPageFuture = new CompletableFuture<>();

        searchResults.getNextPage(mExecutor, getNextPageFuture::complete);

        results = getNextPageFuture.get().getResultValue();

        assertThat(results).hasSize(1);

        outDocs.add(results.get(0).getGenericDocument());



        assertThat(outDocs).containsExactly(document1, document2, document3);



        // We get all documents, and it shouldn't fail if we keep calling getNextPage().

        getNextPageFuture = new CompletableFuture<>();

        searchResults.getNextPage(mExecutor, getNextPageFuture::complete);

        results = getNextPageFuture.get().getResultValue();

        assertThat(results).isEmpty();

    }

}