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.

Sign up for GitHub

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 PushRuleService to suspend functions #2414

Merged
merged 3 commits into from
Nov 19, 2020
Merged

Convert PushRuleService to suspend functions #2414

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94b135a
Convert PushRuleService to suspend functions
Dominaezzz Nov 13, 2020
92a6e9e
Remove redundant return
Dominaezzz Nov 18, 2020
796ba72
Reorder
Dominaezzz Nov 18, 2020
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
10 changes: 4 additions & 6 deletions matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushRuleService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@
*/
package org.matrix.android.sdk.api.pushrules

import org.matrix.android.sdk.api.MatrixCallback
import org.matrix.android.sdk.api.pushrules.rest.PushRule
import org.matrix.android.sdk.api.pushrules.rest.RuleSet
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.util.Cancelable

interface PushRuleService {
/**
Expand All @@ -29,13 +27,13 @@ interface PushRuleService {

fun getPushRules(scope: String = RuleScope.GLOBAL): RuleSet

fun updatePushRuleEnableStatus(kind: RuleKind, pushRule: PushRule, enabled: Boolean, callback: MatrixCallback<Unit>): Cancelable
suspend fun updatePushRuleEnableStatus(kind: RuleKind, pushRule: PushRule, enabled: Boolean)

fun addPushRule(kind: RuleKind, pushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable
suspend fun addPushRule(kind: RuleKind, pushRule: PushRule)

fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable
suspend fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule)

fun removePushRule(kind: RuleKind, pushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable
suspend fun removePushRule(kind: RuleKind, pushRule: PushRule)

fun addPushRuleListener(listener: PushRuleListener)

Expand Down
34 changes: 8 additions & 26 deletions .../main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@
package org.matrix.android.sdk.internal.session.notification

import com.zhuinden.monarchy.Monarchy
import org.matrix.android.sdk.api.MatrixCallback
import org.matrix.android.sdk.api.pushrules.PushRuleService
import org.matrix.android.sdk.api.pushrules.RuleKind
import org.matrix.android.sdk.api.pushrules.RuleSetKey
import org.matrix.android.sdk.api.pushrules.getActions
import org.matrix.android.sdk.api.pushrules.rest.PushRule
import org.matrix.android.sdk.api.pushrules.rest.RuleSet
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.util.Cancelable
import org.matrix.android.sdk.internal.database.mapper.PushRulesMapper
import org.matrix.android.sdk.internal.database.model.PushRulesEntity
import org.matrix.android.sdk.internal.database.query.where
Expand Down Expand Up @@ -103,37 +101,21 @@ internal class DefaultPushRuleService @Inject constructor(
)
}

override fun updatePushRuleEnableStatus(kind: RuleKind, pushRule: PushRule, enabled: Boolean, callback: MatrixCallback<Unit>): Cancelable {
override suspend fun updatePushRuleEnableStatus(kind: RuleKind, pushRule: PushRule, enabled: Boolean) {
// The rules will be updated, and will come back from the next sync response
return updatePushRuleEnableStatusTask
.configureWith(UpdatePushRuleEnableStatusTask.Params(kind, pushRule, enabled)) {
this.callback = callback
}
.executeBy(taskExecutor)
return updatePushRuleEnableStatusTask.execute(UpdatePushRuleEnableStatusTask.Params(kind, pushRule, enabled))
}

override fun addPushRule(kind: RuleKind, pushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable {
return addPushRuleTask
.configureWith(AddPushRuleTask.Params(kind, pushRule)) {
this.callback = callback
}
.executeBy(taskExecutor)
override suspend fun addPushRule(kind: RuleKind, pushRule: PushRule) {
return addPushRuleTask.execute(AddPushRuleTask.Params(kind, pushRule))
}

override fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable {
return updatePushRuleActionsTask
.configureWith(UpdatePushRuleActionsTask.Params(kind, oldPushRule, newPushRule)) {
this.callback = callback
}
.executeBy(taskExecutor)
override suspend fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule) {
return updatePushRuleActionsTask.execute(UpdatePushRuleActionsTask.Params(kind, oldPushRule, newPushRule))
}

override fun removePushRule(kind: RuleKind, pushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable {
return removePushRuleTask
.configureWith(RemovePushRuleTask.Params(kind, pushRule)) {
this.callback = callback
}
.executeBy(taskExecutor)
override suspend fun removePushRule(kind: RuleKind, pushRule: PushRule) {
return removePushRuleTask.execute(RemovePushRuleTask.Params(kind, pushRule))
Copy link
Member

Choose a reason for hiding this comment

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

In all those 4 methods, the return statement could be removed.

}

override fun removePushRuleListener(listener: PushRuleService.PushRuleListener) {
Expand Down
45 changes: 21 additions & 24 deletions ...a/im/vector/app/features/settings/VectorSettingsAdvancedNotificationPreferenceFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
*/
package im.vector.app.features.settings

import androidx.lifecycle.lifecycleScope
import androidx.preference.Preference
import im.vector.app.R
import im.vector.app.core.preference.PushRulePreference
import im.vector.app.core.preference.VectorPreference
import im.vector.app.core.utils.toast
import org.matrix.android.sdk.api.MatrixCallback
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.pushrules.RuleIds
import org.matrix.android.sdk.api.pushrules.rest.PushRuleAndKind
import javax.inject.Inject
Expand Down Expand Up @@ -50,29 +51,25 @@ class VectorSettingsAdvancedNotificationPreferenceFragment @Inject constructor()
if (newRule != null) {
displayLoadingView()

session.updatePushRuleActions(
ruleAndKind.kind,
preference.ruleAndKind?.pushRule ?: ruleAndKind.pushRule,
newRule,
object : MatrixCallback<Unit> {
override fun onSuccess(data: Unit) {
if (!isAdded) {
return
}
preference.setPushRule(ruleAndKind.copy(pushRule = newRule))
hideLoadingView()
}

override fun onFailure(failure: Throwable) {
if (!isAdded) {
return
}
hideLoadingView()
// Restore the previous value
refreshDisplay()
activity?.toast(errorFormatter.toHumanReadable(failure))
}
})
lifecycleScope.launch {
val result = runCatching {
session.updatePushRuleActions(ruleAndKind.kind,
preference.ruleAndKind?.pushRule ?: ruleAndKind.pushRule,
newRule)
}
if (!isAdded) {
return@launch
}
result.onSuccess {
preference.setPushRule(ruleAndKind.copy(pushRule = newRule))
}
hideLoadingView()
Copy link
Member

Choose a reason for hiding this comment

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

This is a detail, but now, I would rather call hideLoadingView() just after the check if Fragment is added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically the behaviour would be different (out of scope) but I'm not bothered if you're not bothered.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. I do not think it will introduce a bug if we reorder. Also we could use .fold() instead of onSuccess and onFailure which is a bit more verbose (but equivalent)

result.onFailure { failure ->
// Restore the previous value
refreshDisplay()
activity?.toast(errorFormatter.toHumanReadable(failure))
}
}
}
false
}
Expand Down
36 changes: 18 additions & 18 deletions ...main/java/im/vector/app/features/settings/VectorSettingsNotificationPreferenceFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import android.media.RingtoneManager
import android.net.Uri
import android.os.Parcelable
import android.widget.Toast
import androidx.lifecycle.lifecycleScope
import androidx.preference.Preference
import androidx.preference.SwitchPreference
import im.vector.app.R
Expand All @@ -37,6 +38,7 @@ import im.vector.app.core.utils.isIgnoringBatteryOptimizations
import im.vector.app.core.utils.requestDisablingBatteryOptimization
import im.vector.app.features.notifications.NotificationUtils
import im.vector.app.push.fcm.FcmHelper
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.MatrixCallback
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.pushrules.RuleIds
Expand Down Expand Up @@ -318,24 +320,22 @@ class VectorSettingsNotificationPreferenceFragment @Inject constructor(
.find { it.ruleId == RuleIds.RULE_ID_DISABLE_ALL }
?.let {
// Trick, we must enable this room to disable notifications
pushRuleService.updatePushRuleEnableStatus(RuleKind.OVERRIDE,
it,
!switchPref.isChecked,
object : MatrixCallback<Unit> {
override fun onSuccess(data: Unit) {
// Push rules will be updated from the sync
}

override fun onFailure(failure: Throwable) {
if (!isAdded) {
return
}

// revert the check box
switchPref.isChecked = !switchPref.isChecked
Toast.makeText(activity, R.string.unknown_error, Toast.LENGTH_SHORT).show()
}
})
lifecycleScope.launch {
try {
pushRuleService.updatePushRuleEnableStatus(RuleKind.OVERRIDE,
it,
!switchPref.isChecked)
// Push rules will be updated from the sync
} catch (failure: Throwable) {
if (!isAdded) {
return@launch
}

// revert the check box
switchPref.isChecked = !switchPref.isChecked
Toast.makeText(activity, R.string.unknown_error, Toast.LENGTH_SHORT).show()
}
}
}
}
}
19 changes: 8 additions & 11 deletions vector/src/main/java/im/vector/app/features/settings/troubleshoot/TestAccountSettings.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import androidx.activity.result.ActivityResultLauncher
import im.vector.app.R
import im.vector.app.core.di.ActiveSessionHolder
import im.vector.app.core.resources.StringProvider
import org.matrix.android.sdk.api.MatrixCallback
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.pushrules.RuleIds
import org.matrix.android.sdk.api.pushrules.RuleKind
import javax.inject.Inject
Expand Down Expand Up @@ -48,16 +49,12 @@ class TestAccountSettings @Inject constructor(private val stringProvider: String
override fun doFix() {
if (manager?.diagStatus == TestStatus.RUNNING) return // wait before all is finished

session.updatePushRuleEnableStatus(RuleKind.OVERRIDE, defaultRule, !defaultRule.enabled,
object : MatrixCallback<Unit> {
override fun onSuccess(data: Unit) {
manager?.retry(activityResultLauncher)
}

override fun onFailure(failure: Throwable) {
manager?.retry(activityResultLauncher)
}
})
GlobalScope.launch {
runCatching {
session.updatePushRuleEnableStatus(RuleKind.OVERRIDE, defaultRule, !defaultRule.enabled)
}
manager?.retry(activityResultLauncher)
}
}
}
status = TestStatus.FAILED
Expand Down