Fetch with Room

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<String, Object> defaults = new Task().getDefaultValues();
+        Map<String, AbstractModel.ValueReader<?>> defaults = new Task().getRoomGetters();
         assertTrue(defaults.containsKey(Task.TITLE.name));
         assertTrue(defaults.containsKey(Task.DUE_DATE.name));
         assertTrue(defaults.containsKey(Task.HIDE_UNTIL.name));

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

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 <tim@todoroo.com>
  *
  */
-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> {
+        TYPE getValue(Task instance);
+    }
+
     /** Get the default values for this object */
-    abstract public Map<String, Object> getDefaultValues();
+    abstract public Map<String, ValueReader<?>> 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<String, Object> transitoryData = null;
+    protected HashMap<String, Object> 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<String, Object> entry : getDefaultValues().entrySet()) {
-            AndroidUtilities.putInto(mergedValues, entry.getKey(), entry.getValue());
+        for (Map.Entry<String, ValueReader<?>> 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 <TYPE> boolean shouldSaveValue(
-            Property<TYPE> property, TYPE newValue) {
-
+    private synchronized <TYPE> boolean shouldSaveValue(Property<TYPE> 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<TYPE extends AbstractModel>
-            implements Parcelable.Creator<TYPE> {
-
-        private final Class<TYPE> cls;
-
-        public ModelCreator(Class<TYPE> 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);
-        }
-   }
-
 }

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

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}
      *

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

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<String, Object> defaultValues = new HashMap<>();
+    private static final Map<String, ValueReader<?>> 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<String, Object> getDefaultValues() {
-        return defaultValues;
+    public Map<String, ValueReader<?>> 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<Task> CREATOR = new ModelCreator<>(Task.class);
+    public static final Creator<Task> CREATOR = new Creator<Task>() {
+        @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);
+    }
 }