Skip to content

Commit fa9e4a0

Browse files
committed
Check for notification_open duplicate logging based on message ID.
- Changed to use the message ID when checking for duplicate notification_open logging instead of the Activity's Intent. This should avoid logging again when an app starts another activity using a copy of the notification's intent (#6091).
1 parent d583311 commit fa9e4a0

File tree

3 files changed

+22
-14
lines changed

3 files changed

+22
-14
lines changed

firebase-messaging/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
# Unreleased
2+
* [changed] Checked for notification_open duplicate logging based on message ID
3+
instead of the Activity's Intent.
24

35

46
# 24.0.0

firebase-messaging/src/main/java/com/google/firebase/messaging/FcmLifecycleCallbacks.java

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,25 @@
2424
import android.os.Bundle;
2525
import android.os.Handler;
2626
import android.os.Looper;
27+
import android.text.TextUtils;
2728
import android.util.Log;
28-
import java.util.Collections;
29-
import java.util.Set;
30-
import java.util.WeakHashMap;
29+
import java.util.ArrayDeque;
30+
import java.util.Queue;
3131

3232
class FcmLifecycleCallbacks implements Application.ActivityLifecycleCallbacks {
3333

34-
/** Keep a list of intents that we've seen to avoid accidentally logging events twice. */
35-
private final Set<Intent> seenIntents =
36-
Collections.newSetFromMap(new WeakHashMap<Intent, Boolean>());
34+
private static final int RECENTLY_LOGGED_MESSAGE_IDS_MAX_SIZE = 10;
35+
36+
/** Last N message IDs that have been logged to prevent duplicate logging. */
37+
private final Queue<String> recentlyLoggedMessageIds =
38+
new ArrayDeque<>(RECENTLY_LOGGED_MESSAGE_IDS_MAX_SIZE);
3739

3840
// TODO(b/258424124): Migrate to go/firebase-android-executors
3941
@SuppressLint("ThreadPoolCreation")
4042
@Override
4143
public void onActivityCreated(Activity createdActivity, Bundle instanceState) {
4244
Intent startingIntent = createdActivity.getIntent();
43-
if (startingIntent == null || !seenIntents.add(startingIntent)) {
44-
// already seen (and logged) this, no need to go any further.
45+
if (startingIntent == null) {
4546
return;
4647
}
4748

@@ -57,12 +58,7 @@ public void onActivityCreated(Activity createdActivity, Bundle instanceState) {
5758
}
5859

5960
@Override
60-
public void onActivityPaused(Activity pausedActivity) {
61-
if (pausedActivity.isFinishing()) {
62-
// iff the activity is finished we can remove the intent from our "seen" list
63-
seenIntents.remove(pausedActivity.getIntent());
64-
}
65-
}
61+
public void onActivityPaused(Activity pausedActivity) {}
6662

6763
@Override
6864
public void onActivityDestroyed(Activity destroyedActivity) {}
@@ -84,6 +80,14 @@ private void logNotificationOpen(Intent startingIntent) {
8480
try {
8581
Bundle extras = startingIntent.getExtras();
8682
if (extras != null) {
83+
String messageId = MessagingAnalytics.getMessageId(extras);
84+
if (!TextUtils.isEmpty(messageId)) {
85+
if (recentlyLoggedMessageIds.contains(messageId)) {
86+
// Already logged, don't log again.
87+
return;
88+
}
89+
recentlyLoggedMessageIds.add(messageId);
90+
}
8791
analyticsData = extras.getBundle(Constants.MessageNotificationKeys.ANALYTICS_DATA);
8892
}
8993
} catch (RuntimeException e) {

firebase-messaging/src/test/java/com/google/firebase/messaging/FirebaseMessagingServiceRoboTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import com.google.firebase.FirebaseOptions;
6464
import com.google.firebase.iid.FirebaseInstanceIdReceiver;
6565
import com.google.firebase.messaging.AnalyticsTestHelper.Analytics;
66+
import com.google.firebase.messaging.Constants.MessagePayloadKeys;
6667
import com.google.firebase.messaging.testing.AnalyticsValidator;
6768
import com.google.firebase.messaging.testing.Bundles;
6869
import com.google.firebase.messaging.testing.FakeConnectorComponent;
@@ -612,6 +613,7 @@ private void simulateNotificationMessageWithAnalytics() throws InterruptedExcept
612613
// Robo manifest doesn't have a default activity, so set click action so that we get a
613614
// click pending intent.
614615
builder.addData(DisplayNotificationRoboTest.KEY_CLICK_ACTION, "click_action");
616+
builder.addData(MessagePayloadKeys.MSGID, "msg_id");
615617
AnalyticsTestHelper.addAnalyticsExtras(builder);
616618
startServiceViaReceiver(builder.buildIntent());
617619
}

0 commit comments

Comments
 (0)