Problems when working with a List<T>

[Bradleycorn]

I'm trying to setup a pretty basic Store that works with a List that is backed by a (Room) SoT, but I am running into a few problems, outlined below. The crux of the problem is that a call to the store's get() method never returns in some cases when the list is empty.

My setup is fairly straight forward:

val myStore = StoreBuilder.from(
        fetcher = Fetcher.of { postType: PostType ->

            // Retrofit api call that returns a list of posts
            val response = myApi.getPostsByType(postType) 

            // convert the api response json into a List<Post>
            return when {
                response.isSuccessful -> Post.fromWordpress(response.body())
                else -> listOf()
            }
        },
        sourceOfTruth = SourceOfTruth.of(
            reader = { postType ->
                //Dao method returns a Flow<List<Post>> from the "posts" table
                db.postsDao().postsByTypeObservable(postType)
            },
            writer = { postType, postList ->
                // writing the saved list to the Room DB is a bit more complicated, 
                // because we have some many-many relationships with categories...
                db.postsDao().withTransaction {
                    val posts = postList.map { it.post }
                    val categories = postList.flatMap { it.categories }.distinct()
                    val postCategories = postList.flatMap { it.postCategories }.distinct()

                    db.postsDao().upsert(posts)
                    db.categoriesDao().upsert(categories)
                    db.postCategoriesDao().upsert(postCategories)
                }
            },
            delete = db.postsDao()::deletePostsByType,
            deleteAll = db.postsDao()::deleteAllPosts
        )
    ).build()

This setup works OK, unless/until there are no posts to work with. I've specifically run into 2 problems:

1. If you call get() with this setup and there is no data in the SoT, the fetcher will not be called.

From reading the source code, it appears that the Fetcher only gets called if the reader returns null. But when working with a list, the reader will return an empty list, and not null. Because of that, the fetcher never gets called.

