jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/402104 )
Change subject: Move logic for creating Default list to database upgrade hook.
......................................................................
Move logic for creating Default list to database upgrade hook.
*** IMPORTANTZOMG: This changes the database contract without bumping the
database version. Any existing Dev and Alpha users will need to clear the
app data, otherwise they'll see unexpected behavior. ***
A couple of additional things going on here:
- Slightly updated the database upgrade logic such that the
onUpgradeSchema() method gets called for the initial version of a table,
as well as for subsequent versions.
- Removed the "synchronized" keyword from the methods in the DbHelper,
since they're not really necessary (the DbHelper doesn't actually have
any thread-unsafe state, and thread safety is guaranteed by the
SQLiteOpenHelper anyway), and it was causing problems when trying to add
rows to the db while inside the OnCreate() method.
Change-Id: Ia8985acc0ec498602bcb2c27d7cc14144dca05df
---
M app/src/main/java/org/wikipedia/database/DatabaseTable.java
M app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
M app/src/main/java/org/wikipedia/readinglist/ReadingListItemView.java
M app/src/main/java/org/wikipedia/readinglist/database/ReadingList.java
M app/src/main/java/org/wikipedia/readinglist/database/ReadingListDbHelper.java
M app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
6 files changed, 61 insertions(+), 64 deletions(-)
Approvals:
Sharvaniharan: Looks good to me, approved
jenkins-bot: Verified
diff --git a/app/src/main/java/org/wikipedia/database/DatabaseTable.java
b/app/src/main/java/org/wikipedia/database/DatabaseTable.java
index 6186ec4..c1c057a 100644
--- a/app/src/main/java/org/wikipedia/database/DatabaseTable.java
+++ b/app/src/main/java/org/wikipedia/database/DatabaseTable.java
@@ -85,6 +85,7 @@
public void upgradeSchema(@NonNull SQLiteDatabase db, int fromVersion, int
toVersion) {
if (fromVersion < getDBVersionIntroducedAt()) {
createTables(db);
+ onUpgradeSchema(db, fromVersion, getDBVersionIntroducedAt());
}
for (int ver = Math.max(getDBVersionIntroducedAt(), fromVersion) + 1;
ver <= toVersion; ++ver) {
@@ -101,7 +102,7 @@
db.execSQL(alterTableString);
}
- upgradeSchema(db, ver);
+ onUpgradeSchema(db, ver - 1, ver);
}
}
@@ -109,7 +110,7 @@
return baseContentURI;
}
- protected void upgradeSchema(@NonNull SQLiteDatabase db, int toVersion) {
+ protected void onUpgradeSchema(@NonNull SQLiteDatabase db, int
fromVersion, int toVersion) {
}
private void createTables(@NonNull SQLiteDatabase db) {
diff --git
a/app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
b/app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
index 81f7f43..6dce4bf 100644
--- a/app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
+++ b/app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
@@ -8,7 +8,6 @@
import android.support.design.widget.BottomSheetBehavior;
import android.support.v7.widget.LinearLayoutManager;
import android.support.v7.widget.RecyclerView;
-import android.text.TextUtils;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
@@ -218,7 +217,7 @@
message = getString(R.string.reading_list_already_exists);
} else {
message =
String.format(getString(R.string.reading_list_added_to_named),
- TextUtils.isEmpty(readingList.title()) ?
getString(R.string.default_reading_list_name) : readingList.title());
+ readingList.isDefault() ?
getString(R.string.default_reading_list_name) : readingList.title());
new
ReadingListsFunnel(title.getWikiSite()).logAddToList(readingList,
readingLists.size(), invokeSource);
ReadingListDbHelper.instance().addPageToList(readingList,
title);
@@ -244,7 +243,8 @@
if (numAdded == 0) {
message =
getString(R.string.reading_list_already_contains_selection);
} else {
- message =
String.format(getString(R.string.reading_list_added_articles_list_titled),
numAdded, TextUtils.isEmpty(readingList.title()) ?
getString(R.string.default_reading_list_name) : readingList.title());
+ message =
String.format(getString(R.string.reading_list_added_articles_list_titled),
numAdded,
+ readingList.isDefault() ?
getString(R.string.default_reading_list_name) : readingList.title());
new ReadingListsFunnel().logAddToList(readingList,
readingLists.size(), invokeSource);
}
showViewListSnackBar(readingList, message);
diff --git
a/app/src/main/java/org/wikipedia/readinglist/ReadingListItemView.java
b/app/src/main/java/org/wikipedia/readinglist/ReadingListItemView.java
index d3a9934..97d86bd 100644
--- a/app/src/main/java/org/wikipedia/readinglist/ReadingListItemView.java
+++ b/app/src/main/java/org/wikipedia/readinglist/ReadingListItemView.java
@@ -130,7 +130,7 @@
PopupMenu menu = new PopupMenu(getContext(), anchorView);
menu.getMenuInflater().inflate(R.menu.menu_reading_list_item,
menu.getMenu());
- if (TextUtils.isEmpty(readingList.title())) {
+ if (readingList.isDefault()) {
MenuItem renameItem =
menu.getMenu().findItem(R.id.menu_reading_list_rename);
renameItem.setVisible(false);
MenuItem deleteItem =
menu.getMenu().findItem(R.id.menu_reading_list_delete);
@@ -155,7 +155,7 @@
if (readingList == null) {
return;
}
- titleView.setText(TextUtils.isEmpty(readingList.title())
+ titleView.setText(readingList.isDefault()
? getString(R.string.default_reading_list_name)
: readingList.title());
if (TextUtils.isEmpty(readingList.description()) &&
showDescriptionEmptyHint) {
@@ -177,7 +177,7 @@
if (readingList == null) {
return;
}
-
defaultListEmptyView.setVisibility((TextUtils.isEmpty(readingList.title()) &&
readingList.pages().size() == 0) ? VISIBLE : GONE);
+ defaultListEmptyView.setVisibility((readingList.isDefault() &&
readingList.pages().size() == 0) ? VISIBLE : GONE);
imageContainer.setVisibility(defaultListEmptyView.getVisibility() ==
VISIBLE ? GONE : VISIBLE);
clearThumbnails();
List<String> thumbUrls = new ArrayList<>();
diff --git
a/app/src/main/java/org/wikipedia/readinglist/database/ReadingList.java
b/app/src/main/java/org/wikipedia/readinglist/database/ReadingList.java
index 343ec5d..b996c16 100644
--- a/app/src/main/java/org/wikipedia/readinglist/database/ReadingList.java
+++ b/app/src/main/java/org/wikipedia/readinglist/database/ReadingList.java
@@ -2,6 +2,7 @@
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
+import android.text.TextUtils;
import java.util.ArrayList;
import java.util.Collections;
@@ -47,6 +48,10 @@
return count;
}
+ public boolean isDefault() {
+ return TextUtils.isEmpty(title);
+ }
+
public long id() {
return id;
}
diff --git
a/app/src/main/java/org/wikipedia/readinglist/database/ReadingListDbHelper.java
b/app/src/main/java/org/wikipedia/readinglist/database/ReadingListDbHelper.java
index aa0767d..3b90199 100644
---
a/app/src/main/java/org/wikipedia/readinglist/database/ReadingListDbHelper.java
+++
b/app/src/main/java/org/wikipedia/readinglist/database/ReadingListDbHelper.java
@@ -1,14 +1,11 @@
package org.wikipedia.readinglist.database;
import android.content.ContentValues;
-import android.content.Context;
import android.database.Cursor;
import android.database.sqlite.SQLiteDatabase;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
-import android.text.TextUtils;
-import org.wikipedia.R;
import org.wikipedia.WikipediaApp;
import org.wikipedia.database.contract.ReadingListContract;
import org.wikipedia.database.contract.ReadingListPageContract;
@@ -22,7 +19,6 @@
public class ReadingListDbHelper {
private static ReadingListDbHelper INSTANCE;
- private Context context = WikipediaApp.getInstance();
public static ReadingListDbHelper instance() {
if (INSTANCE == null) {
@@ -31,7 +27,7 @@
return INSTANCE;
}
- public synchronized List<ReadingList> getAllLists() {
+ public List<ReadingList> getAllLists() {
List<ReadingList> lists = new ArrayList<>();
SQLiteDatabase db = getReadableDatabase();
try (Cursor cursor = db.query(ReadingListContract.TABLE, null, null,
null, null, null, null)) {
@@ -40,34 +36,13 @@
lists.add(list);
}
}
- oneTimeDefaultListSyncSetUp(lists, db);
-
for (ReadingList list : lists) {
populateListPages(db, list);
}
return lists;
}
- private void oneTimeDefaultListSyncSetUp(List<ReadingList> lists,
SQLiteDatabase db) {
- checkForDefaultListAndAddIfNeeded(lists, db);
- }
-
- private void checkForDefaultListAndAddIfNeeded(List<ReadingList> lists,
SQLiteDatabase db) {
- if (lists.size() == 0 || !listCollectionContainsDefault(lists)) {
- lists.add(createList("",
context.getString(R.string.default_reading_list_description)));
- }
- }
-
- private boolean listCollectionContainsDefault(List<ReadingList> lists) {
- for (ReadingList list : lists) {
- if (TextUtils.isEmpty(list.title())) {
- return true;
- }
- }
- return false;
- }
-
- public synchronized List<ReadingList> getAllListsWithoutContents() {
+ public List<ReadingList> getAllListsWithoutContents() {
List<ReadingList> lists = new ArrayList<>();
SQLiteDatabase db = getReadableDatabase();
try (Cursor cursor = db.query(ReadingListContract.TABLE, null, null,
null, null, null, null)) {
@@ -80,13 +55,13 @@
}
@NonNull
- public synchronized ReadingList createList(@NonNull String title,
@Nullable String description) {
+ public ReadingList createList(@NonNull String title, @Nullable String
description) {
SQLiteDatabase db = getWritableDatabase();
return createList(db, title, description);
}
@NonNull
- public synchronized ReadingList createList(@NonNull SQLiteDatabase db,
@NonNull String title, @Nullable String description) {
+ public ReadingList createList(@NonNull SQLiteDatabase db, @NonNull String
title, @Nullable String description) {
db.beginTransaction();
try {
ReadingList protoList = new ReadingList(title, description);
@@ -100,7 +75,7 @@
}
}
- public synchronized void updateList(@NonNull ReadingList list) {
+ public void updateList(@NonNull ReadingList list) {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -117,7 +92,7 @@
}
}
- public synchronized void deleteList(@NonNull ReadingList list) {
+ public void deleteList(@NonNull ReadingList list) {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -132,7 +107,7 @@
}
}
- public synchronized void addPageToList(@NonNull ReadingList list, @NonNull
PageTitle title) {
+ public void addPageToList(@NonNull ReadingList list, @NonNull PageTitle
title) {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -144,12 +119,12 @@
SavedPageSyncService.enqueue();
}
- public synchronized void addPagesToList(@NonNull ReadingList list,
@NonNull List<ReadingListPage> pages) {
+ public void addPagesToList(@NonNull ReadingList list, @NonNull
List<ReadingListPage> pages) {
SQLiteDatabase db = getWritableDatabase();
addPagesToList(db, list, pages);
}
- public synchronized void addPagesToList(@NonNull SQLiteDatabase db,
@NonNull ReadingList list, @NonNull List<ReadingListPage> pages) {
+ public void addPagesToList(@NonNull SQLiteDatabase db, @NonNull
ReadingList list, @NonNull List<ReadingListPage> pages) {
db.beginTransaction();
try {
for (ReadingListPage page : pages) {
@@ -162,7 +137,7 @@
SavedPageSyncService.enqueue();
}
- public synchronized int addPagesToListIfNotExist(@NonNull ReadingList
list, @NonNull List<PageTitle> titles) {
+ public int addPagesToListIfNotExist(@NonNull ReadingList list, @NonNull
List<PageTitle> titles) {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
int numAdded = 0;
@@ -182,7 +157,7 @@
return numAdded;
}
- public synchronized void markPageForDeletion(@NonNull ReadingListPage
page) {
+ public void markPageForDeletion(@NonNull ReadingListPage page) {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -195,7 +170,7 @@
SavedPageSyncService.enqueue();
}
- public synchronized void markPagesForDeletion(@NonNull
List<ReadingListPage> pages) {
+ public void markPagesForDeletion(@NonNull List<ReadingListPage> pages) {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -210,7 +185,7 @@
SavedPageSyncService.enqueue();
}
- public synchronized void markPageForOffline(@NonNull ReadingListPage page,
boolean offline) {
+ public void markPageForOffline(@NonNull ReadingListPage page, boolean
offline) {
if (page.offline() == offline) {
return;
}
@@ -226,7 +201,7 @@
SavedPageSyncService.enqueue();
}
- public synchronized void markPagesForOffline(@NonNull
List<ReadingListPage> pages, boolean offline) {
+ public void markPagesForOffline(@NonNull List<ReadingListPage> pages,
boolean offline) {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -244,7 +219,7 @@
SavedPageSyncService.enqueue();
}
- public synchronized void updatePage(@NonNull ReadingListPage page) {
+ public void updatePage(@NonNull ReadingListPage page) {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -279,7 +254,7 @@
}
@Nullable
- public synchronized ReadingListPage getRandomPage() {
+ public ReadingListPage getRandomPage() {
SQLiteDatabase db = getReadableDatabase();
try (Cursor cursor = db.query(ReadingListPageContract.TABLE, null,
ReadingListPageContract.Col.STATUS.getName() + " != ?",
@@ -294,7 +269,7 @@
}
@Nullable
- public synchronized ReadingListPage findPageInAnyList(@NonNull PageTitle
title) {
+ public ReadingListPage findPageInAnyList(@NonNull PageTitle title) {
SQLiteDatabase db = getReadableDatabase();
try (Cursor cursor = db.query(ReadingListPageContract.TABLE, null,
ReadingListPageContract.Col.SITE.getName() + " = ? AND "
@@ -313,13 +288,13 @@
return null;
}
- public synchronized boolean pageExistsInList(@NonNull ReadingList list,
@NonNull PageTitle title) {
+ public boolean pageExistsInList(@NonNull ReadingList list, @NonNull
PageTitle title) {
SQLiteDatabase db = getReadableDatabase();
return pageExistsInList(db, list, title);
}
@NonNull
- public synchronized List<ReadingListPage> getAllPageOccurrences(@NonNull
PageTitle title) {
+ public List<ReadingListPage> getAllPageOccurrences(@NonNull PageTitle
title) {
List<ReadingListPage> pages = new ArrayList<>();
SQLiteDatabase db = getReadableDatabase();
try (Cursor cursor = db.query(ReadingListPageContract.TABLE, null,
@@ -340,7 +315,7 @@
}
@NonNull
- public synchronized List<ReadingList> getListsFromPageOccurrences(@NonNull
List<ReadingListPage> pages) {
+ public List<ReadingList> getListsFromPageOccurrences(@NonNull
List<ReadingListPage> pages) {
List<ReadingList> lists = new ArrayList<>();
SQLiteDatabase db = getReadableDatabase();
List<Long> listIds = new ArrayList<>();
@@ -369,7 +344,7 @@
}
@NonNull
- public synchronized List<ReadingListPage> getAllPagesToBeSaved() {
+ public List<ReadingListPage> getAllPagesToBeSaved() {
List<ReadingListPage> pages = new ArrayList<>();
SQLiteDatabase db = getReadableDatabase();
try (Cursor cursor = db.query(ReadingListPageContract.TABLE, null,
@@ -384,7 +359,7 @@
return pages;
}
- public synchronized List<ReadingListPage> getAllPagesToBeUnsaved() {
+ public List<ReadingListPage> getAllPagesToBeUnsaved() {
List<ReadingListPage> pages = new ArrayList<>();
SQLiteDatabase db = getReadableDatabase();
try (Cursor cursor = db.query(ReadingListPageContract.TABLE, null,
@@ -400,7 +375,7 @@
}
@NonNull
- public synchronized List<ReadingListPage> getAllPagesToBeDeleted() {
+ public List<ReadingListPage> getAllPagesToBeDeleted() {
List<ReadingListPage> pages = new ArrayList<>();
SQLiteDatabase db = getReadableDatabase();
try (Cursor cursor = db.query(ReadingListPageContract.TABLE, null,
@@ -414,7 +389,7 @@
return pages;
}
- public synchronized void resetUnsavedPageStatus() {
+ public void resetUnsavedPageStatus() {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -431,7 +406,7 @@
}
}
- public synchronized void purgeDeletedPages() {
+ public void purgeDeletedPages() {
SQLiteDatabase db = getWritableDatabase();
db.beginTransaction();
try {
@@ -446,7 +421,7 @@
}
@Nullable
- public synchronized ReadingList getFullListById(long id) {
+ public ReadingList getFullListById(long id) {
SQLiteDatabase db = getReadableDatabase();
ReadingList list = null;
try (Cursor cursor = db.query(ReadingListContract.TABLE, null,
diff --git
a/app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
b/app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
index 2dee453..0f4f6f7 100644
---
a/app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
+++
b/app/src/main/java/org/wikipedia/readinglist/database/ReadingListPageTable.java
@@ -6,6 +6,8 @@
import android.support.annotation.NonNull;
import android.util.Base64;
+import org.wikipedia.R;
+import org.wikipedia.WikipediaApp;
import org.wikipedia.database.DatabaseTable;
import org.wikipedia.database.column.Column;
import org.wikipedia.database.contract.OldReadingListContract;
@@ -72,10 +74,14 @@
}
@Override
- public void upgradeSchema(@NonNull SQLiteDatabase db, int fromVersion, int
toVersion) {
- super.upgradeSchema(db, fromVersion, toVersion);
+ public void onUpgradeSchema(@NonNull SQLiteDatabase db, int fromVersion,
int toVersion) {
if (toVersion == DB_VER_INTRODUCED) {
- importOldLists(db);
+ List<ReadingList> currentLists = new ArrayList<>();
+ if (fromVersion > 0) {
+ importOldLists(db, currentLists);
+ }
+ createDefaultList(db, currentLists);
+ // TODO: add other one-time conversions here.
}
}
@@ -111,10 +117,20 @@
return DB_VER_INTRODUCED;
}
+ private void createDefaultList(@NonNull SQLiteDatabase db, @NonNull
List<ReadingList> currentLists) {
+ for (ReadingList list : currentLists) {
+ if (list.isDefault()) {
+ // Already have a default list
+ return;
+ }
+ }
+ currentLists.add(ReadingListDbHelper.instance().createList(db, "",
+
WikipediaApp.getInstance().getString(R.string.default_reading_list_description)));
+ }
+
// TODO: Remove in Dec 2018
- private void importOldLists(@NonNull SQLiteDatabase db) {
+ private void importOldLists(@NonNull SQLiteDatabase db, @NonNull
List<ReadingList> lists) {
try {
- List<ReadingList> lists = new ArrayList<>();
try (Cursor cursor = db.query(OldReadingListContract.TABLE, null,
null, null, null, null, null)) {
while (cursor.moveToNext()) {
ReadingList list = new
ReadingList(OldReadingListContract.Col.TITLE.val(cursor),
--
To view, visit https://gerrit.wikimedia.org/r/402104
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia8985acc0ec498602bcb2c27d7cc14144dca05df
Gerrit-PatchSet: 3
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <dbr...@wikimedia.org>
Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org>
Gerrit-Reviewer: Cooltey <cf...@wikimedia.org>
Gerrit-Reviewer: Sharvaniharan <sha...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
