From eb6276e09d1e08b6e0b4babf5112f70ceb5e62f2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 12 Apr 2023 09:14:55 +0200 Subject: [PATCH] Improve asXId and make tests pass in release and debug mode. --- .../libraries/matrix/api/core/EventId.kt | 8 +++-- .../libraries/matrix/api/core/RoomId.kt | 8 +++-- .../libraries/matrix/api/core/SessionId.kt | 8 +++-- .../libraries/matrix/api/core/SpaceId.kt | 8 +++-- .../libraries/matrix/api/core/ThreadId.kt | 10 ++++-- .../libraries/matrix/api/core/UserId.kt | 8 +++-- .../pushproviders/firebase/build.gradle.kts | 1 + .../firebase/FirebaseNewTokenHandler.kt | 2 +- .../firebase/FirebasePushParserTest.kt | 10 +++--- .../unifiedpush/build.gradle.kts | 1 + .../unifiedpush/UnifiedPushParserTest.kt | 10 +++--- .../impl/DefaultUserPushStoreFactory.kt | 2 +- settings.gradle.kts | 1 + tests/testutils/build.gradle.kts | 36 +++++++++++++++++++ .../android/tests/testutils/NullOrThrow.kt | 33 +++++++++++++++++ 15 files changed, 121 insertions(+), 25 deletions(-) create mode 100644 tests/testutils/build.gradle.kts create mode 100644 tests/testutils/src/main/kotlin/io/element/android/tests/testutils/NullOrThrow.kt diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/EventId.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/EventId.kt index ffd5bb8ea2..b24d886ee8 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/EventId.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/EventId.kt @@ -22,8 +22,12 @@ import java.io.Serializable @JvmInline value class EventId(val value: String) : Serializable -fun String.asEventId() = EventId(this).also { - if (BuildConfig.DEBUG && !MatrixPatterns.isEventId(this)) { +fun String.asEventId() = if (MatrixPatterns.isEventId(this)) { + EventId(this) +} else { + if (BuildConfig.DEBUG) { error("`$this` is not a valid event Id") + } else { + null } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/RoomId.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/RoomId.kt index f711723c3f..f71f4ba4f9 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/RoomId.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/RoomId.kt @@ -22,8 +22,12 @@ import java.io.Serializable @JvmInline value class RoomId(val value: String) : Serializable -fun String.asRoomId() = RoomId(this).also { - if (BuildConfig.DEBUG && !MatrixPatterns.isRoomId(this)) { +fun String.asRoomId() = if (MatrixPatterns.isRoomId(this)) { + RoomId(this) +} else { + if (BuildConfig.DEBUG) { error("`$this` is not a valid room Id") + } else { + null } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/SessionId.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/SessionId.kt index 8591876b29..0f0edf2299 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/SessionId.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/SessionId.kt @@ -20,8 +20,12 @@ import io.element.android.libraries.matrix.api.BuildConfig typealias SessionId = UserId -fun String.asSessionId() = SessionId(this).also { - if (BuildConfig.DEBUG && !MatrixPatterns.isSessionId(this)) { +fun String.asSessionId() = if (MatrixPatterns.isSessionId(this)) { + SessionId(this) +} else { + if (BuildConfig.DEBUG) { error("`$this` is not a valid session Id") + } else { + null } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/SpaceId.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/SpaceId.kt index 1b8b33426b..d4f2e43be6 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/SpaceId.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/SpaceId.kt @@ -27,8 +27,12 @@ value class SpaceId(val value: String) : Serializable */ val MAIN_SPACE = SpaceId("!mainSpace") -fun String.asSpaceId() = SpaceId(this).also { - if (BuildConfig.DEBUG && !MatrixPatterns.isSpaceId(this)) { +fun String.asSpaceId() = if (MatrixPatterns.isSpaceId(this)) { + SpaceId(this) +} else { + if (BuildConfig.DEBUG) { error("`$this` is not a valid space Id") + } else { + null } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/ThreadId.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/ThreadId.kt index f95c33bad3..f57cb8fa23 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/ThreadId.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/ThreadId.kt @@ -22,8 +22,12 @@ import java.io.Serializable @JvmInline value class ThreadId(val value: String) : Serializable -fun String.asThreadId() = ThreadId(this).also { - if (BuildConfig.DEBUG && !MatrixPatterns.isThreadId(this)) { - error("`$this` is not a valid Thread Id") +fun String.asThreadId() = if (MatrixPatterns.isThreadId(this)) { + ThreadId(this) +} else { + if (BuildConfig.DEBUG) { + error("`$this` is not a valid thread Id") + } else { + null } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/UserId.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/UserId.kt index 91f9c6f11c..ba7028c926 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/UserId.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/core/UserId.kt @@ -22,8 +22,12 @@ import java.io.Serializable @JvmInline value class UserId(val value: String) : Serializable -fun String.asUserId() = UserId(this).also { - if (BuildConfig.DEBUG && !MatrixPatterns.isUserId(this)) { +fun String.asUserId() = if (MatrixPatterns.isUserId(this)) { + UserId(this) +} else { + if (BuildConfig.DEBUG) { error("`$this` is not a valid user Id") + } else { + null } } diff --git a/libraries/pushproviders/firebase/build.gradle.kts b/libraries/pushproviders/firebase/build.gradle.kts index eabc70305b..a20398319b 100644 --- a/libraries/pushproviders/firebase/build.gradle.kts +++ b/libraries/pushproviders/firebase/build.gradle.kts @@ -45,4 +45,5 @@ dependencies { testImplementation(libs.test.junit) testImplementation(libs.test.truth) testImplementation(projects.libraries.matrix.test) + testImplementation(projects.tests.testutils) } diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/push/providers/firebase/FirebaseNewTokenHandler.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/push/providers/firebase/FirebaseNewTokenHandler.kt index f26fcfae25..58464b5af0 100644 --- a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/push/providers/firebase/FirebaseNewTokenHandler.kt +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/push/providers/firebase/FirebaseNewTokenHandler.kt @@ -42,7 +42,7 @@ class FirebaseNewTokenHandler @Inject constructor( firebaseStore.storeFcmToken(firebaseToken) // Register the pusher for all the sessions sessionStore.getAllSessions().toUserList() - .map { it.asSessionId() } + .mapNotNull { it.asSessionId() } .forEach { userId -> val userDataStore = userPushStoreFactory.create(userId) if (userDataStore.getPushProviderName() == FirebaseConfig.name) { diff --git a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/push/providers/firebase/FirebasePushParserTest.kt b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/push/providers/firebase/FirebasePushParserTest.kt index 466a2bfedf..562aecc790 100644 --- a/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/push/providers/firebase/FirebasePushParserTest.kt +++ b/libraries/pushproviders/firebase/src/test/kotlin/io/element/android/libraries/push/providers/firebase/FirebasePushParserTest.kt @@ -20,7 +20,7 @@ import com.google.common.truth.Truth.assertThat import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.push.providers.api.PushData -import org.junit.Assert.assertThrows +import io.element.android.tests.testutils.assertNullOrThrow import org.junit.Test class FirebasePushParserTest { @@ -52,26 +52,26 @@ class FirebasePushParserTest { fun `test empty roomId`() { val pushParser = FirebasePushParser() assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", null))).isNull() - assertThrows(IllegalStateException::class.java) { pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", "")) } + assertNullOrThrow { pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", "")) } } @Test fun `test invalid roomId`() { val pushParser = FirebasePushParser() - assertThrows(IllegalStateException::class.java) { pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", "aRoomId:domain")) } + assertNullOrThrow { pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", "aRoomId:domain")) } } @Test fun `test empty eventId`() { val pushParser = FirebasePushParser() assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", null))).isNull() - assertThrows(IllegalStateException::class.java) { pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", "")) } + assertNullOrThrow { pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", "")) } } @Test fun `test invalid eventId`() { val pushParser = FirebasePushParser() - assertThrows(IllegalStateException::class.java) { pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", "anEventId")) } + assertNullOrThrow { pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", "anEventId")) } } companion object { diff --git a/libraries/pushproviders/unifiedpush/build.gradle.kts b/libraries/pushproviders/unifiedpush/build.gradle.kts index 7dcd773c25..3546bb16e1 100644 --- a/libraries/pushproviders/unifiedpush/build.gradle.kts +++ b/libraries/pushproviders/unifiedpush/build.gradle.kts @@ -53,4 +53,5 @@ dependencies { testImplementation(libs.test.junit) testImplementation(libs.test.truth) testImplementation(projects.libraries.matrix.test) + testImplementation(projects.tests.testutils) } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/push/providers/unifiedpush/UnifiedPushParserTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/push/providers/unifiedpush/UnifiedPushParserTest.kt index b18be39e07..19231505cc 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/push/providers/unifiedpush/UnifiedPushParserTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/push/providers/unifiedpush/UnifiedPushParserTest.kt @@ -20,7 +20,7 @@ import com.google.common.truth.Truth.assertThat import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.push.providers.api.PushData -import org.junit.Assert.assertThrows +import io.element.android.tests.testutils.assertNullOrThrow import org.junit.Test class UnifiedPushParserTest { @@ -52,7 +52,7 @@ class UnifiedPushParserTest { @Test fun `test empty roomId`() { val pushParser = UnifiedPushParser() - assertThrows(IllegalStateException::class.java) { + assertNullOrThrow { pushParser.parse(UNIFIED_PUSH_DATA.replace(A_ROOM_ID.value, "").toByteArray(), aClientSecret) } } @@ -60,7 +60,7 @@ class UnifiedPushParserTest { @Test fun `test invalid roomId`() { val pushParser = UnifiedPushParser() - assertThrows(IllegalStateException::class.java) { + assertNullOrThrow { pushParser.parse(UNIFIED_PUSH_DATA.mutate(A_ROOM_ID.value, "aRoomId:domain"), aClientSecret) } } @@ -68,7 +68,7 @@ class UnifiedPushParserTest { @Test fun `test empty eventId`() { val pushParser = UnifiedPushParser() - assertThrows(IllegalStateException::class.java) { + assertNullOrThrow { pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, ""), aClientSecret) } } @@ -76,7 +76,7 @@ class UnifiedPushParserTest { @Test fun `test invalid eventId`() { val pushParser = UnifiedPushParser() - assertThrows(IllegalStateException::class.java) { + assertNullOrThrow { pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, "anEventId"), aClientSecret) } } diff --git a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt index e167f5294a..ed32dba472 100644 --- a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt +++ b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt @@ -60,6 +60,6 @@ class DefaultUserPushStoreFactory @Inject constructor( override suspend fun onSessionDeleted(userId: String) { // Delete the store - create(userId.asSessionId()).reset() + userId.asSessionId()?.let { create(it).reset() } } } diff --git a/settings.gradle.kts b/settings.gradle.kts index 7429f80b43..1173288adb 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -49,6 +49,7 @@ rootProject.name = "ElementX" include(":app") include(":appnav") include(":tests:uitests") +include(":tests:testutils") include(":anvilannotations") include(":anvilcodegen") diff --git a/tests/testutils/build.gradle.kts b/tests/testutils/build.gradle.kts new file mode 100644 index 0000000000..0c28da1f06 --- /dev/null +++ b/tests/testutils/build.gradle.kts @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * 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. + */ + +// TODO: Remove once https://youtrack.jetbrains.com/issue/KTIJ-19369 is fixed +@Suppress("DSL_SCOPE_VIOLATION") +plugins { + id("io.element.android-library") + alias(libs.plugins.ksp) +} + +android { + namespace = "io.element.android.tests.testutils" +} + +dependencies { + implementation(libs.test.junit) + implementation(libs.test.mockk) + implementation(libs.test.truth) + implementation(libs.test.turbine) + implementation(libs.coroutines.test) + implementation(projects.libraries.matrix.test) + implementation(projects.services.appnavstate.test) +} diff --git a/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/NullOrThrow.kt b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/NullOrThrow.kt new file mode 100644 index 0000000000..adfd58da5f --- /dev/null +++ b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/NullOrThrow.kt @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * 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 io.element.android.tests.testutils + +import com.google.common.truth.Truth.assertThat +import org.junit.Assert.assertThrows + +/** + * Assert that the lambda throws on debug and returns null on release. + */ +fun assertNullOrThrow(lambda: () -> Any?) { + if (BuildConfig.DEBUG) { + assertThrows(IllegalStateException::class.java) { + lambda() + } + } else { + assertThat(lambda()).isNull() + } +}