Expressions: Fix expression load with legacy UID -100 (#65950)

* Fix expressions instance settings loading

* Introduce a new method to get name or uid

* Update public/app/features/plugins/datasource_srv.ts

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>

* Move getNameOrUid method outside the class

---------

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>

.betterer.results

@@ -3119,27 +3119,20 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],
       [0, 0, 0, "Do not use any type assertions.", "2"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "3"],
-      [0, 0, 0, "Do not use any type assertions.", "4"],
-      [0, 0, 0, "Do not use any type assertions.", "5"],
-      [0, 0, 0, "Do not use any type assertions.", "6"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "4"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "5"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "6"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "7"],
       [0, 0, 0, "Do not use any type assertions.", "8"],
-      [0, 0, 0, "Do not use any type assertions.", "9"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "10"],
-      [0, 0, 0, "Do not use any type assertions.", "11"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "9"],
+      [0, 0, 0, "Do not use any type assertions.", "10"],
+      [0, 0, 0, "Unexpected any. Specify a different type.", "11"],
       [0, 0, 0, "Do not use any type assertions.", "12"],
       [0, 0, 0, "Do not use any type assertions.", "13"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "14"],
+      [0, 0, 0, "Do not use any type assertions.", "14"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "15"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "16"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "17"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "18"],
-      [0, 0, 0, "Do not use any type assertions.", "19"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "20"],
-      [0, 0, 0, "Do not use any type assertions.", "21"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "22"],
-      [0, 0, 0, "Unexpected any. Specify a different type.", "23"],
-      [0, 0, 0, "Do not use any type assertions.", "24"]
+      [0, 0, 0, "Do not use any type assertions.", "17"]
     ],
     "public/app/features/plugins/importPanelPlugin.ts:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"]

packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts

@@ -307,6 +307,8 @@ describe('DataSourceWithBackend', () => {
       expect(isExpressionReference('Expression')).toBeTruthy(); // Name
       expect(isExpressionReference({ type: '__expr__' })).toBeTruthy();
       expect(isExpressionReference({ type: '-100' })).toBeTruthy();
+      expect(isExpressionReference(null)).toBeFalsy();
+      expect(isExpressionReference(undefined)).toBeFalsy();
     });
   });
 });

public/app/features/plugins/datasource_srv.ts

@@ -65,26 +65,17 @@ export class DatasourceSrv implements DataSourceService {
     ref: string | null | undefined | DataSourceRef,
     scopedVars?: ScopedVars
   ): DataSourceInstanceSettings | undefined {
-    const isstring = typeof ref === 'string';
-    let nameOrUid = isstring ? (ref as string) : ((ref as any)?.uid as string | undefined);
-
-    if (nameOrUid === 'default' || nameOrUid === null || nameOrUid === undefined) {
-      if (!isstring && ref) {
-        const type = (ref as any)?.type as string;
-        if (type === ExpressionDatasourceRef.type) {
-          return expressionDatasource.instanceSettings;
-        } else if (type) {
-          console.log('FIND Default instance for datasource type?', ref);
-        }
-      }
-      return this.settingsMapByUid[this.defaultName] ?? this.settingsMapByName[this.defaultName];
-    }
+    let nameOrUid = getNameOrUid(ref);
 
     // Expressions has a new UID as __expr__ See: https://github.com/grafana/grafana/pull/62510/
     // But we still have dashboards/panels with old expression UID (-100)
     // To support both UIDs until we migrate them all to new one, this check is necessary
     if (isExpressionReference(nameOrUid)) {
-      return expressionDatasource.instanceSettings;
+      return expressionInstanceSettings;
+    }
+
+    if (nameOrUid === 'default' || nameOrUid == null) {
+      return this.settingsMapByUid[this.defaultName] ?? this.settingsMapByName[this.defaultName];
     }
 
     // Complex logic to support template variable data source names
@@ -114,15 +105,19 @@ export class DatasourceSrv implements DataSourceService {
       };
     }
 
