From 0a0f94d410ee74708e6f330e175a4730ad4a4330 Mon Sep 17 00:00:00 2001 From: ningOTI Date: Mon, 12 Aug 2019 22:17:09 +0800 Subject: [PATCH] Improve search suggestion load timings (#22) * Fix progressBar not showing * Load PDB titles in a background thread --- .../io/ningyuan/palantir/MainActivity.java | 7 -- .../ningyuan/palantir/utils/PdbSearcher.java | 90 +++++++++++-------- .../palantir/utils/PdbTitleSearcher.java | 59 ++++++++++++ .../ningyuan/palantir/views/SearchView.java | 4 +- 4 files changed, 113 insertions(+), 47 deletions(-) create mode 100644 app/src/main/java/io/ningyuan/palantir/utils/PdbTitleSearcher.java diff --git a/app/src/main/java/io/ningyuan/palantir/MainActivity.java b/app/src/main/java/io/ningyuan/palantir/MainActivity.java index 2354c20..31b0b3a 100644 --- a/app/src/main/java/io/ningyuan/palantir/MainActivity.java +++ b/app/src/main/java/io/ningyuan/palantir/MainActivity.java @@ -2,9 +2,6 @@ import android.os.Bundle; import android.support.v7.app.AppCompatActivity; -import android.text.Html; -import android.text.method.LinkMovementMethod; -import android.widget.EditText; import android.widget.ProgressBar; import android.widget.TextView; @@ -68,8 +65,4 @@ public void updateModelRenderable(String name, File glbFile) { public void showAbout() { aboutView.show(); } - - public void hideAbout() { - aboutView.hide(); - } } \ No newline at end of file diff --git a/app/src/main/java/io/ningyuan/palantir/utils/PdbSearcher.java b/app/src/main/java/io/ningyuan/palantir/utils/PdbSearcher.java index a70532b..e282aaa 100644 --- a/app/src/main/java/io/ningyuan/palantir/utils/PdbSearcher.java +++ b/app/src/main/java/io/ningyuan/palantir/utils/PdbSearcher.java @@ -1,16 +1,14 @@ package io.ningyuan.palantir.utils; -import android.app.SearchManager; import android.database.MatrixCursor; import android.os.AsyncTask; -import android.provider.BaseColumns; import android.util.Log; import android.view.View; import android.widget.CursorAdapter; import android.widget.ProgressBar; -import java.io.FileNotFoundException; import java.io.IOException; +import java.util.LinkedList; import java.util.List; import javax.xml.parsers.ParserConfigurationException; @@ -19,10 +17,15 @@ import io.ningyuan.jPdbApi.Query; import io.ningyuan.palantir.views.SearchView; -public class PdbSearcher extends AsyncTask { +/** + * Represents a search instance. For example, at the instance when 'insul' is just keyed into the + * searchView's EditText, a PdbSearcher instance is formed for 'insul'. However, when 'insuli' is + * keyed in, the 'insul' PdbSearcher instance is invalidated in favour of the new 'insuli' instance. + */ +public class PdbSearcher extends AsyncTask> { private static final String TAG = String.format("PALANTIR::%s", PdbSearcher.class.getSimpleName()); - private boolean inProgress = false; + private boolean valid = true; private CursorAdapter adapter; private ProgressBar progressBar; @@ -31,70 +34,81 @@ public PdbSearcher(CursorAdapter adapter, ProgressBar progressBar) { this.progressBar = progressBar; } - public boolean isRunning() { - return inProgress; + public boolean isValid() { + return valid; + } + + /** + * The parent SearchView has been deactivated, or a newer PdbSearcher was spawned. + */ + public void makeInvalid() { + valid = false; } @Override protected void onPreExecute() { + Log.d(TAG, "onPreExecute called."); if (progressBar != null) { progressBar.setVisibility(View.VISIBLE); } - inProgress = true; } @Override - protected MatrixCursor doInBackground(String... queryStrings) { + protected LinkedList doInBackground(String... queryStrings) { String queryString = queryStrings[0]; try { Query query = new Query(Query.KEYWORD_QUERY, queryString); List results = query.execute(); - MatrixCursor cursor = SearchView.getEmptyCursor(false); - int index = 1; - Log.i(TAG, "Start for in doInBackground"); + LinkedList pdbResults = new LinkedList<>(); + Log.i(TAG, String.format("Start doInBackground for %s", queryString)); for (String pdbId : results) { - Log.i(TAG, pdbId); - Pdb pdb = new Pdb(pdbId); - try { - pdb.load(); - cursor.addRow(new Object[]{index++, pdb.getStructureId(), pdb.getTitle()}); - } catch (FileNotFoundException e) { - Log.e(TAG, "Encountered an exception.", e); - } + ((LinkedList) pdbResults).addLast(new Pdb(pdbId)); + // TODO: do this later, after just the pdbIds are passed to UI + // Log.d(TAG, String.format("Fetching %s for %s", pdbId, queryString)); + // Pdb pdb = new Pdb(pdbId); + // try { + // pdb.load(); + // cursor.addRow(new Object[]{index++, pdb.getStructureId(), pdb.getTitle()}); + // } catch (FileNotFoundException e) { + // Log.e(TAG, String.format("Encountered an exception for %s.", queryString), e); + // } } - - return cursor; + return pdbResults; } catch (IOException | ParserConfigurationException e) { - Log.e(TAG, null, e); + Log.e(TAG, String.format("Encountered an exception for %s.", queryString), e); return null; } } @Override - protected void onPostExecute(MatrixCursor cursor) { - inProgress = false; + protected void onPostExecute(LinkedList results) { if (progressBar != null) { progressBar.setVisibility(View.INVISIBLE); } - if (cursor == null) { - String[] columns = { - BaseColumns._ID, - SearchManager.SUGGEST_COLUMN_TEXT_1, - SearchManager.SUGGEST_COLUMN_TEXT_2 - }; - cursor = new MatrixCursor(columns); + MatrixCursor cursor = SearchView.getEmptyCursor(false); + int cursorIndex = 0; + for (Pdb pdb : results) { + cursor.addRow(new Object[]{ + cursorIndex++, pdb.getStructureId(), pdb.getTitle() + }); } - adapter.changeCursor(cursor); + + new PdbTitleSearcher(this, adapter, results).execute(0); } @Override - protected void onCancelled(MatrixCursor cursor) { - inProgress = false; - if (progressBar != null) { - progressBar.setVisibility(View.INVISIBLE); - } + protected void onCancelled(LinkedList results) { + // On cancelled, do nothing. I make this explicit because method used to set the progressBar + // visibility to View.INVISIBLE. That was not a good idea: when a search was cancelled and + // another started in its place immediately, a race condition was created. One would expect + // onCancelled to be called first, setting the progressBar to invisible; then, onPreExecute + // of the new pdbSearch would set progressBar to View.VISIBLE. In reality, onPreExecute is + // often called before onCancelled, so the progressBar would not show. Instead, I set the + // progressBar visibility to View.INVISIBLE explicitly in the SearchView methods. + Log.d(TAG, "onCancelled called."); + makeInvalid(); } } \ No newline at end of file diff --git a/app/src/main/java/io/ningyuan/palantir/utils/PdbTitleSearcher.java b/app/src/main/java/io/ningyuan/palantir/utils/PdbTitleSearcher.java new file mode 100644 index 0000000..8ca53d6 --- /dev/null +++ b/app/src/main/java/io/ningyuan/palantir/utils/PdbTitleSearcher.java @@ -0,0 +1,59 @@ +package io.ningyuan.palantir.utils; + +import android.database.MatrixCursor; +import android.os.AsyncTask; +import android.util.Log; +import android.widget.CursorAdapter; + +import java.io.IOException; +import java.util.LinkedList; + +import io.ningyuan.jPdbApi.Pdb; +import io.ningyuan.palantir.views.SearchView; + +/** + * Represents the lazy loading of titles for each result in a PdbSearcher. + */ +public class PdbTitleSearcher extends AsyncTask { + private static final String TAG = String.format("PALANTIR::%s", PdbTitleSearcher.class.getSimpleName()); + + private LinkedList pdbs; + private CursorAdapter adapter; + private PdbSearcher pdbSearcher; + + public PdbTitleSearcher(PdbSearcher pdbSearcher, CursorAdapter adapter, LinkedList pdbs) { + this.adapter = adapter; + this.pdbSearcher = pdbSearcher; + this.pdbs = pdbs; + } + + @Override + protected Integer doInBackground(Integer... integers) { + int indexToLoad = integers[0]; + + try { + pdbs.get(indexToLoad).load(); + } catch (IOException e) { + Log.e(TAG, null, e); + } + + return indexToLoad + 1; + } + + @Override + protected void onPostExecute(Integer nextIndex) { + MatrixCursor cursor = SearchView.getEmptyCursor(false); + int cursorIndex = 0; + for (Pdb pdb : pdbs) { + cursor.addRow(new Object[]{ + cursorIndex++, pdb.getStructureId(), pdb.getTitle() + }); + } + adapter.changeCursor(cursor); + + Log.d(TAG, String.format("pdbSearcher.isValid()=%b", pdbSearcher.isValid())); + if (nextIndex < pdbs.size() && pdbSearcher.isValid()) { + new PdbTitleSearcher(pdbSearcher, adapter, pdbs).execute(nextIndex); + } + } +} \ No newline at end of file diff --git a/app/src/main/java/io/ningyuan/palantir/views/SearchView.java b/app/src/main/java/io/ningyuan/palantir/views/SearchView.java index bda047e..69cf704 100644 --- a/app/src/main/java/io/ningyuan/palantir/views/SearchView.java +++ b/app/src/main/java/io/ningyuan/palantir/views/SearchView.java @@ -141,7 +141,6 @@ public boolean onQueryTextChange(String s) { }); } - private void deactivate() { // Clear the soft keyboard clearFocus(); @@ -163,9 +162,10 @@ private void doSearch(String query) { Log.d(TAG, String.format("doSearch found \"%s\"", query)); if (query.length() < 3) { MatrixCursor cursor = getEmptyCursor(); - suggestionAdapter.swapCursor(cursor); + suggestionAdapter.changeCursor(cursor); } else { pdbSearcher.cancel(true); + pdbSearcher.makeInvalid(); // pdbSearcher.onCancelled not called if doInBackground already complete pdbSearcher = new PdbSearcher(suggestionAdapter, progressBar); Log.i(TAG, String.format("Starting search with %s", query)); pdbSearcher.execute(query);