Skip to content
This repository was archived by the owner on Jun 17, 2024. It is now read-only.

Commit f6bf84b

Browse files
t-p-whitervandermeulen
t-p-white
authored and
rvandermeulen
committed
Bug 1892126 - Updated jsonToPocketApiStory function to handle null values r=android-reviewers,rsainani
Original Revision: https://phabricator.services.mozilla.com/D207990 Differential Revision: https://phabricator.services.mozilla.com/D208117
1 parent 0189696 commit f6bf84b

File tree

6 files changed

+102
-19
lines changed

6 files changed

+102
-19
lines changed

‎android-components/components/service/pocket/src/main/java/mozilla/components/service/pocket/stories/api/PocketJSONParser.kt‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,34 @@ internal class PocketJSONParser {
3333
val stories = storiesJSON.mapNotNull(JSONArray::getJSONObject) { jsonToPocketApiStory(it) }
3434

3535
// We return null, rather than the empty list, because devs might forget to check an empty list.
36-
if (stories.isNotEmpty()) stories else null
36+
stories.ifEmpty { null }
3737
} catch (e: JSONException) {
3838
logger.warn("invalid JSON from the Pocket endpoint", e)
3939
null
4040
}
4141

4242
private fun jsonToPocketApiStory(json: JSONObject): PocketApiStory? = try {
43-
PocketApiStory(
44-
// These three properties are required for any valid recommendation.
45-
title = json.getString(JSON_STORY_TITLE_KEY),
46-
url = json.getString(JSON_STORY_URL_KEY),
47-
imageUrl = json.getString(JSON_STORY_IMAGE_URL_KEY),
48-
// The following three properties are optional.
49-
publisher = json.tryGetString(JSON_STORY_PUBLISHER_KEY) ?: STRING_NOT_FOUND_DEFAULT_VALUE,
50-
category = json.tryGetString(JSON_STORY_CATEGORY_KEY) ?: STRING_NOT_FOUND_DEFAULT_VALUE,
51-
timeToRead = json.tryGetInt(JSON_STORY_TIME_TO_READ_KEY) ?: INT_NOT_FOUND_DEFAULT_VALUE,
52-
)
43+
val title = json.tryGetString(JSON_STORY_TITLE_KEY)
44+
val url = json.tryGetString(JSON_STORY_URL_KEY)
45+
val imageUrl = json.tryGetString(JSON_STORY_IMAGE_URL_KEY)
46+
47+
// These three properties are required for any valid recommendation.
48+
if (title == null || url == null || imageUrl == null) {
49+
null
50+
} else {
51+
PocketApiStory(
52+
title = title,
53+
url = url,
54+
imageUrl = imageUrl,
55+
// The following three properties are optional.
56+
publisher = json.tryGetString(JSON_STORY_PUBLISHER_KEY)
57+
?: STRING_NOT_FOUND_DEFAULT_VALUE,
58+
category = json.tryGetString(JSON_STORY_CATEGORY_KEY)
59+
?: STRING_NOT_FOUND_DEFAULT_VALUE,
60+
timeToRead = json.tryGetInt(JSON_STORY_TIME_TO_READ_KEY)
61+
?: INT_NOT_FOUND_DEFAULT_VALUE,
62+
)
63+
}
5364
} catch (e: JSONException) {
5465
null
5566
}

