From 9866bfefed0a03c7aef8281287bb41989b423104 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 28 Jun 2022 11:56:41 +0200 Subject: [PATCH 1/4] Ensure clearWith lambda is deleting all the list item, else we will get an infinite loop. This specific error will help to figure out what is happening. --- .../matrix/android/sdk/internal/extensions/RealmExtensions.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt index 00cbe0aa85..9bc727541d 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt @@ -30,7 +30,11 @@ internal fun RealmObject.assertIsManaged() { */ internal fun RealmList.clearWith(delete: (T) -> Unit) { while (!isEmpty()) { + val previousSize = size first()?.let { delete.invoke(it) } + if (previousSize != size + 1) { + error("`clearWith` MUST delete all elements of the RealmList") + } } } From 98df2d82db1cb7851d88c5380de69947637c5814 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 28 Jun 2022 12:03:34 +0200 Subject: [PATCH 2/4] Changelog --- changelog.d/6392.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6392.misc diff --git a/changelog.d/6392.misc b/changelog.d/6392.misc new file mode 100644 index 0000000000..c7b95917c3 --- /dev/null +++ b/changelog.d/6392.misc @@ -0,0 +1 @@ +Ensure `RealmList.clearWith()` extension is correctly used. From a0025bc99b6a499f695f326ac8926e26375f5e04 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 29 Jun 2022 15:03:00 +0200 Subject: [PATCH 3/4] Update after PR review. --- .../internal/extensions/RealmExtensions.kt | 18 +++++++---- .../matrix/android/sdk/internal/util/fatal.kt | 32 +++++++++++++++++++ 2 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/fatal.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt index 9bc727541d..1e453d2d3e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt @@ -20,6 +20,7 @@ import io.realm.RealmList import io.realm.RealmObject import io.realm.RealmObjectSchema import org.matrix.android.sdk.internal.database.model.HomeServerCapabilitiesEntityFields +import org.matrix.android.sdk.internal.util.fatalError internal fun RealmObject.assertIsManaged() { check(isManaged) { "${javaClass.simpleName} entity should be managed to use this function" } @@ -27,14 +28,19 @@ internal fun RealmObject.assertIsManaged() { /** * Clear a RealmList by deleting all its items calling the provided lambda. + * The lambda is supposed to delete the item, which means that after this operation, the list will be empty. */ internal fun RealmList.clearWith(delete: (T) -> Unit) { - while (!isEmpty()) { - val previousSize = size - first()?.let { delete.invoke(it) } - if (previousSize != size + 1) { - error("`clearWith` MUST delete all elements of the RealmList") - } + map { item -> + // Create a lambda for all items of the list + { delete(item) } + }.forEach { lambda -> + // Then invoke all the lambda + lambda.invoke() + } + + if (!isEmpty()) { + fatalError("`clearWith` MUST delete all elements of the RealmList") } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/fatal.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/fatal.kt new file mode 100644 index 0000000000..323eee0b1c --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/util/fatal.kt @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2022 The Matrix.org Foundation C.I.C. + * + * 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 org.matrix.android.sdk.internal.util + +import org.matrix.android.sdk.BuildConfig +import timber.log.Timber + +/** + * Throws in debug, only log in production. + * As this method does not always throw, next statement should be a return. +*/ +internal fun fatalError(message: String) { + if (BuildConfig.DEBUG) { + error(message) + } else { + Timber.e(message) + } +} From e53dd1e1a1c6670607d10158b16bbb65b4c7d8da Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 30 Jun 2022 14:50:20 +0200 Subject: [PATCH 4/4] Improve readability. --- .../matrix/android/sdk/internal/extensions/RealmExtensions.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt index 1e453d2d3e..c6ea2bc7bd 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/extensions/RealmExtensions.kt @@ -39,7 +39,7 @@ internal fun RealmList.clearWith(delete: (T) -> Unit) { lambda.invoke() } - if (!isEmpty()) { + if (isNotEmpty()) { fatalError("`clearWith` MUST delete all elements of the RealmList") } }