Mholloway has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/352887 )
Change subject: Add query continuation support to GalleryCollectionClient
......................................................................
Add query continuation support to GalleryCollectionClient
Bug: T152403
Change-Id: I97eec4e349f7da49834b635cf9b503bff3cea7db
---
M app/src/main/java/org/wikipedia/concurrency/CallbackTask.java
M app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java
M app/src/main/java/org/wikipedia/feed/featured/FeaturedArticleCardView.java
M app/src/main/java/org/wikipedia/feed/random/RandomCardView.java
M app/src/main/java/org/wikipedia/gallery/FilePagesWithImageInfo.java
M app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
M app/src/main/java/org/wikipedia/gallery/GalleryCollectionClient.java
M app/src/main/java/org/wikipedia/page/PageFragment.java
M app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java
M app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
M app/src/main/java/org/wikipedia/readinglist/ReadingListBookmarkMenu.java
M app/src/main/java/org/wikipedia/readinglist/ReadingListFragment.java
M app/src/main/java/org/wikipedia/readinglist/ReadingListsFragment.java
13 files changed, 150 insertions(+), 85 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia
refs/changes/87/352887/1
diff --git a/app/src/main/java/org/wikipedia/concurrency/CallbackTask.java
b/app/src/main/java/org/wikipedia/concurrency/CallbackTask.java
index d53b1f2..afa164d 100644
--- a/app/src/main/java/org/wikipedia/concurrency/CallbackTask.java
+++ b/app/src/main/java/org/wikipedia/concurrency/CallbackTask.java
@@ -5,11 +5,12 @@
public class CallbackTask<T> extends SaneAsyncTask<T> {
public interface Callback<T> {
- void success(T row);
+ void success(T result);
+ void failure(Throwable caught);
}
public interface Task<T> {
- T execute();
+ T execute() throws Throwable;
}
@NonNull private final Task<T> task;
@@ -39,4 +40,12 @@
callback = null;
}
}
+
+ @Override public void onCatch(Throwable caught) {
+ super.onCatch(caught);
+ if (callback != null) {
+ callback.failure(caught);
+ callback = null;
+ }
+ }
}
diff --git
a/app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java
b/app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java
index 13642d3..1eb1bb9 100644
--- a/app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java
+++ b/app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java
@@ -5,15 +5,22 @@
import com.google.gson.annotations.SerializedName;
+import java.util.Map;
+
public class MwQueryResponse<T> extends MwResponse {
- @SuppressWarnings("unused") @SerializedName("batchcomplete")
- private boolean batchComplete;
+ @SuppressWarnings("unused") @SerializedName("batchcomplete") private
boolean batchComplete;
+
+ @SuppressWarnings("unused") @SerializedName("continue") @Nullable private
Map<String, String> continuation;
@Nullable private T query;
public boolean batchComplete() {
return batchComplete;
+ }
+
+ @Nullable public Map<String, String> getContinuation() {
+ return continuation;
}
@Nullable public T query() {
@@ -24,8 +31,7 @@
return super.success() && query != null;
}
- @VisibleForTesting
- protected void setQuery(@Nullable T query) {
+ @VisibleForTesting protected void setQuery(@Nullable T query) {
this.query = query;
}
}
diff --git
a/app/src/main/java/org/wikipedia/feed/featured/FeaturedArticleCardView.java
b/app/src/main/java/org/wikipedia/feed/featured/FeaturedArticleCardView.java
index 5be172e..726dbde 100644
--- a/app/src/main/java/org/wikipedia/feed/featured/FeaturedArticleCardView.java
+++ b/app/src/main/java/org/wikipedia/feed/featured/FeaturedArticleCardView.java
@@ -108,8 +108,7 @@
PageTitle title = new PageTitle(card.articleTitle(), card.wikiSite());
ReadingList.DAO.anyListContainsTitleAsync(ReadingListDaoProxy.key(title),
new CallbackTask.Callback<ReadingListPage>() {
- @Override
- public void success(@Nullable ReadingListPage page) {
+ @Override public void success(@Nullable ReadingListPage
page) {
boolean listContainsTitle = page != null;
int actionIcon = listContainsTitle
@@ -135,6 +134,10 @@
footer(footer);
}
+
+ @Override public void failure(Throwable caught) {
+
+ }
});
}
diff --git a/app/src/main/java/org/wikipedia/feed/random/RandomCardView.java
b/app/src/main/java/org/wikipedia/feed/random/RandomCardView.java
index 189bf24..3fdf518 100644
--- a/app/src/main/java/org/wikipedia/feed/random/RandomCardView.java
+++ b/app/src/main/java/org/wikipedia/feed/random/RandomCardView.java
@@ -74,6 +74,10 @@
}
}
}
+
+ @Override public void failure(Throwable caught) {
+
+ }
});
}
}
diff --git
a/app/src/main/java/org/wikipedia/gallery/FilePagesWithImageInfo.java
b/app/src/main/java/org/wikipedia/gallery/FilePagesWithImageInfo.java
index 0988b60..d0a0975 100644
--- a/app/src/main/java/org/wikipedia/gallery/FilePagesWithImageInfo.java
+++ b/app/src/main/java/org/wikipedia/gallery/FilePagesWithImageInfo.java
@@ -6,18 +6,15 @@
import org.wikipedia.dataclient.mwapi.MwQueryPage;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
public class FilePagesWithImageInfo {
- @SuppressWarnings("unused") @Nullable private Map<String, MwQueryPage>
pages;
+ @SuppressWarnings("unused") @Nullable private List<MwQueryPage> pages;
@NonNull Map<String, ImageInfo> images() {
Map<String, ImageInfo> result = new HashMap<>();
- if (pages == null) {
- return result;
- }
- for (Map.Entry<String, MwQueryPage> entry : pages.entrySet()) {
- MwQueryPage page = entry.getValue();
+ for (MwQueryPage page : pages) {
if (page.imageInfo() != null) {
result.put(page.title(), page.imageInfo());
}
diff --git a/app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
b/app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
index 793e433..c48a494 100644
--- a/app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
+++ b/app/src/main/java/org/wikipedia/gallery/GalleryActivity.java
@@ -32,8 +32,8 @@
import org.wikipedia.WikipediaApp;
import org.wikipedia.activity.ThemedActionBarActivity;
import org.wikipedia.analytics.GalleryFunnel;
+import org.wikipedia.concurrency.CallbackTask;
import org.wikipedia.dataclient.WikiSite;
-import org.wikipedia.dataclient.mwapi.MwQueryResponse;
import org.wikipedia.feed.image.FeaturedImage;
import org.wikipedia.history.HistoryEntry;
import org.wikipedia.json.GsonMarshaller;
@@ -59,8 +59,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-
-import retrofit2.Call;
import static org.wikipedia.util.StringUtil.strip;
import static org.wikipedia.util.UriUtil.handleExternalLink;
@@ -471,18 +469,19 @@
}
updateProgressBar(true, true, 0);
- client.request(pageTitle.getWikiSite(), pageTitle, false, new
GalleryCollectionClient.Callback() {
- @Override public void success(@NonNull
Call<MwQueryResponse<FilePagesWithImageInfo>> call,
- @NonNull Map<String, ImageInfo>
result) {
+ CallbackTask.execute(new CallbackTask.Task<Map<String, ImageInfo>>() {
+ @Override public Map<String, ImageInfo> execute() throws Throwable
{
+ return client.request(pageTitle.getWikiSite(), pageTitle,
false);
+ }
+ }, new CallbackTask.Callback<Map<String, ImageInfo>>() {
+ @Override public void success(Map<String, ImageInfo> result) {
updateProgressBar(false, true, 0);
applyGalleryCollection(buildCollection(result));
}
- @Override public void failure(@NonNull
Call<MwQueryResponse<FilePagesWithImageInfo>> call,
- @NonNull Throwable caught) {
+ @Override public void failure(Throwable caught) {
updateProgressBar(false, true, 0);
showError(caught, false);
- L.e("Failed to fetch gallery collection.", caught);
}
});
}
diff --git
a/app/src/main/java/org/wikipedia/gallery/GalleryCollectionClient.java
b/app/src/main/java/org/wikipedia/gallery/GalleryCollectionClient.java
index 188382c..6c84322 100644
--- a/app/src/main/java/org/wikipedia/gallery/GalleryCollectionClient.java
+++ b/app/src/main/java/org/wikipedia/gallery/GalleryCollectionClient.java
@@ -5,74 +5,100 @@
import android.support.annotation.VisibleForTesting;
import org.wikipedia.dataclient.WikiSite;
-import org.wikipedia.dataclient.mwapi.MwException;
import org.wikipedia.dataclient.mwapi.MwQueryResponse;
import org.wikipedia.dataclient.retrofit.MwCachedService;
import org.wikipedia.dataclient.retrofit.WikiCachedService;
import org.wikipedia.page.PageTitle;
+import org.wikipedia.util.log.L;
import java.io.IOException;
+import java.util.HashMap;
import java.util.Map;
import retrofit2.Call;
-import retrofit2.Response;
import retrofit2.http.GET;
import retrofit2.http.Query;
+import retrofit2.http.QueryMap;
import static org.wikipedia.Constants.PREFERRED_THUMB_SIZE;
public class GalleryCollectionClient {
+ // TODO: This limit seems pretty arbitrary. Do we need or want to adjust
it?
private static final int MAX_ITEM_COUNT = 256;
@NonNull private final WikiCachedService<Service> cachedService = new
MwCachedService<>(Service.class);
- public interface Callback {
- void success(@NonNull Call<MwQueryResponse<FilePagesWithImageInfo>>
call, @NonNull Map<String, ImageInfo> result);
- void failure(@NonNull Call<MwQueryResponse<FilePagesWithImageInfo>>
call, @NonNull Throwable caught);
+ public Map<String, ImageInfo> request(@NonNull WikiSite wiki, @NonNull
PageTitle title, boolean getThumbs) throws IOException {
+ return request(cachedService.service(wiki), title, getThumbs);
}
- public Call<MwQueryResponse<FilePagesWithImageInfo>> request(@NonNull
WikiSite wiki,
- @NonNull
PageTitle title,
- boolean
getThumbs,
- @NonNull
Callback cb) {
- return request(cachedService.service(wiki), title, getThumbs, cb);
+ @VisibleForTesting Map<String, ImageInfo> request(@NonNull Service
service, @NonNull PageTitle title, boolean getThumbs)
+ throws IOException {
+
+ Map<String, ImageInfo> result = new HashMap<>();
+ MwQueryResponse<FilePagesWithImageInfo> currentResponse;
+ // TODO: Remove init to null after finished debugging
+ Map<String, String> continuation = null;
+
+ do {
+ L.d("Continuation = " + continuation);
+ currentResponse = continuation == null
+ ? fetch(service, title, getThumbs)
+ : continueFetch(service, title, getThumbs, continuation);
+ if (currentResponse.success()) {
+ // TODO: Technically, new results should be merged with rather
than overwrite old ones.
+ // However, Map.merge() requires Java 8. As of this writing
(May 2017), the Jack
+ // compiler is deprecated, but still required to bump
JAVA_VERSION to 1_8.
+ //
+ // In the meantime, based on manual testing, overwriting seems
not to result in any
+ // information loss, and should be adequate in practice.
+
+ // noinspection ConstantConditions
+ result.putAll(currentResponse.query().images());
+ continuation = currentResponse.getContinuation();
+ }
+ } while (continuation != null);
+
+ if (!currentResponse.batchComplete()) {
+ throw new IllegalStateException("No more continuation info but
!batchcomplete");
+ }
+
+ return result;
}
- @VisibleForTesting Call<MwQueryResponse<FilePagesWithImageInfo>>
request(@NonNull Service service,
-
@NonNull PageTitle title,
-
boolean getThumbs,
-
@NonNull final Callback cb) {
+ private MwQueryResponse<FilePagesWithImageInfo> fetch(@NonNull Service
service, @NonNull PageTitle title, boolean getThumbs)
+ throws IOException {
Call<MwQueryResponse<FilePagesWithImageInfo>> call = getThumbs
- ? service.fetchCollection("dimensions|mime|url",
Integer.toString(PREFERRED_THUMB_SIZE),
+ ? service.fetch("dimensions|mime|url",
Integer.toString(PREFERRED_THUMB_SIZE),
Integer.toString(PREFERRED_THUMB_SIZE), title.toString())
- : service.fetchCollection("dimensions|mime", null, null,
title.toString());
- call.enqueue(new
retrofit2.Callback<MwQueryResponse<FilePagesWithImageInfo>>() {
- @Override public void
onResponse(Call<MwQueryResponse<FilePagesWithImageInfo>> call,
-
Response<MwQueryResponse<FilePagesWithImageInfo>> response) {
- if (response.body().success()) {
- // noinspection ConstantConditions
- cb.success(call, response.body().query().images());
- } else if (response.body().hasError()) {
- // noinspection ConstantConditions
- cb.failure(call, new
MwException(response.body().getError()));
- } else {
- cb.failure(call, new IOException("An unknown error
occurred."));
- }
- }
-
- @Override public void
onFailure(Call<MwQueryResponse<FilePagesWithImageInfo>> call, Throwable
throwable) {
- cb.failure(call, throwable);
- }
- });
- return call;
+ : service.fetch("dimensions|mime", null, null,
title.toString());
+ return call.execute().body();
}
+
+ private MwQueryResponse<FilePagesWithImageInfo> continueFetch(@NonNull
Service service, @NonNull PageTitle title,
+ boolean
getThumbs, @Nullable Map<String, String> continuation)
+ throws IOException {
+ Call<MwQueryResponse<FilePagesWithImageInfo>> call = getThumbs
+ ? service.continueFetch("dimensions|mime|url",
Integer.toString(PREFERRED_THUMB_SIZE),
+ Integer.toString(PREFERRED_THUMB_SIZE), title.toString(),
continuation)
+ : service.continueFetch("dimensions|mime", null, null,
title.toString(), continuation);
+ return call.execute().body();
+ }
+
private interface Service {
-
@GET("w/api.php?action=query&format=json&prop=imageinfo&generator=images&redirects="
- + "gimlimit=" + MAX_ITEM_COUNT)
- Call<MwQueryResponse<FilePagesWithImageInfo>> fetchCollection(@NonNull
@Query("iiprop") String properties,
-
@Nullable @Query("iiurlwidth") String thumbWidth,
-
@Nullable @Query("iiurlheight") String thumbHeight,
- @NonNull
@Query("titles") String title);
+
@GET("w/api.php?action=query&format=json&formatversion=2&prop=imageinfo&generator=images&redirects=&gimlimit="
+ MAX_ITEM_COUNT)
+ Call<MwQueryResponse<FilePagesWithImageInfo>> fetch(@NonNull
@Query("iiprop") String properties,
+ @Nullable
@Query("iiurlwidth") String thumbWidth,
+ @Nullable
@Query("iiurlheight") String thumbHeight,
+ @NonNull
@Query("titles") String title);
+
+ // N.B. @QueryMap will throw if it receives a null parameter, separate
handling is required.
+
@GET("w/api.php?action=query&format=json&formatversion=2&prop=imageinfo&generator=images&redirects=&gimlimit="
+ MAX_ITEM_COUNT)
+ Call<MwQueryResponse<FilePagesWithImageInfo>> continueFetch(@NonNull
@Query("iiprop") String properties,
+ @Nullable
@Query("iiurlwidth") String thumbWidth,
+ @Nullable
@Query("iiurlheight") String thumbHeight,
+ @NonNull
@Query("titles") String title,
+ @NonNull
@QueryMap Map<String, String> continuation);
}
}
diff --git a/app/src/main/java/org/wikipedia/page/PageFragment.java
b/app/src/main/java/org/wikipedia/page/PageFragment.java
index ca4f7d6..b044e04 100755
--- a/app/src/main/java/org/wikipedia/page/PageFragment.java
+++ b/app/src/main/java/org/wikipedia/page/PageFragment.java
@@ -706,6 +706,10 @@
pageActionTabsCallback.updateBookmark(false);
}
}
+
+ @Override public void failure(Throwable caught) {
+
+ }
});
}
diff --git
a/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java
b/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java
index e9fb1c1..6c7c5ef 100755
--- a/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java
+++ b/app/src/main/java/org/wikipedia/page/linkpreview/LinkPreviewDialog.java
@@ -24,11 +24,10 @@
import org.wikipedia.activity.FragmentUtil;
import org.wikipedia.analytics.GalleryFunnel;
import org.wikipedia.analytics.LinkPreviewFunnel;
+import org.wikipedia.concurrency.CallbackTask;
import org.wikipedia.dataclient.ServiceError;
-import org.wikipedia.dataclient.mwapi.MwQueryResponse;
import org.wikipedia.dataclient.page.PageClientFactory;
import org.wikipedia.dataclient.page.PageSummary;
-import org.wikipedia.gallery.FilePagesWithImageInfo;
import org.wikipedia.gallery.GalleryActivity;
import org.wikipedia.gallery.GalleryCollection;
import org.wikipedia.gallery.GalleryCollectionClient;
@@ -125,19 +124,22 @@
thumbnailGallery = (GalleryThumbnailScrollView)
rootView.findViewById(R.id.link_preview_thumbnail_gallery);
if (app.isImageDownloadEnabled()) {
- client.request(pageTitle.getWikiSite(), pageTitle, true, new
GalleryCollectionClient.Callback() {
- @Override public void success(@NonNull
Call<MwQueryResponse<FilePagesWithImageInfo>> call,
- @NonNull Map<String, ImageInfo>
result) {
+ CallbackTask.execute(new CallbackTask.Task<Map<String,
ImageInfo>>() {
+ @Override public Map<String, ImageInfo> execute() throws
Throwable {
+ return client.request(pageTitle.getWikiSite(), pageTitle,
true);
+
+ }
+ }, new CallbackTask.Callback<Map<String, ImageInfo>>() {
+ @Override public void success(@Nullable Map<String, ImageInfo>
result) {
setThumbGallery(result);
+
thumbnailGallery.setGalleryViewListener(galleryViewListener);
}
- @Override public void failure(@NonNull
Call<MwQueryResponse<FilePagesWithImageInfo>> call,
- @NonNull Throwable caught) {
- // Don't worry about showing a notification to the user if
this fails.
+ @Override
+ public void failure(Throwable caught) {
L.w("Failed to fetch gallery collection.", caught);
}
});
- thumbnailGallery.setGalleryViewListener(galleryViewListener);
}
overflowButton =
rootView.findViewById(R.id.link_preview_overflow_button);
diff --git
a/app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
b/app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
index 8e42240..0ce6a5c 100644
--- a/app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
+++ b/app/src/main/java/org/wikipedia/readinglist/AddToReadingListDialog.java
@@ -156,11 +156,14 @@
private void updateLists() {
ReadingList.DAO.queryMruLists(null, new
CallbackTask.Callback<List<ReadingList>>() {
- @Override
- public void success(List<ReadingList> rows) {
+ @Override public void success(List<ReadingList> rows) {
readingLists.set(rows);
readingLists.sort(Prefs.getReadingListSortMode(ReadingLists.SORT_BY_NAME_ASC));
adapter.notifyDataSetChanged();
+ }
+
+ @Override public void failure(Throwable caught) {
+
}
});
}
@@ -199,8 +202,7 @@
private void addAndDismiss(final ReadingList readingList) {
final ReadingListPage page = findOrCreatePage(readingList, pageTitle);
ReadingList.DAO.listContainsTitleAsync(readingList, page, new
CallbackTask.Callback<Boolean>() {
- @Override
- public void success(Boolean contains) {
+ @Override public void success(Boolean contains) {
if (isAdded()) {
String message;
if (contains) {
@@ -220,6 +222,10 @@
dismiss();
}
}
+
+ @Override public void failure(Throwable caught) {
+
+ }
});
}
diff --git
a/app/src/main/java/org/wikipedia/readinglist/ReadingListBookmarkMenu.java
b/app/src/main/java/org/wikipedia/readinglist/ReadingListBookmarkMenu.java
index 3ee8d29..9afc90c 100644
--- a/app/src/main/java/org/wikipedia/readinglist/ReadingListBookmarkMenu.java
+++ b/app/src/main/java/org/wikipedia/readinglist/ReadingListBookmarkMenu.java
@@ -32,14 +32,17 @@
public void show(@NonNull PageTitle title) {
ReadingList.DAO.anyListContainsTitleAsync(ReadingListDaoProxy.key(title),
new CallbackTask.Callback<ReadingListPage>() {
- @Override
- public void success(@Nullable ReadingListPage page) {
+ @Override public void success(@Nullable ReadingListPage
page) {
if (!ViewCompat.isAttachedToWindow(anchorView)) {
return;
}
ReadingListBookmarkMenu.this.page = page;
showMenu();
}
+
+ @Override public void failure(Throwable caught) {
+
+ }
});
}
diff --git
a/app/src/main/java/org/wikipedia/readinglist/ReadingListFragment.java
b/app/src/main/java/org/wikipedia/readinglist/ReadingListFragment.java
index 3d90bb2..0edc9e6 100644
--- a/app/src/main/java/org/wikipedia/readinglist/ReadingListFragment.java
+++ b/app/src/main/java/org/wikipedia/readinglist/ReadingListFragment.java
@@ -211,8 +211,7 @@
private void updateReadingListData() {
ReadingList.DAO.queryMruLists(null, new
CallbackTask.Callback<List<ReadingList>>() {
- @Override
- public void success(List<ReadingList> lists) {
+ @Override public void success(List<ReadingList> lists) {
if (getActivity() == null) {
return;
}
@@ -224,6 +223,10 @@
}
update();
}
+
+ @Override public void failure(Throwable caught) {
+
+ }
});
}
diff --git
a/app/src/main/java/org/wikipedia/readinglist/ReadingListsFragment.java
b/app/src/main/java/org/wikipedia/readinglist/ReadingListsFragment.java
index de33741..595272b 100644
--- a/app/src/main/java/org/wikipedia/readinglist/ReadingListsFragment.java
+++ b/app/src/main/java/org/wikipedia/readinglist/ReadingListsFragment.java
@@ -159,10 +159,8 @@
}
private void updateLists(@Nullable final String searchQuery) {
- ReadingList.DAO.queryMruLists(searchQuery,
- new CallbackTask.Callback<List<ReadingList>>() {
- @Override
- public void success(List<ReadingList> rows) {
+ ReadingList.DAO.queryMruLists(searchQuery, new
CallbackTask.Callback<List<ReadingList>>() {
+ @Override public void success(List<ReadingList> rows) {
if (getActivity() == null) {
return;
}
@@ -171,6 +169,11 @@
updateEmptyState(searchQuery);
maybeDeleteListFromIntent();
}
+
+ @Override
+ public void failure(Throwable caught) {
+
+ }
});
}
--
To view, visit https://gerrit.wikimedia.org/r/352887
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I97eec4e349f7da49834b635cf9b503bff3cea7db
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Mholloway <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
