fix isIntegrityCheck() & add a sample impl class and test for DatabaseErrorHandler

fixed a bug in SQLiteDatabase.isDatabaseIntegrityOk()
and added a new class to demonstrate use of
android.database.DatabaseErrorHandler
and a bunch of nits

Change-Id: Ia81870853fa3bd84074637f6d823a9fd22b66c7e

4 files changed, 149 insertions, 20 deletions

diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java
index b83d5ad..0137ea6 100644
--- a/core/java/android/database/sqlite/SQLiteDatabase.java
+++ b/core/java/android/database/sqlite/SQLiteDatabase.java
@@ -39,7 +39,6 @@ import dalvik.system.BlockGuard;
 import java.io.File;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -273,7 +272,7 @@ public class SQLiteDatabase extends SQLiteClosable {
      * invoked.
      *
      * this cache has an upper limit of mMaxSqlCacheSize (settable by calling the method
-     * (@link setMaxSqlCacheSize(int)}).
+     * (@link #setMaxSqlCacheSize(int)}).
      */
     // default statement-cache size per database connection ( = instance of this class)
     private int mMaxSqlCacheSize = 25;
@@ -903,8 +902,7 @@ public class SQLiteDatabase extends SQLiteClosable {
     public interface CursorFactory {
         /**
          * See
-         * {@link SQLiteCursor#SQLiteCursor(SQLiteDatabase, SQLiteCursorDriver,
-         * String, SQLiteQuery)}.
+         * {@link SQLiteCursor#SQLiteCursor(SQLiteCursorDriver, String, SQLiteQuery)}.
          */
         public Cursor newCursor(SQLiteDatabase db,
                 SQLiteCursorDriver masterQuery, String editTable,
@@ -2080,7 +2078,7 @@ public class SQLiteDatabase extends SQLiteClosable {
                  */
                 if (++mCacheFullWarnings == MAX_WARNINGS_ON_CACHESIZE_CONDITION) {
                     Log.w(TAG, "Reached MAX size for compiled-sql statement cache for database " +
-                            getPath() + ". Consider increasing cachesize.");
+                            getPath() + ". Use setMaxSqlCacheSize() to increase cachesize. ");
                 }
             } 
             /* add the given SQLiteCompiledSql compiledStatement to cache.
@@ -2142,7 +2140,7 @@ public class SQLiteDatabase extends SQLiteClosable {
      *
      * @param cacheSize the size of the cache. can be (0 to {@link #MAX_SQL_CACHE_SIZE})
      * @throws IllegalStateException if input cacheSize > {@link #MAX_SQL_CACHE_SIZE} or
-     * > the value set with previous setMaxSqlCacheSize() call.
+     * the value set with previous setMaxSqlCacheSize() call.
      */
     public synchronized void setMaxSqlCacheSize(int cacheSize) {
         if (cacheSize > MAX_SQL_CACHE_SIZE || cacheSize < 0) {
@@ -2342,10 +2340,12 @@ public class SQLiteDatabase extends SQLiteClosable {
 
     /* package */ SQLiteDatabase getDbConnection(String sql) {
         verifyDbIsOpen();
-        // this method should always be called with main database connection handle
-        // NEVER with pooled database connection handle
+        // this method should always be called with main database connection handle.
+        // the only time when it is called with pooled database connection handle is
+        // corruption occurs while trying to open a pooled database connection handle.
+        // in that case, simply return 'this' handle
         if (isPooledConnection()) {
-            throw new IllegalStateException("incorrect database connection handle");
+            return this;
         }
 
         // use the current connection handle if
@@ -2504,12 +2504,18 @@ public class SQLiteDatabase extends SQLiteClosable {
      */
     public boolean isDatabaseIntegrityOk() {
         verifyDbIsOpen();
-        ArrayList<Pair<String, String>> attachedDbs = getAttachedDbs();
-        if (attachedDbs == null) {
-            throw new IllegalStateException("databaselist for: " + getPath() + " couldn't " +
-                    "be retrieved. probably because the database is closed");
+        ArrayList<Pair<String, String>> attachedDbs = null;
+        try {
+            attachedDbs = getAttachedDbs();
+            if (attachedDbs == null) {
+                throw new IllegalStateException("databaselist for: " + getPath() + " couldn't " +
+                        "be retrieved. probably because the database is closed");
+            }
+        } catch (SQLiteException e) {
+            // can't get attachedDb list. do integrity check on the main database
+            attachedDbs = new ArrayList<Pair<String, String>>();
+            attachedDbs.add(new Pair<String, String>("main", this.mPath));
         }
-        boolean isDatabaseCorrupt = false;
         for (int i = 0; i < attachedDbs.size(); i++) {
             Pair<String, String> p = attachedDbs.get(i);
             SQLiteStatement prog = null;
@@ -2518,14 +2524,14 @@ public class SQLiteDatabase extends SQLiteClosable {
                 String rslt = prog.simpleQueryForString();
                 if (!rslt.equalsIgnoreCase("ok")) {
                     // integrity_checker failed on main or attached databases
-                    isDatabaseCorrupt = true;
                     Log.e(TAG, "PRAGMA integrity_check on " + p.second + " returned: " + rslt);
+                    return false;
                 }
             } finally {
                 if (prog != null) prog.close();
             }
         }
-        return isDatabaseCorrupt;
+        return true;
     }
 
     /**
diff --git a/core/java/android/database/sqlite/SQLiteMisuseException.java b/core/java/android/database/sqlite/SQLiteMisuseException.java
index 685f3ea..546ec08 100644
--- a/core/java/android/database/sqlite/SQLiteMisuseException.java
+++ b/core/java/android/database/sqlite/SQLiteMisuseException.java
@@ -16,6 +16,18 @@
 
 package android.database.sqlite;
 
+/**
+ * This error can occur if the application creates a SQLiteStatement object and allows multiple
+ * threads in the application use it at the same time.
+ * Sqlite returns this error if bind and execute methods on this object occur at the same time
+ * from multiple threads, like so:
+ *     thread # 1: in execute() method of the SQLiteStatement object
+ *     while thread # 2: is in bind..() on the same object.
+ *</p>
+ * FIX this by NEVER sharing the same SQLiteStatement object between threads.
+ * Create a local instance of the SQLiteStatement whenever it is needed, use it and close it ASAP.
+ * NEVER make it globally available.
+ */
 public class SQLiteMisuseException extends SQLiteException {
     public SQLiteMisuseException() {}
 
diff --git a/core/jni/android_database_SQLiteQuery.cpp b/core/jni/android_database_SQLiteQuery.cpp
index 747ee50..e383123 100644
--- a/core/jni/android_database_SQLiteQuery.cpp
+++ b/core/jni/android_database_SQLiteQuery.cpp
@@ -205,7 +205,7 @@ static jint native_fill_window(JNIEnv* env, jobject object, jobject javaWindow,
                     int offset = window->alloc(size);
                     if (!offset) {
                         window->freeLastRow();
-                        LOGE("Failed allocating %u bytes for text/blob at %d,%d", size,
+                        LOGD("Failed allocating %u bytes for text/blob at %d,%d", size,
                                    startPos + numRows, i);
                         return startPos + numRows + finish_program_and_get_row_count(statement) + 1;
                     }
@@ -225,7 +225,7 @@ static jint native_fill_window(JNIEnv* env, jobject object, jobject javaWindow,
                     int64_t value = sqlite3_column_int64(statement, i);
                     if (!window->putLong(numRows, i, value)) {
                         window->freeLastRow();
-                        LOGE("Failed allocating space for a long in column %d", i);
+                        LOGD("Failed allocating space for a long in column %d", i);
                         return startPos + numRows + finish_program_and_get_row_count(statement) + 1;
                     }
                     LOG_WINDOW("%d,%d is INTEGER 0x%016llx", startPos + numRows, i, value);
@@ -234,7 +234,7 @@ static jint native_fill_window(JNIEnv* env, jobject object, jobject javaWindow,
                     double value = sqlite3_column_double(statement, i);
                     if (!window->putDouble(numRows, i, value)) {
                         window->freeLastRow();
-                        LOGE("Failed allocating space for a double in column %d", i);
+                        LOGD("Failed allocating space for a double in column %d", i);
                         return startPos + numRows + finish_program_and_get_row_count(statement) + 1;
                     }
                     LOG_WINDOW("%d,%d is FLOAT %lf", startPos + numRows, i, value);
@@ -245,7 +245,7 @@ static jint native_fill_window(JNIEnv* env, jobject object, jobject javaWindow,
                     int offset = window->alloc(size);
                     if (!offset) {
                         window->freeLastRow();
-                        LOGE("Failed allocating %u bytes for blob at %d,%d", size,
+                        LOGD("Failed allocating %u bytes for blob at %d,%d", size,
                                    startPos + numRows, i);
                         return startPos + numRows + finish_program_and_get_row_count(statement) + 1;
                     }
diff --git a/core/tests/coretests/src/android/database/DatabaseErrorHandlerTest.java b/core/tests/coretests/src/android/database/DatabaseErrorHandlerTest.java
new file mode 100644
index 0000000..48d25b9
--- /dev/null
+++ b/core/tests/coretests/src/android/database/DatabaseErrorHandlerTest.java
@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2010 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 android.database;
+
+import android.content.Context;
+import android.database.sqlite.SQLiteDatabase;
+import android.database.sqlite.SQLiteException;
+import android.test.AndroidTestCase;
+import android.util.Log;
+
+import java.io.BufferedWriter;
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+
+public class DatabaseErrorHandlerTest extends AndroidTestCase {
+
+    private SQLiteDatabase mDatabase;
+    private File mDatabaseFile;
+    private static final String DB_NAME = "database_test.db";
+    private File dbDir;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        dbDir = getContext().getDir(this.getClass().getName(), Context.MODE_PRIVATE);
+        mDatabaseFile = new File(dbDir, DB_NAME);
+        if (mDatabaseFile.exists()) {
+            mDatabaseFile.delete();
+        }
+        mDatabase = SQLiteDatabase.openOrCreateDatabase(mDatabaseFile.getPath(), null,
+                new MyDatabaseCorruptionHandler());
+        assertNotNull(mDatabase);
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        mDatabase.close();
+        mDatabaseFile.delete();
+        super.tearDown();
+    }
+
+    public void testNoCorruptionCase() {
+        new MyDatabaseCorruptionHandler().onCorruption(mDatabase);
+        // database file should still exist
+        assertTrue(mDatabaseFile.exists());
+    }
+
+    public void testDatabaseIsCorrupt() throws IOException {
+        mDatabase.execSQL("create table t (i int);");
+        // write junk into the database file
+        BufferedWriter writer = new BufferedWriter(new FileWriter(mDatabaseFile.getPath()));
+        writer.write("blah");
+        writer.close();
+        assertTrue(mDatabaseFile.exists());
+        // since the database file is now corrupt, doing any sql on this database connection
+        // should trigger call to MyDatabaseCorruptionHandler.onCorruption
+        try {
+            mDatabase.execSQL("select * from t;");
+            fail("expected exception");
+        } catch (SQLiteException e) {
+            // expected
+        }
+        // after corruption handler is called, the database file should be free of
+        // database corruption
+        SQLiteDatabase db = SQLiteDatabase.openOrCreateDatabase(mDatabaseFile.getPath(), null,
+                new MyDatabaseCorruptionHandler());
+        assertTrue(db.isDatabaseIntegrityOk());
+    }
+
+    /**
+     * An example implementation of {@link DatabaseErrorHandler} to demonstrate
+     * database corruption handler which checks to make sure database is indeed
+     * corrupt before deleting the file.
+     */
+    public class MyDatabaseCorruptionHandler implements DatabaseErrorHandler {
+        public void onCorruption(SQLiteDatabase dbObj) {
+            boolean databaseOk = dbObj.isDatabaseIntegrityOk();
+            // close the database
+            try {
+                dbObj.close();
+            } catch (SQLiteException e) {
+                /* ignore */
+            }
+            if (databaseOk) {
+                // database is just fine. no need to delete the database file
+                Log.e("MyDatabaseCorruptionHandler", "no corruption in the database: " +
+                        mDatabaseFile.getPath());
+            } else {
+                // database is corrupt. delete the database file
+                Log.e("MyDatabaseCorruptionHandler", "deleting the database file: " +
+                        mDatabaseFile.getPath());
+                new File(dbDir, DB_NAME).delete();
+            }
+        }
+    }
+}
\ No newline at end of file