From 1a050b5d1bd2cf801b8e824a7151f4c464cc2e7d Mon Sep 17 00:00:00 2001 From: Thorben Primke Date: Fri, 17 Apr 2020 18:49:49 -0700 Subject: [PATCH] [Android][FirebaseAnalytics] Adds Nullable Params Handling to Firebase logEvent Method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: The params parameter is nullable but it was passed to the MapArguments constructor which expects a non-null map. This caused a NPE when the Map’s keySet method is invoked. The fix is to simply pass null to Firebase logEvent method as that is handled as no properties are being passed with the event. Test Plan: - Added an additional test case for logEvent to the test suite and observed it failing without the fix and passing after the change. --- CHANGELOG.md | 3 ++- apps/test-suite/tests/FirebaseAnalytics.js | 20 +++++++++++++++++++ .../analytics/FirebaseAnalyticsModule.java | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78cfff96108d3..4d264faec62d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ This is the log of notable changes to the Expo client that are developer-facing. - Fix `Contacts.presentFormAsync` pre-filling. ([#7285](https://github.com/expo/expo/pull/7285) by [@abdelilah](https://github.com/abdelilah) & [@lukmccall](https://github.com/lukmccall)) - Removed unknown CLI options `--android-package` and `--ios-bundle-identifier` from docs. ([#7354](https://github.com/expo/expo/pull/7354) by [@ca057](https://github.com/ca057)) - Fixed `androidNavigationBar.hidden` configuration not remaining applied after backgrounding & foregrounding the app. ([#7770](https://github.com/expo/expo/pull/7770) by [@cruzach](https://github.com/cruzach)) +- Fixed logEvent in `expo-firebase-analytics` for Android. logEvent's optional properties parameter was causing a NPE on Android when not provided. ([#0](https://github.com/expo/expo/pull/0) by [@cruzach](https://github.com/thorbenprimke)) ## 37.0.0 @@ -43,7 +44,7 @@ This is the log of notable changes to the Expo client that are developer-facing. ### 🛠 Breaking changes -- **Android push notifications:** your Google API key specified in `google-services.json` must allow access to the Cloud Messaging API if you have restricted your API key to access only specific APIs. In the Google Cloud console, browse to [APIs & Services -> Credentials](https://console.cloud.google.com/apis/credentials). Find the API key that is associated with your app and click the pencil icon to edit it. Under "API restrictions", if the key is restricted, add "Firebase Installations API" and "Cloud Messaging" to the set of allowed APIs and save the changes. (Technical note: Google changed the underlying Firebase Cloud Messaging library in `com.google.firebase:firebase-messaging:20.1.2` to depend on the Firebase Installations API, which applies API key restrictions. See Google's notes [here](https://firebase.google.com/support/release-notes/android#2020-02-27) and [here](https://github.com/firebase/firebase-android-sdk/blob/master/firebase-installations/API_KEY_RESTRICTIONS.md).) +- **Android push notifications:** your Google API key specified in `google-services.json` must allow access to the Cloud Messaging API if you have restricted your API key to access only specific APIs. In the Google Cloud console, browse to [APIs & Services -> Credentials](https://console.cloud.google.com/apis/credentials). Find the API key that is associated with your app and click the pencil icon to edit it. Under "API restrictions", if the key is restricted, add "Firebase Installations API" and "Cloud Messaging" to the set of allowed APIs and save the changes. (Technical note: Google changed the underlying Firebase Cloud Messaging library in `com.google.firebase:firebase-messaging:20.1.2` to depend on the Firebase Installations API, which applies API key restrictions. See Google's notes [here](https://firebase.google.com/support/release-notes/android#2020-02-27) and [here](https://github.com/firebase/firebase-android-sdk/blob/master/firebase-installations/API_KEY_RESTRICTIONS.md).) - `expo-app-auth` Remove SSL features from unsafe connection builder. ([#7187](https://github.com/expo/expo/pull/7187) by [@evanbacon](https://github.com/evanbacon)) - `expo-constants` `Constants.deviceName` now only returns the possible Browser name and doesn't fallback to engine or OS name. ([#6809](https://github.com/expo/expo/pull/6809) [@evanbacon](https://github.com/evanbacon)) - `expo-constants` `Constants.platform.web` now only returns the `ua` (user agent string). ([#6809](https://github.com/expo/expo/pull/6809) [@evanbacon](https://github.com/evanbacon)) diff --git a/apps/test-suite/tests/FirebaseAnalytics.js b/apps/test-suite/tests/FirebaseAnalytics.js index 01b43ffeffeec..81a54a3c6080f 100644 --- a/apps/test-suite/tests/FirebaseAnalytics.js +++ b/apps/test-suite/tests/FirebaseAnalytics.js @@ -44,6 +44,26 @@ export async function test({ describe, beforeAll, afterAll, it, xit, expect }) { expect(error).not.toBeNull(); }); }); + describe('logEvent() - without optional properties', async () => { + itWhenConfigured(`runs`, async () => { + let error = null; + try { + await Analytics.logEvent('event_name'); + } catch (e) { + error = e; + } + expect(error).toBeNull(); + }); + itWhenNotConfigured('fails when not configured', async () => { + let error = null; + try { + await Analytics.logEvent('event_name'); + } catch (e) { + error = e; + } + expect(error).not.toBeNull(); + }); + }); describe('setCurrentScreen()', async () => { itWhenConfigured(`runs`, async () => { let error = null; diff --git a/packages/expo-firebase-analytics/android/src/main/java/expo/modules/firebase/analytics/FirebaseAnalyticsModule.java b/packages/expo-firebase-analytics/android/src/main/java/expo/modules/firebase/analytics/FirebaseAnalyticsModule.java index d3a46434d768b..dda449de5518c 100644 --- a/packages/expo-firebase-analytics/android/src/main/java/expo/modules/firebase/analytics/FirebaseAnalyticsModule.java +++ b/packages/expo-firebase-analytics/android/src/main/java/expo/modules/firebase/analytics/FirebaseAnalyticsModule.java @@ -81,7 +81,7 @@ public void logEvent(final String name, @Nullable Map params, Pr FirebaseAnalytics analytics = getFirebaseAnalyticsOrReject(promise); if (analytics == null) return; - analytics.logEvent(name, new MapArguments(params).toBundle()); + analytics.logEvent(name, params == null ? null : new MapArguments(params).toBundle()); promise.resolve(null); } catch (Exception e) { promise.reject(e);