Bug / Integer primary key conflict should upsert

core/src/androidTest/assets/migrations/129467607_create.sql

@@ -1,2 +1,4 @@
 CREATE TABLE 'parents' (_id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT UNIQUE, description TEXT, latitude REAL, longitude REAL, createdAt INTEGER);
 CREATE TABLE 'childs' (_id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, parent_id INTEGER NOT NULL, FOREIGN KEY(parent_id) REFERENCES parent(_id));
+-- This table intentionally has no Foreign Keys, no UNIQUE fields, no indexes, apart from the implicit index from the INTEGER PRIMARY KEY.
+CREATE TABLE 'integer_primary_key_table' (_id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT);

core/src/androidTest/java/novoda/lib/sqliteprovider/sqlite/InsertHelperTest.java

@@ -9,6 +9,30 @@
 
 public class InsertHelperTest extends AndroidTestCase {
 
+    private static final String A_NAME_VALUE = "a name";
+    private static final String DIFFERENT_NAME_VALUE = "another name, not the same";
+    private static final String A_DESCRIPTION_VALUE = "a description";
+    private static final String DIFFERENT_DESCRIPTION_VALUE = "a different description";
+    private static final String ID_COLUMN = "_id";
+
+    private class ParentsColumns {
+        static final String ID = ID_COLUMN;
+        static final String NAME = "name";
+        static final String DESCRIPTION = "description";
+    }
+
+    private class IntegerPrimaryKeyTableColumns {
+        static final String ID = ID_COLUMN;
+        static final String NAME = "name";
+    }
+
+    private static final String PARENTS_TABLE = "parents";
+    private static final String INTEGER_PRIMARY_KEY_TABLE = "integer_primary_key_table";
+
+    private static final String BASE_URI_STRING = "content://novoda.lib.sqliteprovider.test/";
+    private static final Uri PARENTS_URI = Uri.parse(BASE_URI_STRING + PARENTS_TABLE);
+    private static final Uri INTEGER_PRIMARY_KEY_TABLE_URI = Uri.parse(BASE_URI_STRING + INTEGER_PRIMARY_KEY_TABLE);
+
     private InsertHelper helper;
     private ExtendedSQLiteOpenHelper openHelper;
 
@@ -17,26 +41,109 @@ protected void setUp() throws Exception {
         super.setUp();
         openHelper = new ExtendedSQLiteOpenHelper(getContext());
         helper = new InsertHelper(openHelper);
+
+        clearParentsTable();
     }
 
-    public void testInsertWithKeyShouldInsertThenUpdate() throws Exception {
-        Uri uri = Uri.parse("content://novoda.lib.sqliteprovider.test/parents");
-        ContentValues values = new ContentValues();
-        values.put("name", "something unique");
-        values.put("description", "something");
-        helper.insert(uri, values);
+    public void testInsertWithNoConflictShouldInsert() {
+        helper.insert(PARENTS_URI, parentsContentValues(A_NAME_VALUE, A_DESCRIPTION_VALUE));
 
-        Cursor cursor = openHelper.getReadableDatabase().rawQuery("select * from parents", null);
-        assertEquals(1, cursor.getCount());
-        cursor.close();
+        onQueryOf(PARENTS_TABLE, new CursorOperations() {
+            @Override
+            public void doOperationsOn(Cursor cursor) {
+                assertEquals(1, cursor.getCount());
+                assertEquals(A_NAME_VALUE, stringFrom(cursor, ParentsColumns.NAME));
+                assertEquals(A_DESCRIPTION_VALUE, stringFrom(cursor, ParentsColumns.DESCRIPTION));
+            }
+        });
+    }
+
+    public void testInsertWithUniqueColumnConflictShouldUpdate() {
+        helper.insert(PARENTS_URI, parentsContentValues(A_NAME_VALUE, A_DESCRIPTION_VALUE));
+        final int existingRowId = idOfFirstRowIn(queryOf(PARENTS_TABLE));
+
+        helper.insert(PARENTS_URI, parentsContentValues(A_NAME_VALUE, DIFFERENT_DESCRIPTION_VALUE));
+
+        onQueryOf(PARENTS_TABLE, new CursorOperations() {
+            @Override
+            public void doOperationsOn(Cursor cursor) {
+                assertEquals(1, cursor.getCount());
+                assertEquals(existingRowId, intFrom(cursor, ParentsColumns.ID));
+                assertEquals(A_NAME_VALUE, stringFrom(cursor, ParentsColumns.NAME));
+                assertEquals(DIFFERENT_DESCRIPTION_VALUE, stringFrom(cursor, ParentsColumns.DESCRIPTION));
+            }
+        });
+    }
+
+    public void testInsertWithIntegerPrimaryKeyConflictShouldUpdate() {
+        helper.insert(INTEGER_PRIMARY_KEY_TABLE_URI, integerPrimaryKeyTableContentValues(null, A_NAME_VALUE));
+        final int existingRowId = idOfFirstRowIn(queryOf(INTEGER_PRIMARY_KEY_TABLE));
 
-        values.put("description", "else");
-        helper.insert(uri, values);
+        helper.insert(INTEGER_PRIMARY_KEY_TABLE_URI, integerPrimaryKeyTableContentValues(existingRowId, DIFFERENT_NAME_VALUE));
+
+        onQueryOf(INTEGER_PRIMARY_KEY_TABLE, new CursorOperations() {
+            @Override
+            public void doOperationsOn(Cursor cursor) {
+                assertEquals(1, cursor.getCount());
+                assertEquals(existingRowId, intFrom(cursor, ParentsColumns.ID));
+                assertEquals(DIFFERENT_NAME_VALUE, stringFrom(cursor, ParentsColumns.NAME));
+            }
+        });
+    }
 
-        cursor = openHelper.getReadableDatabase().rawQuery("select * from parents", null);
-        assertEquals(1, cursor.getCount());
+    private String stringFrom(Cursor cursor, String columnName) {
+        return cursor.getString(cursor.getColumnIndex(columnName));
+    }
+
+    private int intFrom(Cursor cursor, String columnName) {
+        return cursor.getInt(cursor.getColumnIndex(columnName));
+    }
+
+    private int idOfFirstRowIn(Cursor cursor) {
+        try {
+            assertTrue("No rows in table!", cursor.getCount() > 0);
+            cursor.moveToFirst();
+            return intFrom(cursor, ID_COLUMN);
+        } finally {
+            cursor.close();
+        }
+    }
+
+    private void clearParentsTable() {
+        openHelper.getWritableDatabase().delete(PARENTS_TABLE, null, null);
+    }
+
+    private ContentValues parentsContentValues(String name, String description) {
+        ContentValues contentValues = new ContentValues();
+        contentValues.put(ParentsColumns.NAME, name);
+        contentValues.put(ParentsColumns.DESCRIPTION, description);
+        return contentValues;
+    }
+
+    private ContentValues integerPrimaryKeyTableContentValues(Integer existingRowId, String name) {
+        ContentValues contentValues = new ContentValues();
+        if (existingRowId != null) {
+            contentValues.put(IntegerPrimaryKeyTableColumns.ID, existingRowId);
+        }
+        contentValues.put(IntegerPrimaryKeyTableColumns.NAME, name);
+        return contentValues;
+    }
+
+    private Cursor queryOf(String tableName) {
+        return openHelper.getReadableDatabase().rawQuery("select * from " + tableName, null);
+    }
+
+    private void onQueryOf(String tableName, CursorOperations cursorOperations) {
+        Cursor cursor = queryOf(tableName);
         cursor.moveToFirst();
-        assertEquals("else", cursor.getString(cursor.getColumnIndex("description")));
+        try {
+            cursorOperations.doOperationsOn(cursor);
+        } finally {
+            cursor.close();
+        }
+    }
 
+    private static interface CursorOperations {
+        void doOperationsOn(Cursor cursor);
     }
 }

core/src/androidTest/java/novoda/lib/sqliteprovider/util/DBUtilsTest.java

@@ -19,6 +19,7 @@ public class DBUtilsTest extends AndroidTestCase {
     private static final String CREATE_2_TABLES_WITH_FOREIGN_KEY = "CREATE TABLE t(id INTEGER);\nCREATE TABLE t2(id INTEGER, t_id INTEGER);\n";
     private static final String CREATE_TABLE_WITH_CONSTRAINT = "CREATE TABLE t(id INTEGER, const TEXT UNIQUE NOT NULL);";
     private static final String CREATE_TABLE_WITH_MULTI_COLUMN_CONSTRAINT = "CREATE TABLE t(id INTEGER, name TEXT, desc TEXT NOT NULL, UNIQUE(name, desc) ON CONFLICT REPLACE);";
+    private static final String CREATE_TABLE_WITH_INTEGER_PRIMARY_KEY = "CREATE TABLE t(_id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT);";
 
     @Override
     protected void setUp() throws Exception {
@@ -71,6 +72,14 @@ public void testGettingUniqueConstraints() throws Exception {
         MoreAsserts.assertContentsInAnyOrder(constrains, new Constraint(Arrays.asList("const")));
     }
 
+    public void testGettingUniqueConstraintForIntegerPrimaryKey() throws Exception {
+        android.database.DatabaseUtils.createDbFromSqlStatements(getContext(), DB_NAME, 1, CREATE_TABLE_WITH_INTEGER_PRIMARY_KEY);
+
+        SQLiteDatabase db = getContext().openOrCreateDatabase(DB_NAME, 0, null);
+        List<Constraint> constrains = DBUtils.getUniqueConstraints(db, "t");
+        MoreAsserts.assertContentsInAnyOrder(constrains, new Constraint(Arrays.asList("_id")));
+    }
+
     public void testGettingMultiColumnUniqueConstraints() throws Exception {
         android.database.DatabaseUtils.createDbFromSqlStatements(getContext(), DB_NAME, 1, CREATE_TABLE_WITH_MULTI_COLUMN_CONSTRAINT);
 

core/src/main/java/novoda/lib/sqliteprovider/sqlite/IDatabaseMetaInfo.java

@@ -12,6 +12,10 @@ public interface IDatabaseMetaInfo {
 
     public enum SQLiteType {
         NULL, INTEGER, REAL, TEXT, BLOB, NUMERIC;
+
+        public static SQLiteType fromName(String columnType) {
+            return valueOf(columnType.toUpperCase());
+        }
     }
     Map<String, SQLiteType> getColumns(String table);
 

core/src/main/java/novoda/lib/sqliteprovider/util/DBUtils.java

@@ -18,12 +18,18 @@ public final class DBUtils {
 
     private static final String SELECT_TABLES_NAME = "SELECT name FROM sqlite_master WHERE type='table';";
 
-    private static final String PRAGMA_TABLE = "PRAGMA table_info(\"%1$s\");";
+    private static final String PRAGMA_TABLE_INFO = "PRAGMA table_info('%1$s');";
 
     private static final String PRGAMA_INDEX_LIST = "PRAGMA index_list('%1$s');";
 
     private static final String PRGAMA_INDEX_INFO = "PRAGMA index_info('%1$s');";
 
+    private static final String COLUMN_PRIMARY_KEY = "pk";
+
+    private static final String COLUMN_NAME = "name";
+
+    private static final String COLUMN_TYPE = "type";
+
     private static List<String> defaultTables = Arrays.asList(new String[]{
             "android_metadata"
     });
@@ -33,13 +39,13 @@ private DBUtils() {
     }
 
     public static List<String> getForeignTables(SQLiteDatabase db, String table) {
-        final Cursor cur = db.rawQuery(String.format(PRAGMA_TABLE, table), null);
+        final Cursor cur = queryTableColumnsFor(table, db);
         List<String> tables = getTables(db);
         List<String> foreignTables = new ArrayList<String>(5);
         String name;
         String tableName;
         while (cur.moveToNext()) {
-            name = cur.getString(cur.getColumnIndexOrThrow("name"));
+            name = cur.getString(cur.getColumnIndexOrThrow(COLUMN_NAME));
             if (name.endsWith("_id")) {
                 tableName = name.substring(0, name.lastIndexOf('_'));
                 if (tables.contains(tableName + "s")) {
@@ -89,16 +95,16 @@ public static Map<String, String> getProjectionMap(SQLiteDatabase db, String par
     }
 
     public static Map<String, SQLiteType> getFields(SQLiteDatabase db, String table) {
-        final Cursor cur = db.rawQuery(String.format(PRAGMA_TABLE, table), null);
-        Map<String, SQLiteType> fields = new HashMap<String, SQLiteType>(cur.getCount());
+        final Cursor cursor = queryTableColumnsFor(table, db);
+        Map<String, SQLiteType> fields = new HashMap<String, SQLiteType>(cursor.getCount());
         String name;
         String type;
-        while (cur.moveToNext()) {
-            name = cur.getString(cur.getColumnIndexOrThrow("name"));
-            type = cur.getString(cur.getColumnIndexOrThrow("type"));
-            fields.put(name, SQLiteType.valueOf(type.toUpperCase()));
+        while (cursor.moveToNext()) {
+            name = cursor.getString(cursor.getColumnIndexOrThrow(COLUMN_NAME));
+            type = cursor.getString(cursor.getColumnIndexOrThrow(COLUMN_TYPE));
+            fields.put(name, SQLiteType.fromName(type));
         }
-        cur.close();
+        cursor.close();
         return Collections.unmodifiableMap(fields);
     }
 
@@ -125,31 +131,38 @@ public static String getSQLiteVersion() {
     @Deprecated
     public static List<String> getUniqueConstrains(SQLiteDatabase db, String table) {
         List<String> constrains = new ArrayList<String>();
-        final Cursor pragmas = db.rawQuery(String.format(PRGAMA_INDEX_LIST, table), null);
-        while (pragmas.moveToNext()) {
-            int isUnique = pragmas.getInt(2);
+        final Cursor indices = queryIndexListForTable(table, db);
+        while (indices.moveToNext()) {
+            int isUnique = indices.getInt(2);
             if (isUnique == 1) {
-                String name = pragmas.getString(1);
-                final Cursor pragmaInfo = db.rawQuery(String.format(PRGAMA_INDEX_INFO, name), null);
+                String indexName = indices.getString(1);
+                final Cursor pragmaInfo = queryIndexInfo(indexName, db);
                 if (pragmaInfo.moveToFirst()) {
                     constrains.add(pragmaInfo.getString(2));
                 }
                 pragmaInfo.close();
             }
         }
-        pragmas.close();
+        indices.close();
         return constrains;
     }
 
 
     public static List<Constraint> getUniqueConstraints(SQLiteDatabase db, String table) {
         List<Constraint> constraints = new ArrayList<Constraint>();
-        final Cursor indexCursor = db.rawQuery(String.format(PRGAMA_INDEX_LIST, table), null);
+
+        // This is an implicit unique index, and won't show up querying the other indexes
+        Constraint integerPrimaryKeyConstraint = findIntegerPrimaryKeyConstraint(db, table);
+        if (integerPrimaryKeyConstraint != null) {
+            constraints.add(integerPrimaryKeyConstraint);
+        }
+
+        final Cursor indexCursor = queryIndexListForTable(table, db);
         while (indexCursor.moveToNext()) {
             int isUnique = indexCursor.getInt(2);
             if (isUnique == 1) {
                 String indexName = indexCursor.getString(1);
-                final Cursor columnCursor = db.rawQuery(String.format(PRGAMA_INDEX_INFO, indexName), null);
+                final Cursor columnCursor = queryIndexInfo(indexName, db);
                 List<String> columns = new ArrayList<>(columnCursor.getCount());
                 while (columnCursor.moveToNext()) {
                     String columnName = columnCursor.getString(2);
@@ -162,4 +175,42 @@ public static List<Constraint> getUniqueConstraints(SQLiteDatabase db, String ta
         indexCursor.close();
         return constraints;
     }
+
+    private static Constraint findIntegerPrimaryKeyConstraint(SQLiteDatabase database, String table) {
+        final Cursor cursor = queryTableColumnsFor(table, database);
+        try {
+            while (cursor.moveToNext()) {
+                if (isTableInfoItemAnIntegerPrimaryKey(cursor)) {
+                    String columnName = cursor.getString(cursor.getColumnIndex(COLUMN_NAME));
+                    return new Constraint(Collections.singletonList(columnName));
+                }
+            }
+        } finally {
+            cursor.close();
+        }
+        return null;
+    }
+
+    private static Cursor queryTableColumnsFor(String table, SQLiteDatabase database) {
+        return database.rawQuery(String.format(PRAGMA_TABLE_INFO, table), null);
+    }
+
+    private static Cursor queryIndexListForTable(String table, SQLiteDatabase database) {
+        return database.rawQuery(String.format(PRGAMA_INDEX_LIST, table), null);
+    }
+
+    private static Cursor queryIndexInfo(String index, SQLiteDatabase database) {
+        return database.rawQuery(String.format(PRGAMA_INDEX_INFO, index), null);
+    }
+
+    private static boolean isTableInfoItemAnIntegerPrimaryKey(Cursor cursor) {
+        int pkInt = cursor.getInt(cursor.getColumnIndex(COLUMN_PRIMARY_KEY));
+        String columnType = cursor.getString(cursor.getColumnIndex(COLUMN_TYPE));
+
+        boolean isPrimaryKey = pkInt == 1;
+        boolean isInteger = SQLiteType.fromName(columnType) == SQLiteType.INTEGER;
+
+        return isPrimaryKey && isInteger;
+    }
+
 }