diff --git a/bento/build.gradle b/bento/build.gradle index 65e9de8..244e81b 100644 --- a/bento/build.gradle +++ b/bento/build.gradle @@ -40,6 +40,7 @@ task androidSourcesJar(type: Jar) { dependencies { // Kotlin implementation Libs.KOTLIN + implementation Libs.KOTLIN_REFLECT // Apache Commons implementation Libs.APACHE_COMMONS diff --git a/bento/src/main/java/com/yelp/android/bento/core/ComponentGroup.java b/bento/src/main/java/com/yelp/android/bento/core/ComponentGroup.java index d7b248a..0674938 100644 --- a/bento/src/main/java/com/yelp/android/bento/core/ComponentGroup.java +++ b/bento/src/main/java/com/yelp/android/bento/core/ComponentGroup.java @@ -35,6 +35,7 @@ public class ComponentGroup extends Component { private final Map mComponentDataObserverMap = new HashMap<>(); private final ComponentGroupObservable mObservable = new ComponentGroupObservable(); + private final NotifyChecker mNotifyChecker = new NotifyChecker(); public ComponentGroup() { mSpanSizeLookup = @@ -166,6 +167,7 @@ public ComponentGroup addComponent(int index, @NonNull final Component component ComponentDataObserver componentDataObserver = new ChildComponentDataObserver(component); component.registerComponentDataObserver(componentDataObserver); mComponentDataObserverMap.put(component, componentDataObserver); + mNotifyChecker.prefetch(component); notifyItemRangeInserted(insertionStartIndex, component.getCountInternal()); mObservable.notifyOnChanged(); @@ -346,7 +348,10 @@ public void unregisterComponentGroupObserver(@NonNull ComponentGroupDataObserver public Object getItem(int position) { RangedValue compPair = mComponentAccordionList.rangedValueAt(position); Component component = mComponentAccordionList.valueAt(position); - return component.getItemInternal(position - compPair.mRange.mLower); + int positionInComponent = position - compPair.mRange.mLower; + Object itemInternal = component.getItemInternal(positionInComponent); + mNotifyChecker.save(component, positionInComponent, itemInternal); + return itemInternal; } /** @@ -591,6 +596,7 @@ private void cleanupComponent(@NonNull Component component) { entry.setValue(entry.getValue() - 1); } } + mNotifyChecker.remove(component); mObservable.notifyOnComponentRemoved(component); } @@ -610,6 +616,7 @@ private ChildComponentDataObserver(@NonNull Component component) { @Override public void onChanged() { + mNotifyChecker.onChanged(mComponent); int listPosition = mComponentIndexMap.get(mComponent); Range originalRange = mComponentAccordionList.get(listPosition).mRange; int newSize = mComponent.getCountInternal(); @@ -621,6 +628,7 @@ public void onChanged() { @Override public void onItemRangeChanged(int positionStart, int itemCount) { + mNotifyChecker.onItemRangeChanged(mComponent, positionStart, itemCount); int listPosition = mComponentIndexMap.get(mComponent); Range originalRange = mComponentAccordionList.get(listPosition).mRange; @@ -630,6 +638,7 @@ public void onItemRangeChanged(int positionStart, int itemCount) { @Override public void onItemRangeInserted(int positionStart, int itemCount) { + mNotifyChecker.onItemRangeInserted(mComponent, positionStart, itemCount); int listPosition = mComponentIndexMap.get(mComponent); Range originalRange = mComponentAccordionList.get(listPosition).mRange; mComponentAccordionList.set( @@ -643,6 +652,7 @@ public void onItemRangeInserted(int positionStart, int itemCount) { @Override public void onItemRangeRemoved(int positionStart, int itemCount) { + mNotifyChecker.onItemRangeRemoved(mComponent, positionStart, itemCount); int listPosition = mComponentIndexMap.get(mComponent); Range originalRange = mComponentAccordionList.get(listPosition).mRange; mComponentAccordionList.set( @@ -656,6 +666,7 @@ public void onItemRangeRemoved(int positionStart, int itemCount) { @Override public void onItemMoved(int fromPosition, int toPosition) { + mNotifyChecker.onItemMoved(mComponent, fromPosition, toPosition); int listPosition = mComponentIndexMap.get(mComponent); Range originalRange = mComponentAccordionList.get(listPosition).mRange; diff --git a/bento/src/main/java/com/yelp/android/bento/core/NotifyChecker.kt b/bento/src/main/java/com/yelp/android/bento/core/NotifyChecker.kt new file mode 100644 index 0000000..a82bb4e --- /dev/null +++ b/bento/src/main/java/com/yelp/android/bento/core/NotifyChecker.kt @@ -0,0 +1,239 @@ +package com.yelp.android.bento.core + +import android.util.Log +import com.yelp.android.bento.utils.EqualsHelper.deepEquals +import java.lang.ref.WeakReference + +private const val TAG = "NotifyChecker" + +class NotifyChecker { + private val componentsToItem: MutableMap = mutableMapOf() + + fun prefetch(component: Component) { + val items = componentsToItem.getOrPut(component) { + ItemStorage(component.countInternal) + } + + val result = runCatching { + for (position in 0 until component.countInternal) { + items[position] = component.getItemInternal(position) + } + } + if (result.isFailure) { + Log.v(TAG, "Could not prefetch $component", result.exceptionOrNull()) + } + } + + fun save(component: Component, position: Int, item: Any?) { + val items = componentsToItem.getOrPut(component) { + ItemStorage(component.countInternal) + } + + items[position] = item + } + + fun remove(component: Component) { + componentsToItem.remove(component) + } + + fun onChanged(component: Component) { + val verifyChange = verifyOnChanged(component) + if (verifyChange is NotifyCheckResult.IncorrectOnChange) { + Log.d(TAG, "onChanged for $component: $verifyChange") + } + } + + fun onItemRangeChanged(component: Component, positionStart: Int, itemCount: Int) { + val verifyChange = verifyItemRangeChanged(component, positionStart, itemCount) + if (verifyChange is NotifyCheckResult.IncorrectOnItemRangeChange) { + Log.d(TAG, "onItemRangeChanged for $component: $verifyChange") + } + } + + fun onItemRangeInserted(component: Component, positionStart: Int, itemCount: Int) { + val itemStorage = componentsToItem[component] ?: return + + itemStorage.onItemRangeInserted(positionStart, itemCount) + } + + fun onItemRangeRemoved(component: Component, positionStart: Int, itemCount: Int) { + val itemStorage = componentsToItem[component] ?: return + + itemStorage.onItemRangeRemoved(positionStart, itemCount) + } + + fun onItemMoved(component: Component, fromPosition: Int, toPosition: Int) { + val itemStorage = componentsToItem[component] ?: return + + itemStorage.onItemMoved(fromPosition, toPosition) + } + + // Check to confirm that the whole component indeed changed. + private fun verifyOnChanged(component: Component): NotifyCheckResult { + val itemStorage = componentsToItem[component] + if (itemStorage == null) { + return NotifyCheckResult.NotEnoughData + } else if (!itemStorage.isFullyPrefetched) { + return NotifyCheckResult.NotEnoughData + } + + val updatedItems = ItemStorage(component.countInternal) + + val unchanged = mutableListOf() + // We can also keep track of the "null"? A bunch of components use null as items, + // like dividers, simple components, and so on. + val undecided = mutableListOf() + + when { + itemStorage.items.size == component.countInternal -> { + // Same size, let's compare items one by one. + itemStorage.items.forEachIndexed { index, weakReference -> + val item: Any? = component.getItem(index) + updatedItems[index] = item + + if (item == null) { + undecided.add(index) + } else if (item.deepEquals(weakReference?.get())) { + unchanged.add(index) + } + } + } + itemStorage.items.size < component.countInternal -> { + // Let's compare the stored items, to see if it's okay to call onChanged, + // or if we should call onItemRangeInsertedInstead + itemStorage.items.forEachIndexed { index, weakReference -> + val item: Any? = component.getItem(index) + updatedItems[index] = item + + if (item == null) { + undecided.add(index) + } else if (item.deepEquals(weakReference?.get())) { + unchanged.add(index) + } + } + for (index in itemStorage.items.size until component.countInternal) { + val item: Any? = component.getItem(index) + updatedItems[index] = item + } + } + else -> { + // Well, the count shrunk, let's make sure that the items in the component are different + // than the one stored. If not, it would be better to call OnItemRangeChange instead. + for (index in 0 until component.countInternal) { + val weakReference: WeakReference<*>? = itemStorage[index] + val item: Any? = component.getItem(index) + updatedItems[index] = item + if (item == null) { + undecided.add(index) + } else if (item.deepEquals(weakReference?.get())) { + unchanged.add(index) + } + } + } + } + + componentsToItem[component] = updatedItems + + return if (unchanged.size > 0) { + NotifyCheckResult.IncorrectOnChange(unchanged) + } else { + NotifyCheckResult.CorrectChange + } + } + + private fun verifyItemRangeChanged(component: Component, positionStart: Int, itemCount: Int): NotifyCheckResult { + val itemStorage = componentsToItem[component] + if (itemStorage == null) { + return NotifyCheckResult.NotEnoughData + } else if (!itemStorage.isFullyPrefetched) { + return NotifyCheckResult.NotEnoughData + } + + val unchanged = mutableListOf() + val undecided = mutableListOf() + val updatedItems: Array?> = arrayOf(*itemStorage.items) + + for (index in positionStart until positionStart + itemCount) { + val weakReference: WeakReference<*>? = itemStorage[index] + val item: Any? = component.getItem(index) + updatedItems[index] = WeakReference(item) + + if (item == null) { + undecided.add(index) + } else if (item.deepEquals(weakReference?.get())) { + unchanged.add(index) + } + } + itemStorage.items = updatedItems + return if (unchanged.size > 0) { + NotifyCheckResult.IncorrectOnItemRangeChange(unchanged) + } else { + NotifyCheckResult.CorrectChange + } + } + + class ItemStorage(capacity: Int) { + var items: Array?> = Array(capacity) { null } + + /** + * Are all the item pre-fetched? Check that the items array is full of weak references. + */ + val isFullyPrefetched: Boolean get() = items.none { it == null } + + operator fun set(index: Int, item: Any?) { + items[index] = WeakReference(item) + } + + operator fun get(index: Int) = items[index] + + fun onItemRangeInserted(positionStart: Int, itemCount: Int) { + val newItems: Array?> = Array(items.size + itemCount) { null } + items.copyInto(newItems, 0, 0, positionStart) + items.copyInto(newItems, positionStart + itemCount, positionStart, items.size) + + items = newItems + } + + fun onItemRangeRemoved(positionStart: Int, itemCount: Int) { + val newItems: Array?> = Array(items.size - itemCount) { null } + items.copyInto(newItems, 0, 0, positionStart) + items.copyInto(newItems, positionStart, positionStart + itemCount, items.size) + + items = newItems + } + + fun onItemMoved(fromPosition: Int, toPosition: Int) { + val itemsAsList = items.toMutableList() + val item: WeakReference<*>? = itemsAsList.removeAt(fromPosition) + itemsAsList.add(toPosition, item) + + items = itemsAsList.toTypedArray() + } + } +} + +sealed class NotifyCheckResult { + object NotEnoughData : NotifyCheckResult() { + override fun toString(): String { + return "NotifyCheckResult: Not enough data to establish validity of data change" + } + } + + class IncorrectOnChange(private val unchanged: List) : NotifyCheckResult() { + override fun toString(): String { + return "NotifyCheckResult: You should not call onChange, indexes $unchanged stayed the same." + } + } + + class IncorrectOnItemRangeChange(private val unchanged: List) : NotifyCheckResult() { + override fun toString(): String { + return "NotifyCheckResult: You should not call onItemRangeChange, indexes $unchanged stayed the same." + } + } + + object CorrectChange : NotifyCheckResult() { + override fun toString(): String { + return "NotifyCheckResult: The onChange call was justified" + } + } +} diff --git a/bento/src/main/java/com/yelp/android/bento/utils/EqualsHelper.kt b/bento/src/main/java/com/yelp/android/bento/utils/EqualsHelper.kt new file mode 100644 index 0000000..2b63fdf --- /dev/null +++ b/bento/src/main/java/com/yelp/android/bento/utils/EqualsHelper.kt @@ -0,0 +1,16 @@ +package com.yelp.android.bento.utils + +import org.apache.commons.lang3.builder.EqualsBuilder + +object EqualsHelper { + // Works well enough, but not for collections. + fun Any?.deepEquals(other: Any?): Boolean { + return when { + this == null && other == null -> true + this == null || other == null -> false + this::class != other::class -> false + this::class.isData -> this == other + else -> EqualsBuilder.reflectionEquals(this, other) + } + } +} diff --git a/buildSrc/src/main/java/com/yelp/gradle/bento/GlobalDependencies.kt b/buildSrc/src/main/java/com/yelp/gradle/bento/GlobalDependencies.kt index eb0f6ea..b4ab4e7 100644 --- a/buildSrc/src/main/java/com/yelp/gradle/bento/GlobalDependencies.kt +++ b/buildSrc/src/main/java/com/yelp/gradle/bento/GlobalDependencies.kt @@ -5,7 +5,7 @@ import org.gradle.api.artifacts.repositories.MavenArtifactRepository object Publishing { const val GROUP = "com.yelp.android" - const val VERSION = "18.0.2" + const val VERSION = "18.0.2-bvermont-SNAPSHOT" } object Versions {