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

Reproducible: ItemKeyedDataSource's loadBefore gets called infinitely #816

Closed
mwajeeh opened this issue Aug 29, 2019 · 5 comments · May be fixed by #817
Closed

Reproducible: ItemKeyedDataSource's loadBefore gets called infinitely #816

mwajeeh opened this issue Aug 29, 2019 · 5 comments · May be fixed by #817

Comments

@mwajeeh
Copy link

mwajeeh commented Aug 29, 2019

There is a similar issue here #621
I just attempted to convert custom two way paging mechanism to epoxy paging library and this issue is making it impossible. I am attaching the code which can easily reproduce this. PagedListModelCache relies on lastPosition to trigger loadAround() and I think thats where the bug is.

How to reproduce?
When you launch this activity, you will see that first page keeps loading which is expected. Now if you scroll down 1 or 2 pages and then scroll up just enough to trigger loadBefore() then you will see that it will still go in infinite loop invoking loadBefore(). This is because lastPosition is stored indefinitely in PagedListModelCache.

updateCallback.onInserted() -> requestModelBuild()-> modelCache.getModels() -> lastPosition?.let{triggerLoadAround()} -> updateCallback.onInserted()

Following is modified code from epoxy paging sample that can easily produce this issue: https://github.com/airbnb/epoxy/tree/master/epoxy-pagingsample

import android.app.Application
import android.content.Context
import android.os.Bundle
import android.util.Log
import androidx.appcompat.app.AppCompatActivity
import androidx.appcompat.widget.AppCompatTextView
import androidx.lifecycle.AndroidViewModel
import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.Observer
import androidx.lifecycle.ViewModelProviders
import androidx.paging.DataSource
import androidx.paging.ItemKeyedDataSource
import androidx.paging.LivePagedListBuilder
import androidx.paging.PagedList
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import androidx.room.Room
import com.airbnb.epoxy.EpoxyModel
import com.airbnb.epoxy.ModelView
import com.airbnb.epoxy.TextProp
import com.airbnb.epoxy.paging.PagedListEpoxyController
import java.util.UUID
import java.util.concurrent.Executors

class PagingNetworkSampleActivity : AppCompatActivity() {
    public override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)
        val pagingController = TestController()
        val recyclerView = findViewById<RecyclerView>(R.id.recycler_view)
        recyclerView.layoutManager = LinearLayoutManager(this)
        recyclerView.adapter = pagingController.adapter

        val viewModel = ViewModelProviders.of(this).get(ActivityViewModel::class.java)
        viewModel.pagedList.observe(this, Observer {
            pagingController.submitList(it)
        })
    }
}

class TestController : PagedListEpoxyController<NetworkUser>() {
    override fun buildItemModel(currentPosition: Int, item: NetworkUser?): EpoxyModel<*> {
        return if (item == null) {
            PagingViewModel_()
                .id(-currentPosition)
                .name("loading $currentPosition")
        } else {
            PagingViewModel_()
                .id(item.uid)
                .name("${item.uid}: ${item.firstName} / ${item.lastName}")
        }
    }

    override fun addModels(models: List<EpoxyModel<*>>) {
        pagingView {
            id("header")
            name("showing ${models.size} items")
        }
        super.addModels(models)
    }

    init {
        isDebugLoggingEnabled = true
    }

    override fun onExceptionSwallowed(exception: RuntimeException) {
        throw exception
    }
}

@ModelView(autoLayout = ModelView.Size.MATCH_WIDTH_WRAP_HEIGHT)
class PagingView(context: Context) : AppCompatTextView(context) {

    @TextProp
    fun name(name: CharSequence) {
        text = name
    }
}

class ActivityViewModel(app: Application) : AndroidViewModel(app) {
    val db by lazy {
        Room.inMemoryDatabaseBuilder(app, PagingDatabase::class.java).build()
    }
    val pagedList: LiveData<PagedList<NetworkUser>> by lazy {
        LivePagedListBuilder<String, NetworkUser>(NetworkDataSource.Factory(), 5)
            .setFetchExecutor(Executors.newFixedThreadPool(2))
            .build()

    }
}

data class NetworkUser(
    var uid: String,

    var firstName: String = "first name $uid",

    var lastName: String = "last name $uid"
)

class NetworkDataSource : ItemKeyedDataSource<String, NetworkUser>() {
    override fun loadInitial(
        params: LoadInitialParams<String>,
        callback: LoadInitialCallback<NetworkUser>
    ) {
        Thread.sleep(1000)
        callback.onResult((0..5).map { NetworkUser(UUID.randomUUID().toString()) })
    }

    override fun loadAfter(params: LoadParams<String>, callback: LoadCallback<NetworkUser>) {
        Log.d(">>", "loadAfter called")
        Thread.sleep(1000)
        callback.onResult((0..5).map { NetworkUser(UUID.randomUUID().toString()) })
    }

    override fun loadBefore(params: LoadParams<String>, callback: LoadCallback<NetworkUser>) {
        Log.d(">>", "loadBefore called")
        Thread.sleep(1000)
        callback.onResult((0..5).map { NetworkUser(UUID.randomUUID().toString()) })
    }

    override fun getKey(item: NetworkUser): String = item.uid

    class Factory : DataSource.Factory<String, NetworkUser>() {
        private val sourceLiveData = MutableLiveData<NetworkDataSource>()
        override fun create(): DataSource<String, NetworkUser> {
            val source = NetworkDataSource()
            sourceLiveData.postValue(source)
            return source
        }
    }
}
@mwajeeh
Copy link
Author

mwajeeh commented Aug 29, 2019

Following implementation of ListUpdateCallback for PagedListModelCache.updateCallback seems to fix the issue:
It adjusts the lastPosition if newly inserted data is above that lastPosition.

        @Synchronized
        override fun onInserted(position: Int, count: Int) {
            assertUpdateCallbacksAllowed()
            (0 until count).forEach {
                modelCache.add(position, null)
            } 
            //adjust lastPosition
            lastPosition?.let {
                if (position < it) {
                    lastPosition = it + count
                }
            }
            rebuildCallback()
        }

@mwajeeh mwajeeh changed the title ItemKeyedDataSource' loadBefore gets called infinitely ItemKeyedDataSource's loadBefore gets called infinitely Aug 30, 2019
@mwajeeh
Copy link
Author

mwajeeh commented Sep 2, 2019

Any updates on fix?

@mwajeeh mwajeeh changed the title ItemKeyedDataSource's loadBefore gets called infinitely Reproducible: ItemKeyedDataSource's loadBefore gets called infinitely Sep 3, 2019
@elihart
Copy link
Contributor

elihart commented Sep 3, 2019

No update, feel free to submit a PR to fix it.

Does this need to be a separate issue from #621?

mwajeeh added a commit to mwajeeh/epoxy that referenced this issue Sep 3, 2019
Adjust last position if inserted item is before last position.
Fixes airbnb#816 and airbnb#621
@mwajeeh
Copy link
Author

mwajeeh commented Sep 3, 2019

Both issue seem same to me.

@elihart
Copy link
Contributor

elihart commented Sep 11, 2019

Closing because #621 was opened first, move communication there

@elihart elihart closed this as completed Sep 11, 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

Successfully merging a pull request may close this issue.

2 participants