Skip to content

Commit

Permalink
fix(core/api): Fix type safety for API.one() builder (#8739)
Browse files Browse the repository at this point in the history
* fix(core/api): Adds type safety for API.one() builder
  • Loading branch information
christopherthielen committed Nov 18, 2020
1 parent 43d58e8 commit b3536fd
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 40 deletions.
14 changes: 2 additions & 12 deletions app/scripts/modules/core/src/api/ApiService.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Spy = jasmine.Spy;
import { mock, noop } from 'angular';
import { AuthenticationInitializer } from '../authentication/AuthenticationInitializer';
import { ICache } from '../cache';
import { API, InvalidAPIResponse } from './ApiService';
import { SETTINGS } from 'core/config/settings';

Expand Down Expand Up @@ -293,12 +294,6 @@ describe('API Service', function () {
});

describe('create config with data', function () {
it('should not alter the config if no data object passed', function () {
const result = API.one('foo').data();
expected.url = `${baseUrl}/foo`;
expect(result.config).toEqual(expected);
});

it('should add data to the config if data object passed', function () {
const data = { bar: 'baz' };
const result = API.one('foo').data(data);
Expand All @@ -310,11 +305,6 @@ describe('API Service', function () {
});

describe('create a config with params', function () {
it('when no params are provided do not alter config', function () {
const result = API.one('foo').withParams();
expect(result.config).toEqual({ method: '', url: `${baseUrl}/foo` });
});

it('when params are provided', function () {
const result = API.one('foo').withParams({ one: 1 });
expect(result.config).toEqual({ method: '', url: `${baseUrl}/foo`, params: { one: 1 } });
Expand All @@ -333,7 +323,7 @@ describe('API Service', function () {
});

it('should set cache to cache object if explicitly set', function () {
const cacheObj = { count: 1 };
const cacheObj = ({ count: 1 } as unknown) as ICache;
const result = API.one('foo').useCache(cacheObj);
expect(result.config.cache).toBe(cacheObj);
});
Expand Down
25 changes: 14 additions & 11 deletions app/scripts/modules/core/src/api/ApiService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,21 @@ import { SETTINGS } from 'core/config/settings';
import { ICache } from 'core/cache';
import { isNil } from 'lodash';

type IPrimitive = string | boolean | number;
type IParams = Record<string, IPrimitive | IPrimitive[]>;

export interface IRequestBuilder {
config?: IRequestConfig;
one?: (...urls: string[]) => IRequestBuilder;
all?: (...urls: string[]) => IRequestBuilder;
useCache?: (useCache: boolean | ICache) => IRequestBuilder;
withParams?: (data: any) => IRequestBuilder;
useCache?: (useCache?: boolean | ICache) => IRequestBuilder;
withParams?: (params: IParams) => IRequestBuilder;
data?: (data: any) => IRequestBuilder;
get?: (data?: any) => PromiseLike<any>;
getList?: (data?: any) => PromiseLike<any>;
post?: (data: any) => PromiseLike<any>;
remove?: (data: any) => PromiseLike<any>;
put?: (data: any) => PromiseLike<any>;
get?: <T = any>(params?: IParams) => PromiseLike<T>;
getList?: <T = any>(params?: IParams) => PromiseLike<T>;
post?: <T = any>(data?: any) => PromiseLike<T>;
remove?: <T = any>(params?: IParams) => PromiseLike<T>;
put?: <T = any>(data?: any) => PromiseLike<T>;
}

export class InvalidAPIResponse extends Error {
Expand Down Expand Up @@ -97,7 +100,7 @@ export class API {
}

// HTTP GET operation
private static getFn(config: IRequestConfig): (data: any) => PromiseLike<any> {
private static getFn(config: IRequestConfig): (params: any) => PromiseLike<any> {
return (params: any) => {
config.method = 'get';
Object.assign(config, this.defaultParams);
Expand All @@ -123,7 +126,7 @@ export class API {
}

// HTTP DELETE operation
private static removeFn(config: IRequestConfig): (data: any) => PromiseLike<any> {
private static removeFn(config: IRequestConfig): (params: any) => PromiseLike<any> {
return (params: any) => {
config.method = 'delete';
if (params) {
Expand Down Expand Up @@ -176,11 +179,11 @@ export class API {
return this.baseReturn(config);
}

public static one(...urls: string[]): any {
public static one(...urls: string[]): IRequestBuilder {
return this.init(urls);
}

public static all(...urls: string[]): any {
public static all(...urls: string[]): IRequestBuilder {
return this.init(urls);
}

Expand Down
8 changes: 4 additions & 4 deletions app/scripts/modules/core/src/image/image.reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@ import { module } from 'angular';

import { ProviderServiceDelegate, PROVIDER_SERVICE_DELEGATE } from 'core/cloudProvider/providerService.delegate';

export interface IFindImageParams {
export type IFindImageParams = {
provider: string;
q?: string;
region?: string;
account?: string;
count?: number;
}
};

export interface IFindTagsParams {
export type IFindTagsParams = {
provider: string;
account: string;
repository: string;
}
};

// marker interface
export interface IImage {}
Expand Down
3 changes: 1 addition & 2 deletions app/scripts/modules/core/src/managed/ManagedReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
IManagedResourceEventHistory,
IManagedResourceDiff,
IManagedResourceEvent,
IManagedApplicationEnvironmentSummary,
} from 'core/domain';

const KIND_NAME_MATCHER = /.*\/(.*?)@/i;
Expand Down Expand Up @@ -89,7 +88,7 @@ export class ManagedReader {
.then(this.decorateResources);
}

public static getEnvironmentsSummary(app: string): PromiseLike<IManagedApplicationEnvironmentSummary> {
public static getEnvironmentsSummary(app: string): PromiseLike<IManagedApplicationSummary> {
return API.one('managed')
.one('application', app)
.withParams({ entities: ['resources', 'artifacts', 'environments'], maxArtifactVersions: 30 })
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/modules/core/src/network/NetworkReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface INetwork {
}

export class NetworkReader {
public static listNetworks(): INetwork[] {
public static listNetworks(): PromiseLike<INetwork[]> {
return API.one('networks').getList();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface INotificationTypeMetadata {
}

export class NotificationService {
public static getNotificationTypeMetadata(): Promise<INotificationTypeMetadata[]> {
public static getNotificationTypeMetadata(): PromiseLike<INotificationTypeMetadata[]> {
return API.one('notifications').all('metadata').useCache().getList();
}
}
4 changes: 2 additions & 2 deletions app/scripts/modules/core/src/plugins/plugin.registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class PluginRegistry {
public loadPluginManifestFromGate() {
const source = 'gate';
const uri = 'plugins/deck/plugin-manifest.json';
const loadPromise = API.one(...uri.split('/'))
const loadPromise: PromiseLike<IPluginMetaData[]> = API.one(...uri.split('/'))
.get()
.catch((error: any) => {
console.error(`Failed to load ${uri} from ${source}`);
Expand All @@ -122,7 +122,7 @@ export class PluginRegistry {
public async loadPluginManifest(
source: ISource,
location: string,
pluginsMetaDataPromise: Promise<IPluginMetaData[]>,
pluginsMetaDataPromise: PromiseLike<IPluginMetaData[]>,
): Promise<IPluginMetaData[]> {
try {
const plugins = await pluginsMetaDataPromise;
Expand Down
4 changes: 2 additions & 2 deletions app/scripts/modules/core/src/search/search.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface ISearchResult {
type: string;
}

export const getFallbackResults = (): ISearchResults<ISearchResult> => {
const getFallbackResults = <T extends ISearchResult>(): ISearchResults<T> => {
return { results: [] };
};

Expand Down Expand Up @@ -58,7 +58,7 @@ export class SearchService {
return requestBuilder
.get()
.then((response: Array<ISearchResults<T>>) => {
return response[0] || getFallbackResults();
return response[0] || getFallbackResults<T>();
})
.catch((response: IHttpPromiseCallbackArg<any>) => {
$log.error(response.data, response);
Expand Down
6 changes: 4 additions & 2 deletions app/scripts/modules/core/src/task/task.read.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

import { $log, $q, $timeout } from 'ngimport';
import { Subject } from 'rxjs';

Expand Down Expand Up @@ -36,7 +35,10 @@ export class TaskReader {
}
return task;
})
.catch((error: any) => $log.warn('There was an issue retrieving taskId: ', taskId, error));
.catch((error: any) => {
$log.warn('There was an issue retrieving taskId: ', taskId, error);
return undefined;
});
}

public static waitUntilTaskMatches(
Expand Down
5 changes: 2 additions & 3 deletions app/scripts/modules/core/src/task/task.write.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@


import { API } from 'core/api/ApiService';
import { ITask } from '../domain';
import { TaskReader } from './task.read.service';
import { ITaskCommand } from './taskExecutor';
import { DebugWindow } from 'core/utils/consoleDebug';
Expand All @@ -14,7 +13,7 @@ export class TaskWriter {
return API.all('tasks').post(taskCommand);
}

public static cancelTask(taskId: string): PromiseLike<void> {
public static cancelTask(taskId: string): PromiseLike<ITask> {
return API.all('tasks')
.one(taskId, 'cancel')
.put()
Expand Down

0 comments on commit b3536fd

Please sign in to comment.