Make filtered MemoryHabitLists update automatically

android-base/src/main/java/org/isoron/androidbase/storage/SQLiteRepository.java

@@ -41,7 +41,7 @@ public class SQLiteRepository<T>
     @NonNull
     private final SQLiteDatabase db;
 
-    public SQLiteRepository(@NonNull Class klass, @NonNull SQLiteDatabase db)
+    public SQLiteRepository(@NonNull Class<T> klass, @NonNull SQLiteDatabase db)
     {
         this.klass = klass;
         this.db = db;

uhabits-android/src/androidTest/java/org/isoron/uhabits/models/sqlite/SQLiteHabitListTest.java

@@ -37,6 +37,8 @@ import java.util.*;
 
 import static org.hamcrest.MatcherAssert.*;
 import static org.hamcrest.core.IsEqual.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 @SuppressWarnings("JavaDoc")
 @RunWith(AndroidJUnit4.class)
@@ -52,6 +54,8 @@ public class SQLiteHabitListTest extends BaseAndroidTest
 
     private SQLiteRepository<HabitRecord> repository;
 
+    private ModelObservable.Listener listener;
+
     @Override
     public void setUp()
     {
@@ -78,6 +82,16 @@ public class SQLiteHabitListTest extends BaseAndroidTest
         }
 
         habitList.reload();
+
+        listener = mock(ModelObservable.Listener.class);
+        habitList.getObservable().addListener(listener);
+    }
+
+    @Override
+    protected void tearDown() throws Exception
+    {
+        habitList.getObservable().removeListener(listener);
+        super.tearDown();
     }
 
     @Test
@@ -85,6 +99,8 @@ public class SQLiteHabitListTest extends BaseAndroidTest
     {
         Habit habit = modelFactory.buildHabit();
         habitList.add(habit);
+        verify(listener).onModelChange();
+
         exception.expect(IllegalArgumentException.class);
         habitList.add(habit);
     }

uhabits-android/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteHabitList.java

@@ -92,6 +92,8 @@ public class SQLiteHabitList extends HabitList
         record.copyFrom(habit);
         record.position = list.indexOf(habit);
         repository.save(record);
+
+        getObservable().notifyListeners();
     }
 
     @Override
@@ -177,20 +179,21 @@ public class SQLiteHabitList extends HabitList
             repository.remove(record);
         });
         rebuildOrder();
+        getObservable().notifyListeners();
     }
 
     @Override
     public synchronized void removeAll()
     {
-        loadRecords();
         list.removeAll();
         SQLiteDatabase db = DatabaseUtils.openDatabase();
         db.execSQL("delete from habits");
         db.execSQL("delete from repetitions");
+        getObservable().notifyListeners();
     }
 
     @Override
-    public synchronized void reorder(Habit from, Habit to)
+    public synchronized void reorder(@NonNull Habit from, @NonNull Habit to)
     {
         loadRecords();
         list.reorder(from, to);
@@ -222,6 +225,7 @@ public class SQLiteHabitList extends HabitList
         fromRecord.position = toPos;
         repository.save(fromRecord);
         update(from);
+
         getObservable().notifyListeners();
     }
 
@@ -251,6 +255,8 @@ public class SQLiteHabitList extends HabitList
             record.copyFrom(h);
             repository.save(record);
         }
+
+        getObservable().notifyListeners();
     }
 
     public void reload()

uhabits-core/src/main/java/org/isoron/uhabits/core/models/HabitList.java

@@ -155,7 +155,7 @@ public abstract class HabitList implements Iterable<Habit>
      * @param from the habit that should be moved
      * @param to   the habit that currently occupies the desired position
      */
