fix(functions): normalizeFunction expects a Promise (#8468)

72af6cc introduced a bug when refactoring that calling `normalizeFunction` always returned a Promise.

When it was removed, some calls to `normalizeFunction` may return plain objects that aren't wrapped in a Promise.

Fix: wrap the return value with a `$q` to satisfy `IPromise`

app/scripts/modules/core/src/function/function.read.service.spec.ts

@@ -0,0 +1,205 @@
+import { API, FunctionReader, IFunctionSourceData } from 'core';
+import { mock } from 'angular';
+import { IFunctionTransformer } from 'core/function/function.transformer';
+
+describe('FunctionReadService', () => {
+  let $http: ng.IHttpBackendService;
+  const functionTransformerMock: IFunctionTransformer = {
+    normalizeFunction: (data): IFunctionSourceData => {
+      return data;
+    },
+    normalizeFunctionSet: (data): IFunctionSourceData[] => {
+      return data;
+    },
+  };
+
+  beforeEach(
+    mock.inject(function($httpBackend: ng.IHttpBackendService) {
+      $http = $httpBackend;
+    }),
+  );
+
+  describe('loadFunctions', () => {
+    it(`should set cloudprovider if not set`, done => {
+      const functionReader = new FunctionReader(functionTransformerMock);
+
+      $http.expectGET(`${API.baseUrl}/applications/app1/functions`).respond(200, [
+        {
+          name: 'account1',
+          provider: 'aws',
+          type: 'aws',
+        },
+        {
+          name: 'account1',
+          provider: 'aws',
+          type: 'aws',
+        },
+      ]);
+
+      functionReader.loadFunctions('app1').then(data => {
+        expect(data).toEqual([
+          {
+            name: 'account1',
+            cloudProvider: 'aws',
+            provider: 'aws',
+            type: 'aws',
+          },
+          {
+            name: 'account1',
+            cloudProvider: 'aws',
+            provider: 'aws',
+            type: 'aws',
+          },
+        ]);
+
+        done();
+      });
+
+      $http.flush();
+    });
+
+    it(`should not set cloudprovider if provided`, done => {
+      const functionReader = new FunctionReader(functionTransformerMock);
+
+      $http.expectGET(`${API.baseUrl}/applications/app1/functions`).respond(200, [
+        {
+          name: 'account1',
+          cloudProvider: 'fluffyCloud',
+          provider: 'aws',
+          type: 'aws',
+        },
+        {
+          name: 'account1',
+          cloudProvider: 'fluffyCloud',
+          provider: 'aws',
+          type: 'aws',
+        },
+      ]);
+
+      functionReader.loadFunctions('app1').then(data => {
+        expect(data).toEqual([
+          {
+            name: 'account1',
+            cloudProvider: 'fluffyCloud',
+            provider: 'aws',
+            type: 'aws',
+          },
+          {
+            name: 'account1',
+            cloudProvider: 'fluffyCloud',
+            provider: 'aws',
+            type: 'aws',
+          },
+        ]);
+
+        done();
+      });
+
+      $http.flush();
+    });
+  });
+
+  describe('getFunctionDetails', () => {
+    it(`should set cloudprovider if not set`, done => {
+      const functionReader = new FunctionReader(functionTransformerMock);
+
+      $http.whenGET(/.*\/functions\?.*/).respond((_method, _url, _data, _headers, params) => {
+        expect(params).toEqual({
+          provider: 'aws',
+          account: 'acct1',
+          region: 'us-west-1',
+          functionName: 'runner1',
+        });
+
+        return [
+          200,
+          [
+            {
+              name: 'account1',
+              provider: 'aws',
+              type: 'aws',
+            },
+            {
+              name: 'account1',
+              provider: 'aws',
+              type: 'aws',
+            },
+          ],
+        ];
+      });
+
+      functionReader.getFunctionDetails('aws', 'acct1', 'us-west-1', 'runner1').then(data => {
+        expect(data).toEqual([
+          {
+            name: 'account1',
+            cloudProvider: 'aws',
+            provider: 'aws',
+            type: 'aws',
+          },
+          {
+            name: 'account1',
+            cloudProvider: 'aws',
+            provider: 'aws',
+            type: 'aws',
+          },
+        ]);
+
+        done();
+      });
+
+      $http.flush();
+    });
+
+    it(`should not set cloudprovider if provided`, done => {
+      const functionReader = new FunctionReader(functionTransformerMock);
+
+      $http.whenGET(/.*\/functions\?.*/).respond((_method, _url, _data, _headers, params) => {
+        expect(params).toEqual({
+          provider: 'aws',
+          account: 'acct1',
+          region: 'us-west-1',
+          functionName: 'runner1',
+        });
+
+        return [
+          200,
+          [
+            {
+              name: 'account1',
+              cloudProvider: 'fluffyCloud',
+              provider: 'aws',
+              type: 'aws',
+            },
+            {
+              name: 'account1',
+              cloudProvider: 'fluffyCloud',
+              provider: 'aws',
+              type: 'aws',
+            },
+          ],
+        ];
+      });
+
+      functionReader.getFunctionDetails('aws', 'acct1', 'us-west-1', 'runner1').then(data => {
+        expect(data).toEqual([
+          {
+            name: 'account1',
+            cloudProvider: 'fluffyCloud',
+            provider: 'aws',
+            type: 'aws',
+          },
+          {
+            name: 'account1',
+            cloudProvider: 'fluffyCloud',
+            provider: 'aws',
+            type: 'aws',
+          },
+        ]);
+
+        done();
+      });
+
+      $http.flush();
+    });
+  });
+});

app/scripts/modules/core/src/function/function.read.service.ts

@@ -1,8 +1,8 @@
-import { IPromise, IQService, module } from 'angular';
+import { IPromise, module } from 'angular';
 
 import { API } from 'core/api/ApiService';
-import { IFunctionSourceData, IFunction } from 'core/domain';
-import { CORE_FUNCTION_FUNCTION_TRANSFORMER } from './function.transformer';
+import { IFunctionSourceData } from 'core/domain';
+import { CORE_FUNCTION_FUNCTION_TRANSFORMER, IFunctionTransformer } from './function.transformer';
 
 export interface IFunctionByAccount {
   name: string;
@@ -16,16 +16,17 @@ export interface IFunctionByAccount {
 }
 
 export class FunctionReader {
-  public static $inject = ['$q', 'functionTransformer'];
-  public constructor(private $q: IQService, private functionTransformer: any) {}
+  public static $inject = ['functionTransformer'];
+
+  public constructor(private functionTransformer: IFunctionTransformer) {}
 
   public loadFunctions(applicationName: string): IPromise<IFunctionSourceData[]> {
     return API.one('applications', applicationName)
       .all('functions')
       .getList()
       .then((functions: IFunctionSourceData[]) => {
         functions = this.functionTransformer.normalizeFunctionSet(functions);
-        return this.$q.all(functions.map(fn => this.normalizeFunction(fn)));
+        return functions.map(fn => this.normalizeFunction(fn));
       });
   }
 
@@ -40,7 +41,7 @@ export class FunctionReader {
       .get()
       .then((functions: IFunctionSourceData[]) => {
         functions = this.functionTransformer.normalizeFunctionSet(functions);
-        return this.$q.all(functions.map(fn => this.normalizeFunction(fn)));
+        return functions.map(fn => this.normalizeFunction(fn));
       });
   }
 
@@ -50,11 +51,10 @@ export class FunctionReader {
       .getList();
   }
 
-  private normalizeFunction(functionDef: IFunctionSourceData): IPromise<IFunction> {
-    return this.functionTransformer.normalizeFunction(functionDef).then((fn: IFunction) => {
-      fn.cloudProvider = fn.cloudProvider || 'aws';
-      return fn;
-    });
+  private normalizeFunction(functionDef: IFunctionSourceData): IFunctionSourceData {
+    const fn = this.functionTransformer.normalizeFunction(functionDef);
+    fn.cloudProvider = fn.cloudProvider || 'aws';
+    return fn;
   }
 }
 

app/scripts/modules/core/src/function/function.transformer.js → app/scripts/modules/core/src/function/function.transformer.ts

@@ -5,20 +5,27 @@ import { module } from 'angular';
 import { chain, flow } from 'lodash';
 
 import { PROVIDER_SERVICE_DELEGATE } from 'core/cloudProvider/providerService.delegate';
+import { IFunctionSourceData } from 'core';
 
 export const CORE_FUNCTION_FUNCTION_TRANSFORMER = 'spinnaker.core.function.transformer';
 export const name = CORE_FUNCTION_FUNCTION_TRANSFORMER; // for backwards compatibility
+
+export interface IFunctionTransformer {
+  normalizeFunction: (functionDef: IFunctionSourceData) => IFunctionSourceData;
+  normalizeFunctionSet: (functions: IFunctionSourceData[]) => IFunctionSourceData[];
+}
+
 module(CORE_FUNCTION_FUNCTION_TRANSFORMER, [PROVIDER_SERVICE_DELEGATE]).factory('functionTransformer', [
   'providerServiceDelegate',
-  function(providerServiceDelegate) {
-    function normalizeFunction(functionDef) {
+  function(providerServiceDelegate: any): IFunctionTransformer {
+    function normalizeFunction(functionDef: IFunctionSourceData): IFunctionSourceData {
       return providerServiceDelegate
         .getDelegate(functionDef.provider ? functionDef.provider : 'aws', 'function.transformer')
         .normalizeFunction(functionDef);
     }
 
-    function normalizeFunctionSet(functions) {
-      const setNormalizers = chain(functions)
+    function normalizeFunctionSet(functions: IFunctionSourceData[]): IFunctionSourceData[] {
+      const setNormalizers: any = chain(functions)
         .filter(fn => providerServiceDelegate.hasDelegate(fn.provider ? fn.provider : 'aws', 'function.setTransformer'))
         .compact()
         .map(fn => providerServiceDelegate.getDelegate(fn.provider, 'function.setTransformer').normalizeFunctionSet)