Skip to content

Commit

Permalink
[Android][FirebaseAnalytics] Adds Nullable Params Handling to Firebas…
Browse files Browse the repository at this point in the history
…e logEvent Method

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.
  • Loading branch information
thorbenprimke committed Apr 18, 2020
1 parent be72205 commit 1a050b5
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -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

Expand All @@ -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))
Expand Down
20 changes: 20 additions & 0 deletions apps/test-suite/tests/FirebaseAnalytics.js
Expand Up @@ -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;
Expand Down
Expand Up @@ -81,7 +81,7 @@ public void logEvent(final String name, @Nullable Map<String, Object> 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);
Expand Down

0 comments on commit 1a050b5

Please sign in to comment.