Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

delete old log file if it exceeds kMaxLogFileSize of 25MiB #277

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkgs/unified_analytics/lib/src/constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ const String kGoogleAnalyticsMeasurementId = 'G-04BXPVBCWJ';
/// How many data records to store in the log file.
const int kLogFileLength = 2500;

/// The maximum allowed size of the telemetry log file.
///
/// 25 MiB.
const int kMaxLogFileSize = 25 * (1 << 20);

/// Filename for the log file to persist sent events on user's machine.
const String kLogFileName = 'dart-flutter-telemetry.log';

Expand Down
14 changes: 11 additions & 3 deletions pkgs/unified_analytics/lib/src/log_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,18 @@ class LogHandler {
/// This will keep the max number of records limited to equal to
/// or less than [kLogFileLength] records.
void save({required Map<String, Object?> data}) {
var records = logFile.readAsLinesSync();
final content = '${jsonEncode(data)}\n';

try {
final stat = logFile.statSync();
List<String> records;
if (stat.size > kMaxLogFileSize) {
logFile.deleteSync();
logFile.createSync();
records = [];
} else {
records = logFile.readAsLinesSync();
}
christopherfujino marked this conversation as resolved.
Show resolved Hide resolved
final content = '${jsonEncode(data)}\n';

// When the record count is less than the max, add as normal;
// else drop the oldest records until equal to max
if (records.length < kLogFileLength) {
Expand Down
92 changes: 92 additions & 0 deletions pkgs/unified_analytics/test/log_handler_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
// 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:file/file.dart';
import 'package:file/memory.dart';
import 'package:path/path.dart' as p;
import 'package:test/fake.dart';
import 'package:test/test.dart';

import 'package:unified_analytics/src/constants.dart';
Expand Down Expand Up @@ -212,6 +215,47 @@ void main() {
logHandler.save(data: {});
});

test('deletes log file larger than kMaxLogFileSize', () async {
var deletedLargeLogFile = false;
var wroteDataToLogFile = false;
const data = <String, Object?>{};
final logFile = _FakeFile('log.txt')
.._deleteSyncImpl = (() => deletedLargeLogFile = true)
.._createSyncImpl = () {}
.._statSyncImpl = (() => _FakeFileStat(kMaxLogFileSize + 1))
.._writeAsStringSync = (contents, {mode = FileMode.append}) {
expect(contents.trim(), data.toString());
expect(mode, FileMode.writeOnlyAppend);
wroteDataToLogFile = true;
};
final logHandler = LogHandler(logFile: logFile);

logHandler.save(data: data);
expect(deletedLargeLogFile, isTrue);
expect(wroteDataToLogFile, isTrue);
});

test('does not delete log file if smaller than kMaxLogFileSize', () async {
var wroteDataToLogFile = false;
const data = <String, Object?>{};
final logFile = _FakeFile('log.txt')
.._deleteSyncImpl =
(() => fail('called logFile.deleteSync() when file was less than '
'kMaxLogFileSize'))
.._createSyncImpl = () {}
.._readAsLinesSyncImpl = (() => ['three', 'previous', 'lines'])
.._statSyncImpl = (() => _FakeFileStat(kMaxLogFileSize - 1))
.._writeAsStringSync = (contents, {mode = FileMode.append}) {
expect(contents.trim(), data.toString());
expect(mode, FileMode.writeOnlyAppend);
wroteDataToLogFile = true;
};
final logHandler = LogHandler(logFile: logFile);

logHandler.save(data: data);
expect(wroteDataToLogFile, isTrue);
});

test('Catching cast errors for each log record silently', () async {
// Write a json array to the log file which will cause
// a cast error when parsing each line
Expand Down Expand Up @@ -290,3 +334,51 @@ void main() {
expect(newString, testString);
});
}

class _FakeFileStat extends Fake implements FileStat {
_FakeFileStat(this.size);

@override
final int size;
}

class _FakeFile extends Fake implements File {
_FakeFile(this.path);
Comment on lines +345 to +346
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why use mutable fields for method implementations over optional constructor parameters? Do you find this more readable?


List<String> Function()? _readAsLinesSyncImpl;

@override
List<String> readAsLinesSync({Encoding encoding = utf8}) =>
_readAsLinesSyncImpl!();

@override
final String path;

FileStat Function()? _statSyncImpl;

@override
FileStat statSync() => _statSyncImpl!();

void Function()? _deleteSyncImpl;

@override
void deleteSync({bool recursive = false}) => _deleteSyncImpl!();

void Function()? _createSyncImpl;

@override
void createSync({bool recursive = false, bool exclusive = false}) {
return _createSyncImpl!();
}

void Function(String contents, {FileMode mode})? _writeAsStringSync;

@override
void writeAsStringSync(
String contents, {
FileMode mode = FileMode.write,
Encoding encoding = utf8,
bool flush = false,
}) =>
_writeAsStringSync!(contents, mode: mode);
}