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

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.

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.

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 20, 2020
1 parent a12d91f commit 76517e3
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
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
4 changes: 3 additions & 1 deletion packages/expo-firebase-analytics/CHANGELOG.md
Expand Up @@ -12,4 +12,6 @@

- Fix no events recorded on the Expo client when running on certain Android devices. ([#7679](https://github.com/expo/expo/pull/7679) by [@IjzerenHein](https://github.com/IjzerenHein))
- Fix `setAnalyticsCollectionEnabled` throwing an error.
- Fixes & improvements to the pure JS analytics client. ([#7796](https://github.com/expo/expo/pull/7796) by [@IjzerenHein](https://github.com/IjzerenHein))
- Fixes & improvements to the pure JS analytics client. ([#7796](https://github.com/expo/expo/pull/7796) by [@IjzerenHein](https://github.com/IjzerenHein))
- 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))

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 76517e3

Please sign in to comment.