Skip to content

Commit 3162bf7

Browse files
authored
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
1 parent 48e530f commit 3162bf7

File tree

2 files changed

+123
-19
lines changed

2 files changed

+123
-19
lines changed

ElementX/Sources/Screens/EditRoomAddressScreen/EditRoomAddressScreenViewModel.swift

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,13 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo
129129
hideLoadingIndicator()
130130
}
131131

132-
guard let canonicalAlias = String.makeCanonicalAlias(aliasLocalPart: state.bindings.desiredAliasLocalPart, serverName: state.serverName),
133-
isRoomAliasFormatValid(alias: canonicalAlias) else {
132+
guard let desiredCanonicalAlias = String.makeCanonicalAlias(aliasLocalPart: state.bindings.desiredAliasLocalPart, serverName: state.serverName),
133+
isRoomAliasFormatValid(alias: desiredCanonicalAlias) else {
134134
state.aliasErrors = [.invalidSymbols]
135135
return
136136
}
137137

138-
switch await clientProxy.isAliasAvailable(canonicalAlias) {
138+
switch await clientProxy.isAliasAvailable(desiredCanonicalAlias) {
139139
case .success(true):
140140
break
141141
case .success(false):
@@ -146,24 +146,44 @@ class EditRoomAddressScreenViewModel: EditRoomAddressScreenViewModelType, EditRo
146146
return
147147
}
148148

149-
let oldAlias = roomProxy.infoPublisher.value.firstAliasMatching(serverName: clientProxy.userIDServerName, useFallback: false)
149+
let savedAliasFromHomeserver = roomProxy.infoPublisher.value.firstAliasMatching(serverName: state.serverName, useFallback: false)
150+
let savedCanonicalAlias = roomProxy.infoPublisher.value.canonicalAlias
150151

151-
// First publish the new alias
152-
if case .failure = await roomProxy.publishRoomAliasInRoomDirectory(canonicalAlias) {
152+
// First publish the desired new alias in the room directory
153+
if case .failure = await roomProxy.publishRoomAliasInRoomDirectory(desiredCanonicalAlias) {
153154
userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
154155
return
155156
}
156157

157-
// Then set it as the main alias
158-
if case .failure = await roomProxy.updateCanonicalAlias(canonicalAlias, altAliases: []) {
159-
userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
160-
return
158+
// Then try remove the old alias from the room directory on our current HS
159+
if let savedAliasFromHomeserver {
160+
if case .failure = await roomProxy.removeRoomAliasFromRoomDirectory(savedAliasFromHomeserver) {
161+
userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
162+
return
163+
}
161164
}
162165

163-
// And finally delete the old one
164-
if let oldAlias, case .failure = await roomProxy.removeRoomAliasFromRoomDirectory(oldAlias) {
165-
userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
166-
return
166+
// Finally update the canonical alias state..
167+
// Allow to update the canonical alias only if the saved canonical alias matches the homeserver or if there is no canonical alias
168+
if savedCanonicalAlias == nil || savedCanonicalAlias?.hasSuffix(state.serverName) == true {
169+
var newAlternativeAliases = roomProxy.infoPublisher.value.alternativeAliases
170+
newAlternativeAliases.removeAll { $0 == savedAliasFromHomeserver }
171+
172+
if case .failure = await roomProxy.updateCanonicalAlias(desiredCanonicalAlias, altAliases: newAlternativeAliases) {
173+
userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
174+
return
175+
}
176+
// Otherwise, update the alternative aliases and keep the current canonical alias
177+
} else {
178+
var newAlternativeAliases = roomProxy.infoPublisher.value.alternativeAliases
179+
// We also remove the existing saved alias from our homeserver if exists
180+
newAlternativeAliases.removeAll { $0 == savedAliasFromHomeserver }
181+
newAlternativeAliases.insert(desiredCanonicalAlias, at: 0)
182+
183+
if case .failure = await roomProxy.updateCanonicalAlias(savedCanonicalAlias, altAliases: newAlternativeAliases) {
184+
userIndicatorController.submitIndicator(.init(title: L10n.errorUnknown))
185+
return
186+
}
167187
}
168188

169189
actionsSubject.send(.dismiss)

UnitTests/Sources/EditRoomAddressScreenViewModelTests.swift

Lines changed: 89 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,19 +65,103 @@ class EditRoomAddressScreenViewModelTests: XCTestCase {
6565
try await deferred.fulfill()
6666
}
6767

68-
func testCorrectMethodsCalledOnSave() async throws {
68+
func testCorrectMethodsCalledOnSaveWhenNoAliasExists() async throws {
6969
let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org"))
7070
clientProxy.isAliasAvailableReturnValue = .success(true)
71-
7271
let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name"))
73-
roomProxy.publishRoomAliasInRoomDirectoryReturnValue = .success(true)
74-
roomProxy.updateCanonicalAliasAltAliasesReturnValue = .success(())
75-
roomProxy.removeRoomAliasFromRoomDirectoryReturnValue = .success(true)
7672

7773
viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
7874
clientProxy: clientProxy,
7975
userIndicatorController: UserIndicatorControllerMock())
8076

77+
XCTAssertNil(roomProxy.infoPublisher.value.canonicalAlias)
78+
XCTAssertEqual(viewModel.context.viewState.bindings.desiredAliasLocalPart, "room-name")
79+
80+
let publishingExpectation = expectation(description: "Wait for publishing")
81+
roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in
82+
defer { publishingExpectation.fulfill() }
83+
XCTAssertEqual(roomAlias, "#room-name:matrix.org")
84+
return .success(true)
85+
}
86+
87+
let updateAliasExpectation = expectation(description: "Wait for alias update")
88+
roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in
89+
defer { updateAliasExpectation.fulfill() }
90+
XCTAssertEqual(altAliases, [])
91+
XCTAssertEqual(roomAlias, "#room-name:matrix.org")
92+
return .success(())
93+
}
94+
95+
context.send(viewAction: .save)
96+
await fulfillment(of: [publishingExpectation, updateAliasExpectation], timeout: 1.0)
97+
XCTAssertFalse(roomProxy.removeRoomAliasFromRoomDirectoryCalled)
98+
}
99+
100+
func testCorrectMethodsCalledOnSaveWhenAliasOnSameHomeserverExists() async throws {
101+
let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org"))
102+
clientProxy.isAliasAvailableReturnValue = .success(true)
103+
let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#old-room-name:matrix.org"))
104+
105+
viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
106+
clientProxy: clientProxy,
107+
userIndicatorController: UserIndicatorControllerMock())
108+
109+
context.desiredAliasLocalPart = "room-name"
110+
111+
let publishingExpectation = expectation(description: "Wait for publishing")
112+
roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in
113+
defer { publishingExpectation.fulfill() }
114+
XCTAssertEqual(roomAlias, "#room-name:matrix.org")
115+
return .success(true)
116+
}
117+
118+
let updateAliasExpectation = expectation(description: "Wait for alias update")
119+
roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in
120+
defer { updateAliasExpectation.fulfill() }
121+
XCTAssertEqual(altAliases, [])
122+
XCTAssertEqual(roomAlias, "#room-name:matrix.org")
123+
return .success(())
124+
}
125+
126+
let removeAliasExpectation = expectation(description: "Wait for alias removal")
127+
roomProxy.removeRoomAliasFromRoomDirectoryClosure = { roomAlias in
128+
defer { removeAliasExpectation.fulfill() }
129+
XCTAssertEqual(roomAlias, "#old-room-name:matrix.org")
130+
return .success(true)
131+
}
132+
133+
context.send(viewAction: .save)
134+
await fulfillment(of: [publishingExpectation, updateAliasExpectation, removeAliasExpectation], timeout: 1.0)
135+
}
136+
137+
func testCorrectMethodsCalledOnSaveWhenAliasOnOtherHomeserverExists() async throws {
138+
let clientProxy = ClientProxyMock(.init(userIDServerName: "matrix.org"))
139+
clientProxy.isAliasAvailableReturnValue = .success(true)
140+
let roomProxy = JoinedRoomProxyMock(.init(name: "Room Name", canonicalAlias: "#old-room-name:element.io"))
141+
142+
viewModel = EditRoomAddressScreenViewModel(roomProxy: roomProxy,
143+
clientProxy: clientProxy,
144+
userIndicatorController: UserIndicatorControllerMock())
145+
146+
context.desiredAliasLocalPart = "room-name"
147+
148+
let publishingExpectation = expectation(description: "Wait for publishing")
149+
roomProxy.publishRoomAliasInRoomDirectoryClosure = { roomAlias in
150+
defer { publishingExpectation.fulfill() }
151+
XCTAssertEqual(roomAlias, "#room-name:matrix.org")
152+
return .success(true)
153+
}
154+
155+
let updateAliasExpectation = expectation(description: "Wait for alias update")
156+
roomProxy.updateCanonicalAliasAltAliasesClosure = { roomAlias, altAliases in
157+
defer { updateAliasExpectation.fulfill() }
158+
XCTAssertEqual(altAliases, ["#room-name:matrix.org"])
159+
XCTAssertEqual(roomAlias, "#old-room-name:element.io")
160+
return .success(())
161+
}
162+
81163
context.send(viewAction: .save)
164+
await fulfillment(of: [publishingExpectation, updateAliasExpectation], timeout: 1.0)
165+
XCTAssertFalse(roomProxy.removeRoomAliasFromRoomDirectoryCalled)
82166
}
83167
}

0 commit comments

Comments
 (0)