-    return this.settingsMapByUid[nameOrUid] ?? this.settingsMapByName[nameOrUid];
+    return this.settingsMapByUid[nameOrUid] ?? this.settingsMapByName[nameOrUid] ?? this.settingsMapById[nameOrUid];
   }
 
   get(ref?: string | DataSourceRef | null, scopedVars?: ScopedVars): Promise<DataSourceApi> {
-    let nameOrUid = typeof ref === 'string' ? (ref as string) : ((ref as any)?.uid as string | undefined);
+    let nameOrUid = getNameOrUid(ref);
     if (!nameOrUid) {
       return this.get(this.defaultName);
     }
 
+    if (isExpressionReference(ref)) {
+      return Promise.resolve(this.datasources[ExpressionDatasourceUID]);
+    }
+
     // Check if nameOrUid matches a uid and then get the name
     const byName = this.settingsMapByName[nameOrUid];
     if (byName) {
@@ -154,7 +149,7 @@ export class DatasourceSrv implements DataSourceService {
     }
 
     // find the metadata
-    const instanceSettings = this.settingsMapByUid[key] ?? this.settingsMapByName[key] ?? this.settingsMapById[key];
+    const instanceSettings = this.getInstanceSettings(key);
     if (!instanceSettings) {
       return Promise.reject({ message: `Datasource ${key} was not found` });
     }
@@ -349,6 +344,15 @@ export class DatasourceSrv implements DataSourceService {
   }
 }
 
+export function getNameOrUid(ref?: string | DataSourceRef | null): string | undefined {
+  if (isExpressionReference(ref)) {
+    return ExpressionDatasourceRef.uid;
+  }
+
+  const isString = typeof ref === 'string';
+  return isString ? (ref as string) : ((ref as any)?.uid as string | undefined);
+}
+
 export function variableInterpolation(value: any[]) {
   if (Array.isArray(value)) {
     return value[0];

public/app/features/plugins/tests/datasource_srv.test.ts

@@ -6,7 +6,7 @@ import {
   ScopedVars,
 } from '@grafana/data';
 import { ExpressionDatasourceRef } from '@grafana/runtime/src/utils/DataSourceWithBackend';
-import { DatasourceSrv } from 'app/features/plugins/datasource_srv';
+import { DatasourceSrv, getNameOrUid } from 'app/features/plugins/datasource_srv';
 
 // Datasource variable $datasource with current value 'BBB'
 const templateSrv: any = {
@@ -221,6 +221,29 @@ describe('datasource_srv', () => {
       });
     });
 
+    describe('when loading datasource', () => {
+      it('should load expressions', async () => {
+        let api = await dataSourceSrv.loadDatasource('-100'); // Legacy expression id
+        expect(api.uid).toBe(ExpressionDatasourceRef.uid);
+
+        api = await dataSourceSrv.loadDatasource('__expr__'); // Legacy expression id
+        expect(api.uid).toBe(ExpressionDatasourceRef.uid);
+
+        api = await dataSourceSrv.loadDatasource('Expression'); // Legacy expression id
+        expect(api.uid).toBe(ExpressionDatasourceRef.uid);
+      });
+
+      it('should load by variable', async () => {
+        const api = await dataSourceSrv.loadDatasource('${datasource}');
+        expect(api.meta).toBe(dataSourceInit.BBB.meta);
+      });
+
+      it('should load by name', async () => {
+        let api = await dataSourceSrv.loadDatasource('ZZZ');
+        expect(api.meta).toBe(dataSourceInit.ZZZ.meta);
+      });
+    });
+
     describe('when getting external metric sources', () => {
       it('should return list of explore sources', () => {
         const externalSources = dataSourceSrv.getExternal();
@@ -344,4 +367,34 @@ describe('datasource_srv', () => {
       expect(initMock).toHaveBeenCalledWith(dataSourceInit, 'aaa');
     });
   });
+
+  describe('getNameOrUid', () => {
+    it('should return expression uid __expr__', () => {
+      expect(getNameOrUid('__expr__')).toBe(ExpressionDatasourceRef.uid);
+      expect(getNameOrUid('-100')).toBe(ExpressionDatasourceRef.uid);
+      expect(getNameOrUid('Expression')).toBe(ExpressionDatasourceRef.uid);
+      expect(getNameOrUid({ type: '__expr__' })).toBe(ExpressionDatasourceRef.uid);
+      expect(getNameOrUid({ type: '-100' })).toBe(ExpressionDatasourceRef.uid);
+    });
+
+    it('should return ref if it is string', () => {
+      const value = 'mixed-datasource';
+      const nameOrUid = getNameOrUid(value);
+      expect(nameOrUid).not.toBeUndefined();
+      expect(nameOrUid).toBe(value);
+    });
+
+    it('should return the uid if the ref is not string', () => {
+      const value = { type: 'mixed', uid: 'theUID' };
+      const nameOrUid = getNameOrUid(value);
+      expect(nameOrUid).not.toBeUndefined();
+      expect(nameOrUid).toBe(value.uid);
+    });
+
+    it('should return undefined if the ref has no uid', () => {
+      const value = { type: 'mixed' };
+      const nameOrUid = getNameOrUid(value);
+      expect(nameOrUid).toBeUndefined();
+    });
+  });
 });