diff --git a/pkgs/unified_analytics/CHANGELOG.md b/pkgs/unified_analytics/CHANGELOG.md index a35dcd692..fd40259c2 100644 --- a/pkgs/unified_analytics/CHANGELOG.md +++ b/pkgs/unified_analytics/CHANGELOG.md @@ -1,3 +1,7 @@ +## 5.8.6-wip + +- Refactored session handler class to use the last modified timestamp as the last ping value + ## 5.8.5 - Fix late initialization error for `Analytics.userProperty` [bug](https://github.com/dart-lang/tools/issues/238) diff --git a/pkgs/unified_analytics/lib/src/analytics.dart b/pkgs/unified_analytics/lib/src/analytics.dart index 0c3a98721..0a28a40c6 100644 --- a/pkgs/unified_analytics/lib/src/analytics.dart +++ b/pkgs/unified_analytics/lib/src/analytics.dart @@ -454,8 +454,8 @@ class AnalyticsImpl implements Analytics { errorHandler: _errorHandler, ); - // Initialize the session handler with the session_id and last_ping - // variables by parsing the json file + // Initialize the session handler with the session_id + // by parsing the json file _sessionHandler.initialize(telemetryEnabled); } diff --git a/pkgs/unified_analytics/lib/src/constants.dart b/pkgs/unified_analytics/lib/src/constants.dart index 4dfca3e2e..e9034bbb1 100644 --- a/pkgs/unified_analytics/lib/src/constants.dart +++ b/pkgs/unified_analytics/lib/src/constants.dart @@ -82,7 +82,7 @@ const int kLogFileLength = 2500; const String kLogFileName = 'dart-flutter-telemetry.log'; /// The current version of the package, should be in line with pubspec version. -const String kPackageVersion = '5.8.5'; +const String kPackageVersion = '5.8.6-wip'; /// The minimum length for a session. const int kSessionDurationMinutes = 30; diff --git a/pkgs/unified_analytics/lib/src/initializer.dart b/pkgs/unified_analytics/lib/src/initializer.dart index 22898a92d..4f84ef1d5 100644 --- a/pkgs/unified_analytics/lib/src/initializer.dart +++ b/pkgs/unified_analytics/lib/src/initializer.dart @@ -2,8 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -import 'dart:convert'; - import 'package:clock/clock.dart'; import 'package:file/file.dart'; import 'package:path/path.dart' as p; @@ -75,17 +73,19 @@ class Initializer { logFile.createSync(recursive: true); } - /// Creates the session json file which will contain - /// the current session id along with the timestamp for - /// the last ping which will be used to increment the session - /// if current timestamp is greater than the session window. - static void createSessionFile({required File sessionFile}) { - final now = clock.now(); + /// Creates the session file which will contain + /// the current session id which is the current timestamp. + /// + /// [sessionIdOverride] can be provided as an override, otherwise it + /// will use the current timestamp from [Clock.now]. + static void createSessionFile({ + required File sessionFile, + DateTime? sessionIdOverride, + }) { + final now = sessionIdOverride ?? clock.now(); sessionFile.createSync(recursive: true); - sessionFile.writeAsStringSync(jsonEncode({ - 'session_id': now.millisecondsSinceEpoch, - 'last_ping': now.millisecondsSinceEpoch, - })); + sessionFile + .writeAsStringSync('{"session_id": ${now.millisecondsSinceEpoch}}'); } /// This will check that there is a client ID populated in diff --git a/pkgs/unified_analytics/lib/src/session.dart b/pkgs/unified_analytics/lib/src/session.dart index 80305ee18..d19b749c5 100644 --- a/pkgs/unified_analytics/lib/src/session.dart +++ b/pkgs/unified_analytics/lib/src/session.dart @@ -19,8 +19,7 @@ class Session { final File sessionFile; final ErrorHandler _errorHandler; - late int _sessionId; - late int _lastPing; + int? _sessionId; Session({ required this.homeDirectory, @@ -31,7 +30,7 @@ class Session { _errorHandler = errorHandler; /// This will use the data parsed from the - /// session json file in the dart-tool directory + /// session file in the dart-tool directory /// to get the session id if the last ping was within /// [kSessionDurationMinutes]. /// @@ -40,30 +39,27 @@ class Session { /// /// Note, the file will always be updated when calling this method /// because the last ping variable will always need to be persisted. - int getSessionId() { + int? getSessionId() { _refreshSessionData(); final now = clock.now(); // Convert the epoch time from the last ping into datetime and check if we // are within the kSessionDurationMinutes. - final lastPingDateTime = DateTime.fromMillisecondsSinceEpoch(_lastPing); + final lastPingDateTime = sessionFile.lastModifiedSync(); if (now.difference(lastPingDateTime).inMinutes > kSessionDurationMinutes) { - // In this case, we will need to change both the session id - // and the last ping value + // Update the session file with the latest session id _sessionId = now.millisecondsSinceEpoch; + sessionFile.writeAsStringSync('{"session_id": $_sessionId}'); + } else { + // Update the last modified timestamp with the current timestamp so that + // we can use it for the next _lastPing calculation + sessionFile.setLastModifiedSync(now); } - // Update the last ping to reflect session activity continuing - _lastPing = now.millisecondsSinceEpoch; - - // Rewrite the session object back to the file to persist - // for future events - sessionFile.writeAsStringSync(toJson()); - return _sessionId; } - /// Preps the [Session] class with the data found in the session json file. + /// Preps the [Session] class with the data found in the session file. /// /// We must check if telemetry is enabled to refresh the session data /// because the refresh method will write to the session file and for @@ -73,15 +69,9 @@ class Session { if (telemetryEnabled) _refreshSessionData(); } - /// Return a json formatted representation of the class. - String toJson() => jsonEncode({ - 'session_id': _sessionId, - 'last_ping': _lastPing, - }); - /// This will go to the session file within the dart-tool - /// directory and fetch the latest data from the json to update - /// the class's variables. If the json file is malformed, a new + /// directory and fetch the latest data from the session file to update + /// the class's variables. If the session file is malformed, a new /// session file will be recreated. /// /// This allows the session data in this class to always be up @@ -94,13 +84,18 @@ class Session { final sessionObj = jsonDecode(sessionFileContents) as Map; _sessionId = sessionObj['session_id'] as int; - _lastPing = sessionObj['last_ping'] as int; } try { + // Failing to parse the contents will result in the current timestamp + // being used as the session id and will get used to recreate the file parseContents(); } on FormatException catch (err) { - Initializer.createSessionFile(sessionFile: sessionFile); + final now = clock.now(); + Initializer.createSessionFile( + sessionFile: sessionFile, + sessionIdOverride: now, + ); _errorHandler.log(Event.analyticsException( workflow: 'Session._refreshSessionData', @@ -109,11 +104,13 @@ class Session { )); // Fallback to setting the session id as the current time - final now = clock.now(); _sessionId = now.millisecondsSinceEpoch; - _lastPing = now.millisecondsSinceEpoch; } on FileSystemException catch (err) { - Initializer.createSessionFile(sessionFile: sessionFile); + final now = clock.now(); + Initializer.createSessionFile( + sessionFile: sessionFile, + sessionIdOverride: now, + ); _errorHandler.log(Event.analyticsException( workflow: 'Session._refreshSessionData', @@ -122,9 +119,7 @@ class Session { )); // Fallback to setting the session id as the current time - final now = clock.now(); _sessionId = now.millisecondsSinceEpoch; - _lastPing = now.millisecondsSinceEpoch; } } } diff --git a/pkgs/unified_analytics/pubspec.yaml b/pkgs/unified_analytics/pubspec.yaml index 8cd92f041..6c4f24809 100644 --- a/pkgs/unified_analytics/pubspec.yaml +++ b/pkgs/unified_analytics/pubspec.yaml @@ -4,7 +4,7 @@ description: >- to Google Analytics. # When updating this, keep the version consistent with the changelog and the # value in lib/src/constants.dart. -version: 5.8.5 +version: 5.8.6-wip repository: https://github.com/dart-lang/tools/tree/main/pkgs/unified_analytics environment: diff --git a/pkgs/unified_analytics/test/unified_analytics_test.dart b/pkgs/unified_analytics/test/unified_analytics_test.dart index 39046f74d..8c0849338 100644 --- a/pkgs/unified_analytics/test/unified_analytics_test.dart +++ b/pkgs/unified_analytics/test/unified_analytics_test.dart @@ -132,7 +132,7 @@ void main() { test('Resetting session file when data is malformed', () { // Purposefully write content to the session file that - // can't be decoded as json + // can't be decoded as an integer sessionFile.writeAsStringSync('contents'); // Define the initial time to start @@ -143,8 +143,7 @@ void main() { final timestamp = clock.now().millisecondsSinceEpoch.toString(); expect(sessionFile.readAsStringSync(), 'contents'); analytics.userProperty.preparePayload(); - expect(sessionFile.readAsStringSync(), - '{"session_id":$timestamp,"last_ping":$timestamp}'); + expect(sessionFile.readAsStringSync(), '{"session_id": $timestamp}'); // Attempting to fetch the session id when malformed should also // send an error event while parsing @@ -158,9 +157,9 @@ void main() { test('Handles malformed session file on startup', () { // Ensure that we are able to send an error message on startup if - // we encounter an error while parsing the contents of the json file + // we encounter an error while parsing the contents of the session file // for session data - sessionFile.writeAsStringSync('contents'); + sessionFile.writeAsStringSync('not a valid session id'); analytics = Analytics.test( tool: initialTool, @@ -185,7 +184,7 @@ void main() { expect(errorEvent!.eventData['workflow'], 'Session._refreshSessionData'); expect(errorEvent.eventData['error'], 'FormatException'); expect(errorEvent.eventData['description'], - 'message: Unexpected character\nsource: contents'); + 'message: Unexpected character\nsource: not a valid session id'); }); test('Resetting session file when file is removed', () { @@ -200,8 +199,7 @@ void main() { final timestamp = clock.now().millisecondsSinceEpoch.toString(); expect(sessionFile.existsSync(), false); analytics.userProperty.preparePayload(); - expect(sessionFile.readAsStringSync(), - '{"session_id":$timestamp,"last_ping":$timestamp}'); + expect(sessionFile.readAsStringSync(), '{"session_id": $timestamp}'); // Attempting to fetch the session id when malformed should also // send an error event while parsing @@ -701,14 +699,10 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion ); secondAnalytics.clientShowedMessage(); - // Read the contents of the session file - final sessionFileContents = sessionFile.readAsStringSync(); - final sessionObj = - jsonDecode(sessionFileContents) as Map; - expect(secondAnalytics.userPropertyMap['session_id']?['value'], start.millisecondsSinceEpoch); - expect(sessionObj['last_ping'], start.millisecondsSinceEpoch); + expect(sessionFile.lastModifiedSync().millisecondsSinceEpoch, + start.millisecondsSinceEpoch); }); // Add time to the start time that is less than the duration @@ -739,17 +733,13 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion // no events will be sent thirdAnalytics.send(testEvent); - // Read the contents of the session file - final sessionFileContents = sessionFile.readAsStringSync(); - final sessionObj = - jsonDecode(sessionFileContents) as Map; - expect(thirdAnalytics.userPropertyMap['session_id']?['value'], start.millisecondsSinceEpoch, reason: 'The session id should not have changed since it was made ' 'within the duration'); - expect(sessionObj['last_ping'], end.millisecondsSinceEpoch, - reason: 'The last_ping value should have been updated'); + expect(sessionFile.lastModifiedSync().millisecondsSinceEpoch, + end.millisecondsSinceEpoch, + reason: 'The last modified value should have been updated'); }); }); @@ -782,14 +772,10 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion ); secondAnalytics.clientShowedMessage(); - // Read the contents of the session file - final sessionFileContents = sessionFile.readAsStringSync(); - final sessionObj = - jsonDecode(sessionFileContents) as Map; - expect(secondAnalytics.userPropertyMap['session_id']?['value'], start.millisecondsSinceEpoch); - expect(sessionObj['last_ping'], start.millisecondsSinceEpoch); + expect(sessionFile.lastModifiedSync().millisecondsSinceEpoch, + start.millisecondsSinceEpoch); secondAnalytics.send(testEvent); }); @@ -822,17 +808,13 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion // no events will be sent thirdAnalytics.send(testEvent); - // Read the contents of the session file - final sessionFileContents = sessionFile.readAsStringSync(); - final sessionObj = - jsonDecode(sessionFileContents) as Map; - expect(thirdAnalytics.userPropertyMap['session_id']?['value'], end.millisecondsSinceEpoch, reason: 'The session id should have changed since it was made ' 'outside the duration'); - expect(sessionObj['last_ping'], end.millisecondsSinceEpoch, - reason: 'The last_ping value should have been updated'); + expect(sessionFile.lastModifiedSync().millisecondsSinceEpoch, + end.millisecondsSinceEpoch, + reason: 'The last modified value should have been updated'); }); });