android / architecture-components-samples

Samples for Android Architecture Components.
https://d.android.com/arch
Apache License 2.0
23.4k stars 8.29k forks source link

[Paging3] Add example for custom implmentation of PagingSource + Paging3 Remote Meditator #889

Closed vincent-paing closed 3 years ago

vincent-paing commented 4 years ago

There's example with Room now, but for those that doesn't use Room, it would be nice if there's an example for using paging3 without the help of Room. I tried implementation with other db and I couldn't figure it out.

The current example is unclear on showing how one should use paging3. The problem lies in the fact that RemoteMeditator insert into Room Database and Room internally handles in reloading the data for you. If one want to go about creating their own paging source, there's no guiding hands for that. That's why I believe it would be great to provide a official example on how one can go about creating a paging source and use RemoteMeditator with it.

dlam commented 4 years ago

We have a few samples on DAC that show how to setup a PagingSourceusing network APIs here: https://developer.android.com/topic/libraries/architecture/paging/v3-paged-data#pagingsource

About additional samples here, I feel it would be best to keep them focused on arch components, but I'm happy to address your question and perhaps improve the DAC docs if it's insufficient in some way.

As for the PagingSource implemention, it basically reads from SQL DB using LIMIT + OFFSET, and returns it as a LoadResult.Page with the starting position (offset) as the key. Anytime that table is written to, all active PagingSource implementations get invalidated, so you need some kind of callback or knowledge of when your table is updated and hook your PagingSourceFactory implementation into that. So those are the two bits you need to setup if you're interested in a custom db implementation.

Btw, if your DB implementation is relatively popular, feel free to file a feature request on our issuetracker! I'm happy to explore ways to make Paging more accessible.

orgmir commented 4 years ago

Thank you for the link @dlam, I also have a use case without room and had some trouble setting this up. It would be useful to have a sample without Room, I could not find the Room's PagingSource implementation for reference. since it uses LegacyPagingSource that wraps a Datasource which we should no longer use.

Maybe it will help you @vincent-paing so here is what I ended up with: My pagingSourceFactory creates a paging source and registers a listener on the mediator that will invalidate that source.

fun createFlow(): Flow<PagingData<MagicItem>> {
    val mediator = MagicRemoteMediator(cache)
    val config = PagingConfig(100)
    val pager = Pager(
        config = config,
        remoteMediator = mediator,
        pagingSourceFactory = {
            val source = MagicPagingSource(api)
            mediator.addListener { source.invalidate() } // <-- listening and calling invalidate
            source
        }
    )
    return pager.flow
}

then on the mediator I have:

private var listeners = arrayListOf<() -> Unit>()

fun addListener(listener: () -> Unit) {
        listeners.add(listener)
    }

private fun removeListener(listener: () -> Unit) {
    listeners.remove(listener)
}

private fun notifyListeners() {
    ArrayList(listeners).forEach {
        it()
        removeListener(it)
    }
}

Every time the mediator fetches data and receives a successful response, you call notifyListeners() and invalidate() will be called on the paging source, triggering a load for the new data.

dlam commented 3 years ago

FYI: You may find this ticket interesting to follow, which aims to put the PagingSource tracking logic into an abstract class you can extend directly: https://issuetracker.google.com/160716447

dlam commented 3 years ago

The magic sauce you get from Room is that it automatically hooks up DB updates to invalidate callbacks to all the PagingSources it generates, so if not using Room or SqlDelight or similar with Paging support, you'll need to hook this up manually.

dlam commented 3 years ago

Either way, I think we want to keep these samples focused on architecture components working together to not distract from the general / common usage, but this is a good tip for improving the documentation on DAC once we flesh out the RemoteMediator API and get it stable. I'm going to close this issue out as it's pertinent to the samples on this repo.

felipepedroso commented 3 years ago

@dlam do you have any resources or more examples about how to use mediator without Room? I already tried a couple of things but nothing works. I even copied the same invalidating mechanism from Room Paging Source generated code and tried to use the LegacyPagingSource connected to our local database solution.

