From c2255296d7c9e96a9321555f656812ab1d6f699e Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Fri, 5 Jan 2018 13:03:05 -0600 Subject: [PATCH] Begin conversion to Room --- .../39.json} | 10 +- .../java/org/tasks/injection/TestModule.java | 1 - .../com/todoroo/andlib/data/DatabaseDao.java | 3 +- .../java/com/todoroo/astrid/dao/Database.java | 229 +++--------------- .../main/java/org/tasks/db/AppDatabase.java | 12 - .../main/java/org/tasks/db/Migrations.java | 107 ++++++++ .../tasks/injection/ApplicationModule.java | 18 +- .../org/tasks/notifications/Notification.java | 2 +- .../NotificationClearedReceiver.java | 2 - .../notifications/NotificationManager.java | 6 +- 10 files changed, 162 insertions(+), 228 deletions(-) rename app/schemas/{org.tasks.db.AppDatabase/1.json => com.todoroo.astrid.dao.Database/39.json} (86%) delete mode 100644 app/src/main/java/org/tasks/db/AppDatabase.java create mode 100644 app/src/main/java/org/tasks/db/Migrations.java diff --git a/app/schemas/org.tasks.db.AppDatabase/1.json b/app/schemas/com.todoroo.astrid.dao.Database/39.json similarity index 86% rename from app/schemas/org.tasks.db.AppDatabase/1.json rename to app/schemas/com.todoroo.astrid.dao.Database/39.json index 35988ac97..efaa72c9e 100644 --- a/app/schemas/org.tasks.db.AppDatabase/1.json +++ b/app/schemas/com.todoroo.astrid.dao.Database/39.json @@ -1,12 +1,12 @@ { "formatVersion": 1, "database": { - "version": 1, - "identityHash": "eab7679fcfaa5fd45ac7da7a4b205348", + "version": 39, + "identityHash": "7e082aef9f43061a29426ac5c9489db0", "entities": [ { "tableName": "notification", - "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `task` INTEGER, `timestamp` INTEGER NOT NULL, `type` INTEGER NOT NULL)", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `task` INTEGER NOT NULL, `timestamp` INTEGER NOT NULL, `type` INTEGER NOT NULL)", "fields": [ { "fieldPath": "uid", @@ -18,7 +18,7 @@ "fieldPath": "taskId", "columnName": "task", "affinity": "INTEGER", - "notNull": false + "notNull": true }, { "fieldPath": "timestamp", @@ -54,7 +54,7 @@ ], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, \"eab7679fcfaa5fd45ac7da7a4b205348\")" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, \"7e082aef9f43061a29426ac5c9489db0\")" ] } } \ No newline at end of file diff --git a/app/src/androidTest/java/org/tasks/injection/TestModule.java b/app/src/androidTest/java/org/tasks/injection/TestModule.java index e3d1bf802..4bfee1bdf 100644 --- a/app/src/androidTest/java/org/tasks/injection/TestModule.java +++ b/app/src/androidTest/java/org/tasks/injection/TestModule.java @@ -6,7 +6,6 @@ import android.content.Context; import com.todoroo.astrid.dao.Database; import org.tasks.analytics.Tracker; -import org.tasks.db.AppDatabase; import org.tasks.notifications.NotificationDao; import org.tasks.preferences.PermissionChecker; import org.tasks.preferences.PermissivePermissionChecker; diff --git a/app/src/main/java/com/todoroo/andlib/data/DatabaseDao.java b/app/src/main/java/com/todoroo/andlib/data/DatabaseDao.java index 99987fe25..8ad692120 100644 --- a/app/src/main/java/com/todoroo/andlib/data/DatabaseDao.java +++ b/app/src/main/java/com/todoroo/andlib/data/DatabaseDao.java @@ -173,8 +173,7 @@ public class DatabaseDao { DatabaseChangeOp insert = new DatabaseChangeOp() { @Override public boolean makeChange() { - long newRow = database.insert(table.name, - AbstractModel.ID_PROPERTY.name, item.getMergedValues()); + long newRow = database.insert(table.name, item.getMergedValues()); boolean result = newRow >= 0; if (result) { item.setId(newRow); diff --git a/app/src/main/java/com/todoroo/astrid/dao/Database.java b/app/src/main/java/com/todoroo/astrid/dao/Database.java index fff9c861e..ee20953f4 100644 --- a/app/src/main/java/com/todoroo/astrid/dao/Database.java +++ b/app/src/main/java/com/todoroo/astrid/dao/Database.java @@ -5,18 +5,14 @@ */ package com.todoroo.astrid.dao; +import android.arch.persistence.db.SupportSQLiteDatabase; +import android.arch.persistence.room.RoomDatabase; import android.content.ContentValues; -import android.content.Context; import android.database.Cursor; import android.database.sqlite.SQLiteConstraintException; import android.database.sqlite.SQLiteDatabase; -import android.database.sqlite.SQLiteException; -import android.database.sqlite.SQLiteOpenHelper; -import android.text.TextUtils; import com.todoroo.andlib.data.AbstractModel; -import com.todoroo.andlib.data.Property; -import com.todoroo.andlib.data.SqlConstructorVisitor; import com.todoroo.andlib.data.Table; import com.todoroo.andlib.utility.AndroidUtilities; import com.todoroo.astrid.data.Metadata; @@ -26,13 +22,11 @@ import com.todoroo.astrid.data.Task; import com.todoroo.astrid.data.TaskAttachment; import com.todoroo.astrid.data.TaskListMetadata; import com.todoroo.astrid.data.UserActivity; -import com.todoroo.astrid.provider.Astrid2TaskProvider; -import org.tasks.analytics.Tracker; -import org.tasks.injection.ApplicationScope; -import org.tasks.injection.ForApplication; +import org.tasks.notifications.Notification; +import org.tasks.notifications.NotificationDao; -import javax.inject.Inject; +import java.io.IOException; import timber.log.Timber; @@ -42,12 +36,14 @@ import timber.log.Timber; * @author Tim Su * */ -@ApplicationScope -public class Database { +@android.arch.persistence.room.Database(entities = {Notification.class}, version = 39) +public abstract class Database extends RoomDatabase { - private static final int VERSION = 38; - private static final String NAME = "database"; - private static final Table[] TABLES = new Table[] { + public abstract NotificationDao notificationDao(); + + public static final String NAME = "database"; + + public static final Table[] TABLES = new Table[] { Task.TABLE, Metadata.TABLE, StoreObject.TABLE, @@ -57,19 +53,8 @@ public class Database { TaskListMetadata.TABLE, }; - private final SQLiteOpenHelper helper; - private final Context context; - private final Tracker tracker; - private SQLiteDatabase database; - - // --- listeners - - @Inject - public Database(@ForApplication Context context, Tracker tracker) { - this.context = context; - this.tracker = tracker; - helper = new DatabaseHelper(context, getName(), VERSION); - } + private SupportSQLiteDatabase database; + private Runnable onDatabaseUpdated; // --- implementation @@ -77,109 +62,17 @@ public class Database { return NAME; } - /** - * Create indices - */ - private void onCreateTables() { - StringBuilder sql = new StringBuilder(); - sql.append("CREATE INDEX IF NOT EXISTS md_tid ON "). - append(Metadata.TABLE).append('('). - append(Metadata.TASK.name). - append(')'); - database.execSQL(sql.toString()); - sql.setLength(0); - - sql.append("CREATE INDEX IF NOT EXISTS md_tkid ON "). - append(Metadata.TABLE).append('('). - append(Metadata.TASK.name).append(','). - append(Metadata.KEY.name). - append(')'); - database.execSQL(sql.toString()); - sql.setLength(0); - - sql.append("CREATE INDEX IF NOT EXISTS so_id ON "). - append(StoreObject.TABLE).append('('). - append(StoreObject.TYPE.name).append(','). - append(StoreObject.ITEM.name). - append(')'); - database.execSQL(sql.toString()); - sql.setLength(0); - - sql.append("CREATE UNIQUE INDEX IF NOT EXISTS t_rid ON "). - append(Task.TABLE).append('('). - append(Task.UUID.name). - append(')'); - database.execSQL(sql.toString()); - sql.setLength(0); - } - - private boolean onUpgrade(int oldVersion, int newVersion) { - SqlConstructorVisitor visitor = new SqlConstructorVisitor(); - switch(oldVersion) { - case 35: - tryExecSQL(addColumnSql(TagData.TABLE, TagData.COLOR, visitor, "-1")); - case 36: - tryExecSQL(addColumnSql(StoreObject.TABLE, StoreObject.DELETION_DATE, visitor, "0")); - case 37: - tryExecSQL(addColumnSql(StoreObject.TABLE, StoreObject.VALUE4, visitor, "-1")); - return true; - } - return false; - } - - private void tryExecSQL(String sql) { - try { - database.execSQL(sql); - } catch (SQLiteException e) { - tracker.reportException(e); - } - } - - private static String addColumnSql(Table table, Property property, SqlConstructorVisitor visitor, String defaultValue) { - StringBuilder builder = new StringBuilder(); - builder.append("ALTER TABLE ") - .append(table.name) - .append(" ADD ") - .append(property.accept(visitor, null)); - if (!TextUtils.isEmpty(defaultValue)) { - builder.append(" DEFAULT ").append(defaultValue); - } - return builder.toString(); - } - - private void tryAddColumn(Table table, Property column, String defaultValue) { - try { - SqlConstructorVisitor visitor = new SqlConstructorVisitor(); - String sql = "ALTER TABLE " + table.name + " ADD " + //$NON-NLS-1$//$NON-NLS-2$ - column.accept(visitor, null); - if (!TextUtils.isEmpty(defaultValue)) { - sql += " DEFAULT " + defaultValue; - } - database.execSQL(sql); - } catch (SQLiteException e) { - // ignored, column already exists - Timber.e(e, e.getMessage()); - } - } - private String createTableSql(SqlConstructorVisitor visitor, - String tableName, Property[] properties) { - StringBuilder sql = new StringBuilder(); - sql.append("CREATE TABLE IF NOT EXISTS ").append(tableName).append('('). - append(AbstractModel.ID_PROPERTY).append(" INTEGER PRIMARY KEY AUTOINCREMENT"); - for(Property property : properties) { - if(AbstractModel.ID_PROPERTY.name.equals(property.name)) { - continue; - } - sql.append(',').append(property.accept(visitor, null)); - } - sql.append(')'); - return sql.toString(); + public Database setOnDatabaseUpdated(Runnable onDatabaseUpdated) { + this.onDatabaseUpdated = onDatabaseUpdated; + return this; } private void onDatabaseUpdated() { - Astrid2TaskProvider.notifyDatabaseModification(context); + if (onDatabaseUpdated != null) { + onDatabaseUpdated.run(); + } } /** @@ -204,7 +97,7 @@ public class Database { } try { - database = helper.getWritableDatabase(); + database = getOpenHelper().getWritableDatabase(); } catch (NullPointerException e) { Timber.e(e, e.getMessage()); throw new IllegalStateException(e); @@ -228,7 +121,7 @@ public class Database { if(database != null && database.isOpen()) { return; } - database = helper.getReadableDatabase(); + database = getOpenHelper().getReadableDatabase(); } /** @@ -236,7 +129,11 @@ public class Database { */ public synchronized final void close() { if(database != null) { - database.close(); + try { + database.close(); + } catch (IOException e) { + Timber.e(e, e.getMessage()); + } } database = null; } @@ -244,7 +141,7 @@ public class Database { /** * @return sql database. opens database if not yet open */ - public synchronized final SQLiteDatabase getDatabase() { + public synchronized final SupportSQLiteDatabase getDatabase() { if(database == null) { AndroidUtilities.sleepDeep(300L); openForWriting(); @@ -263,13 +160,13 @@ public class Database { // --- database wrapper public Cursor rawQuery(String sql) { - return getDatabase().rawQuery(sql, null); + return getDatabase().query(sql, null); } - public long insert(String table, String nullColumnHack, ContentValues values) { + public long insert(String table, ContentValues values) { long result; try { - result = getDatabase().insertOrThrow(table, nullColumnHack, values); + result = getDatabase().insert(table, SQLiteDatabase.CONFLICT_REPLACE, values); } catch (SQLiteConstraintException e) { // Throw these exceptions throw e; } catch (Exception e) { // Suppress others @@ -287,71 +184,9 @@ public class Database { } public int update(String table, ContentValues values, String whereClause) { - int result = getDatabase().update(table, values, whereClause, null); + int result = getDatabase().update(table, SQLiteDatabase.CONFLICT_REPLACE, values, whereClause, null); onDatabaseUpdated(); return result; } - - // --- helper classes - - /** - * Default implementation of Astrid database helper - */ - private class DatabaseHelper extends SQLiteOpenHelper { - - public DatabaseHelper(Context context, String name, int version) { - super(context, name, null, version); - } - - /** - * Called to create the database tables - */ - @Override - public void onCreate(SQLiteDatabase db) { - StringBuilder sql = new StringBuilder(); - SqlConstructorVisitor sqlVisitor = new SqlConstructorVisitor(); - - // create tables - for(Table table : TABLES) { - sql.append("CREATE TABLE IF NOT EXISTS ").append(table.name).append('('). - append(AbstractModel.ID_PROPERTY).append(" INTEGER PRIMARY KEY AUTOINCREMENT"); - for(Property property : table.getProperties()) { - if(AbstractModel.ID_PROPERTY.name.equals(property.name)) { - continue; - } - sql.append(',').append(property.accept(sqlVisitor, null)); - } - sql.append(')'); - db.execSQL(sql.toString()); - sql.setLength(0); - } - - // post-table-creation - database = db; - onCreateTables(); - } - - /** - * Called to upgrade the database to a new version - */ - @Override - public void onUpgrade(SQLiteDatabase db, int oldVersion, int newVersion) { - Timber.i("Upgrading database from version %s to %s", oldVersion, newVersion); - - database = db; - try { - if(!Database.this.onUpgrade(oldVersion, newVersion)) { - // We don't know how to handle this case because someone forgot to - // implement the upgrade. We can't drop tables, we can only - // throw a nasty exception at this time - - throw new IllegalStateException("Missing database migration " + - "from " + oldVersion + " to " + newVersion); - } - } catch (Exception e) { - Timber.e(e, "database-upgrade-%s-%s-%s", getName(), oldVersion, newVersion); - } - } - } } diff --git a/app/src/main/java/org/tasks/db/AppDatabase.java b/app/src/main/java/org/tasks/db/AppDatabase.java deleted file mode 100644 index 3ceb02354..000000000 --- a/app/src/main/java/org/tasks/db/AppDatabase.java +++ /dev/null @@ -1,12 +0,0 @@ -package org.tasks.db; - -import android.arch.persistence.room.Database; -import android.arch.persistence.room.RoomDatabase; - -import org.tasks.notifications.Notification; -import org.tasks.notifications.NotificationDao; - -@Database(entities = {Notification.class}, version = 1) -public abstract class AppDatabase extends RoomDatabase { - public abstract NotificationDao notificationDao(); -} diff --git a/app/src/main/java/org/tasks/db/Migrations.java b/app/src/main/java/org/tasks/db/Migrations.java new file mode 100644 index 000000000..26b329ef6 --- /dev/null +++ b/app/src/main/java/org/tasks/db/Migrations.java @@ -0,0 +1,107 @@ +package org.tasks.db; + +import android.arch.persistence.db.SupportSQLiteDatabase; +import android.arch.persistence.room.RoomDatabase; +import android.arch.persistence.room.migration.Migration; +import android.support.annotation.NonNull; + +import com.todoroo.andlib.data.AbstractModel; +import com.todoroo.andlib.data.Property; +import com.todoroo.andlib.data.SqlConstructorVisitor; +import com.todoroo.andlib.data.Table; +import com.todoroo.astrid.dao.Database; +import com.todoroo.astrid.data.Metadata; +import com.todoroo.astrid.data.StoreObject; +import com.todoroo.astrid.data.Task; + +public class Migrations { + private static final Migration MIGRATION_35_36 = new Migration(35, 36) { + @Override + public void migrate(@NonNull SupportSQLiteDatabase database) { + database.execSQL("ALTER TABLE `tagdata` ADD COLUMN `color` INTEGER DEFAULT -1"); + } + }; + + private static final Migration MIGRATION_36_37 = new Migration(36, 37) { + @Override + public void migrate(@NonNull SupportSQLiteDatabase database) { + database.execSQL("ALTER TABLE `store` ADD COLUMN `deleted` INTEGER DEFAULT 0"); + } + }; + + private static final Migration MIGRATION_37_38 = new Migration(37, 38) { + @Override + public void migrate(@NonNull SupportSQLiteDatabase database) { + database.execSQL("ALTER TABLE `store` ADD COLUMN `value4` TEXT DEFAULT -1"); + } + }; + + private static final Migration MIGRATION_38_39 = new Migration(38, 39) { + @Override + public void migrate(@NonNull SupportSQLiteDatabase database) { + database.execSQL("CREATE TABLE IF NOT EXISTS `notification` (`uid` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `task` INTEGER NOT NULL, `timestamp` INTEGER NOT NULL, `type` INTEGER NOT NULL)"); + database.execSQL("CREATE UNIQUE INDEX `index_notification_task` ON `notification` (`task`)"); + } + }; + + public static Migration[] MIGRATIONS = new Migration[] { + MIGRATION_35_36, + MIGRATION_36_37, + MIGRATION_37_38, + MIGRATION_38_39 + }; + + public static RoomDatabase.Callback ON_CREATE = new RoomDatabase.Callback() { + @Override + public void onCreate(@NonNull SupportSQLiteDatabase db) { + StringBuilder sql = new StringBuilder(); + SqlConstructorVisitor sqlVisitor = new SqlConstructorVisitor(); + + // create tables + for(Table table : Database.TABLES) { + sql.append("CREATE TABLE IF NOT EXISTS ").append(table.name).append('('). + append(AbstractModel.ID_PROPERTY).append(" INTEGER PRIMARY KEY AUTOINCREMENT"); + for(Property property : table.getProperties()) { + if(AbstractModel.ID_PROPERTY.name.equals(property.name)) { + continue; + } + sql.append(',').append(property.accept(sqlVisitor, null)); + } + sql.append(')'); + db.execSQL(sql.toString()); + sql.setLength(0); + } + + sql.setLength(0); + sql.append("CREATE INDEX IF NOT EXISTS md_tid ON "). + append(Metadata.TABLE).append('('). + append(Metadata.TASK.name). + append(')'); + db.execSQL(sql.toString()); + sql.setLength(0); + + sql.append("CREATE INDEX IF NOT EXISTS md_tkid ON "). + append(Metadata.TABLE).append('('). + append(Metadata.TASK.name).append(','). + append(Metadata.KEY.name). + append(')'); + db.execSQL(sql.toString()); + sql.setLength(0); + + sql.append("CREATE INDEX IF NOT EXISTS so_id ON "). + append(StoreObject.TABLE).append('('). + append(StoreObject.TYPE.name).append(','). + append(StoreObject.ITEM.name). + append(')'); + db.execSQL(sql.toString()); + sql.setLength(0); + + sql.append("CREATE UNIQUE INDEX IF NOT EXISTS t_rid ON "). + append(Task.TABLE).append('('). + append(Task.UUID.name). + append(')'); + db.execSQL(sql.toString()); + sql.setLength(0); + } + }; +} diff --git a/app/src/main/java/org/tasks/injection/ApplicationModule.java b/app/src/main/java/org/tasks/injection/ApplicationModule.java index 4a284d717..dc03ca21b 100644 --- a/app/src/main/java/org/tasks/injection/ApplicationModule.java +++ b/app/src/main/java/org/tasks/injection/ApplicationModule.java @@ -3,9 +3,12 @@ package org.tasks.injection; import android.arch.persistence.room.Room; import android.content.Context; +import com.todoroo.astrid.dao.Database; +import com.todoroo.astrid.provider.Astrid2TaskProvider; + import org.tasks.ErrorReportingSingleThreadExecutor; import org.tasks.analytics.Tracker; -import org.tasks.db.AppDatabase; +import org.tasks.db.Migrations; import org.tasks.locale.Locale; import org.tasks.notifications.NotificationDao; @@ -44,12 +47,17 @@ public class ApplicationModule { @Provides @ApplicationScope - public AppDatabase getAppDatabase() { - return Room.databaseBuilder(context, AppDatabase.class, "app-database").build(); + public Database getAppDatabase() { + return Room + .databaseBuilder(context, Database.class, Database.NAME) + .addMigrations(Migrations.MIGRATIONS) + .addCallback(Migrations.ON_CREATE) + .build() + .setOnDatabaseUpdated(() -> Astrid2TaskProvider.notifyDatabaseModification(context)); } @Provides - public NotificationDao getNotificationDao(AppDatabase appDatabase) { - return appDatabase.notificationDao(); + public NotificationDao getNotificationDao(Database database) { + return database.notificationDao(); } } diff --git a/app/src/main/java/org/tasks/notifications/Notification.java b/app/src/main/java/org/tasks/notifications/Notification.java index bcdd5b2b3..db223f7c8 100644 --- a/app/src/main/java/org/tasks/notifications/Notification.java +++ b/app/src/main/java/org/tasks/notifications/Notification.java @@ -14,7 +14,7 @@ public class Notification { public int uid; @ColumnInfo(name = "task") - public Long taskId; + public long taskId; @ColumnInfo(name = "timestamp") public long timestamp; diff --git a/app/src/main/java/org/tasks/notifications/NotificationClearedReceiver.java b/app/src/main/java/org/tasks/notifications/NotificationClearedReceiver.java index 0d83dd400..2d54e4b8c 100644 --- a/app/src/main/java/org/tasks/notifications/NotificationClearedReceiver.java +++ b/app/src/main/java/org/tasks/notifications/NotificationClearedReceiver.java @@ -3,7 +3,6 @@ package org.tasks.notifications; import android.content.Context; import android.content.Intent; -import org.tasks.db.AppDatabase; import org.tasks.injection.BroadcastComponent; import org.tasks.injection.InjectingBroadcastReceiver; @@ -14,7 +13,6 @@ import timber.log.Timber; public class NotificationClearedReceiver extends InjectingBroadcastReceiver { @Inject NotificationManager notificationManager; - @Inject AppDatabase appDatabase; @Override public void onReceive(Context context, Intent intent) { diff --git a/app/src/main/java/org/tasks/notifications/NotificationManager.java b/app/src/main/java/org/tasks/notifications/NotificationManager.java index 62505dd1a..3b7bc3eec 100644 --- a/app/src/main/java/org/tasks/notifications/NotificationManager.java +++ b/app/src/main/java/org/tasks/notifications/NotificationManager.java @@ -139,7 +139,7 @@ public class NotificationManager { List notifications = notificationDao.getAllOrdered(); if (cancelExisting) { for (org.tasks.notifications.Notification notification : notifications) { - notificationManagerCompat.cancel(notification.taskId.intValue()); + notificationManagerCompat.cancel((int) notification.taskId); } } @@ -185,10 +185,10 @@ public class NotificationManager { for (org.tasks.notifications.Notification notification : notifications) { NotificationCompat.Builder builder = getTaskNotification(notification); if (builder == null) { - notificationManagerCompat.cancel(notification.taskId.intValue()); + notificationManagerCompat.cancel((int) notification.taskId); notificationDao.delete(notification.taskId); } else { - builder.setGroup(useGroupKey ? GROUP_KEY : (atLeastNougat() ? notification.taskId.toString() : null)) + builder.setGroup(useGroupKey ? GROUP_KEY : (atLeastNougat() ? Long.toString(notification.taskId) : null)) .setGroupAlertBehavior(alert ? NotificationCompat.GROUP_ALERT_CHILDREN : NotificationCompat.GROUP_ALERT_SUMMARY); notify(notification.taskId, builder, alert, nonstop, fiveTimes); alert = false;