From 0120558fce642093343fa5f1abf6a04ba1f46b6c Mon Sep 17 00:00:00 2001 From: Colin Lee Date: Thu, 31 Jan 2019 00:49:41 -0600 Subject: [PATCH] Enforce unidirectional arch better --- .../java/org/mozilla/fenix/components/Core.kt | 2 -- .../fenix/home/sessions/SessionsComponent.kt | 21 ++++---------- .../fenix/home/sessions/SessionsUIView.kt | 11 ++++++-- .../org/mozilla/fenix/mvi/ActionBusFactory.kt | 8 ++++++ .../java/org/mozilla/fenix/mvi/UIComponent.kt | 21 +++++++------- .../main/java/org/mozilla/fenix/mvi/UIView.kt | 7 +++-- .../mozilla/fenix/search/SearchFragment.kt | 28 +++++++++---------- .../search/awesomebar/AwesomeBarComponent.kt | 28 +++++++++++-------- .../search/awesomebar/AwesomeBarUIView.kt | 18 ++++++++---- .../fenix/search/toolbar/ToolbarComponent.kt | 18 ++++++------ .../fenix/search/toolbar/ToolbarUIView.kt | 18 +++++++----- 11 files changed, 100 insertions(+), 80 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/Core.kt b/app/src/main/java/org/mozilla/fenix/components/Core.kt index 3ec5a0250..de81ba774 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Core.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Core.kt @@ -8,7 +8,6 @@ import android.content.Context import android.content.SharedPreferences import android.preference.PreferenceManager import mozilla.components.browser.engine.gecko.GeckoEngine -import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager import mozilla.components.browser.session.storage.SessionStorage import mozilla.components.browser.storage.sync.PlacesHistoryStorage @@ -19,7 +18,6 @@ import mozilla.components.feature.session.HistoryDelegate import mozilla.components.lib.crash.handler.CrashHandlerService import org.mozilla.geckoview.GeckoRuntime import org.mozilla.geckoview.GeckoRuntimeSettings -import java.util.concurrent.TimeUnit /** * Component group for all core browser functionality. diff --git a/app/src/main/java/org/mozilla/fenix/home/sessions/SessionsComponent.kt b/app/src/main/java/org/mozilla/fenix/home/sessions/SessionsComponent.kt index 6e1adbceb..a8eac848c 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessions/SessionsComponent.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessions/SessionsComponent.kt @@ -4,10 +4,8 @@ package org.mozilla.fenix.home.sessions -import android.annotation.SuppressLint import android.view.ViewGroup import mozilla.components.browser.session.Session -import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.mvi.Action import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.Change @@ -16,10 +14,13 @@ import org.mozilla.fenix.mvi.ViewState class SessionsComponent( private val container: ViewGroup, - override val bus: ActionBusFactory, + bus: ActionBusFactory, override var initialState: SessionsState = SessionsState(emptyList()) ) : - UIComponent(bus) { + UIComponent( + bus.getManagedEmitter(SessionsAction::class.java), + bus.getSafeManagedObservable(SessionsChange::class.java) + ) { override val reducer: (SessionsState, SessionsChange) -> SessionsState = { state, change -> when (change) { @@ -27,20 +28,10 @@ class SessionsComponent( } } - override fun initView() = SessionsUIView(container, bus) + override fun initView() = SessionsUIView(container, actionEmitter, changesObservable) init { - setup() - } - - @SuppressLint("CheckResult") - fun setup(): SessionsComponent { render(reducer) - getUserInteractionEvents() - .subscribe { - Logger("SessionsComponent").debug(it.toString()) - } - return this } } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessions/SessionsUIView.kt b/app/src/main/java/org/mozilla/fenix/home/sessions/SessionsUIView.kt index 95d201431..37d752bae 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessions/SessionsUIView.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessions/SessionsUIView.kt @@ -8,13 +8,18 @@ import android.view.LayoutInflater import android.view.ViewGroup import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView +import io.reactivex.Observable +import io.reactivex.Observer import io.reactivex.functions.Consumer import org.mozilla.fenix.R -import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.UIView -class SessionsUIView(container: ViewGroup, bus: ActionBusFactory) : - UIView(container, bus) { +class SessionsUIView( + container: ViewGroup, + actionEmitter: Observer, + changesObservable: Observable +) : + UIView(container, actionEmitter, changesObservable) { override val view: RecyclerView = LayoutInflater.from(container.context) .inflate(R.layout.component_sessions, container, true) diff --git a/app/src/main/java/org/mozilla/fenix/mvi/ActionBusFactory.kt b/app/src/main/java/org/mozilla/fenix/mvi/ActionBusFactory.kt index c1c6f6fc0..a2c9dbbdc 100644 --- a/app/src/main/java/org/mozilla/fenix/mvi/ActionBusFactory.kt +++ b/app/src/main/java/org/mozilla/fenix/mvi/ActionBusFactory.kt @@ -27,6 +27,7 @@ import androidx.lifecycle.LifecycleObserver import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.OnLifecycleEvent import io.reactivex.Observable +import io.reactivex.Observer import io.reactivex.rxkotlin.merge import io.reactivex.subjects.PublishSubject import io.reactivex.subjects.Subject @@ -102,6 +103,10 @@ class ActionBusFactory private constructor(val owner: LifecycleOwner) { return if (map[clazz] != null) map[clazz] as Observable else create(clazz) } + fun getManagedEmitter(clazz: Class): Observer { + return if (map[clazz] != null) map[clazz] as Observer else create(clazz) + } + fun logMergedObservables() { // TODO make this observe new items in the map and combine them map.values.merge().compose(logState()).subscribe() @@ -131,6 +136,9 @@ inline fun LifecycleOwner.emit(event: T) = inline fun LifecycleOwner.getSafeManagedObservable(): Observable = ActionBusFactory.get(this).getSafeManagedObservable(T::class.java) +inline fun LifecycleOwner.getManagedEmitter(): Observer = + ActionBusFactory.get(this).getManagedEmitter(T::class.java) + /** * This method returns a destroy observable that can be passed to [org.mozilla.fenix.mvi.UIView]s as needed. * This is deliberately scoped to the attached [LifecycleOwner]'s [Lifecycle.Event.ON_DESTROY] diff --git a/app/src/main/java/org/mozilla/fenix/mvi/UIComponent.kt b/app/src/main/java/org/mozilla/fenix/mvi/UIComponent.kt index 4e43a9575..1ce08dba8 100644 --- a/app/src/main/java/org/mozilla/fenix/mvi/UIComponent.kt +++ b/app/src/main/java/org/mozilla/fenix/mvi/UIComponent.kt @@ -5,29 +5,28 @@ package org.mozilla.fenix.mvi import io.reactivex.Observable +import io.reactivex.Observer import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable import io.reactivex.schedulers.Schedulers -abstract class UIComponent(open val bus: ActionBusFactory) { +abstract class UIComponent( + protected val actionEmitter: Observer, + protected val changesObservable: Observable +) { + abstract var initialState: S abstract val reducer: Reducer - val uiView: UIView by lazy { initView() } + val uiView: UIView by lazy { initView() } - abstract fun initView(): UIView + abstract fun initView(): UIView open fun getContainerId() = uiView.containerId - inline fun getUserInteractionEvents(): - Observable = bus.getSafeManagedObservable(A::class.java) - - inline fun getModelChangeEvents(): - Observable = bus.getSafeManagedObservable(C::class.java) - /** * Render the ViewState to the View through the Reducer */ - inline fun render(noinline reducer: Reducer): Disposable = - getModelChangeEvents() + fun render(reducer: Reducer): Disposable = + changesObservable .scan(initialState, reducer) .distinctUntilChanged() .subscribeOn(Schedulers.io()) diff --git a/app/src/main/java/org/mozilla/fenix/mvi/UIView.kt b/app/src/main/java/org/mozilla/fenix/mvi/UIView.kt index 1e73e21b9..f319ad511 100644 --- a/app/src/main/java/org/mozilla/fenix/mvi/UIView.kt +++ b/app/src/main/java/org/mozilla/fenix/mvi/UIView.kt @@ -7,12 +7,15 @@ package org.mozilla.fenix.mvi import android.view.View import android.view.ViewGroup import androidx.annotation.IdRes +import io.reactivex.Observable +import io.reactivex.Observer import io.reactivex.functions.Consumer import kotlinx.android.extensions.LayoutContainer -abstract class UIView( +abstract class UIView( private val container: ViewGroup, - val bus: ActionBusFactory + protected val actionEmitter: Observer, + protected val changesObservable: Observable ) : LayoutContainer { abstract val view: View diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt index 2fc2ca54d..f5ade7c90 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.search +import android.annotation.SuppressLint import android.os.Bundle import android.view.LayoutInflater import android.view.View @@ -13,10 +14,13 @@ import androidx.navigation.Navigation import kotlinx.android.synthetic.main.fragment_search.view.* import org.mozilla.fenix.R import org.mozilla.fenix.mvi.ActionBusFactory +import org.mozilla.fenix.mvi.getSafeManagedObservable import org.mozilla.fenix.search.awesomebar.AwesomeBarAction import org.mozilla.fenix.search.awesomebar.AwesomeBarChange import org.mozilla.fenix.search.awesomebar.AwesomeBarComponent -import org.mozilla.fenix.search.toolbar.* +import org.mozilla.fenix.search.toolbar.SearchAction +import org.mozilla.fenix.search.toolbar.ToolbarComponent +import org.mozilla.fenix.search.toolbar.ToolbarUIView class SearchFragment : Fragment() { private lateinit var toolbarComponent: ToolbarComponent @@ -30,6 +34,7 @@ class SearchFragment : Fragment() { val view = inflater.inflate(R.layout.fragment_search, container, false) toolbarComponent = ToolbarComponent(view.toolbar_wrapper, ActionBusFactory.get(this)) awesomeBarComponent = AwesomeBarComponent(view.search_layout, ActionBusFactory.get(this)) + ActionBusFactory.get(this).logMergedObservables() return view } @@ -38,6 +43,7 @@ class SearchFragment : Fragment() { toolbarComponent.editMode() } + @SuppressLint("CheckResult") override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) @@ -47,26 +53,18 @@ class SearchFragment : Fragment() { view.toolbar_wrapper.clipToOutline = false - toolbarComponent - .getModelChangeEvents() + getSafeManagedObservable() .subscribe { when (it) { - is SearchChange.QueryChanged -> { - ActionBusFactory.get(this).emit(AwesomeBarChange::class.java, AwesomeBarChange.UpdateQuery(it.query)) + is SearchAction.UrlCommitted -> transitionToBrowser() + is SearchAction.TextChanged -> { + ActionBusFactory.get(this) + .emit(AwesomeBarChange::class.java, AwesomeBarChange.UpdateQuery(it.query)) } } } - toolbarComponent - .getUserInteractionEvents() - .subscribe { - when (it) { - is SearchAction.UrlCommitted -> transitionToBrowser() - } - } - - awesomeBarComponent - .getUserInteractionEvents() + getSafeManagedObservable() .subscribe { when (it) { is AwesomeBarAction.ItemSelected -> transitionToBrowser() diff --git a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarComponent.kt b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarComponent.kt index 8ee88af32..fd86be5be 100644 --- a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarComponent.kt +++ b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarComponent.kt @@ -4,32 +4,38 @@ package org.mozilla.fenix.search.awesomebar file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import android.view.ViewGroup -import org.mozilla.fenix.mvi.* +import org.mozilla.fenix.mvi.Action +import org.mozilla.fenix.mvi.ActionBusFactory +import org.mozilla.fenix.mvi.Change +import org.mozilla.fenix.mvi.Reducer +import org.mozilla.fenix.mvi.UIComponent +import org.mozilla.fenix.mvi.ViewState -data class AwesomeBarState(val query: String) : ViewState { - fun updateQuery(query: String) = AwesomeBarState(query) -} +data class AwesomeBarState(val query: String) : ViewState -sealed class AwesomeBarAction: Action { - object ItemSelected: AwesomeBarAction() +sealed class AwesomeBarAction : Action { + object ItemSelected : AwesomeBarAction() } sealed class AwesomeBarChange : Change { - data class UpdateQuery(val query: String): AwesomeBarChange() + data class UpdateQuery(val query: String) : AwesomeBarChange() } class AwesomeBarComponent( private val container: ViewGroup, - override val bus: ActionBusFactory, + bus: ActionBusFactory, override var initialState: AwesomeBarState = AwesomeBarState("") -) : UIComponent(bus) { +) : UIComponent( + bus.getManagedEmitter(AwesomeBarAction::class.java), + bus.getSafeManagedObservable(AwesomeBarChange::class.java) +) { override val reducer: Reducer = { state, change -> when (change) { - is AwesomeBarChange.UpdateQuery -> state.updateQuery(change.query) + is AwesomeBarChange.UpdateQuery -> state.copy(query = change.query) } } - override fun initView() = AwesomeBarUIView(container, bus) + override fun initView() = AwesomeBarUIView(container, actionEmitter, changesObservable) init { render(reducer) diff --git a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt index b290fcbb4..270c9db57 100644 --- a/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt +++ b/app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt @@ -5,6 +5,8 @@ package org.mozilla.fenix.search.awesomebar import android.view.LayoutInflater import android.view.ViewGroup +import io.reactivex.Observable +import io.reactivex.Observer import io.reactivex.functions.Consumer import mozilla.components.browser.awesomebar.BrowserAwesomeBar import mozilla.components.feature.awesomebar.provider.ClipboardSuggestionProvider @@ -13,11 +15,14 @@ import mozilla.components.feature.awesomebar.provider.SessionSuggestionProvider import mozilla.components.support.ktx.android.graphics.drawable.toBitmap import org.mozilla.fenix.R import org.mozilla.fenix.ext.components -import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.UIView -class AwesomeBarUIView(container: ViewGroup, bus: ActionBusFactory) : - UIView(container, bus) { +class AwesomeBarUIView( + container: ViewGroup, + actionEmitter: Observer, + changesObservable: Observable +) : + UIView(container, actionEmitter, changesObservable) { override val view: BrowserAwesomeBar = LayoutInflater.from(container.context) .inflate(R.layout.component_awesomebar, container, true) .findViewById(R.id.awesomeBar) @@ -31,18 +36,19 @@ class AwesomeBarUIView(container: ViewGroup, bus: ActionBusFactory) : getString(R.string.awesomebar_clipboard_title) ) ) - view.addProviders(SessionSuggestionProvider(components.core.sessionManager, components.useCases.tabsUseCases.selectTab)) + view.addProviders(SessionSuggestionProvider(components.core.sessionManager, + components.useCases.tabsUseCases.selectTab)) view.addProviders(SearchSuggestionProvider( components.search.searchEngineManager.getDefaultSearchEngine(this), components.useCases.searchUseCases.defaultSearch, SearchSuggestionProvider.Mode.MULTIPLE_SUGGESTIONS) ) - view.setOnStopListener { bus.emit(AwesomeBarAction::class.java, AwesomeBarAction.ItemSelected) } + view.setOnStopListener { actionEmitter.onNext(AwesomeBarAction.ItemSelected) } } } override fun updateView() = Consumer { view.onInputChanged(it.query) } -} \ No newline at end of file +} diff --git a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarComponent.kt b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarComponent.kt index bc3465320..683448d5d 100644 --- a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarComponent.kt +++ b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarComponent.kt @@ -16,18 +16,21 @@ import org.mozilla.fenix.mvi.ViewState class ToolbarComponent( private val container: ViewGroup, - override val bus: ActionBusFactory, + bus: ActionBusFactory, override var initialState: SearchState = SearchState("") ) : - UIComponent(bus) { + UIComponent( + bus.getManagedEmitter(SearchAction::class.java), + bus.getSafeManagedObservable(SearchChange::class.java) + ) { override val reducer: Reducer = { state, change -> when (change) { - is SearchChange.QueryChanged -> state.updateQuery(change.query) + is SearchChange.QueryChanged -> state.copy(query = change.query) } } - override fun initView() = ToolbarUIView(container, bus) + override fun initView() = ToolbarUIView(container, actionEmitter, changesObservable) init { render(reducer) } @@ -36,12 +39,11 @@ class ToolbarComponent( fun editMode() = getView().editMode() } -data class SearchState(val query: String) : ViewState { - fun updateQuery(query: String) = SearchState(query) -} +data class SearchState(val query: String) : ViewState sealed class SearchAction : Action { - data class UrlCommitted(val url: String): SearchAction() + data class UrlCommitted(val url: String) : SearchAction() + data class TextChanged(val query: String) : SearchAction() } sealed class SearchChange : Change { diff --git a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarUIView.kt b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarUIView.kt index fba8ae495..07ee7a111 100644 --- a/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarUIView.kt +++ b/app/src/main/java/org/mozilla/fenix/search/toolbar/ToolbarUIView.kt @@ -7,6 +7,8 @@ package org.mozilla.fenix.search.toolbar import android.view.LayoutInflater import android.view.ViewGroup import androidx.core.content.ContextCompat +import io.reactivex.Observable +import io.reactivex.Observer import io.reactivex.functions.Consumer import mozilla.components.browser.domains.autocomplete.ShippedDomainsProvider import mozilla.components.browser.toolbar.BrowserToolbar @@ -14,12 +16,14 @@ import mozilla.components.support.ktx.android.content.res.pxToDp import org.mozilla.fenix.R import org.mozilla.fenix.components.toolbar.ToolbarIntegration import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.requireComponents -import org.mozilla.fenix.mvi.ActionBusFactory import org.mozilla.fenix.mvi.UIView -class ToolbarUIView(container: ViewGroup, bus: ActionBusFactory) : - UIView(container, bus) { +class ToolbarUIView( + container: ViewGroup, + actionEmitter: Observer, + changesObservable: Observable +) : + UIView(container, actionEmitter, changesObservable) { val toolbarIntegration: ToolbarIntegration @@ -33,7 +37,7 @@ class ToolbarUIView(container: ViewGroup, bus: ActionBusFactory) : init { view.apply { onUrlClicked = { false } - setOnUrlCommitListener { bus.emit(SearchAction::class.java, SearchAction.UrlCommitted(it)) } + setOnUrlCommitListener { actionEmitter.onNext(SearchAction.UrlCommitted(it)) } browserActionMargin = resources.pxToDp(browserActionMarginDp) urlBoxView = urlBackground @@ -46,11 +50,11 @@ class ToolbarUIView(container: ViewGroup, bus: ActionBusFactory) : setOnEditListener(object : mozilla.components.concept.toolbar.Toolbar.OnEditListener { override fun onTextChanged(text: String) { - bus.emit(SearchChange::class.java, SearchChange.QueryChanged(text)) + actionEmitter.onNext(SearchAction.TextChanged(text)) } override fun onStopEditing() { - bus.emit(SearchAction::class.java, SearchAction.UrlCommitted("foo")) + actionEmitter.onNext(SearchAction.UrlCommitted("foo")) } }) }