From 335c764117a71f54f5ee1a18fa6c02e22a71747a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Kalici=C5=84ski?= <wojciech.kalicinski@pubnub.com> Date: Wed, 15 Jan 2025 17:48:54 +0100 Subject: [PATCH] New method for computing User updates that uses ETag Fixes the problem of overwriting custom data at regular intervals when `storeUserActivityInterval` is enabled. --- gradle/libs.versions.toml | 2 +- pubnub-chat-api/api/pubnub-chat-api.api | 34 +++++++++ .../commonMain/kotlin/com/pubnub/chat/User.kt | 65 ++++++++++++++++- .../com/pubnub/chat/internal/ChatImpl.kt | 38 ++++++---- .../com/pubnub/chat/internal/Constants.kt | 1 + .../com/pubnub/chat/internal/UserImpl.kt | 60 ++++++++++++++++ .../pubnub/integration/ChatIntegrationTest.kt | 7 +- .../pubnub/integration/UserIntegrationTest.kt | 72 ++++++++++++++++++- settings.gradle.kts | 1 + 9 files changed, 259 insertions(+), 21 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 05829652..09eb1bd3 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -6,7 +6,7 @@ ktlint = "12.1.0" dokka = "1.9.20" kotlinx_serialization = "1.7.3" kotlinx_coroutines = "1.9.0" -pubnub = "10.3.4" +pubnub = "10.4.0-dev" pubnub_swift = "8.2.4" [libraries] diff --git a/pubnub-chat-api/api/pubnub-chat-api.api b/pubnub-chat-api/api/pubnub-chat-api.api index 39624b73..c82ab4f8 100644 --- a/pubnub-chat-api/api/pubnub-chat-api.api +++ b/pubnub-chat-api/api/pubnub-chat-api.api @@ -330,6 +330,7 @@ public abstract interface class com/pubnub/chat/User { public static synthetic fun getChannelsRestrictions$default (Lcom/pubnub/chat/User;Ljava/lang/Integer;Lcom/pubnub/api/models/consumer/objects/PNPage;Ljava/util/Collection;ILjava/lang/Object;)Lcom/pubnub/kmp/PNFuture; public abstract fun getChat ()Lcom/pubnub/chat/Chat; public abstract fun getCustom ()Ljava/util/Map; + public abstract fun getETag ()Ljava/lang/String; public abstract fun getEmail ()Ljava/lang/String; public abstract fun getExternalId ()Ljava/lang/String; public abstract fun getId ()Ljava/lang/String; @@ -347,6 +348,7 @@ public abstract interface class com/pubnub/chat/User { public static synthetic fun setRestrictions$default (Lcom/pubnub/chat/User;Lcom/pubnub/chat/Channel;ZZLjava/lang/String;ILjava/lang/Object;)Lcom/pubnub/kmp/PNFuture; public abstract fun streamUpdates (Lkotlin/jvm/functions/Function1;)Ljava/lang/AutoCloseable; public abstract fun update (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/String;)Lcom/pubnub/kmp/PNFuture; + public abstract fun update (Lkotlin/jvm/functions/Function2;)Lcom/pubnub/kmp/PNFuture; public static synthetic fun update$default (Lcom/pubnub/chat/User;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lcom/pubnub/kmp/PNFuture; public abstract fun wherePresent ()Lcom/pubnub/kmp/PNFuture; } @@ -354,6 +356,38 @@ public abstract interface class com/pubnub/chat/User { public final class com/pubnub/chat/User$Companion { } +public final class com/pubnub/chat/User$UpdatedValues { + public fun <init> ()V + public fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/String;)V + public synthetic fun <init> (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/String;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun component1 ()Ljava/lang/String; + public final fun component2 ()Ljava/lang/String; + public final fun component3 ()Ljava/lang/String; + public final fun component4 ()Ljava/lang/String; + public final fun component5 ()Ljava/lang/Object; + public final fun component6 ()Ljava/lang/String; + public final fun component7 ()Ljava/lang/String; + public final fun copy (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/String;)Lcom/pubnub/chat/User$UpdatedValues; + public static synthetic fun copy$default (Lcom/pubnub/chat/User$UpdatedValues;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lcom/pubnub/chat/User$UpdatedValues; + public fun equals (Ljava/lang/Object;)Z + public final fun getCustom ()Ljava/lang/Object; + public final fun getEmail ()Ljava/lang/String; + public final fun getExternalId ()Ljava/lang/String; + public final fun getName ()Ljava/lang/String; + public final fun getProfileUrl ()Ljava/lang/String; + public final fun getStatus ()Ljava/lang/String; + public final fun getType ()Ljava/lang/String; + public fun hashCode ()I + public final fun setCustom (Ljava/lang/Object;)V + public final fun setEmail (Ljava/lang/String;)V + public final fun setExternalId (Ljava/lang/String;)V + public final fun setName (Ljava/lang/String;)V + public final fun setProfileUrl (Ljava/lang/String;)V + public final fun setStatus (Ljava/lang/String;)V + public final fun setType (Ljava/lang/String;)V + public fun toString ()Ljava/lang/String; +} + public abstract interface class com/pubnub/chat/config/ChatConfiguration { public abstract fun getCustomPayloads ()Lcom/pubnub/chat/config/CustomPayloads; public abstract fun getLogLevel ()Lcom/pubnub/chat/config/LogLevel; diff --git a/pubnub-chat-api/src/commonMain/kotlin/com/pubnub/chat/User.kt b/pubnub-chat-api/src/commonMain/kotlin/com/pubnub/chat/User.kt index 23595824..7205ecee 100644 --- a/pubnub-chat-api/src/commonMain/kotlin/com/pubnub/chat/User.kt +++ b/pubnub-chat-api/src/commonMain/kotlin/com/pubnub/chat/User.kt @@ -61,10 +61,17 @@ interface User { val type: String? /** - * Type of the user, like admin, member, guest. + * The moment in time when the data contained in this User object was updated on the server. */ val updated: String? + /** + * The eTag that was returned by the server with this User object. + * + * It is a random string that changes with each data update. + */ + val eTag: String? + /** * Timestamp for the last time the user information was updated or modified. */ @@ -98,6 +105,28 @@ interface User { type: String? = null, ): PNFuture<User> + /** + * Updates the metadata of the user with information provided in [updateAction]. + * + * Please note that `updateAction` will be called _at least_ once with the current data from the `User` object in + * the argument. Inside `updateAction`, new values for `User` fields should be computed and assigned into the + * context `UpdatedValues` object. + * + * In case the user's information has changed on the server since the original User object was retrieved, the + * `updateAction` will be called again with new User data that represents the current server state. This might + * happen multiple times until either new data is saved successfully, or the request fails. + * + * @param updateAction a function for computing new values for the User fields based on the provided `user` argument + * and saving it into the `UpdatedValues` context object. + * + * @return [PNFuture] containing the updated [User]. + */ + fun update( + updateAction: UpdatedValues.( + user: User + ) -> Unit + ): PNFuture<User> + /** * Deletes the user. If soft deletion is enabled, the user's data is retained but marked as inactive. * @@ -202,5 +231,39 @@ interface User { */ operator fun plus(update: PNUUIDMetadata): User + /** + * An object representing the fields to update in a [User] object. + */ + data class UpdatedValues( + /** + * The new value for [User.name]. + */ + var name: String? = null, + /** + * The new value for [User.externalId]. + */ + var externalId: String? = null, + /** + * The new value for [User.profileUrl]. + */ + var profileUrl: String? = null, + /** + * The new value for [User.email]. + */ + var email: String? = null, + /** + * The new value for [User.custom]. + */ + var custom: CustomObject? = null, + /** + * The new value for [User.status]. + */ + var status: String? = null, + /** + * The new value for [User.type]. + */ + var type: String? = null, + ) + companion object } diff --git a/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/ChatImpl.kt b/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/ChatImpl.kt index fdd0bf50..25e5e79d 100644 --- a/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/ChatImpl.kt +++ b/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/ChatImpl.kt @@ -224,7 +224,16 @@ class ChatImpl( if (user != null) { log.pnError(USER_ID_ALREADY_EXIST) } else { - setUserMetadata(id, name, externalId, profileUrl, email, custom, type, status) + setUserMetadata( + id = id, + name = name, + externalId = externalId, + profileUrl = profileUrl, + email = email, + custom = custom, + type = type, + status = status + ) } } } @@ -1074,10 +1083,6 @@ class ChatImpl( return config.pushNotifications } - private fun isValidId(id: String): Boolean { - return id.isNotEmpty() - } - private fun performSoftUserDelete(user: User): PNFuture<User> { val updatedUser = (user as UserImpl).copy(status = DELETED) return pubNub.setUUIDMetadata( @@ -1260,16 +1265,15 @@ class ChatImpl( } private fun saveTimeStampFunc(): PNFuture<Unit> { - val customWithUpdatedLastActiveTimestamp = buildMap { - currentUser.custom?.let { putAll(it) } - put(LAST_ACTIVE_TIMESTAMP, Clock.System.now().toEpochMilliseconds().toString()) - } - return pubNub.setUUIDMetadata( - uuid = currentUser.id, - custom = createCustomObject(customWithUpdatedLastActiveTimestamp), - includeCustom = true, - ).then { pnUUIDMetadataResult: PNUUIDMetadataResult -> - currentUser = UserImpl.fromDTO(this, pnUUIDMetadataResult.data) + return currentUser.update { user -> + this.custom = createCustomObject( + buildMap { + user.custom?.let { putAll(it) } + put(LAST_ACTIVE_TIMESTAMP, Clock.System.now().toEpochMilliseconds().toString()) + } + ) + }.then { updatedUser -> + currentUser = updatedUser } } @@ -1294,3 +1298,7 @@ class ChatImpl( } internal expect fun generateRandomUuid(): String + +internal fun isValidId(id: String): Boolean { + return id.isNotEmpty() +} diff --git a/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/Constants.kt b/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/Constants.kt index defa0dd3..cdcdef86 100644 --- a/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/Constants.kt +++ b/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/Constants.kt @@ -15,6 +15,7 @@ internal const val DELETED = "deleted" internal const val ORIGINAL_PUBLISHER = "originalPublisher" internal const val ORIGINAL_CHANNEL_ID = "originalChannelId" internal const val HTTP_ERROR_404 = 404 +internal const val HTTP_ERROR_412 = 412 internal const val INTERNAL_MODERATION_PREFIX = "PUBNUB_INTERNAL_MODERATION_" internal const val INTERNAL_USER_MODERATION_CHANNEL_PREFIX = "PUBNUB_INTERNAL_MODERATION." internal const val PUBNUB_INTERNAL_AUTOMODERATED = "PUBNUB_INTERNAL_AUTOMODERATED" diff --git a/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/UserImpl.kt b/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/UserImpl.kt index bd305b2e..9afeb4f6 100644 --- a/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/UserImpl.kt +++ b/pubnub-chat-impl/src/commonMain/kotlin/com/pubnub/chat/internal/UserImpl.kt @@ -2,6 +2,7 @@ package com.pubnub.chat.internal import co.touchlab.kermit.Logger import com.pubnub.api.PubNubException +import com.pubnub.api.endpoints.objects.uuid.SetUUIDMetadata import com.pubnub.api.models.consumer.objects.PNMembershipKey import com.pubnub.api.models.consumer.objects.PNPage import com.pubnub.api.models.consumer.objects.PNSortKey @@ -9,6 +10,7 @@ import com.pubnub.api.models.consumer.objects.membership.MembershipInclude import com.pubnub.api.models.consumer.objects.membership.PNChannelMembership import com.pubnub.api.models.consumer.objects.membership.PNChannelMembershipArrayResult import com.pubnub.api.models.consumer.objects.uuid.PNUUIDMetadata +import com.pubnub.api.models.consumer.objects.uuid.PNUUIDMetadataResult import com.pubnub.api.models.consumer.pubsub.objects.PNDeleteUUIDMetadataEventMessage import com.pubnub.api.models.consumer.pubsub.objects.PNSetUUIDMetadataEventMessage import com.pubnub.api.utils.Clock @@ -20,7 +22,9 @@ import com.pubnub.chat.Membership import com.pubnub.chat.User import com.pubnub.chat.internal.error.PubNubErrorMessage import com.pubnub.chat.internal.error.PubNubErrorMessage.CAN_NOT_STREAM_USER_UPDATES_ON_EMPTY_LIST +import com.pubnub.chat.internal.error.PubNubErrorMessage.FAILED_TO_CREATE_UPDATE_USER_DATA import com.pubnub.chat.internal.error.PubNubErrorMessage.MODERATION_CAN_BE_SET_ONLY_BY_CLIENT_HAVING_SECRET_KEY +import com.pubnub.chat.internal.error.PubNubErrorMessage.USER_NOT_EXIST import com.pubnub.chat.internal.restrictions.RestrictionImpl import com.pubnub.chat.internal.util.logErrorAndReturnException import com.pubnub.chat.internal.util.pnError @@ -33,6 +37,7 @@ import com.pubnub.kmp.asFuture import com.pubnub.kmp.catch import com.pubnub.kmp.createEventListener import com.pubnub.kmp.then +import com.pubnub.kmp.thenAsync import tryLong data class UserImpl( @@ -46,6 +51,7 @@ data class UserImpl( override val status: String? = null, override val type: String? = null, override val updated: String? = null, + override val eTag: String? = null, override val lastActiveTimestamp: Long? = null, ) : User { override val active: Boolean @@ -76,6 +82,14 @@ data class UserImpl( ) } + override fun update( + updateAction: User.UpdatedValues.( + user: User + ) -> Unit + ): PNFuture<User> { + return updateInternal(this, updateAction) + } + override fun delete(soft: Boolean): PNFuture<User?> { return chat.deleteUser(id, soft) } @@ -249,6 +263,38 @@ data class UserImpl( companion object { private val log = Logger.withTag("UserImpl") + private fun updateInternal( + user: User, + updateAction: User.UpdatedValues.( + user: User + ) -> Unit, + retriesLeft: Int = 1 + ): PNFuture<User> { + val updatedValues = User.UpdatedValues() + updatedValues.updateAction(user) + return user.setUserUpdatedValues(updatedValues) + .catch { + if (it is PubNubException && it.statusCode == HTTP_ERROR_412) { + Result.success(null) + } else { + Result.failure(it) + } + }.thenAsync { userDataResult: PNUUIDMetadataResult? -> + if (userDataResult != null) { + return@thenAsync fromDTO(user.chat as ChatInternal, userDataResult.data).asFuture() + } + user.chat.getUser(user.id).thenAsync { newUser: User? -> + if (newUser == null) { + log.pnError(USER_NOT_EXIST) + } else if (retriesLeft > 0) { + updateInternal(newUser, updateAction, retriesLeft - 1) + } else { + log.pnError(FAILED_TO_CREATE_UPDATE_USER_DATA) + } + } + } + } + internal fun fromDTO(chat: ChatInternal, user: PNUUIDMetadata): User = UserImpl( chat, id = user.id, @@ -260,6 +306,7 @@ data class UserImpl( updated = user.updated?.value, status = user.status?.value, type = user.type?.value, + eTag = user.eTag?.value, lastActiveTimestamp = user.custom?.value?.get(LAST_ACTIVE_TIMESTAMP)?.tryLong() ) @@ -301,5 +348,18 @@ data class UserImpl( } } +private fun User.setUserUpdatedValues(updatedValues: User.UpdatedValues): SetUUIDMetadata = chat.pubNub.setUUIDMetadata( + uuid = id, + name = updatedValues.name, + externalId = updatedValues.externalId, + profileUrl = updatedValues.profileUrl, + email = updatedValues.email, + custom = updatedValues.custom, + includeCustom = true, + type = updatedValues.type, + status = updatedValues.status, + ifMatchesEtag = eTag +) + internal val User.uuidFilterString get() = "uuid.id == '${this.id}'" internal val User.isInternalModerator get() = this.id == INTERNAL_MODERATOR_DATA_ID && this.type == INTERNAL_MODERATOR_DATA_TYPE diff --git a/pubnub-chat-impl/src/commonTest/kotlin/com/pubnub/integration/ChatIntegrationTest.kt b/pubnub-chat-impl/src/commonTest/kotlin/com/pubnub/integration/ChatIntegrationTest.kt index 89274ffd..5f5d8a91 100644 --- a/pubnub-chat-impl/src/commonTest/kotlin/com/pubnub/integration/ChatIntegrationTest.kt +++ b/pubnub-chat-impl/src/commonTest/kotlin/com/pubnub/integration/ChatIntegrationTest.kt @@ -116,7 +116,7 @@ class ChatIntegrationTest : BaseChatIntegrationTest() { fun createUser() = runTest { val user = chat.createUser(someUser).await() - assertEquals(someUser, user.asImpl().copy(updated = null, lastActiveTimestamp = null)) + assertEquals(someUser, user.asImpl().copy(updated = null, lastActiveTimestamp = null, eTag = null)) assertNotNull(user.updated) } @@ -132,7 +132,8 @@ class ChatIntegrationTest : BaseChatIntegrationTest() { randomString() to randomString() ), type = randomString(), - updated = null + updated = null, + eTag = null ) val updatedUser = chat.updateUser( @@ -146,7 +147,7 @@ class ChatIntegrationTest : BaseChatIntegrationTest() { expectedUser.type ).await() - assertEquals(expectedUser, updatedUser.asImpl().copy(updated = null, lastActiveTimestamp = null)) + assertEquals(expectedUser, updatedUser.asImpl().copy(updated = null, lastActiveTimestamp = null, eTag = null)) assertNotNull(updatedUser.updated) } diff --git a/pubnub-chat-impl/src/commonTest/kotlin/com/pubnub/integration/UserIntegrationTest.kt b/pubnub-chat-impl/src/commonTest/kotlin/com/pubnub/integration/UserIntegrationTest.kt index 5f17ea42..f2bca243 100644 --- a/pubnub-chat-impl/src/commonTest/kotlin/com/pubnub/integration/UserIntegrationTest.kt +++ b/pubnub-chat-impl/src/commonTest/kotlin/com/pubnub/integration/UserIntegrationTest.kt @@ -12,11 +12,15 @@ import com.pubnub.chat.internal.channel.ChannelImpl import com.pubnub.chat.membership.MembershipsResponse import com.pubnub.chat.restrictions.GetRestrictionsResponse import com.pubnub.chat.restrictions.Restriction +import com.pubnub.kmp.createCustomObject import com.pubnub.test.await +import com.pubnub.test.randomString import com.pubnub.test.test import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull import kotlin.test.assertTrue class UserIntegrationTest : BaseChatIntegrationTest() { @@ -123,6 +127,72 @@ class UserIntegrationTest : BaseChatIntegrationTest() { assertEquals(channelId01, secondRestriction.channelId) } + @Test + fun updateUser() = runTest { + val user = chat.createUser(someUser).await() + val expectedUser = user.asImpl().copy( + name = randomString(), + externalId = randomString(), + profileUrl = randomString(), + email = randomString(), + custom = mapOf( + randomString() to randomString() + ), + type = randomString(), + updated = null, + eTag = null + ) + + val updatedUser = chat.updateUser( + expectedUser.id, + expectedUser.name, + expectedUser.externalId, + expectedUser.profileUrl, + expectedUser.email, + expectedUser.custom?.let { createCustomObject(it) }, + expectedUser.status, + expectedUser.type + ).await() + + assertEquals(expectedUser, updatedUser.asImpl().copy(updated = null, lastActiveTimestamp = null, eTag = null)) + assertNotNull(updatedUser.updated) + } + + @Test + fun updateUser_withEtag() = runTest { + val user = chat.createUser( + someUser.id, + randomString(), + randomString(), + randomString(), + randomString(), + createCustomObject(mapOf("def" to randomString())), + randomString(), + randomString() + ).await() + + val updatedUser = user.update { previousUser -> + name = previousUser.name + "1" + externalId = previousUser.externalId + "1" + profileUrl = previousUser.profileUrl + "1" + email = previousUser.email + "1" + custom = createCustomObject(previousUser.custom!! + ("abc" to 1)) + type = previousUser.type + "1" + status = previousUser.status + "1" + }.await() + + assertEquals(user.name + "1", updatedUser.name) + assertEquals(user.externalId + "1", updatedUser.externalId) + assertEquals(user.profileUrl + "1", updatedUser.profileUrl) + assertEquals(user.email + "1", updatedUser.email) + assertEquals(user.custom!!["def"], updatedUser.custom!!["def"]) + assertEquals(1, (updatedUser.custom!!["abc"] as Number).toInt()) + assertEquals(user.type + "1", updatedUser.type) + assertEquals(user.status + "1", updatedUser.status) + assertNotEquals(user.updated, updatedUser.updated) + assertNotEquals(user.eTag, updatedUser.eTag) + } + @Test fun streamUpdatesOn() = runTest { val newName = "newName" @@ -138,7 +208,7 @@ class UserIntegrationTest : BaseChatIntegrationTest() { var dispose: AutoCloseable? = null pubnub.awaitSubscribe(listOf(someUser.id)) { dispose = UserImpl.streamUpdatesOn(listOf(someUser)) { - actualUpdates.add(it.map { it.asImpl().copy(updated = null) }) + actualUpdates.add(it.map { it.asImpl().copy(updated = null, eTag = null) }) } } diff --git a/settings.gradle.kts b/settings.gradle.kts index 1e4eed44..cde62604 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -14,6 +14,7 @@ rootProject.name = "pubnub-chat" dependencyResolutionManagement { repositories { mavenCentral() + mavenLocal() } }