Merge pull request #495 from oddbit/claude/codebase-review-97j9c8

fix: codebase review — Android getApplicationId staleness, clean push-payload errors, 32-bit guard (v0.30.3)

Author: DennisAlund

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 0.30.3
+
+- **Fix (Android):** `getApplicationId` now reads the live `FacebookSdk.getApplicationId()` instead of the app id captured when the plugin attached to the engine, so app-id changes made programmatically after startup are reflected — matching iOS (`Settings.shared.appID`) and the documented behavior.
+- **Fix (Android):** `logPushNotificationOpen` now returns a clean `INVALID_ARGUMENT` error when the payload contains a value an Android `Bundle` cannot represent (e.g. a list), instead of an opaque platform exception. The error message names the offending key and suggests JSON-encoding structured values.
+- **Fix:** `setDataProcessingOptions` validates in the Dart layer that `country` and `state` fit in a signed 32-bit integer (the type the native APIs take) and throws a `RangeError` otherwise. Out-of-range values previously surfaced as an opaque `ClassCastException` on Android; iOS already rejected them natively.
+- Stop sending explicit `null`s over the method channel for omitted arguments of `setDataProcessingOptions` and `logPushNotificationOpen`, consistent with the rest of the API.
+
 ## 0.30.2
 
 - **Update Android toolchain** — AGP 8.13.0, Gradle 8.13, Kotlin 2.4.0, `compileSdk`/`targetSdk` 36. No change to `minSdk` or the Facebook Android SDK Maven range (`[18.0,19.0)`), which already resolves to the latest 18.x release (18.3.0); the CocoaPods/SPM `~> 18.0` / `"18.0.0"..<"19.0.0"` iOS pins likewise already cover the latest 18.x release (18.1.0), so no iOS dependency changes were needed this round.

android/src/main/kotlin/id/oddbit/flutter/facebook_app_events/FacebookAppEventsPlugin.kt

@@ -146,12 +146,17 @@ class FacebookAppEventsPlugin: FlutterPlugin, MethodCallHandler {
   }
 
   private fun handleGetApplicationId(call: MethodCall, result: Result) {
-    result.success(appEventsLogger.applicationId)
+    // Read the live SDK setting rather than `appEventsLogger.applicationId`,
+    // which is captured when the logger is created at engine attach and would
+    // go stale if the app id is changed programmatically afterwards. This
+    // matches the iOS handler, which reads `Settings.shared.appID`.
+    result.success(FacebookSdk.getApplicationId())
   }