-    public abstract void reorder(Habit from, Habit to);
+    public abstract void reorder(@NonNull Habit from, @NonNull Habit to);
 
     public void repair()
     {

uhabits-core/src/main/java/org/isoron/uhabits/core/models/memory/MemoryHabitList.java

@@ -36,31 +36,37 @@ import static org.isoron.uhabits.core.models.HabitList.Order.BY_SCORE;
 public class MemoryHabitList extends HabitList
 {
     @NonNull
-    private LinkedList<Habit> list;
+    private LinkedList<Habit> list = new LinkedList<>();
 
     private Comparator<Habit> comparator = null;
 
     @NonNull
-    private Order order;
+    private Order order = Order.BY_POSITION;
+
+    @Nullable
+    private MemoryHabitList parent = null;
 
     public MemoryHabitList()
     {
         super();
-        list = new LinkedList<>();
-        order = Order.BY_POSITION;
     }
 
-    protected MemoryHabitList(@NonNull HabitMatcher matcher)
+    protected MemoryHabitList(@NonNull HabitMatcher matcher,
+                              Comparator<Habit> comparator,
+                              @NonNull MemoryHabitList parent)
     {
         super(matcher);
-        list = new LinkedList<>();
-        order = Order.BY_POSITION;
+        this.parent = parent;
+        this.comparator = comparator;
+        parent.getObservable().addListener(this::loadFromParent);
+        loadFromParent();
     }
 
     @Override
     public synchronized void add(@NonNull Habit habit)
         throws IllegalArgumentException
     {
+        throwIfHasParent();
         if (list.contains(habit))
             throw new IllegalArgumentException("habit already added");
 
@@ -71,6 +77,8 @@ public class MemoryHabitList extends HabitList
         if (id == null) habit.setId((long) list.size());
         list.addLast(habit);
         resort();
+
+        getObservable().notifyListeners();
     }
 
     @Override
@@ -78,7 +86,7 @@ public class MemoryHabitList extends HabitList
     {
         for (Habit h : list)
         {
-            if (h.getId() == null) continue;
+            if (h.getId() == null) throw new IllegalStateException();
             if (h.getId() == id) return h;
         }
         return null;
@@ -95,10 +103,7 @@ public class MemoryHabitList extends HabitList
     @Override
     public synchronized HabitList getFiltered(HabitMatcher matcher)
     {
-        MemoryHabitList habits = new MemoryHabitList(matcher);
-        habits.comparator = comparator;
-        for (Habit h : this) if (matcher.matches(h)) habits.add(h);
-        return habits;
+        return new MemoryHabitList(matcher, comparator, this);
     }
 
     @Override
@@ -115,12 +120,40 @@ public class MemoryHabitList extends HabitList
         resort();
     }
 
+    private Comparator<Habit> getComparatorByOrder(Order order)
+    {
+        Comparator<Habit> nameComparator =
+            (h1, h2) -> h1.getName().compareTo(h2.getName());
+
+        Comparator<Habit> colorComparator = (h1, h2) ->
+        {
+            Integer c1 = h1.getColor();
+            Integer c2 = h2.getColor();
+            if (c1.equals(c2)) return nameComparator.compare(h1, h2);
+            else return c1.compareTo(c2);
+        };
+
+        Comparator<Habit> scoreComparator = (h1, h2) ->
+        {
+            double s1 = h1.getScores().getTodayValue();
+            double s2 = h2.getScores().getTodayValue();
+            return Double.compare(s2, s1);
+        };
+
+        if (order == BY_POSITION) return null;
+        if (order == BY_NAME) return nameComparator;
+        if (order == BY_COLOR) return colorComparator;
+        if (order == BY_SCORE) return scoreComparator;
+        throw new IllegalStateException();
+    }
+
     @Override
     public int indexOf(@NonNull Habit h)
     {
         return list.indexOf(h);
     }
 
+    @NonNull
     @Override
     public Iterator<Habit> iterator()
     {
@@ -130,13 +163,23 @@ public class MemoryHabitList extends HabitList
     @Override
     public synchronized void remove(@NonNull Habit habit)
     {
+        throwIfHasParent();
         list.remove(habit);
     }
 
     @Override
-    public synchronized void reorder(Habit from, Habit to)
+    public synchronized void reorder(@NonNull Habit from, @NonNull Habit to)
     {
+        throwIfHasParent();
+        if (indexOf(from) < 0)
+            throw new IllegalArgumentException(
+                "list does not contain (from) habit");
+
         int toPos = indexOf(to);
+        if (toPos < 0)
+            throw new IllegalArgumentException(
+                "list does not contain (to) habit");
+
         list.remove(from);
         list.add(toPos, from);
     }
@@ -153,31 +196,20 @@ public class MemoryHabitList extends HabitList
         // NOP
     }
 
-    private Comparator<Habit> getComparatorByOrder(Order order)
-    {
-        Comparator<Habit> nameComparator =
-            (h1, h2) -> h1.getName().compareTo(h2.getName());
-
-        Comparator<Habit> colorComparator = (h1, h2) ->
+    private void throwIfHasParent()
     {
-            Integer c1 = h1.getColor();
-            Integer c2 = h2.getColor();
-            if (c1.equals(c2)) return nameComparator.compare(h1, h2);
-            else return c1.compareTo(c2);
-        };
+        if (parent != null) throw new IllegalStateException(
+            "Filtered lists cannot be modified directly. " +
+            "You should modify the parent list instead.");
+    }
 
-        Comparator<Habit> scoreComparator = (h1, h2) ->
+    private synchronized void loadFromParent()
     {
-            double s1 = h1.getScores().getTodayValue();
-            double s2 = h2.getScores().getTodayValue();
-            return Double.compare(s2, s1);
-        };
+        if (parent == null) throw new IllegalStateException();
 
-        if (order == BY_POSITION) return null;
-        if (order == BY_NAME) return nameComparator;
-        if (order == BY_COLOR) return colorComparator;
-        if (order == BY_SCORE) return scoreComparator;
-        throw new IllegalStateException();
+        list.clear();
+        for (Habit h : parent) if (filter.matches(h)) list.add(h);
+        resort();
     }
 
     private synchronized void resort()

uhabits-core/src/test/java/org/isoron/uhabits/core/models/HabitListTest.java

@@ -19,20 +19,29 @@
 
 package org.isoron.uhabits.core.models;
 
-import org.hamcrest.*;
 import org.isoron.uhabits.*;
 import org.junit.*;
+import org.junit.rules.*;
 
 import java.io.*;
 import java.util.*;
 
+import static junit.framework.TestCase.assertFalse;
+import static junit.framework.TestCase.fail;
 import static org.hamcrest.CoreMatchers.*;
-import static org.isoron.uhabits.core.models.HabitList.Order.*;
-import static org.junit.Assert.*;
+import static org.hamcrest.MatcherAssert.*;
+import static org.isoron.uhabits.core.models.HabitList.Order.BY_COLOR;
+import static org.isoron.uhabits.core.models.HabitList.Order.BY_NAME;
+import static org.isoron.uhabits.core.models.HabitList.Order.BY_POSITION;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 
 @SuppressWarnings("JavaDoc")
 public class HabitListTest extends BaseUnitTest
 {
+    @Rule
+    public ExpectedException thrown = ExpectedException.none();
+
     private ArrayList<Habit> habitsArray;
 
     private HabitList activeHabits;
@@ -69,45 +78,41 @@ public class HabitListTest extends BaseUnitTest
     }
 
     @Test
-    public void test_countActive()
+    public void testSize()
     {
+        assertThat(habitList.size(), equalTo(10));
         assertThat(activeHabits.size(), equalTo(6));
+        assertThat(reminderHabits.size(), equalTo(4));
     }
 
     @Test
-    public void test_getByPosition()
+    public void testGetByPosition()
     {
         assertThat(habitList.getByPosition(0), equalTo(habitsArray.get(0)));
         assertThat(habitList.getByPosition(3), equalTo(habitsArray.get(3)));
         assertThat(habitList.getByPosition(9), equalTo(habitsArray.get(9)));
 
         assertThat(activeHabits.getByPosition(0), equalTo(habitsArray.get(2)));
-    }
-
-    @Test
-    public void test_getHabitsWithReminder()
-    {
-        assertThat(reminderHabits.size(), equalTo(4));
         assertThat(reminderHabits.getByPosition(1),
             equalTo(habitsArray.get(3)));
     }
 
     @Test
-    public void test_get_withInvalidId()
+    public void testGetById()
     {
-        assertThat(habitList.getById(100L), is(nullValue()));
+        Habit habit1 = habitsArray.get(0);
+        Habit habit2 = habitList.getById(habit1.getId());
+        assertThat(habit1, equalTo(habit2));
     }
 
     @Test
-    public void test_get_withValidId()
+    public void testGetById_withInvalidId()
     {
-        Habit habit1 = habitsArray.get(0);
-        Habit habit2 = habitList.getById(habit1.getId());
-        assertThat(habit1, equalTo(habit2));
+        assertNull(habitList.getById(100L));
     }
 
     @Test
-    public void test_ordering()
+    public void testOrdering()
     {
         HabitList list = modelFactory.buildHabitList();
         Habit h1 = fixtures.createEmptyHabit();
@@ -155,7 +160,7 @@ public class HabitListTest extends BaseUnitTest
     }
 
     @Test
-    public void test_reorder()
+    public void testReorder()
     {
         int operations[][] = {
             { 5, 2 }, { 3, 7 }, { 4, 4 }, { 3, 2 }
@@ -191,13 +196,16 @@ public class HabitListTest extends BaseUnitTest
     }
 
     @Test
-    public void test_size()
+    public void testReorder_withInvalidArguments() throws Exception
     {
-        assertThat(habitList.size(), equalTo(10));
+        Habit h1 = habitsArray.get(0);
+        Habit h2 = fixtures.createEmptyHabit();
+        thrown.expect(IllegalArgumentException.class);
+        habitList.reorder(h1, h2);
     }
 
     @Test
-    public void test_writeCSV() throws IOException
+    public void testWriteCSV() throws IOException
     {
         HabitList list = modelFactory.buildHabitList();
 
@@ -224,6 +232,43 @@ public class HabitListTest extends BaseUnitTest
         StringWriter writer = new StringWriter();
         list.writeCSV(writer);
 
-        MatcherAssert.assertThat(writer.toString(), equalTo(expectedCSV));
+        assertThat(writer.toString(), equalTo(expectedCSV));
+    }
+
+    @Test
+    public void testAdd() throws Exception
+    {
+        Habit h1 = fixtures.createEmptyHabit();
+        assertFalse(h1.isArchived());
+        assertNull(h1.getId());
+        assertThat(habitList.indexOf(h1), equalTo(-1));
+
+        habitList.add(h1);
+        assertNotNull(h1.getId());
+        assertThat(habitList.indexOf(h1), not(equalTo(-1)));
+        assertThat(activeHabits.indexOf(h1), not(equalTo(-1)));
+    }
+
+    @Test
+    public void testAdd_withFilteredList() throws Exception
+    {
+        thrown.expect(IllegalStateException.class);
+        activeHabits.add(fixtures.createEmptyHabit());
+    }
+
+    @Test
+    public void testRemove_onFilteredList() throws Exception
+    {
+        thrown.expect(IllegalStateException.class);
+        activeHabits.remove(fixtures.createEmptyHabit());
+    }
+
+    @Test
+    public void testReorder_onFilteredList() throws Exception
+    {
+        Habit h1 = fixtures.createEmptyHabit();
+        Habit h2 = fixtures.createEmptyHabit();
+        thrown.expect(IllegalStateException.class);
+        activeHabits.reorder(h1, h2);
     }
 }