‎android-components/components/service/pocket/src/test/java/mozilla/components/service/pocket/helpers/PocketTestResources.kt‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,26 @@ private const val POCKET_DIR = "pocket"
1818
* Accessors to resources used in testing.
1919
*/
2020
internal object PocketTestResources {
21-
val pocketEndointFiveStoriesResponse = this::class.java.classLoader!!.getResource(
21+
val pocketEndpointFiveStoriesResponse = this::class.java.classLoader!!.getResource(
2222
"$POCKET_DIR/stories_recommendations_response.json",
2323
)!!.readText()
2424

2525
val pocketEndpointThreeSpocsResponse = this::class.java.classLoader!!.getResource(
2626
"$POCKET_DIR/sponsored_stories_response.json",
2727
)!!.readText()
2828

29+
val pocketEndpointNullTitleStoryBadResponse = this::class.java.classLoader!!.getResource(
30+
"$POCKET_DIR/story_recommendation_null_title_response.json",
31+
)!!.readText()
32+
33+
val pocketEndpointNullUrlStoryBadResponse = this::class.java.classLoader!!.getResource(
34+
"$POCKET_DIR/story_recommendation_null_url_response.json",
35+
)!!.readText()
36+
37+
val pocketEndpointNullImageUrlStoryBadResponse = this::class.java.classLoader!!.getResource(
38+
"$POCKET_DIR/story_recommendation_null_imageUrl_response.json",
39+
)!!.readText()
40+
2941
val apiExpectedPocketStoriesRecommendations: List<PocketApiStory> = listOf(
3042
PocketApiStory(
3143
title = "How to Remember Anything You Really Want to Remember, Backed by Science",

‎android-components/components/service/pocket/src/test/java/mozilla/components/service/pocket/stories/api/PocketJSONParserTest.kt‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class PocketJSONParserTest {
3535
@Test
3636
fun `GIVEN PocketJSONParser WHEN parsing valid stories recommendations THEN PocketApiStories are returned`() {
3737
val expectedStories = PocketTestResources.apiExpectedPocketStoriesRecommendations
38-
val pocketJSON = PocketTestResources.pocketEndointFiveStoriesResponse
38+
val pocketJSON = PocketTestResources.pocketEndpointFiveStoriesResponse
3939
val actualStories = parser.jsonToPocketApiStories(pocketJSON)
4040

4141
assertNotNull(actualStories)
@@ -45,7 +45,7 @@ class PocketJSONParserTest {
4545

4646
@Test
4747
fun `WHEN parsing stories recommendations with missing titles THEN those entries are dropped`() {
48-
val pocketJSON = PocketTestResources.pocketEndointFiveStoriesResponse
48+
val pocketJSON = PocketTestResources.pocketEndpointFiveStoriesResponse
4949
val expectedStoriesIfMissingTitle = ArrayList(PocketTestResources.apiExpectedPocketStoriesRecommendations)
5050
.apply { removeAt(4) }
5151
val pocketJsonWithMissingTitle = removeJsonFieldFromArrayIndex("title", 4, pocketJSON)
@@ -56,9 +56,17 @@ class PocketJSONParserTest {
5656
assertEquals(expectedStoriesIfMissingTitle.joinToString(), result.joinToString())
5757
}
5858

59+
@Test
60+
fun `WHEN parsing stories recommendations with a null title value THEN those entries are dropped`() {
61+
val pocketJSON = PocketTestResources.pocketEndpointNullTitleStoryBadResponse
62+
val result = parser.jsonToPocketApiStories(pocketJSON)
63+
64+
assertNull(result)
65+
}
66+
5967
@Test
6068
fun `WHEN parsing stories recommendations with missing urls THEN those entries are dropped`() {
61-
val pocketJSON = PocketTestResources.pocketEndointFiveStoriesResponse
69+
val pocketJSON = PocketTestResources.pocketEndpointFiveStoriesResponse
6270
val expectedStoriesIfMissingUrl = ArrayList(PocketTestResources.apiExpectedPocketStoriesRecommendations)
6371
.apply { removeAt(3) }
6472
val pocketJsonWithMissingUrl = removeJsonFieldFromArrayIndex("url", 3, pocketJSON)
@@ -69,9 +77,17 @@ class PocketJSONParserTest {
6977
assertEquals(expectedStoriesIfMissingUrl.joinToString(), result.joinToString())
7078
}
7179

80+
@Test
81+
fun `WHEN parsing stories recommendations with a null url THEN those entries are dropped`() {
82+
val pocketJSON = PocketTestResources.pocketEndpointNullUrlStoryBadResponse
83+
val result = parser.jsonToPocketApiStories(pocketJSON)
84+
85+
assertNull(result)
86+
}
87+
7288
@Test
7389
fun `WHEN parsing stories recommendations with missing imageUrls THEN those entries are dropped`() {
74-
val pocketJSON = PocketTestResources.pocketEndointFiveStoriesResponse
90+
val pocketJSON = PocketTestResources.pocketEndpointFiveStoriesResponse
7591
val expectedStoriesIfMissingImageUrl = ArrayList(PocketTestResources.apiExpectedPocketStoriesRecommendations)
7692
.apply { removeAt(2) }
7793
val pocketJsonWithMissingImageUrl = removeJsonFieldFromArrayIndex("imageUrl", 2, pocketJSON)
@@ -82,9 +98,17 @@ class PocketJSONParserTest {
8298
assertEquals(expectedStoriesIfMissingImageUrl.joinToString(), result.joinToString())
8399
}
84100

101+
@Test
102+
fun `WHEN parsing story recommendations with a null imageUrl THEN those entries are dropped`() {
103+
val pocketJSON = PocketTestResources.pocketEndpointNullImageUrlStoryBadResponse
104+
val result = parser.jsonToPocketApiStories(pocketJSON)
105+
106+
assertNull(result)
107+
}
108+
85109
@Test
86110
fun `WHEN parsing stories recommendations with missing publishers THEN those entries are kept but with default values`() {
87-
val pocketJSON = PocketTestResources.pocketEndointFiveStoriesResponse
111+
val pocketJSON = PocketTestResources.pocketEndpointFiveStoriesResponse
88112
val expectedStoriesIfMissingPublishers = PocketTestResources.apiExpectedPocketStoriesRecommendations
89113
.mapIndexed { index, story ->
90114
if (index == 2) {
@@ -103,7 +127,7 @@ class PocketJSONParserTest {
103127

104128
@Test
105129
fun `WHEN parsing stories recommendations with missing categories THEN those entries are kept but with default values`() {
106-
val pocketJSON = PocketTestResources.pocketEndointFiveStoriesResponse
130+
val pocketJSON = PocketTestResources.pocketEndpointFiveStoriesResponse
107131
val expectedStoriesIfMissingCategories = PocketTestResources.apiExpectedPocketStoriesRecommendations
108132
.mapIndexed { index, story ->
109133
if (index == 3) {
@@ -122,7 +146,7 @@ class PocketJSONParserTest {
122146

123147
@Test
124148
fun `WHEN parsing stories recommendations with missing timeToRead THEN those entries are kept but with default values`() {
125-
val pocketJSON = PocketTestResources.pocketEndointFiveStoriesResponse
149+
val pocketJSON = PocketTestResources.pocketEndpointFiveStoriesResponse
126150
val expectedStoriesIfMissingTimeToRead = PocketTestResources.apiExpectedPocketStoriesRecommendations
127151
.mapIndexed { index, story ->
128152
if (index == 4) {

‎android-components/components/service/pocket/src/test/resources/pocket/story_recommendation_null_imageUrl_response.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"recommendations": [
3+
{
4+
"category": "science",
5+
"url": "https://getpocket.com/explore/item/you-think-you-know-what-blue-is-but-you-have-no-idea",
6+
"title": "You Think You Know What Blue Is, But You Have No Idea",
7+
"imageUrl": null,
8+
"publisher": "Pocket",
9+
"timeToRead": 3
10+
}
11+
]
12+
}

‎android-components/components/service/pocket/src/test/resources/pocket/story_recommendation_null_title_response.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"recommendations": [
3+
{
4+
"category": "science",
5+
"url": "https://getpocket.com/explore/item/you-think-you-know-what-blue-is-but-you-have-no-idea",
6+
"title": null,
7+
"imageUrl": "https://img-getpocket.cdn.mozilla.net/{wh}/filters:format(jpeg):quality(60):no_upscale():strip_exif()/https%3A%2F%2Fpocket-image-cache.com%2F1200x%2Ffilters%3Aformat(jpg)%3Aextract_focal()%2Fhttps%253A%252F%252Fpocket-syndicated-images.s3.amazonaws.com%252Farticles%252F3713%252F1584373694_GettyImages-83522858.jpg",
8+
"publisher": "Pocket",
9+
"timeToRead": 3
10+
}
11+
]
12+
}

‎android-components/components/service/pocket/src/test/resources/pocket/story_recommendation_null_url_response.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"recommendations": [
3+
{
4+
"category": "science",
5+
"url": null,
6+
"title": "You Think You Know What Blue Is, But You Have No Idea",
7+
"imageUrl": "https://img-getpocket.cdn.mozilla.net/{wh}/filters:format(jpeg):quality(60):no_upscale():strip_exif()/https%3A%2F%2Fpocket-image-cache.com%2F1200x%2Ffilters%3Aformat(jpg)%3Aextract_focal()%2Fhttps%253A%252F%252Fpocket-syndicated-images.s3.amazonaws.com%252Farticles%252F3713%252F1584373694_GettyImages-83522858.jpg",
8+
"publisher": "Pocket",
9+
"timeToRead": 3
10+
}
11+
]
12+
}

0 commit comments

Comments
 (0)