Improve search suggestion load timings (#22)

* Fix progressBar not showing

* Load PDB titles in a background thread

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();
-    }
 }

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<String, Void, MatrixCursor> {
+/**
+ * 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<String, Void, LinkedList<Pdb>> {
     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<Pdb> doInBackground(String... queryStrings) {
         String queryString = queryStrings[0];
 
         try {
             Query query = new Query(Query.KEYWORD_QUERY, queryString);
             List<String> results = query.execute();
-            MatrixCursor cursor = SearchView.getEmptyCursor(false);
-            int index = 1;
-            Log.i(TAG, "Start for in doInBackground");
+            LinkedList<Pdb> 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<Pdb>) 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<Pdb> 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<Pdb> 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();
     }
 }

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<Integer, Void, Integer> {
+    private static final String TAG = String.format("PALANTIR::%s", PdbTitleSearcher.class.getSimpleName());
+
+    private LinkedList<Pdb> pdbs;
+    private CursorAdapter adapter;
+    private PdbSearcher pdbSearcher;
+
+    public PdbTitleSearcher(PdbSearcher pdbSearcher, CursorAdapter adapter, LinkedList<Pdb> 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);
+        }
+    }
+}

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);