Dedupe unknown query params from feature flag query params. (#6824)

Since #6784 we have seen the unusual behavior where query params for
feature flags will be duplicated on the command line.

For example, if we load `localhost:6006/?showFlags=` it will be
rewritten as `localhost:6006/?showFlags=&showFlags=`.

This change fixes this by deduplicating "unknown" flags with known
feature flag query params.

Author: bmd3k

tensorboard/webapp/routes/dashboard_deeplink_provider.ts

@@ -15,7 +15,7 @@ limitations under the License.
 import {Injectable} from '@angular/core';
 import {Store} from '@ngrx/store';
 import {combineLatest, Observable} from 'rxjs';
-import {map} from 'rxjs/operators';
+import {combineLatestWith, map} from 'rxjs/operators';
 import {DeepLinkProvider} from '../app_routing/deep_link_provider';
 import {SerializableQueryParams} from '../app_routing/types';
 import {State} from '../app_state';
@@ -152,16 +152,18 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
           return [{key: RUN_FILTER_KEY, value}];
         })
       ),
-      store
-        .select(selectors.getUnknownQueryParams)
-        .pipe(
-          map((queryParams) =>
-            Object.entries(queryParams).map(([key, value]) => ({key, value}))
-          )
-        ),
     ]).pipe(
-      map((queryParamList) => {
-        return queryParamList.flat();
+      combineLatestWith(store.select(selectors.getUnknownQueryParams)),
+      map(([queryParamList, unknownQueryParams]) => {
+        // Filter out any known params from unknownQueryParams.
+        const knownQueryParamKeys = new Set(
+          queryParamList.flat().map((qp) => qp.key)
+        );
+        const filteredUnknownQueryParams = Object.entries(unknownQueryParams)
+          .filter(([key]) => !knownQueryParamKeys.has(key))
+          .map(([key, value]) => ({key, value}));
+
+        return [...queryParamList, ...filteredUnknownQueryParams].flat();
       })
     );
   }

tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts

@@ -400,6 +400,30 @@ describe('core deeplink provider', () => {
     ]);
   });
 
+  it('dedupes unknown query params with feature flags', () => {
+    store.overrideSelector(selectors.getOverriddenFeatureFlags, {
+      enabledExperimentalPlugins: ['foo', 'bar', 'baz'],
+      showFlags: '',
+    });
+    store.overrideSelector(selectors.getUnknownQueryParams, {
+      unknown1: 'unknown1_value',
+      unknown2: 'unknown2_value',
+      // Should be ignored since it is also a feature flag query param.
+      experimentalPlugin:
+        'this_is_known_but_has_different_value_for_some_reason',
+      // Should be ignored since it is also a feature flag query param.
+      showFlags: 'this_is_also_known_and_also_has_different_value',
+    });
+    store.refreshState();
+
+    expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
+      {key: 'experimentalPlugin', value: 'foo,bar,baz'},
+      {key: 'showFlags', value: ''},
+      {key: 'unknown1', value: 'unknown1_value'},
+      {key: 'unknown2', value: 'unknown2_value'},
+    ]);
+  });
+
   describe('runs', () => {
     describe('color group', () => {
       it('does not put state in the URL when user set color group is null', () => {