From 8c3dc96b210d17eb8af0967394ef2a87f61044e1 Mon Sep 17 00:00:00 2001 From: Fan Zhang Date: Wed, 15 Mar 2017 11:39:17 -0700 Subject: [PATCH] Refactor all saved query related logic into a controller Change-Id: I96f7dd91d7d2715141a9905cb59262d927560da4 Fix: 27391895 Test: make RunSettingsRoboTests --- .../search2/SavedQueryController.java | 96 +++++++++++++++++++ .../settings/search2/SearchFragment.java | 93 ++++++------------ .../settings/search2/SearchFragmentTest.java | 43 ++++----- 3 files changed, 144 insertions(+), 88 deletions(-) create mode 100644 src/com/android/settings/search2/SavedQueryController.java diff --git a/src/com/android/settings/search2/SavedQueryController.java b/src/com/android/settings/search2/SavedQueryController.java new file mode 100644 index 00000000000..92ca42af731 --- /dev/null +++ b/src/com/android/settings/search2/SavedQueryController.java @@ -0,0 +1,96 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.search2; + +import android.app.LoaderManager; +import android.content.Context; +import android.content.Loader; +import android.os.Bundle; + +import com.android.settings.overlay.FeatureFactory; + +import java.util.List; + +public class SavedQueryController implements LoaderManager.LoaderCallbacks { + + // TODO: make a generic background task manager to handle one-off tasks like this one. + + private static final int LOADER_ID_SAVE_QUERY_TASK = 0; + private static final int LOADER_ID_REMOVE_QUERY_TASK = 1; + private static final int LOADER_ID_SAVED_QUERIES = 2; + private static final String ARG_QUERY = "remove_query"; + + private final Context mContext; + private final LoaderManager mLoaderManager; + private final SearchFeatureProvider mSearchFeatureProvider; + private final SearchResultsAdapter mResultAdapter; + + public SavedQueryController(Context context, LoaderManager loaderManager, + SearchResultsAdapter resultsAdapter) { + mContext = context; + mLoaderManager = loaderManager; + mResultAdapter = resultsAdapter; + mSearchFeatureProvider = FeatureFactory.getFactory(context) + .getSearchFeatureProvider(); + } + + @Override + public Loader onCreateLoader(int id, Bundle args) { + switch (id) { + case LOADER_ID_SAVE_QUERY_TASK: + return new SavedQueryRecorder(mContext, args.getString(ARG_QUERY)); + case LOADER_ID_REMOVE_QUERY_TASK: + return new SavedQueryRemover(mContext, args.getString(ARG_QUERY)); + case LOADER_ID_SAVED_QUERIES: + return mSearchFeatureProvider.getSavedQueryLoader(mContext); + } + return null; + } + + @Override + public void onLoadFinished(Loader loader, Object data) { + switch (loader.getId()) { + case LOADER_ID_REMOVE_QUERY_TASK: + mLoaderManager.restartLoader(LOADER_ID_SAVED_QUERIES, null, this); + break; + case LOADER_ID_SAVED_QUERIES: + mResultAdapter.displaySavedQuery((List) data); + break; + } + } + + @Override + public void onLoaderReset(Loader loader) { + + } + + public void saveQuery(String query) { + final Bundle args = new Bundle(); + args.putString(ARG_QUERY, query); + mLoaderManager.restartLoader(LOADER_ID_SAVE_QUERY_TASK, args, this); + } + + public void removeQuery(String query) { + final Bundle args = new Bundle(); + args.putString(ARG_QUERY, query); + mLoaderManager.restartLoader(LOADER_ID_REMOVE_QUERY_TASK, args, this); + } + + public void loadSavedQueries() { + mLoaderManager.restartLoader(LOADER_ID_SAVED_QUERIES, null, this); + } +} diff --git a/src/com/android/settings/search2/SearchFragment.java b/src/com/android/settings/search2/SearchFragment.java index 02ff2c8ff3f..eb760ee52ba 100644 --- a/src/com/android/settings/search2/SearchFragment.java +++ b/src/com/android/settings/search2/SearchFragment.java @@ -54,11 +54,11 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O // State values private static final String STATE_QUERY = "state_query"; + private static final String STATE_SHOWING_SAVED_QUERY = "state_showing_saved_query"; private static final String STATE_NEVER_ENTERED_QUERY = "state_never_entered_query"; private static final String STATE_RESULT_CLICK_COUNT = "state_result_click_count"; // Loader IDs - private static final int LOADER_ID_RECENTS = 0; private static final int LOADER_ID_DATABASE = 1; private static final int LOADER_ID_INSTALLED_APPS = 2; @@ -74,12 +74,11 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O @VisibleForTesting String mQuery; - private final SaveQueryCallback mSaveQueryCallback = - new SaveQueryCallback(); - private boolean mNeverEnteredQuery = true; + private boolean mShowingSavedQuery; private int mResultClickCount; private MetricsFeatureProvider mMetricsFeatureProvider; + private SavedQueryController mSavedQueryController; @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) SearchFeatureProvider mSearchFeatureProvider; @@ -117,20 +116,26 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); setHasOptionsMenu(true); - mSearchAdapter = new SearchResultsAdapter(this); - - mSearchFeatureProvider.initFeedbackButton(); - final LoaderManager loaderManager = getLoaderManager(); + mSearchAdapter = new SearchResultsAdapter(this); + mSavedQueryController = new SavedQueryController( + getContext(), loaderManager, mSearchAdapter); + mSearchFeatureProvider.initFeedbackButton(); if (savedInstanceState != null) { mQuery = savedInstanceState.getString(STATE_QUERY); mNeverEnteredQuery = savedInstanceState.getBoolean(STATE_NEVER_ENTERED_QUERY); mResultClickCount = savedInstanceState.getInt(STATE_RESULT_CLICK_COUNT); - loaderManager.initLoader(LOADER_ID_DATABASE, null, this); - loaderManager.initLoader(LOADER_ID_INSTALLED_APPS, null, this); + mShowingSavedQuery = savedInstanceState.getBoolean(STATE_SHOWING_SAVED_QUERY); + if (mShowingSavedQuery) { + mSavedQueryController.loadSavedQueries(); + } else { + loaderManager.initLoader(LOADER_ID_DATABASE, null, this); + loaderManager.initLoader(LOADER_ID_INSTALLED_APPS, null, this); + } } else { - loaderManager.initLoader(LOADER_ID_RECENTS, null, this); + mShowingSavedQuery = true; + mSavedQueryController.loadSavedQueries(); } final Activity activity = getActivity(); @@ -180,6 +185,7 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O super.onSaveInstanceState(outState); outState.putString(STATE_QUERY, mQuery); outState.putBoolean(STATE_NEVER_ENTERED_QUERY, mNeverEnteredQuery); + outState.putBoolean(STATE_SHOWING_SAVED_QUERY, mShowingSavedQuery); outState.putInt(STATE_RESULT_CLICK_COUNT, mResultClickCount); } @@ -206,7 +212,8 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O final LoaderManager loaderManager = getLoaderManager(); loaderManager.destroyLoader(LOADER_ID_DATABASE); loaderManager.destroyLoader(LOADER_ID_INSTALLED_APPS); - loaderManager.restartLoader(LOADER_ID_RECENTS, null /* args */, this /* callback */); + mShowingSavedQuery = true; + mSavedQueryController.loadSavedQueries(); mSearchFeatureProvider.hideFeedbackButton(); } else { restartLoaders(); @@ -218,8 +225,7 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O @Override public boolean onQueryTextSubmit(String query) { // Save submitted query. - getLoaderManager().restartLoader(SaveQueryCallback.LOADER_ID_SAVE_QUERY_TASK, null, - mSaveQueryCallback); + mSavedQueryController.saveQuery(mQuery); hideKeyboard(); return true; } @@ -233,8 +239,6 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O return mSearchFeatureProvider.getDatabaseSearchLoader(activity, mQuery); case LOADER_ID_INSTALLED_APPS: return mSearchFeatureProvider.getInstalledAppSearchLoader(activity, mQuery); - case LOADER_ID_RECENTS: - return mSearchFeatureProvider.getSavedQueryLoader(activity); default: return null; } @@ -243,18 +247,11 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O @Override public void onLoadFinished(Loader> loader, List data) { - final int resultCount; - switch (loader.getId()) { - case LOADER_ID_RECENTS: - resultCount = mSearchAdapter.displaySavedQuery(data); - break; - default: - mSearchAdapter.addSearchResults(data, loader.getClass().getName()); - if (mUnfinishedLoadersCount.decrementAndGet() != 0) { - return; - } - resultCount = mSearchAdapter.displaySearchResults(); + mSearchAdapter.addSearchResults(data, loader.getClass().getName()); + if (mUnfinishedLoadersCount.decrementAndGet() != 0) { + return; } + final int resultCount = mSearchAdapter.displaySearchResults(); mNoResultsView.setVisibility(resultCount == 0 ? View.VISIBLE : View.GONE); mSearchFeatureProvider.showFeedbackButton(this, getView()); } @@ -264,8 +261,7 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O } public void onSearchResultClicked() { - getLoaderManager().restartLoader(SaveQueryCallback.LOADER_ID_SAVE_QUERY_TASK, null, - mSaveQueryCallback); + mSavedQueryController.saveQuery(mQuery); mResultClickCount++; } @@ -278,13 +274,11 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O } public void onRemoveSavedQueryClicked(CharSequence title) { - final Bundle args = new Bundle(); - args.putString(SaveQueryCallback.ARG_REMOVE_QUERY, title.toString()); - getLoaderManager().restartLoader(SaveQueryCallback.LOADER_ID_REMOVE_QUERY_TASK, - args, mSaveQueryCallback); + mSavedQueryController.removeQuery(title.toString()); } private void restartLoaders() { + mShowingSavedQuery = false; final LoaderManager loaderManager = getLoaderManager(); mUnfinishedLoadersCount.set(NUM_QUERY_LOADERS); loaderManager.restartLoader(LOADER_ID_DATABASE, null /* args */, this /* callback */); @@ -325,37 +319,4 @@ public class SearchFragment extends InstrumentedFragment implements SearchView.O mResultsRecyclerView.requestFocus(); } } - - private class SaveQueryCallback implements LoaderManager.LoaderCallbacks { - // TODO: make a generic background task manager to handle one-off tasks like this one. - - private static final int LOADER_ID_SAVE_QUERY_TASK = 0; - private static final int LOADER_ID_REMOVE_QUERY_TASK = 1; - private static final String ARG_REMOVE_QUERY = "remove_query"; - - @Override - public Loader onCreateLoader(int id, Bundle args) { - switch (id) { - case LOADER_ID_SAVE_QUERY_TASK: - return new SavedQueryRecorder(getActivity(), mQuery); - case LOADER_ID_REMOVE_QUERY_TASK: - return new SavedQueryRemover(getActivity(), args.getString(ARG_REMOVE_QUERY)); - } - return null; - } - - @Override - public void onLoadFinished(Loader loader, Void data) { - switch (loader.getId()) { - case LOADER_ID_REMOVE_QUERY_TASK: - getLoaderManager().restartLoader(LOADER_ID_RECENTS, null, SearchFragment.this); - break; - } - } - - @Override - public void onLoaderReset(Loader loader) { - - } - } } \ No newline at end of file diff --git a/tests/robotests/src/com/android/settings/search2/SearchFragmentTest.java b/tests/robotests/src/com/android/settings/search2/SearchFragmentTest.java index 3e22d5661f8..2b6ebafbb99 100644 --- a/tests/robotests/src/com/android/settings/search2/SearchFragmentTest.java +++ b/tests/robotests/src/com/android/settings/search2/SearchFragmentTest.java @@ -43,7 +43,6 @@ import org.robolectric.util.ReflectionHelpers; import java.util.List; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyList; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -66,7 +65,8 @@ public class SearchFragmentTest { @Mock private SavedQueryLoader mSavedQueryLoader; - + @Mock + private SavedQueryController mSavedQueryController; private FakeFeatureFactory mFeatureFactory; @Before @@ -96,6 +96,7 @@ public class SearchFragmentTest { SearchFragment fragment = (SearchFragment) activityController.get().getFragmentManager() .findFragmentById(R.id.main_content); + ReflectionHelpers.setField(fragment, "mShowingSavedQuery", false); fragment.mQuery = testQuery; activityController.saveInstanceState(bundle).pause().stop().destroy(); @@ -111,12 +112,6 @@ public class SearchFragmentTest { @Test public void screenRotateEmptyString_ShouldNotCrash() { - when(mFeatureFactory.searchFeatureProvider - .getDatabaseSearchLoader(any(Context.class), anyString())) - .thenReturn(mDatabaseResultLoader); - when(mFeatureFactory.searchFeatureProvider - .getInstalledAppSearchLoader(any(Context.class), anyString())) - .thenReturn(mInstalledAppResultLoader); when(mFeatureFactory.searchFeatureProvider.getSavedQueryLoader(any(Context.class))) .thenReturn(mSavedQueryLoader); @@ -134,10 +129,12 @@ public class SearchFragmentTest { activityController = Robolectric.buildActivity(SearchActivity.class); activityController.setup(bundle); - verify(mFeatureFactory.searchFeatureProvider) + verify(mFeatureFactory.searchFeatureProvider, never()) .getDatabaseSearchLoader(any(Context.class), anyString()); - verify(mFeatureFactory.searchFeatureProvider) + verify(mFeatureFactory.searchFeatureProvider, never()) .getInstalledAppSearchLoader(any(Context.class), anyString()); + verify(mFeatureFactory.searchFeatureProvider, times(2)) + .getSavedQueryLoader(any(Context.class)); } @Test @@ -160,6 +157,7 @@ public class SearchFragmentTest { fragment.onQueryTextChange(testQuery); activityController.get().onBackPressed(); + activityController.pause().stop().destroy(); verify(mFeatureFactory.metricsFeatureProvider, never()).action( @@ -174,7 +172,7 @@ public class SearchFragmentTest { } @Test - public void queryTextChangeToEmpty_shouldTriggerSavedQueryLoader() { + public void queryTextChangeToEmpty_shouldLoadSavedQuery() { when(mFeatureFactory.searchFeatureProvider .getDatabaseSearchLoader(any(Context.class), anyString())) .thenReturn(mDatabaseResultLoader); @@ -190,20 +188,15 @@ public class SearchFragmentTest { SearchFragment fragment = spy((SearchFragment) activityController.get().getFragmentManager() .findFragmentById(R.id.main_content)); - - final SearchResultsAdapter adapter = mock(SearchResultsAdapter.class); - ReflectionHelpers.setField(fragment, "mSearchAdapter", adapter); + ReflectionHelpers.setField(fragment, "mSavedQueryController", mSavedQueryController); + fragment.mQuery = "123"; + fragment.onQueryTextChange(""); verify(mFeatureFactory.searchFeatureProvider, never()) .getDatabaseSearchLoader(any(Context.class), anyString()); verify(mFeatureFactory.searchFeatureProvider, never()) .getInstalledAppSearchLoader(any(Context.class), anyString()); - verify(mFeatureFactory.searchFeatureProvider) - .getSavedQueryLoader(any(Context.class)); - - fragment.onLoadFinished(mSavedQueryLoader, null /* data */); - - verify(adapter).displaySavedQuery(anyList()); + verify(mSavedQueryController).loadSavedQueries(); } @Test @@ -235,6 +228,8 @@ public class SearchFragmentTest { when(mFeatureFactory.searchFeatureProvider .getInstalledAppSearchLoader(any(Context.class), anyString())) .thenReturn(new MockAppLoader(RuntimeEnvironment.application)); + when(mFeatureFactory.searchFeatureProvider.getSavedQueryLoader(any(Context.class))) + .thenReturn(mSavedQueryLoader); ActivityController activityController = Robolectric.buildActivity(SearchActivity.class); @@ -258,6 +253,8 @@ public class SearchFragmentTest { when(mFeatureFactory.searchFeatureProvider .getInstalledAppSearchLoader(any(Context.class), anyString())) .thenReturn(new MockAppLoader(RuntimeEnvironment.application)); + when(mFeatureFactory.searchFeatureProvider.getSavedQueryLoader(any(Context.class))) + .thenReturn(mSavedQueryLoader); ActivityController activityController = Robolectric.buildActivity(SearchActivity.class); @@ -282,12 +279,14 @@ public class SearchFragmentTest { when(mFeatureFactory.searchFeatureProvider .getInstalledAppSearchLoader(any(Context.class), anyString())) .thenReturn(new MockAppLoader(RuntimeEnvironment.application)); + when(mFeatureFactory.searchFeatureProvider.getSavedQueryLoader(any(Context.class))) + .thenReturn(mSavedQueryLoader); ActivityController activityController = Robolectric.buildActivity(SearchActivity.class); activityController.setup(); - SearchFragment fragment = (SearchFragment) spy(activityController.get().getFragmentManager() - .findFragmentById(R.id.main_content)); + SearchFragment fragment = (SearchFragment) activityController.get().getFragmentManager() + .findFragmentById(R.id.main_content); fragment.onQueryTextChange("non-empty");