We can actually fix this problem fairly easily by converting an empty list to null in the reader:

 reader = { postType ->
     //Dao method returns a Flow<List<Post> from the "posts" table
     db.postsDao().postsByTypeObservable(postType).map { entries ->
         when {
             entries.isEmpty() -> null
             else -> entries
     }
 },

Now, an empty list is treated as null, and thus the fetcher gets called.

2. But now we have a second problem. If you call get() and there are no entries in the SoT, and the Fetcher also returns no entries, the suspended get() method will never finish/complete.

That is because once the fetcher finishes up, there are no new entries to put into the database. The get() is ultimately (via a stream) observing the Flow from the reader, and that flow will not emit anything (since there were no writes). Because of that, there is never any value emitted on the Flow (due to the filters setup in the get()) method. And since there is no value ever emitted, the suspended get() method never finishes/completes.

Is there a way to setup a Store that works with a List and ensure that the get() method will always return?

[digitalbuddha]

Maybe I'm missing something, why would the fetcher return no entries?

[Bradleycorn]

Because the Store's Key is based on a "post type" (effectively a "category"). There really could be any number of reasons that the fetcher may return an empty data set. For example, we may try to fetch posts with a postType of "Kotlin", but on the server, no posts have been created yet with the "Kotlin" category. Or maybe the underlying API call enforces some date restrictions, to only fetch data from the api that is new within the last 24 hours, and there is no new data. As I mentioned, there could be any number of reasons that a fetcher (an API call) returns an empty data set.

[Bradleycorn]

@digitalbuddha hmm ... I guess I should distinguish between the fetcher and the api .... and that raises a question ...
As I said, there are any number of reasons that an api may return an empty data set.
Is there an optimal way to setup the fetcher to handle that case? Maybe the fetcher should return null when there is no actual data to return? I haven't read through the source far enough to see what would happen in that case.

[digitalbuddha]

What do you expect to be returned in this case from get?
I believe we are battling against it being your source of truth. If you want to write and read back nulls that fine, maybe in that case you can wrap the response in a result type so that you can do something like Result.Empty (null object pattern).

Alternatively you can not read directly from room but rather have a middle broker (state flow maybe?) which emits when null is written.

Finally you can throw an exception from the fetcher, exceptions do not get saved to source of truth but do hit the collector.

If you want to create an example project/test case if be happy to show the different variants of the above suggestions

[Bradleycorn]

What do you expect to be returned in this case from get?

Good question. In this case, I would expect that if I call get() and there are no records returned from my source of truth, it would then call my fetcher, and if that also wound up not adding any records, then the get() call would eventually return an empty list. I understand that the above example doesn't return an empty list because I explicitly setup the source of truth to return null when the list is empty. Honestly, whether it's an empty list or null is trivial to me. But as of now it doesn't return at all and the coroutine that called get() just remains suspended forever.

So, if I do the following:

viewModelScope.launch {
    Log.d("TEST", "Getting Data from Store")
    val data = myStore.get(PostType.KOTLIN)
    Log.d("Test", "Done getting Data from Store")
}

That 2nd log message will never show up.

If you want to create an example project/test case if be happy to show the different variants of the above suggestions.

I may take you up on that. First I'll try some of your suggestions myself and see where it gets me. Throwing an exception fro the Fetcher may make sense in my case.. As I mentioned above, It's not imperative that my store can read/write nulls. But it seems like I HAVE to make the reader return nulls in order to trigger the fetcher.
And then I need to figure out how to make get() return SOMETHING (null or empty list, doesn't really matter) when there's no data. The only thing I can't do, is just have that coroutine hanging out suspended forever waiting on a value that's never coming.

[Bradleycorn]

Thinking about this more, and triggering an error in the Fetcher when an empty data set is returned from the api seems like the right thing to do ... As I thought about it, this "problem" could also potentially arise from a single item (i.e. not a list).

For example, ake a Store that returns a product by it's ID ... If the reader doesn't find the product on disk, it returns null and the fetcher is triggered to lookup the product via a network api. If the network returns null (or, an "empty" product...), then there is no product written back to disk (via the writer), and when the reader then does another disk lookup, it's still null. stream is setup such that nulls from the reader are not emitted. Since get() is just a stream that filters out the Loading emits and returns the first item emitted, it would never emit anything. This is the problem I'm having with my List.

But, using the product example above, there's a key part that doesn't really make much sense:

If the network returns null (or, an "empty" product...)

that goes back to the original question from @digitalbuddha, "why would the fetcher return no entries?" ... Any sane network would not return an "empty" product. If the product wasn't found, it would return some sort of error code or message ("product not found"), and the Fetcher would handle that error or lack of data in some graceful way. My list scenario is the same ... if we ask the network for a list of X, and there are none, it should result in an error or lack of data that the fetcher handles. In my case (a list of posts) I think it's reasonable to throw a "No posts available for type X" Exception.

[brescia123]

I have the same problem: my network source returns a list of item owned by a user and that list could be of course empty, and in that case the server correctly returns an empty collection. My local db has a table containing items owned by the user as rows. Now the problem is that simply observing the table content Store is not able to understand whether it is empty because user has no items or because values weren't fetched yet.

I think this could be a pretty common use case for Store. Is there a simple solution to this or is it something that Store doesn't support?
@digitalbuddha

[digitalbuddha]

Nothing built in. What is your expectation for an api? Do you want store response to tell you if any writes have occurred? What if you write to the dB table from outside store? Store only forwards what you tell it, do you want a isDiskEmpty callback that we can call?

Let me know what a perspective api looks like. I'm not sure I understand what the expectation is

[TylerMcCraw]

I think we need Store to let the collector know that both the source of truth (the local data storage, in this case) and the fetcher have returned an empty data set.
I believe this is related to something mentioned in another issue: As mentioned here by @yigit, #185 (comment), we need some way of modeling an empty state.

But, without understanding the full internals of Store, it seems that maybe this is simply a bug that ought to be fixed and not so much that there needs to be a new empty state returned from Store to the collector.
I would fully expect that if a source of truth (with an output of a List type) returns an empty list, then Store uses the Fetcher to fetch that list and the response from the Fetcher is also an empty list, then the collector should receive an empty list.
Currently, it's getting hung up and no Data type ever returns.

[TylerMcCraw]

So, I've found that this is actually causing serious issues for our app in some occasions and I can't wait for an official fix.
As a workaround, I've found that if I throw a custom exception in my Fetcher when the network response returns an empty data set, my collector can consume that thrown exception and handle it accordingly.
Running off of Bradleycorn's example, something like:

        Fetcher.of { postType: PostType ->

            // Retrofit api call that returns a list of posts
            val response = myApi.getPostsByType(postType) 

            val result = response.mapToResult()
            if (result is Success && result.data.isEmpty()) {
               throw NoNewDataException()
            }

            // convert the api response json into a List<Post>
            return when {
                response.isSuccessful -> Post.fromWordpress(response.body())
                else -> listOf()
            }
        }

In my case, the stream does not cancel so any new updates to the source of truth will still notify the collector, assuming you're properly handling the exception at the collection side.

[digitalbuddha]

Hi folks, out on paternity and catching up. I think store should have a configuration for the list handling @TylerMcCraw suggested... Treat empty set same as null or error. Would someone want to try to contribute, I would expect a configuration in store builder that then causes a wrapped fetcher/source of truth to be used.

[digitalbuddha]

Hi friends, I'm back from paternity and am going to add a new fetcher type that throws on empty. Let me know if anyone has suggestions on how it should work

[TylerMcCraw]

Welcome back, Mike!
I was going back through my notes and looking at our current workaround.
Like I mentioned, I think we need a Fetcher that doesn't suspend indefinitely due to there being an empty result set (an empty list).
So, my thinking is that whatever the current Fetcher implementation is doing to wait on a non-empty result should be removed for this type of Fetcher that we want.

Would it make more sense to create a different type of Fetcher? Or do you think this is something that should be passed in as some sort of configuration when instantiating the Fetcher that we have now?

[digitalbuddha]

I think we should have a new ThrowOnEmptyFetcher which has the wrapper logic you had above. Basically it would throw NoNewDataException anytime an empty list is returned. Would that solve your use case? Would you want to contribute this new Fetcher subclass?

[TylerMcCraw]

I can try to find some time to contribute something, but if someone else has more free time than me, please don't hesitate :)

One thing I want to clarify is that I don't know yet that throwing an exception is the right solution.
If the source of truth (with an output of a List type) returns an empty list, then Store uses the Fetcher to fetch that list and the response from the Fetcher is also an empty list, then only the initial StoreResponse.Loading response is emitted.
I think we actually need something like StoreResponse.Data with an empty list or something new, like StoreResponse.EmptyList to be emitted and that's not currently happening.

[digitalbuddha]

Got it yeah not ideal, its almost like we want a custom Store.Data.EmptyableData wrapper type for store responses. Now you see why this hasn't been taken on yet ;-)

[Kernald]

This documentation conflating lists and single elements is a bit confusing. Are there plans to tackle this issue with Store5?

[matt-ramotar]

Hey @Kernald - Thanks for checking in. Docs and sample apps are WIP. The larger issue is on our backlog. Will do our best to tackle it soon. PRs are very much welcomed!

[canny]

This issue has been linked to a Canny post: https://github.com/MobileNativeFoundation/Store/issues/255 🎉

[matt-ramotar]

Hey all - First stab at making it easier to work with lists: #548. Please consider taking a look and leaving feedback

[matt-ramotar]

Hey all - #548 just merged. Please share any feedback 🙏