From c830b7f7ecbedde86a7b55b4730e8d67e5ae2de1 Mon Sep 17 00:00:00 2001 From: Alex Baker Date: Thu, 25 Jan 2018 11:23:28 -0600 Subject: [PATCH] Fetch with Room --- .../com/todoroo/astrid/model/TaskTest.java | 4 +- .../todoroo/astrid/sync/SyncModelTest.java | 2 +- .../todoroo/andlib/data/AbstractModel.java | 138 +++------------- .../com/todoroo/andlib/data/DatabaseDao.java | 22 +-- .../todoroo/andlib/data/TodorooCursor.java | 8 - .../java/com/todoroo/astrid/dao/TaskDao.java | 5 +- .../java/com/todoroo/astrid/data/Task.java | 151 ++++++++++++++---- 7 files changed, 152 insertions(+), 178 deletions(-) diff --git a/app/src/androidTest/java/com/todoroo/astrid/model/TaskTest.java b/app/src/androidTest/java/com/todoroo/astrid/model/TaskTest.java index 8d6488ff4..80b23b470 100644 --- a/app/src/androidTest/java/com/todoroo/astrid/model/TaskTest.java +++ b/app/src/androidTest/java/com/todoroo/astrid/model/TaskTest.java @@ -2,6 +2,7 @@ package com.todoroo.astrid.model; import android.support.test.runner.AndroidJUnit4; +import com.todoroo.andlib.data.AbstractModel; import com.todoroo.astrid.dao.TaskDao; import com.todoroo.astrid.data.Task; @@ -48,13 +49,12 @@ public class TaskTest extends InjectingTestCase { taskDao.save(task); final Task fromDb = taskDao.fetch(task.getId()); assertEquals(task, fromDb); - compareRemoteModel(task, fromDb); } @Test public void testDefaults() { preferences.setDefaults(); - Map defaults = new Task().getDefaultValues(); + Map> defaults = new Task().getRoomGetters(); assertTrue(defaults.containsKey(Task.TITLE.name)); assertTrue(defaults.containsKey(Task.DUE_DATE.name)); assertTrue(defaults.containsKey(Task.HIDE_UNTIL.name)); diff --git a/app/src/androidTest/java/com/todoroo/astrid/sync/SyncModelTest.java b/app/src/androidTest/java/com/todoroo/astrid/sync/SyncModelTest.java index b3f142259..0a5fae27d 100644 --- a/app/src/androidTest/java/com/todoroo/astrid/sync/SyncModelTest.java +++ b/app/src/androidTest/java/com/todoroo/astrid/sync/SyncModelTest.java @@ -16,7 +16,7 @@ public class SyncModelTest extends NewSyncTestCase { @Test public void testCreateTaskMakesUuid() { Task task = createTask(); - assertFalse(Task.NO_UUID.equals(task.getUUID())); + assertFalse(Task.NO_UUID.equals(task.getUuid())); } @Test diff --git a/app/src/main/java/com/todoroo/andlib/data/AbstractModel.java b/app/src/main/java/com/todoroo/andlib/data/AbstractModel.java index bf8d8d742..882c61cb1 100644 --- a/app/src/main/java/com/todoroo/andlib/data/AbstractModel.java +++ b/app/src/main/java/com/todoroo/andlib/data/AbstractModel.java @@ -7,15 +7,13 @@ package com.todoroo.andlib.data; import android.arch.persistence.room.Ignore; import android.content.ContentValues; -import android.os.Parcel; -import android.os.Parcelable; import com.todoroo.andlib.data.Property.IntegerProperty; import com.todoroo.andlib.data.Property.LongProperty; import com.todoroo.andlib.data.Property.PropertyVisitor; import com.todoroo.andlib.utility.AndroidUtilities; +import com.todoroo.astrid.data.Task; -import java.lang.reflect.Array; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -34,7 +32,7 @@ import static com.todoroo.andlib.data.Property.DoubleProperty; * @author Tim Su * */ -public abstract class AbstractModel implements Parcelable, Cloneable { +public abstract class AbstractModel { private static final ContentValuesSavingVisitor saver = new ContentValuesSavingVisitor(); @@ -49,8 +47,12 @@ public abstract class AbstractModel implements Parcelable, Cloneable { // --- abstract methods + public interface ValueReader { + TYPE getValue(Task instance); + } + /** Get the default values for this object */ - abstract public Map getDefaultValues(); + abstract public Map> getRoomGetters(); // --- data store variables and management @@ -58,7 +60,7 @@ public abstract class AbstractModel implements Parcelable, Cloneable { * * In order to return the best data, we want to check first what the user * has explicitly set (setValues), then the values we have read out of - * the database (values), then defaults (getDefaultValues) + * the database (values), then defaults (getRoomGetters) */ /** User set values */ @@ -70,9 +72,8 @@ public abstract class AbstractModel implements Parcelable, Cloneable { protected ContentValues values = null; /** Transitory Metadata (not saved in database) */ - @SuppressWarnings("WeakerAccess") @Ignore - HashMap transitoryData = null; + protected HashMap transitoryData = null; protected AbstractModel() { } @@ -90,10 +91,13 @@ public abstract class AbstractModel implements Parcelable, Cloneable { public ContentValues getMergedValues() { ContentValues mergedValues = new ContentValues(); - for (Map.Entry entry : getDefaultValues().entrySet()) { - AndroidUtilities.putInto(mergedValues, entry.getKey(), entry.getValue()); + for (Map.Entry> entry : getRoomGetters().entrySet()) { + Object value = entry.getValue().getValue((Task) this); + if (value != null) { + AndroidUtilities.putInto(mergedValues, entry.getKey(), value); + } } - if(values != null) { + if (values != null) { mergedValues.putAll(values); } if(setValues != null) { @@ -139,23 +143,6 @@ public abstract class AbstractModel implements Parcelable, Cloneable { return getClass().getSimpleName() + "\n" + "set values:\n" + setValues + "\n" + "values:\n" + values + "\n"; } - @Override - public AbstractModel clone() { - AbstractModel clone; - try { - clone = (AbstractModel) super.clone(); - } catch (CloneNotSupportedException e) { - throw new RuntimeException(e); - } - if(setValues != null) { - clone.setValues = new ContentValues(setValues); - } - if(values != null) { - clone.values = new ContentValues(values); - } - return clone; - } - /** * Reads all properties from the supplied cursor and store */ @@ -188,8 +175,8 @@ public abstract class AbstractModel implements Parcelable, Cloneable { value = setValues.get(columnName); } else if(values != null && values.containsKey(columnName)) { value = values.get(columnName); - } else if(getDefaultValues().containsKey(columnName)) { - value = getDefaultValues().get(columnName); + } else if(getRoomGetters().containsKey(columnName)) { + value = getRoomGetters().get(columnName).getValue((Task) this); } else { throw new UnsupportedOperationException( "Model Error: Did not read property " + property.name); //$NON-NLS-1$ @@ -209,7 +196,7 @@ public abstract class AbstractModel implements Parcelable, Cloneable { return (TYPE) value; } catch (NumberFormatException e) { Timber.e(e, e.getMessage()); - return (TYPE) getDefaultValues().get(property.name); + return (TYPE) getRoomGetters().get(property.name).getValue((Task) this); } } @@ -220,16 +207,6 @@ public abstract class AbstractModel implements Parcelable, Cloneable { */ abstract public long getId(); - protected long getIdHelper(LongProperty id) { - if(setValues != null && setValues.containsKey(id.name)) { - return setValues.getAsLong(id.name); - } else if(values != null && values.containsKey(id.name)) { - return values.getAsLong(id.name); - } else { - return NO_ID; - } - } - public void setId(long id) { if (setValues == null) { setValues = new ContentValues(); @@ -268,24 +245,19 @@ public abstract class AbstractModel implements Parcelable, Cloneable { * Check whether the user has changed this property value and it should be * stored for saving in the database */ - private synchronized boolean shouldSaveValue( - Property property, TYPE newValue) { - + private synchronized boolean shouldSaveValue(Property property, TYPE newValue) { // we've already decided to save it, so overwrite old value if (setValues.containsKey(property.getColumnName())) { return true; } - // values contains this key, we should check it out - if(values != null && values.containsKey(property.getColumnName())) { - TYPE value = getValue(property); - if (value == null) { - if (newValue == null) { - return false; - } - } else if (value.equals(newValue)) { + TYPE value = getValue(property); + if (value == null) { + if (newValue == null) { return false; } + } else if (value.equals(newValue)) { + return false; } // otherwise, good to save @@ -414,66 +386,4 @@ public abstract class AbstractModel implements Parcelable, Cloneable { return null; } } - - // --- parcelable helpers - - /** - * {@inheritDoc} - */ - @Override - public int describeContents() { - return 0; - } - - /** - * {@inheritDoc} - */ - @Override - public void writeToParcel(Parcel dest, int flags) { - dest.writeParcelable(setValues, 0); - dest.writeParcelable(values, 0); - dest.writeMap(transitoryData); - } - - /** - * Parcelable creator helper - */ - protected static final class ModelCreator - implements Parcelable.Creator { - - private final Class cls; - - public ModelCreator(Class cls) { - super(); - this.cls = cls; - } - - /** - * {@inheritDoc} - */ - @Override - public TYPE createFromParcel(Parcel source) { - TYPE model; - try { - model = cls.newInstance(); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } catch (InstantiationException e) { - throw new RuntimeException(e); - } - model.setValues = source.readParcelable(ContentValues.class.getClassLoader()); - model.values = source.readParcelable(ContentValues.class.getClassLoader()); - model.transitoryData = source.readHashMap(ContentValues.class.getClassLoader()); - return model; - } - - /** - * {@inheritDoc} - */ - @Override - public TYPE[] newArray(int size) { - return (TYPE[]) Array.newInstance(cls, size); - } - } - } 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 8cdbc31dd..eaf2037d2 100644 --- a/app/src/main/java/com/todoroo/andlib/data/DatabaseDao.java +++ b/app/src/main/java/com/todoroo/andlib/data/DatabaseDao.java @@ -42,10 +42,6 @@ public class DatabaseDao { return query(query).toList(); } - private Task getFirst(Query query) { - return query(query).first(); - } - /** * Construct a query with SQL DSL objects */ @@ -59,18 +55,6 @@ public class DatabaseDao { return new TodorooCursor(cursor, query.getFields()); } - /** - * Returns object corresponding to the given identifier - * @param properties - * properties to read - * @param id - * id of item - * @return null if no item found - */ - public Task fetch(long id, Property... properties) { - return getFirst(Query.select(properties).where(AbstractModel.ID_PROPERTY.eq(id))); - } - /** * Delete the given id * @return true if delete was successful @@ -138,12 +122,12 @@ public class DatabaseDao { item.setUuid(UUIDHelper.newUUID()); } - item.clearValue(AbstractModel.ID_PROPERTY); - DatabaseChangeOp insert = new DatabaseChangeOp() { @Override public boolean makeChange() { - long newRow = database.insert(table.name, item.getMergedValues()); + ContentValues mergedValues = item.getMergedValues(); + mergedValues.remove(AbstractModel.ID_PROPERTY.name); + long newRow = database.insert(table.name, mergedValues); boolean result = newRow >= 0; if (result) { item.setId(newRow); diff --git a/app/src/main/java/com/todoroo/andlib/data/TodorooCursor.java b/app/src/main/java/com/todoroo/andlib/data/TodorooCursor.java index bfed12eea..83394defe 100644 --- a/app/src/main/java/com/todoroo/andlib/data/TodorooCursor.java +++ b/app/src/main/java/com/todoroo/andlib/data/TodorooCursor.java @@ -64,14 +64,6 @@ public class TodorooCursor extends CursorWrapper { return new Task(this); } - public Task first() { - try { - return moveToFirst() ? toModel() : null; - } finally { - close(); - } - } - /** * Get the value for the given property on the underlying {@link Cursor} * diff --git a/app/src/main/java/com/todoroo/astrid/dao/TaskDao.java b/app/src/main/java/com/todoroo/astrid/dao/TaskDao.java index ff97d2f21..5c91b9093 100644 --- a/app/src/main/java/com/todoroo/astrid/dao/TaskDao.java +++ b/app/src/main/java/com/todoroo/astrid/dao/TaskDao.java @@ -79,9 +79,8 @@ public abstract class TaskDao { return dao.toList(Query.select(Task.PROPERTIES).where(Criterion.and(TaskCriteria.isActive(), criterion))); } - public Task fetch(long id) { - return dao.fetch(id, Task.PROPERTIES); - } + @android.arch.persistence.room.Query("SELECT * FROM tasks WHERE _id = :id LIMIT 1") + public abstract Task fetch(long id); public int count(Filter filter) { String query = PermaSql.replacePlaceholders(filter.getSqlQuery()); diff --git a/app/src/main/java/com/todoroo/astrid/data/Task.java b/app/src/main/java/com/todoroo/astrid/data/Task.java index 331103e47..5eba2e617 100644 --- a/app/src/main/java/com/todoroo/astrid/data/Task.java +++ b/app/src/main/java/com/todoroo/astrid/data/Task.java @@ -12,8 +12,11 @@ import android.arch.persistence.room.Ignore; import android.arch.persistence.room.Index; import android.arch.persistence.room.PrimaryKey; import android.content.ContentValues; +import android.os.Parcel; +import android.os.Parcelable; import android.text.TextUtils; +import com.google.common.base.Strings; import com.google.ical.values.RRule; import com.todoroo.andlib.data.AbstractModel; import com.todoroo.andlib.data.Property; @@ -41,7 +44,7 @@ import static org.tasks.date.DateTimeUtils.newDateTime; */ @Entity(tableName = "tasks", indices = @Index(name = "t_rid", value = "remoteId", unique = true)) -public class Task extends AbstractModel { +public class Task extends AbstractModel implements Parcelable { // --- table and uri @@ -53,7 +56,7 @@ public class Task extends AbstractModel { /** ID */ @PrimaryKey(autoGenerate = true) @ColumnInfo(name = "_id") - public Long id; + public Long id = NO_ID; public static final LongProperty ID = new LongProperty( TABLE, ID_PROPERTY_NAME); @@ -83,13 +86,13 @@ public class Task extends AbstractModel { /** Unixtime Task was created */ @ColumnInfo(name = "created") - public Long created; + public Long created = 0L; public static final LongProperty CREATION_DATE = new LongProperty( TABLE, "created"); /** Unixtime Task was last touched */ @ColumnInfo(name = "modified") - public Long modified; + public Long modified = 0L; public static final LongProperty MODIFICATION_DATE = new LongProperty( TABLE, "modified"); @@ -226,32 +229,35 @@ public class Task extends AbstractModel { // --- defaults /** Default values container */ - private static final Map defaultValues = new HashMap<>(); + private static final Map> roomGetters = new HashMap<>(); static { - defaultValues.put(TITLE.name, ""); - defaultValues.put(DUE_DATE.name, 0L); - defaultValues.put(HIDE_UNTIL.name, 0); - defaultValues.put(COMPLETION_DATE.name, 0); - defaultValues.put(DELETION_DATE.name, 0); - defaultValues.put(IMPORTANCE.name, IMPORTANCE_NONE); - defaultValues.put(CALENDAR_URI.name, ""); - defaultValues.put(RECURRENCE.name, ""); - defaultValues.put(REPEAT_UNTIL.name, 0L); - defaultValues.put(REMINDER_PERIOD.name, 0); - defaultValues.put(REMINDER_FLAGS.name, 0); - defaultValues.put(REMINDER_LAST.name, 0); - defaultValues.put(REMINDER_SNOOZE.name, 0); - defaultValues.put(ESTIMATED_SECONDS.name, 0); - defaultValues.put(ELAPSED_SECONDS.name, 0); - defaultValues.put(NOTES.name, ""); - defaultValues.put(TIMER_START.name, 0); - defaultValues.put(UUID.name, NO_UUID); + roomGetters.put(CALENDAR_URI.name, t -> t.calendarUri); + roomGetters.put(COMPLETION_DATE.name, t -> t.completed); + roomGetters.put(CREATION_DATE.name, t -> t.created); + roomGetters.put(DELETION_DATE.name, t -> t.deleted); + roomGetters.put(DUE_DATE.name, t -> t.dueDate); + roomGetters.put(ELAPSED_SECONDS.name, t -> t.elapsedSeconds); + roomGetters.put(ESTIMATED_SECONDS.name, t -> t.estimatedSeconds); + roomGetters.put(HIDE_UNTIL.name, t -> t.hideUntil); + roomGetters.put(ID.name, t -> t.id); + roomGetters.put(IMPORTANCE.name, t -> t.importance); + roomGetters.put(MODIFICATION_DATE.name, t -> t.modified); + roomGetters.put(NOTES.name, t -> t.notes); + roomGetters.put(RECURRENCE.name, t -> t.recurrence); + roomGetters.put(REMINDER_FLAGS.name, t -> t.notificationFlags); + roomGetters.put(REMINDER_LAST.name, t -> t.lastNotified); + roomGetters.put(REMINDER_PERIOD.name, t -> t.notifications); + roomGetters.put(REMINDER_SNOOZE.name, t -> t.snoozeTime); + roomGetters.put(REPEAT_UNTIL.name, t -> t.repeatUntil); + roomGetters.put(TIMER_START.name, t -> t.timerStart); + roomGetters.put(TITLE.name, t -> t.title); + roomGetters.put(UUID.name, t -> t.remoteId); } @Override - public Map getDefaultValues() { - return defaultValues; + public Map> getRoomGetters() { + return roomGetters; } // --- data access boilerplate @@ -265,9 +271,45 @@ public class Task extends AbstractModel { super(cursor); } + @Ignore + public Task(Parcel parcel) { + calendarUri = parcel.readString(); + completed = parcel.readLong(); + created = parcel.readLong(); + deleted = parcel.readLong(); + dueDate = parcel.readLong(); + elapsedSeconds = parcel.readInt(); + estimatedSeconds = parcel.readInt(); + hideUntil = parcel.readLong(); + id = parcel.readLong(); + importance = parcel.readInt(); + modified = parcel.readLong(); + notes = parcel.readString(); + recurrence = parcel.readString(); + notificationFlags = parcel.readInt(); + lastNotified = parcel.readLong(); + notifications = parcel.readLong(); + snoozeTime = parcel.readLong(); + repeatUntil = parcel.readLong(); + timerStart = parcel.readLong(); + title = parcel.readString(); + remoteId = parcel.readString(); + setValues = parcel.readParcelable(ContentValues.class.getClassLoader()); + values = parcel.readParcelable(ContentValues.class.getClassLoader()); + transitoryData = parcel.readHashMap(ContentValues.class.getClassLoader()); + } + @Override public long getId() { - return getIdHelper(ID); + if(setValues != null && setValues.containsKey(ID.name)) { + return setValues.getAsLong(ID.name); + } else if(values != null && values.containsKey(ID.name)) { + return values.getAsLong(ID.name); + } else if (id != null) { + return id; + } else { + return NO_ID; + } } public String getUuid() { @@ -275,6 +317,8 @@ public class Task extends AbstractModel { return setValues.getAsString(UUID.name); } else if(values != null && values.containsKey(UUID.name)) { return values.getAsString(UUID.name); + } else if (!Strings.isNullOrEmpty(remoteId)) { + return remoteId; } else { return NO_UUID; } @@ -282,7 +326,17 @@ public class Task extends AbstractModel { // --- parcelable helpers - public static final Creator CREATOR = new ModelCreator<>(Task.class); + public static final Creator CREATOR = new Creator() { + @Override + public Task createFromParcel(Parcel source) { + return new Task(source); + } + + @Override + public Task[] newArray(int size) { + return new Task[size]; + } + }; // --- data access methods @@ -515,10 +569,6 @@ public class Task extends AbstractModel { setValue(CREATION_DATE, creationDate); } - public String getUUID() { - return getValue(UUID); - } - public String getTitle() { return getValue(TITLE); } @@ -696,4 +746,43 @@ public class Task extends AbstractModel { public static boolean isUuidEmpty(String uuid) { return NO_UUID.equals(uuid) || TextUtils.isEmpty(uuid); } + + /** + * {@inheritDoc} + */ + @Override + public int describeContents() { + return 0; + } + + /** + * {@inheritDoc} + */ + @Override + public void writeToParcel(Parcel dest, int flags) { + dest.writeString(calendarUri); + dest.writeLong(completed); + dest.writeLong(created); + dest.writeLong(deleted); + dest.writeLong(dueDate); + dest.writeInt(elapsedSeconds); + dest.writeInt(estimatedSeconds); + dest.writeLong(hideUntil); + dest.writeLong(id); + dest.writeInt(importance); + dest.writeLong(modified); + dest.writeString(notes); + dest.writeString(recurrence); + dest.writeInt(notificationFlags); + dest.writeLong(lastNotified); + dest.writeLong(notifications); + dest.writeLong(snoozeTime); + dest.writeLong(repeatUntil); + dest.writeLong(timerStart); + dest.writeString(title); + dest.writeString(remoteId); + dest.writeParcelable(setValues, 0); + dest.writeParcelable(values, 0); + dest.writeMap(transitoryData); + } }