Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert StateService to suspend functions #2500

Merged
merged 7 commits into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Translations 🗣:
-

SDK API changes ⚠️:
-
- StateService now exposes suspendable function instead of using MatrixCallback.

Build 🧱:
- Upgrade some dependencies and Kotlin version
Expand Down
1 change: 1 addition & 0 deletions matrix-sdk-android-rx/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ dependencies {
implementation 'androidx.appcompat:appcompat:1.2.0'
implementation 'io.reactivex.rxjava2:rxkotlin:2.3.0'
implementation 'io.reactivex.rxjava2:rxandroid:2.1.1'
implementation "org.jetbrains.kotlinx:kotlinx-coroutines-rx2:$kotlin_coroutines_version"

// Paging
implementation "androidx.paging:paging-runtime-ktx:2.1.2"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,27 @@
package org.matrix.android.sdk.rx

import android.net.Uri
import io.reactivex.Completable
import io.reactivex.Observable
import io.reactivex.Single
import kotlinx.coroutines.rx2.rxCompletable
import org.matrix.android.sdk.api.query.QueryStringValue
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.identity.ThreePid
import org.matrix.android.sdk.api.session.room.Room
import org.matrix.android.sdk.api.session.room.members.RoomMemberQueryParams
import org.matrix.android.sdk.api.session.room.model.EventAnnotationsSummary
import org.matrix.android.sdk.api.session.room.model.GuestAccess
import org.matrix.android.sdk.api.session.room.model.ReadReceipt
import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibility
import org.matrix.android.sdk.api.session.room.model.RoomJoinRules
import org.matrix.android.sdk.api.session.room.model.RoomMemberSummary
import org.matrix.android.sdk.api.session.room.model.RoomSummary
import org.matrix.android.sdk.api.session.room.notification.RoomNotificationState
import org.matrix.android.sdk.api.session.room.send.UserDraft
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
import org.matrix.android.sdk.api.util.Optional
import org.matrix.android.sdk.api.util.toOptional
import io.reactivex.Completable
import io.reactivex.Observable
import io.reactivex.Single
import org.matrix.android.sdk.api.session.room.model.GuestAccess
import org.matrix.android.sdk.api.session.room.model.RoomJoinRules

class RxRoom(private val room: Room) {

Expand Down Expand Up @@ -121,28 +122,28 @@ class RxRoom(private val room: Room) {
room.invite3pid(threePid, it)
}

fun updateTopic(topic: String): Completable = completableBuilder<Unit> {
room.updateTopic(topic, it)
fun updateTopic(topic: String): Completable = rxCompletable {
room.updateTopic(topic)
}

fun updateName(name: String): Completable = completableBuilder<Unit> {
room.updateName(name, it)
fun updateName(name: String): Completable = rxCompletable {
room.updateName(name)
}

fun updateHistoryReadability(readability: RoomHistoryVisibility): Completable = completableBuilder<Unit> {
room.updateHistoryReadability(readability, it)
fun updateHistoryReadability(readability: RoomHistoryVisibility): Completable = rxCompletable {
room.updateHistoryReadability(readability)
}

fun updateJoinRule(joinRules: RoomJoinRules?, guestAccess: GuestAccess?): Completable = completableBuilder<Unit> {
room.updateJoinRule(joinRules, guestAccess, it)
fun updateJoinRule(joinRules: RoomJoinRules?, guestAccess: GuestAccess?): Completable = rxCompletable {
room.updateJoinRule(joinRules, guestAccess)
}

fun updateAvatar(avatarUri: Uri, fileName: String): Completable = completableBuilder<Unit> {
room.updateAvatar(avatarUri, fileName, it)
fun updateAvatar(avatarUri: Uri, fileName: String): Completable = rxCompletable {
room.updateAvatar(avatarUri, fileName)
}

fun deleteAvatar(): Completable = completableBuilder<Unit> {
room.deleteAvatar(it)
fun deleteAvatar(): Completable = rxCompletable {
room.deleteAvatar()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,41 +33,41 @@ interface StateService {
/**
* Update the topic of the room
*/
fun updateTopic(topic: String, callback: MatrixCallback<Unit>): Cancelable
suspend fun updateTopic(topic: String)

/**
* Update the name of the room
*/
fun updateName(name: String, callback: MatrixCallback<Unit>): Cancelable
suspend fun updateName(name: String)

/**
* Update the canonical alias of the room
* @param alias the canonical alias, or null to reset the canonical alias of this room
* @param altAliases the alternative aliases for this room. It should include the canonical alias if any.
*/
fun updateCanonicalAlias(alias: String?, altAliases: List<String>, callback: MatrixCallback<Unit>): Cancelable
suspend fun updateCanonicalAlias(alias: String?, altAliases: List<String>)

/**
* Update the history readability of the room
*/
fun updateHistoryReadability(readability: RoomHistoryVisibility, callback: MatrixCallback<Unit>): Cancelable
suspend fun updateHistoryReadability(readability: RoomHistoryVisibility)

/**
* Update the join rule and/or the guest access
*/
fun updateJoinRule(joinRules: RoomJoinRules?, guestAccess: GuestAccess?, callback: MatrixCallback<Unit>): Cancelable
suspend fun updateJoinRule(joinRules: RoomJoinRules?, guestAccess: GuestAccess?)

/**
* Update the avatar of the room
*/
fun updateAvatar(avatarUri: Uri, fileName: String, callback: MatrixCallback<Unit>): Cancelable
suspend fun updateAvatar(avatarUri: Uri, fileName: String)

/**
* Delete the avatar of the room
*/
fun deleteAvatar(callback: MatrixCallback<Unit>): Cancelable
suspend fun deleteAvatar()

fun sendStateEvent(eventType: String, stateKey: String?, body: JsonDict, callback: MatrixCallback<Unit>): Cancelable
suspend fun sendStateEvent(eventType: String, stateKey: String?, body: JsonDict)

fun getStateEvent(eventType: String, stateKey: QueryStringValue = QueryStringValue.NoCondition): Event?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import android.net.Uri
import androidx.lifecycle.LiveData
import com.squareup.inject.assisted.Assisted
import com.squareup.inject.assisted.AssistedInject
import org.matrix.android.sdk.api.MatrixCallback
import kotlinx.coroutines.withContext
import org.matrix.android.sdk.api.query.QueryStringValue
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.events.model.EventType
Expand All @@ -32,20 +32,14 @@ import org.matrix.android.sdk.api.session.room.model.RoomHistoryVisibility
import org.matrix.android.sdk.api.session.room.model.RoomJoinRules
import org.matrix.android.sdk.api.session.room.model.RoomJoinRulesContent
import org.matrix.android.sdk.api.session.room.state.StateService
import org.matrix.android.sdk.api.util.Cancelable
import org.matrix.android.sdk.api.util.JsonDict
import org.matrix.android.sdk.api.util.Optional
import org.matrix.android.sdk.internal.session.content.FileUploader
import org.matrix.android.sdk.internal.session.room.alias.AddRoomAliasTask
import org.matrix.android.sdk.internal.task.TaskExecutor
import org.matrix.android.sdk.internal.task.configureWith
import org.matrix.android.sdk.internal.task.launchToCallback
import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers
import org.matrix.android.sdk.internal.util.awaitCallback

internal class DefaultStateService @AssistedInject constructor(@Assisted private val roomId: String,
private val stateEventDataSource: StateEventDataSource,
private val taskExecutor: TaskExecutor,
private val sendStateTask: SendStateTask,
private val coroutineDispatchers: MatrixCoroutineDispatchers,
private val fileUploader: FileUploader,
Expand Down Expand Up @@ -73,45 +67,41 @@ internal class DefaultStateService @AssistedInject constructor(@Assisted private
return stateEventDataSource.getStateEventsLive(roomId, eventTypes, stateKey)
}

override fun sendStateEvent(
override suspend fun sendStateEvent(
eventType: String,
stateKey: String?,
body: JsonDict,
callback: MatrixCallback<Unit>
): Cancelable {
val params = SendStateTask.Params(
roomId = roomId,
stateKey = stateKey,
eventType = eventType,
body = body
)
return sendStateTask
.configureWith(params) {
this.callback = callback
}
.executeBy(taskExecutor)
body: JsonDict
) {
withContext(coroutineDispatchers.main) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why changing the context here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, didn't read executeBy(taskExecutor) internals properly.
TaskExecutor.execute ran this on the callback's thread (by transforming it with toDispatcher() internally). I'll remove the context change.

val params = SendStateTask.Params(
roomId = roomId,
stateKey = stateKey,
eventType = eventType,
body = body
)

sendStateTask.execute(params)
}
}

override fun updateTopic(topic: String, callback: MatrixCallback<Unit>): Cancelable {
return sendStateEvent(
override suspend fun updateTopic(topic: String) {
sendStateEvent(
eventType = EventType.STATE_ROOM_TOPIC,
body = mapOf("topic" to topic),
callback = callback,
stateKey = null
)
}

override fun updateName(name: String, callback: MatrixCallback<Unit>): Cancelable {
return sendStateEvent(
override suspend fun updateName(name: String) {
sendStateEvent(
eventType = EventType.STATE_ROOM_NAME,
body = mapOf("name" to name),
callback = callback,
stateKey = null
)
}

override fun updateCanonicalAlias(alias: String?, altAliases: List<String>, callback: MatrixCallback<Unit>): Cancelable {
return sendStateEvent(
override suspend fun updateCanonicalAlias(alias: String?, altAliases: List<String>) {
sendStateEvent(
eventType = EventType.STATE_ROOM_CANONICAL_ALIAS,
body = RoomCanonicalAliasContent(
canonicalAlias = alias,
Expand All @@ -123,64 +113,52 @@ internal class DefaultStateService @AssistedInject constructor(@Assisted private
// Sort for the cleanup
.sorted()
).toContent(),
callback = callback,
stateKey = null
)
}

override fun updateHistoryReadability(readability: RoomHistoryVisibility, callback: MatrixCallback<Unit>): Cancelable {
return sendStateEvent(
override suspend fun updateHistoryReadability(readability: RoomHistoryVisibility) {
sendStateEvent(
eventType = EventType.STATE_ROOM_HISTORY_VISIBILITY,
body = mapOf("history_visibility" to readability),
callback = callback,
stateKey = null
)
}

override fun updateJoinRule(joinRules: RoomJoinRules?, guestAccess: GuestAccess?, callback: MatrixCallback<Unit>): Cancelable {
return taskExecutor.executorScope.launchToCallback(coroutineDispatchers.main, callback) {
override suspend fun updateJoinRule(joinRules: RoomJoinRules?, guestAccess: GuestAccess?) {
withContext(coroutineDispatchers.main) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was launched this in the coroutineDispatchers.main context by taskExecutor.executorScope.launchToCallback(coroutineDispatchers.main, callback) before.
I'm happy to remove the explicit withContext though.

if (joinRules != null) {
awaitCallback<Unit> {
sendStateEvent(
eventType = EventType.STATE_ROOM_JOIN_RULES,
body = RoomJoinRulesContent(joinRules).toContent(),
callback = it,
stateKey = null
)
}
sendStateEvent(
eventType = EventType.STATE_ROOM_JOIN_RULES,
body = RoomJoinRulesContent(joinRules).toContent(),
stateKey = null
)
}
if (guestAccess != null) {
awaitCallback<Unit> {
sendStateEvent(
eventType = EventType.STATE_ROOM_GUEST_ACCESS,
body = RoomGuestAccessContent(guestAccess).toContent(),
callback = it,
stateKey = null
)
}
sendStateEvent(
eventType = EventType.STATE_ROOM_GUEST_ACCESS,
body = RoomGuestAccessContent(guestAccess).toContent(),
stateKey = null
)
}
}
}

override fun updateAvatar(avatarUri: Uri, fileName: String, callback: MatrixCallback<Unit>): Cancelable {
return taskExecutor.executorScope.launchToCallback(coroutineDispatchers.main, callback) {
override suspend fun updateAvatar(avatarUri: Uri, fileName: String) {
withContext(coroutineDispatchers.main) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

val response = fileUploader.uploadFromUri(avatarUri, fileName, "image/jpeg")
awaitCallback<Unit> {
sendStateEvent(
eventType = EventType.STATE_ROOM_AVATAR,
body = mapOf("url" to response.contentUri),
callback = it,
stateKey = null
)
}
sendStateEvent(
eventType = EventType.STATE_ROOM_AVATAR,
body = mapOf("url" to response.contentUri),
stateKey = null
)
}
}

override fun deleteAvatar(callback: MatrixCallback<Unit>): Cancelable {
return sendStateEvent(
override suspend fun deleteAvatar() {
sendStateEvent(
eventType = EventType.STATE_ROOM_AVATAR,
body = emptyMap(),
callback = callback,
stateKey = null
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,7 @@ class RoomDetailViewModel @AssistedInject constructor(
private fun handleSetNewAvatar(action: RoomDetailAction.SetAvatarAction) {
viewModelScope.launch(Dispatchers.IO) {
try {
awaitCallback<Unit> {
room.updateAvatar(action.newAvatarUri, action.newAvatarFileName, it)
}
room.updateAvatar(action.newAvatarUri, action.newAvatarFileName)
_viewEvents.post(RoomDetailViewEvents.ActionSuccess(action))
} catch (failure: Throwable) {
_viewEvents.post(RoomDetailViewEvents.ActionFailure(action, failure))
Expand Down Expand Up @@ -854,8 +852,8 @@ class RoomDetailViewModel @AssistedInject constructor(
}

private fun handleChangeTopicSlashCommand(changeTopic: ParsedCommand.ChangeTopic) {
launchSlashCommandFlow {
room.updateTopic(changeTopic.topic, it)
launchSlashCommandFlowSuspendable {
room.updateTopic(changeTopic.topic)
}
}

Expand All @@ -876,9 +874,9 @@ class RoomDetailViewModel @AssistedInject constructor(
?.content
?.toModel<PowerLevelsContent>() ?: return

launchSlashCommandFlow {
launchSlashCommandFlowSuspendable {
currentPowerLevelsContent.setUserPowerLevel(setUserPowerLevel.userId, setUserPowerLevel.powerLevel)
room.sendStateEvent(EventType.STATE_ROOM_POWER_LEVELS, null, currentPowerLevelsContent.toContent(), it)
room.sendStateEvent(EventType.STATE_ROOM_POWER_LEVELS, null, currentPowerLevelsContent.toContent())
}
}

Expand Down Expand Up @@ -920,6 +918,19 @@ class RoomDetailViewModel @AssistedInject constructor(
lambda.invoke(matrixCallback)
}

private fun launchSlashCommandFlowSuspendable(block: suspend () -> Unit) {
_viewEvents.post(RoomDetailViewEvents.SlashCommandHandled())
viewModelScope.launch {
val event = try {
block()
RoomDetailViewEvents.SlashCommandResultOk
} catch (failure: Exception) {
RoomDetailViewEvents.SlashCommandResultError(failure)
}
_viewEvents.post(event)
}
}

private fun handleSendReaction(action: RoomDetailAction.SendReaction) {
room.sendReaction(action.targetEventId, action.reaction)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ class RoomMemberProfileViewModel @AssistedInject constructor(@Assisted private v
viewModelScope.launch {
_viewEvents.post(RoomMemberProfileViewEvents.Loading())
try {
awaitCallback<Unit> {
room.sendStateEvent(EventType.STATE_ROOM_POWER_LEVELS, null, currentPowerLevelsContent.toContent(), it)
}
room.sendStateEvent(EventType.STATE_ROOM_POWER_LEVELS, null, currentPowerLevelsContent.toContent())
_viewEvents.post(RoomMemberProfileViewEvents.OnSetPowerLevelSuccess)
} catch (failure: Throwable) {
_viewEvents.post(RoomMemberProfileViewEvents.Failure(failure))
Expand Down
Loading