I did some experimentation on the current samples and noticed that the sample doesn't work if I decorate LegacyPagingSource with a debug paging source that logs its operations. This leads me to believe that RemoteMediator behaviour is highly coupled with Room, is that true?

During my tests, even using the invalidation mechanism that you suggest above the mediator always get stuck on prepend and append. :(

dlam commented 3 years ago

RemoteMediator doesn't directly affect anything in Paging at all except LoadState, it's simply a callback that lets you load some data from network, insert it somewhere your PagingSource can read from, then invalidate PagingSource.

Are you able to share your project? I can take a look, but to actually answer your question - RemoteMediator doesn't depend on Room (nothing in paging does, room actually depends on paging to provide their integration).

felipepedroso commented 3 years ago

Are you able to share your project?

I'll create a POC with parts of the code today to share with you.

Thanks for the prompt answer :)

felipepedroso commented 3 years ago

Hi @dlam, here is a simplified version of what we are trying to do here: https://github.com/felipepedroso/RemoteMediatorTest

From what I can see, mediator is able to invalidate the PagingSource but nothing is calling the load method from the new instance of PagingSource, making append and prepend to fail.

felipepedroso commented 3 years ago

HI @dlam, I also tried the InvalidatingPagingSourceFactory to invalidate the PagingSource when the DB is updated but it didn't work as well... Are there any examples about how to integrate Paging 3 with other DBs?

Thanks!

dlam commented 3 years ago

Sorry it took me awhile to get back to you here.

You need to change the following to collectLatest.

viewModel.pagingDataFlow.collect { pagingData ->
  itemsAdapter.submitData(pagingData)
}

.submitData is a suspending function which doesn't return until that generation is terminated, although you should be able to depend on paging to close it for you, it can always take some unknown amount of time until that cleanup happens, so you should always use collectLatest to propagate cancellation as early as possible.

felipepedroso commented 3 years ago

Nice catch! Changing this allowed the sample to load only the first page:

drawing

The refresh doesn't work as well, I have no clue why :sweat_smile:

I'll try to debug around a bit more, thanks for the help :)

dlam commented 3 years ago

That sounds like you aren't calling invalidation. You want them to trigger on any insert / delete / modification to the table and ensure you call PagingSource.invalidate() (again InvalidatingPagingSourceFactory can help you keep track of PagingSources produced) so that a new generation is kicked off to pick up the DB changes.

felipepedroso commented 3 years ago

Hey @dlam, I'm still trying to make this work without success. Is there any working example of a third party library that have Remote Mediator?

I added a couple of logs on PagingSource and RemoteMediator and I can't understand why remote mediator is not being actioned after PagingSource tried to get the second page and there is nothing on the "DB" yet. I also did some debugging and the invalidation mechanism seems to be working properly when the mediator is actioned... Do you have any clues that I should check?

Here are the logs that I added on PagingSource, RemoteMediator and data invalidation mechanism:


D/LogTest: Snapshot created
D/LogTest: Snapshot: 8808266 
    Content: {} 
    Page index: {}
D/LogTest: -----------
D/LogTest: PagingSource load params: androidx.paging.PagingSource$LoadParams$Refresh@5f2f7d8
D/LogTest: PagingSource tried to load page PageKey(value=1, previousPageKey=null, nextPageKey=null)
D/LogTest: PagingSource load result: Page(data=[], prevKey=null, nextKey=null, itemsBefore=-2147483648, itemsAfter=-2147483648)
D/LogTest: -----------
D/LogTest: Snapshot invalidated
D/LogTest: Snapshot: 8808266 
    Content: {} 
    Page index: {}
