Bug 1398368: Drop malformed Pocket Top Stories. r=liuche
I tested this through the unit tests I added.
In theory, we could also validate URLs to make sure they're valid but users
should see 404s if they're not valid so this seems like unnecessary code.
MozReview-Commit-ID: 3XqsMawLabj
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamRecyclerAdapter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamRecyclerAdapter.java
@@ -50,17 +50,17 @@ public class StreamRecyclerAdapter exten
private static final String LOGTAG = StringUtils.safeSubstring("Gecko" + StreamRecyclerAdapter.class.getSimpleName(), 0, 23);
private Cursor topSitesCursor;
private List<RowModel> recyclerViewModel; // List of item types backing this RecyclerView.
private List<TopStory> topStoriesQueue;
// Content sections available on the Activity Stream page. These may be hidden if the sections are disabled.
private final RowItemType[] ACTIVITY_STREAM_SECTIONS = { RowItemType.TOP_PANEL, RowItemType.TOP_STORIES_TITLE, RowItemType.HIGHLIGHTS_TITLE };
- private final int MAX_TOP_STORIES = 3;
+ public static final int MAX_TOP_STORIES = 3;
private HomePager.OnUrlOpenListener onUrlOpenListener;
private HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener;
private int tiles;
private int tilesSize;
public enum RowItemType {
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topstories/PocketStoriesLoader.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/topstories/PocketStoriesLoader.java
@@ -12,16 +12,17 @@ import android.support.v4.content.AsyncT
import android.text.TextUtils;
import android.util.Log;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.mozilla.gecko.AppConstants;
import org.mozilla.gecko.Locales;
+import org.mozilla.gecko.activitystream.homepanel.StreamRecyclerAdapter;
import org.mozilla.gecko.activitystream.homepanel.model.TopStory;
import org.mozilla.gecko.util.FileUtils;
import org.mozilla.gecko.util.ProxySelector;
import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
@@ -57,17 +58,17 @@ public class PocketStoriesLoader extends
private static final String CACHE_TIMESTAMP_MILLIS_PREFIX = "timestampMillis-";
private static final String STORIES_CACHE_PREFIX = "storiesCache-";
// Pocket API params and defaults
private static final String GLOBAL_ENDPOINT = "https://getpocket.cdn.mozilla.net/v3/firefox/global-recs";
private static final String PARAM_APIKEY = "consumer_key";
private static final String APIKEY = AppConstants.MOZ_POCKET_API_KEY;
private static final String PARAM_COUNT = "count";
- private static final int DEFAULT_COUNT = 3;
+ private static final int DEFAULT_COUNT = StreamRecyclerAdapter.MAX_TOP_STORIES + 1; // We have extra so we can hide malformed items.
private static final String PARAM_LOCALE = "locale_lang";
private static final long REFRESH_INTERVAL_MILLIS = TimeUnit.HOURS.toMillis(1);
private static final int BUFFER_SIZE = 2048;
private static final int CONNECT_TIMEOUT = (int) TimeUnit.SECONDS.toMillis(15);
private static final int READ_TIMEOUT = (int) TimeUnit.SECONDS.toMillis(15);
@@ -159,29 +160,38 @@ public class PocketStoriesLoader extends
final JSONObject jsonObject = new JSONObject(jsonResponse);
JSONArray arr = jsonObject.getJSONArray("list");
for (int i = 0; i < arr.length(); i++) {
try {
final JSONObject item = arr.getJSONObject(i);
final String title = item.getString("title");
final String url = item.getString("dedupe_url");
final String imageUrl = item.getString("image_src");
+
+ // Drop malformed items.
+ if (TextUtils.isEmpty(title) ||
+ TextUtils.isEmpty(url) ||
+ TextUtils.isEmpty(imageUrl)) {
+ Log.d(LOGTAG, "Dropping malformed Pocket top story.");
+ continue;
+ }
+
topStories.add(new TopStory(title, url, imageUrl));
} catch (JSONException e) {
Log.e(LOGTAG, "Couldn't parse fields in Pocket response", e);
}
}
} catch (JSONException e) {
Log.e(LOGTAG, "Couldn't load Pocket response", e);
}
return topStories;
}
private static List<TopStory> makePlaceholderStories() {
final List<TopStory> stories = new LinkedList<>();
final String TITLE_PREFIX = "Placeholder ";
- for (int i = 0; i < 3; i++) {
+ for (int i = 0; i < DEFAULT_COUNT; i++) {
// Urls must be different for bookmark/pinning UI to work properly. Assume this is true for Pocket stories.
stories.add(new TopStory(TITLE_PREFIX + i, "https://www.mozilla.org/#" + i, null));
}
return stories;
}
}
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/activitystream/homepanel/topstories/TestPocketStoriesLoader.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/activitystream/homepanel/topstories/TestPocketStoriesLoader.java
@@ -1,16 +1,15 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.gecko.activitystream.homepanel.topstories;
import junit.framework.Assert;
-
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mozilla.gecko.activitystream.homepanel.model.TopStory;
import org.mozilla.gecko.background.testhelpers.TestRunner;
@@ -77,16 +76,49 @@ public class TestPocketStoriesLoader {
public void testJSONStringToTopStoriesWithMissingTitle() {
final JSONObject storyItem = makeBasicStoryItem();
storyItem.remove(KEY_TITLE);
final String malformedResponseString = makeBasicPocketResponse(new JSONObject[] { storyItem });
final List<TopStory> stories = PocketStoriesLoader.jsonStringToTopStories(malformedResponseString);
Assert.assertEquals(0, stories.size()); // Should skip malformed item.
}
+ @Test
+ public void testJSONStringToTopStoriesDropsMalformedTitle() throws Exception {
+ final JSONObject malformedStory = makeBasicStoryItem();
+ malformedStory.put(KEY_TITLE, "");
+ assertJSONStringToTopStoriesDropsMalformedStory(malformedStory);
+ }
+
+ @Test
+ public void testJSONStringToTopStoriesDropsMalformedURL() throws Exception {
+ final JSONObject malformedStory = makeBasicStoryItem();
+ malformedStory.put(KEY_DEDUPE_URL, "");
+ assertJSONStringToTopStoriesDropsMalformedStory(malformedStory);
+ }
+
+ @Test
+ public void testJSONStringToTopStoriesDropsMalformedImageURL() throws Exception {
+ final JSONObject malformedStory = makeBasicStoryItem();
+ malformedStory.put(KEY_IMAGE_SRC, "");
+ assertJSONStringToTopStoriesDropsMalformedStory(malformedStory);
+ }
+
+ private void assertJSONStringToTopStoriesDropsMalformedStory(final JSONObject malformedStory) throws JSONException {
+ final JSONObject expectedStory = makeBasicStoryItem();
+ final String expectedStoryTitle = "expectedItem";
+ expectedStory.put(KEY_TITLE, expectedStoryTitle);
+
+ final String jsonResponse = makeBasicPocketResponse(new JSONObject[] { expectedStory, malformedStory });
+ final List<TopStory> actualTopStories = PocketStoriesLoader.jsonStringToTopStories(jsonResponse);
+
+ Assert.assertEquals("Expected one malformed item to be removed, leaving size 1", 1, actualTopStories.size());
+ Assert.assertEquals(expectedStoryTitle, actualTopStories.get(0).getTitle());
+ }
+
// Pulled 8/28 Pocket response, with some trimming for content/brevity.
final String VALID_POCKET_RESPONSE = "{\"status\":1,\"list\":[" +
"{\"id\":2910,\"url\":\"https:\\/\\/pocket.co\\/pocket-shorted-url\"," +
"\"dedupe_url\":\"http:\\/\\/www.bbc.co.uk\\/actual-url\"," +
"\"title\":\"A Title Here\"," +
"\"excerpt\":\"Middle-aged people are being urged...\"," +
"\"domain\":\"bbc.co.uk\"," +
"\"image_src\":\"https:\\/\\/img.cloudfront.net\"," +