From 2064bb64feeb72a62907fd45938f2583536d77e9 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Thu, 11 Jul 2024 17:21:29 +0530 Subject: [PATCH 1/6] binding: Prefetch deviceInfo during initialization This is a preparatory commit for the work of embedding the device info in user-agent header. --- lib/main.dart | 2 +- lib/model/binding.dart | 43 +++++++++++++++++++++++++++++++------- lib/widgets/clipboard.dart | 2 +- test/model/binding.dart | 11 +++++----- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/lib/main.dart b/lib/main.dart index 1e2577bbc5..118fc65e0b 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -13,8 +13,8 @@ void main() { return true; }()); LicenseRegistry.addLicense(additionalLicenses); - LiveZulipBinding.ensureInitialized(); WidgetsFlutterBinding.ensureInitialized(); + LiveZulipBinding.ensureInitialized(); NotificationService.instance.start(); runApp(const ZulipApp()); } diff --git a/lib/model/binding.dart b/lib/model/binding.dart index 31ed514973..4959e36e82 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -6,6 +6,7 @@ import 'package:flutter_local_notifications/flutter_local_notifications.dart'; import 'package:url_launcher/url_launcher.dart' as url_launcher; import '../host/android_notifications.dart'; +import '../log.dart'; import '../widgets/store.dart'; import 'store.dart'; @@ -101,8 +102,18 @@ abstract class ZulipBinding { /// Provides device and operating system information, /// via package:device_info_plus. /// + /// The returned Future resolves to null if an error is + /// encountered while fetching the data. + /// /// This wraps [device_info_plus.DeviceInfoPlugin.deviceInfo]. - Future deviceInfo(); + Future get deviceInfo; + + /// Provides device and operating system information, + /// via package:device_info_plus. + /// + /// This is the value [deviceInfo] resolved to, + /// or null if that hasn't resolved yet. + BaseDeviceInfo? get syncDeviceInfo; /// Initialize Firebase, to use for notifications. /// @@ -161,6 +172,10 @@ class IosDeviceInfo extends BaseDeviceInfo { /// Methods wrapping a plugin, like [launchUrl], invoke the actual /// underlying plugin method. class LiveZulipBinding extends ZulipBinding { + LiveZulipBinding() { + _deviceInfo = _prefetchDeviceInfo(); + } + /// Initialize the binding if necessary, and ensure it is a [LiveZulipBinding]. static LiveZulipBinding ensureInitialized() { if (ZulipBinding._instance == null) { @@ -196,13 +211,25 @@ class LiveZulipBinding extends ZulipBinding { } @override - Future deviceInfo() async { - final deviceInfo = await device_info_plus.DeviceInfoPlugin().deviceInfo; - return switch (deviceInfo) { - device_info_plus.AndroidDeviceInfo(:var version) => AndroidDeviceInfo(sdkInt: version.sdkInt), - device_info_plus.IosDeviceInfo(:var systemVersion) => IosDeviceInfo(systemVersion: systemVersion), - _ => throw UnimplementedError(), - }; + Future get deviceInfo => _deviceInfo; + late Future _deviceInfo; + + @override + BaseDeviceInfo? get syncDeviceInfo => _syncDeviceInfo; + BaseDeviceInfo? _syncDeviceInfo; + + Future _prefetchDeviceInfo() async { + try { + final info = await device_info_plus.DeviceInfoPlugin().deviceInfo; + _syncDeviceInfo = switch (info) { + device_info_plus.AndroidDeviceInfo(:var version) => AndroidDeviceInfo(sdkInt: version.sdkInt), + device_info_plus.IosDeviceInfo(:var systemVersion) => IosDeviceInfo(systemVersion: systemVersion), + _ => throw UnimplementedError(), + }; + } catch (e, st) { + assert(debugLog('Failed to prefetch device info: $e\n$st')); // TODO(log) + } + return _syncDeviceInfo; } @override diff --git a/lib/widgets/clipboard.dart b/lib/widgets/clipboard.dart index 27497c3713..9977af3f0c 100644 --- a/lib/widgets/clipboard.dart +++ b/lib/widgets/clipboard.dart @@ -18,7 +18,7 @@ void copyWithPopup({ required Widget successContent, }) async { await Clipboard.setData(data); - final deviceInfo = await ZulipBinding.instance.deviceInfo(); + final deviceInfo = await ZulipBinding.instance.deviceInfo; if (!context.mounted) return; diff --git a/test/model/binding.dart b/test/model/binding.dart index 9f88847e0f..5b5f71fdc5 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -204,9 +204,7 @@ class TestZulipBinding extends ZulipBinding { _closeInAppWebViewCallCount++; } - /// The value that `ZulipBinding.instance.deviceInfo()` should return. - /// - /// See also [takeDeviceInfoCalls]. + /// The value that `ZulipBinding.instance.deviceInfo` should return. BaseDeviceInfo deviceInfoResult = _defaultDeviceInfoResult; static final _defaultDeviceInfoResult = AndroidDeviceInfo(sdkInt: 33); @@ -215,9 +213,10 @@ class TestZulipBinding extends ZulipBinding { } @override - Future deviceInfo() { - return Future(() => deviceInfoResult); - } + Future get deviceInfo async => deviceInfoResult; + + @override + BaseDeviceInfo? get syncDeviceInfo => deviceInfoResult; void _resetFirebase() { _firebaseInitialized = false; From ea4efc71fa0533ac6f7d6c9a8257f5c6f799fcb6 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Thu, 11 Jul 2024 17:51:49 +0530 Subject: [PATCH 2/6] binding: Add packageInfo getter This is a preparatory commit for the work of embedding the package info in user-agent header. --- lib/model/binding.dart | 50 +++++++++++++++++++++++++++++++++++++++++ test/model/binding.dart | 15 +++++++++++++ 2 files changed, 65 insertions(+) diff --git a/lib/model/binding.dart b/lib/model/binding.dart index 4959e36e82..573bc680fa 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -3,6 +3,7 @@ import 'package:firebase_core/firebase_core.dart' as firebase_core; import 'package:firebase_messaging/firebase_messaging.dart' as firebase_messaging; import 'package:flutter/foundation.dart'; import 'package:flutter_local_notifications/flutter_local_notifications.dart'; +import 'package:package_info_plus/package_info_plus.dart' as package_info_plus; import 'package:url_launcher/url_launcher.dart' as url_launcher; import '../host/android_notifications.dart'; @@ -115,6 +116,22 @@ abstract class ZulipBinding { /// or null if that hasn't resolved yet. BaseDeviceInfo? get syncDeviceInfo; + /// Provides application package information, + /// via package:package_info_plus. + /// + /// The returned Future resolves to null if an error is + /// encountered while fetching the data. + /// + /// This wraps [package_info_plus.PackageInfo.fromPlatform]. + Future get packageInfo; + + /// Provides application package information, + /// via package:package_info_plus. + /// + /// This is the value [packageInfo] resolved to, + /// or null if that hasn't resolved yet. + PackageInfo? get syncPackageInfo; + /// Initialize Firebase, to use for notifications. /// /// This wraps [firebase_core.Firebase.initializeApp]. @@ -163,6 +180,17 @@ class IosDeviceInfo extends BaseDeviceInfo { IosDeviceInfo({required this.systemVersion}); } +/// Like [package_info_plus.PackageInfo], but without things we don't use. +class PackageInfo { + final String version; + final String buildNumber; + + PackageInfo({ + required this.version, + required this.buildNumber, + }); +} + /// A concrete binding for use in the live application. /// /// The global store returned by [loadGlobalStore], and consequently by @@ -174,6 +202,7 @@ class IosDeviceInfo extends BaseDeviceInfo { class LiveZulipBinding extends ZulipBinding { LiveZulipBinding() { _deviceInfo = _prefetchDeviceInfo(); + _packageInfo = _prefetchPackageInfo(); } /// Initialize the binding if necessary, and ensure it is a [LiveZulipBinding]. @@ -232,6 +261,27 @@ class LiveZulipBinding extends ZulipBinding { return _syncDeviceInfo; } + @override + Future get packageInfo => _packageInfo; + late Future _packageInfo; + + @override + PackageInfo? get syncPackageInfo => _syncPackageInfo; + PackageInfo? _syncPackageInfo; + + Future _prefetchPackageInfo() async { + try { + final info = await package_info_plus.PackageInfo.fromPlatform(); + _syncPackageInfo = PackageInfo( + version: info.version, + buildNumber: info.buildNumber, + ); + } catch (e, st) { + assert(debugLog('Failed to prefetch package info: $e\n$st')); // TODO(log) + } + return _syncPackageInfo; + } + @override Future firebaseInitializeApp({ required firebase_core.FirebaseOptions options}) { diff --git a/test/model/binding.dart b/test/model/binding.dart index 5b5f71fdc5..6966a4d8a6 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -70,6 +70,7 @@ class TestZulipBinding extends ZulipBinding { _resetLaunchUrl(); _resetCloseInAppWebView(); _resetDeviceInfo(); + _resetPackageInfo(); _resetFirebase(); _resetNotifications(); } @@ -218,6 +219,20 @@ class TestZulipBinding extends ZulipBinding { @override BaseDeviceInfo? get syncDeviceInfo => deviceInfoResult; + /// The value that `ZulipBinding.instance.packageInfo` should return. + PackageInfo packageInfoResult = _defaultPackageInfo; + static final _defaultPackageInfo = PackageInfo(version: '0.0.1', buildNumber: '1'); + + void _resetPackageInfo() { + packageInfoResult = _defaultPackageInfo; + } + + @override + Future get packageInfo async => packageInfoResult; + + @override + PackageInfo? get syncPackageInfo => packageInfoResult; + void _resetFirebase() { _firebaseInitialized = false; _firebaseMessaging = null; From 9031c66e4271e7f2b90d237abdff9636e34b34d9 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Thu, 11 Jul 2024 17:52:43 +0530 Subject: [PATCH 3/6] about_zulip [nfc]: Refactor to use packageInfo from ZulipBinding --- lib/widgets/about_zulip.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/widgets/about_zulip.dart b/lib/widgets/about_zulip.dart index 3cd55eacaf..a30566caa7 100644 --- a/lib/widgets/about_zulip.dart +++ b/lib/widgets/about_zulip.dart @@ -1,7 +1,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; -import 'package:package_info_plus/package_info_plus.dart'; +import '../model/binding.dart'; import 'page.dart'; class AboutZulipPage extends StatefulWidget { @@ -22,7 +22,7 @@ class _AboutZulipPageState extends State { void initState() { super.initState(); (() async { - final result = await PackageInfo.fromPlatform(); + final result = await ZulipBinding.instance.packageInfo; setState(() { _packageInfo = result; }); From 0659c48417638b96da37c6a4fd9bc5fa0e9c5fde Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Thu, 11 Jul 2024 18:14:27 +0530 Subject: [PATCH 4/6] binding: Support more device/os variants for deviceInfo getter --- lib/model/binding.dart | 87 ++++++++++++++++++++++++++++++-- test/model/binding.dart | 2 +- test/widgets/clipboard_test.dart | 4 +- 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/lib/model/binding.dart b/lib/model/binding.dart index 573bc680fa..5ee77b94eb 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -161,13 +161,21 @@ abstract class BaseDeviceInfo { /// Like [device_info_plus.AndroidDeviceInfo], but without things we don't use. class AndroidDeviceInfo extends BaseDeviceInfo { + /// The Android version string, Build.VERSION.RELEASE, e.g. "14". + /// + /// Upstream documents this as an opaque string with no particular structure, + /// but e.g. on stock Android 14 it's "14". + /// + /// See: https://developer.android.com/reference/android/os/Build.VERSION#RELEASE + final String release; + /// The Android SDK version. /// /// Possible values are defined in: /// https://developer.android.com/reference/android/os/Build.VERSION_CODES.html final int sdkInt; - AndroidDeviceInfo({required this.sdkInt}); + AndroidDeviceInfo({required this.release, required this.sdkInt}); } /// Like [device_info_plus.IosDeviceInfo], but without things we don't use. @@ -180,6 +188,70 @@ class IosDeviceInfo extends BaseDeviceInfo { IosDeviceInfo({required this.systemVersion}); } +/// Like [device_info_plus.MacOsDeviceInfo], but without things we don't use. +class MacOsDeviceInfo extends BaseDeviceInfo { + /// See: https://developer.apple.com/documentation/foundation/operatingsystemversion/1414662-majorversion + final int majorVersion; + + /// See: https://developer.apple.com/documentation/foundation/operatingsystemversion/1413801-minorversion + final int minorVersion; + + /// See: https://developer.apple.com/documentation/foundation/operatingsystemversion/1415564-patchversion + final int patchVersion; + + MacOsDeviceInfo({ + required this.majorVersion, + required this.minorVersion, + required this.patchVersion, + }); +} + +/// Like [device_info_plus.WindowsDeviceInfo], currently only used to +/// determine if we're on Windows. +// TODO Determine a method to identify the Windows version. +// Currently, we do not include Windows version information because +// Windows OS does not provide a straightforward way to obtain +// recognizable version information. +// Here's an example of `WindowsDeviceInfo` data[1]. Based on that +// data, there are two possible approaches to identify the Windows +// version: +// - One approach is to use a combination of the majorVersion, +// minorVersion, and buildNumber fields. However, this data does +// not directly correspond to recognizable Windows versions +// (for example major=10, minor=0, build=22631 actually represents +// "Windows 11, 23H2"). Refer to the link in this comment[2] for +// Chromium's implementation of parsing Windows version numbers. +// - Another approach is to use the productName field. While this +// field contains the Windows version, it also includes extraneous +// information. For example, some productName strings are: +// "Windows 11 Pro" and "Windows 10 Home Single Language", which +// makes it less ideal. +// [1]: https://gist.github.com/rajveermalviya/58b3add437280cc7f8356f3697099b7c +// [2]: https://github.com/zulip/zulip-flutter/pull/724#discussion_r1628318991 +class WindowsDeviceInfo implements BaseDeviceInfo {} + +/// Like [device_info_plus.LinuxDeviceInfo], but without things we don't use. +class LinuxDeviceInfo implements BaseDeviceInfo { + /// The operating system name, 'NAME' field in /etc/os-release. + /// + /// Examples: 'Fedora', 'Debian GNU/Linux', or just 'Linux'. + /// + /// See: https://www.freedesktop.org/software/systemd/man/latest/os-release.html#NAME= + final String name; + + /// The operating system version, 'VERSION_ID' field in /etc/os-release. + /// + /// This string contains only the version number and excludes the + /// OS name and version codenames. + /// + /// Examples: '17', '11.04'. + /// + /// See: https://www.freedesktop.org/software/systemd/man/latest/os-release.html#VERSION_ID= + final String? versionId; + + LinuxDeviceInfo({required this.name, required this.versionId}); +} + /// Like [package_info_plus.PackageInfo], but without things we don't use. class PackageInfo { final String version; @@ -251,9 +323,16 @@ class LiveZulipBinding extends ZulipBinding { try { final info = await device_info_plus.DeviceInfoPlugin().deviceInfo; _syncDeviceInfo = switch (info) { - device_info_plus.AndroidDeviceInfo(:var version) => AndroidDeviceInfo(sdkInt: version.sdkInt), - device_info_plus.IosDeviceInfo(:var systemVersion) => IosDeviceInfo(systemVersion: systemVersion), - _ => throw UnimplementedError(), + device_info_plus.AndroidDeviceInfo() => AndroidDeviceInfo(release: info.version.release, + sdkInt: info.version.sdkInt), + device_info_plus.IosDeviceInfo() => IosDeviceInfo(systemVersion: info.systemVersion), + device_info_plus.MacOsDeviceInfo() => MacOsDeviceInfo(majorVersion: info.majorVersion, + minorVersion: info.minorVersion, + patchVersion: info.patchVersion), + device_info_plus.WindowsDeviceInfo() => WindowsDeviceInfo(), + device_info_plus.LinuxDeviceInfo() => LinuxDeviceInfo(name: info.name, + versionId: info.versionId), + _ => throw UnimplementedError(), }; } catch (e, st) { assert(debugLog('Failed to prefetch device info: $e\n$st')); // TODO(log) diff --git a/test/model/binding.dart b/test/model/binding.dart index 6966a4d8a6..a120c15a27 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -207,7 +207,7 @@ class TestZulipBinding extends ZulipBinding { /// The value that `ZulipBinding.instance.deviceInfo` should return. BaseDeviceInfo deviceInfoResult = _defaultDeviceInfoResult; - static final _defaultDeviceInfoResult = AndroidDeviceInfo(sdkInt: 33); + static final _defaultDeviceInfoResult = AndroidDeviceInfo(sdkInt: 33, release: '13'); void _resetDeviceInfo() { deviceInfoResult = _defaultDeviceInfoResult; diff --git a/test/widgets/clipboard_test.dart b/test/widgets/clipboard_test.dart index 2b06625377..5a017f9b92 100644 --- a/test/widgets/clipboard_test.dart +++ b/test/widgets/clipboard_test.dart @@ -65,14 +65,14 @@ void main() { }); testWidgets('Android', (WidgetTester tester) async { - testBinding.deviceInfoResult = AndroidDeviceInfo(sdkInt: 33); + testBinding.deviceInfoResult = AndroidDeviceInfo(sdkInt: 33, release: '13'); await call(tester, text: 'asdf'); await checkClipboardText('asdf'); await checkSnackBar(tester, expected: false); }); testWidgets('Android <13', (WidgetTester tester) async { - testBinding.deviceInfoResult = AndroidDeviceInfo(sdkInt: 32); + testBinding.deviceInfoResult = AndroidDeviceInfo(sdkInt: 32, release: '12'); await call(tester, text: 'asdf'); await checkClipboardText('asdf'); await checkSnackBar(tester, expected: true); From 1ff70711cd2394531ada3a0ee4cd59baef08b2ad Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Sat, 13 Jul 2024 14:18:58 +0530 Subject: [PATCH 5/6] binding [nfc]: Make `PackageInfo` and `*DeviceInfo` constructors `const` --- lib/model/binding.dart | 18 ++++++++++-------- test/model/binding.dart | 4 ++-- test/widgets/action_sheet_test.dart | 2 +- test/widgets/clipboard_test.dart | 6 +++--- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/model/binding.dart b/lib/model/binding.dart index 5ee77b94eb..6cbc876e63 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -156,7 +156,7 @@ abstract class ZulipBinding { /// Like [device_info_plus.BaseDeviceInfo], but without things we don't use. abstract class BaseDeviceInfo { - BaseDeviceInfo(); + const BaseDeviceInfo(); } /// Like [device_info_plus.AndroidDeviceInfo], but without things we don't use. @@ -175,7 +175,7 @@ class AndroidDeviceInfo extends BaseDeviceInfo { /// https://developer.android.com/reference/android/os/Build.VERSION_CODES.html final int sdkInt; - AndroidDeviceInfo({required this.release, required this.sdkInt}); + const AndroidDeviceInfo({required this.release, required this.sdkInt}); } /// Like [device_info_plus.IosDeviceInfo], but without things we don't use. @@ -185,7 +185,7 @@ class IosDeviceInfo extends BaseDeviceInfo { /// See: https://developer.apple.com/documentation/uikit/uidevice/1620043-systemversion final String systemVersion; - IosDeviceInfo({required this.systemVersion}); + const IosDeviceInfo({required this.systemVersion}); } /// Like [device_info_plus.MacOsDeviceInfo], but without things we don't use. @@ -199,7 +199,7 @@ class MacOsDeviceInfo extends BaseDeviceInfo { /// See: https://developer.apple.com/documentation/foundation/operatingsystemversion/1415564-patchversion final int patchVersion; - MacOsDeviceInfo({ + const MacOsDeviceInfo({ required this.majorVersion, required this.minorVersion, required this.patchVersion, @@ -228,7 +228,9 @@ class MacOsDeviceInfo extends BaseDeviceInfo { // makes it less ideal. // [1]: https://gist.github.com/rajveermalviya/58b3add437280cc7f8356f3697099b7c // [2]: https://github.com/zulip/zulip-flutter/pull/724#discussion_r1628318991 -class WindowsDeviceInfo implements BaseDeviceInfo {} +class WindowsDeviceInfo implements BaseDeviceInfo { + const WindowsDeviceInfo(); +} /// Like [device_info_plus.LinuxDeviceInfo], but without things we don't use. class LinuxDeviceInfo implements BaseDeviceInfo { @@ -249,7 +251,7 @@ class LinuxDeviceInfo implements BaseDeviceInfo { /// See: https://www.freedesktop.org/software/systemd/man/latest/os-release.html#VERSION_ID= final String? versionId; - LinuxDeviceInfo({required this.name, required this.versionId}); + const LinuxDeviceInfo({required this.name, required this.versionId}); } /// Like [package_info_plus.PackageInfo], but without things we don't use. @@ -257,7 +259,7 @@ class PackageInfo { final String version; final String buildNumber; - PackageInfo({ + const PackageInfo({ required this.version, required this.buildNumber, }); @@ -329,7 +331,7 @@ class LiveZulipBinding extends ZulipBinding { device_info_plus.MacOsDeviceInfo() => MacOsDeviceInfo(majorVersion: info.majorVersion, minorVersion: info.minorVersion, patchVersion: info.patchVersion), - device_info_plus.WindowsDeviceInfo() => WindowsDeviceInfo(), + device_info_plus.WindowsDeviceInfo() => const WindowsDeviceInfo(), device_info_plus.LinuxDeviceInfo() => LinuxDeviceInfo(name: info.name, versionId: info.versionId), _ => throw UnimplementedError(), diff --git a/test/model/binding.dart b/test/model/binding.dart index a120c15a27..d070bc1d46 100644 --- a/test/model/binding.dart +++ b/test/model/binding.dart @@ -207,7 +207,7 @@ class TestZulipBinding extends ZulipBinding { /// The value that `ZulipBinding.instance.deviceInfo` should return. BaseDeviceInfo deviceInfoResult = _defaultDeviceInfoResult; - static final _defaultDeviceInfoResult = AndroidDeviceInfo(sdkInt: 33, release: '13'); + static const _defaultDeviceInfoResult = AndroidDeviceInfo(sdkInt: 33, release: '13'); void _resetDeviceInfo() { deviceInfoResult = _defaultDeviceInfoResult; @@ -221,7 +221,7 @@ class TestZulipBinding extends ZulipBinding { /// The value that `ZulipBinding.instance.packageInfo` should return. PackageInfo packageInfoResult = _defaultPackageInfo; - static final _defaultPackageInfo = PackageInfo(version: '0.0.1', buildNumber: '1'); + static const _defaultPackageInfo = PackageInfo(version: '0.0.1', buildNumber: '1'); void _resetPackageInfo() { packageInfoResult = _defaultPackageInfo; diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 9b654eb4ff..c0f970b643 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -415,7 +415,7 @@ void main() { testWidgets('can show snackbar on success', (tester) async { // Regression test for: https://github.com/zulip/zulip-flutter/issues/732 - testBinding.deviceInfoResult = IosDeviceInfo(systemVersion: '16.0'); + testBinding.deviceInfoResult = const IosDeviceInfo(systemVersion: '16.0'); final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); diff --git a/test/widgets/clipboard_test.dart b/test/widgets/clipboard_test.dart index 5a017f9b92..9f9796220f 100644 --- a/test/widgets/clipboard_test.dart +++ b/test/widgets/clipboard_test.dart @@ -58,21 +58,21 @@ void main() { } testWidgets('iOS', (WidgetTester tester) async { - testBinding.deviceInfoResult = IosDeviceInfo(systemVersion: '16.0'); + testBinding.deviceInfoResult = const IosDeviceInfo(systemVersion: '16.0'); await call(tester, text: 'asdf'); await checkClipboardText('asdf'); await checkSnackBar(tester, expected: true); }); testWidgets('Android', (WidgetTester tester) async { - testBinding.deviceInfoResult = AndroidDeviceInfo(sdkInt: 33, release: '13'); + testBinding.deviceInfoResult = const AndroidDeviceInfo(sdkInt: 33, release: '13'); await call(tester, text: 'asdf'); await checkClipboardText('asdf'); await checkSnackBar(tester, expected: false); }); testWidgets('Android <13', (WidgetTester tester) async { - testBinding.deviceInfoResult = AndroidDeviceInfo(sdkInt: 32, release: '12'); + testBinding.deviceInfoResult = const AndroidDeviceInfo(sdkInt: 32, release: '12'); await call(tester, text: 'asdf'); await checkClipboardText('asdf'); await checkSnackBar(tester, expected: true); From 022df71bbc50bbd87bb73507984bc104b5c50ad8 Mon Sep 17 00:00:00 2001 From: rajveermalviya Date: Sat, 13 Jul 2024 14:21:43 +0530 Subject: [PATCH 6/6] api: Embed platform and app info in user-agent Generate the user-agent using `deviceInfo` and `packageInfo` from ZulipBinding. Fixes: #467 --- lib/api/core.dart | 79 +++++++++++++++++++++++++++++-- test/api/core_test.dart | 63 ++++++++++++++++++++++-- test/api/fake_api.dart | 16 +++++-- test/api/route/messages_test.dart | 2 +- 4 files changed, 147 insertions(+), 13 deletions(-) diff --git a/lib/api/core.dart b/lib/api/core.dart index e2bdf83061..884d984834 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -1,9 +1,11 @@ import 'dart:convert'; import 'dart:io'; +import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; import '../log.dart'; +import '../model/binding.dart'; import '../model/localizations.dart'; import 'exception.dart'; @@ -37,6 +39,7 @@ class ApiConnection { String? email, String? apiKey, required http.Client client, + required this.useBinding, }) : assert((email != null) == (apiKey != null)), _authValue = (email != null && apiKey != null) ? _authHeaderValue(email: email, apiKey: apiKey) @@ -51,7 +54,7 @@ class ApiConnection { String? apiKey, }) : this(client: http.Client(), realmUrl: realmUrl, zulipFeatureLevel: zulipFeatureLevel, - email: email, apiKey: apiKey); + email: email, apiKey: apiKey, useBinding: true); final Uri realmUrl; @@ -69,6 +72,36 @@ class ApiConnection { /// * API docs at . int? zulipFeatureLevel; + /// Toggles the use of a user-agent generated via [ZulipBinding]. + /// + /// When set to true, the user-agent will be generated using + /// [ZulipBinding.deviceInfo] and [ZulipBinding.packageInfo]. + /// Otherwise, a fallback user-agent [kFallbackUserAgentHeader] will be used. + final bool useBinding; + + Map? _cachedUserAgentHeader; + + void addUserAgent(http.BaseRequest request) { + if (!useBinding) { + request.headers.addAll(kFallbackUserAgentHeader); + return; + } + + if (_cachedUserAgentHeader != null) { + request.headers.addAll(_cachedUserAgentHeader!); + return; + } + + final deviceInfo = ZulipBinding.instance.syncDeviceInfo; + final packageInfo = ZulipBinding.instance.syncPackageInfo; + if (deviceInfo == null || packageInfo == null) { + request.headers.addAll(kFallbackUserAgentHeader); + return; + } + _cachedUserAgentHeader = _buildUserAgentHeader(deviceInfo, packageInfo); + request.headers.addAll(_cachedUserAgentHeader!); + } + final String? _authValue; void addAuth(http.BaseRequest request) { @@ -88,9 +121,10 @@ class ApiConnection { assert(debugLog("${request.method} ${request.url}")); addAuth(request); - request.headers.addAll(userAgentHeader()); if (overrideUserAgent != null) { request.headers['User-Agent'] = overrideUserAgent; + } else { + addUserAgent(request); } final http.StreamedResponse response; @@ -213,10 +247,47 @@ Map authHeader({required String email, required String apiKey}) }; } +/// Fallback user-agent header. +/// +/// See documentation on [ApiConnection.useBinding]. +@visibleForTesting +const kFallbackUserAgentHeader = {'User-Agent': 'ZulipFlutter'}; + Map userAgentHeader() { + final deviceInfo = ZulipBinding.instance.syncDeviceInfo; + final packageInfo = ZulipBinding.instance.syncPackageInfo; + if (deviceInfo == null || packageInfo == null) { + return kFallbackUserAgentHeader; + } + return _buildUserAgentHeader(deviceInfo, packageInfo); +} + +Map _buildUserAgentHeader(BaseDeviceInfo deviceInfo, PackageInfo packageInfo) { + final osInfo = switch (deviceInfo) { + AndroidDeviceInfo( + :var release) => 'Android $release', // "Android 14" + IosDeviceInfo( + :var systemVersion) => 'iOS $systemVersion', // "iOS 17.4" + MacOsDeviceInfo( + :var majorVersion, + :var minorVersion, + :var patchVersion) => 'macOS $majorVersion.$minorVersion.$patchVersion', // "macOS 14.5.0" + WindowsDeviceInfo() => 'Windows', // "Windows" + LinuxDeviceInfo( + :var name, + :var versionId) => 'Linux; $name${versionId != null ? ' $versionId' : ''}', // "Linux; Fedora Linux 40" or "Linux; Fedora Linux" + _ => throw UnimplementedError(), + }; + final PackageInfo(:version, :buildNumber) = packageInfo; + + // Possible examples: + // 'ZulipFlutter/0.0.15+15 (Android 14)' + // 'ZulipFlutter/0.0.15+15 (iOS 17.4)' + // 'ZulipFlutter/0.0.15+15 (macOS 14.5.0)' + // 'ZulipFlutter/0.0.15+15 (Windows)' + // 'ZulipFlutter/0.0.15+15 (Linux; Fedora Linux 40)' return { - // TODO(#467) include platform, platform version, and app version - 'User-Agent': 'ZulipFlutter', + 'User-Agent': 'ZulipFlutter/$version+$buildNumber ($osInfo)', }; } diff --git a/test/api/core_test.dart b/test/api/core_test.dart index f2bd34bb2d..b70b9aec33 100644 --- a/test/api/core_test.dart +++ b/test/api/core_test.dart @@ -6,14 +6,18 @@ import 'package:http/http.dart' as http; import 'package:test/scaffolding.dart'; import 'package:zulip/api/core.dart'; import 'package:zulip/api/exception.dart'; +import 'package:zulip/model/binding.dart'; import 'package:zulip/model/localizations.dart'; +import '../model/binding.dart'; import '../stdlib_checks.dart'; import 'exception_checks.dart'; import 'fake_api.dart'; import '../example_data.dart' as eg; void main() { + TestZulipBinding.ensureInitialized(); + test('ApiConnection.get', () async { Future checkRequest(Map? params, String expectedRelativeUrl) { return FakeApiConnection.with_(account: eg.selfAccount, (connection) async { @@ -24,7 +28,7 @@ void main() { ..url.asString.equals('${eg.realmUrl.origin}$expectedRelativeUrl') ..headers.deepEquals({ ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey), - ...userAgentHeader(), + ...kFallbackUserAgentHeader, }) ..body.equals(''); }); @@ -55,7 +59,7 @@ void main() { ..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route') ..headers.deepEquals({ ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey), - ...userAgentHeader(), + ...kFallbackUserAgentHeader, if (expectContentType) 'content-type': 'application/x-www-form-urlencoded; charset=utf-8', }) @@ -88,7 +92,7 @@ void main() { ..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route') ..headers.deepEquals({ ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey), - ...userAgentHeader(), + ...kFallbackUserAgentHeader, }) ..fields.deepEquals({}) ..files.single.which((it) => it @@ -121,7 +125,7 @@ void main() { ..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route') ..headers.deepEquals({ ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey), - ...userAgentHeader(), + ...kFallbackUserAgentHeader, if (expectContentType) 'content-type': 'application/x-www-form-urlencoded; charset=utf-8', }) @@ -308,6 +312,57 @@ void main() { check(st.toString()).contains("distinctivelyNamedFromJson"); } }); + + group('ApiConnection user-agent', () { + Future checkUserAgent(String expectedUserAgent) async { + return FakeApiConnection.with_(account: eg.selfAccount, useBinding: true, + (connection) async { + connection.prepare(json: {}); + await connection.get(kExampleRouteName, (json) => json, 'example/route', null); + check(connection.lastRequest!).isA() + .headers['User-Agent'].equals(expectedUserAgent); + + connection.prepare(json: {}); + await connection.post(kExampleRouteName, (json) => json, 'example/route', null); + check(connection.lastRequest!).isA() + .headers['User-Agent'].equals(expectedUserAgent); + + connection.prepare(json: {}); + await connection.postFileFromStream( + kExampleRouteName, + (json) => json, 'example/route', + Stream.value([1]), 1, + ); + check(connection.lastRequest!).isA() + .headers['User-Agent'].equals(expectedUserAgent); + + connection.prepare(json: {}); + await connection.delete(kExampleRouteName, (json) => json, 'example/route', null); + check(connection.lastRequest!).isA() + .headers['User-Agent'].equals(expectedUserAgent); + }); + } + + const packageInfo = PackageInfo(version: '0.0.1', buildNumber: '1'); + + const testCases = [ + ('ZulipFlutter/0.0.1+1 (Android 14)', AndroidDeviceInfo(release: '14', sdkInt: 34), ), + ('ZulipFlutter/0.0.1+1 (iOS 17.4)', IosDeviceInfo(systemVersion: '17.4'), ), + ('ZulipFlutter/0.0.1+1 (macOS 14.5.0)', MacOsDeviceInfo(majorVersion: 14, minorVersion: 5, patchVersion: 0)), + ('ZulipFlutter/0.0.1+1 (Windows)', WindowsDeviceInfo(), ), + ('ZulipFlutter/0.0.1+1 (Linux; Fedora Linux 40)', LinuxDeviceInfo(name: 'Fedora Linux', versionId: '40'), ), + ('ZulipFlutter/0.0.1+1 (Linux; Fedora Linux)', LinuxDeviceInfo(name: 'Fedora Linux', versionId: null), ), + ]; + + for (final (userAgent, deviceInfo) in testCases) { + test('matches $userAgent', () async { + testBinding.deviceInfoResult = deviceInfo; + testBinding.packageInfoResult = packageInfo; + addTearDown(testBinding.reset); + await checkUserAgent(userAgent); + }); + } + }); } class DistinctiveError extends Error { diff --git a/test/api/fake_api.dart b/test/api/fake_api.dart index 87c450f3dc..0837f72e97 100644 --- a/test/api/fake_api.dart +++ b/test/api/fake_api.dart @@ -134,20 +134,23 @@ class FakeApiConnection extends ApiConnection { int? zulipFeatureLevel = eg.futureZulipFeatureLevel, String? email, String? apiKey, + bool useBinding = false, }) : this._( realmUrl: realmUrl ?? eg.realmUrl, zulipFeatureLevel: zulipFeatureLevel, email: email, apiKey: apiKey, client: FakeHttpClient(), + useBinding: useBinding, ); - FakeApiConnection.fromAccount(Account account) + FakeApiConnection.fromAccount(Account account, {required bool useBinding}) : this( realmUrl: account.realmUrl, zulipFeatureLevel: account.zulipFeatureLevel, email: account.email, - apiKey: account.apiKey); + apiKey: account.apiKey, + useBinding: useBinding); FakeApiConnection._({ required super.realmUrl, @@ -155,6 +158,7 @@ class FakeApiConnection extends ApiConnection { super.email, super.apiKey, required this.client, + required super.useBinding, }) : super(client: client); final FakeHttpClient client; @@ -171,12 +175,16 @@ class FakeApiConnection extends ApiConnection { Uri? realmUrl, int? zulipFeatureLevel = eg.futureZulipFeatureLevel, Account? account, + bool useBinding = false, }) async { assert((account == null) || (realmUrl == null && zulipFeatureLevel == eg.futureZulipFeatureLevel)); final connection = (account != null) - ? FakeApiConnection.fromAccount(account) - : FakeApiConnection(realmUrl: realmUrl, zulipFeatureLevel: zulipFeatureLevel); + ? FakeApiConnection.fromAccount(account, useBinding: useBinding) + : FakeApiConnection( + realmUrl: realmUrl, + zulipFeatureLevel: zulipFeatureLevel, + useBinding: useBinding); try { return await fn(connection); } finally { diff --git a/test/api/route/messages_test.dart b/test/api/route/messages_test.dart index 1156fa91da..c860c4de43 100644 --- a/test/api/route/messages_test.dart +++ b/test/api/route/messages_test.dart @@ -316,7 +316,7 @@ void main() { ..method.equals('POST') ..url.path.equals('/api/v1/messages') ..bodyFields.deepEquals(expectedBodyFields) - ..headers['User-Agent'].equals(expectedUserAgent ?? userAgentHeader()['User-Agent']!); + ..headers['User-Agent'].equals(expectedUserAgent ?? kFallbackUserAgentHeader['User-Agent']!); } test('smoke', () {