Skip to content

Commit eeaf9d7

Browse files
committed
Make date-header keys collision-proof
The header LazyColumn key relied on the invariant that date-sorted comments make each label one contiguous group. If the list ever arrived out of date order a label could repeat non-contiguously, producing a duplicate key -> LazyColumn IllegalArgumentException (a crash, where the legacy list only showed a redundant header). withDateHeaders now mints a guaranteed-unique key per header (still just the label in the normal case, disambiguated only on a non-contiguous repeat), so correctness no longer depends on the sort order.
1 parent 4f45b2d commit eeaf9d7

3 files changed

Lines changed: 45 additions & 11 deletions

File tree

WordPress/src/main/java/org/wordpress/android/ui/commentsrs/CommentsRsListRow.kt

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ package org.wordpress.android.ui.commentsrs
22

33
/** A rendered row in the rs comment list: either a date-group header or a comment. */
44
sealed interface CommentsRsListRow {
5-
data class DateHeader(val label: String) : CommentsRsListRow
5+
/** [key] is a guaranteed-unique LazyColumn key; [label] is the (date) text shown to the user. */
6+
data class DateHeader(val label: String, val key: String) : CommentsRsListRow
67
data class Item(val comment: CommentRsUiModel) : CommentsRsListRow
78
}
89

@@ -12,19 +13,31 @@ sealed interface CommentsRsListRow {
1213
* the previous one. The label is the row's own [CommentRsUiModel.relativeDate] — the same
1314
* javaDateToTimeSpan value the legacy list groups by — so no extra date handling is needed here.
1415
*
15-
* The label doubles as the header's LazyColumn key: comments are date-sorted, so a label maps to
16-
* exactly one contiguous group (unique key), and keying by the label rather than the group's first
17-
* comment keeps the header stable when a newer comment is prepended into an existing group.
16+
* Each header carries a unique [CommentsRsListRow.DateHeader.key] for the LazyColumn. Comments are
17+
* date-sorted, so a label normally maps to one contiguous group and the key is just the label —
18+
* which keeps the header stable when a newer comment is prepended into an existing group. Should
19+
* the list ever arrive out of date order, a repeated label is disambiguated rather than emitting a
20+
* duplicate key (which LazyColumn rejects with a hard crash, unlike the legacy RecyclerView).
1821
*/
1922
fun withDateHeaders(comments: List<CommentRsUiModel>): List<CommentsRsListRow> {
2023
val rows = ArrayList<CommentsRsListRow>(comments.size + 1)
24+
val usedKeys = HashSet<String>()
2125
var lastLabel: String? = null
2226
for (comment in comments) {
2327
if (comment.relativeDate != lastLabel) {
24-
rows.add(CommentsRsListRow.DateHeader(comment.relativeDate))
28+
rows.add(CommentsRsListRow.DateHeader(comment.relativeDate, uniqueKey(comment.relativeDate, usedKeys)))
2529
lastLabel = comment.relativeDate
2630
}
2731
rows.add(CommentsRsListRow.Item(comment))
2832
}
2933
return rows
3034
}
35+
36+
/** A stable per-label key, suffixed only if the same label recurs non-contiguously (see above). */
37+
private fun uniqueKey(label: String, used: MutableSet<String>): String {
38+
val base = "header_$label"
39+
if (used.add(base)) return base
40+
var n = 1
41+
while (!used.add("$base#$n")) n++
42+
return "$base#$n"
43+
}

WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsTabListScreen.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ private fun CommentListContent(
148148
items = rows,
149149
key = { row ->
150150
when (row) {
151-
is CommentsRsListRow.DateHeader -> "header_${row.label}"
151+
is CommentsRsListRow.DateHeader -> row.key
152152
is CommentsRsListRow.Item -> row.comment.remoteCommentId
153153
}
154154
},

WordPress/src/test/java/org/wordpress/android/ui/commentsrs/CommentsRsListRowTest.kt

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class CommentsRsListRowTest {
1717
val comment = comment(id = 1, date = "Today")
1818

1919
assertThat(withDateHeaders(listOf(comment))).containsExactly(
20-
DateHeader("Today"),
20+
header("Today"),
2121
Item(comment)
2222
)
2323
}
@@ -28,7 +28,7 @@ class CommentsRsListRowTest {
2828
val b = comment(id = 2, date = "Today")
2929

3030
assertThat(withDateHeaders(listOf(a, b))).containsExactly(
31-
DateHeader("Today"),
31+
header("Today"),
3232
Item(a),
3333
Item(b)
3434
)
@@ -42,12 +42,12 @@ class CommentsRsListRowTest {
4242
val d = comment(id = 4, date = "January 8")
4343

4444
assertThat(withDateHeaders(listOf(a, b, c, d))).containsExactly(
45-
DateHeader("Today"),
45+
header("Today"),
4646
Item(a),
4747
Item(b),
48-
DateHeader("Yesterday"),
48+
header("Yesterday"),
4949
Item(c),
50-
DateHeader("January 8"),
50+
header("January 8"),
5151
Item(d)
5252
)
5353
}
@@ -64,6 +64,27 @@ class CommentsRsListRowTest {
6464
assertThat(afterHeader).isEqualTo(beforeHeader)
6565
}
6666

67+
@Test
68+
fun `a label that recurs non-contiguously gets distinct header keys instead of crashing`() {
69+
// Defensive: comments are normally date-sorted so a label is one contiguous group, but if
70+
// the list ever arrives out of order two groups can share a label. LazyColumn rejects
71+
// duplicate keys with a crash, so each header must still get a unique key.
72+
val rows = withDateHeaders(
73+
listOf(
74+
comment(id = 1, date = "Today"),
75+
comment(id = 2, date = "Yesterday"),
76+
comment(id = 3, date = "Today")
77+
)
78+
)
79+
80+
val headers = rows.filterIsInstance<DateHeader>()
81+
assertThat(headers.map { it.label }).containsExactly("Today", "Yesterday", "Today")
82+
assertThat(headers.map { it.key }).doesNotHaveDuplicates()
83+
}
84+
85+
/** A header as it appears in the normal (contiguous) case: key derived directly from the label. */
86+
private fun header(label: String) = DateHeader(label, "header_$label")
87+
6788
private fun comment(id: Long, date: String) = CommentRsUiModel(
6889
remoteCommentId = id,
6990
authorName = "Jane",

0 commit comments

Comments
 (0)