RemoteMessage.getData() should return a defensive copy of the intneral ArrayMap from its public API

[dlam]

Context: https://issuetracker.google.com/393513316

ArrayMap is a performance-sensitive collection which does not support holding references to the returned Map.Entry when iterating through it. Many kotlin-stdlib extensions on collections do not create defensive copies (intentionally for performance), so it is quite confusing to end users that for example RemoteMessage.data.entries.sortedBy { ..} returned a list of the last entry repeated size times.

Instead, I would generally recommend that public API return defensive copies of collections to avoid this confusion and to keep the performance sensitive stuff internal. We have a similar rule for android platform + libraries here: go/android-api-guidelines#type-semantics

I believe this is the problematic call-site:

firebase-android-sdk/firebase-messaging/src/main/java/com/google/firebase/messaging/Constants.java

Line 120 in 5e778b9

public static ArrayMap<String, String> extractDeveloperDefinedPayload(Bundle bundle) {

and the affected api in the linked buganizer issue against androidx.collection:

firebase-android-sdk/firebase-messaging/src/main/java/com/google/firebase/messaging/RemoteMessage.java

Line 130 in 5e778b9

public Map<String, String> getData() {

[google-oss-bot]

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

[lehcar09]

Hi @dlam, thank you for raising the issue. Let me raise this to our engineers and get back to you! Thanks!