Stop removing experiment related query parameters

tensorboard/webapp/feature_flag/store/BUILD

@@ -10,6 +10,7 @@ tf_ng_module(
         "feature_flag_store_config_provider.ts",
     ],
     deps = [
+        ":feature_flag_metadata",
         ":types",
         "//tensorboard/webapp/feature_flag:types",
         "//tensorboard/webapp/feature_flag/actions",
@@ -21,6 +22,17 @@ tf_ng_module(
     ],
 )
 
+tf_ng_module(
+    name = "feature_flag_metadata",
+    srcs = [
+        "feature_flag_metadata.ts",
+    ],
+    deps = [
+        "//tensorboard/webapp/feature_flag:types",
+        "@npm//@angular/core",
+    ],
+)
+
 tf_ng_module(
     name = "types",
     srcs = [
@@ -45,10 +57,12 @@ tf_ng_module(
     name = "store_test_lib",
     testonly = True,
     srcs = [
+        "feature_flag_metadata_test.ts",
         "feature_flag_reducers_test.ts",
         "feature_flag_selectors_test.ts",
     ],
     deps = [
+        ":feature_flag_metadata",
         ":store",
         ":testing",
         ":types",

tensorboard/webapp/feature_flag/store/feature_flag_metadata.ts

@@ -0,0 +1,127 @@
+/* Copyright 2022 The TensorFlow Authors. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+==============================================================================*/
+import {FeatureFlags} from '../types';
+
+export type BaseFeatureFlagType = boolean | number | string | null | undefined;
+
+export type FeatureFlagType = BaseFeatureFlagType | Array<BaseFeatureFlagType>;
+
+export type FeatureFlagMetadata<T> = {
+  defaultValue: T;
+  queryParamOverride?: string;
+  parseValue: (str: string) => T extends (infer U)[] ? U : T; // The type, or, if the type is an array, the type of the array contents
+  isArray?: boolean;
+};
+
+export type FeatureFlagMetadataMapType<T extends FeatureFlags> = {
+  [FlagName in keyof T]: FeatureFlagMetadata<T[FlagName]>;
+};
+
+export function parseBoolean(str: string): boolean {
+  return str !== 'false';
+}
+
+export function parseBooleanOrNull(str: string): boolean | null {
+  if (str === 'null') {
+    return null;
+  }
+  return parseBoolean(str);
+}
+
+export const FeatureFlagMetadataMap: FeatureFlagMetadataMapType<FeatureFlags> =
+  {
+    scalarsBatchSize: {
+      defaultValue: undefined,
+      queryParamOverride: 'scalarsBatchSize',
+      parseValue: parseInt,
+    },
+    enabledColorGroup: {
+      defaultValue: true,
+      queryParamOverride: 'enableColorGroup',
+      parseValue: parseBoolean,
+    },
+    enabledColorGroupByRegex: {
+      defaultValue: true,
+      queryParamOverride: 'enableColorGroupByRegex',
+      parseValue: parseBoolean,
+    },
+    enabledExperimentalPlugins: {
+      defaultValue: [],
+      queryParamOverride: 'experimentalPlugin',
+      parseValue: (str: string) => str,
+      isArray: true,
+    },
+    enabledLinkedTime: {
+      defaultValue: false,
+      queryParamOverride: 'enableLinkedTime',
+      parseValue: parseBoolean,
+    },
+    enabledCardWidthSetting: {
+      defaultValue: true,
+      queryParamOverride: 'enableCardWidthSetting',
+      parseValue: parseBoolean,
+    },
+    enabledScalarDataTable: {
+      defaultValue: false,
+      queryParamOverride: 'enableDataTable',
+      parseValue: parseBoolean,
+    },
+    forceSvg: {
+      defaultValue: false,
+      queryParamOverride: 'forceSVG',
+      parseValue: parseBoolean,
+    },
+    enableDarkModeOverride: {
+      defaultValue: null,
+      parseValue: parseBooleanOrNull,
+    },
+    defaultEnableDarkMode: {
+      defaultValue: false,
+      queryParamOverride: 'darkMode',
+      parseValue: parseBoolean,
+    },
+    isAutoDarkModeAllowed: {
+      defaultValue: true,
+      parseValue: parseBoolean,
+    },
+    inColab: {
+      defaultValue: false,
+      queryParamOverride: 'tensorboardColab',
+      parseValue: parseBoolean,
+    },
+    metricsImageSupportEnabled: {
+      defaultValue: true,
+      parseValue: parseBoolean,
+    },
+    enableTimeSeriesPromotion: {
+      defaultValue: false,
+      parseValue: parseBoolean,
+    },
+  };
+
+/**
+ * Gets gets just the default values of each feature flag from the provided metadata.
+ */
+export function generateFeatureFlagDefaults<T extends FeatureFlags>(
+  featureFlagMetadataMap: FeatureFlagMetadataMapType<T>
+): T {
+  return Object.entries(featureFlagMetadataMap).reduce(
+    (map, [key, {defaultValue}]) => {
+      map[key] = defaultValue;
+      return map;
+    },
+    {} as Record<string, any>
+  ) as T;
+}

tensorboard/webapp/feature_flag/store/feature_flag_metadata_test.ts

@@ -0,0 +1,45 @@
+import {parseBoolean, parseBooleanOrNull} from './feature_flag_metadata';
+
+/* Copyright 2022 The TensorFlow Authors. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+==============================================================================*/
+describe('feature flag query parameters', () => {
+  describe('parseBoolean', () => {
+    it('"false" should evaluate to false', () => {
+      expect(parseBoolean('false')).toBeFalse();
+    });
+
+    it('values other than "false" should evaluate to true', () => {
+      expect(parseBoolean('true')).toBeTrue();
+      expect(parseBoolean('foo bar')).toBeTrue();
+      expect(parseBoolean('')).toBeTrue();
+    });
+  });
+
+  describe('parseBooleanOrNull', () => {
+    it('"null" should return null', () => {
+      expect(parseBooleanOrNull('null')).toBeNull();
+    });
+
+    it('"false" should evaluate to false', () => {
+      expect(parseBooleanOrNull('false')).toBeFalse();
+    });
+
+    it('values other than "false" should evaluate to true', () => {
+      expect(parseBooleanOrNull('true')).toBeTrue();
+      expect(parseBooleanOrNull('foo bar')).toBeTrue();
+      expect(parseBooleanOrNull('')).toBeTrue();
+    });
+  });
+});

tensorboard/webapp/feature_flag/store/feature_flag_store_config_provider.ts

@@ -14,26 +14,15 @@ limitations under the License.
 ==============================================================================*/
 import {InjectionToken} from '@angular/core';
 import {StoreConfig} from '@ngrx/store';
+import {
+  FeatureFlagMetadataMap,
+  generateFeatureFlagDefaults,
+} from './feature_flag_metadata';
 import {FeatureFlagState} from './feature_flag_types';
 
 export const initialState: FeatureFlagState = {
   isFeatureFlagsLoaded: false,
-  defaultFlags: {
-    isAutoDarkModeAllowed: true,
-    defaultEnableDarkMode: false,
-    enableDarkModeOverride: null,
-    enabledColorGroup: true,
-    enabledColorGroupByRegex: true,
-    enabledExperimentalPlugins: [],
-    inColab: false,
-    scalarsBatchSize: undefined,
-    metricsImageSupportEnabled: true,
-    enabledLinkedTime: false,
-    enableTimeSeriesPromotion: false,
-    enabledCardWidthSetting: true,
-    forceSvg: false,
-    enabledScalarDataTable: false,
-  },
+  defaultFlags: generateFeatureFlagDefaults(FeatureFlagMetadataMap),
   flagOverrides: {},
 };
 

tensorboard/webapp/routes/BUILD

@@ -37,11 +37,15 @@ tf_ts_library(
     ],
     deps = [
         ":dashboard_deeplink_provider_types",
+        ":feature_flag_serializer",
         "//tensorboard/webapp:app_state",
         "//tensorboard/webapp:selectors",
         "//tensorboard/webapp/app_routing:deep_link_provider",
         "//tensorboard/webapp/app_routing:route_config",
         "//tensorboard/webapp/app_routing:types",
+        "//tensorboard/webapp/feature_flag:types",
+        "//tensorboard/webapp/feature_flag/store",
+        "//tensorboard/webapp/feature_flag/store:feature_flag_metadata",
         "//tensorboard/webapp/metrics:types",
         "//tensorboard/webapp/metrics/data_source:types",
         "//tensorboard/webapp/runs:types",
@@ -53,6 +57,18 @@ tf_ts_library(
     ],
 )
 
+tf_ts_library(
+    name = "feature_flag_serializer",
+    srcs = [
+        "feature_flag_serializer.ts",
+    ],
+    deps = [
+        "//tensorboard/webapp/app_routing:types",
+        "//tensorboard/webapp/feature_flag:types",
+        "//tensorboard/webapp/feature_flag/store:feature_flag_metadata",
+    ],
+)
+
 tf_ts_library(
     name = "testing",
     testonly = True,
@@ -69,16 +85,20 @@ tf_ts_library(
     testonly = True,
     srcs = [
         "dashboard_deeplink_provider_test.ts",
+        "feature_flag_serializer_test.ts",
     ],
     deps = [
         ":dashboard_deeplink_provider",
+        ":feature_flag_serializer",
         ":testing",
         "//tensorboard/webapp:app_state",
         "//tensorboard/webapp:selectors",
         "//tensorboard/webapp/angular:expect_angular_core_testing",
         "//tensorboard/webapp/angular:expect_ngrx_store_testing",
         "//tensorboard/webapp/app_routing:deep_link_provider",
+        "//tensorboard/webapp/app_routing:location",
         "//tensorboard/webapp/app_routing:types",
+        "//tensorboard/webapp/feature_flag/store:feature_flag_metadata",
         "//tensorboard/webapp/metrics:test_lib",
         "//tensorboard/webapp/metrics:types",
         "//tensorboard/webapp/metrics/data_source:types",

tensorboard/webapp/routes/dashboard_deeplink_provider.ts

@@ -19,6 +19,12 @@ import {map} from 'rxjs/operators';
 import {DeepLinkProvider} from '../app_routing/deep_link_provider';
 import {SerializableQueryParams} from '../app_routing/types';
 import {State} from '../app_state';
+import {
+  FeatureFlagMetadata,
+  FeatureFlagMetadataMap,
+  FeatureFlagType,
+} from '../feature_flag/store/feature_flag_metadata';
+import {getOverriddenFeatureFlags} from '../feature_flag/store/feature_flag_selectors';
 import {
   isPluginType,
   isSampledPlugin,
@@ -27,11 +33,6 @@ import {
 import {CardUniqueInfo} from '../metrics/types';
 import {GroupBy, GroupByKey} from '../runs/types';
 import * as selectors from '../selectors';
-import {
-  ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY,
-  ENABLE_COLOR_GROUP_QUERY_PARAM_KEY,
-  EXPERIMENTAL_PLUGIN_QUERY_PARAM_KEY,
-} from '../webapp_data_source/tb_feature_flag_data_source_types';
 import {
   DeserializedState,
   PINNED_CARDS_KEY,
@@ -40,6 +41,7 @@ import {
   SMOOTHING_KEY,
   TAG_FILTER_KEY,
 } from './dashboard_deeplink_provider_types';
+import {featureFlagsToSerializableQueryParams} from './feature_flag_serializer';
 
 const COLOR_GROUP_REGEX_VALUE_PREFIX = 'regex:';
 
@@ -83,36 +85,6 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
     );
   }
 
-  private getFeatureFlagStates(
-    store: Store<State>
-  ): Observable<SerializableQueryParams> {
-    return combineLatest([
-      store.select(selectors.getEnabledExperimentalPlugins),
-      store.select(selectors.getOverriddenFeatureFlags),
-    ]).pipe(
-      map(([experimentalPlugins, overriddenFeatureFlags]) => {
-        const queryParams = experimentalPlugins.map((pluginId) => {
-          return {key: EXPERIMENTAL_PLUGIN_QUERY_PARAM_KEY, value: pluginId};
-        });
-        if (typeof overriddenFeatureFlags.enabledColorGroup === 'boolean') {
-          queryParams.push({
-            key: ENABLE_COLOR_GROUP_QUERY_PARAM_KEY,
-            value: String(overriddenFeatureFlags.enabledColorGroup),
-          });
-        }
-        if (
-          typeof overriddenFeatureFlags.enabledColorGroupByRegex === 'boolean'
-        ) {
-          queryParams.push({
-            key: ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY,
-            value: String(overriddenFeatureFlags.enabledColorGroupByRegex),
-          });
-        }
-        return queryParams;
-      })
-    );
-  }
-
   serializeStateToQueryParams(
     store: Store<State>
   ): Observable<SerializableQueryParams> {
@@ -126,7 +98,17 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
           return [{key: TAG_FILTER_KEY, value: filterText}];
         })
       ),
-      this.getFeatureFlagStates(store),
+      store.select(getOverriddenFeatureFlags).pipe(
+        map((featureFlags) => {
+          return featureFlagsToSerializableQueryParams(
+            featureFlags,
+            FeatureFlagMetadataMap as Record<
+              string,
+              FeatureFlagMetadata<FeatureFlagType>
+            >
+          );
+        })
+      ),
       store.select(selectors.getMetricsSettingOverrides).pipe(
         map((settingOverrides) => {
           if (Number.isFinite(settingOverrides.scalarSmoothing)) {