Skip to content

Commit

Permalink
Changes after review.
Browse files Browse the repository at this point in the history
  • Loading branch information
marcin-cebo committed Nov 7, 2023
1 parent 8987e71 commit d079478
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.pubnub.api.CommonUtils
import com.pubnub.api.CommonUtils.emoji
import com.pubnub.api.CommonUtils.randomChannel
import com.pubnub.api.PubNub
import com.pubnub.api.PubNubError
import com.pubnub.api.crypto.CryptoModule
import com.pubnub.api.models.consumer.history.Action
import com.pubnub.api.models.consumer.history.HistoryMessageType
Expand Down Expand Up @@ -106,7 +107,7 @@ class HistoryIntegrationTest : BaseIntegrationTest() {
entry = JsonPrimitive(expectedMessage),
timetoken = result.timetoken,
meta = expectedMeta,
error = "Crypto is configured but message is not encrypted."
error = PubNubError.CRYPTO_IS_CONFIGURED_BUT_MESSAGE_IS_NOT_ENCRYPTED.message

This comment has been minimized.

Copy link
@wkal-pubnub

wkal-pubnub Nov 7, 2023

Contributor

could this actually take the enum value CRYPTO_IS_CONFIGURED_BUT_MESSAGE_IS_NOT_ENCRYPTED, and not the String (CRYPTO_IS_CONFIGURED_BUT_MESSAGE_IS_NOT_ENCRYPTED.message?

that way the user will not have to do String comparison and will know what to expect here

This comment has been minimized.

Copy link
@marcin-cebo

marcin-cebo Nov 8, 2023

Author Contributor

Yes, good point. I will change it.

)
)

Expand Down Expand Up @@ -247,7 +248,7 @@ class HistoryIntegrationTest : BaseIntegrationTest() {
meta = expectedMeta,
messageType = HistoryMessageType.Message,
actions = emptyMap<String, Map<String, List<Action>>>(),
error = "Crypto is configured but message is not encrypted."
error = PubNubError.CRYPTO_IS_CONFIGURED_BUT_MESSAGE_IS_NOT_ENCRYPTED.message
)

val expectedChannelsResponse: Map<String, List<PNFetchMessageItem>> =
Expand Down
5 changes: 5 additions & 0 deletions src/main/kotlin/com/pubnub/api/PubNubError.kt
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@ enum class PubNubError(private val code: Int, val message: String) {
"Encryption of empty data not allowed."
),

CRYPTO_IS_CONFIGURED_BUT_MESSAGE_IS_NOT_ENCRYPTED(
177,
"Crypto is configured but message is not encrypted."
),

;

override fun toString(): String {
Expand Down
8 changes: 4 additions & 4 deletions src/main/kotlin/com/pubnub/extension/JsonElement.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.pubnub.extension

import com.google.gson.JsonElement
import com.pubnub.api.PubNubError
import com.pubnub.api.crypto.CryptoModule
import com.pubnub.api.crypto.decryptString
import com.pubnub.api.managers.MapperManager
Expand All @@ -16,7 +17,6 @@ internal fun JsonElement.processHistoryMessage(
): Pair<JsonElement, String?> {

cryptoModule ?: return Pair(this, null)
val error: String?

val inputText = if (mapper.isJsonObject(this)) {
// property pn_other is used when we want to send encrypted Push Notification, not whole JSON object is encrypted but only value of pn_other property
Expand All @@ -25,7 +25,7 @@ internal fun JsonElement.processHistoryMessage(
mapper.elementToString(this, PN_OTHER)
} else {
// plain JSON object indicates that this is not encrypted message
error = logAndReturnDecryptionError()
val error = logAndReturnDecryptionError()
return Pair(this, error)
}
} else {
Expand All @@ -36,7 +36,7 @@ internal fun JsonElement.processHistoryMessage(
val outputText = try {
cryptoModule.decryptString(inputText!!)
} catch (e: Exception) {
error = logAndReturnDecryptionError()
val error = logAndReturnDecryptionError()
return Pair(this, error)
}

Expand All @@ -52,7 +52,7 @@ internal fun JsonElement.processHistoryMessage(
}

private fun logAndReturnDecryptionError(): String {
val errorMessage = "Crypto is configured but message is not encrypted."
val errorMessage = PubNubError.CRYPTO_IS_CONFIGURED_BUT_MESSAGE_IS_NOT_ENCRYPTED.message
log.warn(errorMessage)
return errorMessage
}

0 comments on commit d079478

Please sign in to comment.