Skip to content

Commit

Permalink
join and leave methods moved from MembershipService to RoomService an… (
Browse files Browse the repository at this point in the history
  • Loading branch information
fedrunov authored Feb 10, 2022
1 parent e710632 commit 06b5563
Show file tree
Hide file tree
Showing 20 changed files with 52 additions and 63 deletions.
1 change: 1 addition & 0 deletions changelog.d/5183.sdk
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`join` and `leave` methods moved from MembershipService to RoomService and SpaceService to split logic for rooms and spaces
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,6 @@ class SpaceHierarchyTest : InstrumentedTest {
// Test part one of the rooms

val bRoomId = spaceBInfo.roomIds.first()
val bRoom = session.getRoom(bRoomId)

commonTestHelper.waitWithLatch { latch ->
val flatAChildren = session.getFlattenRoomSummaryChildrenOfLive(spaceAInfo.spaceId)
Expand All @@ -360,7 +359,7 @@ class SpaceHierarchyTest : InstrumentedTest {
}

// part from b room
bRoom!!.leave(null)
session.leaveRoom(bRoomId)
// The room should have disapear from flat children
flatAChildren.observeForever(childObserver)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ interface RoomService {
thirdPartySigned: SignInvitationResult
)

/**
* Leave the room, or reject an invitation.
* @param roomId the roomId of the room to leave
* @param reason optional reason for leaving the room
*/
suspend fun leaveRoom(roomId: String, reason: String? = null)

/**
* Get a room from a roomId
* @param roomId the roomId to look for.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,4 @@ interface MembershipService {

@Deprecated("Use remove instead", ReplaceWith("remove(userId, reason)"))
suspend fun kick(userId: String, reason: String? = null) = remove(userId, reason)

/**
* Join the room, or accept an invitation.
*/
suspend fun join(reason: String? = null, viaServers: List<String> = emptyList())

/**
* Leave the room, or reject an invitation.
*/
suspend fun leave(reason: String? = null)
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ interface Space {

val spaceId: String

suspend fun leave(reason: String? = null)

/**
* A current snapshot of [RoomSummary] associated with the space
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ interface SpaceService {

suspend fun rejectInvite(spaceId: String, reason: String?)

/**
* Leave the space, or reject an invitation.
* @param spaceId the spaceId of the space to leave
* @param reason optional reason for leaving the space
*/
suspend fun leaveSpace(spaceId: String, reason: String? = null)

// fun getSpaceParentsOfRoom(roomId: String) : List<SpaceSummary>

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import org.matrix.android.sdk.internal.session.room.create.CreateRoomTask
import org.matrix.android.sdk.internal.session.room.membership.RoomChangeMembershipStateDataSource
import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper
import org.matrix.android.sdk.internal.session.room.membership.joining.JoinRoomTask
import org.matrix.android.sdk.internal.session.room.membership.leaving.LeaveRoomTask
import org.matrix.android.sdk.internal.session.room.peeking.PeekRoomTask
import org.matrix.android.sdk.internal.session.room.peeking.ResolveRoomStateTask
import org.matrix.android.sdk.internal.session.room.read.MarkAllRoomsReadTask
Expand All @@ -66,7 +67,8 @@ internal class DefaultRoomService @Inject constructor(
private val peekRoomTask: PeekRoomTask,
private val roomGetter: RoomGetter,
private val roomSummaryDataSource: RoomSummaryDataSource,
private val roomChangeMembershipStateDataSource: RoomChangeMembershipStateDataSource
private val roomChangeMembershipStateDataSource: RoomChangeMembershipStateDataSource,
private val leaveRoomTask: LeaveRoomTask,
) : RoomService {

override suspend fun createRoom(createRoomParams: CreateRoomParams): String {
Expand Down Expand Up @@ -133,6 +135,10 @@ internal class DefaultRoomService @Inject constructor(
joinRoomTask.execute(JoinRoomTask.Params(roomId, reason, thirdPartySigned = thirdPartySigned))
}

override suspend fun leaveRoom(roomId: String, reason: String?) {
leaveRoomTask.execute(LeaveRoomTask.Params(roomId, reason))
}

override suspend fun markAllAsRead(roomIds: List<String>) {
markAllRoomsReadTask.execute(MarkAllRoomsReadTask.Params(roomIds))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ import org.matrix.android.sdk.internal.query.QueryStringValueProcessor
import org.matrix.android.sdk.internal.query.process
import org.matrix.android.sdk.internal.session.room.membership.admin.MembershipAdminTask
import org.matrix.android.sdk.internal.session.room.membership.joining.InviteTask
import org.matrix.android.sdk.internal.session.room.membership.joining.JoinRoomTask
import org.matrix.android.sdk.internal.session.room.membership.leaving.LeaveRoomTask
import org.matrix.android.sdk.internal.session.room.membership.threepid.InviteThreePidTask
import org.matrix.android.sdk.internal.util.fetchCopied

Expand All @@ -48,8 +46,6 @@ internal class DefaultMembershipService @AssistedInject constructor(
private val loadRoomMembersTask: LoadRoomMembersTask,
private val inviteTask: InviteTask,
private val inviteThreePidTask: InviteThreePidTask,
private val joinTask: JoinRoomTask,
private val leaveRoomTask: LeaveRoomTask,
private val membershipAdminTask: MembershipAdminTask,
@UserId
private val userId: String,
Expand Down Expand Up @@ -139,14 +135,4 @@ internal class DefaultMembershipService @AssistedInject constructor(
val params = InviteThreePidTask.Params(roomId, threePid)
return inviteThreePidTask.execute(params)
}

override suspend fun join(reason: String?, viaServers: List<String>) {
val params = JoinRoomTask.Params(roomId, reason, viaServers)
joinTask.execute(params)
}

override suspend fun leave(reason: String?) {
val params = LeaveRoomTask.Params(roomId, reason)
leaveRoomTask.execute(params)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ internal class DefaultSpace(

override val spaceId = room.roomId

override suspend fun leave(reason: String?) {
return room.leave(reason)
}

override fun spaceSummary(): RoomSummary? {
return spaceSummaryDataSource.getSpaceSummary(room.roomId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ internal class DefaultSpaceService @Inject constructor(
return joinSpaceTask.execute(JoinSpaceTask.Params(spaceIdOrAlias, reason, viaServers))
}

override suspend fun leaveSpace(spaceId: String, reason: String?) {
leaveRoomTask.execute(LeaveRoomTask.Params(spaceId, reason))
}

override suspend fun rejectInvite(spaceId: String, reason: String?) {
leaveRoomTask.execute(LeaveRoomTask.Params(spaceId, reason))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -801,14 +801,14 @@ class TimelineViewModel @AssistedInject constructor(

private fun handleRejectInvite() {
viewModelScope.launch {
tryOrNull { room.leave(null) }
tryOrNull { session.leaveRoom(room.roomId) }
}
}

private fun handleAcceptInvite() {
viewModelScope.launch {
tryOrNull {
room.join()
session.joinRoom(room.roomId)
analyticsTracker.capture(room.roomSummary().toAnalyticsJoinedRoom())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ class MessageComposerViewModel @AssistedInject constructor(
is ParsedCommand.LeaveRoom -> {
viewModelScope.launch(Dispatchers.IO) {
try {
session.getRoom(slashCommandResult.roomId)?.leave(null)
session.leaveRoom(slashCommandResult.roomId)
popDraft()
_viewEvents.post(MessageComposerViewEvents.SlashCommandResultOk())
} catch (failure: Throwable) {
Expand Down Expand Up @@ -683,7 +683,9 @@ class MessageComposerViewModel @AssistedInject constructor(
?.roomId
?.let { session.getRoom(it) }
}
?.leave(reason = null)
?.let {
session.leaveRoom(it.roomId)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,9 @@ class RoomListViewModel @AssistedInject constructor(
)
}

val room = session.getRoom(roomId) ?: return@withState
viewModelScope.launch {
try {
room.join()
session.joinRoom(roomId)
analyticsTracker.capture(action.roomSummary.toAnalyticsJoinedRoom())
// We do not update the joiningRoomsIds here, because, the room is not joined yet regarding the sync data.
// Instead, we wait for the room to be joined
Expand All @@ -245,10 +244,9 @@ class RoomListViewModel @AssistedInject constructor(
return@withState
}

val room = session.getRoom(roomId) ?: return@withState
viewModelScope.launch {
try {
room.leave(null)
session.leaveRoom(roomId)
// We do not update the rejectingRoomsIds here, because, the room is not rejected yet regarding the sync data.
// Instead, we wait for the room to be rejected
// Known bug: if the user is invited again (after rejecting the first invitation), the loading will be displayed instead of the buttons.
Expand Down Expand Up @@ -333,9 +331,8 @@ class RoomListViewModel @AssistedInject constructor(

private fun handleLeaveRoom(action: RoomListAction.LeaveRoom) {
_viewEvents.post(RoomListViewEvents.Loading(null))
val room = session.getRoom(action.roomId) ?: return
viewModelScope.launch {
val value = runCatching { room.leave(null) }
val value = runCatching { session.leaveRoom(action.roomId) }
.fold({ RoomListViewEvents.Done }, { RoomListViewEvents.Failure(it) })
_viewEvents.post(value)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import kotlinx.coroutines.sync.withPermit
import org.matrix.android.sdk.api.extensions.orFalse
import org.matrix.android.sdk.api.failure.Failure
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.api.session.room.Room
import org.matrix.android.sdk.api.session.room.members.ChangeMembershipState
import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.api.session.room.roomSummaryQueryParams
Expand Down Expand Up @@ -109,7 +108,7 @@ class InvitesAcceptor @Inject constructor(

private suspend fun Session.joinRoomSafely(roomId: String) {
if (shouldRejectRoomIds.contains(roomId)) {
getRoom(roomId)?.rejectInviteSafely()
rejectInviteSafely(roomId)
return
}
val roomMembershipChanged = getChangeMemberships(roomId)
Expand All @@ -126,16 +125,16 @@ class InvitesAcceptor @Inject constructor(
// if the inviting user is on the same HS, there can only be one cause: they left, so we try to reject the invite.
if (inviterId?.endsWith(sessionParams.credentials.homeServer.orEmpty()).orFalse()) {
shouldRejectRoomIds.add(roomId)
room.rejectInviteSafely()
rejectInviteSafely(roomId)
}
}
}
}
}

private suspend fun Room.rejectInviteSafely() {
private suspend fun Session.rejectInviteSafely(roomId: String) {
try {
leave(null)
leaveRoom(roomId)
shouldRejectRoomIds.remove(roomId)
} catch (failure: Throwable) {
Timber.v("Fail rejecting invite for room: $roomId")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class NotificationBroadcastReceiver : BroadcastReceiver() {
if (room != null) {
session.coroutineScope.launch {
tryOrNull {
room.join()
session.joinRoom(room.roomId)
analyticsTracker.capture(room.roomSummary().toAnalyticsJoinedRoom())
}
}
Expand All @@ -93,11 +93,8 @@ class NotificationBroadcastReceiver : BroadcastReceiver() {

private fun handleRejectRoom(roomId: String) {
activeSessionHolder.getSafeActiveSession()?.let { session ->
val room = session.getRoom(roomId)
if (room != null) {
session.coroutineScope.launch {
tryOrNull { room.leave() }
}
session.coroutineScope.launch {
tryOrNull { session.leaveRoom(roomId) }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ class RoomProfileViewModel @AssistedInject constructor(
_viewEvents.post(RoomProfileViewEvents.Loading(stringProvider.getString(R.string.room_profile_leaving_room)))
viewModelScope.launch {
try {
room.leave(null)
session.leaveRoom(room.roomId)
// Do nothing, we will be closing the room automatically when it will get back from sync
} catch (failure: Throwable) {
_viewEvents.post(RoomProfileViewEvents.Failure(failure))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ class SpaceListViewModel @AssistedInject constructor(@Assisted initialState: Spa
private fun handleLeaveSpace(action: SpaceListAction.LeaveSpace) {
viewModelScope.launch {
tryOrNull("Failed to leave space ${action.spaceSummary.roomId}") {
session.spaceService().getSpace(action.spaceSummary.roomId)?.leave(null)
session.spaceService().leaveSpace(action.spaceSummary.roomId)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class SpaceMenuViewModel @AssistedInject constructor(
session.coroutineScope.launch {
try {
if (state.leaveMode == SpaceMenuState.LeaveMode.LEAVE_NONE) {
session.getRoom(initialState.spaceId)?.leave(null)
session.spaceService().leaveSpace(initialState.spaceId)
} else if (state.leaveMode == SpaceMenuState.LeaveMode.LEAVE_ALL) {
// need to find all child rooms that i have joined

Expand All @@ -143,13 +143,13 @@ class SpaceMenuViewModel @AssistedInject constructor(
}
).forEach {
try {
session.getRoom(it.roomId)?.leave(null)
session.spaceService().leaveSpace(it.roomId)
} catch (failure: Throwable) {
// silently ignore?
Timber.e(failure, "Fail to leave sub rooms/spaces")
}
}
session.getRoom(initialState.spaceId)?.leave(null)
session.spaceService().leaveSpace(initialState.spaceId)
}

// We observe the membership and to dismiss when we have remote echo of leaving
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class SpaceInviteBottomSheetViewModel @AssistedInject constructor(
setState { copy(joinActionState = Loading()) }
session.coroutineScope.launch(Dispatchers.IO) {
try {
session.getRoom(initialState.spaceId)?.join()
session.spaceService().joinSpace(initialState.spaceId)
setState { copy(joinActionState = Success(Unit)) }
} catch (failure: Throwable) {
setState { copy(joinActionState = Fail(failure)) }
Expand All @@ -116,7 +116,7 @@ class SpaceInviteBottomSheetViewModel @AssistedInject constructor(
setState { copy(rejectActionState = Loading()) }
session.coroutineScope.launch(Dispatchers.IO) {
try {
session.getRoom(initialState.spaceId)?.leave()
session.spaceService().leaveSpace(initialState.spaceId)
setState { copy(rejectActionState = Success(Unit)) }
} catch (failure: Throwable) {
setState { copy(rejectActionState = Fail(failure)) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ class SpaceLeaveAdvancedViewModel @AssistedInject constructor(
try {
state.selectedRooms.forEach {
try {
session.getRoom(it)?.leave(null)
session.leaveRoom(it)
} catch (failure: Throwable) {
// silently ignore?
Timber.e(failure, "Fail to leave sub rooms/spaces")
}
}

session.getRoom(initialState.spaceId)?.leave(null)
session.spaceService().leaveSpace(initialState.spaceId)
// We observe the membership and to dismiss when we have remote echo of leaving
} catch (failure: Throwable) {
setState { copy(leaveState = Fail(failure)) }
Expand Down

0 comments on commit 06b5563

Please sign in to comment.