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

Implements MSC3881 (enabled and device_id fields for Pusher API) #7217

Merged
merged 22 commits into from
Oct 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions changelog.d/7217.wip
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implements MSC3881: Parses `enabled` and `device_id` fields from updated Pusher API
10 changes: 6 additions & 4 deletions library/ui-strings/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1702,13 +1702,15 @@
<string name="settings_push_rules_no_rules">No push rules defined</string>
<string name="settings_push_gateway_no_pushers">No registered push gateways</string>

<string name="push_gateway_item_app_id">app_id:</string>
<string name="push_gateway_item_push_key">push_key:</string>
<string name="push_gateway_item_app_display_name">app_display_name:</string>
<string name="push_gateway_item_device_name">session_name:</string>
<string name="push_gateway_item_app_id">App ID:</string>
<string name="push_gateway_item_push_key">Push Key:</string>
<string name="push_gateway_item_app_display_name">App Display Name:</string>
<string name="push_gateway_item_device_name">Session Display Name:</string>
<string name="push_gateway_item_device_id">Session ID:</string>
<string name="push_gateway_item_url">Url:</string>
<string name="push_gateway_item_format">Format:</string>
<string name="push_gateway_item_profile_tag">Profile tag:</string>
<string name="push_gateway_item_enabled">Enabled:</string>

<string name="preference_voice_and_video">Voice &amp; Video</string>
<string name="preference_root_help_about">Help &amp; About</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ data class HttpPusher(
*/
val url: String,

/**
* Whether the pusher should actively create push notifications.
*/
val enabled: Boolean,

/**
* The device ID of the session that registered the pusher.
*/
val deviceId: String,

/**
* If true, the homeserver should add another pusher with the given pushkey and App ID in addition
* to any others with different user IDs. Otherwise, the homeserver must remove any other pushers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ data class Pusher(
val profileTag: String? = null,
val lang: String?,
val data: PusherData,

val state: PusherState
val enabled: Boolean,
val deviceId: String?,
val state: PusherState,
) {
companion object {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ internal object PushersMapper {
profileTag = pushEntity.profileTag,
lang = pushEntity.lang,
data = PusherData(pushEntity.data?.url, pushEntity.data?.format),
state = pushEntity.state
enabled = pushEntity.enabled,
deviceId = pushEntity.deviceId,
state = pushEntity.state,
)
}

Expand All @@ -46,7 +48,9 @@ internal object PushersMapper {
deviceDisplayName = pusher.deviceDisplayName,
profileTag = pusher.profileTag,
lang = pusher.lang,
data = PusherDataEntity(pusher.data?.url, pusher.data?.format)
data = PusherDataEntity(pusher.data?.url, pusher.data?.format),
enabled = pusher.enabled,
deviceId = pusher.deviceId,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@ package org.matrix.android.sdk.internal.database.model
import io.realm.RealmObject
import org.matrix.android.sdk.api.session.pushers.PusherState

// TODO
// at java.lang.Thread.run(Thread.java:764)
// Caused by: java.lang.IllegalArgumentException: 'value' is not a valid managed object.
// at io.realm.ProxyState.checkValidObject(ProxyState.java:213)
// at io.realm.im_vector_matrix_android_internal_database_model_PusherEntityRealmProxy
// .realmSet$data(im_vector_matrix_android_internal_database_model_PusherEntityRealmProxy.java:413)
// at org.matrix.android.sdk.internal.database.model.PusherEntity.setData(PusherEntity.kt:16)
// at org.matrix.android.sdk.internal.session.pushers.AddHttpPusherWorker$doWork$$inlined$fold$lambda$2.execute(AddHttpPusherWorker.kt:70)
// at io.realm.Realm.executeTransaction(Realm.java:1493)
internal open class PusherEntity(
var pushKey: String = "",
var kind: String? = null,
Expand All @@ -35,7 +26,9 @@ internal open class PusherEntity(
var deviceDisplayName: String? = null,
var profileTag: String? = null,
var lang: String? = null,
var data: PusherDataEntity? = null
var data: PusherDataEntity? = null,
var enabled: Boolean = true,
var deviceId: String? = null,
) : RealmObject() {
private var stateStr: String = PusherState.UNREGISTERED.name

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ internal class DefaultAddPusherTask @Inject constructor(
private val requestExecutor: RequestExecutor,
private val globalErrorReceiver: GlobalErrorReceiver
) : AddPusherTask {

override suspend fun execute(params: AddPusherTask.Params) {
val pusher = params.pusher
try {
Expand Down Expand Up @@ -71,6 +72,8 @@ internal class DefaultAddPusherTask @Inject constructor(
echo.profileTag = pusher.profileTag
echo.data?.format = pusher.data?.format
echo.data?.url = pusher.data?.url
echo.enabled = pusher.enabled
echo.deviceId = pusher.deviceId
echo.state = PusherState.REGISTERED
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ internal class DefaultPushersService @Inject constructor(
appDisplayName = appDisplayName,
deviceDisplayName = deviceDisplayName,
data = JsonPusherData(url, EVENT_ID_ONLY.takeIf { withEventIdOnly }),
append = append
append = append,
enabled = enabled,
deviceId = deviceId,
)

override suspend fun addEmailPusher(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import java.security.InvalidParameterException
* "device_display_name": "Alice's Phone",
* "profile_tag": "xyz",
* "lang": "en-US",
* "enabled": true,
* "device_id": "abc123",
* "data": {
* "url": "https://example.com/_matrix/push/v1/notify"
* }
Expand Down Expand Up @@ -112,7 +114,19 @@ internal data class JsonPusher(
* The default is false.
*/
@Json(name = "append")
val append: Boolean? = false
val append: Boolean? = false,

/**
* Whether the pusher should actively create push notifications.
*/
@Json(name = "org.matrix.msc3881.enabled")
val enabled: Boolean = true,

/**
* The device_id of the session that registered the pusher.
*/
@Json(name = "org.matrix.msc3881.device_id")
val deviceId: String? = null,
) {
init {
// Do some parameter checks. It's ok to throw Exception, to inform developer of the problem
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright 2022 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.matrix.android.sdk.internal.database.mapper

import org.amshove.kluent.shouldBeEqualTo
import org.junit.Test
import org.matrix.android.sdk.test.fixtures.JsonPusherFixture.aJsonPusher
import org.matrix.android.sdk.test.fixtures.PusherEntityFixture.aPusherEntity

class PushersMapperTest {

@Test
fun `when mapping PusherEntity, then it is mapped into Pusher successfully`() {
val pusherEntity = aPusherEntity()

val mappedPusher = PushersMapper.map(pusherEntity)

mappedPusher.pushKey shouldBeEqualTo pusherEntity.pushKey
mappedPusher.kind shouldBeEqualTo pusherEntity.kind.orEmpty()
mappedPusher.appId shouldBeEqualTo pusherEntity.appId
mappedPusher.appDisplayName shouldBeEqualTo pusherEntity.appDisplayName
mappedPusher.deviceDisplayName shouldBeEqualTo pusherEntity.deviceDisplayName
mappedPusher.profileTag shouldBeEqualTo pusherEntity.profileTag
mappedPusher.lang shouldBeEqualTo pusherEntity.lang
mappedPusher.data.url shouldBeEqualTo pusherEntity.data?.url
mappedPusher.data.format shouldBeEqualTo pusherEntity.data?.format
mappedPusher.enabled shouldBeEqualTo pusherEntity.enabled
mappedPusher.deviceId shouldBeEqualTo pusherEntity.deviceId
mappedPusher.state shouldBeEqualTo pusherEntity.state
}

@Test
fun `when mapping JsonPusher, then it is mapped into Pusher successfully`() {
val jsonPusher = aJsonPusher()

val mappedPusherEntity = PushersMapper.map(jsonPusher)

mappedPusherEntity.pushKey shouldBeEqualTo jsonPusher.pushKey
mappedPusherEntity.kind shouldBeEqualTo jsonPusher.kind
mappedPusherEntity.appId shouldBeEqualTo jsonPusher.appId
mappedPusherEntity.appDisplayName shouldBeEqualTo jsonPusher.appDisplayName
mappedPusherEntity.deviceDisplayName shouldBeEqualTo jsonPusher.deviceDisplayName
mappedPusherEntity.profileTag shouldBeEqualTo jsonPusher.profileTag
mappedPusherEntity.lang shouldBeEqualTo jsonPusher.lang
mappedPusherEntity.data?.url shouldBeEqualTo jsonPusher.data?.url
mappedPusherEntity.data?.format shouldBeEqualTo jsonPusher.data?.format
mappedPusherEntity.enabled shouldBeEqualTo jsonPusher.enabled
mappedPusherEntity.deviceId shouldBeEqualTo jsonPusher.deviceId
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class DefaultAddPusherTaskTest {
}

@Test
fun `given a persisted pusher when adding Pusher then updates api and mutates persisted result with Registered state`() {
fun `given a persisted pusher, when adding Pusher, then updates api and mutates persisted result with Registered state`() {
val realmResult = PusherEntity(appDisplayName = null)
monarchy.givenWhereReturns(result = realmResult)
.givenEqualTo(PusherEntityFields.PUSH_KEY, A_JSON_PUSHER.pushKey)
Expand All @@ -85,7 +85,7 @@ class DefaultAddPusherTaskTest {
}

@Test
fun `given a persisted push entity and SetPush API fails when adding Pusher then mutates persisted result with Failed registration state and rethrows`() {
fun `given a persisted push entity and SetPush API fails, when adding Pusher, then mutates persisted result with Failed registration state and rethrows`() {
val realmResult = PusherEntity()
monarchy.givenWhereReturns(result = realmResult)
.givenEqualTo(PusherEntityFields.PUSH_KEY, A_JSON_PUSHER.pushKey)
Expand All @@ -99,7 +99,7 @@ class DefaultAddPusherTaskTest {
}

@Test
fun `given no persisted push entity and SetPush API fails when adding Pusher then rethrows error`() {
fun `given no persisted push entity and SetPush API fails, when adding Pusher, then rethrows error`() {
monarchy.givenWhereReturns<PusherEntity>(result = null)
.givenEqualTo(PusherEntityFields.PUSH_KEY, A_JSON_PUSHER.pushKey)
pushersAPI.givenSetPusherErrors(SocketException())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright 2022 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.matrix.android.sdk.test.fixtures

import org.matrix.android.sdk.internal.session.pushers.JsonPusher
import org.matrix.android.sdk.internal.session.pushers.JsonPusherData

internal object JsonPusherFixture {

fun aJsonPusher(
pushKey: String = "",
kind: String? = null,
appId: String = "",
appDisplayName: String? = null,
deviceDisplayName: String? = null,
profileTag: String? = null,
lang: String? = null,
data: JsonPusherData? = null,
append: Boolean? = false,
enabled: Boolean = true,
deviceId: String? = null,
) = JsonPusher(
pushKey,
kind,
appId,
appDisplayName,
deviceDisplayName,
profileTag,
lang,
data,
append,
enabled,
deviceId,
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright 2022 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.matrix.android.sdk.test.fixtures

import org.matrix.android.sdk.internal.database.model.PusherDataEntity
import org.matrix.android.sdk.internal.database.model.PusherEntity

internal object PusherEntityFixture {

fun aPusherEntity(
pushKey: String = "",
kind: String? = null,
appId: String = "",
appDisplayName: String? = null,
deviceDisplayName: String? = null,
profileTag: String? = null,
lang: String? = null,
data: PusherDataEntity? = null,
enabled: Boolean = true,
deviceId: String? = null,
) = PusherEntity(
pushKey,
kind,
appId,
appDisplayName,
deviceDisplayName,
profileTag,
lang,
data,
enabled,
deviceId,
)
}
10 changes: 10 additions & 0 deletions vector-app/src/fdroid/java/im/vector/app/di/FlavorModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import im.vector.app.core.pushers.FcmHelper
import im.vector.app.core.resources.AppNameProvider
import im.vector.app.core.resources.DefaultAppNameProvider
import im.vector.app.core.resources.DefaultLocaleProvider
import im.vector.app.core.resources.LocaleProvider
import im.vector.app.core.services.GuardServiceStarter
import im.vector.app.fdroid.service.FDroidGuardServiceStarter
import im.vector.app.features.home.NightlyProxy
Expand Down Expand Up @@ -59,4 +63,10 @@ abstract class FlavorModule {

@Binds
abstract fun bindsFcmHelper(fcmHelper: FdroidFcmHelper): FcmHelper

@Binds
abstract fun bindsLocaleProvider(localeProvider: DefaultLocaleProvider): LocaleProvider

@Binds
abstract fun bindsAppNameProvider(appNameProvider: DefaultAppNameProvider): AppNameProvider
}
10 changes: 10 additions & 0 deletions vector-app/src/gplay/java/im/vector/app/di/FlavorModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import im.vector.app.GoogleFlavorLegals
import im.vector.app.core.pushers.FcmHelper
import im.vector.app.core.resources.AppNameProvider
import im.vector.app.core.resources.DefaultAppNameProvider
import im.vector.app.core.resources.DefaultLocaleProvider
import im.vector.app.core.resources.LocaleProvider
import im.vector.app.core.services.GuardServiceStarter
import im.vector.app.features.home.NightlyProxy
import im.vector.app.features.settings.legals.FlavorLegals
Expand All @@ -46,6 +50,12 @@ abstract class FlavorModule {
@Binds
abstract fun bindsFcmHelper(fcmHelper: GoogleFcmHelper): FcmHelper

@Binds
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to add those bindings in FDroid variant as well, no (in the dedicated FlavorModule)?

abstract fun bindsLocaleProvider(localeProvider: DefaultLocaleProvider): LocaleProvider

@Binds
abstract fun bindsAppNameProvider(appNameProvider: DefaultAppNameProvider): AppNameProvider

@Binds
abstract fun bindsFlavorLegals(legals: GoogleFlavorLegals): FlavorLegals
}
Loading