D/LogTest: -----------
D/LogTest: Snapshot created
D/LogTest: Snapshot: 148838775 
    Content: {1=[Item(id=Page 1 - Item 1), Item(id=Page 1 - Item 2), Item(id=Page 1 - Item 3), Item(id=Page 1 - Item 4), Item(id=Page 1 - Item 5), Item(id=Page 1 - Item 6), Item(id=Page 1 - Item 7), Item(id=Page 1 - Item 8), Item(id=Page 1 - Item 9), Item(id=Page 1 - Item 10)]} 
    Page index: {1=PageKey(value=1, previousPageKey=null, nextPageKey=2)}
D/LogTest: -----------
D/LogTest: PagingSource load params: androidx.paging.PagingSource$LoadParams$Refresh@9ca49e4
D/LogTest: PagingSource tried to load page PageKey(value=1, previousPageKey=null, nextPageKey=2)
D/LogTest: PagingSource load result: Page(data=[Item(id=Page 1 - Item 1), Item(id=Page 1 - Item 2), Item(id=Page 1 - Item 3), Item(id=Page 1 - Item 4), Item(id=Page 1 - Item 5), Item(id=Page 1 - Item 6), Item(id=Page 1 - Item 7), Item(id=Page 1 - Item 8), Item(id=Page 1 - Item 9), Item(id=Page 1 - Item 10)], prevKey=null, nextKey=2, itemsBefore=-2147483648, itemsAfter=-2147483648)
D/LogTest: -----------
D/LogTest: Mediator load type: REFRESH
D/LogTest: Mediator state: PagingState(pages=[], anchorPosition=null, config=androidx.paging.PagingConfig@7fd1e4d, leadingPlaceholderCount=0)
D/LogTest: Mediator tried to load key 1
D/LogTest: Mediator fetcher response: Extra(result=[Item(id=Page 1 - Item 1), Item(id=Page 1 - Item 2), Item(id=Page 1 - Item 3), Item(id=Page 1 - Item 4), Item(id=Page 1 - Item 5), Item(id=Page 1 - Item 6), Item(id=Page 1 - Item 7), Item(id=Page 1 - Item 8), Item(id=Page 1 - Item 9), Item(id=Page 1 - Item 10)], totalCount=null, href=null, nextPage=2, nextCursor=null, hasNext=true, totalUnseenItemsCount=0, isYourNetworkFeedSeen=null, showBookmarksDialog=null)
D/LogTest: Mediator fetcher error: null
D/LogTest: Mediator loaded with success. Reached end: false
D/LogTest: -----------
D/LogTest: PagingSource load params: androidx.paging.PagingSource$LoadParams$Append@ad42050
D/LogTest: PagingSource tried to load page PageKey(value=2, previousPageKey=null, nextPageKey=null)
D/LogTest: PagingSource load result: Page(data=[], prevKey=null, nextKey=null, itemsBefore=-2147483648, itemsAfter=-2147483648)
D/LogTest: -----------```
dlam commented 3 years ago

@felipepedroso I took a brief look at your sample, but I couldn't find where you call PagingSource.invalidate() after writing to PagedCache. In order to get Paging to pick up your changes you need to call .invalidate() in order to get it to reload from cache. This is needed because Paging doesn't know how you are storing your data so it can't automatically pick up changes.

You should be getting a PagingSource REFRESH load after your write to PagedCache from a successful network fetch in RemoteMediator because writing to PagedCache should trigger invalidation. This means either your pagingSourceFactory needs to know about your PagedCache or the other way around. Typically, this is done via registering a callback on writes to your backing cache which then calls .invalidate().

felipepedroso commented 3 years ago

Hey @dlam, thanks for the quick answer!

The invalidation call is happening inside mediator when the method withTransaction is called (CachedRemoteMediator L85-93), triggering the method invalidateSnapshots (InMemoryPageCache L90-L95), and and invalidating the paging source (CachePagingSource L12).

From what I could understand from my logs, the problem is happening during the append of the second page where the PagingSource tries to get it from PageCache, gets an empty result, and does not trigger the RemoteMediator to fetch the data from the network. Here is the flow of what I could observe from my logs:

  1. PagingSource 1 is created
  2. PagingSource 1 receives a refresh request, tries to get page 1 from PagedCache, and gets an empty response
  3. Mediator receives a refresh request, fetches the content of page 1, inserts to the PagedCache, and finishes successfuly
  4. PagingSource 1 is invalidated, triggering the creation of PagingSource 2
  5. PagingSource 2 receives a refresh request, tries to get the content of page 1, and gets the content inserted by the mediator
  6. PagingSource 2 receives an append request, tries to get page 2 from PagedCache, and gets an empty response

From my understanding, RemoteMediator should receive an append request after step 6 to fetch page 2 and invalidate PagingSource 2, which is not happening. Am I missing something else?

dlam commented 3 years ago

Paging will continue to request APPEND from PagingSource until you pass null for nextKey which is how you let Paging know there is no more data to load. An empty page does not terminate and trigger RemoteMediator, null for the key does.

felipepedroso commented 3 years ago

Paging will continue to request APPEND from PagingSource until you pass null for nextKey which is how you let Paging know there is no more data to load. An empty page does not terminate and trigger RemoteMediator, null for the key does.

Not sure if I understood it here.

We have a PagingSource trying to get the page 2 data from DB to append but since nothing is available on the DB yet, shouldn't it trigger the append on RemoteMediator? Without it the DB won't have the data and, since the API returns if there are other pages, we won't have indication there is page 3...

masterwok commented 3 years ago

I'm facing the same issue; the first two pages are fetched by the RemoteMediator but the third page is never requested. Were you able to figure this out @felipepedroso ?

Below is my PagingSource.load implementation:

override suspend fun load(params: LoadParams<Int>): LoadResult<Int, User> {
    val currentPageKey = params.key ?: 1

    val users = pagingCache.getPageItems(currentPageKey) ?: emptyList()

    return LoadResult.Page(
        data = users,
        prevKey = if (currentPageKey == 1) null else currentPageKey - 1,
        nextKey = if (users.isEmpty()) null else currentPageKey + 1
    )
}

From the documentation on RemoteMediator.load:

Callback triggered when Paging needs to request more data from a remote source due to any of the following events:

  • Stream initialization if initialize returns LAUNCH_INITIAL_REFRESH
  • REFRESH signal driven from UI
  • PagingSource returns a LoadResult which signals a boundary condition, i.e., the most recent LoadResult.Page in the PREPEND or APPEND direction has LoadResult.Page.prevKey or LoadResult.Page.nextKey set to null respectively.

Logcat output:

D/PAGING_SOURCE: Created: com.example.pagingpoc.data.repositories.regres.UsersPagingSource@9b01e8e
I/okhttp.OkHttpClient: --> GET https://reqres.in/api/users?page=1&per_page=2
I/okhttp.OkHttpClient: <-- 200 https://reqres.in/api/users?page=1&per_page=2 (598ms, unknown-length body)
D/PAGING_SOURCE: Invalidated: com.example.pagingpoc.data.repositories.regres.UsersPagingSource@9b01e8e
D/PAGING_SOURCE: Created: com.example.pagingpoc.data.repositories.regres.UsersPagingSource@eb326f6
I/okhttp.OkHttpClient: --> GET https://reqres.in/api/users?page=2&per_page=2
I/okhttp.OkHttpClient: <-- 200 https://reqres.in/api/users?page=2&per_page=2 (50ms, unknown-length body)
D/PAGING_SOURCE: Invalidated: com.example.pagingpoc.data.repositories.regres.UsersPagingSource@eb326f6
D/PAGING_SOURCE: Created: com.example.pagingpoc.data.repositories.regres.UsersPagingSource@4730d64
felipepedroso commented 3 years ago

@masterwok, not really.

I did some changes on my sample to identify potential problems but I didn't have any success: https://github.com/felipepedroso/RemoteMediatorTest

Hey @dlam, are you aware of any example or project that is using RemoteMediator without Room? We are reaching to a critical step during our Paging 3 migration and not being able to use without Room will make us get stuck to Paging 2 or find an alternative to implement it... :cry:

felipepedroso commented 3 years ago

We did some changes on PagingSource to reflect this comment from @dlam and the paging source started to load all the pages:

https://user-images.githubusercontent.com/985993/125288244-d1496a80-e315-11eb-9220-d3015426e39a.mp4

The only problem now is that every time that we fetch a page, the list scrolls to the top...

Does anyone has any clue why this is happening? :thinking:

masterwok commented 3 years ago

Thank you, @felipepedroso I'm going to look over your changes. In regards to the scroll to top, I noticed your implementation of getRefreshKey(..) always returns the starting page index:

https://github.com/felipepedroso/RemoteMediatorTest/blob/096e66b529aba51efd0b477424ed40eb781efbf8/app/src/main/java/br/pedroso/remotemediatortest/paging/PagesCachePagingSource.kt#L13

Docs:

https://developer.android.com/topic/libraries/architecture/paging/v3-migration#refresh-keys

My implementation uses the anchorPosition maybe this will help:

override fun getRefreshKey(
    state: PagingState<Int, User>
): Int? = state.anchorPosition?.let { anchorPosition ->
    state.closestPageToPosition(anchorPosition)?.prevKey?.plus(1)
        ?: state.closestPageToPosition(anchorPosition)?.nextKey?.minus(1)
}
dlam commented 3 years ago

The only problem now is that every time that we fetch a page, the list scrolls to the top...

Does anyone has any clue why this is happening? :thinking:

Did you implement PagingSource.getRefreshKey()? On invalidation, Paging uses this method to fogure out which key to use to reload from the current position.

felipepedroso commented 3 years ago

Thank you, @felipepedroso I'm going to look over your changes. In regards to the scroll to top, I noticed your implementation of getRefreshKey(..) always returns the starting page index:

@masterwok I was exactly this, thanks a lot! :)

Did you implement PagingSource.getRefreshKey()? On invalidation, Paging uses this method to fogure out which key to use to reload from the current position.

@dlam yes, this was missing.

Now I'm facing a problem with the in memory cache that is causing ANRs when I edit items on the data source. I'll investigate further and I'll add the solution on my repo afterwards.

masterwok commented 3 years ago

@felipepedroso nice, glad it worked out! Did you ever encounter the issue where the InvalidatingPagingSourceFactory becomes stuck in an infinite loop between invalidation and creating a new paging source? It seems to occur when using an actual PageFetcher with Retrofit.

felipepedroso commented 3 years ago

@felipepedroso nice, glad it worked out! Did you ever encounter the issue where the InvalidatingPagingSourceFactory becomes stuck in an infinite loop between invalidation and creating a new paging source? It seems to occur when using an actual PageFetcher with Retrofit.

It's probably what is happenning with me right now :sweat_smile:

Did you find a solution? I couldn't find the source of the infinite loop tbh...

masterwok commented 3 years ago

@felipepedroso I wasn't able to resolve the issue. I think it has something to do with Retrofit offloading to Dispatchers.IO but everything I've tried results in the same issue.

dlam commented 3 years ago

Instead of using InvalidatingPagingSourceFactory can you try something like this and see if it resolves your problem?

public class InvalidatingPagingSourceFactory<Key : Any, Value : Any>(
    private val pagingSourceFactory: () -> PagingSource<Key, Value>
) : () -> PagingSource<Key, Value> {
    private val pagingSources = mutableListOf<PagingSource<Key, Value>>()

    override fun invoke(): PagingSource<Key, Value> {
        return pagingSourceFactory().also { pagingSources.add(it) }
    }

    public fun invalidate() {
        for (pagingSource in pagingSources.toList()) {
            if (!pagingSource.invalid) {
                pagingSource.invalidate()
            }
        }

        pagingSources.removeAll { it.invalid }
    }
}
masterwok commented 3 years ago

@dlam thank you!! That seems to have resolved the looping issue.

felipepedroso commented 3 years ago

Thanks @dlam, replacing the factory fixed the ANR :)

My problem now is with the list jumping to the second page whenever I remove an item from the list:

https://user-images.githubusercontent.com/985993/126130427-f4f6c8df-09d7-45b4-837e-b919086ac403.mp4

Anyway, I'll investigate more to see if I didn't mess with anything from RecyclerView.

Thanks @dlam and @masterwok :rocket:

masterwok commented 3 years ago

@felipepedroso I have a repository that borrows heavily from your repository. My repository is hooked up to a mock API and has a few differenent changes to the remote mediator and paging source. I seem to have an issue where invoking refresh on the adapter doesn't function correctly and current page flashing on load of next.

https://github.com/masterwok/poc-cached-paging-3

dlam commented 3 years ago

My problem now is with the list jumping to the second page whenever I remove an item from the list:

@felipepedroso Make sure your .getRefreshKey() returns a key which PagingSource will use to load the same items currently visible on the screen for the initial load to prevent jumping.

I seem to have an issue where invoking refresh on the adapter doesn't function correctly and current page flashing on load of next

@masterwok By refresh not functioning correctly, do you just mean the flashing or does it not reload at all?

masterwok commented 3 years ago

@dlam, thanks for the quick response. I was experiencing two separate issues.

The first issue was resolved by fixing a bug in my implementation of getRefreshKey.

The second issue (which I'm still struggling with) is that invoking refresh() on the adapter does not reload the data until the second invocation. Even then, the second invocation causes the RecyclerView to scroll to some offset.

The latest implementation of my PoC is available here if you are able to take a look: https://github.com/masterwok/poc-cached-paging-3

Thank you!

masterwok commented 3 years ago

I was able to resolve all known issues with the following changes:

https://github.com/masterwok/poc-cached-paging-3/commit/7231adbf258e1639300bbcf60577b7064a82796b

https://github.com/masterwok/poc-cached-paging-3/commit/af50ab2b72c06dc75cc83d0a36cc019ee02d4052

Thanks @dlam and @felipepedroso for your help!

felipepedroso commented 3 years ago

Hi @dlam and @masterwok, I made changes on the getRefreshKey method using the same approach from @masterwok sample and we don't have the list jumping around anymore. However, the list keeps flickering when I remove items or scroll:

https://user-images.githubusercontent.com/985993/126461123-ae1757bf-4364-4b47-a98a-eeb8269c4d56.mp4

I tried do debug this yesterday but I have no clue why this is happening... Do you have any clues of what I should look for?

Thanks a lot for the help!

dlam commented 3 years ago

For the second part around 0:08 where only the bottom half loads in first after an item deletion - it looks like the key for .getRefreshKey() doesn't load enough items to cover the viewport entirely. You generally want enough items before and after anchorPosition to fulfill the entire viewport so DiffUtil can do its work.

As for the flashing - are you sure that DiffUtil is able to recognize the items across invalidation / refresh as unchanged?

felipepedroso commented 3 years ago

Hey @dlam, thanks for the answer :)

For the second part around 0:08 where only the bottom half loads in first after an item deletion - it looks like the key for .getRefreshKey() doesn't load enough items to cover the viewport entirely. You generally want enough items before and after anchorPosition to fulfill the entire viewport so DiffUtil can do its work.

Not sure if I got this... Does it means that I would need to rearrange items across the pages when an item is removed?

As for the flashing - are you sure that DiffUtil is able to recognize the items across invalidation / refresh as unchanged?

Well, the item object is a data class (data class Item(val id: Int, val description: String)) and here is its diffUtil:

        private val DIFF_CALLBACK = object : DiffUtil.ItemCallback<Item>() {
            override fun areContentsTheSame(oldItem: Item, newItem: Item) = oldItem == newItem
            override fun areItemsTheSame(oldItem: Item, newItem: Item) = oldItem.id == newItem.id
        }

I revisited the DiffUtil.ItemCallback documentation and I believe that it's right... Am I missing something? :thinking: