Skip to content

Commit

Permalink
Dependency injection in the client process (#1471)
Browse files Browse the repository at this point in the history
  • Loading branch information
timotheeguerin authored Jun 28, 2018
1 parent 8a11064 commit fdd2cf4
Show file tree
Hide file tree
Showing 33 changed files with 296 additions and 173 deletions.
1 change: 0 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
"tslint.autoFixOnSave": true,
"vsicons.presets.angular": true,
"tslint.alwaysShowRuleFailuresAsWarnings": true,
"stylelint.enable": true,
"css.validate": false,
"scss.validate": false,
"python.formatting.provider": "yapf"
Expand Down
3 changes: 1 addition & 2 deletions app/services/adal/adal.service.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { Injectable } from "@angular/core";
import { AccessToken, ServerError } from "@batch-flask/core";
import { AccessToken, AccessTokenCache, ServerError } from "@batch-flask/core";
import { ElectronRemote, NotificationService } from "@batch-flask/ui";
import { BehaviorSubject, Observable } from "rxjs";

import { BatchLabsService } from "app/services/batch-labs.service";
import { AADService } from "client/core/aad";
import { AccessTokenCache } from "client/core/aad/access-token/access-token-cache";
import { Constants } from "common";

@Injectable()
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@
"@angular/material": "6.2.1",
"@angular/platform-browser": "6.0.5",
"@angular/platform-browser-dynamic": "6.0.5",
"@angular/platform-server": "^6.0.6",
"@angular/router": "6.0.5",
"@tweenjs/tween.js": "~17.1.1",
"azure-storage": "~2.8.3",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import * as moment from "moment";

import { AccessToken } from "@batch-flask/core";
import { localStorage } from "client/core/local-storage";
import { Constants } from "common";
import { mockNodeStorage } from "test/utils/mocks/storage";
import { AccessToken, DataStoreKeys, InMemoryDataStore } from "@batch-flask/core";
import { AccessTokenCache } from "./access-token-cache";

const tenant1 = "tenant-1";
Expand All @@ -20,15 +17,16 @@ const token1 = {

describe("AccessTokenCache", () => {
let cache: AccessTokenCache;
let localStorageSpy: InMemoryDataStore;

describe("when using localstorage", () => {
localStorageSpy = new InMemoryDataStore();
beforeEach(() => {
mockNodeStorage(localStorage);
cache = new AccessTokenCache(localStorage);
cache = new AccessTokenCache(localStorageSpy as any);
});

it("doesn't set the access token if not in localstorage", () => {
localStorage.removeItem(Constants.localStorageKey.currentAccessToken);
localStorageSpy.removeItem(DataStoreKeys.currentAccessToken);
cache.init();
expect((cache as any)._tokens).toEqual({});
});
Expand All @@ -39,7 +37,7 @@ describe("AccessTokenCache", () => {
[resource1]: { access_token: "sometoken", expires_on: moment().subtract(1, "hour") },
},
};
localStorage.setItem(Constants.localStorageKey.currentAccessToken, JSON.stringify(token));
localStorageSpy.setItem(DataStoreKeys.currentAccessToken, JSON.stringify(token));
cache.init();
expect(cache.getToken(tenant1, resource1)).toBeFalsy();
});
Expand All @@ -50,7 +48,7 @@ describe("AccessTokenCache", () => {
[resource1]: token1,
},
};
localStorage.setItem(Constants.localStorageKey.currentAccessToken, JSON.stringify(data));
localStorageSpy.setItem(DataStoreKeys.currentAccessToken, JSON.stringify(data));
await cache.init();
const token = cache.getToken(tenant1, resource1);
expect(token).not.toBeFalsy();
Expand All @@ -61,7 +59,7 @@ describe("AccessTokenCache", () => {

describe("without local storage(memory only)", () => {
beforeEach(() => {
cache = new AccessTokenCache();
cache = new AccessTokenCache(localStorageSpy as any);
});

it("save and retrieve tokens", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import { AccessToken } from "@batch-flask/core";
import { LocalStorage } from "client/core/local-storage";
import { Constants } from "common";
import { DataStoreKeys } from "@batch-flask/core/constants";
import { DataStore } from "@batch-flask/core/data-store";
import { AccessToken } from "../access-token";

const dataStoreKey = DataStoreKeys.currentAccessToken;
/**
* Hellper class to storage the access tokens in memory and in the localstorage.
*/
export class AccessTokenCache {
private _tokens: any = {};

constructor(private storage: LocalStorage = null) { }
constructor(private storage?: DataStore) { }

public async init() {
return this._loadFromStorage();
Expand Down Expand Up @@ -41,7 +42,7 @@ export class AccessTokenCache {
public clear() {
this._tokens = {};
if (this.storage) {
this.storage.removeItem(Constants.localStorageKey.currentAccessToken);
this.storage.removeItem(dataStoreKey);
}
}

Expand All @@ -54,12 +55,12 @@ export class AccessTokenCache {
tokens[tenantId][resource] = this._tokens[tenantId][resource];
}
}
return this.storage.setItem(Constants.localStorageKey.currentAccessToken, JSON.stringify(tokens));
return this.storage.setItem(dataStoreKey, JSON.stringify(tokens));
}

private async _loadFromStorage() {
if (!this.storage) { return; }
const tokenStr = await this.storage.getItem(Constants.localStorageKey.currentAccessToken);
const tokenStr = await this.storage.getItem<string>(dataStoreKey);
if (!tokenStr) {
return;
}
Expand All @@ -71,7 +72,7 @@ export class AccessTokenCache {
}
} catch (e) {
if (this.storage) {
this.storage.removeItem(Constants.localStorageKey.currentAccessToken);
this.storage.removeItem(dataStoreKey);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/@batch-flask/core/aad/access-token-cache/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./access-token-cache";
1 change: 1 addition & 0 deletions src/@batch-flask/core/aad/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from "./access-token";
export * from "./access-token-cache";
7 changes: 7 additions & 0 deletions src/@batch-flask/core/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,10 @@ export const RetryableHttpCode = new Set([
HttpCode.ServiceUnavailable,
HttpCode.GatewayTimeout,
]);

export const DataStoreKeys = {
/**
* LocalStorage key for storing the access token(For AAD request)
*/
currentAccessToken: "current_access_token",
};
14 changes: 14 additions & 0 deletions src/@batch-flask/core/data-store/data-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Data store is a generic interface for saving key values
*/
export interface DataStore {
size: number;

setItem<T= string>(key: string, value: T): Promise<void>;

getItem<T= string>(key: string): Promise<T>;

removeItem(key: string): Promise<void>;

clear(): Promise<void>;
}
28 changes: 28 additions & 0 deletions src/@batch-flask/core/data-store/in-memory-data-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { DataStore } from "./data-store";

export class InMemoryDataStore implements DataStore {
protected _data = new Map<string, any>();

public async setItem<T>(key: string, value: T): Promise<void> {
this._data.set(key, value);
return Promise.resolve();
}

public async getItem<T>(key: string): Promise<T> {
return Promise.resolve(this._data.get(key));
}

public async removeItem(key: string): Promise<void> {
this._data.delete(key);
return Promise.resolve();
}

public async clear(): Promise<void> {
this._data.clear();
return Promise.resolve();
}

public get size() {
return this._data.size;
}
}
2 changes: 2 additions & 0 deletions src/@batch-flask/core/data-store/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from "./data-store";
export * from "./in-memory-data-store";
1 change: 1 addition & 0 deletions src/@batch-flask/core/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from "./constants";
export * from "./data-store";
export * from "./dto";
export * from "./aad";
export * from "./record";
Expand Down
38 changes: 38 additions & 0 deletions src/client/client.module.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { NgModule } from "@angular/core";
import { ServerModule } from "@angular/platform-server";
import { AADService } from "client/core/aad";
import { BatchLabsInitializer } from "client/core/batchlabs-initializer";
import { BlIpcMain } from "client/core/bl-ipc-main";
import { FileSystem } from "client/core/fs";
import { LocalDataStore } from "client/core/local-data-store";
import { LocalFileStorage } from "client/core/local-file-storage";
import { ProxySettingsManager } from "client/proxy";
import { autoUpdater } from "electron-updater";
import { AUTO_UPDATER, BatchLabsApplication } from "./core/batchlabs-application";

/**
* BatchLabs client module. This is the root module for managing dependency injection in the Client process
* Only import other modules or use providers.
* DO NOT define any components here.
*/
@NgModule({
imports: [
ServerModule,
],
providers: [
{ provide: AUTO_UPDATER, useValue: autoUpdater },
BatchLabsApplication,
BatchLabsInitializer,
ProxySettingsManager,
LocalDataStore,
LocalFileStorage,
AADService,
BlIpcMain,
FileSystem,
],
})
export class BatchLabsClientModule {
public ngDoBootstrap() {
// Nothing to do
}
}
1 change: 1 addition & 0 deletions src/client/core/aad/access-token/access-token.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface AccessTokenErrorResult {
*/
export class AccessTokenService {
constructor(private app: BatchLabsApplication, private config: AADConfig) {

}

/**
Expand Down
1 change: 0 additions & 1 deletion src/client/core/aad/access-token/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export * from "./access-token.service";
export * from "./access-token-cache";
19 changes: 12 additions & 7 deletions src/client/core/aad/adal/aad.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import * as moment from "moment";

import { AccessToken } from "@batch-flask/core";
import { localStorage } from "client/core/local-storage";
import { AccessToken, InMemoryDataStore } from "@batch-flask/core";
import { Constants } from "common";
import { F } from "test/utils";
import { mockNodeStorage } from "test/utils/mocks/storage";
import { MockBrowserWindow, MockSplashScreen } from "test/utils/mocks/windows";
import { AADUser } from "./aad-user";
import { AADService } from "./aad.service";
Expand Down Expand Up @@ -37,21 +35,28 @@ describe("AADService", () => {
let service: AADService;
let currentUser: AADUser;
let appSpy;
let localStorage;
let ipcMainMock;

beforeEach(() => {
localStorage = new InMemoryDataStore();
appSpy = {
mainWindow: new MockBrowserWindow(),
splashScreen: new MockSplashScreen(),
};
mockNodeStorage(localStorage);
service = new AADService(appSpy);

ipcMainMock = {
on: () => null,
};

service = new AADService(appSpy, localStorage, ipcMainMock);
service.currentUser.subscribe(x => currentUser = x);
service.init();
});

it("when there is no item in the localstorage it should not set the id_token", () => {
localStorage.removeItem(Constants.localStorageKey.currentUser);
const tmpService = new AADService(appSpy);
const tmpService = new AADService(appSpy, localStorage, ipcMainMock);
tmpService.init();
let user: AADUser = null;
tmpService.currentUser.subscribe(x => user = x);
Expand All @@ -60,7 +65,7 @@ describe("AADService", () => {

it("when localstorage has currentUser it should load it", async (done) => {
await localStorage.setItem(Constants.localStorageKey.currentUser, JSON.stringify(sampleUser));
const tmpService = new AADService(appSpy);
const tmpService = new AADService(appSpy, localStorage, ipcMainMock);
await tmpService.init();
let user: AADUser = null;
tmpService.currentUser.subscribe(x => user = x);
Expand Down
Loading

0 comments on commit fdd2cf4

Please sign in to comment.