Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better paging support #59

Open
levibostian opened this issue Oct 28, 2019 · 0 comments
Open

Better paging support #59

levibostian opened this issue Oct 28, 2019 · 0 comments
Assignees
Milestone

Comments

@levibostian
Copy link
Owner

Problem

Here is how Teller currently suggests you add paging support to your app:

  1. Add a "page number" property reference to your OnlineRepository.Requirements class and it's tag. That way Teller will only update your cache if the page is out-of-date.
  2. As you scroll and hit the end of the current page, set a new OnlineRepository.requirements property instance with an incremented page number such as MyTellerRespoitory.Requirements(pageNumber: currentRequirements.pageNumber + 1)
  3. Because of RecyclerView's DiffUtil, the new data will be inserted into your list in the correct order when a new cache is saved.

Then, when you want to do a pull-to-refresh at the top of the RV list, you can set the requirements property to MyTellerRespoitory.Requirements(pageNumber: 1) and then perform a force refresh on the repository instance.

This leads to problems, however.

  1. The recommended way to do paging above does not work well with the Android paging library. The paging library is not required to add paging supoort, but after using it more over the past week, I have found that it has a lot of flexibility and power that developers can take advantage of. If Teller works well with the paging library but also still not be required to use it, that would allow the most power and flexibility into all apps.
  2. After using an app for a while, the amount of old cache data that might seem irrelevant can add up. The currently recommended way of doing paging will not delete old cache data. It will simply update chunks of it and fill in the "gaps" as you scroll from new data to old. It would be a good idea for the developer to every now and then go through and delete old paging cache data to prevent the app taking up too much space.

After looking into the PagingNetwork example app from Google, I realized that how it does paging is:

  • As you scroll, ping the network API to get another page of data.
  • Save that new page of data to the DB.
  • Observe the DB in the UI to show the list of paged data.

This is just like what Teller recommends. But with one difference:

  • When the user performs a pull-to-refresh or updates the first page of the cache, it will first delete all of the existing cached data and then insert the new first page of cache data.

I ran some experiments with the GMail Android app to see how it behaves. When you open the app, you are shown offline/cached emails that you can scroll through and view. When you scroll to the bottom of the list, it grabs another page of data from the GMail API and inserts it at the end of the list.

If you close the app and open it again, that cache still exists. You see the full cache.

But, if you force close the app and open it up again, you will notice that GMail has deleted for you all of the emails from the list beyond the first page of data. Just like the GitHub example app shows.

  1. Lastly, there is a bug. Here is a scenario:
  • Open app for the first time (no cache exists). Scroll a list and retrieve the first 3 pages of data, save it to the cache.
  • The Repository's cache life is set for 6 hours.
  • Force close app, open it again. Pull-to-refresh the list to force retrieve the first page of data. Now, scroll down the list to the next page of data. Bug! Why? Well, Teller allows you to force refresh 1 page of data but it does not allow you to force refresh multiple pages of data. The requirement's tag is setup for each page which means that pages > page 1 cannot be force refreshed and will only be updated after the cache life is over.

Solution

Ignore the current way Teller recommends paging while reading this new proposal.

Here are the goals in mind with their proposed solutions:

  • Goal: Allow Teller's paging support to be as fast and easy to implement as possible. Take the existing Teller architecture that you have and having paging support within that. Right now, the docs are more trying to explain how Teller and paging works together instead of a step-by-step instruction. It can be difficult to follow and the docs are quite long and complex. If you don't do something correct, you could mess it all up.

Proposed solution

Create a new abstract OnlineRepository subclass that extends OnlineRepository that is made just for paging.

This new abstract class will have a new property called pagingRequirements that cannot be null. It requires an initial page to be set.

When you scroll the list and want to retrieve the next cache page number, you leave the requirements property alone and only change the pagingRequirements property which will kick off an API call to get the next page of data. The repository will always attempt to do the API call because it's assumed that if you are changing the page number, you do not have that page available already in the cache. If it was in the cache, you would just be showing it after a query.

Everything else is the same with Teller's OnlineRepository. The OnlineRepository will not perform more network calls then it needs to because it will respect the life of cache of the repository.

  • Goal: If Teller can help you delete old cache data, great. Or at least recommend a way to do it within your app and put this recommendation within the docs.

Teller will be able to tell if it has (1) performed a force refresh and (2) performed a successful fetch on the first page of data. In other of these scenarios, it's recommended to delete the old cache and replace with new. So, add a new parameter to the saveCache() function that indicates if this is a new first page of data. If this property is true, it's recommended you delete the old and then save the new.

  • Goal: Not have bugs! Have the cache always be up-to-date with the API's data.

Because this new subclass would respect the untouched OnlineRepository.Requirements.tag, this bug would no longer be present.

  • Goal: Have Android paging library support but not require it to be used.

The paging library can work out of the box if you do this:

  1. Set the OnlineRepository.Cache data type to be a PagedList.
  2. Assuming that you're using Room and Room's built-in support for PositionalDataSource, when the user performs a pull-to-refresh, you call invalidate() on the DataSource and reset to the first pagingRequirements`. Teller will then make sure to delete the old cache.

Besides that, you can implement any DataSource subclass that you wish! It's up to you. Teller is not bothered by this.

@levibostian levibostian added this to the 0.3.0-alpha milestone Oct 28, 2019
@levibostian levibostian self-assigned this Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant