Merge pull request #6600 from vector-im/bugfix/mna/lls-ended-too-soon

[Location Share] - Live is considered as ended while still active (PSG-617)
This commit is contained in:
Maxime NATUREL 2022-07-20 14:01:05 +02:00 committed by GitHub
commit 7639f158d7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 64 additions and 4 deletions

1
changelog.d/6596.bugfix Normal file
View File

@ -0,0 +1 @@
[Location Share] - Live is considered as ended while still active

View File

@ -50,6 +50,7 @@ import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo030
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo031 import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo031
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo032 import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo032
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo033 import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo033
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo034
import org.matrix.android.sdk.internal.util.Normalizer import org.matrix.android.sdk.internal.util.Normalizer
import org.matrix.android.sdk.internal.util.database.MatrixRealmMigration import org.matrix.android.sdk.internal.util.database.MatrixRealmMigration
import javax.inject.Inject import javax.inject.Inject
@ -58,7 +59,7 @@ internal class RealmSessionStoreMigration @Inject constructor(
private val normalizer: Normalizer private val normalizer: Normalizer
) : MatrixRealmMigration( ) : MatrixRealmMigration(
dbName = "Session", dbName = "Session",
schemaVersion = 33L, schemaVersion = 34L,
) { ) {
/** /**
* Forces all RealmSessionStoreMigration instances to be equal. * Forces all RealmSessionStoreMigration instances to be equal.
@ -101,5 +102,6 @@ internal class RealmSessionStoreMigration @Inject constructor(
if (oldVersion < 31) MigrateSessionTo031(realm).perform() if (oldVersion < 31) MigrateSessionTo031(realm).perform()
if (oldVersion < 32) MigrateSessionTo032(realm).perform() if (oldVersion < 32) MigrateSessionTo032(realm).perform()
if (oldVersion < 33) MigrateSessionTo033(realm).perform() if (oldVersion < 33) MigrateSessionTo033(realm).perform()
if (oldVersion < 34) MigrateSessionTo034(realm).perform()
} }
} }

View File

@ -0,0 +1,34 @@
/*
* Copyright (c) 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.migration
import io.realm.DynamicRealm
import org.matrix.android.sdk.internal.database.model.livelocation.LiveLocationShareAggregatedSummaryEntityFields
import org.matrix.android.sdk.internal.util.database.RealmMigrator
/**
* Migrating to:
* Live location sharing aggregated summary: adding new field startOfLiveTimestampMillis.
*/
internal class MigrateSessionTo034(realm: DynamicRealm) : RealmMigrator(realm, 34) {
override fun doMigrate(realm: DynamicRealm) {
realm.schema.get("LiveLocationShareAggregatedSummaryEntity")
?.addField(LiveLocationShareAggregatedSummaryEntityFields.START_OF_LIVE_TIMESTAMP_MILLIS, Long::class.java)
?.setNullable(LiveLocationShareAggregatedSummaryEntityFields.START_OF_LIVE_TIMESTAMP_MILLIS, true)
}
}

View File

@ -44,6 +44,8 @@ internal open class LiveLocationShareAggregatedSummaryEntity(
*/ */
var isActive: Boolean? = null, var isActive: Boolean? = null,
var startOfLiveTimestampMillis: Long? = null,
var endOfLiveTimestampMillis: Long? = null, var endOfLiveTimestampMillis: Long? = null,
/** /**

View File

@ -92,12 +92,14 @@ internal fun LiveLocationShareAggregatedSummaryEntity.Companion.findActiveLiveIn
roomId: String, roomId: String,
userId: String, userId: String,
ignoredEventId: String, ignoredEventId: String,
startOfLiveTimestampThreshold: Long,
): List<LiveLocationShareAggregatedSummaryEntity> { ): List<LiveLocationShareAggregatedSummaryEntity> {
return LiveLocationShareAggregatedSummaryEntity return LiveLocationShareAggregatedSummaryEntity
.whereRoomId(realm, roomId = roomId) .whereRoomId(realm, roomId = roomId)
.equalTo(LiveLocationShareAggregatedSummaryEntityFields.USER_ID, userId) .equalTo(LiveLocationShareAggregatedSummaryEntityFields.USER_ID, userId)
.equalTo(LiveLocationShareAggregatedSummaryEntityFields.IS_ACTIVE, true) .equalTo(LiveLocationShareAggregatedSummaryEntityFields.IS_ACTIVE, true)
.notEqualTo(LiveLocationShareAggregatedSummaryEntityFields.EVENT_ID, ignoredEventId) .notEqualTo(LiveLocationShareAggregatedSummaryEntityFields.EVENT_ID, ignoredEventId)
.lessThan(LiveLocationShareAggregatedSummaryEntityFields.START_OF_LIVE_TIMESTAMP_MILLIS, startOfLiveTimestampThreshold)
.findAll() .findAll()
.toList() .toList()
} }

View File

@ -84,11 +84,12 @@ internal class LiveLocationAggregationProcessor @Inject constructor(
val endOfLiveTimestampMillis = content.getBestTimestampMillis()?.let { it + (content.timeout ?: 0) } val endOfLiveTimestampMillis = content.getBestTimestampMillis()?.let { it + (content.timeout ?: 0) }
Timber.d("updating summary of id=$targetEventId with isActive=$isActive and endTimestamp=$endOfLiveTimestampMillis") Timber.d("updating summary of id=$targetEventId with isActive=$isActive and endTimestamp=$endOfLiveTimestampMillis")
aggregatedSummary.startOfLiveTimestampMillis = content.getBestTimestampMillis()
aggregatedSummary.endOfLiveTimestampMillis = endOfLiveTimestampMillis aggregatedSummary.endOfLiveTimestampMillis = endOfLiveTimestampMillis
aggregatedSummary.isActive = isActive aggregatedSummary.isActive = isActive
aggregatedSummary.userId = event.senderId aggregatedSummary.userId = event.senderId
deactivateAllPreviousBeacons(realm, roomId, event.senderId, targetEventId) deactivateAllPreviousBeacons(realm, roomId, event.senderId, targetEventId, content.getBestTimestampMillis() ?: 0)
if (isActive) { if (isActive) {
scheduleDeactivationAfterTimeout(targetEventId, roomId, endOfLiveTimestampMillis) scheduleDeactivationAfterTimeout(targetEventId, roomId, endOfLiveTimestampMillis)
@ -182,13 +183,20 @@ internal class LiveLocationAggregationProcessor @Inject constructor(
aggregatedSummary.relatedEventIds = RealmList(*updatedEventIds.toTypedArray()) aggregatedSummary.relatedEventIds = RealmList(*updatedEventIds.toTypedArray())
} }
private fun deactivateAllPreviousBeacons(realm: Realm, roomId: String, userId: String, currentEventId: String) { private fun deactivateAllPreviousBeacons(
realm: Realm,
roomId: String,
userId: String,
currentEventId: String,
currentEventTimestamp: Long
) {
LiveLocationShareAggregatedSummaryEntity LiveLocationShareAggregatedSummaryEntity
.findActiveLiveInRoomForUser( .findActiveLiveInRoomForUser(
realm = realm, realm = realm,
roomId = roomId, roomId = roomId,
userId = userId, userId = userId,
ignoredEventId = currentEventId ignoredEventId = currentEventId,
startOfLiveTimestampThreshold = currentEventTimestamp
) )
.forEach { it.isActive = false } .forEach { it.isActive = false }
} }

View File

@ -36,6 +36,7 @@ import org.matrix.android.sdk.test.fakes.FakeWorkManagerProvider
import org.matrix.android.sdk.test.fakes.givenEqualTo import org.matrix.android.sdk.test.fakes.givenEqualTo
import org.matrix.android.sdk.test.fakes.givenFindAll import org.matrix.android.sdk.test.fakes.givenFindAll
import org.matrix.android.sdk.test.fakes.givenFindFirst import org.matrix.android.sdk.test.fakes.givenFindFirst
import org.matrix.android.sdk.test.fakes.givenLessThan
import org.matrix.android.sdk.test.fakes.givenNotEqualTo import org.matrix.android.sdk.test.fakes.givenNotEqualTo
private const val A_SESSION_ID = "session_id" private const val A_SESSION_ID = "session_id"
@ -183,6 +184,7 @@ internal class LiveLocationAggregationProcessorTest {
aggregatedEntity.roomId shouldBeEqualTo A_ROOM_ID aggregatedEntity.roomId shouldBeEqualTo A_ROOM_ID
aggregatedEntity.userId shouldBeEqualTo A_SENDER_ID aggregatedEntity.userId shouldBeEqualTo A_SENDER_ID
aggregatedEntity.isActive shouldBeEqualTo true aggregatedEntity.isActive shouldBeEqualTo true
aggregatedEntity.startOfLiveTimestampMillis shouldBeEqualTo A_TIMESTAMP
aggregatedEntity.endOfLiveTimestampMillis shouldBeEqualTo A_TIMESTAMP + A_TIMEOUT_MILLIS aggregatedEntity.endOfLiveTimestampMillis shouldBeEqualTo A_TIMESTAMP + A_TIMEOUT_MILLIS
aggregatedEntity.lastLocationContent shouldBeEqualTo null aggregatedEntity.lastLocationContent shouldBeEqualTo null
previousEntities.forEach { entity -> previousEntities.forEach { entity ->
@ -404,6 +406,7 @@ internal class LiveLocationAggregationProcessorTest {
.givenNotEqualTo(LiveLocationShareAggregatedSummaryEntityFields.EVENT_ID, AN_EVENT_ID) .givenNotEqualTo(LiveLocationShareAggregatedSummaryEntityFields.EVENT_ID, AN_EVENT_ID)
.givenEqualTo(LiveLocationShareAggregatedSummaryEntityFields.USER_ID, A_SENDER_ID) .givenEqualTo(LiveLocationShareAggregatedSummaryEntityFields.USER_ID, A_SENDER_ID)
.givenEqualTo(LiveLocationShareAggregatedSummaryEntityFields.IS_ACTIVE, true) .givenEqualTo(LiveLocationShareAggregatedSummaryEntityFields.IS_ACTIVE, true)
.givenLessThan(LiveLocationShareAggregatedSummaryEntityFields.START_OF_LIVE_TIMESTAMP_MILLIS, A_TIMESTAMP)
.givenFindAll(summaryList) .givenFindAll(summaryList)
return summaryList return summaryList
} }

View File

@ -101,6 +101,14 @@ inline fun <reified T : RealmModel> RealmQuery<T>.givenIsNotNull(
return this return this
} }
inline fun <reified T : RealmModel> RealmQuery<T>.givenLessThan(
fieldName: String,
value: Long
): RealmQuery<T> {
every { lessThan(fieldName, value) } returns this
return this
}
/** /**
* Should be called on a mocked RealmObject and not on a real RealmObject so that the underlying final method is mocked. * Should be called on a mocked RealmObject and not on a real RealmObject so that the underlying final method is mocked.
*/ */