From 3162bf7dcc3c2b332d147f6d36febd612103be3c Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Wed, 22 Jan 2025 18:33:43 +0100 Subject: [PATCH] Better handling for editing alias in case of different HS (#3695) * better handling for aliases from different HS * insert the alias at the top * removing the old homeserver alias * code improvement * always remove the old canonical alias found on the server if exists * added extensive testing for all the possible cases on how the save is handled given the various context of the existing room alias --- .../EditRoomAddressScreenViewModel.swift | 48 +++++++--- .../EditRoomAddressScreenViewModelTests.swift | 94 ++++++++++++++++++- 2 files changed, 123 insertions(+), 19 deletions(-) diff --git a/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift b/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift index ec430775a..371109e99 100644 --- a/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift +++ b/ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift @@ -129,13 +129,13 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo hideLoadingIndicator() } - guard let canonicalAlias = String.makeCanonicalAlias(aliasLocalPart: state.bindings.desiredAliasLocalPart, serverName: state.serverName), - isRoomAliasFormatValid(alias: canonicalAlias) else { + guard let desiredCanonicalAlias = String.makeCanonicalAlias(aliasLocalPart: state.bindings.desiredAliasLocalPart, serverName: state.serverName), + isRoomAliasFormatValid(alias: desiredCanonicalAlias) else { state.aliasErrors = [.invalidSymbols] return } - switch await clientProxy.isAliasAvailable(canonicalAlias) { + switch await clientProxy.isAliasAvailable(desiredCanonicalAlias) { case .success(true): break case .success(false): @@ -146,24 +146,44 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo return } - let oldAlias = roomProxy.infoPublisher.value.firstAliasMatching(serverName: clientProxy.userIDServerName, useFallback: false) + let savedAliasFromHomeserver = roomProxy.infoPublisher.value.firstAliasMatching(serverName: state.serverName, useFallback: false) + let savedCanonicalAlias = roomProxy.infoPublisher.value.canonicalAlias - // First publish the new alias - if case .failure = await roomProxy.publishRoomAliasInRoomDirectory(canonicalAlias) { + // First publish the desired new alias in the room directory + if case .failure = await roomProxy.publishRoomAliasInRoomDirectory(desiredCanonicalAlias) { userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) return } - // Then set it as the main alias - if case .failure = await roomProxy.updateCanonicalAlias(canonicalAlias, altAliases: []) { - userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) - return + // Then try remove the old alias from the room directory on our current HS + if let savedAliasFromHomeserver { + if case .failure = await roomProxy.removeRoomAliasFromRoomDirectory(savedAliasFromHomeserver) { + userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) + return + } } - // And finally delete the old one - if let oldAlias, case .failure = await roomProxy.removeRoomAliasFromRoomDirectory(oldAlias) { - userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) - return + // Finally update the canonical alias state.. + // Allow to update the canonical alias only if the saved canonical alias matches the homeserver or if there is no canonical alias + if savedCanonicalAlias == nil || savedCanonicalAlias?.hasSuffix(state.serverName) == true { + var newAlternativeAliases = roomProxy.infoPublisher.value.alternativeAliases + newAlternativeAliases.removeAll { $0 == savedAliasFromHomeserver } + + if case .failure = await roomProxy.updateCanonicalAlias(desiredCanonicalAlias, altAliases: newAlternativeAliases) { + userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) + return + } + // Otherwise, update the alternative aliases and keep the current canonical alias + } else { + var newAlternativeAliases = roomProxy.infoPublisher.value.alternativeAliases + // We also remove the existing saved alias from our homeserver if exists + newAlternativeAliases.removeAll { $0 == savedAliasFromHomeserver } + newAlternativeAliases.insert(desiredCanonicalAlias, at: 0) + + if case .failure = await roomProxy.updateCanonicalAlias(savedCanonicalAlias, altAliases: newAlternativeAliases) { + userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown)) + return + } } actionsSubject.send(.dismiss) diff --git a/UnitTests/Sources/EditRoomAddressScreenViewModelTests.swift b/UnitTests/Sources/EditRoomAddressScreenViewModelTests.swift index fa4acd243..133ed37db 100644 --- a/UnitTests/Sources/EditRoomAddressScreenViewModelTests.swift +++ b/UnitTests/Sources/EditRoomAddressScreenViewModelTests.swift @@ -65,19 +65,103 @@ class EditRoomAddressScreenViewModelTests: XCTestCase { try await deferred.fulfill() } - func testCorrectMethodsCalledOnSave() async throws { + func testCorrectMethodsCalledOnSaveWhenNoAliasExists() async throws { let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org")) clientProxy.isAliasAvailableReturnValue = .success(true) - let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name")) - roomProxy.publishRoomAliasInRoomDirectoryReturnValue = .success(true) - roomProxy.updateCanonicalAliasAltAliasesReturnValue = .success(()) - roomProxy.removeRoomAliasFromRoomDirectoryReturnValue = .success(true) viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy, clientProxy: clientProxy, userIndicatorController: UserIndicatorControllerMock()) + XCTAssertNil(roomProxy.infoPublisher.value.canonicalAlias) + XCTAssertEqual(viewModel.context.viewState.bindings.desiredAliasLocalPart, "room-name") + + let publishingExpectation = expectation(description: "Wait for publishing") + roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in + defer { publishingExpectation.fulfill() } + XCTAssertEqual(roomAlias, "#room-name:matrix.org") + return .success(true) + } + + let updateAliasExpectation = expectation(description: "Wait for alias update") + roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in + defer { updateAliasExpectation.fulfill() } + XCTAssertEqual(altAliases, []) + XCTAssertEqual(roomAlias, "#room-name:matrix.org") + return .success(()) + } + context.send(viewAction: .save) + await fulfillment(of: [publishingExpectation, updateAliasExpectation], timeout: 1.0) + XCTAssertFalse(roomProxy.removeRoomAliasFromRoomDirectoryCalled) + } + + func testCorrectMethodsCalledOnSaveWhenAliasOnSameHomeserverExists() async throws { + let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org")) + clientProxy.isAliasAvailableReturnValue = .success(true) + let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#old-room-name:matrix.org")) + + viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy, + clientProxy: clientProxy, + userIndicatorController: UserIndicatorControllerMock()) + + context.desiredAliasLocalPart = "room-name" + + let publishingExpectation = expectation(description: "Wait for publishing") + roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in + defer { publishingExpectation.fulfill() } + XCTAssertEqual(roomAlias, "#room-name:matrix.org") + return .success(true) + } + + let updateAliasExpectation = expectation(description: "Wait for alias update") + roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in + defer { updateAliasExpectation.fulfill() } + XCTAssertEqual(altAliases, []) + XCTAssertEqual(roomAlias, "#room-name:matrix.org") + return .success(()) + } + + let removeAliasExpectation = expectation(description: "Wait for alias removal") + roomProxy.removeRoomAliasFromRoomDirectoryClosure = { roomAlias in + defer { removeAliasExpectation.fulfill() } + XCTAssertEqual(roomAlias, "#old-room-name:matrix.org") + return .success(true) + } + + context.send(viewAction: .save) + await fulfillment(of: [publishingExpectation, updateAliasExpectation, removeAliasExpectation], timeout: 1.0) + } + + func testCorrectMethodsCalledOnSaveWhenAliasOnOtherHomeserverExists() async throws { + let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org")) + clientProxy.isAliasAvailableReturnValue = .success(true) + let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#old-room-name:element.io")) + + viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy, + clientProxy: clientProxy, + userIndicatorController: UserIndicatorControllerMock()) + + context.desiredAliasLocalPart = "room-name" + + let publishingExpectation = expectation(description: "Wait for publishing") + roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in + defer { publishingExpectation.fulfill() } + XCTAssertEqual(roomAlias, "#room-name:matrix.org") + return .success(true) + } + + let updateAliasExpectation = expectation(description: "Wait for alias update") + roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in + defer { updateAliasExpectation.fulfill() } + XCTAssertEqual(altAliases, ["#room-name:matrix.org"]) + XCTAssertEqual(roomAlias, "#old-room-name:element.io") + return .success(()) + } + + context.send(viewAction: .save) + await fulfillment(of: [publishingExpectation, updateAliasExpectation], timeout: 1.0) + XCTAssertFalse(roomProxy.removeRoomAliasFromRoomDirectoryCalled) } }