From c20d5c87297d8597d3060a87822e6b8cb93a6509 Mon Sep 17 00:00:00 2001 From: Alinson Xavier Date: Thu, 24 Nov 2016 20:06:28 -0500 Subject: [PATCH] Make model classes thread-safe --- app/build.gradle | 1 + .../org/isoron/uhabits/models/Checkmark.java | 3 + .../isoron/uhabits/models/CheckmarkList.java | 25 ++++++--- .../org/isoron/uhabits/models/Frequency.java | 3 + .../java/org/isoron/uhabits/models/Habit.java | 56 ++++++++++--------- .../org/isoron/uhabits/models/HabitList.java | 5 +- .../uhabits/models/ModelObservable.java | 9 ++- .../uhabits/tasks/AndroidTaskRunner.java | 2 +- 8 files changed, 65 insertions(+), 39 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 6bdb20d23..f15b72924 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -77,6 +77,7 @@ dependencies { compile 'com.opencsv:opencsv:3.7' compile 'org.apmem.tools:layouts:1.10@aar' compile 'org.jetbrains:annotations-java5:15.0' + compile 'com.google.code.findbugs:jsr305:2.0.1' provided 'javax.annotation:jsr250-api:1.0' diff --git a/app/src/main/java/org/isoron/uhabits/models/Checkmark.java b/app/src/main/java/org/isoron/uhabits/models/Checkmark.java index 3ae1c4b1e..617a09db4 100644 --- a/app/src/main/java/org/isoron/uhabits/models/Checkmark.java +++ b/app/src/main/java/org/isoron/uhabits/models/Checkmark.java @@ -21,6 +21,8 @@ package org.isoron.uhabits.models; import org.apache.commons.lang3.builder.*; +import javax.annotation.concurrent.*; + /** * A Checkmark represents the completion status of the habit for a given day. *

@@ -30,6 +32,7 @@ import org.apache.commons.lang3.builder.*; *

* Checkmarks are computed automatically from the list of repetitions. */ +@ThreadSafe public final class Checkmark { /** diff --git a/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java b/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java index 86488a8d1..2f97d7fd3 100644 --- a/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java +++ b/app/src/main/java/org/isoron/uhabits/models/CheckmarkList.java @@ -27,18 +27,22 @@ import java.io.*; import java.text.*; import java.util.*; +import javax.annotation.concurrent.*; + /** * The collection of {@link Checkmark}s belonging to a habit. */ +@ThreadSafe public abstract class CheckmarkList { - protected Habit habit; + protected final Habit habit; - public ModelObservable observable = new ModelObservable(); + public final ModelObservable observable; public CheckmarkList(Habit habit) { this.habit = habit; + this.observable = new ModelObservable(); } /** @@ -64,7 +68,7 @@ public abstract class CheckmarkList * @return values for the checkmarks in the interval */ @NonNull - public final int[] getAllValues() + public synchronized final int[] getAllValues() { Repetition oldestRep = habit.getRepetitions().getOldest(); if (oldestRep == null) return new int[0]; @@ -97,7 +101,7 @@ public abstract class CheckmarkList * @return checkmark for today */ @Nullable - public final Checkmark getToday() + public synchronized final Checkmark getToday() { computeAll(); return getNewestComputed(); @@ -108,7 +112,7 @@ public abstract class CheckmarkList * * @return value of today's checkmark */ - public final int getTodayValue() + public synchronized final int getTodayValue() { Checkmark today = getToday(); if (today != null) return today.getValue(); @@ -159,9 +163,14 @@ public abstract class CheckmarkList */ public final void writeCSV(Writer out) throws IOException { - computeAll(); + int values[]; + + synchronized (this) + { + computeAll(); + values = getAllValues(); + } - int values[] = getAllValues(); long timestamp = DateUtils.getStartOfToday(); SimpleDateFormat dateFormat = DateFormats.getCSVDateFormat(); @@ -271,7 +280,7 @@ public abstract class CheckmarkList * repetition of the habit until today. Days that already have a * corresponding checkmark are skipped. */ - protected final void computeAll() + private synchronized void computeAll() { Repetition oldest = habit.getRepetitions().getOldest(); if (oldest == null) return; diff --git a/app/src/main/java/org/isoron/uhabits/models/Frequency.java b/app/src/main/java/org/isoron/uhabits/models/Frequency.java index 5b893b5a1..b21349b18 100644 --- a/app/src/main/java/org/isoron/uhabits/models/Frequency.java +++ b/app/src/main/java/org/isoron/uhabits/models/Frequency.java @@ -21,9 +21,12 @@ package org.isoron.uhabits.models; import org.apache.commons.lang3.builder.*; +import javax.annotation.concurrent.*; + /** * Represents how often is the habit repeated. */ +@ThreadSafe public class Frequency { public static final Frequency DAILY = new Frequency(1, 1); diff --git a/app/src/main/java/org/isoron/uhabits/models/Habit.java b/app/src/main/java/org/isoron/uhabits/models/Habit.java index 5ba711407..ea0df6f7b 100644 --- a/app/src/main/java/org/isoron/uhabits/models/Habit.java +++ b/app/src/main/java/org/isoron/uhabits/models/Habit.java @@ -26,11 +26,13 @@ import org.apache.commons.lang3.builder.*; import java.util.*; +import javax.annotation.concurrent.*; import javax.inject.*; /** * The thing that the user wants to track. */ +@ThreadSafe public class Habit { public static final String HABIT_URI_FORMAT = @@ -54,22 +56,23 @@ public class Habit @NonNull private boolean archived; - @NonNull - private StreakList streaks; + @Nullable + private Reminder reminder; @NonNull - private ScoreList scores; + private final StreakList streaks; @NonNull - private RepetitionList repetitions; + private final ScoreList scores; @NonNull - private CheckmarkList checkmarks; + private final RepetitionList repetitions; - @Nullable - private Reminder reminder; + @NonNull + private final CheckmarkList checkmarks; - private ModelObservable observable = new ModelObservable(); + @NonNull + private final ModelObservable observable; /** * Constructs a habit with default attributes. @@ -88,12 +91,13 @@ public class Habit streaks = factory.buildStreakList(this); scores = factory.buildScoreList(this); repetitions = factory.buildRepetitionList(this); + observable = new ModelObservable(); } /** * Clears the reminder for a habit. */ - public void clearReminder() + public synchronized void clearReminder() { reminder = null; observable.notifyListeners(); @@ -104,7 +108,7 @@ public class Habit * * @param model the model whose attributes should be copied from */ - public void copyFrom(@NonNull Habit model) + public synchronized void copyFrom(@NonNull Habit model) { this.name = model.getName(); this.description = model.getDescription(); @@ -119,7 +123,7 @@ public class Habit * List of checkmarks belonging to this habit. */ @NonNull - public CheckmarkList getCheckmarks() + public synchronized CheckmarkList getCheckmarks() { return checkmarks; } @@ -133,56 +137,56 @@ public class Habit * habit.color). */ @NonNull - public Integer getColor() + public synchronized Integer getColor() { return color; } - public void setColor(@NonNull Integer color) + public synchronized void setColor(@NonNull Integer color) { this.color = color; } @NonNull - public String getDescription() + public synchronized String getDescription() { return description; } - public void setDescription(@NonNull String description) + public synchronized void setDescription(@NonNull String description) { this.description = description; } @NonNull - public Frequency getFrequency() + public synchronized Frequency getFrequency() { return frequency; } - public void setFrequency(@NonNull Frequency frequency) + public synchronized void setFrequency(@NonNull Frequency frequency) { this.frequency = frequency; } @Nullable - public Long getId() + public synchronized Long getId() { return id; } - public void setId(@Nullable Long id) + public synchronized void setId(@Nullable Long id) { this.id = id; } @NonNull - public String getName() + public synchronized String getName() { return name; } - public void setName(@NonNull String name) + public synchronized void setName(@NonNull String name) { this.name = name; } @@ -203,13 +207,13 @@ public class Habit * @throws IllegalStateException if habit has no reminder */ @NonNull - public Reminder getReminder() + public synchronized Reminder getReminder() { if (reminder == null) throw new IllegalStateException(); return reminder; } - public void setReminder(@Nullable Reminder reminder) + public synchronized void setReminder(@Nullable Reminder reminder) { this.reminder = reminder; } @@ -248,17 +252,17 @@ public class Habit * * @return true if habit has reminder, false otherwise */ - public boolean hasReminder() + public synchronized boolean hasReminder() { return reminder != null; } - public boolean isArchived() + public synchronized boolean isArchived() { return archived; } - public void setArchived(boolean archived) + public synchronized void setArchived(boolean archived) { this.archived = archived; } diff --git a/app/src/main/java/org/isoron/uhabits/models/HabitList.java b/app/src/main/java/org/isoron/uhabits/models/HabitList.java index 444a5824c..2892a2c30 100644 --- a/app/src/main/java/org/isoron/uhabits/models/HabitList.java +++ b/app/src/main/java/org/isoron/uhabits/models/HabitList.java @@ -28,12 +28,15 @@ import org.isoron.uhabits.utils.*; import java.io.*; import java.util.*; +import javax.annotation.concurrent.*; + /** * An ordered collection of {@link Habit}s. */ +@ThreadSafe public abstract class HabitList implements Iterable { - private ModelObservable observable; + private final ModelObservable observable; @NonNull protected final HabitMatcher filter; diff --git a/app/src/main/java/org/isoron/uhabits/models/ModelObservable.java b/app/src/main/java/org/isoron/uhabits/models/ModelObservable.java index a762b5f8a..6ab80813b 100644 --- a/app/src/main/java/org/isoron/uhabits/models/ModelObservable.java +++ b/app/src/main/java/org/isoron/uhabits/models/ModelObservable.java @@ -21,10 +21,13 @@ package org.isoron.uhabits.models; import java.util.*; +import javax.annotation.concurrent.*; + /** * A ModelObservable allows objects to subscribe themselves to it and receive * notifications whenever the model is changed. */ +@ThreadSafe public class ModelObservable { private List listeners; @@ -43,7 +46,7 @@ public class ModelObservable * * @param l the listener to be added. */ - public void addListener(Listener l) + public synchronized void addListener(Listener l) { listeners.add(l); } @@ -53,7 +56,7 @@ public class ModelObservable *

* Only models should call this method. */ - public void notifyListeners() + public synchronized void notifyListeners() { for (Listener l : listeners) l.onModelChange(); } @@ -66,7 +69,7 @@ public class ModelObservable * * @param l the listener to be removed */ - public void removeListener(Listener l) + public synchronized void removeListener(Listener l) { listeners.remove(l); } diff --git a/app/src/main/java/org/isoron/uhabits/tasks/AndroidTaskRunner.java b/app/src/main/java/org/isoron/uhabits/tasks/AndroidTaskRunner.java index e1626fe34..fac5849f4 100644 --- a/app/src/main/java/org/isoron/uhabits/tasks/AndroidTaskRunner.java +++ b/app/src/main/java/org/isoron/uhabits/tasks/AndroidTaskRunner.java @@ -60,7 +60,7 @@ public class AndroidTaskRunner implements TaskRunner public void execute(Task task) { task.onAttached(this); - new CustomAsyncTask(task).execute(); + new CustomAsyncTask(task).executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); } @Override