Limit writes to session file (#243)

* Refactor to use modified dt for _lastPing

* Fix tests + misc dartdoc fixes

* Bump version

* Create `sessionFile` in constructor

* Simplify `lastPingDateTime`

* Revert filename + pass session id to initializer

* Use nullable field for `_sessionId`

* Format fix

* Change to wip + use `else` block for timestamp

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)

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);
   }
 

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;

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(<String, int>{
-      '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

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(<String, int>{
-        '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<String, Object?>;
       _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;
     }
   }
 }

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:

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<String, Object?>;
-
       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<String, Object?>;
-
       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<String, Object?>;
-
       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<String, Object?>;
-
       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');
     });
   });