- private fun handleGetAnonymousId(call: MethodCall, result: Result) {
+
+  private fun handleGetAnonymousId(call: MethodCall, result: Result) {
     result.success(anonymousId)
   }
-  
+
   private fun handleSetGraphApiVersion(call: MethodCall, result: Result) {
     val version = call.arguments as? String
     if (version == null) {
@@ -215,7 +220,15 @@ class FacebookAppEventsPlugin: FlutterPlugin, MethodCallHandler {
   private fun handlePushNotificationOpen(call: MethodCall, result: Result) {
     val action = call.argument<String>("action")
     val payload = call.argument<Map<String, Any>>("payload")
-    val payloadBundle = createBundleFromMap(payload)
+    // Unlike logEvent parameters, the payload is not validated in the Dart
+    // layer, so map Bundle-incompatible values (e.g. lists) to a clean error
+    // instead of an opaque platform exception.
+    val payloadBundle = try {
+      createBundleFromMap(payload)
+    } catch (e: IllegalArgumentException) {
+      result.error("INVALID_ARGUMENT", e.message, null)
+      return
+    }
     if (payloadBundle == null) {
       result.error("INVALID_ARGUMENT", "Payload is required", null)
       return
@@ -266,7 +279,8 @@ class FacebookAppEventsPlugin: FlutterPlugin, MethodCallHandler {
           }
         }
         else -> throw IllegalArgumentException(
-            "Unsupported value type: ${value::class}")
+            "Unsupported value type ${value.javaClass.simpleName} for key '$key'; " +
+            "encode structured values as a JSON string.")
       }
     }
     return bundle

example/lib/main.dart

@@ -4,6 +4,8 @@ import 'package:flutter/material.dart';
 void main() => runApp(MyApp());
 
 class MyApp extends StatelessWidget {
+  const MyApp({super.key});
+
   static final facebookAppEvents = FacebookAppEvents();
   @override
   Widget build(BuildContext context) {

example/pubspec.lock

@@ -42,12 +42,12 @@ packages:
     source: hosted
     version: "1.19.1"
   facebook_app_events:
-    dependency: "direct dev"
+    dependency: "direct main"
     description:
       path: ".."
       relative: true
     source: path
-    version: "0.30.0"
+    version: "0.30.3"
   fake_async:
     dependency: transitive
     description:

example/pubspec.yaml

@@ -10,12 +10,12 @@ dependencies:
   flutter:
     sdk: flutter
 
+  facebook_app_events:
+    path: ../
+
 dev_dependencies:
   flutter_test:
     sdk: flutter
 
-  facebook_app_events:
-    path: ../
-
 flutter:
   uses-material-design: true

ios/facebook_app_events.podspec

@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name             = 'facebook_app_events'
-  s.version          = '0.30.2'
+  s.version          = '0.30.3'
   s.summary          = 'Flutter plugin for Facebook Analytics and App Events'
   s.description      = <<-DESC
 Flutter plugin for Facebook Analytics and App Events

ios/facebook_app_events/Sources/facebook_app_events/FacebookAppEventsPlugin.swift

@@ -196,7 +196,7 @@ public class FacebookAppEventsPlugin: NSObject, FlutterPlugin, FlutterSceneLifeC
     private func handleGetApplicationId(_ call: FlutterMethodCall, result: @escaping FlutterResult) {
         // Settings.shared.appID resolves the Info.plist FacebookAppID by
         // default and reflects any app id set programmatically on the SDK,
-        // matching Android's `appEventsLogger.applicationId`.
+        // matching Android's `FacebookSdk.getApplicationId()`.
         result(Settings.shared.appID)
     }
 

lib/facebook_app_events.dart

@@ -194,7 +194,7 @@ class FacebookAppEvents {
 
   /// Returns the app ID this logger was configured to log to.
   ///
-  /// Maps to `appEventsLogger.applicationId` on Android and
+  /// Maps to `FacebookSdk.getApplicationId()` on Android and
   /// `Settings.shared.appID` on iOS — both resolve the app id from platform
   /// configuration (`AndroidManifest.xml` / `Info.plist`) and reflect any app
   /// id set programmatically on the SDK.
@@ -260,7 +260,10 @@ class FacebookAppEvents {
       'action': action,
     };
 
-    return _channel.invokeMethod<void>('logPushNotificationOpen', args);
+    return _channel.invokeMethod<void>(
+      'logPushNotificationOpen',
+      _filterOutNulls(args),
+    );
   }
 
   /// Sets a user [id] to associate with all app events.
@@ -422,6 +425,9 @@ class FacebookAppEvents {
   /// `setDataProcessingOptions(['LDU'], country: 0, state: 0)`. Passing an
   /// empty [options] list disables Limited Data Use.
   ///
+  /// [country] and [state] must fit in a signed 32-bit integer (the type the
+  /// native APIs take); a [RangeError] is thrown otherwise.
+  ///
   /// See documentation:
   /// - https://developers.facebook.com/docs/development/data-processing-options
   /// - [iOS Settings](https://developers.facebook.com/docs/reference/iossdk/current/FBSDKCoreKit/classes/settings.html)
@@ -431,13 +437,19 @@ class FacebookAppEvents {
     int? country,
     int? state,
   }) {
+    _checkFitsIn32Bits(country, 'country');
+    _checkFitsIn32Bits(state, 'state');
+
     final args = <String, dynamic>{
       'options': options,
       'country': country,
       'state': state,
     };
 
-    return _channel.invokeMethod<void>('setDataProcessingOptions', args);
+    return _channel.invokeMethod<void>(
+      'setDataProcessingOptions',
+      _filterOutNulls(args),
+    );
   }
 
   /// Logs a purchase event.
@@ -813,6 +825,18 @@ class FacebookAppEvents {
   //
   // PRIVATE METHODS BELOW HERE
 
+  /// Throws a [RangeError] if [value] does not fit in a signed 32-bit integer.
+  ///
+  /// The standard method codec delivers larger Dart ints as 64-bit values,
+  /// which the native handlers cannot pass to SDK APIs typed as 32-bit ints.
+  static void _checkFitsIn32Bits(int? value, String name) {
+    const min = -0x80000000;
+    const max = 0x7FFFFFFF;
+    if (value != null && (value < min || value > max)) {
+      throw RangeError.range(value, min, max, name);
+    }
+  }
+
   /// Creates a new map containing all of the key/value pairs from [parameters]
   /// except those whose value is `null`.
   Map<String, dynamic> _filterOutNulls(Map<String, dynamic> parameters) {

pubspec.yaml

@@ -1,7 +1,7 @@
 name: facebook_app_events
 description: Flutter plugin for Facebook App Events, an app measurement
   solution that provides insight on app usage and user engagement in Facebook Analytics.
-version: 0.30.2
+version: 0.30.3
 homepage: https://oddb.it/app-events-pubspec
 repository: https://github.com/oddbit/flutter_facebook_app_events
 

test/facebook_app_events_test.dart

@@ -501,6 +501,38 @@ void main() {
         ),
       );
     });
+
+    test('setDataProcessingOptions omits null country and state', () async {
+      await facebookAppEvents.setDataProcessingOptions(['LDU']);
+
+      expect(
+        methodCall,
+        isMethodCall(
+          'setDataProcessingOptions',
+          arguments: <String, dynamic>{
+            'options': ['LDU'],
+          },
+        ),
+      );
+    });
+
+    test('setDataProcessingOptions throws RangeError for values that do not '
+        'fit in 32 bits', () {
+      expect(
+        () => facebookAppEvents.setDataProcessingOptions(
+          ['LDU'],
+          country: 0x80000000,
+        ),
+        throwsRangeError,
+      );
+      expect(
+        () => facebookAppEvents.setDataProcessingOptions(
+          ['LDU'],
+          state: -0x80000001,
+        ),
+        throwsRangeError,
+      );
+    });
   });
 
   group('User lifecycle', () {
@@ -559,6 +591,22 @@ void main() {
         ),
       );
     });
+
+    test('logPushNotificationOpen omits action when null', () async {
+      await facebookAppEvents.logPushNotificationOpen(
+        payload: {'campaign': 'spring-sale'},
+      );
+
+      expect(
+        methodCall,
+        isMethodCall(
+          'logPushNotificationOpen',
+          arguments: <String, dynamic>{
+            'payload': {'campaign': 'spring-sale'},
+          },
+        ),
+      );
+    });
   });
 
   group('Standard event shorthands', () {