[WIP] split read and write SOT types #560
Conversation
digitalbuddha
changed the title
There was a problem hiding this comment.
Overall LGTM. I wonder if we should separate the converter for Store and MutableStore usage. I think when we support #554 users would otherwise find fromOutputToLocal confusing. The failing test is with generics in rx2
rx2/src/main/kotlin/org/mobilenativefoundation/store/rx2/RxSourceOfTruth.kt
Outdated
Show resolved
Hide resolved
store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/Converter.kt
Outdated
Show resolved
Hide resolved
store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/Converter.kt
Outdated
Show resolved
Hide resolved
| fun fromNetworkToLocal(network: Network): Local | ||
| fun fromOutputToLocal(output: Output): Local |
There was a problem hiding this comment.
My understanding is OutputToLocal is only used with MutableStore. Should this interface be separated?
| @@ -55,7 +56,16 @@ internal class FetcherController<Key : Any, Network : Any, Output : Any, Local : | |||
| */ | |||
| private val sourceOfTruth: SourceOfTruthWithBarrier<Key, Network, Output, Local>?, | |||
|
|
|||
| private val converter: Converter<Network, Output, Local>? = null | |||
| private val converter: Converter<Network, Local, Output> = object : Converter<Network, Local, Output> { | |||
There was a problem hiding this comment.
Can you add a comment explaining why it is safe to use this object as the converter?
There was a problem hiding this comment.
This is due to a bit of tangling between store and mutable store. In store, there is no concept of local, the network type is always the same as what is going into the source of truth. This default parameter keeps us from having to pass in that dummy everywhere. The second function fromOutputToLocal will never be called, I'll throw an exception from there.
| private val memoryCache: Cache<Key, Output>? = null, | ||
| private val converter: Converter<Network, Local, Output> = object : | ||
| Converter<Network, Local, Output> { | ||
| override fun fromOutputToLocal(output: Output): Local = |
There was a problem hiding this comment.
This fromOutputToLocal method is only invoked internally right? Should the error message convey it's not an issue with their usage?
|
@matt-ramotar UpdaterTests was failing for me locally, I don't think it was rx related |
* split read and write SOT types * all but 2 tests passing * remove extranous parameterized type on simple store factory * pr review * pr review * Passing except for UpdaterTests (MobileNativeFoundation#565) Signed-off-by: Matt Ramotar <mramotar@dropbox.com> * mark mutablestore experimental --------- Signed-off-by: Matt Ramotar <mramotar@dropbox.com> Co-authored-by: Matt Ramotar <mramotar@dropbox.com>
Need to fix some of the newer tests and document. Fixes #559 #558