Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class DataplanBlockingUserTests : BaseKitOptionsTest() {
// make sure these are the attributes that are being removed
count++
}
attributeListenerKitKit.removeUserAttribute = {
attributeListenerKitKit.removeUserAttributeListener = {
assertTrue(allowedAttributes.containsKey(it))
assertFalse(blockedAttributes.containsKey(it))
count++
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.mparticle.kits.testkits

import com.mparticle.MParticle
import com.mparticle.kits.FilteredMParticleUser
import com.mparticle.kits.KitIntegration.AttributeListener
import com.mparticle.kits.KitIntegration.LogoutListener
import com.mparticle.kits.ReportingMessage
Expand All @@ -15,7 +16,7 @@ open class AttributeListenerTestKit :
var supportsAttributeLists: (() -> Boolean)? = null
var setAllUserAttributes: ((userAttributes: Map<String, String>?, userAttributeLists: Map<String, List<String>>?) -> Unit)? =
null
var removeUserAttribute: ((key: String?) -> Unit)? = null
var removeUserAttributeListener: ((key: String?) -> Unit)? = null
var setUserIdentity: ((identityType: MParticle.IdentityType?, identity: String?) -> Unit)? =
null
var removeUserIdentity: ((identityType: MParticle.IdentityType?) -> Unit)? = null
Expand Down Expand Up @@ -61,8 +62,11 @@ open class AttributeListenerTestKit :
onIdentityReceived?.invoke(identityType, null)
}

override fun removeUserAttribute(key: String) {
removeUserAttribute?.invoke(key)
override fun onRemoveUserAttribute(
key: String,
user: FilteredMParticleUser,
) {
removeUserAttributeListener?.invoke(key)
onAttributeReceived?.invoke(key, null)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,29 +361,41 @@ public interface LogoutListener {
}

/**
* Kits should implement this interface when their underlying service has the notion
* of a user with attributes.
* Temporary shared contract used while {@link AttributeListener} behavior is migrated to
* {@link UserAttributeListener}. Factoring out common API surface lets the SDK land incremental changes
* and smaller pull requests instead of a single large refactor.
* <p>
* Kits implement {@link AttributeListener} and/or {@link UserAttributeListener}; they do not implement this
* type directly.
*/
@Deprecated
public interface AttributeListener {

void setUserAttribute(String attributeKey, String attributeValue);

void setUserAttributeList(String attributeKey, List<String> attributeValueList);
public interface BaseAttributeListener {

/**
* Indicate to the mParticle Kit framework if this AttributeListener supports attribute-values as lists.
* Indicate to the mParticle Kit framework if this listener supports attribute-values as lists.
* <p>
* If an AttributeListener returns false, the setUserAttributeList method will never be called. Instead, setUserAttribute
* will be called with the attribute-value lists combined as a csv.
* If false, list-specific APIs are not used; values are passed via scalar/csv paths instead.
*
* @return true if this AttributeListener supports attribute values as lists.
* @return true if this listener supports attribute values as lists.
*/
boolean supportsAttributeLists();

void setAllUserAttributes(Map<String, String> userAttributes, Map<String, List<String>> userAttributeLists);
/**
* Called when a user attribute is removed for the current user.
*
* @param key attribute key
* @param user filtered user context for this kit
*/
void onRemoveUserAttribute(String key, FilteredMParticleUser user);
}

void removeUserAttribute(String key);
public interface AttributeListener extends BaseAttributeListener {

void setUserAttribute(String attributeKey, String attributeValue);

void setUserAttributeList(String attributeKey, List<String> attributeValueList);

void setAllUserAttributes(Map<String, String> userAttributes, Map<String, List<String>> userAttributeLists);

void setUserIdentity(MParticle.IdentityType identityType, String identity);

Expand Down Expand Up @@ -540,12 +552,10 @@ public interface IdentityListener {

}

public interface UserAttributeListener {
public interface UserAttributeListener extends BaseAttributeListener {

void onIncrementUserAttribute(String key, Number incrementedBy, String value, FilteredMParticleUser user);

void onRemoveUserAttribute(String key, FilteredMParticleUser user);

void onSetUserAttribute(String key, Object value, FilteredMParticleUser user);

void onSetUserTag(String key, FilteredMParticleUser user);
Expand All @@ -554,8 +564,6 @@ public interface UserAttributeListener {

void onSetAllUserAttributes(Map<String, String> userAttributes, Map<String, List<String>> userAttributeLists, FilteredMParticleUser user);

boolean supportsAttributeLists();

void onConsentStateUpdated(ConsentState oldState, ConsentState newState, FilteredMParticleUser user);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,9 @@ public void onUserAttributesReceived(Map<String, String> userAttributes, Map<Str
userAttributes);
Map<String, List<String>> filteredAttributeLists = (Map<String, List<String>>) KitConfiguration.filterAttributes(provider.getConfiguration().getUserAttributeFilters(),
userAttributeLists);
boolean supportsAttributeLists = ((KitIntegration.BaseAttributeListener) provider).supportsAttributeLists();
Comment thread
denischilik marked this conversation as resolved.
if (provider instanceof KitIntegration.AttributeListener) {
if (((KitIntegration.AttributeListener) provider).supportsAttributeLists()) {
if (supportsAttributeLists) {
((KitIntegration.AttributeListener) provider).setAllUserAttributes(filteredAttributeSingles, filteredAttributeLists);
} else {
Map<String, String> singlesCopy = new HashMap<>(filteredAttributeSingles);
Expand All @@ -617,7 +618,7 @@ public void onUserAttributesReceived(Map<String, String> userAttributes, Map<Str
}
}
if (provider instanceof KitIntegration.UserAttributeListener) {
if (((KitIntegration.UserAttributeListener) provider).supportsAttributeLists()) {
if (supportsAttributeLists) {
((KitIntegration.UserAttributeListener) provider).onSetAllUserAttributes(filteredAttributeSingles, filteredAttributeLists, FilteredMParticleUser.getInstance(mpid, provider));
} else {
Map<String, String> singlesCopy = new HashMap<>(filteredAttributeSingles);
Expand Down Expand Up @@ -683,15 +684,16 @@ private void setUserAttribute(KitIntegration provider, String attributeKey, List
if ((provider instanceof KitIntegration.AttributeListener || provider instanceof KitIntegration.UserAttributeListener)
&& !provider.isDisabled()
&& KitConfiguration.shouldForwardAttribute(provider.getConfiguration().getUserAttributeFilters(), attributeKey)) {
boolean supportsAttributeLists = ((KitIntegration.BaseAttributeListener) provider).supportsAttributeLists();
if (provider instanceof KitIntegration.AttributeListener) {
if (((KitIntegration.AttributeListener) provider).supportsAttributeLists()) {
if (supportsAttributeLists) {
((KitIntegration.AttributeListener) provider).setUserAttributeList(attributeKey, valueList);
} else {
((KitIntegration.AttributeListener) provider).setUserAttribute(attributeKey, KitUtils.join(valueList));
}
}
if (provider instanceof KitIntegration.UserAttributeListener) {
if (((KitIntegration.UserAttributeListener) provider).supportsAttributeLists()) {
if (supportsAttributeLists) {
((KitIntegration.UserAttributeListener) provider).onSetUserAttributeList(attributeKey, valueList, FilteredMParticleUser.getInstance(mpid, provider));
} else {
((KitIntegration.UserAttributeListener) provider).onSetUserAttribute(attributeKey, KitUtils.join(valueList), FilteredMParticleUser.getInstance(mpid, provider));
Expand Down Expand Up @@ -721,15 +723,10 @@ public void removeUserAttribute(String key, long mpid) {
}
for (KitIntegration provider : providers.values()) {
try {
if ((provider instanceof KitIntegration.AttributeListener || provider instanceof KitIntegration.UserAttributeListener)
if ((provider instanceof KitIntegration.BaseAttributeListener listener)
&& !provider.isDisabled()
&& KitConfiguration.shouldForwardAttribute(provider.getConfiguration().getUserAttributeFilters(), key)) {
if (provider instanceof KitIntegration.AttributeListener) {
((KitIntegration.AttributeListener) provider).removeUserAttribute(key);
}
if (provider instanceof KitIntegration.UserAttributeListener) {
((KitIntegration.UserAttributeListener) provider).onRemoveUserAttribute(key, FilteredMParticleUser.getInstance(mpid, provider));
}
listener.onRemoveUserAttribute(key, FilteredMParticleUser.getInstance(mpid, provider));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nullable user passed to non-null Kotlin parameter

Medium Severity

FilteredMParticleUser.getInstance(mpid, provider) can return null (e.g. if MParticle.getInstance() is null or user lookup fails), but several AttributeListener-only kits (Adobe, Apptimize, Branch, Comscore, Kochava, Localytics, Urban Airship) now declare onRemoveUserAttribute with a non-null user: FilteredMParticleUser parameter. Kotlin's intrinsic null check on non-null parameters will throw a runtime exception if null is passed from the Java caller. Previously these kits received removeUserAttribute(String) with no user parameter, so this crash path is newly introduced.

Additional Locations (2)
Fix in Cursor Fix in Web

}
} catch (Exception e) {
Logger.warning("Failed to call removeUserAttribute/onRemoveUserAttribute for kit: " + provider.getName() + ": " + e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ abstract class AdobeKitBase :
syncIds()
}

override fun removeUserAttribute(s: String) {
override fun onRemoveUserAttribute(
key: String,
user: FilteredMParticleUser,
) {
syncIds()
}

Expand All @@ -93,7 +96,9 @@ abstract class AdobeKitBase :
override fun onPushMessageReceived(
context: Context,
intent: Intent,
) {}
) {
// No-op: this kit does not implement push message handling.
}

override fun onPushRegistration(
instanceId: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ open class AdobeKit :
syncIds()
}

override fun removeUserAttribute(s: String) {
override fun onRemoveUserAttribute(
key: String,
user: FilteredMParticleUser,
) {
syncIds()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ class AppsFlyerKit :
value: Any?,
user: FilteredMParticleUser?,
) {
// No-op: this kit does not implement this feature.
}

override fun onSetUserTag(
Expand Down Expand Up @@ -304,9 +305,9 @@ class AppsFlyerKit :
override fun setAllUserAttributes(
map: Map<String, String>,
map1: Map<String, List<String>>,
) {}

override fun removeUserAttribute(key: String) {}
) {
// No-op: this kit does not implement this feature.
}

override fun removeUserIdentity(identityType: MParticle.IdentityType) {
with(instance) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,10 @@ class ApptimizeKit :
}
}

override fun removeUserAttribute(key: String) {
override fun onRemoveUserAttribute(
key: String,
user: FilteredMParticleUser,
) {
Apptimize.clearUserAttribute(key)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,19 @@ class BranchMetricsKit :
map1: Map<String, List<String>>,
) {}

override fun removeUserAttribute(s: String) {}
override fun onRemoveUserAttribute(
key: String,
user: FilteredMParticleUser,
) {
// No-op: this kit does not implement this feature.
}

override fun setUserIdentity(
identityType: IdentityType,
s: String,
) {}
) {
// No-op: this kit does not implement this feature.
}

override fun removeUserIdentity(identityType: IdentityType) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,37 @@ open class AppboyKit :
key: String?,
user: FilteredMParticleUser?,
) {
if (key == null) {
return
}
var keyMut = key
Braze
.getInstance(context)
.getCurrentUser(
object : IValueCallback<BrazeUser> {
override fun onSuccess(value: BrazeUser) {
when (keyMut) {
UserAttributes.CITY -> value.setHomeCity(null)
UserAttributes.COUNTRY -> value.setCountry(null)
UserAttributes.FIRSTNAME -> value.setFirstName(null)
UserAttributes.LASTNAME -> value.setLastName(null)
UserAttributes.MOBILE_NUMBER -> value.setPhoneNumber(null)
else -> {
var customKey = keyMut
if (customKey.startsWith("$")) {
customKey = customKey.substring(1)
}
value.unsetCustomUserAttribute(customKey)
}
}
queueDataFlush()
}

override fun onError() {
Logger.warning("unable to remove User Attribute with key: " + key)
}
},
)
}

override fun onSetUserAttribute(
Expand All @@ -457,13 +488,15 @@ open class AppboyKit :
key: String?,
user: FilteredMParticleUser?,
) {
// No-op: this kit does not implement this feature.
}

override fun onSetUserAttributeList(
attributeKey: String?,
attributeValueList: MutableList<String>?,
user: FilteredMParticleUser?,
) {
// No-op: this kit does not implement this feature.
}

override fun onSetAllUserAttributes(
Expand Down Expand Up @@ -626,45 +659,14 @@ open class AppboyKit :
}
}

override fun removeUserAttribute(keyIn: String) {
var key = keyIn
Braze
.getInstance(context)
.getCurrentUser(
object : IValueCallback<BrazeUser> {
override fun onSuccess(value: BrazeUser) {
if (UserAttributes.CITY == key) {
value.setHomeCity(null)
} else if (UserAttributes.COUNTRY == key) {
value.setCountry(null)
} else if (UserAttributes.FIRSTNAME == key) {
value.setFirstName(null)
} else if (UserAttributes.LASTNAME == key) {
value.setLastName(null)
} else if (UserAttributes.MOBILE_NUMBER == key) {
value.setPhoneNumber(null)
} else {
if (key.startsWith("$")) {
key = key.substring(1)
}
value.unsetCustomUserAttribute(key)
}
queueDataFlush()
}

override fun onError() {
Logger.warning("unable to remove User Attribute with key: " + key)
}
},
)
}

override fun setUserIdentity(
identityType: IdentityType,
identity: String,
) {}

override fun removeUserIdentity(identityType: IdentityType) {}
override fun removeUserIdentity(identityType: IdentityType) {
// No-op: this kit does not implement this feature.
}

override fun logout(): List<ReportingMessage> = emptyList()

Expand Down
Loading
Loading