idanatz / OneAdapter

A Viewholderless Adapter for RecyclerView, who supports builtin diffing, states (paging, empty...), events (clicking, swiping...), and more.
MIT License
470 stars 45 forks source link

Again: IllegalStateException: Two different ViewHolders have the same stable ID.... #25

Closed Minification closed 4 years ago

Minification commented 4 years ago

Hi,

I appear to have the same problem again that I have had numerous times before, plus an additional but maybe related problem. Skip to the bottom to see a summary.

Exception:

2020-07-21 22:02:09.269 8841-8841/de.mariushubatschek.virtuellelehrpfade E/AndroidRuntime: FATAL EXCEPTION: main
    Process: de.mariushubatschek.virtuellelehrpfade, PID: 8841
    java.lang.IllegalStateException: Two different ViewHolders have the same stable ID. Stable IDs in your adapter MUST BE unique and SHOULD NOT change.
     ViewHolder 1:ViewHolder{ee6663e position=8 id=-1687197900, oldPos=-1, pLpos:-1 not recyclable(1)} 
     View Holder 2:ViewHolder{d9b71cb position=5 id=-1687197900, oldPos=-1, pLpos:-1} androidx.recyclerview.widget.RecyclerView{c4902e4 VFED..... ......ID 0,0-733,100 #7f080130 app:id/photoRecyclerView}, adapter:com.idanatz.oneadapter.internal.InternalAdapter@a4fe34d, layout:androidx.recyclerview.widget.LinearLayoutManager@5d36e02, context:de.mariushubatschek.virtuellelehrpfade.MainActivity@d9a6051
        at androidx.recyclerview.widget.RecyclerView.handleMissingPreInfoForChangeError(RecyclerView.java:4268)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayoutStep3(RecyclerView.java:4192)
        at androidx.recyclerview.widget.RecyclerView.dispatchLayout(RecyclerView.java:3862)
        at androidx.recyclerview.widget.RecyclerView.onLayout(RecyclerView.java:4404)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at androidx.constraintlayout.widget.ConstraintLayout.onLayout(ConstraintLayout.java:1915)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at androidx.constraintlayout.widget.ConstraintLayout.onLayout(ConstraintLayout.java:1915)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at androidx.constraintlayout.widget.ConstraintLayout.onLayout(ConstraintLayout.java:1915)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1812)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1656)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1565)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1812)
        at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1656)
        at android.widget.LinearLayout.onLayout(LinearLayout.java:1565)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.widget.FrameLayout.layoutChildren(FrameLayout.java:323)
        at android.widget.FrameLayout.onLayout(FrameLayout.java:261)
        at com.android.internal.policy.DecorView.onLayout(DecorView.java:753)
        at android.view.View.layout(View.java:20672)
        at android.view.ViewGroup.layout(ViewGroup.java:6194)
        at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:2792)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2319)
2020-07-21 22:02:09.269 8841-8841/de.mariushubatschek.virtuellelehrpfade E/AndroidRuntime:     at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1460)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:7183)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:949)
        at android.view.Choreographer.doCallbacks(Choreographer.java:761)
        at android.view.Choreographer.doFrame(Choreographer.java:696)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:935)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

What did I do?

I added 4 items to a RecyclerView backed with a OneAdapter.

How do I setup the RecyclerView and OneAdapter in question?

In the Fragment:

val photoRecyclerView = view.findViewById<RecyclerView>(R.id.photoRecyclerView)
val layoutManager = LinearLayoutManager(context, LinearLayoutManager.HORIZONTAL, false)
photoRecyclerView.layoutManager = layoutManager
val snapHelper = LinearSnapHelper()
snapHelper.attachToRecyclerView(photoRecyclerView)
photosOneAdapter = OneAdapter(photoRecyclerView)
    photosOneAdapter
            .attachItemModule(PhotoItemModule(requireContext()))
            .attachEmptinessModule(VoiceOverEmptinessModule())

PhotoItemModule:

class PhotoItemModule (val context: Context) : ItemModule<Photo>() {

    override fun onBind(item: Item<Photo>, viewBinder: ViewBinder) {
        val imageView = viewBinder.findViewById<ImageView>(R.id.imageView)
        val imageUri = FileStorage.getFileUri(context, item.model.imagePath)
        Glide.with(context).load(imageUri).thumbnail(0.2f).override(200,100).fitCenter().diskCacheStrategy(DiskCacheStrategy.ALL).into(imageView)
    }

    override fun provideModuleConfig() = object : ItemModuleConfig() {
        override fun withLayoutResource() = R.layout.photo_item
    }

}

VoiceOverEmptinessModule:

class VoiceOverEmptinessModule : EmptinessModule() {
    override fun provideModuleConfig() = object : EmptinessModuleConfig() {
        override fun withLayoutResource() = R.layout.empty_recycler
    }
}

