do not warn about out-of-date Flutter installation too often (#9271)

* do not warn about out-of-date Flutter installation too often

* style fix

packages/flutter_tools/lib/src/version.dart

@@ -162,9 +162,18 @@ class FlutterVersion {
   @visibleForTesting
   static final Duration kVersionAgeConsideredUpToDate = kCheckAgeConsideredUpToDate * 4;
 
-  /// The prefix of the stamp file where we cache Flutter version check data.
+  /// The amount of time we wait between issuing a warning.
+  ///
+  /// This is to avoid annoying users who are unable to upgrade right away.
   @visibleForTesting
-  static const String kFlutterVersionCheckStampFile = 'flutter_version_check';
+  static const Duration kMaxTimeSinceLastWarning = const Duration(days: 1);
+
+  /// The amount of time we pause for to let the user read the message about
+  /// outdated Flutter installation.
+  ///
+  /// This can be customized in tests to speed them up.
+  @visibleForTesting
+  static Duration kPauseToLetUserReadTheMessage = const Duration(seconds: 2);
 
   /// Checks if the currently installed version of Flutter is up-to-date, and
   /// warns the user if it isn't.
@@ -185,8 +194,17 @@ class FlutterVersion {
       return latestFlutterCommitDate.isAfter(localFrameworkCommitDate);
     }
 
-    if (installationSeemsOutdated && await newerFrameworkVersionAvailable())
+    final VersionCheckStamp stamp = await VersionCheckStamp.load();
+    final DateTime lastTimeWarningWasPrinted = stamp.lastTimeWarningWasPrinted ?? _clock.agoBy(kMaxTimeSinceLastWarning * 2);
+    final bool beenAWhileSinceWarningWasPrinted = _clock.now().difference(lastTimeWarningWasPrinted) > kMaxTimeSinceLastWarning;
+
+    if (beenAWhileSinceWarningWasPrinted && installationSeemsOutdated && await newerFrameworkVersionAvailable()) {
       printStatus(versionOutOfDateMessage(frameworkAge), emphasis: true);
+      stamp.store(
+        newTimeWarningWasPrinted: _clock.now(),
+      );
+      await new Future<Null>.delayed(kPauseToLetUserReadTheMessage);
+    }
   }
 
   @visibleForTesting
@@ -213,26 +231,23 @@ class FlutterVersion {
   /// unable to reach the server to get the latest version.
   Future<DateTime> _getLatestAvailableFlutterVersion() async {
     Cache.checkLockAcquired();
-    const JsonEncoder kPrettyJsonEncoder = const JsonEncoder.withIndent('  ');
-    final String versionCheckStamp = Cache.instance.getStampFor(kFlutterVersionCheckStampFile);
+    final VersionCheckStamp versionCheckStamp = await VersionCheckStamp.load();
 
-    if (versionCheckStamp != null) {
-      final Map<String, String> data = JSON.decode(versionCheckStamp);
-      final DateTime lastTimeVersionWasChecked = DateTime.parse(data['lastTimeVersionWasChecked']);
-      final Duration timeSinceLastCheck = _clock.now().difference(lastTimeVersionWasChecked);
+    if (versionCheckStamp.lastTimeVersionWasChecked != null) {
+      final Duration timeSinceLastCheck = _clock.now().difference(versionCheckStamp.lastTimeVersionWasChecked);
 
       // Don't ping the server too often. Return cached value if it's fresh.
       if (timeSinceLastCheck < kCheckAgeConsideredUpToDate)
-        return DateTime.parse(data['lastKnownRemoteVersion']);
+        return versionCheckStamp.lastKnownRemoteVersion;
     }
 
     // Cache is empty or it's been a while since the last server ping. Ping the server.
     try {
       final DateTime remoteFrameworkCommitDate = DateTime.parse(await FlutterVersion.fetchRemoteFrameworkCommitDate());
-      Cache.instance.setStampFor(kFlutterVersionCheckStampFile, kPrettyJsonEncoder.convert(<String, String>{
-        'lastTimeVersionWasChecked': '${_clock.now()}',
-        'lastKnownRemoteVersion': '$remoteFrameworkCommitDate',
-      }));
+      versionCheckStamp.store(
+        newTimeVersionWasChecked: _clock.now(),
+        newKnownRemoteVersion: remoteFrameworkCommitDate,
+      );
       return remoteFrameworkCommitDate;
     } on VersionCheckError catch (error) {
       // This happens when any of the git commands fails, which can happen when
@@ -244,6 +259,102 @@ class FlutterVersion {
   }
 }
 
+/// Contains data and load/save logic pertaining to Flutter version checks.
+@visibleForTesting
+class VersionCheckStamp {
+  /// The prefix of the stamp file where we cache Flutter version check data.
+  @visibleForTesting
+  static const String kFlutterVersionCheckStampFile = 'flutter_version_check';
+
+  const VersionCheckStamp({
+    this.lastTimeVersionWasChecked,
+    this.lastKnownRemoteVersion,
+    this.lastTimeWarningWasPrinted,
+  });
+
+  final DateTime lastTimeVersionWasChecked;
+  final DateTime lastKnownRemoteVersion;
+  final DateTime lastTimeWarningWasPrinted;
+
+  static Future<VersionCheckStamp> load() async {
+    final String versionCheckStamp = Cache.instance.getStampFor(kFlutterVersionCheckStampFile);
+
+    if (versionCheckStamp != null) {
+      // Attempt to parse stamp JSON.
+      try {
+        final dynamic json = JSON.decode(versionCheckStamp);
+        if (json is Map) {
+          printTrace('Warning: expected version stamp to be a Map but found: $json');
+          return fromJson(json);
+        }
+      } catch (error, stackTrace) {
+        // Do not crash if JSON is malformed.
+        printTrace('${error.runtimeType}: $error\n$stackTrace');
+      }
+    }
+
+    // Stamp is missing or is malformed.
+    return new VersionCheckStamp();
+  }
+
+  static VersionCheckStamp fromJson(Map<String, String> json) {
+    DateTime readDateTime(String property) {
+      return json.containsKey(property)
+        ? DateTime.parse(json[property])
+        : null;
+    }
+
+    return new VersionCheckStamp(
+      lastTimeVersionWasChecked: readDateTime('lastTimeVersionWasChecked'),
+      lastKnownRemoteVersion: readDateTime('lastKnownRemoteVersion'),
+      lastTimeWarningWasPrinted: readDateTime('lastTimeWarningWasPrinted'),
+    );
+  }
+
+  Future<Null> store({
+    DateTime newTimeVersionWasChecked,
+    DateTime newKnownRemoteVersion,
+    DateTime newTimeWarningWasPrinted,
+  }) async {
+    final Map<String, String> jsonData = toJson();
+
+    if (newTimeVersionWasChecked != null)
+      jsonData['lastTimeVersionWasChecked'] = '$newTimeVersionWasChecked';
+
+    if (newKnownRemoteVersion != null)
+      jsonData['lastKnownRemoteVersion'] = '$newKnownRemoteVersion';
+
+    if (newTimeWarningWasPrinted != null)
+      jsonData['lastTimeWarningWasPrinted'] = '$newTimeWarningWasPrinted';
+
+    const JsonEncoder kPrettyJsonEncoder = const JsonEncoder.withIndent('  ');
+    Cache.instance.setStampFor(kFlutterVersionCheckStampFile, kPrettyJsonEncoder.convert(jsonData));
+  }
+
+  Map<String, String> toJson({
+    DateTime updateTimeVersionWasChecked,
+    DateTime updateKnownRemoteVersion,
+    DateTime updateTimeWarningWasPrinted,
+  }) {
+    updateTimeVersionWasChecked = updateTimeVersionWasChecked ?? lastTimeVersionWasChecked;
+    updateKnownRemoteVersion = updateKnownRemoteVersion ?? lastKnownRemoteVersion;
+    updateTimeWarningWasPrinted = updateTimeWarningWasPrinted ?? lastTimeWarningWasPrinted;
+
+    final Map<String, String> jsonData = <String, String>{};
+
+    if (updateTimeVersionWasChecked != null)
+      jsonData['lastTimeVersionWasChecked'] = '$updateTimeVersionWasChecked';
+
+    if (updateKnownRemoteVersion != null)
+      jsonData['lastKnownRemoteVersion'] = '$updateKnownRemoteVersion';
+
+    if (updateTimeWarningWasPrinted != null)
+      jsonData['lastTimeWarningWasPrinted'] = '$updateTimeWarningWasPrinted';
+
+    return jsonData;
+  }
+}
+
 /// Thrown when we fail to check Flutter version.
 ///
 /// This can happen when we attempt to `git fetch` but there is no network, or

packages/flutter_tools/test/src/version_test.dart

@@ -5,7 +5,6 @@
 import 'dart:convert';
 
 import 'package:collection/collection.dart';
-import 'package:meta/meta.dart';
 import 'package:mockito/mockito.dart';
 import 'package:process/process.dart';
 import 'package:quiver/time.dart';
@@ -25,12 +24,12 @@ final DateTime _upToDateVersion = _testClock.agoBy(FlutterVersion.kVersionAgeCon
 final DateTime _outOfDateVersion = _testClock.agoBy(FlutterVersion.kVersionAgeConsideredUpToDate * 2);
 final DateTime _stampUpToDate = _testClock.agoBy(FlutterVersion.kCheckAgeConsideredUpToDate ~/ 2);
 final DateTime _stampOutOfDate = _testClock.agoBy(FlutterVersion.kCheckAgeConsideredUpToDate * 2);
-const String _stampMissing = '____stamp_missing____';
 
 void main() {
-  group('FlutterVersion', () {
+  group('$FlutterVersion', () {
     setUpAll(() {
       Cache.disableLocking();
+      FlutterVersion.kPauseToLetUserReadTheMessage = Duration.ZERO;
     });
 
     testFlutterVersion('prints nothing when Flutter installation looks fresh', () async {
@@ -44,12 +43,13 @@ void main() {
 
       fakeData(
         localCommitDate: _outOfDateVersion,
-        versionCheckStamp: _testStamp(
+        stamp: new VersionCheckStamp(
           lastTimeVersionWasChecked: _stampOutOfDate,
           lastKnownRemoteVersion: _outOfDateVersion,
         ),
         remoteCommitDate: _outOfDateVersion,
         expectSetStamp: true,
+        expectServerPing: true,
       );
 
       await version.checkFlutterVersionFreshness();
@@ -61,28 +61,57 @@ void main() {
 
       fakeData(
         localCommitDate: _outOfDateVersion,
-        versionCheckStamp: _testStamp(
+        stamp: new VersionCheckStamp(
           lastTimeVersionWasChecked: _stampUpToDate,
           lastKnownRemoteVersion: _upToDateVersion,
         ),
+        expectSetStamp: true,
+      );
+
+      await version.checkFlutterVersionFreshness();
+      _expectVersionMessage(FlutterVersion.versionOutOfDateMessage(_testClock.now().difference(_outOfDateVersion)));
+    });
+
+    testFlutterVersion('does not print warning if printed recently', () async {
+      final FlutterVersion version = FlutterVersion.instance;
+
+      fakeData(
+          localCommitDate: _outOfDateVersion,
+          stamp: new VersionCheckStamp(
+              lastTimeVersionWasChecked: _stampUpToDate,
+              lastKnownRemoteVersion: _upToDateVersion,
+          ),
+          expectSetStamp: true,
       );
 
       await version.checkFlutterVersionFreshness();
       _expectVersionMessage(FlutterVersion.versionOutOfDateMessage(_testClock.now().difference(_outOfDateVersion)));
+      expect((await VersionCheckStamp.load()).lastTimeWarningWasPrinted, _testClock.now());
+
+      await version.checkFlutterVersionFreshness();
+      _expectVersionMessage('');
     });
 
-    testFlutterVersion('pings server when version stamp is missing', () async {
+    testFlutterVersion('pings server when version stamp is missing then does not', () async {
       final FlutterVersion version = FlutterVersion.instance;
 
       fakeData(
           localCommitDate: _outOfDateVersion,
-          versionCheckStamp: _stampMissing,
           remoteCommitDate: _upToDateVersion,
           expectSetStamp: true,
+          expectServerPing: true,
       );
 
       await version.checkFlutterVersionFreshness();
       _expectVersionMessage(FlutterVersion.versionOutOfDateMessage(_testClock.now().difference(_outOfDateVersion)));
+
+      // Immediate subsequent check is not expected to ping the server.
+      fakeData(
+        localCommitDate: _outOfDateVersion,
+        stamp: await VersionCheckStamp.load(),
+      );
+      await version.checkFlutterVersionFreshness();
+      _expectVersionMessage('');
     });
 
     testFlutterVersion('pings server when version stamp is out-of-date', () async {
@@ -90,12 +119,13 @@ void main() {
 
       fakeData(
           localCommitDate: _outOfDateVersion,
-          versionCheckStamp: _testStamp(
+          stamp: new VersionCheckStamp(
               lastTimeVersionWasChecked: _stampOutOfDate,
               lastKnownRemoteVersion: _testClock.ago(days: 2),
           ),
           remoteCommitDate: _upToDateVersion,
           expectSetStamp: true,
+          expectServerPing: true,
       );
 
       await version.checkFlutterVersionFreshness();
@@ -107,26 +137,98 @@ void main() {
 
       fakeData(
           localCommitDate: _outOfDateVersion,
-          versionCheckStamp: _stampMissing,
           errorOnFetch: true,
+          expectServerPing: true,
       );
 
       await version.checkFlutterVersionFreshness();
       _expectVersionMessage('');
     });
   });
+
+  group('$VersionCheckStamp', () {
+    void _expectDefault(VersionCheckStamp stamp) {
+      expect(stamp.lastKnownRemoteVersion, isNull);
+      expect(stamp.lastTimeVersionWasChecked, isNull);
+      expect(stamp.lastTimeWarningWasPrinted, isNull);
+    }
+
+    testFlutterVersion('loads blank when stamp file missing', () async {
+      fakeData();
+      _expectDefault(await VersionCheckStamp.load());
+    });
+
+    testFlutterVersion('loads blank when stamp file is malformed JSON', () async {
+      fakeData(stampJson: '<');
+      _expectDefault(await VersionCheckStamp.load());
+    });
+
+    testFlutterVersion('loads blank when stamp file is well-formed but invalid JSON', () async {
+      fakeData(stampJson: '[]');
+      _expectDefault(await VersionCheckStamp.load());
+    });
+
+    testFlutterVersion('loads valid JSON', () async {
+      fakeData(stampJson: '''
+      {
+        "lastKnownRemoteVersion": "${_testClock.ago(days: 1)}",
+        "lastTimeVersionWasChecked": "${_testClock.ago(days: 2)}",
+        "lastTimeWarningWasPrinted": "${_testClock.now()}"
+      }
+      ''');
+
+      final VersionCheckStamp stamp = await VersionCheckStamp.load();
+      expect(stamp.lastKnownRemoteVersion, _testClock.ago(days: 1));
+      expect(stamp.lastTimeVersionWasChecked, _testClock.ago(days: 2));
+      expect(stamp.lastTimeWarningWasPrinted, _testClock.now());
+    });
+
+    testFlutterVersion('stores version stamp', () async {
+      fakeData(expectSetStamp: true);
+
+      _expectDefault(await VersionCheckStamp.load());
+
+      final VersionCheckStamp stamp = new VersionCheckStamp(
+        lastKnownRemoteVersion: _testClock.ago(days: 1),
+        lastTimeVersionWasChecked: _testClock.ago(days: 2),
+        lastTimeWarningWasPrinted: _testClock.now(),
+      );
+      await stamp.store();
+
+      final VersionCheckStamp storedStamp = await VersionCheckStamp.load();
+      expect(storedStamp.lastKnownRemoteVersion, _testClock.ago(days: 1));
+      expect(storedStamp.lastTimeVersionWasChecked, _testClock.ago(days: 2));
+      expect(storedStamp.lastTimeWarningWasPrinted, _testClock.now());
+    });
+
+    testFlutterVersion('overwrites individual fields', () async {
+      fakeData(expectSetStamp: true);
+
+      _expectDefault(await VersionCheckStamp.load());
+
+      final VersionCheckStamp stamp = new VersionCheckStamp(
+        lastKnownRemoteVersion: _testClock.ago(days: 10),
+        lastTimeVersionWasChecked: _testClock.ago(days: 9),
+        lastTimeWarningWasPrinted: _testClock.ago(days: 8),
+      );
+      await stamp.store(
+        newKnownRemoteVersion: _testClock.ago(days: 1),
+        newTimeVersionWasChecked: _testClock.ago(days: 2),
+        newTimeWarningWasPrinted: _testClock.now(),
+      );
+
+      final VersionCheckStamp storedStamp = await VersionCheckStamp.load();
+      expect(storedStamp.lastKnownRemoteVersion, _testClock.ago(days: 1));
+      expect(storedStamp.lastTimeVersionWasChecked, _testClock.ago(days: 2));
+      expect(storedStamp.lastTimeWarningWasPrinted, _testClock.now());
+    });
+  });
 }
 
 void _expectVersionMessage(String message) {
   final BufferLogger logger = context[Logger];
   expect(logger.statusText.trim(), message.trim());
-}
-
-String _testStamp({@required DateTime lastTimeVersionWasChecked, @required DateTime lastKnownRemoteVersion}) {
-  return _kPrettyJsonEncoder.convert(<String, String>{
-    'lastTimeVersionWasChecked': '$lastTimeVersionWasChecked',
-    'lastKnownRemoteVersion': '$lastKnownRemoteVersion',
-  });
+  logger.clear();
 }
 
 void testFlutterVersion(String description, dynamic testMethod()) {
@@ -135,21 +237,24 @@ void testFlutterVersion(String description, dynamic testMethod()) {
     testMethod,
     overrides: <Type, Generator>{
       FlutterVersion: () => new FlutterVersion(_testClock),
-      ProcessManager: () => new MockProcessManager(),
-      Cache: () => new MockCache(),
     },
   );
 }
 
 void fakeData({
-  @required DateTime localCommitDate,
+  DateTime localCommitDate,
   DateTime remoteCommitDate,
-  String versionCheckStamp,
-  bool expectSetStamp: false,
+  VersionCheckStamp stamp,
+  String stampJson,
   bool errorOnFetch: false,
+  bool expectSetStamp: false,
+  bool expectServerPing: false,
 }) {
-  final MockProcessManager pm = context[ProcessManager];
-  final MockCache cache = context[Cache];
+  final MockProcessManager pm = new MockProcessManager();
+  context.setVariable(ProcessManager, pm);
+
+  final MockCache cache = new MockCache();
+  context.setVariable(Cache, cache);
 
   ProcessResult success(String standardOutput) {
     return new ProcessResult(1, 0, standardOutput, '');
@@ -160,27 +265,22 @@ void fakeData({
   }
 
   when(cache.getStampFor(any)).thenAnswer((Invocation invocation) {
-    expect(invocation.positionalArguments.single, FlutterVersion.kFlutterVersionCheckStampFile);
+    expect(invocation.positionalArguments.single, VersionCheckStamp.kFlutterVersionCheckStampFile);
 
-    if (versionCheckStamp == _stampMissing) {
-      return null;
-    }
+    if (stampJson != null)
+      return stampJson;
 
-    if (versionCheckStamp != null) {
-      return versionCheckStamp;
-    }
+    if (stamp != null)
+      return JSON.encode(stamp.toJson());
 
-    throw new StateError('Unexpected call to Cache.getStampFor(${invocation.positionalArguments}, ${invocation.namedArguments})');
+    return null;
   });
 
   when(cache.setStampFor(any, any)).thenAnswer((Invocation invocation) {
-    expect(invocation.positionalArguments.first, FlutterVersion.kFlutterVersionCheckStampFile);
+    expect(invocation.positionalArguments.first, VersionCheckStamp.kFlutterVersionCheckStampFile);
 
     if (expectSetStamp) {
-      expect(invocation.positionalArguments[1], _testStamp(
-        lastKnownRemoteVersion: remoteCommitDate,
-        lastTimeVersionWasChecked: _testClock.now(),
-      ));
+      stamp = VersionCheckStamp.fromJson(JSON.decode(invocation.positionalArguments[1]));
       return null;
     }
 
@@ -205,6 +305,8 @@ void fakeData({
     } else if (argsAre('git', 'remote', 'add', '__flutter_version_check__', 'https://github.com/flutter/flutter.git')) {
       return success('');
     } else if (argsAre('git', 'fetch', '__flutter_version_check__', 'master')) {
+      if (!expectServerPing)
+        fail('Did not expect server ping');
       return errorOnFetch ? failure(128) : success('');
     } else if (remoteCommitDate != null && argsAre('git', 'log', '__flutter_version_check__/master', '-n', '1', '--pretty=format:%ad', '--date=iso')) {
       return success(remoteCommitDate.toString());