From be10d427e83037ab70ebbc88134039f0cd759835 Mon Sep 17 00:00:00 2001 From: Emily Kager Date: Thu, 18 Jul 2019 16:07:48 -0700 Subject: [PATCH] For #4127 - Converts Exceptions to LibState and adds tests --- .../java/org/mozilla/fenix/HomeActivity.kt | 6 ++ .../fenix/exceptions/ExceptionsAdapter.kt | 15 +++- .../fenix/exceptions/ExceptionsComponent.kt | 62 ------------- .../fenix/exceptions/ExceptionsFragment.kt | 90 ++++++++++--------- .../fenix/exceptions/ExceptionsInteractor.kt | 27 ++++++ .../fenix/exceptions/ExceptionsStore.kt | 43 +++++++++ ...{ExceptionsUIView.kt => ExceptionsView.kt} | 61 ++++++++----- .../ExceptionsDeleteButtonViewHolder.kt | 7 +- .../ExceptionsListItemViewHolder.kt | 7 +- .../exceptions/ExceptionsInteractorTest.kt | 49 ++++++++++ .../fenix/exceptions/ExceptionsStoreTest.kt | 30 +++++++ 11 files changed, 262 insertions(+), 135 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsComponent.kt create mode 100644 app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsInteractor.kt create mode 100644 app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsStore.kt rename app/src/main/java/org/mozilla/fenix/exceptions/{ExceptionsUIView.kt => ExceptionsView.kt} (59%) create mode 100644 app/src/test/java/org/mozilla/fenix/exceptions/ExceptionsInteractorTest.kt create mode 100644 app/src/test/java/org/mozilla/fenix/exceptions/ExceptionsStoreTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index b4ad5ee49..2eef66eb0 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -258,6 +258,12 @@ open class HomeActivity : AppCompatActivity(), ShareFragment.TabsSharedCallback customTabSessionId ) } + BrowserDirection.FromExceptions -> { + fragmentId = R.id.exceptionsFragment + ExceptionsFragmentDirections.actionExceptionsFragmentToBrowserFragment( + customTabSessionId + ) + } } } else { null diff --git a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsAdapter.kt b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsAdapter.kt index 310c620e2..3b3a0ffb5 100644 --- a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsAdapter.kt @@ -7,7 +7,6 @@ package org.mozilla.fenix.exceptions import android.view.LayoutInflater import android.view.ViewGroup import androidx.recyclerview.widget.RecyclerView -import io.reactivex.Observer import org.mozilla.fenix.exceptions.viewholders.ExceptionsDeleteButtonViewHolder import org.mozilla.fenix.exceptions.viewholders.ExceptionsHeaderViewHolder import org.mozilla.fenix.exceptions.viewholders.ExceptionsListItemViewHolder @@ -21,6 +20,7 @@ private sealed class AdapterItem { private class ExceptionsList(val exceptions: List) { val items: List + init { val items = mutableListOf() items.add(AdapterItem.Header) @@ -33,7 +33,7 @@ private class ExceptionsList(val exceptions: List) { } class ExceptionsAdapter( - private val actionEmitter: Observer + private val interactor: ExceptionsInteractor ) : AdapterWithJob() { private var exceptionsList: ExceptionsList = ExceptionsList(emptyList()) @@ -56,9 +56,16 @@ class ExceptionsAdapter( val view = LayoutInflater.from(parent.context).inflate(viewType, parent, false) return when (viewType) { - ExceptionsDeleteButtonViewHolder.LAYOUT_ID -> ExceptionsDeleteButtonViewHolder(view, actionEmitter) + ExceptionsDeleteButtonViewHolder.LAYOUT_ID -> ExceptionsDeleteButtonViewHolder( + view, + interactor + ) ExceptionsHeaderViewHolder.LAYOUT_ID -> ExceptionsHeaderViewHolder(view) - ExceptionsListItemViewHolder.LAYOUT_ID -> ExceptionsListItemViewHolder(view, actionEmitter, adapterJob) + ExceptionsListItemViewHolder.LAYOUT_ID -> ExceptionsListItemViewHolder( + view, + interactor, + adapterJob + ) else -> throw IllegalStateException() } } diff --git a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsComponent.kt b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsComponent.kt deleted file mode 100644 index 87fe487fc..000000000 --- a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsComponent.kt +++ /dev/null @@ -1,62 +0,0 @@ -/* 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.fenix.exceptions - -import android.view.ViewGroup -import org.mozilla.fenix.mvi.Action -import org.mozilla.fenix.mvi.ActionBusFactory -import org.mozilla.fenix.mvi.Change -import org.mozilla.fenix.mvi.UIComponent -import org.mozilla.fenix.mvi.UIComponentViewModelBase -import org.mozilla.fenix.mvi.UIComponentViewModelProvider -import org.mozilla.fenix.mvi.ViewState -import org.mozilla.fenix.test.Mockable - -data class ExceptionsItem(val url: String) - -@Mockable -class ExceptionsComponent( - private val container: ViewGroup, - bus: ActionBusFactory, - viewModelProvider: UIComponentViewModelProvider -) : - UIComponent( - bus.getManagedEmitter(ExceptionsAction::class.java), - bus.getSafeManagedObservable(ExceptionsChange::class.java), - viewModelProvider - ) { - - override fun initView() = ExceptionsUIView(container, actionEmitter, changesObservable) - - init { - bind() - } -} - -data class ExceptionsState(val items: List) : ViewState - -sealed class ExceptionsAction : Action { - object LearnMore : ExceptionsAction() - sealed class Delete : ExceptionsAction() { - object All : Delete() - data class One(val item: ExceptionsItem) : Delete() - } -} - -sealed class ExceptionsChange : Change { - data class Change(val list: List) : ExceptionsChange() -} - -class ExceptionsViewModel( - initialState: ExceptionsState -) : UIComponentViewModelBase(initialState, reducer) { - companion object { - val reducer: (ExceptionsState, ExceptionsChange) -> ExceptionsState = { state, change -> - when (change) { - is ExceptionsChange.Change -> state.copy(items = change.list) - } - } - } -} diff --git a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsFragment.kt b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsFragment.kt index 76c8dbe18..4efb6b48b 100644 --- a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsFragment.kt @@ -11,23 +11,24 @@ import android.view.ViewGroup import androidx.appcompat.app.AppCompatActivity import androidx.fragment.app.Fragment import androidx.lifecycle.lifecycleScope +import androidx.lifecycle.whenStarted import androidx.navigation.Navigation import kotlinx.android.synthetic.main.fragment_exceptions.view.* import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.Dispatchers.Main import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch +import mozilla.components.lib.state.ext.observe import org.mozilla.fenix.BrowserDirection -import org.mozilla.fenix.FenixViewModelProvider import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R -import org.mozilla.fenix.mvi.ActionBusFactory -import org.mozilla.fenix.mvi.getAutoDisposeObservable -import org.mozilla.fenix.mvi.getManagedEmitter +import org.mozilla.fenix.components.StoreProvider import org.mozilla.fenix.settings.SupportUtils class ExceptionsFragment : Fragment() { - private lateinit var exceptionsComponent: ExceptionsComponent + private lateinit var exceptionsStore: ExceptionsStore + private lateinit var exceptionsView: ExceptionsView + private lateinit var exceptionsInteractor: ExceptionsInteractor override fun onResume() { super.onResume() @@ -41,45 +42,54 @@ class ExceptionsFragment : Fragment() { savedInstanceState: Bundle? ): View? { val view = inflater.inflate(R.layout.fragment_exceptions, container, false) - exceptionsComponent = ExceptionsComponent( - view.exceptions_layout, - ActionBusFactory.get(this), - FenixViewModelProvider.create( - this, - ExceptionsViewModel::class.java - ) { - ExceptionsViewModel(ExceptionsState(loadAndMapExceptions())) - } - ) + exceptionsStore = StoreProvider.get(this) { + ExceptionsStore( + ExceptionsState( + items = loadAndMapExceptions() + ) + ) + } + exceptionsInteractor = + ExceptionsInteractor(::openLearnMore, ::deleteOneItem, ::deleteAllItems) + exceptionsView = ExceptionsView(view.exceptions_layout, exceptionsInteractor) return view } - override fun onStart() { - super.onStart() - getAutoDisposeObservable() - .subscribe { - when (it) { - is ExceptionsAction.LearnMore -> { - (activity as HomeActivity).openToBrowserAndLoad( - searchTermOrURL = SupportUtils.getGenericSumoURLForTopic - (SupportUtils.SumoTopic.TRACKING_PROTECTION), - newTab = true, - from = BrowserDirection.FromExceptions - ) - } - is ExceptionsAction.Delete.All -> viewLifecycleOwner.lifecycleScope.launch(IO) { - val domains = ExceptionDomains.load(context!!) - ExceptionDomains.remove(context!!, domains) - launch(Main) { - view?.let { view -> Navigation.findNavController(view).navigateUp() } - } - } - is ExceptionsAction.Delete.One -> viewLifecycleOwner.lifecycleScope.launch(IO) { - ExceptionDomains.remove(context!!, listOf(it.item.url)) - reloadData() - } + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + exceptionsStore.observe(view) { + viewLifecycleOwner.lifecycleScope.launch { + whenStarted { + exceptionsView.update(it) } } + } + } + + private fun deleteAllItems() { + viewLifecycleOwner.lifecycleScope.launch(IO) { + val domains = ExceptionDomains.load(context!!) + ExceptionDomains.remove(context!!, domains) + launch(Main) { + view?.let { view -> Navigation.findNavController(view).navigateUp() } + } + } + } + + private fun deleteOneItem(item: ExceptionsItem) { + viewLifecycleOwner.lifecycleScope.launch(IO) { + ExceptionDomains.remove(context!!, listOf(item.url)) + reloadData() + } + } + + private fun openLearnMore() { + (activity as HomeActivity).openToBrowserAndLoad( + searchTermOrURL = SupportUtils.getGenericSumoURLForTopic + (SupportUtils.SumoTopic.TRACKING_PROTECTION), + newTab = true, + from = BrowserDirection.FromExceptions + ) } private fun loadAndMapExceptions(): List { @@ -100,7 +110,7 @@ class ExceptionsFragment : Fragment() { view?.let { view: View -> Navigation.findNavController(view).navigateUp() } return@launch } - getManagedEmitter().onNext(ExceptionsChange.Change(items)) + exceptionsStore.dispatch(ExceptionsAction.Change(items)) } } } diff --git a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsInteractor.kt b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsInteractor.kt new file mode 100644 index 000000000..1f02c3c5b --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsInteractor.kt @@ -0,0 +1,27 @@ +/* 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.fenix.exceptions + +/** + * Interactor for the exceptions screen + * Provides implementations for the ExceptionsViewInteractor + */ +class ExceptionsInteractor( + private val learnMore: () -> Unit, + private val deleteOne: (ExceptionsItem) -> Unit, + private val deleteAll: () -> Unit +) : ExceptionsViewInteractor { + override fun onLearnMore() { + learnMore.invoke() + } + + override fun onDeleteAll() { + deleteAll.invoke() + } + + override fun onDeleteOne(item: ExceptionsItem) { + deleteOne.invoke(item) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsStore.kt b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsStore.kt new file mode 100644 index 000000000..b77137e25 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsStore.kt @@ -0,0 +1,43 @@ +/* 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.fenix.exceptions + +import mozilla.components.lib.state.Action +import mozilla.components.lib.state.State +import mozilla.components.lib.state.Store + +/** + * Class representing an exception item + * @property url Host of the exception + */ +data class ExceptionsItem(val url: String) + +/** + * The [Store] for holding the [ExceptionsState] and applying [ExceptionsAction]s. + */ +class ExceptionsStore(initialState: ExceptionsState) : + Store(initialState, ::exceptionsStateReducer) + +/** + * Actions to dispatch through the `ExceptionsStore` to modify `ExceptionsState` through the reducer. + */ +sealed class ExceptionsAction : Action { + data class Change(val list: List) : ExceptionsAction() +} + +/** + * The state for the Exceptions Screen + * @property items List of exceptions to display + */ +data class ExceptionsState(val items: List) : State + +/** + * The ExceptionsState Reducer. + */ +fun exceptionsStateReducer(state: ExceptionsState, action: ExceptionsAction): ExceptionsState { + return when (action) { + is ExceptionsAction.Change -> state.copy(items = action.list) + } +} diff --git a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsUIView.kt b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsView.kt similarity index 59% rename from app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsUIView.kt rename to app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsView.kt index 557022414..80bcd3237 100644 --- a/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsUIView.kt +++ b/app/src/main/java/org/mozilla/fenix/exceptions/ExceptionsView.kt @@ -13,31 +13,49 @@ import android.view.View import android.view.ViewGroup import android.widget.FrameLayout import androidx.recyclerview.widget.LinearLayoutManager -import io.reactivex.Observable -import io.reactivex.Observer -import io.reactivex.functions.Consumer +import kotlinx.android.extensions.LayoutContainer import kotlinx.android.synthetic.main.component_exceptions.view.* import org.mozilla.fenix.R -import org.mozilla.fenix.mvi.UIView -class ExceptionsUIView( - container: ViewGroup, - actionEmitter: Observer, - changesObservable: Observable -) : - UIView( - container, - actionEmitter, - changesObservable - ) { +/** + * Interface for the ExceptionsViewInteractor. This interface is implemented by objects that want + * to respond to user interaction on the ExceptionsView + */ +interface ExceptionsViewInteractor { + /** + * Called whenever learn more about tracking protection is tapped + */ + fun onLearnMore() - override val view: FrameLayout = LayoutInflater.from(container.context) + /** + * Called whenever all exception items are deleted + */ + fun onDeleteAll() + + /** + * Called whenever one exception item is deleted + */ + fun onDeleteOne(item: ExceptionsItem) +} + +/** + * View that contains and configures the Exceptions List + */ +class ExceptionsView( + private val container: ViewGroup, + val interactor: ExceptionsInteractor +) : LayoutContainer { + + val view: FrameLayout = LayoutInflater.from(container.context) .inflate(R.layout.component_exceptions, container, true) .findViewById(R.id.exceptions_wrapper) + override val containerView: View? + get() = container + init { view.exceptions_list.apply { - adapter = ExceptionsAdapter(actionEmitter) + adapter = ExceptionsAdapter(interactor) layoutManager = LinearLayoutManager(container.context) } val descriptionText = String @@ -48,7 +66,7 @@ class ExceptionsUIView( val linkStartIndex = descriptionText.indexOf("\n\n") + 2 val linkAction = object : ClickableSpan() { override fun onClick(widget: View?) { - actionEmitter.onNext(ExceptionsAction.LearnMore) + interactor.onLearnMore() } } val textWithLink = SpannableString(descriptionText).apply { @@ -61,9 +79,10 @@ class ExceptionsUIView( view.exceptions_empty_view.text = textWithLink } - override fun updateView() = Consumer { - view.exceptions_empty_view.visibility = if (it.items.isEmpty()) View.VISIBLE else View.GONE - view.exceptions_list.visibility = if (it.items.isEmpty()) View.GONE else View.VISIBLE - (view.exceptions_list.adapter as ExceptionsAdapter).updateData(it.items) + fun update(state: ExceptionsState) { + view.exceptions_empty_view.visibility = + if (state.items.isEmpty()) View.VISIBLE else View.GONE + view.exceptions_list.visibility = if (state.items.isEmpty()) View.GONE else View.VISIBLE + (view.exceptions_list.adapter as ExceptionsAdapter).updateData(state.items) } } diff --git a/app/src/main/java/org/mozilla/fenix/exceptions/viewholders/ExceptionsDeleteButtonViewHolder.kt b/app/src/main/java/org/mozilla/fenix/exceptions/viewholders/ExceptionsDeleteButtonViewHolder.kt index 756c20f5a..9a0ed172c 100644 --- a/app/src/main/java/org/mozilla/fenix/exceptions/viewholders/ExceptionsDeleteButtonViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/exceptions/viewholders/ExceptionsDeleteButtonViewHolder.kt @@ -6,20 +6,19 @@ package org.mozilla.fenix.exceptions.viewholders import android.view.View import androidx.recyclerview.widget.RecyclerView -import io.reactivex.Observer import kotlinx.android.synthetic.main.delete_exceptions_button.view.* import org.mozilla.fenix.R -import org.mozilla.fenix.exceptions.ExceptionsAction +import org.mozilla.fenix.exceptions.ExceptionsInteractor class ExceptionsDeleteButtonViewHolder( view: View, - private val actionEmitter: Observer + private val interactor: ExceptionsInteractor ) : RecyclerView.ViewHolder(view) { private val deleteButton = view.removeAllExceptions init { deleteButton.setOnClickListener { - actionEmitter.onNext(ExceptionsAction.Delete.All) + interactor.onDeleteAll() } } diff --git a/app/src/main/java/org/mozilla/fenix/exceptions/viewholders/ExceptionsListItemViewHolder.kt b/app/src/main/java/org/mozilla/fenix/exceptions/viewholders/ExceptionsListItemViewHolder.kt index b2463926b..60bdf564b 100644 --- a/app/src/main/java/org/mozilla/fenix/exceptions/viewholders/ExceptionsListItemViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/exceptions/viewholders/ExceptionsListItemViewHolder.kt @@ -6,7 +6,6 @@ package org.mozilla.fenix.exceptions.viewholders import android.view.View import androidx.recyclerview.widget.RecyclerView -import io.reactivex.Observer import kotlinx.android.synthetic.main.exception_item.view.* import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -14,14 +13,14 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.launch import mozilla.components.browser.icons.IconRequest import org.mozilla.fenix.R -import org.mozilla.fenix.exceptions.ExceptionsAction +import org.mozilla.fenix.exceptions.ExceptionsInteractor import org.mozilla.fenix.exceptions.ExceptionsItem import org.mozilla.fenix.ext.components import kotlin.coroutines.CoroutineContext class ExceptionsListItemViewHolder( view: View, - private val actionEmitter: Observer, + private val interactor: ExceptionsInteractor, val job: Job ) : RecyclerView.ViewHolder(view), CoroutineScope { @@ -37,7 +36,7 @@ class ExceptionsListItemViewHolder( init { deleteButton.setOnClickListener { item?.let { - actionEmitter.onNext(ExceptionsAction.Delete.One(it)) + interactor.onDeleteOne(it) } } } diff --git a/app/src/test/java/org/mozilla/fenix/exceptions/ExceptionsInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/exceptions/ExceptionsInteractorTest.kt new file mode 100644 index 000000000..7f03f94bb --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/exceptions/ExceptionsInteractorTest.kt @@ -0,0 +1,49 @@ +/* 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.fenix.exceptions + +import io.mockk.mockk +import org.junit.Assert.assertEquals +import org.junit.Test + +class ExceptionsInteractorTest { + + @Test + fun onLearnMore() { + var learnMoreClicked = false + val interactor = ExceptionsInteractor( + { learnMoreClicked = true }, + mockk(), + mockk() + ) + interactor.onLearnMore() + assertEquals(true, learnMoreClicked) + } + + @Test + fun onDeleteAll() { + var onDeleteAll = false + val interactor = ExceptionsInteractor( + mockk(), + mockk(), + { onDeleteAll = true } + ) + interactor.onDeleteAll() + assertEquals(true, onDeleteAll) + } + + @Test + fun onDeleteOne() { + var exceptionsItemReceived: ExceptionsItem? = null + val exceptionsItem = ExceptionsItem("url") + val interactor = ExceptionsInteractor( + mockk(), + { exceptionsItemReceived = exceptionsItem }, + mockk() + ) + interactor.onDeleteOne(exceptionsItem) + assertEquals(exceptionsItemReceived, exceptionsItem) + } +} \ No newline at end of file diff --git a/app/src/test/java/org/mozilla/fenix/exceptions/ExceptionsStoreTest.kt b/app/src/test/java/org/mozilla/fenix/exceptions/ExceptionsStoreTest.kt new file mode 100644 index 000000000..058f38cea --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/exceptions/ExceptionsStoreTest.kt @@ -0,0 +1,30 @@ +/* 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.fenix.exceptions + +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotSame +import org.junit.Test + +class ExceptionsStoreTest { + @Test + fun onChange() = runBlocking { + val initialState = emptyDefaultState() + val store = ExceptionsStore(initialState) + val newExceptionsItem = ExceptionsItem("URL") + + store.dispatch(ExceptionsAction.Change(listOf(newExceptionsItem))).join() + assertNotSame(initialState, store.state) + assertEquals( + store.state.items, + listOf(newExceptionsItem) + ) + } + + private fun emptyDefaultState(): ExceptionsState = ExceptionsState( + items = listOf() + ) +}