Skip to content

Commit

Permalink
Merge pull request #285 from rubensousa/listener-removal
Browse files Browse the repository at this point in the history
Fix concurrent modification exceptions when listeners remove themselves
  • Loading branch information
rubensousa authored Nov 14, 2024
2 parents 985f5d3 + b85ba5d commit 02545b3
Show file tree
Hide file tree
Showing 10 changed files with 353 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright 2024 Rúben Sousa
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.rubensousa.dpadrecyclerview.test.helpers

import androidx.test.platform.app.InstrumentationRegistry

fun runOnMainThread(action: () -> Unit) {
InstrumentationRegistry.getInstrumentation().runOnMainSync {
action()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import com.rubensousa.dpadrecyclerview.test.helpers.onRecyclerView
import com.rubensousa.dpadrecyclerview.test.helpers.selectPosition
import com.rubensousa.dpadrecyclerview.test.helpers.waitForCondition
import com.rubensousa.dpadrecyclerview.test.helpers.waitForIdleScrollState
import com.rubensousa.dpadrecyclerview.test.helpers.waitForLayout
import com.rubensousa.dpadrecyclerview.test.tests.DpadRecyclerViewTest
import com.rubensousa.dpadrecyclerview.testing.KeyEvents
import com.rubensousa.dpadrecyclerview.testing.rules.DisableIdleTimeoutRule
Expand Down Expand Up @@ -256,4 +257,31 @@ class FocusListenerTest : DpadRecyclerViewTest() {
assertThat(events).isEqualTo(0)
}

@Test
fun testFocusListenerThatRemovesItself() = report {
val listeners = 5
var events = 0
Given("Attach listeners") {
launchFragment()
onRecyclerView("Get recyclerView") { recyclerView ->
repeat(listeners) {
recyclerView.addOnViewFocusedListener(object : OnViewFocusedListener {
override fun onViewFocused(parent: RecyclerView.ViewHolder, child: View) {
events++
recyclerView.removeOnViewFocusedListener(this)
}
})
}
}
}
When("Select another position") {
selectPosition(2)
waitForLayout()
}

Then("Events are received") {
assertThat(events).isEqualTo(listeners)
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/*
* Copyright 2024 Rúben Sousa
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.rubensousa.dpadrecyclerview.test.tests.layout

import androidx.recyclerview.widget.RecyclerView
import com.google.common.truth.Truth.assertThat
import com.rubensousa.dpadrecyclerview.ChildAlignment
import com.rubensousa.dpadrecyclerview.DpadRecyclerView
import com.rubensousa.dpadrecyclerview.OnChildLaidOutListener
import com.rubensousa.dpadrecyclerview.ParentAlignment
import com.rubensousa.dpadrecyclerview.test.TestLayoutConfiguration
import com.rubensousa.dpadrecyclerview.test.helpers.onRecyclerView
import com.rubensousa.dpadrecyclerview.test.helpers.runOnMainThread
import com.rubensousa.dpadrecyclerview.test.helpers.selectPosition
import com.rubensousa.dpadrecyclerview.test.helpers.waitForLayout
import com.rubensousa.dpadrecyclerview.test.tests.DpadRecyclerViewTest
import com.rubensousa.dpadrecyclerview.testing.rules.DisableIdleTimeoutRule
import org.junit.Before
import org.junit.Rule
import org.junit.Test

class LayoutCompletionTest : DpadRecyclerViewTest() {

@get:Rule
val idleTimeoutRule = DisableIdleTimeoutRule()

override fun getDefaultLayoutConfiguration(): TestLayoutConfiguration {
return TestLayoutConfiguration(
spans = 1,
orientation = RecyclerView.VERTICAL,
parentAlignment = ParentAlignment(
edge = ParentAlignment.Edge.NONE,
fraction = 0.0f
),
childAlignment = ChildAlignment(
fraction = 0.0f
)
)
}

@Before
fun setup() {
launchFragment()
}

@Test
fun testLayoutCompleteListenerThatRemovesItself() = report {
var currentRecyclerView: DpadRecyclerView? = null
var events = 0
val listeners = 3
Given("Attach listener that removes itself") {
onRecyclerView("Get recyclerview") { recyclerView ->
currentRecyclerView = recyclerView
}
runOnMainThread {
// Add listeners that remove themselves
repeat(listeners) {
currentRecyclerView!!.addOnLayoutCompletedListener(
object : DpadRecyclerView.OnLayoutCompletedListener {
override fun onLayoutCompleted(state: RecyclerView.State) {
currentRecyclerView.removeOnLayoutCompletedListener(this)
events++
}
}
)
}
}
}

When("Request layout") {
selectPosition(10)
waitForLayout()
}

Then("Listener got removed") {
assertThat(events).isEqualTo(listeners)
}
}

@Test
fun testChildLayoutListenerThatRemovesItself() = report {
var currentRecyclerView: DpadRecyclerView? = null
var events = 0
val listeners = 3
Given("Attach listener that removes itself") {
onRecyclerView("Get recyclerview") { recyclerView ->
currentRecyclerView = recyclerView
}
runOnMainThread {
// Add listeners that remove themselves
repeat(listeners) {
currentRecyclerView!!.addOnChildLaidOutListener(object : OnChildLaidOutListener {
override fun onChildLaidOut(
parent: RecyclerView,
child: RecyclerView.ViewHolder
) {
currentRecyclerView.removeOnChildLaidOutListener(this)
events++
}
})
}
}
}

When("Request layout") {
selectPosition(10)
waitForLayout()
}

Then("Listener got removed") {
assertThat(events).isEqualTo(listeners)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import androidx.test.espresso.matcher.ViewMatchers
import com.google.common.truth.Truth.assertThat
import com.rubensousa.dpadrecyclerview.ChildAlignment
import com.rubensousa.dpadrecyclerview.DpadRecyclerView
import com.rubensousa.dpadrecyclerview.OnViewHolderSelectedListener
import com.rubensousa.dpadrecyclerview.ParentAlignment
import com.rubensousa.dpadrecyclerview.ParentAlignment.Edge
import com.rubensousa.dpadrecyclerview.test.TestAdapterConfiguration
Expand Down Expand Up @@ -337,4 +338,38 @@ class SelectionTest : DpadRecyclerViewTest() {
}
}

@Test
fun testSelectionListenerThatRemovesItself() = report {
val listeners = 5
var events = 0
Given("Attach listeners") {
launchFragment()
onRecyclerView("Get recyclerView") { recyclerView ->
repeat(listeners) {
recyclerView.addOnViewHolderSelectedListener(object :
OnViewHolderSelectedListener {
override fun onViewHolderSelected(
parent: RecyclerView,
child: RecyclerView.ViewHolder?,
position: Int,
subPosition: Int
) {
super.onViewHolderSelected(parent, child, position, subPosition)
recyclerView.removeOnViewHolderSelectedListener(this)
events++
}
})
}
}
}
When("Select another position") {
selectPosition(10)
waitForLayout()
}

Then("Events are received") {
assertThat(events).isEqualTo(listeners)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ import com.rubensousa.dpadrecyclerview.test.TestViewHolder
import com.rubensousa.dpadrecyclerview.test.assertions.ViewHolderAlignmentCountAssertion
import com.rubensousa.dpadrecyclerview.test.assertions.ViewHolderSelectionCountAssertion
import com.rubensousa.dpadrecyclerview.test.helpers.assertFocusAndSelection
import com.rubensousa.dpadrecyclerview.test.helpers.onRecyclerView
import com.rubensousa.dpadrecyclerview.test.helpers.runOnMainThread
import com.rubensousa.dpadrecyclerview.test.helpers.selectPosition
import com.rubensousa.dpadrecyclerview.test.helpers.selectSubPosition
import com.rubensousa.dpadrecyclerview.test.helpers.waitForIdleScrollState
import com.rubensousa.dpadrecyclerview.test.helpers.waitForLayout
import com.rubensousa.dpadrecyclerview.test.tests.DpadRecyclerViewTest
import com.rubensousa.dpadrecyclerview.testfixtures.DpadSelectionEvent
import com.rubensousa.dpadrecyclerview.testing.KeyEvents
Expand Down Expand Up @@ -175,6 +178,32 @@ class SubSelectionTest : DpadRecyclerViewTest() {
)
}

@Test
fun testSubSelectionIsAppliedForNextLayout() = report {
val position = 10
val subPosition = 1
param("position", position.toString())
param("subPosition", subPosition.toString())

var recyclerView: DpadRecyclerView? = null
Given("Launch fragment") {
launchSubPositionFragment()
onRecyclerView("Retrieve recyclerView instance") {
recyclerView = it
}
}

When("Select target positions") {
runOnMainThread {
recyclerView?.setSelectedSubPosition(position, subPosition)
}
waitForLayout()
}

Then("Assert selection is at position $position and sub position $subPosition") {
assertFocusAndSelection(position, subPosition)
}
}

private fun getSelectionsFromTasks(): List<DpadSelectionEvent> {
var events = listOf<DpadSelectionEvent>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import androidx.recyclerview.widget.RecyclerView
import androidx.recyclerview.widget.SimpleItemAnimator
import com.rubensousa.dpadrecyclerview.layoutmanager.PivotLayoutManager
import com.rubensousa.dpadrecyclerview.layoutmanager.focus.GlobalFocusChangeListener
import com.rubensousa.dpadrecyclerview.layoutmanager.forEachReversed
import com.rubensousa.dpadrecyclerview.spacing.DpadGridSpacingDecoration
import com.rubensousa.dpadrecyclerview.spacing.DpadLinearSpacingDecoration
import com.rubensousa.dpadrecyclerview.spacing.DpadSpacingDecoration
Expand Down Expand Up @@ -88,7 +89,7 @@ open class DpadRecyclerView @JvmOverloads constructor(
}
private val globalFocusChangeListener by lazy {
GlobalFocusChangeListener(this) {
focusLossListeners.forEach { listener ->
focusLossListeners.forEachReversed { listener ->
listener.onFocusLost(this)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright 2024 Rúben Sousa
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.rubensousa.dpadrecyclerview.layoutmanager

internal inline fun <T> List<T>.forEachReversed(action: (T) -> Unit) {
for (i in size - 1 downTo 0) {
action(get(i))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ internal class PivotSelector(
}
}
val focusedViewHolder = currentRecyclerView.findContainingViewHolder(view) ?: return
focusListeners.forEach { listener ->
focusListeners.forEachReversed { listener ->
listener.onViewFocused(
parent = focusedViewHolder,
child = view,
Expand All @@ -152,7 +152,7 @@ internal class PivotSelector(
pendingChildFocus = view
return
}
focusListeners.forEach { listener ->
focusListeners.forEachReversed { listener ->
listener.onViewFocused(
parent = parentViewHolder,
child = view
Expand Down Expand Up @@ -346,7 +346,7 @@ internal class PivotSelector(
if (viewHolder is DpadViewHolder) {
viewHolder.onViewHolderDeselected()
}
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderDeselected(recyclerView, viewHolder)
}
}
Expand All @@ -358,13 +358,13 @@ internal class PivotSelector(
selectedViewHolder = viewHolder

if (viewHolder != null) {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderSelected(
recyclerView, viewHolder, position, subPosition
)
}
} else {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderSelected(
recyclerView, null, RecyclerView.NO_POSITION, 0
)
Expand Down Expand Up @@ -406,13 +406,13 @@ internal class PivotSelector(
}

if (viewHolder != null) {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderSelectedAndAligned(
recyclerView, viewHolder, position, subPosition
)
}
} else {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderSelectedAndAligned(
recyclerView, null, RecyclerView.NO_POSITION, 0
)
Expand Down Expand Up @@ -459,7 +459,7 @@ internal class PivotSelector(
viewHolder.onViewHolderDeselected()
}
recyclerView?.let {
selectionListeners.forEach { listener ->
selectionListeners.forEachReversed { listener ->
listener.onViewHolderDeselected(it, viewHolder)
}
}
Expand Down
Loading

0 comments on commit 02545b3

Please sign in to comment.