flutter_tools: ensure the context value passed to pub is consistent (#13366)

Adds a class `PubContext` with a fixed set of allowed values
Make it clear these values should not be changed casually

Fixed one inconsistency already: update_packages vs update_pkgs
Provide more information for verify calls
Eliminate `ctx_` prefix.

packages/flutter_tools/lib/src/commands/create.dart

@@ -162,7 +162,7 @@ class CreateCommand extends FlutterCommand {
       generatedCount += _renderTemplate('package', dirPath, templateContext);
 
       if (argResults['pub'])
-        await pubGet(context: 'create_pkg', directory: dirPath);
+        await pubGet(context: PubContext.createPackage, directory: dirPath);
 
       final String relativePath = fs.path.relative(dirPath);
       printStatus('Wrote $generatedCount files.');
@@ -180,7 +180,7 @@ class CreateCommand extends FlutterCommand {
       generatedCount += _renderTemplate('plugin', dirPath, templateContext);
 
       if (argResults['pub'])
-        await pubGet(context: 'create_plugin', directory: dirPath);
+        await pubGet(context: PubContext.createPlugin, directory: dirPath);
 
       if (android_sdk.androidSdk != null)
         gradle.updateLocalProperties(projectPath: dirPath);
@@ -218,7 +218,7 @@ class CreateCommand extends FlutterCommand {
     );
 
     if (argResults['pub']) {
-      await pubGet(context: 'create', directory: appPath);
+      await pubGet(context: PubContext.create, directory: appPath);
       injectPlugins(directory: appPath);
     }
 

packages/flutter_tools/lib/src/commands/packages.dart

@@ -69,7 +69,7 @@ class PackagesGetCommand extends FlutterCommand {
       );
     }
 
-    await pubGet(context: 'get',
+    await pubGet(context: PubContext.pubGet,
       directory: target,
       upgrade: upgrade,
       offline: argResults['offline'],
@@ -102,7 +102,7 @@ class PackagesTestCommand extends FlutterCommand {
   }
 
   @override
-  Future<Null> runCommand() => pub(<String>['run', 'test']..addAll(argResults.rest), context: 'run_test', retry: false);
+  Future<Null> runCommand() => pub(<String>['run', 'test']..addAll(argResults.rest), context:  PubContext.runTest, retry: false);
 }
 
 class PackagesPassthroughCommand extends FlutterCommand {

packages/flutter_tools/lib/src/commands/update_packages.dart

@@ -146,7 +146,7 @@ class UpdatePackagesCommand extends FlutterCommand {
         fakePackage.createSync();
         fakePackage.writeAsStringSync(_generateFakePubspec(dependencies.values));
         // First we run "pub upgrade" on this generated package:
-        await pubGet(context: 'update_packages', directory: temporaryDirectory.path, upgrade: true, checkLastModified: false);
+        await pubGet(context: PubContext.updatePackages, directory: temporaryDirectory.path, upgrade: true, checkLastModified: false);
         // Then we run "pub deps --style=compact" on the result. We pipe all the
         // output to tree.fill(), which parses it so that it can create a graph
         // of all the dependencies so that we can figure out the transitive
@@ -154,7 +154,7 @@ class UpdatePackagesCommand extends FlutterCommand {
         // each package.
         await pub(
           <String>['deps', '--style=compact'],
-          context: 'update_pkgs',
+          context: PubContext.updatePackages,
           directory: temporaryDirectory.path,
           filter: tree.fill,
           retry: false, // errors here are usually fatal since we're not hitting the network
@@ -211,7 +211,7 @@ class UpdatePackagesCommand extends FlutterCommand {
     int count = 0;
 
     for (Directory dir in packages) {
-      await pubGet(context: 'update_packages', directory: dir.path, checkLastModified: false);
+      await pubGet(context: PubContext.updatePackages, directory: dir.path, checkLastModified: false);
       count += 1;
     }
 

packages/flutter_tools/lib/src/commands/upgrade.dart

@@ -68,7 +68,7 @@ class UpgradeCommand extends FlutterCommand {
     final String projRoot = findProjectRoot();
     if (projRoot != null) {
       printStatus('');
-      await pubGet(context: 'upgrade', directory: projRoot, upgrade: true, checkLastModified: false);
+      await pubGet(context: PubContext.pubUpgrade, directory: projRoot, upgrade: true, checkLastModified: false);
     }
 
     // Run a doctor check in case system requirements have changed.

packages/flutter_tools/lib/src/dart/pub.dart

@@ -16,6 +16,43 @@ import '../cache.dart';
 import '../globals.dart';
 import 'sdk.dart';
 
+/// Represents Flutter-specific data that is added to the `PUB_ENVIRONMENT`
+/// environment variable and allows understanding the type of requests made to
+/// the package site on Flutter's behalf.
+// DO NOT update without contacting kevmoo.
+// We have server-side tooling that assumes the values are consistent.
+class PubContext {
+  static final RegExp _validContext = new RegExp('[a-z][a-z_]*[a-z]');
+
+  static final PubContext create = new PubContext._(<String>['create']);
+  static final PubContext createPackage = new PubContext._(<String>['create_pkg']);
+  static final PubContext createPlugin = new PubContext._(<String>['create_plugin']);
+  static final PubContext interactive = new PubContext._(<String>['interactive']);
+  static final PubContext pubGet = new PubContext._(<String>['get']);
+  static final PubContext pubUpgrade = new PubContext._(<String>['upgrade']);
+  static final PubContext runTest = new PubContext._(<String>['run_test']);
+
+  static final PubContext flutterTests = new PubContext._(<String>['flutter_tests']);
+  static final PubContext updatePackages = new PubContext._(<String>['update_packages']);
+
+  final List<String> _values;
+
+  PubContext._(this._values) {
+    for (String item in _values) {
+      if (!_validContext.hasMatch(item)) {
+        throw new ArgumentError.value(
+            _values, 'value', 'Must match RegExp ${_validContext.pattern}');
+      }
+    }
+  }
+
+  static PubContext getVerifyContext(String commandName) =>
+      new PubContext._(<String>['verify', commandName.replaceAll('-', '_')]);
+
+  @override
+  String toString() => 'PubContext: ${_values.join(':')}';
+}
+
 bool _shouldRunPubGet({ File pubSpecYaml, File dotPackages }) {
   if (!dotPackages.existsSync())
     return true;
@@ -30,9 +67,9 @@ bool _shouldRunPubGet({ File pubSpecYaml, File dotPackages }) {
 }
 
 /// [context] provides extra information to package server requests to
-/// understand usage. It must match the regular expression `[a-z][a-z_]*[a-z]`.
+/// understand usage.
 Future<Null> pubGet({
-  @required String context,
+  @required PubContext context,
   String directory,
   bool skipIfAbsent: false,
   bool upgrade: false,
@@ -90,9 +127,9 @@ typedef String MessageFilter(String message);
 /// `arguments` list) unless `showTraceForErrors` is false.
 ///
 /// [context] provides extra information to package server requests to
-/// understand usage. It must match the regular expression `[a-z][a-z_]*[a-z]`.
+/// understand usage.
 Future<Null> pub(List<String> arguments, {
-  @required String context,
+  @required PubContext context,
   String directory,
   MessageFilter filter,
   String failureMessage: 'pub failed',
@@ -133,7 +170,7 @@ Future<Null> pubInteractively(List<String> arguments, {
   final int code = await runInteractively(
     _pubCommand(arguments),
     workingDirectory: directory,
-    environment: _createPubEnvironment('interactive'),
+    environment: _createPubEnvironment(PubContext.interactive),
   );
   if (code != 0)
     throwToolExit('pub finished with exit code $code', exitCode: code);
@@ -147,8 +184,8 @@ List<String> _pubCommand(List<String> arguments) {
 /// The full environment used when running pub.
 ///
 /// [context] provides extra information to package server requests to
-/// understand usage. It must match the regular expression `[a-z][a-z_]*[a-z]`.
-Map<String, String> _createPubEnvironment(String context) {
+/// understand usage.
+Map<String, String> _createPubEnvironment(PubContext context) {
   final Map<String, String> environment = <String, String>{
     'FLUTTER_ROOT': Cache.flutterRoot,
     _pubEnvironmentKey: _getPubEnvironmentValue(context),
@@ -168,15 +205,15 @@ const String _pubEnvironmentKey = 'PUB_ENVIRONMENT';
 /// The console environment key used by the pub tool to find the cache directory.
 const String _pubCacheEnvironmentKey = 'PUB_CACHE';
 
-final RegExp _validContext = new RegExp('[a-z][a-z_]*[a-z]');
-
 /// Returns the environment value that should be used when running pub.
 ///
 /// Includes any existing environment variable, if one exists.
 ///
 /// [context] provides extra information to package server requests to
-/// understand usage. It must match the regular expression `[a-z][a-z_]*[a-z]`.
-String _getPubEnvironmentValue(String pubContext) {
+/// understand usage.
+String _getPubEnvironmentValue(PubContext pubContext) {
+  // DO NOT update this function without contacting kevmoo.
+  // We have server-side tooling that assumes the values are consistent.
   final List<String> values = <String>[];
 
   final String existing = platform.environment[_pubEnvironmentKey];
@@ -190,11 +227,7 @@ String _getPubEnvironmentValue(String pubContext) {
   }
 
   values.add('flutter_cli');
-
-  if (!_validContext.hasMatch(pubContext)) {
-    throw new ArgumentError.value(pubContext, 'pubContext', 'Must match RegExp ${_validContext.pattern}');
-  }
-  values.add('ctx_$pubContext');
+  values.addAll(pubContext._values);
 
   return values.join(':');
 }

packages/flutter_tools/lib/src/runner/flutter_command.dart

@@ -242,7 +242,7 @@ abstract class FlutterCommand extends Command<Null> {
       await cache.updateAll();
 
     if (shouldRunPub)
-      await pubGet(context: 'verify');
+      await pubGet(context: PubContext.getVerifyContext(name));
 
     setupApplicationPackages();
 

packages/flutter_tools/test/commands/analyze_continuously_test.dart

@@ -32,7 +32,7 @@ void main() {
     testUsingContext('AnalysisServer success', () async {
       _createSampleProject(tempDir);
 
-      await pubGet(context: 'flutter_tests', directory: tempDir.path);
+      await pubGet(context: PubContext.flutterTests, directory: tempDir.path);
 
       server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
 
@@ -52,7 +52,7 @@ void main() {
   testUsingContext('AnalysisServer errors', () async {
     _createSampleProject(tempDir, brokenCode: true);
 
-    await pubGet(context: 'flutter_tests', directory: tempDir.path);
+    await pubGet(context: PubContext.flutterTests, directory: tempDir.path);
 
     server = new AnalysisServer(dartSdkPath, <String>[tempDir.path]);
 

packages/flutter_tools/test/dart/pub_get_test.dart

@@ -26,7 +26,7 @@ void main() {
 
     new FakeAsync().run((FakeAsync time) {
       expect(processMock.lastPubEnvironmment, isNull);
-      pubGet(context: 'flutter_tests', checkLastModified: false).then((Null value) {
+      pubGet(context: PubContext.flutterTests, checkLastModified: false).then((Null value) {
         error = 'test completed unexpectedly';
       }, onError: (dynamic thrownError) {
         error = 'test failed unexpectedly: $thrownError';
@@ -37,7 +37,7 @@ void main() {
         'Running "flutter packages get" in /...\n'
         'pub get failed (69) -- attempting retry 1 in 1 second...\n'
       );
-      expect(processMock.lastPubEnvironmment, contains('flutter_cli:ctx_flutter_tests'));
+      expect(processMock.lastPubEnvironmment, contains('flutter_cli:flutter_tests'));
       expect(processMock.lastPubCache, isNull);
       time.elapse(const Duration(milliseconds: 500));
       expect(testLogger.statusText,
@@ -96,7 +96,7 @@ void main() {
       MockDirectory.findCache = true;
       expect(processMock.lastPubEnvironmment, isNull);
       expect(processMock.lastPubCache, isNull);
-      pubGet(context: 'flutter_tests', checkLastModified: false).then((Null value) {
+      pubGet(context: PubContext.flutterTests, checkLastModified: false).then((Null value) {
         error = 'test completed unexpectedly';
       }, onError: (dynamic thrownError) {
         error = 'test failed unexpectedly: $thrownError';
@@ -122,7 +122,7 @@ void main() {
       MockDirectory.findCache = false;
       expect(processMock.lastPubEnvironmment, isNull);
       expect(processMock.lastPubCache, isNull);
-      pubGet(context: 'flutter_tests', checkLastModified: false).then((Null value) {
+      pubGet(context: PubContext.flutterTests, checkLastModified: false).then((Null value) {
         error = 'test completed unexpectedly';
       }, onError: (dynamic thrownError) {
         error = 'test failed unexpectedly: $thrownError';