Skip to content

Commit

Permalink
[Android][FirebaseAnalytics] Adds Null Params Handling to Firebase's …
Browse files Browse the repository at this point in the history
…logEvent

# Why
The params parameter of `logEvent` 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.

# How
The fix is to simply pass null to Firebase `logEvent` method as that
is handled as in 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 5ece2d3
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 1 deletion.
1 change: 1 addition & 0 deletions 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. ([#7897](https://github.com/expo/expo/pull/7897) by [@thorbenprimke](https://github.com/thorbenprimke))

## 37.0.0

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 5ece2d3

Please sign in to comment.