diff --git a/clients/android/NewsBlur/src/com/newsblur/activity/ItemsList.java b/clients/android/NewsBlur/src/com/newsblur/activity/ItemsList.java index 0153839c6..8b1d365c3 100644 --- a/clients/android/NewsBlur/src/com/newsblur/activity/ItemsList.java +++ b/clients/android/NewsBlur/src/com/newsblur/activity/ItemsList.java @@ -146,9 +146,9 @@ public abstract class ItemsList extends NbActivity implements StateChangedListen protected abstract DefaultFeedView getDefaultFeedView(); @Override - public void handleUpdate() { + public void handleUpdate(boolean freshData) { updateStatusIndicators(); - if (itemListFragment != null) { + if (freshData && (itemListFragment != null)) { itemListFragment.hasUpdated(); } } diff --git a/clients/android/NewsBlur/src/com/newsblur/activity/Main.java b/clients/android/NewsBlur/src/com/newsblur/activity/Main.java index 94fce141c..a6834c1cf 100644 --- a/clients/android/NewsBlur/src/com/newsblur/activity/Main.java +++ b/clients/android/NewsBlur/src/com/newsblur/activity/Main.java @@ -163,9 +163,9 @@ public class Main extends NbActivity implements StateChangedListener, SwipeRefre } @Override - public void handleUpdate() { - folderFeedList.hasUpdated(); + public void handleUpdate(boolean freshData) { updateStatusIndicators(); + if (freshData) folderFeedList.hasUpdated(); } private void updateStatusIndicators() { diff --git a/clients/android/NewsBlur/src/com/newsblur/activity/NbActivity.java b/clients/android/NewsBlur/src/com/newsblur/activity/NbActivity.java index 89b5baa7f..9891775ea 100644 --- a/clients/android/NewsBlur/src/com/newsblur/activity/NbActivity.java +++ b/clients/android/NewsBlur/src/com/newsblur/activity/NbActivity.java @@ -108,14 +108,14 @@ public class NbActivity extends Activity { * Called on each NB activity after the DB has been updated by the sync service. This method * should return as quickly as possible. */ - protected void handleUpdate() { + protected void handleUpdate(boolean freshData) { Log.w(this.getClass().getName(), "activity doesn't implement handleUpdate"); } - private void _handleUpdate() { + private void _handleUpdate(final boolean freshData) { runOnUiThread(new Runnable() { public void run() { - handleUpdate(); + handleUpdate(freshData); } }); } @@ -124,14 +124,19 @@ public class NbActivity extends Activity { * Notify all activities in the app that the DB has been updated. Should only be called * by the sync service, which owns updating the DB. */ - public static void updateAllActivities() { + public static void updateAllActivities(boolean freshData) { synchronized (AllActivities) { for (NbActivity activity : AllActivities) { - activity._handleUpdate(); + activity._handleUpdate(freshData); } } } + public static void updateAllActivities() { + Log.w(NbActivity.class.getName(), "legacy handleUpdate used"); + NbActivity.updateAllActivities(true); + } + /** * Gets the number of active/foreground NB activities. Used by the sync service to * determine if the app is active so we can honour user requests not to run in diff --git a/clients/android/NewsBlur/src/com/newsblur/activity/Reading.java b/clients/android/NewsBlur/src/com/newsblur/activity/Reading.java index 16e0fa3df..d362b410c 100644 --- a/clients/android/NewsBlur/src/com/newsblur/activity/Reading.java +++ b/clients/android/NewsBlur/src/com/newsblur/activity/Reading.java @@ -326,9 +326,9 @@ public abstract class Reading extends NbActivity implements OnPageChangeListener } @Override - protected void handleUpdate() { + protected void handleUpdate(boolean freshData) { enableMainProgress(NBSyncService.isFeedSetSyncing(this.fs)); - updateCursor(); + if (freshData) updateCursor(); } private void updateCursor() { diff --git a/clients/android/NewsBlur/src/com/newsblur/fragment/DeleteFeedFragment.java b/clients/android/NewsBlur/src/com/newsblur/fragment/DeleteFeedFragment.java index e582f9ce6..559811193 100644 --- a/clients/android/NewsBlur/src/com/newsblur/fragment/DeleteFeedFragment.java +++ b/clients/android/NewsBlur/src/com/newsblur/fragment/DeleteFeedFragment.java @@ -58,7 +58,7 @@ public class DeleteFeedFragment extends DialogFragment { // called from the feed view so finish Activity activity = DeleteFeedFragment.this.getActivity(); if (activity instanceof Main) { - ((Main)activity).handleUpdate(); + ((Main)activity).handleUpdate(true); } else { activity.finish(); } diff --git a/clients/android/NewsBlur/src/com/newsblur/service/NBSyncService.java b/clients/android/NewsBlur/src/com/newsblur/service/NBSyncService.java index e36c4ecef..220dc7b52 100644 --- a/clients/android/NewsBlur/src/com/newsblur/service/NBSyncService.java +++ b/clients/android/NewsBlur/src/com/newsblur/service/NBSyncService.java @@ -23,12 +23,10 @@ import com.newsblur.network.APIManager; import com.newsblur.network.domain.FeedFolderResponse; import com.newsblur.network.domain.NewsBlurResponse; import com.newsblur.network.domain.StoriesResponse; -import com.newsblur.network.domain.StoryTextResponse; import com.newsblur.network.domain.UnreadStoryHashesResponse; import com.newsblur.util.AppConstants; import com.newsblur.util.DefaultFeedView; import com.newsblur.util.FeedSet; -import com.newsblur.util.FeedUtils; import com.newsblur.util.ImageCache; import com.newsblur.util.NetworkUtils; import com.newsblur.util.PrefsUtils; @@ -70,18 +68,12 @@ public class NBSyncService extends Service { */ public enum ActivationMode { ALL, OLDER, NEWER }; - // this value is somewhat arbitrary. ideally we would wait the max network timeout, but - // the system like to force-kill terminating services that take too long, so it is often - // moot to tune. - private final static long SHUTDOWN_SLACK_SECONDS = 60L; - private volatile static boolean ActionsRunning = false; private volatile static boolean CleanupRunning = false; private volatile static boolean FFSyncRunning = false; private volatile static boolean UnreadSyncRunning = false; private volatile static boolean UnreadHashSyncRunning = false; private volatile static boolean StorySyncRunning = false; - private volatile static boolean OriginalTextSyncRunning = false; private volatile static boolean ImagePrefetchRunning = false; private volatile static boolean DoFeedsFolders = false; @@ -118,21 +110,18 @@ public class NBSyncService extends Service { private static Set ImageQueue; static { ImageQueue = new HashSet(); } - /** Stories for which we want to fetch original text data. */ - private static Set OriginalTextQueue; - static { OriginalTextQueue = new HashSet(); } - /** Actions that may need to be double-checked locally due to overlapping API calls. */ private static List FollowupActions; static { FollowupActions = new ArrayList(); } private Set orphanFeedIds; - private PowerManager.WakeLock wl = null; private ExecutorService primaryExecutor; - private ExecutorService secondaryExecutor; - private APIManager apiManager; - private BlurDatabaseHelper dbHelper; + private OriginalTextService originalTextService; + + PowerManager.WakeLock wl = null; + APIManager apiManager; + BlurDatabaseHelper dbHelper; private ImageCache imageCache; private int lastStartIdCompleted = -1; @@ -144,12 +133,14 @@ public class NBSyncService extends Service { PowerManager pm = (PowerManager) getApplicationContext().getSystemService(POWER_SERVICE); wl = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, this.getClass().getSimpleName()); wl.setReferenceCounted(true); - primaryExecutor = Executors.newFixedThreadPool(1); - secondaryExecutor = Executors.newFixedThreadPool(1); + apiManager = new APIManager(this); PrefsUtils.checkForUpgrade(this); dbHelper = new BlurDatabaseHelper(this); imageCache = new ImageCache(this); + + primaryExecutor = Executors.newFixedThreadPool(1); + originalTextService = new OriginalTextService(this); } /** @@ -185,7 +176,7 @@ public class NBSyncService extends Service { try { if (HaltNow) return; - wl.acquire(); + incrementRunningChild(); if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "starting primary sync"); // check to see if we are on an allowable network only after ensuring we have CPU @@ -211,17 +202,15 @@ public class NBSyncService extends Service { syncMetadata(); finishActions(); - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "finishing primary sync"); - Runnable r = new Runnable() { - public void run() { - doSyncSecondary(startId); - } - }; - secondaryExecutor.execute(r); + originalTextService.start(startId); + + if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "finishing primary sync"); } catch (Exception e) { Log.e(this.getClass().getName(), "Sync error.", e); + } finally { + decrementRunningChild(startId); } } @@ -231,25 +220,13 @@ public class NBSyncService extends Service { */ private synchronized void doSyncSecondary(int startId) { try { - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "starting secondary sync"); - // run one step below normal background priority - Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND + Process.THREAD_PRIORITY_LESS_FAVORABLE); - syncUnreads(); - syncOriginalTexts(); - prefetchImages(); - if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "finishing secondary sync"); } catch (Exception e) { Log.e(this.getClass().getName(), "Sync error.", e); } finally { - if (NbActivity.getActiveActivityCount() < 1) { - stopSelf(startId); - } - lastStartIdCompleted = startId; - if (wl != null) wl.release(); } } @@ -308,7 +285,7 @@ public class NBSyncService extends Service { if (FollowupActions.size() < 1) return; for (ReadingAction ra : FollowupActions) { - ra.doLocal(dbHelper); + //ra.doLocal(dbHelper); } FollowupActions.clear(); } @@ -511,7 +488,7 @@ public class NBSyncService extends Service { } DefaultFeedView mode = PrefsUtils.getDefaultFeedViewForFeed(this, story.feedId); if (mode == DefaultFeedView.TEXT) { - OriginalTextQueue.add(story.storyHash); + originalTextService.addHash(story.storyHash); } } } @@ -523,43 +500,6 @@ public class NBSyncService extends Service { } } - private void syncOriginalTexts() { - try { - while (OriginalTextQueue.size() > 0) { - OriginalTextSyncRunning = true; - NbActivity.updateAllActivities(); - - Set fetchedHashes = new HashSet(); - Set batch = new HashSet(AppConstants.IMAGE_PREFETCH_BATCH_SIZE); - batchloop: for (String hash : OriginalTextQueue) { - batch.add(hash); - if (batch.size() >= AppConstants.IMAGE_PREFETCH_BATCH_SIZE) break batchloop; - } - try { - fetchloop: for (String hash : batch) { - if (stopSync()) return; - - String result = ""; - StoryTextResponse response = apiManager.getStoryText(FeedUtils.inferFeedId(hash), hash); - if ((response != null) && (response.originalText != null)) { - result = response.originalText; - } - dbHelper.putStoryText(hash, result); - - fetchedHashes.add(hash); - } - } finally { - OriginalTextQueue.removeAll(fetchedHashes); - } - } - } finally { - if (OriginalTextSyncRunning) { - OriginalTextSyncRunning = false; - NbActivity.updateAllActivities(); - } - } - } - /** * Fetch stories needed because the user is actively viewing a feed or folder. */ @@ -665,7 +605,23 @@ public class NBSyncService extends Service { return true; } - private boolean stopSync() { + void incrementRunningChild() { + wl.acquire(); + } + + void decrementRunningChild(int startId) { + if (wl != null) wl.release(); + // our wakelock reference counts. only stop the service if it is in the background and if + // we are the last thread to release the lock. + if (!wl.isHeld()) { + if (NbActivity.getActiveActivityCount() < 1) { + stopSelf(startId); + } + lastStartIdCompleted = startId; + } + } + + boolean stopSync() { if (HaltNow) { if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "stopping sync, soft interrupt set."); return true; @@ -690,7 +646,7 @@ public class NBSyncService extends Service { * Is the main feed/folder list sync running? */ public static boolean isFeedFolderSyncRunning() { - return (ActionsRunning || FFSyncRunning || CleanupRunning || UnreadSyncRunning || StorySyncRunning || OriginalTextSyncRunning || ImagePrefetchRunning); + return (ActionsRunning || FFSyncRunning || CleanupRunning || UnreadSyncRunning || StorySyncRunning || OriginalTextService.running() || ImagePrefetchRunning); } /** @@ -708,7 +664,7 @@ public class NBSyncService extends Service { if (UnreadHashSyncRunning) return "Syncing unread status . . ."; if (UnreadSyncRunning) return "Syncing " + StoryHashQueue.size() + " stories . . ."; if (ImagePrefetchRunning) return "Caching " + ImageQueue.size() + " images . . ."; - if (OriginalTextSyncRunning) return "Syncing text for " + OriginalTextQueue.size() + " stories. . ."; + if (OriginalTextService.running()) return "Syncing text for " + OriginalTextService.getPendingCount() + " stories. . ."; return null; } @@ -778,7 +734,7 @@ public class NBSyncService extends Service { } public static void getOriginalText(String hash) { - OriginalTextQueue.add(hash); + OriginalTextService.addHash(hash); } public static void softInterrupt() { @@ -794,20 +750,14 @@ public class NBSyncService extends Service { public void onDestroy() { if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "onDestroy - stopping execution"); HaltNow = true; + originalTextService.shutdown(); primaryExecutor.shutdown(); - secondaryExecutor.shutdown(); try { - primaryExecutor.awaitTermination(SHUTDOWN_SLACK_SECONDS, TimeUnit.SECONDS); + primaryExecutor.awaitTermination(AppConstants.SHUTDOWN_SLACK_SECONDS, TimeUnit.SECONDS); } catch (InterruptedException e) { primaryExecutor.shutdownNow(); Thread.currentThread().interrupt(); } - try { - secondaryExecutor.awaitTermination(SHUTDOWN_SLACK_SECONDS, TimeUnit.SECONDS); - } catch (InterruptedException e) { - secondaryExecutor.shutdownNow(); - Thread.currentThread().interrupt(); - } if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "onDestroy - execution halted"); super.onDestroy(); diff --git a/clients/android/NewsBlur/src/com/newsblur/service/OriginalTextService.java b/clients/android/NewsBlur/src/com/newsblur/service/OriginalTextService.java new file mode 100644 index 000000000..97d4afe0f --- /dev/null +++ b/clients/android/NewsBlur/src/com/newsblur/service/OriginalTextService.java @@ -0,0 +1,71 @@ +package com.newsblur.service; + +import android.util.Log; + +import com.newsblur.network.domain.StoryTextResponse; +import com.newsblur.util.AppConstants; +import com.newsblur.util.FeedUtils; + +import java.util.HashSet; +import java.util.Set; + +public class OriginalTextService extends SubService { + + /** story hashes we need to fetch (from newly found stories) */ + private static Set Hashes; + static {Hashes = new HashSet();} + /** story hashes we should fetch ASAP (they are waiting on-screen) */ + private static Set PriorityHashes; + static {PriorityHashes = new HashSet();} + + public OriginalTextService(NBSyncService parent) { + super(parent); + } + + @Override + protected void exec() { + while ((Hashes.size() > 0) || (PriorityHashes.size() > 0)) { + gotWork(); + fetchBatch(PriorityHashes); + fetchBatch(Hashes); + } + } + + private void fetchBatch(Set hashes) { + Set fetchedHashes = new HashSet(); + Set batch = new HashSet(AppConstants.IMAGE_PREFETCH_BATCH_SIZE); + batchloop: for (String hash : hashes) { + batch.add(hash); + if (batch.size() >= AppConstants.IMAGE_PREFETCH_BATCH_SIZE) break batchloop; + } + try { + fetchloop: for (String hash : batch) { + if (parent.stopSync()) return; + String result = ""; + StoryTextResponse response = parent.apiManager.getStoryText(FeedUtils.inferFeedId(hash), hash); + if ((response != null) && (response.originalText != null)) { + result = response.originalText; + } + parent.dbHelper.putStoryText(hash, result); + fetchedHashes.add(hash); + } + } finally { + gotData(); + hashes.removeAll(fetchedHashes); + } + } + + public static void addHash(String hash) { + Hashes.add(hash); + } + + public static void addPriorityHash(String hash) { + PriorityHashes.add(hash); + } + + public static int getPendingCount() { + return (Hashes.size() + PriorityHashes.size()); + } + +} + diff --git a/clients/android/NewsBlur/src/com/newsblur/service/SubService.java b/clients/android/NewsBlur/src/com/newsblur/service/SubService.java new file mode 100644 index 000000000..ba0e227cd --- /dev/null +++ b/clients/android/NewsBlur/src/com/newsblur/service/SubService.java @@ -0,0 +1,98 @@ +package com.newsblur.service; + +import android.os.Process; +import android.util.Log; + +import com.newsblur.activity.NbActivity; +import com.newsblur.util.AppConstants; + +import java.util.concurrent.Executors; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; + +/** + * A utility construct to make NbSyncService a bit more modular by encapsulating sync tasks + * that can be run fully asynchronously from the main sync loop. Like all of the sync service, + * flags and data used by these modules need to be static so that parts of the app without a + * handle to the service object can access them. + */ +public abstract class SubService { + + private static volatile boolean Running = false; + + protected NBSyncService parent; + private ExecutorService executor; + private int startId; + + private SubService() { + ; // no default construction + } + + SubService(NBSyncService parent) { + this.parent = parent; + executor = Executors.newFixedThreadPool(1); + if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService created"); + } + + public void start(final int startId) { + this.startId = startId; + Runnable r = new Runnable() { + public void run() { + parent.incrementRunningChild(); + if (NbActivity.getActiveActivityCount() < 1) { + Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND + Process.THREAD_PRIORITY_LESS_FAVORABLE ); + } else { + Process.setThreadPriority(Process.THREAD_PRIORITY_DEFAULT + Process.THREAD_PRIORITY_LESS_FAVORABLE + Process.THREAD_PRIORITY_LESS_FAVORABLE ); + } + if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService started"); + exec_(); + if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService completed"); + parent.decrementRunningChild(startId); + } + }; + executor.execute(r); + } + + private synchronized void exec_() { + try { + exec(); + } catch (Exception e) { + Log.e(this.getClass().getName(), "Sync error.", e); + } finally { + if (Running) { + Running = false; + NbActivity.updateAllActivities(false); + } + } + } + + protected abstract void exec(); + + public void shutdown() { + if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService stopping"); + executor.shutdown(); + try { + executor.awaitTermination(AppConstants.SHUTDOWN_SLACK_SECONDS, TimeUnit.SECONDS); + } catch (InterruptedException e) { + executor.shutdownNow(); + Thread.currentThread().interrupt(); + } finally { + if (AppConstants.VERBOSE_LOG) Log.d(this.getClass().getName(), "SubService stopped"); + } + } + + protected void gotWork() { + Running = true; + NbActivity.updateAllActivities(false); + } + + protected void gotData() { + NbActivity.updateAllActivities(true); + } + + public static boolean running() { + return Running; + } + +} + diff --git a/clients/android/NewsBlur/src/com/newsblur/util/AppConstants.java b/clients/android/NewsBlur/src/com/newsblur/util/AppConstants.java index 6e4adf934..f13c5cdee 100644 --- a/clients/android/NewsBlur/src/com/newsblur/util/AppConstants.java +++ b/clients/android/NewsBlur/src/com/newsblur/util/AppConstants.java @@ -5,7 +5,7 @@ public class AppConstants { // Enables high-volume logging that may be useful for debugging. This should // never be enabled for releases, as it not only slows down the app considerably, // it will log sensitive info such as passwords! - public static final boolean VERBOSE_LOG = false; + public static final boolean VERBOSE_LOG = true; public static final boolean VERBOSE_LOG_DB = false; public static final boolean VERBOSE_LOG_NET = false; @@ -28,7 +28,7 @@ public class AppConstants { public static final String LAST_SYNC_TIME = "LAST_SYNC_TIME"; // how long to wait before auto-syncing the feed/folder list - public static final long AUTO_SYNC_TIME_MILLIS = 7L * 60L * 1000L; + public static final long AUTO_SYNC_TIME_MILLIS = 15L * 60L * 1000L; // how often to trigger the BG service. slightly longer than how often we will find new stories, // to account for the fact that it is approximate, and missing a cycle is bad. @@ -62,4 +62,9 @@ public class AppConstants { // link to app feedback page public static final String FEEDBACK_URL = "https://getsatisfaction.com/newsblur/topics/new?topic[style]=question&from=company&product=NewsBlur+Android+App&topic[additional_detail]="; + // how long to wait for sync threads to shutdown. ideally we would wait the max network timeout, + // but the system like to force-kill terminating services that take too long, so it is often + // moot to tune. + public final static long SHUTDOWN_SLACK_SECONDS = 60L; + }