From 14f1d0becd64877e8ab8a30ec62da449901d2771 Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Fri, 31 May 2019 16:52:16 -0700 Subject: [PATCH] Issue #2325: Add test seam and tests for PocketVideoStore/JSONValidator. --- .../tv/firefox/pocket/PocketVideoStore.kt | 60 ++++-- .../tv/firefox/utils/ServiceLocator.kt | 4 +- .../pocket/PocketVideoJSONValidatorTest.kt | 71 +++++++ .../tv/firefox/pocket/PocketVideoStoreTest.kt | 187 ++++++++++++++++++ 4 files changed, 300 insertions(+), 22 deletions(-) create mode 100644 app/src/test/java/org/mozilla/tv/firefox/pocket/PocketVideoJSONValidatorTest.kt create mode 100644 app/src/test/java/org/mozilla/tv/firefox/pocket/PocketVideoStoreTest.kt diff --git a/app/src/main/java/org/mozilla/tv/firefox/pocket/PocketVideoStore.kt b/app/src/main/java/org/mozilla/tv/firefox/pocket/PocketVideoStore.kt index 1b5fb53ce9..c24c7373d1 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/pocket/PocketVideoStore.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/pocket/PocketVideoStore.kt @@ -8,21 +8,16 @@ import android.content.Context import android.content.res.AssetManager import android.util.Log import androidx.annotation.AnyThread +import androidx.annotation.VisibleForTesting +import androidx.annotation.VisibleForTesting.PRIVATE import org.mozilla.tv.firefox.telemetry.SentryIntegration private const val LOGTAG = "PocketVideoStore" -private const val KEY_VIDEO_JSON = "video_json" private const val VIDEO_STORE_NAME = "Pocket-Global-Video-Recs" private const val BUNDLED_VIDEOS_PATH = "bundled/pocket_videos.json" -/** - * The minimum number of valid videos we must receive from the server if we want to display them. - * This number is set to the number of videos that appear on screen at the time of writing. - */ -private const val REQUIRED_POCKET_VIDEO_COUNT = 4 - /** * Saves the Pocket video recommendations as a raw JSON String and loads them in data structures for the app. * Bad data should not be saved so bad data should never be returned. @@ -32,7 +27,7 @@ private const val REQUIRED_POCKET_VIDEO_COUNT = 4 class PocketVideoStore( appContext: Context, private val assets: AssetManager, - private val convertJSONToPocketVideos: (String) -> List? + private val jsonValidator: PocketVideoJSONValidator ) { // We use SharedPrefs because it's simple, it handles concurrency (so we don't even need to think about @@ -44,7 +39,7 @@ class PocketVideoStore( */ @AnyThread fun save(json: String): Boolean { - if (!isJSONValid(json)) { + if (!jsonValidator.isJSONValidForSaving(json)) { return false } @@ -55,17 +50,6 @@ class PocketVideoStore( return true } - private fun isJSONValid(rawJSON: String): Boolean { - // While we don't need the conversion result, this function already handles validation so we use - // it to validate the videos. - val convertedVideos = convertJSONToPocketVideos(rawJSON) - return convertedVideos != null && - - // Guarantee a minimum number of Pocket videos: e.g. if the server only returns one valid video, - // we wouldn't want to overwrite what the user already has to show only one video. - convertedVideos.size >= REQUIRED_POCKET_VIDEO_COUNT - } - /** * @return the list of loaded videos. This should never happen but in case of error, the empty list is returned. */ @@ -75,7 +59,7 @@ class PocketVideoStore( val rawJSON = sharedPrefs.getString(KEY_VIDEO_JSON, null) ?: loadBundledTiles() - val convertedVideos = convertJSONToPocketVideos(rawJSON) + val convertedVideos = jsonValidator.convertJSONToPocketVideos(rawJSON) if (convertedVideos == null) { // We don't expect the conversion to ever fail: we only save valid JSON and we fallback to the presumably // valid bundled content if we've never saved. We don't crash because it may cause an infinite crash loop @@ -88,4 +72,38 @@ class PocketVideoStore( return convertedVideos } + + companion object { + @VisibleForTesting(otherwise = PRIVATE) const val KEY_VIDEO_JSON = "video_json" + } +} + +/** + * Validates video recommendation json from the Pocket server. + */ +class PocketVideoJSONValidator( + @Suppress("DEPRECATION") // We need PocketVideoParser until we move to a-c's impl. + private val pocketVideoParser: PocketVideoParser +) { + fun isJSONValidForSaving(rawJSON: String): Boolean { + // While we don't need the conversion result, this function already handles validation so we use + // it to validate the videos. + val convertedVideos = pocketVideoParser.convertVideosJSON(rawJSON) + return convertedVideos != null && + + // Guarantee a minimum number of Pocket videos: e.g. if the server only returns one valid video, + // we wouldn't want to overwrite what the user already has to show only one video. + convertedVideos.size >= REQUIRED_POCKET_VIDEO_COUNT + } + + // Using a function reference causes typing problems so we wrap this in a function call instead. + fun convertJSONToPocketVideos(json: String) = pocketVideoParser.convertVideosJSON(json) + + companion object { + /** + * The minimum number of valid videos we must receive from the server if we want to display them. + * This number is set to the number of videos that appear on screen at the time of writing. + */ + @VisibleForTesting(otherwise = PRIVATE) const val REQUIRED_POCKET_VIDEO_COUNT = 4 + } } diff --git a/app/src/main/java/org/mozilla/tv/firefox/utils/ServiceLocator.kt b/app/src/main/java/org/mozilla/tv/firefox/utils/ServiceLocator.kt index fbf399a9bb..f093721881 100644 --- a/app/src/main/java/org/mozilla/tv/firefox/utils/ServiceLocator.kt +++ b/app/src/main/java/org/mozilla/tv/firefox/utils/ServiceLocator.kt @@ -27,6 +27,7 @@ import org.mozilla.tv.firefox.channels.pinnedtile.PinnedTileImageUtilWrapper import org.mozilla.tv.firefox.channels.pinnedtile.PinnedTileRepo import org.mozilla.tv.firefox.pocket.PocketEndpointRaw import org.mozilla.tv.firefox.pocket.PocketVideoFetchScheduler +import org.mozilla.tv.firefox.pocket.PocketVideoJSONValidator import org.mozilla.tv.firefox.pocket.PocketVideoParser import org.mozilla.tv.firefox.pocket.PocketVideoRepo import org.mozilla.tv.firefox.pocket.PocketVideoStore @@ -73,6 +74,7 @@ open class ServiceLocator(val app: Application) { private val isPocketEnabledByLocale = { LocaleManager.getInstance().currentLanguageIsEnglish(app) } // Pocket is en-US only. private val bundleTileStore by lazy { BundleTilesStore(app) } private val pocketVideoParser by lazy { PocketVideoParser } + private val pocketVideoJSONValidator by lazy { PocketVideoJSONValidator(pocketVideoParser) } val intentLiveData by lazy { MutableLiveData>() } val fretboardProvider: FretboardProvider by lazy { FretboardProvider(app) } @@ -91,7 +93,7 @@ open class ServiceLocator(val app: Application) { val channelRepo by lazy { ChannelRepo(pinnedTileRepo) } @Suppress("DEPRECATION") // We need PocketEndpointRaw until we move to a-c's impl. val pocketEndpointRaw by lazy { PocketEndpointRaw(appVersion, buildConfigDerivables.globalPocketVideoEndpoint) } - val pocketVideoStore by lazy { PocketVideoStore(app, app.assets, pocketVideoParser::convertVideosJSON) } + val pocketVideoStore by lazy { PocketVideoStore(app, app.assets, pocketVideoJSONValidator) } val pocketVideoFetchScheduler by lazy { PocketVideoFetchScheduler(isPocketEnabledByLocale) } // These open vals are overridden in testing diff --git a/app/src/test/java/org/mozilla/tv/firefox/pocket/PocketVideoJSONValidatorTest.kt b/app/src/test/java/org/mozilla/tv/firefox/pocket/PocketVideoJSONValidatorTest.kt new file mode 100644 index 0000000000..d384077340 --- /dev/null +++ b/app/src/test/java/org/mozilla/tv/firefox/pocket/PocketVideoJSONValidatorTest.kt @@ -0,0 +1,71 @@ +/* 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.tv.firefox.pocket + +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.slot +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.mozilla.tv.firefox.helpers.PocketTestData + +class PocketVideoJSONValidatorTest { + + @Suppress("DEPRECATION") + @MockK private lateinit var pocketVideoParser: PocketVideoParser + + private lateinit var validator: PocketVideoJSONValidator + + @Before + fun setup() { + MockKAnnotations.init(this) + + validator = PocketVideoJSONValidator(pocketVideoParser) + } + + @Test + fun `WHEN validating for saving THEN the json argument is passed verbatim into the parser convert function`() { + val json = "{ }" + val slot = slot() + every { pocketVideoParser.convertVideosJSON(capture(slot)) } returns null + validator.isJSONValidForSaving(json) + + assertEquals(json, slot.captured) + } + + @Test + fun `WHEN converting json to pocket videos THEN it delegates to the pocket parser`() { + val json = "{ }" + val slot = slot() + every { pocketVideoParser.convertVideosJSON(capture(slot)) } returns null + validator.convertJSONToPocketVideos(json) + + assertEquals(json, slot.captured) + } + + @Test + fun `WHEN validating for saving and converted videos is null THEN return false`() { + every { pocketVideoParser.convertVideosJSON(any()) } returns null + assertFalse(validator.isJSONValidForSaving("{ }")) + } + + @Test + fun `WHEN validating for saving and converted videos size is equal to required video count THEN return true`() { + every { pocketVideoParser.convertVideosJSON(any()) } returns + PocketTestData.getVideoFeed(PocketVideoJSONValidator.REQUIRED_POCKET_VIDEO_COUNT) + assertTrue(validator.isJSONValidForSaving("{ }")) + } + + @Test + fun `WHEN validating for saving and converted videos size is less than required video count THEN return false`() { + every { pocketVideoParser.convertVideosJSON(any()) } returns + PocketTestData.getVideoFeed(PocketVideoJSONValidator.REQUIRED_POCKET_VIDEO_COUNT - 1) + assertFalse(validator.isJSONValidForSaving("{ }")) + } +} diff --git a/app/src/test/java/org/mozilla/tv/firefox/pocket/PocketVideoStoreTest.kt b/app/src/test/java/org/mozilla/tv/firefox/pocket/PocketVideoStoreTest.kt new file mode 100644 index 0000000000..47edd446aa --- /dev/null +++ b/app/src/test/java/org/mozilla/tv/firefox/pocket/PocketVideoStoreTest.kt @@ -0,0 +1,187 @@ +/* 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.tv.firefox.pocket + +import android.content.Context +import android.content.SharedPreferences +import android.content.res.AssetManager +import androidx.test.core.app.ApplicationProvider +import io.mockk.CapturingSlot +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.mockk +import io.mockk.slot +import io.mockk.spyk +import io.mockk.verify +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.tv.firefox.helpers.PocketTestData +import org.robolectric.RobolectricTestRunner +import java.io.ByteArrayInputStream +import java.io.IOException +import java.io.InputStream + +private val POCKET_FEED_TEST_DATA = PocketTestData.getVideoFeed(1) + +@RunWith(RobolectricTestRunner::class) +class PocketVideoStoreTest { + private lateinit var pocketVideoStore: PocketVideoStore + private lateinit var sharedPrefs: SharedPreferences + + @MockK private lateinit var context: Context + @MockK private lateinit var assetManager: AssetManager + @MockK private lateinit var jsonValidator: PocketVideoJSONValidator + + @Before + fun setup() { + MockKAnnotations.init(this) + + sharedPrefs = ApplicationProvider.getApplicationContext().getSharedPreferences("PocketFetch", 0) + every { context.getSharedPreferences(any(), any()) } returns sharedPrefs + + pocketVideoStore = PocketVideoStore(context, assetManager, jsonValidator) + } + + @Test + fun `WHEN the raw json is valid THEN it is saved in shared preferences`() { + every { jsonValidator.isJSONValidForSaving(any()) } returns true + val expected = "{ }" + + assertTrue(pocketVideoStore.save(expected)) + + val actual = sharedPrefs.getString("video_json", null) + + assertEquals(expected, actual) + } + + @Test + fun `WHEN the raw json is invalid THEN it is not saved in shared preferences`() { + every { jsonValidator.isJSONValidForSaving(any()) } returns false + + assertFalse(pocketVideoStore.save("{ }")) + } + + @Test + fun `WHEN saving THEN the json to save is passed verbatim into the json validator`() { + arrayOf("{ }", "", " ", "{ ").forEachIndexed { i, json -> + // The return value doesn't matter. + val validatorJSONCaptured = slot() + every { jsonValidator.isJSONValidForSaving(capture(validatorJSONCaptured)) } returns true + + pocketVideoStore.save(json) + + println("Input index $i: $json") + assertEquals(json, validatorJSONCaptured.captured) + } + } + + @Test + fun `GIVEN shared preferences is empty WHEN loading THEN the bundled tiles json is passed verbatim into the json validator`() { + val expectedBundledJSON = "assets" + everyAssetManagerOpensInputStream(expectedBundledJSON.toInputStream()) + + // The return value doesn't matter. + val validatorJSONCaptured = everyJSONValidatorConversionReturnsList(emptyList()) + + pocketVideoStore.load() + + assertEquals(expectedBundledJSON, validatorJSONCaptured.captured) + } + + // shared pref videos is copied into validator + @Test + fun `GIVEN shared preferences is not empty WHEN loading THEN the json from shared prefs is passed verbatim into the json validator`() { + everyAssetManagerOpensInputStream("assets".toInputStream()) + val expectedSharedPrefsValue = "{ }" + setJSONInSharedPrefs(expectedSharedPrefsValue) + + // The return value doesn't matter. + val validatorJSONCaptured = everyJSONValidatorConversionReturnsList(emptyList()) + + pocketVideoStore.load() + + assertEquals(expectedSharedPrefsValue, validatorJSONCaptured.captured) + } + + @Test + fun `GIVEN shared preferences is not empty WHEN loading THEN the converted videos are returned`() { + everyAssetManagerOpensInputStream("assets".toInputStream()) + setJSONInSharedPrefs("{ }") + everyJSONValidatorConversionReturnsList(POCKET_FEED_TEST_DATA) + + val returnVal = pocketVideoStore.load() + + assertEquals(POCKET_FEED_TEST_DATA, returnVal) + } + + @Test + fun `GIVEN shared preferences is empty WHEN the converted videos is null THEN loading returns an empty list`() { + everyAssetManagerOpensInputStream("".toInputStream()) + everyJSONValidatorConversionReturnsList(null) + val returnVal = pocketVideoStore.load() + assertEquals(emptyList(), returnVal) + } + + @Test + fun `GIVEN shared preferences is empty and conversion returns null WHEN loading THEN the input stream gets closed`() { + verifyLoadInputStreamIsClosed(given = { + everyJSONValidatorConversionReturnsList(null) + }) + } + + @Test + fun `GIVEN shared preferences is empty and conversion does not return null WHEN loading THEN the input stream gets closed`() { + verifyLoadInputStreamIsClosed(given = { + everyJSONValidatorConversionReturnsList(POCKET_FEED_TEST_DATA) + }) + } + + @Test(expected = IOException::class) + fun `GIVEN shared preferences is empty WHEN opening asset manager throws an exception THEN load throws the exception`() { + every { assetManager.open(any()) } throws IOException() + pocketVideoStore.load() + } + + @Test(expected = IOException::class) + fun `GIVEN shared preferences is empty WHEN read text throws an exception THEN load throws the exception`() { + val inputStream = mockk().also { + every { it.read(any(), any(), any()) } throws IOException() + } + + everyAssetManagerOpensInputStream(inputStream) + pocketVideoStore.load() + } + + private fun verifyLoadInputStreamIsClosed(given: (InputStream) -> Unit) { + val inputStream = spyk("assets".toInputStream()) + everyAssetManagerOpensInputStream(inputStream) + given(inputStream) + + pocketVideoStore.load() + + verify { inputStream.close() } + } + + private fun setJSONInSharedPrefs(json: String) { + sharedPrefs.edit().putString(PocketVideoStore.KEY_VIDEO_JSON, json).apply() + } + + private fun everyAssetManagerOpensInputStream(inputStream: InputStream) { + every { assetManager.open(any()) } returns inputStream + } + + private fun everyJSONValidatorConversionReturnsList(pocketVideoList: List?): CapturingSlot { + val validatorJSONCaptured = slot() + every { jsonValidator.convertJSONToPocketVideos(capture(validatorJSONCaptured)) } returns pocketVideoList + return validatorJSONCaptured + } + + private fun String.toInputStream() = ByteArrayInputStream(this.toByteArray()) +}