From 349e50c6c49fd0d60872d4bfbd3eafb870585ce8 Mon Sep 17 00:00:00 2001 From: Tim Su Date: Thu, 4 Jun 2009 07:40:39 +0000 Subject: [PATCH] Updates to the locale receiver so that notification only occurs for tags with visible and active tasks, and the last notification time is recorded. Adding "synchronized" to the onOpen calls for all the controllers to avoid a race condition where the database would get opened from two different threads, get created in one, and while in the process of creation, get queried from the other thread. --- .../astrid/data/sync/SyncDataController.java | 2 +- .../timsu/astrid/data/tag/TagController.java | 2 +- .../astrid/data/task/TaskController.java | 43 ++++++++++--------- .../astrid/utilities/LocaleReceiver.java | 18 +++----- .../timsu/astrid/utilities/Preferences.java | 13 ++++++ 5 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/com/timsu/astrid/data/sync/SyncDataController.java b/src/com/timsu/astrid/data/sync/SyncDataController.java index 424de3e09..7e7198c34 100644 --- a/src/com/timsu/astrid/data/sync/SyncDataController.java +++ b/src/com/timsu/astrid/data/sync/SyncDataController.java @@ -140,7 +140,7 @@ public class SyncDataController extends AbstractController { * @throws SQLException if the database could be neither opened or created */ @Override - public void open() throws SQLException { + public synchronized void open() throws SQLException { SQLiteOpenHelper helper = new SyncMappingDatabaseHelper(context, SYNC_TABLE_NAME, SYNC_TABLE_NAME); syncDatabase = helper.getWritableDatabase(); diff --git a/src/com/timsu/astrid/data/tag/TagController.java b/src/com/timsu/astrid/data/tag/TagController.java index c395cc03d..59ecd2c2b 100644 --- a/src/com/timsu/astrid/data/tag/TagController.java +++ b/src/com/timsu/astrid/data/tag/TagController.java @@ -292,7 +292,7 @@ public class TagController extends AbstractController { * @throws SQLException if the database could be neither opened or created */ @Override - public void open() throws SQLException { + public synchronized void open() throws SQLException { tagToTaskMapDatabase = new TagToTaskMappingDatabaseHelper(context, TAG_TASK_MAP_NAME, TAG_TASK_MAP_NAME).getWritableDatabase(); tagDatabase = new TagModelDatabaseHelper(context, diff --git a/src/com/timsu/astrid/data/task/TaskController.java b/src/com/timsu/astrid/data/task/TaskController.java index 1f4b2fb45..742978e6a 100644 --- a/src/com/timsu/astrid/data/task/TaskController.java +++ b/src/com/timsu/astrid/data/task/TaskController.java @@ -143,12 +143,10 @@ public class TaskController extends AbstractController { return list; } - /** Get identifiers for all tasks */ - public HashSet getAllTaskIdentifiers() { + /** Helper method to take a cursor pointing to a list of id's and generate + * a hashset */ + private HashSet createTaskIdentifierSet(Cursor cursor) { HashSet list = new HashSet(); - Cursor cursor = database.query(TASK_TABLE_NAME, new String[] { KEY_ROWID }, - null, null, null, null, null, null); - try { if(cursor.getCount() == 0) return list; @@ -165,29 +163,32 @@ public class TaskController extends AbstractController { } } + /** Get identifiers for all tasks */ + public HashSet getAllTaskIdentifiers() { + Cursor cursor = database.query(TASK_TABLE_NAME, new String[] { KEY_ROWID }, + null, null, null, null, null, null); + return createTaskIdentifierSet(cursor); + } + /** Get identifiers for all non-completed tasks */ public HashSet getActiveTaskIdentifiers() { - HashSet list = new HashSet(); Cursor cursor = database.query(TASK_TABLE_NAME, new String[] { KEY_ROWID }, AbstractTaskModel.PROGRESS_PERCENTAGE + " < " + AbstractTaskModel.COMPLETE_PERCENTAGE, null, null, null, null, null); + return createTaskIdentifierSet(cursor); + } - try { - if(cursor.getCount() == 0) - return list; - - do { - cursor.moveToNext(); - list.add(new TaskIdentifier(cursor.getInt( - cursor.getColumnIndexOrThrow(KEY_ROWID)))); - } while(!cursor.isLast()); - - return list; - } finally { - cursor.close(); - } + /** Get identifiers for all non-completed, non-hidden tasks */ + public HashSet getActiveVisibleTaskIdentifiers() { + Cursor cursor = database.query(TASK_TABLE_NAME, new String[] { KEY_ROWID }, + AbstractTaskModel.PROGRESS_PERCENTAGE + " < " + + AbstractTaskModel.COMPLETE_PERCENTAGE + " AND " + + AbstractTaskModel.HIDDEN_UNTIL + " < " + + System.currentTimeMillis(), null, null, null, null, null); + return createTaskIdentifierSet(cursor); } + /** Create a weighted list of tasks from the db cursor given */ public Cursor getTaskListCursorById(List idList) { @@ -521,7 +522,7 @@ public class TaskController extends AbstractController { * @throws SQLException if the database could be neither opened or created */ @Override - public void open() throws SQLException { + public synchronized void open() throws SQLException { SQLiteOpenHelper databaseHelper = new TaskModelDatabaseHelper( context, TASK_TABLE_NAME, TASK_TABLE_NAME); database = databaseHelper.getWritableDatabase(); diff --git a/src/com/timsu/astrid/utilities/LocaleReceiver.java b/src/com/timsu/astrid/utilities/LocaleReceiver.java index d7ceecf84..674956ba6 100644 --- a/src/com/timsu/astrid/utilities/LocaleReceiver.java +++ b/src/com/timsu/astrid/utilities/LocaleReceiver.java @@ -1,6 +1,5 @@ package com.timsu.astrid.utilities; -import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; @@ -29,10 +28,6 @@ public class LocaleReceiver extends BroadcastReceiver { /** minimum amount of time between two notifications */ private static final long MIN_NOTIFY_INTERVAL = 8*3600*1000L; - /** hash map of times we've notified */ - private static final HashMap notifyTimes = - new HashMap(); - @Override /** Called when the system is started up */ public void onReceive(Context context, Intent intent) { @@ -46,10 +41,9 @@ public class LocaleReceiver extends BroadcastReceiver { } // check if we've already made a notification recently - if(notifyTimes.containsKey(tagId)) { - if(System.currentTimeMillis() - notifyTimes.get(tagId) < - MIN_NOTIFY_INTERVAL) - return; + if(System.currentTimeMillis() - Preferences. + getLocaleLastAlertTime(context, tagId) < MIN_NOTIFY_INTERVAL) { + return; } // find out if we have active tasks with this tag @@ -58,7 +52,7 @@ public class LocaleReceiver extends BroadcastReceiver { TagController tagController = new TagController(context); tagController.open(); try { - HashSet activeTasks = taskController.getActiveTaskIdentifiers(); + HashSet activeTasks = taskController.getActiveVisibleTaskIdentifiers(); LinkedList tasks = tagController.getTaggedTasks( new TagIdentifier(tagId)); int count = TagListSubActivity.countActiveTasks(activeTasks, tasks); @@ -67,7 +61,9 @@ public class LocaleReceiver extends BroadcastReceiver { String reminder = r.getString(R.string.notif_tagNotification, r.getQuantityString(R.plurals.Ntasks, count, count), tagName); Notifications.showTagNotification(context, tagId, reminder); - notifyTimes.put(tagId, System.currentTimeMillis()); + + Preferences.setLocaleLastAlertTime(context, tagId, + System.currentTimeMillis()); } } finally { taskController.close(); diff --git a/src/com/timsu/astrid/utilities/Preferences.java b/src/com/timsu/astrid/utilities/Preferences.java index d9e9e1ee6..8baaf1322 100644 --- a/src/com/timsu/astrid/utilities/Preferences.java +++ b/src/com/timsu/astrid/utilities/Preferences.java @@ -21,6 +21,7 @@ public class Preferences { private static final String P_SYNC_RTM_LAST_SYNC = "rtmlastsync"; private static final String P_SYNC_LAST_SYNC = "lastsync"; private static final String P_SYNC_LAST_SYNC_ATTEMPT = "lastsyncattempt"; + private static final String P_LOCALE_LAST_NOTIFY = "locnot"; // pref values public static final int ICON_SET_PINK = 0; @@ -315,6 +316,18 @@ public class Preferences { editor.commit(); } + // --- locale + + public static void setLocaleLastAlertTime(Context context, long tag, long time) { + Editor editor = getPrefs(context).edit(); + editor.putLong(P_LOCALE_LAST_NOTIFY + tag, time); + editor.commit(); + } + + public static long getLocaleLastAlertTime(Context context, long tag) { + return getPrefs(context).getLong(P_LOCALE_LAST_NOTIFY + tag, 0); + } + // --- helper methods /** Clear the given preference */