[MediaWiki-commits] [Gerrit] apps...wikipedia[master]: Stop thrashing on page save failure and save on subsequent runs
jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/349124 )
Change subject: Stop thrashing on page save failure and save on subsequent runs
..
Stop thrashing on page save failure and save on subsequent runs
Stops relying on a ContentObserver to trigger the saved page sync service.
This isn't viable because of the way the ContentObserver works and the way
the DAO works.
Instead, we'll manually kick off saved page syncing after a reading list
sync event (such as happens when the app is launched or after a page is
added to a reading list).
No more thrashing!
Also added logging for successful page saves for dev convenience.
Bug: T162894
Change-Id: I1d0bfd2e2071942f3c7ad977ae271cf78db82235
---
M app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
M app/src/main/java/org/wikipedia/readinglist/sync/ReadingListSynchronizer.java
M app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java
3 files changed, 11 insertions(+), 6 deletions(-)
Approvals:
Niedzielski: Looks good to me, approved
jenkins-bot: Verified
diff --git
a/app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
b/app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
index 8ace611..56d6c4a 100644
---
a/app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
+++
b/app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
@@ -1,16 +1,13 @@
package org.wikipedia.readinglist.page;
import android.content.Context;
-import android.content.Intent;
import android.database.ContentObserver;
import android.net.Uri;
import android.os.Handler;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
-import org.wikipedia.WikipediaApp;
import org.wikipedia.database.contract.ReadingListPageContract;
-import org.wikipedia.savedpages.SavedPageSyncService;
import java.util.ArrayList;
import java.util.List;
@@ -42,7 +39,6 @@
if (uri.equals(ReadingListPageContract.Disk.URI)) {
notifyListeners();
}
-WikipediaApp.getInstance().startService(new
Intent(WikipediaApp.getInstance(), SavedPageSyncService.class));
}
public void register(@NonNull Context context) {
diff --git
a/app/src/main/java/org/wikipedia/readinglist/sync/ReadingListSynchronizer.java
b/app/src/main/java/org/wikipedia/readinglist/sync/ReadingListSynchronizer.java
index 2866816..cfd5ea4 100644
---
a/app/src/main/java/org/wikipedia/readinglist/sync/ReadingListSynchronizer.java
+++
b/app/src/main/java/org/wikipedia/readinglist/sync/ReadingListSynchronizer.java
@@ -1,5 +1,6 @@
package org.wikipedia.readinglist.sync;
+import android.content.Intent;
import android.os.Handler;
import android.os.Looper;
import android.support.annotation.NonNull;
@@ -16,6 +17,7 @@
import org.wikipedia.readinglist.ReadingListData;
import org.wikipedia.readinglist.page.ReadingListPage;
import org.wikipedia.readinglist.page.database.ReadingListDaoProxy;
+import org.wikipedia.savedpages.SavedPageSyncService;
import org.wikipedia.settings.Prefs;
import org.wikipedia.useroption.UserOption;
import org.wikipedia.useroption.dataclient.UserInfo;
@@ -52,12 +54,14 @@
}
public void sync() {
+// TODO: remove when ready for beta/production
if (!ReleaseUtil.isPreBetaRelease()) {
-// TODO: remove when ready for beta/production
+syncSavedPages();
return;
}
if (!User.isLoggedIn()) {
L.d("Not logged in, so skipping sync of reading lists.");
+syncSavedPages();
return;
}
CallbackTask.execute(new CallbackTask.Task() {
@@ -95,6 +99,7 @@
L.d("Local and remote reading lists are in sync.");
}
}
+syncSavedPages();
} catch (IOException e) {
e.printStackTrace();
@@ -242,4 +247,8 @@
List lists =
ReadingListData.instance().queryMruLists(null);
return new RemoteReadingLists(Prefs.getReadingListSyncRev(), lists);
}
+
+private void syncSavedPages() {
+WikipediaApp.getInstance().startService(new
Intent(WikipediaApp.getInstance(), SavedPageSyncService.class));
+}
}
diff --git
a/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java
b/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java
index d37964f..9f29075 100644
--- a/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java
+++ b/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java
@@ -142,7 +142,7 @@
L.e("Failed to save page " + title, e);
return false;
}
-
+L.i("Saved page " + title);
return true;
}
--
To view, visit
[MediaWiki-commits] [Gerrit] apps...wikipedia[master]: Stop thrashing on page save failure and save on subsequent runs
Mholloway has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/349124 )
Change subject: Stop thrashing on page save failure and save on subsequent runs
..
Stop thrashing on page save failure and save on subsequent runs
This patch does the following:
1. Kicks off the service only for changes to ReadingListPageDisk table.
Previously, it was also being triggered for changes to the Reading-
ListPageHttp and ReadingListPage tables as well, or thrice per
substantive change. This is because the ContentObserver also notifies
on changes to those tables depsite being registered for /page/disk.
2. Removes the upsert() calls from startTransaction() and
failTransaction(), which stops each transaction in the service from
(twice) again retriggering the service.
3. Added (OR status == OUTDATED) to the SQL query for pending rows, so
that rows left in an OUTDATED state due to a failure, and no longer
upserted, will be picked up on subsequent runs and saved as originally
intended.
Also added logging for successful page saves and made minor cosmetic
changes.
Bug: T162894
Change-Id: I1d0bfd2e2071942f3c7ad977ae271cf78db82235
---
M app/src/main/java/org/wikipedia/database/async/AsyncDao.java
M app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
M
app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListPageDao.java
M app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java
4 files changed, 7 insertions(+), 7 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia
refs/changes/24/349124/1
diff --git a/app/src/main/java/org/wikipedia/database/async/AsyncDao.java
b/app/src/main/java/org/wikipedia/database/async/AsyncDao.java
index eb0e193..d8302b8 100644
--- a/app/src/main/java/org/wikipedia/database/async/AsyncDao.java
+++ b/app/src/main/java/org/wikipedia/database/async/AsyncDao.java
@@ -47,13 +47,11 @@
protected void startTransaction(@NonNull Row row) {
row.startTransaction();
-upsert(row);
}
protected synchronized void failTransaction(@NonNull Row row) {
if (completableTransaction(row)) {
row.failTransaction();
-upsert(row);
}
}
diff --git
a/app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
b/app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
index 8ace611..03cf711 100644
---
a/app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
+++
b/app/src/main/java/org/wikipedia/readinglist/page/ReadingListPageObserver.java
@@ -41,8 +41,8 @@
@Override public void onChange(boolean selfChange, Uri uri) {
if (uri.equals(ReadingListPageContract.Disk.URI)) {
notifyListeners();
+WikipediaApp.getInstance().startService(new
Intent(WikipediaApp.getInstance(), SavedPageSyncService.class));
}
-WikipediaApp.getInstance().startService(new
Intent(WikipediaApp.getInstance(), SavedPageSyncService.class));
}
public void register(@NonNull Context context) {
diff --git
a/app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListPageDao.java
b/app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListPageDao.java
index 54f2336..6e8623c 100644
---
a/app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListPageDao.java
+++
b/app/src/main/java/org/wikipedia/readinglist/page/database/ReadingListPageDao.java
@@ -183,10 +183,12 @@
.replaceAll(":keyCol",
ReadingListPageContract.Page.KEY.qualifiedName());
private static final String SELECT_ROWS_WITH_LIST_KEY = "',' ||
:listKeyCol || ',' like '%,' || ? || ',%'"
- .replaceAll(":listKeyCol",
ReadingListPageContract.Page.LIST_KEYS.qualifiedName());
+.replaceAll(":listKeyCol",
ReadingListPageContract.Page.LIST_KEYS.qualifiedName());
-private static String SELECT_ROWS_PENDING_DISK_TRANSACTION =
":transactionIdCol == :noTransactionId"
+private static String SELECT_ROWS_PENDING_DISK_TRANSACTION =
":transactionIdCol == :noTransactionId OR :diskStatusCol == :outdated"
.replaceAll(":transactionIdCol",
ReadingListPageContract.DiskWithPage.DISK_TRANSACTION_ID.qualifiedName())
-.replaceAll(":noTransactionId",
String.valueOf(AsyncConstant.NO_TRANSACTION_ID));
+.replaceAll(":noTransactionId",
String.valueOf(AsyncConstant.NO_TRANSACTION_ID))
+.replaceAll(":diskStatusCol",
ReadingListPageContract.DiskWithPage.DISK_STATUS.qualifiedName())
+.replaceAll(":outdated",
String.valueOf(DiskStatus.OUTDATED.code()));
}
}
diff --git
a/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java
b/app/src/main/java/org/wikipedia/savedpages/SavedPageSyncService.java
index d37964f..9f29075 100644
---
