From 3b0a5658224c195e82028b8c223510bcf3c7e9c1 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Mon, 7 Mar 2022 16:51:40 +0100 Subject: [PATCH 01/28] Changes room hierarchy endpoint --- .../org/matrix/android/sdk/internal/session/space/SpaceApi.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt index edd10bc2ef..3d5fc153bf 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt @@ -31,7 +31,7 @@ internal interface SpaceApi { * @param from: Optional. Pagination token given to retrieve the next set of rooms. * Note that if a pagination token is provided, then the parameters given for suggested_only and max_depth must be the same. */ - @GET(NetworkConstants.URI_API_PREFIX_PATH_UNSTABLE + "org.matrix.msc2946/rooms/{roomId}/hierarchy") + @GET(NetworkConstants.URI_API_PREFIX_PATH_ + "rooms/{roomID}/hierarchy") suspend fun getSpaceHierarchy( @Path("roomId") spaceId: String, @Query("suggested_only") suggestedOnly: Boolean?, From bc3b8d0a16fa1e77d41716cbe1348df13da0aa9b Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 11:51:17 +0100 Subject: [PATCH 02/28] Adds testing for fallback api --- .../sdk/internal/network/NetworkConstants.kt | 1 + .../session/space/ResolveSpaceInfoTask.kt | 18 ++++++++++++------ .../sdk/internal/session/space/SpaceApi.kt | 13 ++++++++++++- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt index 1ab1042129..2d567f273d 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt @@ -21,6 +21,7 @@ internal object NetworkConstants { private const val URI_API_PREFIX_PATH = "_matrix/client" const val URI_API_PREFIX_PATH_ = "$URI_API_PREFIX_PATH/" const val URI_API_PREFIX_PATH_R0 = "$URI_API_PREFIX_PATH/r0/" + const val URI_API_PREFIX_PATH_V1 = "${URI_API_PREFIX_PATH}v1/" const val URI_API_PREFIX_PATH_UNSTABLE = "$URI_API_PREFIX_PATH/unstable/" // Media diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 2a396d6ee7..7c127a4ac3 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.session.space import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.task.Task +import timber.log.Timber import javax.inject.Inject internal interface ResolveSpaceInfoTask : Task { @@ -38,12 +39,17 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( ) : ResolveSpaceInfoTask { override suspend fun execute(params: ResolveSpaceInfoTask.Params): SpacesResponse { return executeRequest(globalErrorReceiver) { - spaceApi.getSpaceHierarchy( - spaceId = params.spaceId, - suggestedOnly = params.suggestedOnly, - limit = params.limit, - maxDepth = params.maxDepth, - from = params.from) + try { + throw RuntimeException("Test space task exception") + } catch (e: Throwable) { + Timber.i("Test fall back api") + spaceApi.getSpaceHierarchy( + spaceId = params.spaceId, + suggestedOnly = params.suggestedOnly, + limit = params.limit, + maxDepth = params.maxDepth, + from = params.from) + } } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt index 3d5fc153bf..164df6f8b8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt @@ -31,11 +31,22 @@ internal interface SpaceApi { * @param from: Optional. Pagination token given to retrieve the next set of rooms. * Note that if a pagination token is provided, then the parameters given for suggested_only and max_depth must be the same. */ - @GET(NetworkConstants.URI_API_PREFIX_PATH_ + "rooms/{roomID}/hierarchy") + @GET(NetworkConstants.URI_API_PREFIX_PATH_V1 + "rooms/{roomID}/hierarchy") suspend fun getSpaceHierarchy( @Path("roomId") spaceId: String, @Query("suggested_only") suggestedOnly: Boolean?, @Query("limit") limit: Int?, @Query("max_depth") maxDepth: Int?, @Query("from") from: String?): SpacesResponse + + /** + * Unstable version of [getSpaceHierarchy] + */ + @GET(NetworkConstants.URI_API_PREFIX_PATH_UNSTABLE + "org.matrix.msc2946/rooms/{roomId}/hierarchy") + suspend fun getSpaceHierarchyUnstable( + @Path("roomId") spaceId: String, + @Query("suggested_only") suggestedOnly: Boolean?, + @Query("limit") limit: Int?, + @Query("max_depth") maxDepth: Int?, + @Query("from") from: String?): SpacesResponse } From 0af6ae6075d73d595db470392a09526d8f4b8b0a Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 13:10:18 +0100 Subject: [PATCH 03/28] Adds logic for using stable and unstable hierarchy endpoints --- .../session/space/ResolveSpaceInfoTask.kt | 41 ++++++++++----- .../space/DefaultResolveSpaceInfoTaskTest.kt | 51 +++++++++++++++++++ .../android/sdk/test/fakes/FakeSpaceApi.kt | 47 +++++++++++++++++ .../ResolveSpaceInfoTaskParamsFixture.kt | 35 +++++++++++++ .../test/fixtures/SpacesResponseFixture.kt | 30 +++++++++++ 5 files changed, 191 insertions(+), 13 deletions(-) create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt create mode 100644 matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 7c127a4ac3..6c9a5c7101 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -19,7 +19,7 @@ package org.matrix.android.sdk.internal.session.space import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.task.Task -import timber.log.Timber +import retrofit2.HttpException import javax.inject.Inject internal interface ResolveSpaceInfoTask : Task { @@ -29,7 +29,6 @@ internal interface ResolveSpaceInfoTask : Task(500, "".toResponseBody()) + coEvery { instance.getSpaceHierarchy(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } throws HttpException(errorResponse) + } + + fun givenUnstableEndpointWorks() { + coEvery { instance.getSpaceHierarchyUnstable(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } returns response + } +} diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt new file mode 100644 index 0000000000..183d26caef --- /dev/null +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2022 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 org.matrix.android.sdk.test.fixtures + +import org.matrix.android.sdk.internal.session.space.ResolveSpaceInfoTask + +internal object ResolveSpaceInfoTaskParamsFixture { + fun aResolveSpaceInfoTaskParams( + spaceId: String = "", + limit: Int? = null, + maxDepth: Int? = null, + from: String? = null, + suggestedOnly: Boolean? = null, + ) = ResolveSpaceInfoTask.Params( + spaceId, + limit, + maxDepth, + from, + suggestedOnly, + ) +} diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt new file mode 100644 index 0000000000..3149d13e7d --- /dev/null +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt @@ -0,0 +1,30 @@ +/* + * Copyright (c) 2022 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 org.matrix.android.sdk.test.fixtures + +import org.matrix.android.sdk.internal.session.space.SpaceChildSummaryResponse +import org.matrix.android.sdk.internal.session.space.SpacesResponse + +internal object SpacesResponseFixture { + fun aSpacesResponse( + nextBatch: String? = null, + rooms: List? = null, + ) = SpacesResponse( + nextBatch, + rooms, + ) +} From e5299d716c8d78231bbfce4bb02904f5f58a3c4d Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 13:15:26 +0100 Subject: [PATCH 04/28] Fixes legal comments --- .../internal/session/space/DefaultResolveSpaceInfoTaskTest.kt | 2 +- .../test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt | 2 +- .../sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt | 2 +- .../matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt index c92b9a5ed2..d23280121c 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 New Vector Ltd + * Copyright 2021 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. diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt index 81f486dbb3..26ee295236 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 New Vector Ltd + * Copyright 2021 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. diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt index 183d26caef..28f8c3637d 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 New Vector Ltd + * Copyright 2021 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. diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt index 3149d13e7d..0a08331114 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 New Vector Ltd + * Copyright 2021 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. From eb46067c0836ed0e6b322eb9ee3c8ca486e2bc66 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 15:40:13 +0100 Subject: [PATCH 05/28] Changes caught exception type to Throwable --- .../android/sdk/internal/session/space/ResolveSpaceInfoTask.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 6c9a5c7101..305bc5db04 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -48,7 +48,7 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( private suspend fun getSpaceHierarchy() = try { getStableSpaceHierarchy() - } catch (e: HttpException) { + } catch (e: Throwable) { getUnstableSpaceHierarchy() } From 82b5fc9557cbd775a5eefe20c9dcfb46304b7e52 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 16:50:49 +0100 Subject: [PATCH 06/28] Removes unused imports --- .../sdk/internal/session/space/ResolveSpaceInfoTask.kt | 1 - .../java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt | 7 +------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 305bc5db04..16a3511e32 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -19,7 +19,6 @@ package org.matrix.android.sdk.internal.session.space import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.task.Task -import retrofit2.HttpException import javax.inject.Inject internal interface ResolveSpaceInfoTask : Task { diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt index 26ee295236..b851e15575 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt @@ -18,13 +18,9 @@ package org.matrix.android.sdk.test.fakes import io.mockk.coEvery import io.mockk.mockk -import okhttp3.ResponseBody.Companion.toResponseBody import org.matrix.android.sdk.internal.session.space.SpaceApi -import org.matrix.android.sdk.internal.session.space.SpacesResponse import org.matrix.android.sdk.test.fixtures.ResolveSpaceInfoTaskParamsFixture import org.matrix.android.sdk.test.fixtures.SpacesResponseFixture -import retrofit2.HttpException -import retrofit2.Response internal class FakeSpaceApi { @@ -37,8 +33,7 @@ internal class FakeSpaceApi { } fun givenStableEndpointFails() { - val errorResponse = Response.error(500, "".toResponseBody()) - coEvery { instance.getSpaceHierarchy(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } throws HttpException(errorResponse) + coEvery { instance.getSpaceHierarchy(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } throws Exception() } fun givenUnstableEndpointWorks() { From 510206aa8af8fd4bde6994eac4a408588412cba9 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 16:54:05 +0100 Subject: [PATCH 07/28] Adds changelog file --- changelog.d/5443.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5443.misc diff --git a/changelog.d/5443.misc b/changelog.d/5443.misc new file mode 100644 index 0000000000..f9fd715403 --- /dev/null +++ b/changelog.d/5443.misc @@ -0,0 +1 @@ +Adds stable room hierarchy endpoint with a fallback to the unstable one From 0892525c84045a2df912949c7bbc4df6b61f5468 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 17:19:11 +0100 Subject: [PATCH 08/28] Adds debug logs --- .../internal/session/space/ResolveSpaceInfoTask.kt | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 16a3511e32..9b0adad54b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.session.space import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.task.Task +import timber.log.Timber import javax.inject.Inject internal interface ResolveSpaceInfoTask : Task { @@ -39,10 +40,12 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( private lateinit var params: ResolveSpaceInfoTask.Params override suspend fun execute(params: ResolveSpaceInfoTask.Params): SpacesResponse { - return executeRequest(globalErrorReceiver) { + val result = executeRequest(globalErrorReceiver) { this.params = params getSpaceHierarchy() } + Timber.w("Space Info result: $result") + return result } private suspend fun getSpaceHierarchy() = try { @@ -57,7 +60,9 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( suggestedOnly = params.suggestedOnly, limit = params.limit, maxDepth = params.maxDepth, - from = params.from) + from = params.from).also { + Timber.w("Spaces response stable: $it") + } private suspend fun getUnstableSpaceHierarchy() = spaceApi.getSpaceHierarchyUnstable( @@ -65,5 +70,7 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( suggestedOnly = params.suggestedOnly, limit = params.limit, maxDepth = params.maxDepth, - from = params.from) + from = params.from).also { + Timber.w("Spaces response unstable: $it") + } } From 54828f76cf900ba5bc9064b8db13939ded06c1c2 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 17:26:01 +0100 Subject: [PATCH 09/28] Adds slash to v1 prefix path --- .../org/matrix/android/sdk/internal/network/NetworkConstants.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt index 2d567f273d..21bba0f535 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt @@ -21,7 +21,7 @@ internal object NetworkConstants { private const val URI_API_PREFIX_PATH = "_matrix/client" const val URI_API_PREFIX_PATH_ = "$URI_API_PREFIX_PATH/" const val URI_API_PREFIX_PATH_R0 = "$URI_API_PREFIX_PATH/r0/" - const val URI_API_PREFIX_PATH_V1 = "${URI_API_PREFIX_PATH}v1/" + const val URI_API_PREFIX_PATH_V1 = "${URI_API_PREFIX_PATH}/v1/" const val URI_API_PREFIX_PATH_UNSTABLE = "$URI_API_PREFIX_PATH/unstable/" // Media From 31f300c724494569989c85330c82ef5c8c5d1408 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Tue, 8 Mar 2022 21:32:13 +0100 Subject: [PATCH 10/28] Adds error print stack trace --- .../android/sdk/internal/session/space/ResolveSpaceInfoTask.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 9b0adad54b..723f640edc 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -51,6 +51,8 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( private suspend fun getSpaceHierarchy() = try { getStableSpaceHierarchy() } catch (e: Throwable) { + Timber.w("Stable space hierarchy failed: ${e.message}") + e.printStackTrace() getUnstableSpaceHierarchy() } From fb374b737494ee7f762789b5d20dd2d67e58b96c Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 9 Mar 2022 09:57:20 +0100 Subject: [PATCH 11/28] Fixes wrong path parameter in getSpaceHierarchy --- .../org/matrix/android/sdk/internal/session/space/SpaceApi.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt index 164df6f8b8..fda9b4b5bc 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt @@ -31,7 +31,7 @@ internal interface SpaceApi { * @param from: Optional. Pagination token given to retrieve the next set of rooms. * Note that if a pagination token is provided, then the parameters given for suggested_only and max_depth must be the same. */ - @GET(NetworkConstants.URI_API_PREFIX_PATH_V1 + "rooms/{roomID}/hierarchy") + @GET(NetworkConstants.URI_API_PREFIX_PATH_V1 + "rooms/{roomId}/hierarchy") suspend fun getSpaceHierarchy( @Path("roomId") spaceId: String, @Query("suggested_only") suggestedOnly: Boolean?, From 63cd79dc4f84f30fcbdae625a6c0ad287e6e543a Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 9 Mar 2022 10:53:18 +0100 Subject: [PATCH 12/28] Removes debug logs --- .../sdk/internal/network/NetworkConstants.kt | 2 +- .../session/space/ResolveSpaceInfoTask.kt | 21 +++++-------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt index 21bba0f535..5aec7db66c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt @@ -21,7 +21,7 @@ internal object NetworkConstants { private const val URI_API_PREFIX_PATH = "_matrix/client" const val URI_API_PREFIX_PATH_ = "$URI_API_PREFIX_PATH/" const val URI_API_PREFIX_PATH_R0 = "$URI_API_PREFIX_PATH/r0/" - const val URI_API_PREFIX_PATH_V1 = "${URI_API_PREFIX_PATH}/v1/" + const val URI_API_PREFIX_PATH_V1 = "$URI_API_PREFIX_PATH/v1/" const val URI_API_PREFIX_PATH_UNSTABLE = "$URI_API_PREFIX_PATH/unstable/" // Media diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 723f640edc..003148cf17 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -19,7 +19,6 @@ package org.matrix.android.sdk.internal.session.space import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.task.Task -import timber.log.Timber import javax.inject.Inject internal interface ResolveSpaceInfoTask : Task { @@ -39,20 +38,14 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( private lateinit var params: ResolveSpaceInfoTask.Params - override suspend fun execute(params: ResolveSpaceInfoTask.Params): SpacesResponse { - val result = executeRequest(globalErrorReceiver) { - this.params = params - getSpaceHierarchy() - } - Timber.w("Space Info result: $result") - return result + override suspend fun execute(params: ResolveSpaceInfoTask.Params) = executeRequest(globalErrorReceiver) { + this.params = params + getSpaceHierarchy() } private suspend fun getSpaceHierarchy() = try { getStableSpaceHierarchy() } catch (e: Throwable) { - Timber.w("Stable space hierarchy failed: ${e.message}") - e.printStackTrace() getUnstableSpaceHierarchy() } @@ -62,9 +55,7 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( suggestedOnly = params.suggestedOnly, limit = params.limit, maxDepth = params.maxDepth, - from = params.from).also { - Timber.w("Spaces response stable: $it") - } + from = params.from) private suspend fun getUnstableSpaceHierarchy() = spaceApi.getSpaceHierarchyUnstable( @@ -72,7 +63,5 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( suggestedOnly = params.suggestedOnly, limit = params.limit, maxDepth = params.maxDepth, - from = params.from).also { - Timber.w("Spaces response unstable: $it") - } + from = params.from) } From 047e767f34c09ddf473a7dc918d1685f69cf5ebc Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 9 Mar 2022 12:18:46 +0100 Subject: [PATCH 13/28] Adds coroutinesTest to matrix sdk gradle --- matrix-sdk-android/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/build.gradle b/matrix-sdk-android/build.gradle index 3e301eebb9..32a8b23f30 100644 --- a/matrix-sdk-android/build.gradle +++ b/matrix-sdk-android/build.gradle @@ -175,7 +175,7 @@ dependencies { // Note: version sticks to 1.9.2 due to https://github.com/mockk/mockk/issues/281 testImplementation libs.mockk.mockk testImplementation libs.tests.kluent - implementation libs.jetbrains.coroutinesAndroid + testImplementation libs.jetbrains.coroutinesTest // Plant Timber tree for test testImplementation 'net.lachlanmckee:timber-junit-rule:1.0.1' // Transitively required for mocking realm as monarchy doesn't expose Rx From bbc6e8bbcea51904937f5420d66e59b0df4379f2 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 9 Mar 2022 16:41:18 +0100 Subject: [PATCH 14/28] Replaces caught Exception with HttpException --- .../session/space/ResolveSpaceInfoTask.kt | 3 ++- .../space/DefaultResolveSpaceInfoTaskTest.kt | 21 +++++++++++++------ .../android/sdk/test/fakes/FakeSpaceApi.kt | 11 +++++----- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 003148cf17..47c3f199b8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.session.space import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.task.Task +import retrofit2.HttpException import javax.inject.Inject internal interface ResolveSpaceInfoTask : Task { @@ -45,7 +46,7 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( private suspend fun getSpaceHierarchy() = try { getStableSpaceHierarchy() - } catch (e: Throwable) { + } catch (e: HttpException) { getUnstableSpaceHierarchy() } diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt index d23280121c..f80c0f06d0 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt @@ -18,13 +18,17 @@ package org.matrix.android.sdk.internal.session.space import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runBlockingTest +import okhttp3.ResponseBody.Companion.toResponseBody import org.amshove.kluent.shouldBeEqualTo import org.junit.Test import org.matrix.android.sdk.test.fakes.FakeGlobalErrorReceiver import org.matrix.android.sdk.test.fakes.FakeSpaceApi +import org.matrix.android.sdk.test.fixtures.SpacesResponseFixture.aSpacesResponse +import retrofit2.HttpException +import retrofit2.Response @ExperimentalCoroutinesApi -class DefaultResolveSpaceInfoTaskTest { +internal class DefaultResolveSpaceInfoTaskTest { private val spaceApi = FakeSpaceApi() private val globalErrorReceiver = FakeGlobalErrorReceiver() @@ -32,20 +36,25 @@ class DefaultResolveSpaceInfoTaskTest { @Test fun `given stable endpoint works, when execute, then return stable api data`() = runBlockingTest { - spaceApi.givenStableEndpointWorks() + spaceApi.givenStableEndpointReturns(response) val result = resolveSpaceInfoTask.execute(spaceApi.params) - result shouldBeEqualTo spaceApi.response + result shouldBeEqualTo response } @Test fun `given stable endpoint fails, when execute, then fallback to unstable endpoint`() = runBlockingTest { - spaceApi.givenStableEndpointFails() - spaceApi.givenUnstableEndpointWorks() + spaceApi.givenStableEndpointThrows(httpException) + spaceApi.givenUnstableEndpointReturns(response) val result = resolveSpaceInfoTask.execute(spaceApi.params) - result shouldBeEqualTo spaceApi.response + result shouldBeEqualTo response + } + + companion object { + private val response = aSpacesResponse() + private val httpException = HttpException(Response.error(500, "".toResponseBody())) } } diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt index b851e15575..d4fc986791 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt @@ -19,24 +19,23 @@ package org.matrix.android.sdk.test.fakes import io.mockk.coEvery import io.mockk.mockk import org.matrix.android.sdk.internal.session.space.SpaceApi +import org.matrix.android.sdk.internal.session.space.SpacesResponse import org.matrix.android.sdk.test.fixtures.ResolveSpaceInfoTaskParamsFixture -import org.matrix.android.sdk.test.fixtures.SpacesResponseFixture internal class FakeSpaceApi { val instance: SpaceApi = mockk() val params = ResolveSpaceInfoTaskParamsFixture.aResolveSpaceInfoTaskParams() - val response = SpacesResponseFixture.aSpacesResponse() - fun givenStableEndpointWorks() { + fun givenStableEndpointReturns(response: SpacesResponse) { coEvery { instance.getSpaceHierarchy(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } returns response } - fun givenStableEndpointFails() { - coEvery { instance.getSpaceHierarchy(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } throws Exception() + fun givenStableEndpointThrows(throwable: Throwable) { + coEvery { instance.getSpaceHierarchy(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } throws throwable } - fun givenUnstableEndpointWorks() { + fun givenUnstableEndpointReturns(response: SpacesResponse) { coEvery { instance.getSpaceHierarchyUnstable(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } returns response } } From f76f73f8ada91089e0e03e82b874a69b90cab5dd Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 9 Mar 2022 18:36:56 +0100 Subject: [PATCH 15/28] Refactors DefaultSpaceService querySpaceChildren --- .../session/space/DefaultSpaceService.kt | 166 +++++++++++------- .../space/SpaceChildSummaryResponse.kt | 2 +- 2 files changed, 100 insertions(+), 68 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt index c18055e089..fa5f356b14 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt @@ -113,71 +113,107 @@ internal class DefaultSpaceService @Inject constructor( return peekSpaceTask.execute(PeekSpaceTask.Params(spaceId)) } - override suspend fun querySpaceChildren(spaceId: String, - suggestedOnly: Boolean?, - limit: Int?, - from: String?, - knownStateList: List?): SpaceHierarchyData { - return resolveSpaceInfoTask.execute( - ResolveSpaceInfoTask.Params( - spaceId = spaceId, limit = limit, maxDepth = 1, from = from, suggestedOnly = suggestedOnly - ) - ).let { response -> - val spaceDesc = response.rooms?.firstOrNull { it.roomId == spaceId } - val root = RoomSummary( - roomId = spaceDesc?.roomId ?: spaceId, - roomType = spaceDesc?.roomType, - name = spaceDesc?.name ?: "", - displayName = spaceDesc?.name ?: "", - topic = spaceDesc?.topic ?: "", - joinedMembersCount = spaceDesc?.numJoinedMembers, - avatarUrl = spaceDesc?.avatarUrl ?: "", - encryptionEventTs = null, - typingUsers = emptyList(), - isEncrypted = false, - flattenParentIds = emptyList(), - canonicalAlias = spaceDesc?.canonicalAlias, - joinRules = RoomJoinRules.PUBLIC.takeIf { spaceDesc?.worldReadable == true } - ) - val children = response.rooms - ?.filter { it.roomId != spaceId } - ?.flatMap { childSummary -> - (spaceDesc?.childrenState ?: knownStateList) - ?.filter { it.stateKey == childSummary.roomId && it.type == EventType.STATE_SPACE_CHILD } - ?.mapNotNull { childStateEv -> - // create a child entry for everytime this room is the child of a space - // beware that a room could appear then twice in this list - childStateEv.content.toModel()?.let { childStateEvContent -> - SpaceChildInfo( - childRoomId = childSummary.roomId, - isKnown = true, - roomType = childSummary.roomType, - name = childSummary.name, - topic = childSummary.topic, - avatarUrl = childSummary.avatarUrl, - order = childStateEvContent.order, -// autoJoin = childStateEvContent.autoJoin ?: false, - viaServers = childStateEvContent.via.orEmpty(), - activeMemberCount = childSummary.numJoinedMembers, - parentRoomId = childStateEv.roomId, - suggested = childStateEvContent.suggested, - canonicalAlias = childSummary.canonicalAlias, - aliases = childSummary.aliases, - worldReadable = childSummary.worldReadable - ) - } - }.orEmpty() - } - .orEmpty() - SpaceHierarchyData( - rootSummary = root, - children = children, - childrenState = spaceDesc?.childrenState.orEmpty(), - nextToken = response.nextBatch - ) - } + override suspend fun querySpaceChildren( + spaceId: String, + suggestedOnly: Boolean?, + limit: Int?, + from: String?, + knownStateList: List? + ): SpaceHierarchyData { + val spacesResponse = getSpacesResponse(spaceId, suggestedOnly, limit, from) + val spaceRootResponse = spacesResponse.getRoot(spaceId) + val spaceRoot = spaceRootResponse?.toRoomSummary() ?: createBlankRoomSummary(spaceId) + val spaceChildren = spacesResponse.rooms.mapToSpaceChildInfoList(spaceId, spaceRootResponse, knownStateList) + + return SpaceHierarchyData( + rootSummary = spaceRoot, + children = spaceChildren, + childrenState = spaceRootResponse?.childrenState.orEmpty(), + nextToken = spacesResponse.nextBatch + ) } + private suspend fun getSpacesResponse(spaceId: String, suggestedOnly: Boolean?, limit: Int?, from: String?) = + resolveSpaceInfoTask.execute( + ResolveSpaceInfoTask.Params(spaceId = spaceId, limit = limit, maxDepth = 1, from = from, suggestedOnly = suggestedOnly) + ) + + private fun SpacesResponse.getRoot(spaceId: String) = rooms?.firstOrNull { it.roomId == spaceId } + + private fun SpaceChildSummaryResponse.toRoomSummary() = RoomSummary( + roomId = roomId, + roomType = roomType, + name = name ?: "", + displayName = name ?: "", + topic = topic ?: "", + joinedMembersCount = numJoinedMembers, + avatarUrl = avatarUrl ?: "", + encryptionEventTs = null, + typingUsers = emptyList(), + isEncrypted = false, + flattenParentIds = emptyList(), + canonicalAlias = canonicalAlias, + joinRules = RoomJoinRules.PUBLIC.takeIf { isWorldReadable } + ) + + private fun createBlankRoomSummary(spaceId: String) = RoomSummary( + roomId = spaceId, + joinedMembersCount = null, + encryptionEventTs = null, + typingUsers = emptyList(), + isEncrypted = false, + flattenParentIds = emptyList(), + canonicalAlias = null, + joinRules = null + ) + + private fun List?.mapToSpaceChildInfoList( + spaceId: String, + spaceRootResponse: SpaceChildSummaryResponse?, + knownStateList: List?, + ) = this?.filterIdIsNot(spaceId) + ?.toSpaceChildInfoList(spaceRootResponse, knownStateList) + .orEmpty() + + private fun List.filterIdIsNot(spaceId: String) = filter { it.roomId != spaceId } + + private fun List.toSpaceChildInfoList( + rootRoomResponse: SpaceChildSummaryResponse?, + knownStateList: List?, + ) = flatMap { spaceChildSummary -> + (rootRoomResponse?.childrenState ?: knownStateList) + ?.filter { it.isChildOf(spaceChildSummary) } + ?.mapNotNull { childStateEvent -> childStateEvent.toSpaceChildInfo(spaceChildSummary) } + .orEmpty() + } + + private fun Event.isChildOf(space: SpaceChildSummaryResponse) = stateKey == space.roomId && type == EventType.STATE_SPACE_CHILD + + private fun Event.toSpaceChildInfo(summary: SpaceChildSummaryResponse) = content.toModel()?.let { content -> + createSpaceChildInfo(summary, this, content) + } + + private fun createSpaceChildInfo( + summary: SpaceChildSummaryResponse, + stateEvent: Event, + content: SpaceChildContent + ) = SpaceChildInfo( + childRoomId = summary.roomId, + isKnown = true, + roomType = summary.roomType, + name = summary.name, + topic = summary.topic, + avatarUrl = summary.avatarUrl, + order = content.order, + viaServers = content.via.orEmpty(), + activeMemberCount = summary.numJoinedMembers, + parentRoomId = stateEvent.roomId, + suggested = content.suggested, + canonicalAlias = summary.canonicalAlias, + aliases = summary.aliases, + worldReadable = summary.isWorldReadable + ) + override suspend fun joinSpace(spaceIdOrAlias: String, reason: String?, viaServers: List): JoinSpaceResult { @@ -192,10 +228,6 @@ internal class DefaultSpaceService @Inject constructor( leaveRoomTask.execute(LeaveRoomTask.Params(spaceId, reason)) } -// override fun getSpaceParentsOfRoom(roomId: String): List { -// return spaceSummaryDataSource.getParentsOfRoom(roomId) -// } - override suspend fun setSpaceParent(childRoomId: String, parentSpaceId: String, canonical: Boolean, viaServers: List) { // Should we perform some validation here?, // and if client want to bypass, it could use sendStateEvent directly? diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceChildSummaryResponse.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceChildSummaryResponse.kt index e0f273d0c2..b6a9c73d36 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceChildSummaryResponse.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceChildSummaryResponse.kt @@ -81,7 +81,7 @@ internal data class SpaceChildSummaryResponse( * Required. Whether the room may be viewed by guest users without joining. */ @Json(name = "world_readable") - val worldReadable: Boolean = false, + val isWorldReadable: Boolean = false, /** * Required. Whether guest users may join the room and participate in it. If they can, From 2f706d6faea9147afbad0870fa01c1339e20959b Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Wed, 9 Mar 2022 18:42:35 +0100 Subject: [PATCH 16/28] Replaces children state event room id with space id --- .../internal/session/space/DefaultSpaceService.kt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt index fa5f356b14..d00fe80de8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt @@ -172,30 +172,31 @@ internal class DefaultSpaceService @Inject constructor( spaceRootResponse: SpaceChildSummaryResponse?, knownStateList: List?, ) = this?.filterIdIsNot(spaceId) - ?.toSpaceChildInfoList(spaceRootResponse, knownStateList) + ?.toSpaceChildInfoList(spaceId, spaceRootResponse, knownStateList) .orEmpty() private fun List.filterIdIsNot(spaceId: String) = filter { it.roomId != spaceId } private fun List.toSpaceChildInfoList( + spaceId: String, rootRoomResponse: SpaceChildSummaryResponse?, knownStateList: List?, ) = flatMap { spaceChildSummary -> (rootRoomResponse?.childrenState ?: knownStateList) ?.filter { it.isChildOf(spaceChildSummary) } - ?.mapNotNull { childStateEvent -> childStateEvent.toSpaceChildInfo(spaceChildSummary) } + ?.mapNotNull { childStateEvent -> childStateEvent.toSpaceChildInfo(spaceId, spaceChildSummary) } .orEmpty() } private fun Event.isChildOf(space: SpaceChildSummaryResponse) = stateKey == space.roomId && type == EventType.STATE_SPACE_CHILD - private fun Event.toSpaceChildInfo(summary: SpaceChildSummaryResponse) = content.toModel()?.let { content -> - createSpaceChildInfo(summary, this, content) + private fun Event.toSpaceChildInfo(spaceId: String, summary: SpaceChildSummaryResponse) = content.toModel()?.let { content -> + createSpaceChildInfo(spaceId, summary, content) } private fun createSpaceChildInfo( + spaceId: String, summary: SpaceChildSummaryResponse, - stateEvent: Event, content: SpaceChildContent ) = SpaceChildInfo( childRoomId = summary.roomId, @@ -207,7 +208,7 @@ internal class DefaultSpaceService @Inject constructor( order = content.order, viaServers = content.via.orEmpty(), activeMemberCount = summary.numJoinedMembers, - parentRoomId = stateEvent.roomId, + parentRoomId = spaceId, suggested = content.suggested, canonicalAlias = summary.canonicalAlias, aliases = summary.aliases, From a891f59397b3a20a9f385b3407b7ddf08487ab87 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 10 Mar 2022 12:02:25 +0100 Subject: [PATCH 17/28] Replaces lateinit var with passing params --- .../session/space/ResolveSpaceInfoTask.kt | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 47c3f199b8..735d0904d9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -37,32 +37,31 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( private val globalErrorReceiver: GlobalErrorReceiver ) : ResolveSpaceInfoTask { - private lateinit var params: ResolveSpaceInfoTask.Params - override suspend fun execute(params: ResolveSpaceInfoTask.Params) = executeRequest(globalErrorReceiver) { - this.params = params - getSpaceHierarchy() + getSpaceHierarchy(params) } - private suspend fun getSpaceHierarchy() = try { - getStableSpaceHierarchy() + private suspend fun getSpaceHierarchy(params: ResolveSpaceInfoTask.Params) = try { + getStableSpaceHierarchy(params) } catch (e: HttpException) { - getUnstableSpaceHierarchy() + getUnstableSpaceHierarchy(params) } - private suspend fun getStableSpaceHierarchy() = + private suspend fun getStableSpaceHierarchy(params: ResolveSpaceInfoTask.Params) = spaceApi.getSpaceHierarchy( spaceId = params.spaceId, suggestedOnly = params.suggestedOnly, limit = params.limit, maxDepth = params.maxDepth, - from = params.from) + from = params.from, + ) - private suspend fun getUnstableSpaceHierarchy() = + private suspend fun getUnstableSpaceHierarchy(params: ResolveSpaceInfoTask.Params) = spaceApi.getSpaceHierarchyUnstable( spaceId = params.spaceId, suggestedOnly = params.suggestedOnly, limit = params.limit, maxDepth = params.maxDepth, - from = params.from) + from = params.from, + ) } From a5af4783ccc9ac63d2e6b7560e2c7d12f41de663 Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 10 Mar 2022 14:41:44 +0100 Subject: [PATCH 18/28] Renames mapToSpaceChildInfoList to mapSpaceChildren in DefaultSpaceService --- .../android/sdk/internal/session/space/DefaultSpaceService.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt index d00fe80de8..e764ab551a 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt @@ -123,7 +123,7 @@ internal class DefaultSpaceService @Inject constructor( val spacesResponse = getSpacesResponse(spaceId, suggestedOnly, limit, from) val spaceRootResponse = spacesResponse.getRoot(spaceId) val spaceRoot = spaceRootResponse?.toRoomSummary() ?: createBlankRoomSummary(spaceId) - val spaceChildren = spacesResponse.rooms.mapToSpaceChildInfoList(spaceId, spaceRootResponse, knownStateList) + val spaceChildren = spacesResponse.rooms.mapSpaceChildren(spaceId, spaceRootResponse, knownStateList) return SpaceHierarchyData( rootSummary = spaceRoot, @@ -167,7 +167,7 @@ internal class DefaultSpaceService @Inject constructor( joinRules = null ) - private fun List?.mapToSpaceChildInfoList( + private fun List?.mapSpaceChildren( spaceId: String, spaceRootResponse: SpaceChildSummaryResponse?, knownStateList: List?, From 23f7f72e38260303c9b8b10e29bd3735ca8bd10c Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Thu, 10 Mar 2022 15:58:00 +0000 Subject: [PATCH 19/28] Compile tests on PR, to ensure we don't break the build when merging. This was accidentally committed to develop and reverted there. --- .github/workflows/nightly.yml | 47 ----------------------------------- .github/workflows/tests.yml | 44 ++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 47 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index de97b26fbf..d8c1bb6c49 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -14,50 +14,6 @@ env: -Porg.gradle.jvmargs=-Xmx4g -Porg.gradle.parallel=false jobs: - # Build Android Tests [Matrix SDK] - build-android-test-matrix-sdk: - name: Matrix SDK - Build Android Tests - runs-on: macos-latest - # No concurrency required, runs every time on a schedule. - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v2 - with: - distribution: 'adopt' - java-version: 11 - - uses: actions/cache@v2 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} - restore-keys: | - ${{ runner.os }}-gradle- - - name: Build Android Tests for matrix-sdk-android - run: ./gradlew clean matrix-sdk-android:assembleAndroidTest $CI_GRADLE_ARG_PROPERTIES --stacktrace - - # Build Android Tests [Matrix APP] - build-android-test-app: - name: App - Build Android Tests - runs-on: macos-latest - # No concurrency required, runs every time on a schedule. - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v2 - with: - distribution: 'adopt' - java-version: 11 - - uses: actions/cache@v2 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} - restore-keys: | - ${{ runner.os }}-gradle- - - name: Build Android Tests for vector - run: ./gradlew clean vector:assembleAndroidTest $CI_GRADLE_ARG_PROPERTIES --stacktrace - # Run Android Tests integration-tests: name: Matrix SDK - Running Integration Tests @@ -367,9 +323,6 @@ jobs: needs: - integration-tests - ui-tests -# - unit-tests - - build-android-test-matrix-sdk - - build-android-test-app - sonarqube if: always() && github.event_name != 'workflow_dispatch' # No concurrency required, runs every time on a schedule. diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d739afcd30..18c225ad30 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,6 +12,50 @@ env: -Porg.gradle.parallel=false jobs: + # Build Android Tests [Matrix SDK] + build-android-test-matrix-sdk: + name: Matrix SDK - Build Android Tests + runs-on: ubuntu-latest + # No concurrency required, runs every time on a schedule. + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-java@v2 + with: + distribution: 'adopt' + java-version: 11 + - uses: actions/cache@v2 + with: + path: | + ~/.gradle/caches + ~/.gradle/wrapper + key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} + restore-keys: | + ${{ runner.os }}-gradle- + - name: Build Android Tests for matrix-sdk-android + run: ./gradlew clean matrix-sdk-android:assembleAndroidTest $CI_GRADLE_ARG_PROPERTIES --stacktrace + + # Build Android Tests [Matrix APP] + build-android-test-app: + name: App - Build Android Tests + runs-on: ubuntu-latest + # No concurrency required, runs every time on a schedule. + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-java@v2 + with: + distribution: 'adopt' + java-version: 11 + - uses: actions/cache@v2 + with: + path: | + ~/.gradle/caches + ~/.gradle/wrapper + key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} + restore-keys: | + ${{ runner.os }}-gradle- + - name: Build Android Tests for vector + run: ./gradlew clean vector:assembleAndroidTest $CI_GRADLE_ARG_PROPERTIES --stacktrace + unit-tests: name: Run Unit Tests runs-on: ubuntu-latest From 26617988f2cf040b3867863652d39c122734e4dd Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 10 Mar 2022 15:51:15 +0100 Subject: [PATCH 20/28] Add colors for shield vector drawable --- changelog.d/4860.bugfix | 1 + library/ui-styles/src/main/res/values/colors.xml | 6 ++++++ vector/src/main/res/drawable/ic_shield_black.xml | 2 +- vector/src/main/res/drawable/ic_shield_black_no_border.xml | 2 +- vector/src/main/res/drawable/ic_shield_custom.xml | 2 +- vector/src/main/res/drawable/ic_shield_trusted.xml | 2 +- .../src/main/res/drawable/ic_shield_trusted_no_border.xml | 2 +- vector/src/main/res/drawable/ic_shield_warning.xml | 2 +- .../src/main/res/drawable/ic_shield_warning_no_border.xml | 4 ++-- vector/src/main/res/drawable/ic_shield_warning_small.xml | 2 +- 10 files changed, 16 insertions(+), 9 deletions(-) create mode 100644 changelog.d/4860.bugfix diff --git a/changelog.d/4860.bugfix b/changelog.d/4860.bugfix new file mode 100644 index 0000000000..32049face4 --- /dev/null +++ b/changelog.d/4860.bugfix @@ -0,0 +1 @@ +Add colors for shield vector drawable \ No newline at end of file diff --git a/library/ui-styles/src/main/res/values/colors.xml b/library/ui-styles/src/main/res/values/colors.xml index 1b214cfc59..75b03a7d2e 100644 --- a/library/ui-styles/src/main/res/values/colors.xml +++ b/library/ui-styles/src/main/res/values/colors.xml @@ -126,4 +126,10 @@ @color/palette_prune @color/palette_prune + + + #0DBD8B + #17191C + #FF4B55 + diff --git a/vector/src/main/res/drawable/ic_shield_black.xml b/vector/src/main/res/drawable/ic_shield_black.xml index 8ab70d981b..ddde3442b7 100644 --- a/vector/src/main/res/drawable/ic_shield_black.xml +++ b/vector/src/main/res/drawable/ic_shield_black.xml @@ -6,6 +6,6 @@ diff --git a/vector/src/main/res/drawable/ic_shield_black_no_border.xml b/vector/src/main/res/drawable/ic_shield_black_no_border.xml index 1e322c77c3..f9fa5de9f6 100644 --- a/vector/src/main/res/drawable/ic_shield_black_no_border.xml +++ b/vector/src/main/res/drawable/ic_shield_black_no_border.xml @@ -4,6 +4,6 @@ android:viewportWidth="24" android:viewportHeight="24"> diff --git a/vector/src/main/res/drawable/ic_shield_custom.xml b/vector/src/main/res/drawable/ic_shield_custom.xml index ca58f028d9..aa6cbabad3 100644 --- a/vector/src/main/res/drawable/ic_shield_custom.xml +++ b/vector/src/main/res/drawable/ic_shield_custom.xml @@ -4,7 +4,7 @@ android:viewportWidth="24" android:viewportHeight="24"> diff --git a/vector/src/main/res/drawable/ic_shield_trusted.xml b/vector/src/main/res/drawable/ic_shield_trusted.xml index cac0cfcaf6..c122446a3c 100644 --- a/vector/src/main/res/drawable/ic_shield_trusted.xml +++ b/vector/src/main/res/drawable/ic_shield_trusted.xml @@ -6,7 +6,7 @@ + android:fillColor="@color/shield_color_trust"/> diff --git a/vector/src/main/res/drawable/ic_shield_warning.xml b/vector/src/main/res/drawable/ic_shield_warning.xml index ace7d7dbae..237f2aa5fd 100644 --- a/vector/src/main/res/drawable/ic_shield_warning.xml +++ b/vector/src/main/res/drawable/ic_shield_warning.xml @@ -6,7 +6,7 @@ + android:fillColor="@color/shield_color_warning"/> Date: Thu, 10 Mar 2022 16:41:00 +0000 Subject: [PATCH 21/28] Additionally notify the matrix channel if these tests fail when run after merge to develop or main. --- .github/workflows/tests.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 18c225ad30..74f31f4888 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -85,3 +85,21 @@ jobs: ( github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository ) with: files: ./**/build/test-results/**/*.xml + +# Notify the channel about runs against develop or main that have failures, as PRs should have caught these first. + notify: + runs-on: ubuntu-latest + needs: + - unit-tests + - build-android-test-matrix-sdk + - build-android-test-app + if: ${{ (github.ref == 'refs/heads/develop' || github.ref == 'refs/heads/main' ) && failure() }} + steps: + - uses: michaelkaye/matrix-hookshot-action@v0.3.0 + with: + github_token: ${{ secrets.GITHUB_TOKEN }} + matrix_access_token: ${{ secrets.ELEMENT_ANDROID_NOTIFICATION_ACCESS_TOKEN }} + matrix_room_id: ${{ secrets.ELEMENT_ANDROID_INTERNAL_ROOM_ID }} + text_template: "Build is broken for ${{ github.ref }} 🎊: {{#each job_statuses }}{{#with this }}{{#if completed }} {{name}} {{conclusion}} at {{completed_at}}, {{/if}}{{/with}}{{/each}}" + html_template: "Build is broken for ${{ github.ref }} 🎊: {{#each job_statuses }}{{#with this }}{{#if completed }}
{{icon conclusion }}{{name}} {{conclusion}} at {{completed_at}} [details]{{/if}}{{/with}}{{/each}}" + From 7226864cc954929cfc2bc6f5e932a9171702a8db Mon Sep 17 00:00:00 2001 From: ericdecanini Date: Thu, 10 Mar 2022 21:41:17 +0100 Subject: [PATCH 22/28] Improves code formatting in ResolveSpaceInfoTask --- .../internal/session/space/ResolveSpaceInfoTask.kt | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt index 735d0904d9..d59ca06c2c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt @@ -38,16 +38,14 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor( ) : ResolveSpaceInfoTask { override suspend fun execute(params: ResolveSpaceInfoTask.Params) = executeRequest(globalErrorReceiver) { - getSpaceHierarchy(params) + try { + getSpaceHierarchy(params) + } catch (e: HttpException) { + getUnstableSpaceHierarchy(params) + } } - private suspend fun getSpaceHierarchy(params: ResolveSpaceInfoTask.Params) = try { - getStableSpaceHierarchy(params) - } catch (e: HttpException) { - getUnstableSpaceHierarchy(params) - } - - private suspend fun getStableSpaceHierarchy(params: ResolveSpaceInfoTask.Params) = + private suspend fun getSpaceHierarchy(params: ResolveSpaceInfoTask.Params) = spaceApi.getSpaceHierarchy( spaceId = params.spaceId, suggestedOnly = params.suggestedOnly, From 2d5638baafedcc996dff03ee8fbfde3f9dc797d9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 10 Mar 2022 23:09:19 +0000 Subject: [PATCH 23/28] Bump libphonenumber from 8.12.44 to 8.12.45 Bumps [libphonenumber](https://github.com/google/libphonenumber) from 8.12.44 to 8.12.45. - [Release notes](https://github.com/google/libphonenumber/releases) - [Changelog](https://github.com/google/libphonenumber/blob/master/making-metadata-changes.md) - [Commits](https://github.com/google/libphonenumber/compare/v8.12.44...v8.12.45) --- updated-dependencies: - dependency-name: com.googlecode.libphonenumber:libphonenumber dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- matrix-sdk-android/build.gradle | 2 +- vector/build.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/build.gradle b/matrix-sdk-android/build.gradle index 4a5b45c4f9..441b031753 100644 --- a/matrix-sdk-android/build.gradle +++ b/matrix-sdk-android/build.gradle @@ -166,7 +166,7 @@ dependencies { implementation libs.apache.commonsImaging // Phone number https://github.com/google/libphonenumber - implementation 'com.googlecode.libphonenumber:libphonenumber:8.12.44' + implementation 'com.googlecode.libphonenumber:libphonenumber:8.12.45' testImplementation libs.tests.junit testImplementation 'org.robolectric:robolectric:4.7.3' diff --git a/vector/build.gradle b/vector/build.gradle index 438e8f2003..2b37c12323 100644 --- a/vector/build.gradle +++ b/vector/build.gradle @@ -367,7 +367,7 @@ dependencies { implementation 'com.facebook.stetho:stetho:1.6.0' // Phone number https://github.com/google/libphonenumber - implementation 'com.googlecode.libphonenumber:libphonenumber:8.12.44' + implementation 'com.googlecode.libphonenumber:libphonenumber:8.12.45' // FlowBinding implementation libs.github.flowBinding From 400a47c39b67914dc6e308e37690c0531b77eede Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Fri, 11 Mar 2022 08:45:04 +0000 Subject: [PATCH 24/28] Only run one gradlew build for all tests, do not split by project --- .github/workflows/tests.yml | 39 +++++++++---------------------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 74f31f4888..3a493e436f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,11 +12,13 @@ env: -Porg.gradle.parallel=false jobs: - # Build Android Tests [Matrix SDK] - build-android-test-matrix-sdk: - name: Matrix SDK - Build Android Tests + # Build Android Tests + build-android-tests: + name: Build Android Tests runs-on: ubuntu-latest - # No concurrency required, runs every time on a schedule. + concurrency: + group: ${{ github.ref == 'refs/heads/main' && format('unit-tests-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('unit-tests-develop-{0}', github.sha) || format('unit-tests-{0}', github.ref) }} + cancel-in-progress: true steps: - uses: actions/checkout@v3 - uses: actions/setup-java@v2 @@ -31,31 +33,9 @@ jobs: key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} restore-keys: | ${{ runner.os }}-gradle- - - name: Build Android Tests for matrix-sdk-android - run: ./gradlew clean matrix-sdk-android:assembleAndroidTest $CI_GRADLE_ARG_PROPERTIES --stacktrace + - name: Build Android Tests + run: ./gradlew clean assembleAndroidTest $CI_GRADLE_ARG_PROPERTIES --stacktrace - # Build Android Tests [Matrix APP] - build-android-test-app: - name: App - Build Android Tests - runs-on: ubuntu-latest - # No concurrency required, runs every time on a schedule. - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-java@v2 - with: - distribution: 'adopt' - java-version: 11 - - uses: actions/cache@v2 - with: - path: | - ~/.gradle/caches - ~/.gradle/wrapper - key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} - restore-keys: | - ${{ runner.os }}-gradle- - - name: Build Android Tests for vector - run: ./gradlew clean vector:assembleAndroidTest $CI_GRADLE_ARG_PROPERTIES --stacktrace - unit-tests: name: Run Unit Tests runs-on: ubuntu-latest @@ -91,8 +71,7 @@ jobs: runs-on: ubuntu-latest needs: - unit-tests - - build-android-test-matrix-sdk - - build-android-test-app + - build-android-tests if: ${{ (github.ref == 'refs/heads/develop' || github.ref == 'refs/heads/main' ) && failure() }} steps: - uses: michaelkaye/matrix-hookshot-action@v0.3.0 From 9a112bb0103ba2247a519451d656992ca3e6ce31 Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Fri, 11 Mar 2022 08:45:27 +0000 Subject: [PATCH 25/28] Remove confetti from build failed message. (there will be red icons if the build has failed in the body anyway) --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3a493e436f..a67239ea36 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -79,6 +79,6 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} matrix_access_token: ${{ secrets.ELEMENT_ANDROID_NOTIFICATION_ACCESS_TOKEN }} matrix_room_id: ${{ secrets.ELEMENT_ANDROID_INTERNAL_ROOM_ID }} - text_template: "Build is broken for ${{ github.ref }} 🎊: {{#each job_statuses }}{{#with this }}{{#if completed }} {{name}} {{conclusion}} at {{completed_at}}, {{/if}}{{/with}}{{/each}}" - html_template: "Build is broken for ${{ github.ref }} 🎊: {{#each job_statuses }}{{#with this }}{{#if completed }}
{{icon conclusion }}{{name}} {{conclusion}} at {{completed_at}} [details]{{/if}}{{/with}}{{/each}}" + text_template: "Build is broken for ${{ github.ref }}: {{#each job_statuses }}{{#with this }}{{#if completed }} {{name}} {{conclusion}} at {{completed_at}}, {{/if}}{{/with}}{{/each}}" + html_template: "Build is broken for ${{ github.ref }}: {{#each job_statuses }}{{#with this }}{{#if completed }}
{{icon conclusion }}{{name}} {{conclusion}} at {{completed_at}} [details]{{/if}}{{/with}}{{/each}}" From 02ea1c0e7cf504ddedbab0ef04faf9ad24ae3ba4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 11 Mar 2022 14:39:38 +0100 Subject: [PATCH 26/28] Add space between icon and name Also remove extra space for the text_template --- .github/workflows/tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a67239ea36..6fbf836dfe 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -79,6 +79,6 @@ jobs: github_token: ${{ secrets.GITHUB_TOKEN }} matrix_access_token: ${{ secrets.ELEMENT_ANDROID_NOTIFICATION_ACCESS_TOKEN }} matrix_room_id: ${{ secrets.ELEMENT_ANDROID_INTERNAL_ROOM_ID }} - text_template: "Build is broken for ${{ github.ref }}: {{#each job_statuses }}{{#with this }}{{#if completed }} {{name}} {{conclusion}} at {{completed_at}}, {{/if}}{{/with}}{{/each}}" - html_template: "Build is broken for ${{ github.ref }}: {{#each job_statuses }}{{#with this }}{{#if completed }}
{{icon conclusion }}{{name}} {{conclusion}} at {{completed_at}} [details]{{/if}}{{/with}}{{/each}}" + text_template: "Build is broken for ${{ github.ref }}: {{#each job_statuses }}{{#with this }}{{#if completed }}{{name}} {{conclusion}} at {{completed_at}}, {{/if}}{{/with}}{{/each}}" + html_template: "Build is broken for ${{ github.ref }}: {{#each job_statuses }}{{#with this }}{{#if completed }}
{{icon conclusion }} {{name}} {{conclusion}} at {{completed_at}} [details]{{/if}}{{/with}}{{/each}}" From fa104adefc271ff993f861b32c2b4c3031ee2c92 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 11 Mar 2022 15:35:21 +0100 Subject: [PATCH 27/28] Fix all warnings in file E2eeSanityTests.kt --- .idea/dictionaries/bmarty.xml | 2 + .../sdk/internal/crypto/E2eeSanityTests.kt | 39 ++++++++++--------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/.idea/dictionaries/bmarty.xml b/.idea/dictionaries/bmarty.xml index ed572b573f..85290e72df 100644 --- a/.idea/dictionaries/bmarty.xml +++ b/.idea/dictionaries/bmarty.xml @@ -11,6 +11,7 @@ emoji emojis fdroid + ganfra gplay hmac homeserver @@ -18,6 +19,7 @@ ktlint linkified linkify + manu megolm msisdn msisdns diff --git a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeSanityTests.kt b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeSanityTests.kt index 767334dfbd..41ec69cdc5 100644 --- a/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeSanityTests.kt +++ b/matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/E2eeSanityTests.kt @@ -41,6 +41,7 @@ import org.matrix.android.sdk.api.session.room.timeline.TimelineSettings import org.matrix.android.sdk.common.CommonTestHelper import org.matrix.android.sdk.common.CryptoTestHelper import org.matrix.android.sdk.common.SessionTestParams +import org.matrix.android.sdk.common.TestConstants import org.matrix.android.sdk.common.TestMatrixCallback import org.matrix.android.sdk.internal.crypto.algorithms.olm.OlmDecryptionResult import org.matrix.android.sdk.internal.crypto.keysbackup.model.MegolmBackupCreationInfo @@ -99,7 +100,6 @@ class E2eeSanityTests : InstrumentedTest { ensureMembersHaveJoined(aliceSession, otherAccounts, e2eRoomID) Log.v("#E2E TEST", "All users have joined the room") - Log.v("#E2E TEST", "Alice is sending the message") val text = "This is my message" @@ -172,7 +172,7 @@ class E2eeSanityTests : InstrumentedTest { } timelineEvent != null && timelineEvent.root.getClearType() == EventType.MESSAGE && - secondMessage.equals(timelineEvent.root.getClearContent().toModel()?.body) + secondMessage == timelineEvent.root.getClearContent().toModel()?.body } } } @@ -186,11 +186,11 @@ class E2eeSanityTests : InstrumentedTest { } /** - * Quick test for basic keybackup + * Quick test for basic key backup * 1. Create e2e between Alice and Bob * 2. Alice sends 3 messages, using 3 different sessions * 3. Ensure bob can decrypt - * 4. Create backup for bob and uplaod keys + * 4. Create backup for bob and upload keys * * 5. Sign out alice and bob to ensure no gossiping will happen * @@ -206,14 +206,14 @@ class E2eeSanityTests : InstrumentedTest { val bobSession = cryptoTestData.secondSession!! val e2eRoomID = cryptoTestData.roomId - Log.v("#E2E TEST", "Create and start keybackup for bob ...") - val keysBackupService = bobSession.cryptoService().keysBackupService() + Log.v("#E2E TEST", "Create and start key backup for bob ...") + val bobKeysBackupService = bobSession.cryptoService().keysBackupService() val keyBackupPassword = "FooBarBaz" val megolmBackupCreationInfo = testHelper.doSync { - keysBackupService.prepareKeysBackupVersion(keyBackupPassword, null, it) + bobKeysBackupService.prepareKeysBackupVersion(keyBackupPassword, null, it) } val version = testHelper.doSync { - keysBackupService.createKeysBackupVersion(megolmBackupCreationInfo, it) + bobKeysBackupService.createKeysBackupVersion(megolmBackupCreationInfo, it) } Log.v("#E2E TEST", "... Key backup started and enabled for bob") // Bob session should now have @@ -249,12 +249,12 @@ class E2eeSanityTests : InstrumentedTest { Log.v("#E2E TEST", "Force key backup for Bob...") testHelper.waitWithLatch { latch -> - keysBackupService.backupAllGroupSessions( + bobKeysBackupService.backupAllGroupSessions( null, TestMatrixCallback(latch, true) ) } - Log.v("#E2E TEST", "... Keybackup done for Bob") + Log.v("#E2E TEST", "... Key backup done for Bob") // Now lets logout both alice and bob to ensure that we won't have any gossiping @@ -397,7 +397,7 @@ class E2eeSanityTests : InstrumentedTest { } /** - * Test that if a better key is forwared (lower index, it is then used) + * Test that if a better key is forwarded (lower index, it is then used) */ @Test fun testForwardBetterKey() { @@ -525,15 +525,16 @@ class E2eeSanityTests : InstrumentedTest { private fun sendMessageInRoom(aliceRoomPOV: Room, text: String): String? { aliceRoomPOV.sendTextMessage(text) var sentEventId: String? = null - testHelper.waitWithLatch(4 * 60_000) { + testHelper.waitWithLatch(4 * TestConstants.timeOutMillis) { latch -> val timeline = aliceRoomPOV.createTimeline(null, TimelineSettings(60)) timeline.start() - testHelper.retryPeriodicallyWithLatch(it) { + testHelper.retryPeriodicallyWithLatch(latch) { val decryptedMsg = timeline.getSnapshot() .filter { it.root.getClearType() == EventType.MESSAGE } - .also { - Log.v("#E2E TEST", "Timeline snapshot is ${it.map { "${it.root.type}|${it.root.sendState}" }.joinToString(",", "[", "]")}") + .also { list -> + val message = list.joinToString(",", "[", "]") { "${it.root.type}|${it.root.sendState}" } + Log.v("#E2E TEST", "Timeline snapshot is $message") } .filter { it.root.sendState == SendState.SYNCED } .firstOrNull { it.root.getClearContent().toModel()?.body?.startsWith(text) == true } @@ -547,8 +548,8 @@ class E2eeSanityTests : InstrumentedTest { } private fun ensureMembersHaveJoined(aliceSession: Session, otherAccounts: List, e2eRoomID: String) { - testHelper.waitWithLatch { - testHelper.retryPeriodicallyWithLatch(it) { + testHelper.waitWithLatch { latch -> + testHelper.retryPeriodicallyWithLatch(latch) { otherAccounts.map { aliceSession.getRoomMember(it.myUserId, e2eRoomID)?.membership }.all { @@ -559,8 +560,8 @@ class E2eeSanityTests : InstrumentedTest { } private fun waitForAndAcceptInviteInRoom(otherSession: Session, e2eRoomID: String) { - testHelper.waitWithLatch { - testHelper.retryPeriodicallyWithLatch(it) { + testHelper.waitWithLatch { latch -> + testHelper.retryPeriodicallyWithLatch(latch) { val roomSummary = otherSession.getRoomSummary(e2eRoomID) (roomSummary != null && roomSummary.membership == Membership.INVITE).also { if (it) { From 9a532fc47f819afe2c35b86479094a827b3f89ca Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Fri, 11 Mar 2022 16:47:40 +0000 Subject: [PATCH 28/28] Use different tags for unit tests and android test compilation. Otherwise we will cancel one in favour of the other. --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6fbf836dfe..587bf14488 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,7 +17,7 @@ jobs: name: Build Android Tests runs-on: ubuntu-latest concurrency: - group: ${{ github.ref == 'refs/heads/main' && format('unit-tests-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('unit-tests-develop-{0}', github.sha) || format('unit-tests-{0}', github.ref) }} + group: ${{ github.ref == 'refs/heads/main' && format('unit-tests-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('unit-tests-develop-{0}', github.sha) || format('build-android-tests-{0}', github.ref) }} cancel-in-progress: true steps: - uses: actions/checkout@v3