diff --git a/android-base/src/main/java/org/isoron/androidbase/storage/SQLiteRepository.java b/android-base/src/main/java/org/isoron/androidbase/storage/SQLiteRepository.java index 3e665a582..4fd3f1906 100644 --- a/android-base/src/main/java/org/isoron/androidbase/storage/SQLiteRepository.java +++ b/android-base/src/main/java/org/isoron/androidbase/storage/SQLiteRepository.java @@ -41,7 +41,7 @@ public class SQLiteRepository @NonNull private final SQLiteDatabase db; - public SQLiteRepository(@NonNull Class klass, @NonNull SQLiteDatabase db) + public SQLiteRepository(@NonNull Class klass, @NonNull SQLiteDatabase db) { this.klass = klass; this.db = db; diff --git a/uhabits-android/src/androidTest/java/org/isoron/uhabits/models/sqlite/SQLiteHabitListTest.java b/uhabits-android/src/androidTest/java/org/isoron/uhabits/models/sqlite/SQLiteHabitListTest.java index 1ddcbf59f..beb9f3b2a 100644 --- a/uhabits-android/src/androidTest/java/org/isoron/uhabits/models/sqlite/SQLiteHabitListTest.java +++ b/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 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); } diff --git a/uhabits-android/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteHabitList.java b/uhabits-android/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteHabitList.java index 23183a585..3d0496936 100644 --- a/uhabits-android/src/main/java/org/isoron/uhabits/models/sqlite/SQLiteHabitList.java +++ b/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() diff --git a/uhabits-core/src/main/java/org/isoron/uhabits/core/models/HabitList.java b/uhabits-core/src/main/java/org/isoron/uhabits/core/models/HabitList.java index cf49b4ba0..34d5d1f6a 100644 --- a/uhabits-core/src/main/java/org/isoron/uhabits/core/models/HabitList.java +++ b/uhabits-core/src/main/java/org/isoron/uhabits/core/models/HabitList.java @@ -155,7 +155,7 @@ public abstract class HabitList implements Iterable * @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() { diff --git a/uhabits-core/src/main/java/org/isoron/uhabits/core/models/memory/MemoryHabitList.java b/uhabits-core/src/main/java/org/isoron/uhabits/core/models/memory/MemoryHabitList.java index fbc82117f..f4cbbbf20 100644 --- a/uhabits-core/src/main/java/org/isoron/uhabits/core/models/memory/MemoryHabitList.java +++ b/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 list; + private LinkedList list = new LinkedList<>(); private Comparator 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 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 getComparatorByOrder(Order order) + { + Comparator nameComparator = + (h1, h2) -> h1.getName().compareTo(h2.getName()); + + Comparator 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 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 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 getComparatorByOrder(Order order) + private void throwIfHasParent() { - Comparator nameComparator = - (h1, h2) -> h1.getName().compareTo(h2.getName()); - - Comparator 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); - }; + if (parent != null) throw new IllegalStateException( + "Filtered lists cannot be modified directly. " + + "You should modify the parent list instead."); + } - Comparator scoreComparator = (h1, h2) -> - { - double s1 = h1.getScores().getTodayValue(); - double s2 = h2.getScores().getTodayValue(); - return Double.compare(s2, s1); - }; + private synchronized void loadFromParent() + { + 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() diff --git a/uhabits-core/src/test/java/org/isoron/uhabits/core/models/HabitListTest.java b/uhabits-core/src/test/java/org/isoron/uhabits/core/models/HabitListTest.java index 06a560763..f5cb6218e 100644 --- a/uhabits-core/src/test/java/org/isoron/uhabits/core/models/HabitListTest.java +++ b/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 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); } } \ No newline at end of file