The item:

public class Photo implements Diffable {

    private static volatile int currentId = 0;

    private static final Object lock = new Object();

    private long id;

    private String imagePath;

    public Photo() {
        //TODO: Do this differently in production?
        synchronized (lock) {
            id = ++currentId;
            Timber.e("id is %d", id);
        }
    }

    public void setImagePath(String imagePath) {
        this.imagePath = imagePath;
    }

    public String getImagePath() {
        return imagePath;
    }

    public void setId(long id) {
        this.id = id;
    }

    public long getId() {
        return id;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        Photo photo = (Photo) o;
        return id == photo.id &&
                Objects.equals(imagePath, photo.imagePath);
    }

    @Override
    public int hashCode() {
        return Objects.hash(id, imagePath);
    }

    @Override
    public boolean areContentTheSame(@NotNull Object o) {
        if (!(o instanceof Photo)) {
            return false;
        }
        Photo other = (Photo) o;
        return other.id == id && other.imagePath.equals(imagePath);
    }

    @Override
    public long getUniqueIdentifier() {
        return id;
    }
}

How do I add items to the adapter?

In the Fragment:

    override fun onActivityCreated(savedInstanceState: Bundle?) {
        super.onActivityCreated(savedInstanceState)
        createMarkerDialogViewModel.photoListLiveData.observe(viewLifecycleOwner, Observer {
            photosOneAdapter.clear() //Why is this here? See point number 1 in the questions section
            photosOneAdapter.add(it)
            Timber.e("item count is ${photosOneAdapter.itemCount} while size is ${it.size}")
        })
    }

Here, createMarkerDialogViewModel is a ViewModel and photoListLiveData is a LiveData.

What do I see on the screen before the app crashes?

The following screenshot was taken after the third item was inserted. NOTE that each pair of the picture and the "a>z"-Image consitutes one item, but we count at least five pairs, i.e. at least 5 items: Screenshot_1595362863

My questions

  1. Why does the photosOneAdapter.clear() call not seem to work?
  2. Why are there at least 5 items visible after adding only 3?
  3. Why does that nasty exception occur? Am I doing anything wrong?

If anything is unclear or I forgot something, notify me and I'll provide the necessary information. Since this is part of a larger project at uni I can't really show the whole source, but I'll do my best to help.

idanatz commented 4 years ago

Hi, Regarding your questions:

  1. It is not working probably because a line after the clear you are adding items to the adapter, thus clearing the empty module and inserting an item module. why are you calling clear on each observation callback? when using observation with a full item list, it is best to use setItems() and let the adapter calculate the difference between the old and the new list automatically. if the list is empty, the empty module will be invoked, if not the adapter will render the changes. no need to call clear, or add in this case.

  2. The itemcount variable is used by the internals of the adapter, it may hold more items than you think to support various functions. Adding a function the will return the number of items per type (let's say a T model) will be helpful?

  3. First, try what I wrote in question 1 and see if the issue is resolved. Else, From your code, the id looks fine but there is a setId() function that I don't know if you calling from outside. The exception means 2 models have the same ID, are you 100% that's not the case?

Minification commented 4 years ago

Hi, thanks for your reply.

  1. I only added the call to clear() because for some reason I can see more items displayed than I added as can be seen in the screenshot I provided (there are 5 items visible but only 3 should be there (because of the call to clear)).

  2. I see. I only added the output line to check how many items are in the adapter because the logics didn't add up with the visuals.

  3. I removed the call to clear and printed the ids of the items to be added to the console. Every id in the output is unique, which is why I don't understand why the exception would occur. With the clear() removed, the output is as follows, where each output block is what is printed in the observation callback:

CreateMarkerDialogKotlin$onActivityCreated: item id is 1
...
CreateMarkerDialogKotlin$onActivityCreated: item id is 1
CreateMarkerDialogKotlin$onActivityCreated: item id is 2
...
CreateMarkerDialogKotlin$onActivityCreated: item id is 1
CreateMarkerDialogKotlin$onActivityCreated: item id is 2
CreateMarkerDialogKotlin$onActivityCreated: item id is 3
idanatz commented 4 years ago

And when you get these logs and crash, are you using add() or setItems()?

Because add() will result in the behavior you are describing and it's intended by design. The items will be added to the previous items, resulting in more and more items, plus the ids are the same since id=1 for example, is inserted 3 times. setItems() will compute a diff between the current list and the new list and send updates only when needed.

Minification commented 4 years ago

I see now, that is why those problems arise. I didn't know add() behaved like this by design (I thought it handled the diff calculation and setItems was just a setter).

With that knowledge, I'm closing the issue of course, though unless I missed it, I feel the documentation should include this rather important detail.

Come to think of it, I don't even know why I used add() instead of setItems()...

Thanks for your help, this definitely restores my faith in this library.