-
Notifications
You must be signed in to change notification settings - Fork 14
[MSDK-3287] GPP implementation #189
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,17 +1,23 @@ | ||||||||||||||||||||||||||||||||||||
| package com.usercentrics.reactnative | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| import com.facebook.react.bridge.* | ||||||||||||||||||||||||||||||||||||
| import com.facebook.react.modules.core.DeviceEventManagerModule | ||||||||||||||||||||||||||||||||||||
| import com.usercentrics.sdk.UsercentricsDisposableEvent | ||||||||||||||||||||||||||||||||||||
| import com.usercentrics.sdk.UsercentricsEvent | ||||||||||||||||||||||||||||||||||||
| import com.usercentrics.reactnative.api.UsercentricsProxy | ||||||||||||||||||||||||||||||||||||
| import com.usercentrics.reactnative.extensions.* | ||||||||||||||||||||||||||||||||||||
| import com.usercentrics.sdk.UsercentricsAnalyticsEventType | ||||||||||||||||||||||||||||||||||||
| import com.usercentrics.sdk.models.settings.UsercentricsConsentType | ||||||||||||||||||||||||||||||||||||
| import com.usercentrics.sdk.services.gpp.GppSectionChangePayload | ||||||||||||||||||||||||||||||||||||
| import com.usercentrics.sdk.services.tcf.TCFDecisionUILayer | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| internal class RNUsercentricsModule( | ||||||||||||||||||||||||||||||||||||
| reactContext: ReactApplicationContext, | ||||||||||||||||||||||||||||||||||||
| private val usercentricsProxy: UsercentricsProxy, | ||||||||||||||||||||||||||||||||||||
| private val reactContextProvider: ReactContextProvider, | ||||||||||||||||||||||||||||||||||||
| ) : RNUsercentricsModuleSpec(reactContext) { | ||||||||||||||||||||||||||||||||||||
| private var gppSectionChangeSubscription: UsercentricsDisposableEvent<GppSectionChangePayload>? = null | ||||||||||||||||||||||||||||||||||||
| private var gppSectionChangeListenersCount = 0 | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| override fun getName() = NAME | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
@@ -121,6 +127,22 @@ internal class RNUsercentricsModule( | |||||||||||||||||||||||||||||||||||
| promise.resolve(usercentricsProxy.instance.getUSPData().serialize()) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @ReactMethod | ||||||||||||||||||||||||||||||||||||
| override fun getGPPData(promise: Promise) { | ||||||||||||||||||||||||||||||||||||
| promise.resolve(usercentricsProxy.instance.getGPPData().serializeGppData()) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @ReactMethod | ||||||||||||||||||||||||||||||||||||
| override fun getGPPString(promise: Promise) { | ||||||||||||||||||||||||||||||||||||
| promise.resolve(usercentricsProxy.instance.getGPPString()) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @ReactMethod | ||||||||||||||||||||||||||||||||||||
| override fun setGPPConsent(sectionName: String, fieldName: String, value: ReadableMap) { | ||||||||||||||||||||||||||||||||||||
| val parsedValue = readableMapValueToAny(value) ?: return | ||||||||||||||||||||||||||||||||||||
| usercentricsProxy.instance.setGPPConsent(sectionName, fieldName, parsedValue) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+140
to
+144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: When the JS layer calls the GPP consent API with an explicit Severity Level: Major
|
||||||||||||||||||||||||||||||||||||
| @ReactMethod | |
| override fun setGPPConsent(sectionName: String, fieldName: String, value: ReadableMap) { | |
| val parsedValue = readableMapValueToAny(value) ?: return | |
| usercentricsProxy.instance.setGPPConsent(sectionName, fieldName, parsedValue) | |
| } | |
| @ReactMethod | |
| override fun setGPPConsent(sectionName: String, fieldName: String, value: ReadableMap) { | |
| if (!value.hasKey("value")) return | |
| val parsedValue = if (value.getType("value") == ReadableType.Null) { | |
| null | |
| } else { | |
| readableMapValueToAny(value) | |
| } | |
| usercentricsProxy.instance.setGPPConsent(sectionName, fieldName, parsedValue) | |
| } |
Steps of Reproduction ✅
1. From JS, call the public API `Usercentrics.setGPPConsent("usnat", "SaleOptOut", null)`
(defined in `src/Usercentrics.tsx:150-153`, which wraps the value into `{ value }` and
calls `RNUsercentricsModule.setGPPConsent`).
2. On Android, React Native bridges this call into
`RNUsercentricsModule.setGPPConsent(sectionName, fieldName, value: ReadableMap)` in
`android/src/main/java/com/usercentrics/reactnative/RNUsercentricsModule.kt:140-144`,
where `value` is a `ReadableMap` containing key `"value"` with type `ReadableType.Null`.
3. Inside `setGPPConsent`, the helper `readableMapValueToAny` at
`RNUsercentricsModule.kt:275-284` is invoked; because the `"value"` key is present and its
type is `ReadableType.Null`, this function returns `null`, causing the Elvis operator `?:
return` at line 142 to short‑circuit and exit `setGPPConsent` without calling
`usercentricsProxy.instance.setGPPConsent(...)`.
4. On iOS, the same JS call is bridged via `ios/RNUsercentricsModule.swift:157-160`, where
`setGPPConsent(_:fieldName:value:)` unwraps `value["value"] ?? NSNull()` and *always*
calls `usercentricsManager.setGPPConsent(...)`, passing `NSNull()` when the JS value is
null, so the GPP consent update/clear is applied on iOS but is a no‑op on Android.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** android/src/main/java/com/usercentrics/reactnative/RNUsercentricsModule.kt
**Line:** 140:144
**Comment:**
*Logic Error: When the JS layer calls the GPP consent API with an explicit `null` value (e.g. to clear a field), the Android bridge silently returns early because `readableMapValueToAny` returns `null` and `?: return` short-circuits, so the underlying SDK never receives the update, unlike the iOS implementation which forwards an `NSNull()`; this leads to inconsistent behavior and prevents clearing GPP values on Android.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Null gpp consent ignored 🐞 Bug ✓ Correctness
Android RNUsercentricsModule.setGPPConsent silently no-ops when JS passes a null value because readableMapValueToAny returns null and the method returns early. This prevents clearing a GPP field on Android and diverges from iOS which forwards the null (as NSNull).
Agent Prompt
### Issue description
Android `setGPPConsent` drops updates when the JS wrapper contains `{ value: null }`, because the code returns early on `null`.
### Issue Context
iOS forwards the wrapped value even when it is null (`NSNull()`), so Android/iOS behavior diverges and Android cannot clear a GPP field.
### Fix Focus Areas
- android/src/main/java/com/usercentrics/reactnative/RNUsercentricsModule.kt[141-144]
- android/src/main/java/com/usercentrics/reactnative/RNUsercentricsModule.kt[275-284]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[VALIDATION] readableMapValueToAny currently expects the incoming ReadableMap to have a 'value' key and silently returns when missing. This makes setGPPConsent a no-op if callers don't wrap the primitive in { value }. Consider: (a) accepting primitive values directly (overload or document expected shape), (b) returning an explicit error/log if 'value' key is missing so the caller learns why nothing happened, or (c) add input validation and a Promise rejection/throw for malformed input to avoid silently ignoring calls.
@ReactMethod
override fun setGPPConsent(sectionName: String, fieldName: String, value: ReadableMap) {
if (!value.hasKey("value")) {
// Option A: surface an explicit error so JS can react
throw IllegalArgumentException("setGPPConsent: 'value' key is missing from payload. Expected shape: { value: any }")
// or, if you prefer non-throwing behavior, log and return:
// Log.w(NAME, "setGPPConsent called without 'value' key. Expected shape: { value: any }")
// return
}
val parsedValue = readableMapValueToAny(value)
usercentricsProxy.instance.setGPPConsent(sectionName, fieldName, parsedValue)
}| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package com.usercentrics.reactnative.extensions | ||
|
|
||
| import com.facebook.react.bridge.Arguments | ||
| import com.facebook.react.bridge.WritableMap | ||
| import com.usercentrics.sdk.services.gpp.GppData | ||
| import com.usercentrics.sdk.services.gpp.GppSectionChangePayload | ||
|
|
||
| internal fun GppData.serializeGppData(): WritableMap { | ||
| val sectionsMap = Arguments.createMap() | ||
| sections.forEach { (sectionName, fields) -> | ||
| val fieldsMap = Arguments.createMap() | ||
| fields.forEach { (fieldName, value) -> | ||
| when (value) { | ||
| null -> fieldsMap.putNull(fieldName) | ||
| is Boolean -> fieldsMap.putBoolean(fieldName, value) | ||
| is Int -> fieldsMap.putInt(fieldName, value) | ||
| is Double -> fieldsMap.putDouble(fieldName, value) | ||
| is String -> fieldsMap.putString(fieldName, value) | ||
| else -> fieldsMap.putString(fieldName, value.toString()) | ||
| } | ||
|
Comment on lines
+10
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Gpp composite values lost Android getGPPData serialization stringifies any non-primitive section field values, losing structure for arrays/maps. Because Android setGPPConsent explicitly supports Map/Array inputs, setting composite values cannot round-trip back through getGPPData. Agent Prompt
|
||
| } | ||
| sectionsMap.putMap(sectionName, fieldsMap) | ||
| } | ||
|
|
||
| val result = Arguments.createMap() | ||
| result.putString("gppString", gppString) | ||
| result.putArray("applicableSections", applicableSections.serialize()) | ||
| result.putMap("sections", sectionsMap) | ||
| return result | ||
|
Comment on lines
+8
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [REFACTORING] Current serialization handles only Int/Double/Boolean/String and falls back to toString() for other numeric types or nested structures. Improve robustness by: (1) treating all numeric types via 'is Number' and mapping integers vs floats appropriately (e.g. detect integral numbers and use putInt, otherwise putDouble), (2) recursively serializing nested maps/arrays into WritableMap/WritableArray (instead of toString()) so complex section values are preserved, and (3) explicitly handling Long/Float/Short types that may appear in the SDK. import com.facebook.react.bridge.Arguments
import com.facebook.react.bridge.WritableArray
import com.facebook.react.bridge.WritableMap
import com.usercentrics.sdk.services.gpp.GppData
import com.usercentrics.sdk.services.gpp.GppSectionChangePayload
private fun Any?.toWritableValue(): Any? = when (this) {
null -> null
is Boolean -> this
is Number -> {
// Preserve integers as Int, others as Double
val doubleValue = this.toDouble()
if (doubleValue % 1.0 == 0.0 && doubleValue <= Int.MAX_VALUE && doubleValue >= Int.MIN_VALUE) {
doubleValue.toInt()
} else {
doubleValue
}
}
is Map<*, *> -> this.toWritableMap()
is Iterable<*> -> this.toWritableArray()
else -> this.toString()
}
private fun Map<*, *>.toWritableMap(): WritableMap {
val map = Arguments.createMap()
for ((key, value) in this) {
val name = key?.toString() ?: continue
when (val v = value.toWritableValue()) {
null -> map.putNull(name)
is Boolean -> map.putBoolean(name, v)
is Int -> map.putInt(name, v)
is Double -> map.putDouble(name, v)
is String -> map.putString(name, v)
is WritableMap -> map.putMap(name, v)
is WritableArray -> map.putArray(name, v)
else -> map.putString(name, v.toString())
}
}
return map
}
private fun Iterable<*>.toWritableArray(): WritableArray {
val array = Arguments.createArray()
for (item in this) {
when (val v = item.toWritableValue()) {
null -> array.pushNull()
is Boolean -> array.pushBoolean(v)
is Int -> array.pushInt(v)
is Double -> array.pushDouble(v)
is String -> array.pushString(v)
is WritableMap -> array.pushMap(v)
is WritableArray -> array.pushArray(v)
else -> array.pushString(v.toString())
}
}
return array
}
internal fun GppData.serializeGppData(): WritableMap {
val sectionsMap = Arguments.createMap()
sections.forEach { (sectionName, fields) ->
val fieldsMap = (fields as? Map<*, *>)?.toWritableMap() ?: Arguments.createMap()
sectionsMap.putMap(sectionName, fieldsMap)
}
return Arguments.createMap().apply {
putString("gppString", gppString)
putArray("applicableSections", applicableSections.serialize())
putMap("sections", sectionsMap)
}
}
internal fun GppSectionChangePayload.serializeGppPayload(): WritableMap {
return Arguments.createMap().apply {
putString("data", data)
}
} |
||
| } | ||
|
|
||
| internal fun GppSectionChangePayload.serializeGppPayload(): WritableMap { | ||
| val result = Arguments.createMap() | ||
| result.putString("data", data) | ||
| return result | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -219,7 +219,6 @@ private fun TCF2Settings.serialize(): WritableMap { | |
| "disabledSpecialFeatures" to disabledSpecialFeatures, | ||
| "firstLayerShowDescriptions" to firstLayerShowDescriptions, | ||
| "hideNonIabOnFirstLayer" to hideNonIabOnFirstLayer, | ||
| "resurfacePeriodEnded" to resurfacePeriodEnded, | ||
| "resurfacePurposeChanged" to resurfacePurposeChanged, | ||
| "resurfaceVendorAdded" to resurfaceVendorAdded, | ||
|
Comment on lines
219
to
223
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3. Missing resurfaceperiodended key Android TCF2Settings.serialize no longer includes resurfacePeriodEnded in the object returned to JS. The TypeScript TCF2Settings model still requires this field and iOS continues to provide it, causing Android/iOS payload divergence and missing data on Android. Agent Prompt
|
||
| "firstLayerDescription" to firstLayerDescription, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import Foundation | ||
| import Usercentrics | ||
|
|
||
| extension GppData { | ||
| func toDictionary() -> NSDictionary { | ||
| return [ | ||
| "gppString": self.gppString, | ||
| "applicableSections": self.applicableSections, | ||
| "sections": self.sections, | ||
| ] | ||
| } | ||
|
Comment on lines
+4
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [REFACTORING] toDictionary returns Swift collections directly — ensure values are converted to Foundation types expected by React bridge (NSArray/NSDictionary) and that nested values are serialized safely (numbers, booleans, nested maps/arrays). Prefer mapping applicableSections -> NSArray and sections -> NSDictionary by explicitly transforming elements to Foundation types to avoid unexpected bridging behavior at runtime. import Foundation
import Usercentrics
extension GppData {
func toDictionary() -> NSDictionary {
return [
"gppString": gppString as NSString,
"applicableSections": applicableSections as NSArray,
"sections": sections as NSDictionary
]
}
} |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import Foundation | ||
| import Usercentrics | ||
|
|
||
| extension GppSectionChangePayload { | ||
| func toDictionary() -> NSDictionary { | ||
| return [ | ||
| "data": self.data | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't turn
{ value: null }into a silent no-op.Line 142 collapses both “missing key” and explicit
nullintoreturn. The JS API acceptsunknown, andios/RNUsercentricsModule.swiftforwards the wrapped null value instead of swallowing it, so Android currently behaves differently for callers trying to clear a GPP field.🐛 Proposed fix
override fun setGPPConsent(sectionName: String, fieldName: String, value: ReadableMap) { - val parsedValue = readableMapValueToAny(value) ?: return - usercentricsProxy.instance.setGPPConsent(sectionName, fieldName, parsedValue) + if (!value.hasKey("value")) return + val parsedValue = readableMapValueToAny(value) + usercentricsProxy.instance.setGPPConsent(sectionName, fieldName, parsedValue) }📝 Committable suggestion
🤖 Prompt for AI Agents