Cleanup tech debt for some of the DevTools server APIs

packages/devtools_app/lib/src/screens/app_size/app_size_screen.dart

@@ -104,13 +104,13 @@ class _AppSizeBodyState extends State<AppSizeBody>
 
   Future<void> maybeLoadAppSizeFiles() async {
     final queryParams = loadQueryParams();
-    final baseFilePath = queryParams[baseAppSizeFilePropertyName];
+    final baseFilePath = queryParams[AppSizeApi.baseAppSizeFilePropertyName];
     if (baseFilePath != null) {
       // TODO(kenz): does this have to be in a setState()?
       _preLoadingData = true;
       final baseAppSizeFile = await server.requestBaseAppSizeFile(baseFilePath);
       DevToolsJsonFile? testAppSizeFile;
-      final testFilePath = queryParams[testAppSizeFilePropertyName];
+      final testFilePath = queryParams[AppSizeApi.testAppSizeFilePropertyName];
       if (testFilePath != null) {
         testAppSizeFile = await server.requestTestAppSizeFile(testFilePath);
       }

packages/devtools_app/lib/src/shared/server/_app_size_api.dart

@@ -6,16 +6,16 @@ part of 'server.dart';
 
 Future<DevToolsJsonFile?> requestBaseAppSizeFile(String path) {
   return requestFile(
-    api: apiGetBaseAppSizeFile,
-    fileKey: baseAppSizeFilePropertyName,
+    api: AppSizeApi.getBaseAppSizeFile,
+    fileKey: AppSizeApi.baseAppSizeFilePropertyName,
     filePath: path,
   );
 }
 
 Future<DevToolsJsonFile?> requestTestAppSizeFile(String path) {
   return requestFile(
-    api: apiGetTestAppSizeFile,
-    fileKey: testAppSizeFilePropertyName,
+    api: AppSizeApi.getTestAppSizeFile,
+    fileKey: AppSizeApi.testAppSizeFilePropertyName,
     filePath: path,
   );
 }

packages/devtools_app/lib/src/shared/server/_release_notes_api.dart

@@ -10,11 +10,11 @@ part of 'server.dart';
 Future<String> getLastShownReleaseNotesVersion() async {
   String version = '';
   if (isDevToolsServerAvailable) {
-    final resp = await request(apiGetLastReleaseNotesVersion);
+    final resp = await request(ReleaseNotesApi.getLastReleaseNotesVersion);
     if (resp?.statusOk ?? false) {
       version = json.decode(resp!.body);
     } else {
-      logWarning(resp, apiGetLastReleaseNotesVersion);
+      logWarning(resp, ReleaseNotesApi.getLastReleaseNotesVersion);
     }
   }
   return version;
@@ -26,11 +26,11 @@ Future<String> getLastShownReleaseNotesVersion() async {
 Future<void> setLastShownReleaseNotesVersion(String version) async {
   if (isDevToolsServerAvailable) {
     final resp = await request(
-      '$apiSetLastReleaseNotesVersion'
-      '?$lastReleaseNotesVersionPropertyName=$version',
+      '${ReleaseNotesApi.setLastReleaseNotesVersion}'
+      '?$apiParameterValueKey=$version',
     );
-    if (resp == null || !resp.statusOk || !(json.decode(resp.body) as bool)) {
-      logWarning(resp, apiSetLastReleaseNotesVersion);
+    if (resp == null || !resp.statusOk) {
+      logWarning(resp, ReleaseNotesApi.setLastReleaseNotesVersion);
     }
   }
 }

packages/devtools_shared/CHANGELOG.md

@@ -1,3 +1,12 @@
+# 10.1.0
+* Deprecate `apiGetLastReleaseNotesVersion` in favor of `ReleaseNotesApi.getLastReleaseNotesVersion`.
+* Deprecate `apiSetLastReleaseNotesVersion` in favor of `ReleaseNotesApi.setLastReleaseNotesVersion`.
+* Deprecate `lastReleaseNotesVersionPropertyName`.
+* Deprecate `apiGetBaseAppSizeFile` in favor of `AppSizeApi.getBaseAppSizeFile`.
+* Deprecate `apiGetTestAppSizeFile` in favor of `AppSizeApi.getTestAppSizeFile`.
+* Deprecate `baseAppSizeFilePropertyName` in favor of `AppSizeApi.baseAppSizeFilePropertyName`.
+* Deprecate `testAppSizeFilePropertyName` in favor of `AppSizeApi.testAppSizeFilePropertyName`.
+
 # 10.0.2
 * Update dependency `web_socket_channel: '>=2.4.0 <4.0.0'`.
 

packages/devtools_shared/lib/src/devtools_api.dart

@@ -52,27 +52,87 @@ const apiGetSurveyShownCount = '${apiPrefix}getSurveyShownCount';
 /// Increments the surveyShownCount of the of the activeSurvey (apiSetActiveSurvey).
 const apiIncrementSurveyShownCount = '${apiPrefix}incrementSurveyShownCount';
 
-const lastReleaseNotesVersionPropertyName = 'lastReleaseNotesVersion';
-
-/// Returns the last DevTools version for which we have shown release notes.
-const apiGetLastReleaseNotesVersion = '${apiPrefix}getLastReleaseNotesVersion';
-
-/// Sets the last DevTools version for which we have shown release notes.
-const apiSetLastReleaseNotesVersion = '${apiPrefix}setLastReleaseNotesVersion';
-
-/// Returns the base app size file, if present.
-const apiGetBaseAppSizeFile = '${apiPrefix}getBaseAppSizeFile';
-
-/// Returns the test app size file used for comparing two files in a diff, if
-/// present.
-const apiGetTestAppSizeFile = '${apiPrefix}getTestAppSizeFile';
-
-const baseAppSizeFilePropertyName = 'appSizeBase';
+@Deprecated(
+  'Use apiParameterValueKey for the query parameter of the '
+  'ReleaseNotesApi.setLastReleaseNotesVersion request instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const lastReleaseNotesVersionPropertyName = apiParameterValueKey;
+
+@Deprecated(
+  'Use ReleaseNotesApi.getLastReleaseNotesVersion instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiGetLastReleaseNotesVersion =
+    ReleaseNotesApi.getLastReleaseNotesVersion;
+
+@Deprecated(
+  'Use ReleaseNotesApi.setLastReleaseNotesVersion instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiSetLastReleaseNotesVersion =
+    ReleaseNotesApi.setLastReleaseNotesVersion;
+
+abstract class ReleaseNotesApi {
+  /// Returns the last DevTools version for which we have shown release notes.
+  static const getLastReleaseNotesVersion =
+      '${apiPrefix}getLastReleaseNotesVersion';
+
+  /// Sets the last DevTools version for which we have shown release notes.
+  static const setLastReleaseNotesVersion =
+      '${apiPrefix}setLastReleaseNotesVersion';
+}
 
-const testAppSizeFilePropertyName = 'appSizeTest';
+@Deprecated(
+  'Use AppSizeApi.getBaseAppSizeFile instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiGetBaseAppSizeFile = AppSizeApi.getBaseAppSizeFile;
+
+@Deprecated(
+  'Use AppSizeApi.getTestAppSizeFile instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const apiGetTestAppSizeFile = AppSizeApi.getTestAppSizeFile;
+
+@Deprecated(
+  'Use AppSizeApi.baseAppSizeFilePropertyName instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const baseAppSizeFilePropertyName = AppSizeApi.baseAppSizeFilePropertyName;
+
+@Deprecated(
+  'Use AppSizeApi.testAppSizeFilePropertyName instead. '
+  'This field will be removed in devtools_shared >= 11.0.0.',
+)
+const testAppSizeFilePropertyName = AppSizeApi.testAppSizeFilePropertyName;
+
+abstract class AppSizeApi {
+  /// Returns the base app size file, if present.
+  static const getBaseAppSizeFile = '${apiPrefix}getBaseAppSizeFile';
+
+  /// Returns the test app size file used for comparing two files in a diff, if
+  /// present.
+  static const getTestAppSizeFile = '${apiPrefix}getTestAppSizeFile';
+
+  /// The property name for the query parameter passed along with the
+  /// [getBaseAppSizeFile] request.
+  static const baseAppSizeFilePropertyName = 'appSizeBase';
+
+  /// The property name for the query parameter passed along with the
+  /// [getTestAppSizeFile] request.
+  static const testAppSizeFilePropertyName = 'appSizeTest';
+}
 
 abstract class DtdApi {
+  /// Gets the DTD URI from the DevTools server.
+  ///
+  /// DTD is either started from the user's IDE and passed to the DevTools
+  /// server, or it is started directly from the DevTools server.
   static const apiGetDtdUri = '${apiPrefix}getDtdUri';
+
+  /// The property name for the response that the server sends back upon
+  /// receiving an [apiGetDtdUri] request.
   static const uriPropertyName = 'dtdUri';
 }
 

packages/devtools_shared/lib/src/server/devtools_store.dart

@@ -4,6 +4,28 @@
 
 import 'file_system.dart';
 
+enum DevToolsStoreKeys {
+  /// The key holding the value for whether Google Analytics (legacy) for
+  /// DevTools have been enabled.
+  analyticsEnabled,
+
+  /// The key holding the value for whether this is a user's first run of
+  /// DevTools.
+  isFirstRun,
+
+  /// The key holding the value for the last DevTools version that the user
+  /// viewed release notes for.
+  lastReleaseNotesVersion,
+
+  /// The key holding the value for whether the user has taken action on the
+  /// DevTools survey prompt.
+  surveyActionTaken,
+
+  /// The key holding the value for number of times the user has seen the
+  /// DevTools survey prompt without taking action.
+  surveyShownCount,
+}
+
 /// Provides access to the local DevTools store (~/.flutter-devtools/.devtools).
 class DevToolsUsage {
   DevToolsUsage() {
@@ -33,47 +55,23 @@ class DevToolsUsage {
 
   late IOPersistentProperties properties;
 
-  static const _surveyActionTaken = 'surveyActionTaken';
-  static const _surveyShownCount = 'surveyShownCount';
-
   void reset() {
-    // TODO(kenz): remove this in Feb 2022. See
-    // https://github.com/flutter/devtools/issues/3264. The `firstRun` property
-    // has been replaced by `isFirstRun`. This is to force all users to answer
-    // the analytics dialog again. The 'enabled' property has been replaced by
-    // 'analyticsEnabled' for better naming.
-    properties.remove('firstRun');
-    properties.remove('enabled');
-
-    properties.remove('firstDevToolsRun');
-    properties['analyticsEnabled'] = false;
+    properties.remove(DevToolsStoreKeys.isFirstRun.name);
+    properties[DevToolsStoreKeys.analyticsEnabled.name] = false;
   }
 
   bool get isFirstRun {
-    // TODO(kenz): remove this in Feb 2022. See
-    // https://github.com/flutter/devtools/issues/3264.The `firstRun` property
-    // has been replaced by `isFirstRun`. This is to force all users to answer
-    // the analytics dialog again.
-    properties.remove('firstRun');
-
-    return properties['isFirstRun'] = properties['isFirstRun'] == null;
+    return properties[DevToolsStoreKeys.isFirstRun.name] =
+        properties[DevToolsStoreKeys.isFirstRun.name] == null;
   }
 
   bool get analyticsEnabled {
-    // TODO(kenz): remove this in Feb 2022. See
-    // https://github.com/flutter/devtools/issues/3264. The `enabled` property
-    // has been replaced by `analyticsEnabled` for better naming.
-    if (properties['enabled'] != null) {
-      properties['analyticsEnabled'] = properties['enabled'];
-      properties.remove('enabled');
-    }
-
-    return properties['analyticsEnabled'] =
-        properties['analyticsEnabled'] == true;
+    return properties[DevToolsStoreKeys.analyticsEnabled.name] =
+        properties[DevToolsStoreKeys.analyticsEnabled.name] == true;
   }
 
   set analyticsEnabled(bool value) {
-    properties['analyticsEnabled'] = value;
+    properties[DevToolsStoreKeys.analyticsEnabled.name] = value;
   }
 
   bool surveyNameExists(String surveyName) => properties[surveyName] != null;
@@ -100,8 +98,8 @@ class DevToolsUsage {
   void rewriteActiveSurvey(bool actionTaken, int shownCount) {
     assert(activeSurvey != null);
     properties[activeSurvey!] = {
-      _surveyActionTaken: actionTaken,
-      _surveyShownCount: shownCount,
+      DevToolsStoreKeys.surveyActionTaken.name: actionTaken,
+      DevToolsStoreKeys.surveyShownCount.name: shownCount,
     };
   }
 
@@ -141,15 +139,18 @@ class DevToolsUsage {
   }
 
   String get lastReleaseNotesVersion {
-    return (properties['lastReleaseNotesVersion'] ??= '') as String;
+    return (properties[DevToolsStoreKeys.lastReleaseNotesVersion.name] ??= '')
+        as String;
   }
 
   set lastReleaseNotesVersion(String value) {
-    properties['lastReleaseNotesVersion'] = value;
+    properties[DevToolsStoreKeys.lastReleaseNotesVersion.name] = value;
   }
 }
 
 extension type _ActiveSurveyJson(Map<String, Object?> json) {
-  bool get surveyActionTaken => json[DevToolsUsage._surveyActionTaken] as bool;
-  int? get surveyShownCount => json[DevToolsUsage._surveyShownCount] as int?;
+  bool get surveyActionTaken =>
+      json[DevToolsStoreKeys.surveyActionTaken.name] as bool;
+  int? get surveyShownCount =>
+      json[DevToolsStoreKeys.surveyShownCount.name] as int?;
 }

packages/devtools_shared/lib/src/server/handlers/_app_size.dart

@@ -0,0 +1,49 @@
+// Copyright 2024 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// ignore_for_file: avoid_classes_with_only_static_members
+
+part of '../server_api.dart';
+
+abstract class _AppSizeHandler {
+  static shelf.Response getBaseAppSizeFile(
+    ServerApi api,
+    Map<String, String> queryParams,
+  ) {
+    final missingRequiredParams = ServerApi._checkRequiredParameters(
+      [AppSizeApi.baseAppSizeFilePropertyName],
+      queryParams: queryParams,
+      api: api,
+      requestName: AppSizeApi.getBaseAppSizeFile,
+    );
+    if (missingRequiredParams != null) return missingRequiredParams;
+
+    final filePath = queryParams[AppSizeApi.baseAppSizeFilePropertyName]!;
+    final fileJson = LocalFileSystem.devToolsFileAsJson(filePath);
+    if (fileJson == null) {
+      return api.badRequest('No JSON file available at $filePath.');
+    }
+    return api.success(fileJson);
+  }
+
+  static shelf.Response getTestAppSizeFile(
+    ServerApi api,
+    Map<String, String> queryParams,
+  ) {
+    final missingRequiredParams = ServerApi._checkRequiredParameters(
+      [AppSizeApi.testAppSizeFilePropertyName],
+      queryParams: queryParams,
+      api: api,
+      requestName: AppSizeApi.getTestAppSizeFile,
+    );
+    if (missingRequiredParams != null) return missingRequiredParams;
+
+    final filePath = queryParams[AppSizeApi.testAppSizeFilePropertyName]!;
+    final fileJson = LocalFileSystem.devToolsFileAsJson(filePath);
+    if (fileJson == null) {
+      return api.badRequest('No JSON file available at $filePath.');
+    }
+    return api.success(fileJson);
+  }
+}

packages/devtools_shared/lib/src/server/handlers/_release_notes.dart

@@ -0,0 +1,42 @@
+// Copyright 2024 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// ignore_for_file: avoid_classes_with_only_static_members
+
+part of '../server_api.dart';
+
+abstract class _ReleaseNotesHandler {
+  static shelf.Response getLastReleaseNotesVersion(
+    ServerApi api,
+    DevToolsUsage devToolsStore,
+  ) {
+    return _StorageHandler.handleGetStorageValue<String>(
+      api,
+      devToolsStore,
+      key: DevToolsStoreKeys.lastReleaseNotesVersion.name,
+      defaultValue: '',
+    );
+  }
+
+  static shelf.Response setLastReleaseNotesVersion(
+    ServerApi api,
+    Map<String, String> queryParams,
+    DevToolsUsage devToolsStore,
+  ) {
+    final missingRequiredParams = ServerApi._checkRequiredParameters(
+      [apiParameterValueKey],
+      queryParams: queryParams,
+      api: api,
+      requestName: ReleaseNotesApi.setLastReleaseNotesVersion,
+    );
+    if (missingRequiredParams != null) return missingRequiredParams;
+
+    return _StorageHandler.handleSetStorageValue<String>(
+      api,
+      devToolsStore,
+      key: DevToolsStoreKeys.lastReleaseNotesVersion.name,
+      value: queryParams[apiParameterValueKey]!,
+    );
+  }
+}