diff --git a/src/app.module.ts b/src/app.module.ts index 4b802eba3b..bdb48834ba 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -9,9 +9,17 @@ import { ChainsModule } from './chains/chains.module'; import { BalancesModule } from './balances/balances.module'; import { LoggerMiddleware } from './common/middleware/logger.middleware'; import { NetworkModule } from './common/network/network.module'; +import { ConfigurationModule } from './common/config/configuration.module'; @Module({ - imports: [BalancesModule, ChainsModule, NetworkModule], + imports: [ + // features + BalancesModule, + ChainsModule, + // common + NetworkModule, + ConfigurationModule, + ], }) export class AppModule implements NestModule { configure(consumer: MiddlewareConsumer) { diff --git a/src/balances/balances.controller.spec.ts b/src/balances/balances.controller.spec.ts index 537736b848..95296c28b6 100644 --- a/src/balances/balances.controller.spec.ts +++ b/src/balances/balances.controller.spec.ts @@ -4,20 +4,38 @@ import * as request from 'supertest'; import safeBalanceFactory from '../services/transaction-service/entities/__tests__/balance.factory'; import exchangeResultFactory from '../services/exchange/entities/__tests__/exchange.factory'; import chainFactory from '../services/config-service/entities/__tests__/chain.factory'; -import configuration from '../config/configuration'; import { mockNetworkService, TestNetworkModule, } from '../common/network/__tests__/test.network.module'; import { BalancesModule } from './balances.module'; +import { + fakeConfigurationService, + TestConfigurationModule, +} from '../common/config/__tests__/test.configuration.module'; describe('Balances Controller (Unit)', () => { let app: INestApplication; + beforeAll(async () => { + fakeConfigurationService.set('exchange.baseUri', 'https://test.exchange'); + fakeConfigurationService.set( + 'safeConfig.baseUri', + 'https://test.safe.config', + ); + }); + beforeEach(async () => { jest.clearAllMocks(); + const moduleFixture: TestingModule = await Test.createTestingModule({ - imports: [BalancesModule, TestNetworkModule], + imports: [ + // feature + BalancesModule, + // common + TestConfigurationModule, + TestNetworkModule, + ], }).compile(); app = moduleFixture.createNestApplication(); await app.init(); @@ -35,7 +53,7 @@ describe('Balances Controller (Unit)', () => { const exchangeResponse = exchangeResultFactory({ USD: 2.0 }); const chainResponse = chainFactory(chainId); mockNetworkService.get.mockImplementation((url) => { - if (url == `https://safe-config.gnosis.io/api/v1/chains/${chainId}`) { + if (url == `https://test.safe.config/api/v1/chains/${chainId}`) { return Promise.resolve({ data: chainResponse }); } else if ( url == @@ -44,7 +62,7 @@ describe('Balances Controller (Unit)', () => { return Promise.resolve({ data: transactionServiceBalancesResponse, }); - } else if (url == configuration().exchange.baseUri) { + } else if (url == 'https://test.exchange') { return Promise.resolve({ data: exchangeResponse }); } else { return Promise.reject(new Error(`Could not match ${url}`)); @@ -78,7 +96,7 @@ describe('Balances Controller (Unit)', () => { // Once caching is in place we don't need to retrieve the Chain Data again expect(mockNetworkService.get.mock.calls.length).toBe(4); expect(mockNetworkService.get.mock.calls[0][0]).toBe( - 'https://safe-config.gnosis.io/api/v1/chains/1', + 'https://test.safe.config/api/v1/chains/1', ); expect(mockNetworkService.get.mock.calls[1][0]).toBe( `${chainResponse.transactionService}/api/v1/safes/0x0000000000000000000000000000000000000001/balances/usd/`, @@ -87,7 +105,7 @@ describe('Balances Controller (Unit)', () => { params: { trusted: undefined, excludeSpam: undefined }, }); expect(mockNetworkService.get.mock.calls[2][0]).toBe( - configuration().exchange.baseUri, + 'https://test.exchange', ); }); @@ -98,7 +116,7 @@ describe('Balances Controller (Unit)', () => { const transactionServiceBalancesResponse = safeBalanceFactory(1); const chainResponse = chainFactory(chainId); mockNetworkService.get.mockImplementation((url) => { - if (url == `https://safe-config.gnosis.io/api/v1/chains/${chainId}`) { + if (url == `https://test.safe.config/api/v1/chains/${chainId}`) { return Promise.resolve({ data: chainResponse }); } else if ( url == @@ -107,7 +125,7 @@ describe('Balances Controller (Unit)', () => { return Promise.resolve({ data: transactionServiceBalancesResponse, }); - } else if (url == configuration().exchange.baseUri) { + } else if (url == 'https://test.exchange') { return Promise.reject({ status: 500 }); } else { return Promise.reject(new Error(`Could not match ${url}`)); @@ -132,7 +150,7 @@ describe('Balances Controller (Unit)', () => { const exchangeResponse = {}; // no rates const chainResponse = chainFactory(chainId); mockNetworkService.get.mockImplementation((url) => { - if (url == `https://safe-config.gnosis.io/api/v1/chains/${chainId}`) { + if (url == `https://test.safe.config/api/v1/chains/${chainId}`) { return Promise.resolve({ data: chainResponse }); } else if ( url == @@ -141,7 +159,7 @@ describe('Balances Controller (Unit)', () => { return Promise.resolve({ data: transactionServiceBalancesResponse, }); - } else if (url == configuration().exchange.baseUri) { + } else if (url == 'https://test.exchange') { return Promise.resolve({ data: exchangeResponse }); } else { return Promise.reject(new Error(`Could not match ${url}`)); @@ -167,7 +185,7 @@ describe('Balances Controller (Unit)', () => { const exchangeResponse = exchangeResultFactory({ XYZ: 2 }); // Returns different rate than USD const chainResponse = chainFactory(chainId); mockNetworkService.get.mockImplementation((url) => { - if (url == `https://safe-config.gnosis.io/api/v1/chains/${chainId}`) { + if (url == `https://test.safe.config/api/v1/chains/${chainId}`) { return Promise.resolve({ data: chainResponse }); } else if ( url == @@ -176,7 +194,7 @@ describe('Balances Controller (Unit)', () => { return Promise.resolve({ data: transactionServiceBalancesResponse, }); - } else if (url == configuration().exchange.baseUri) { + } else if (url == 'https://test.exchange') { return Promise.resolve({ data: exchangeResponse }); } else { return Promise.reject(new Error(`Could not match ${url}`)); @@ -202,7 +220,7 @@ describe('Balances Controller (Unit)', () => { const exchangeResponse = exchangeResultFactory({ USD: 0 }); // rate is zero const chainResponse = chainFactory(chainId); mockNetworkService.get.mockImplementation((url) => { - if (url == `https://safe-config.gnosis.io/api/v1/chains/${chainId}`) { + if (url == `https://test.safe.config/api/v1/chains/${chainId}`) { return Promise.resolve({ data: chainResponse }); } else if ( url == @@ -211,7 +229,7 @@ describe('Balances Controller (Unit)', () => { return Promise.resolve({ data: transactionServiceBalancesResponse, }); - } else if (url == configuration().exchange.baseUri) { + } else if (url == 'https://test.exchange') { return Promise.resolve({ data: exchangeResponse }); } else { return Promise.reject(new Error(`Could not match ${url}`)); @@ -238,7 +256,7 @@ describe('Balances Controller (Unit)', () => { const exchangeResponse = exchangeResultFactory({ USD: 2 }); // Returns different rate than XYZ const chainResponse = chainFactory(chainId); mockNetworkService.get.mockImplementation((url) => { - if (url == `https://safe-config.gnosis.io/api/v1/chains/${chainId}`) { + if (url == `https://test.safe.config/api/v1/chains/${chainId}`) { return Promise.resolve({ data: chainResponse }); } else if ( url == @@ -247,7 +265,7 @@ describe('Balances Controller (Unit)', () => { return Promise.resolve({ data: transactionServiceBalancesResponse, }); - } else if (url == configuration().exchange.baseUri) { + } else if (url == 'https://test.exchange') { return Promise.resolve({ data: exchangeResponse }); } else { return Promise.reject(new Error(`Could not match ${url}`)); @@ -274,14 +292,14 @@ describe('Balances Controller (Unit)', () => { const exchangeResponse = exchangeResultFactory({ USD: 2.0 }); const chainResponse = chainFactory(chainId); mockNetworkService.get.mockImplementation((url) => { - if (url == `https://safe-config.gnosis.io/api/v1/chains/${chainId}`) { + if (url == `https://test.safe.config/api/v1/chains/${chainId}`) { return Promise.resolve({ data: chainResponse }); } else if ( url == `${chainResponse.transactionService}/api/v1/safes/${safeAddress}/balances/usd/` ) { return Promise.reject({ status: 500 }); - } else if (url == configuration().exchange.baseUri) { + } else if (url == 'https://test.exchange') { return Promise.resolve({ data: exchangeResponse }); } else { return Promise.reject(new Error(`Could not match ${url}`)); diff --git a/src/chains/chains.controller.spec.ts b/src/chains/chains.controller.spec.ts index 8ead8680af..4e65d8a100 100644 --- a/src/chains/chains.controller.spec.ts +++ b/src/chains/chains.controller.spec.ts @@ -9,6 +9,10 @@ import { mockNetworkService, TestNetworkModule, } from '../common/network/__tests__/test.network.module'; +import { + fakeConfigurationService, + TestConfigurationModule, +} from '../common/config/__tests__/test.configuration.module'; describe('Chains Controller (Unit)', () => { let app: INestApplication; @@ -23,10 +27,23 @@ describe('Chains Controller (Unit)', () => { const chainResponse: Chain = chainFactory(); const backboneResponse: Backbone = backboneFactory(); + beforeAll(async () => { + fakeConfigurationService.set( + 'safeConfig.baseUri', + 'https://test.safe.config', + ); + }); + beforeEach(async () => { jest.clearAllMocks(); const moduleFixture: TestingModule = await Test.createTestingModule({ - imports: [ChainsModule, TestNetworkModule], + imports: [ + // feature + ChainsModule, + // common + TestConfigurationModule, + TestNetworkModule, + ], }).compile(); app = moduleFixture.createNestApplication(); diff --git a/src/common/config/__tests__/fake.configuration.service.spec.ts b/src/common/config/__tests__/fake.configuration.service.spec.ts new file mode 100644 index 0000000000..8184e7d0e1 --- /dev/null +++ b/src/common/config/__tests__/fake.configuration.service.spec.ts @@ -0,0 +1,36 @@ +import { FakeConfigurationService } from './fake.configuration.service'; + +describe('FakeConfigurationService', () => { + let configurationService: FakeConfigurationService; + + beforeEach(async () => { + configurationService = new FakeConfigurationService(); + }); + + it(`Setting key should store its value`, async () => { + configurationService.set('aaa', 'bbb'); + + const result = configurationService.get('aaa'); + + expect(configurationService.keyCount()).toBe(1); + expect(result).toBe('bbb'); + }); + + it(`Retrieving unknown key should return undefined`, async () => { + configurationService.set('aaa', 'bbb'); + + const result = configurationService.get('unknown_key'); + + expect(result).toBe(undefined); + }); + + it(`Retrieving unknown key should throw`, async () => { + configurationService.set('aaa', 'bbb'); + + const result = () => { + configurationService.getOrThrow('unknown_key'); + }; + + expect(result).toThrow(Error('No value set for key unknown_key')); + }); +}); diff --git a/src/common/config/__tests__/fake.configuration.service.ts b/src/common/config/__tests__/fake.configuration.service.ts new file mode 100644 index 0000000000..17dec8c028 --- /dev/null +++ b/src/common/config/__tests__/fake.configuration.service.ts @@ -0,0 +1,26 @@ +import { IConfigurationService } from '../configuration.service.interface'; + +export class FakeConfigurationService implements IConfigurationService { + private configuration: Record = {}; + + set(key: string, value: any) { + this.configuration[key] = value; + } + + keyCount(): number { + return Object.keys(this.configuration).length; + } + + get(key: string): T | undefined { + return this.configuration[key] as T; + } + + getOrThrow(key: string): T { + const value = this.configuration[key]; + if (value === undefined) { + throw Error(`No value set for key ${key}`); + } + + return value as T; + } +} diff --git a/src/common/config/__tests__/test.configuration.module.ts b/src/common/config/__tests__/test.configuration.module.ts new file mode 100644 index 0000000000..47aeb37a00 --- /dev/null +++ b/src/common/config/__tests__/test.configuration.module.ts @@ -0,0 +1,32 @@ +import { Global, Module } from '@nestjs/common'; +import { IConfigurationService } from '../configuration.service.interface'; +import { FakeConfigurationService } from './fake.configuration.service'; + +/** + * {@link fakeConfigurationService} should be used in a test setup. + * + * It provides the ability to set a specific configuration key to any value. + * + * {@link fakeConfigurationService} is available only when a module imports + * {@link TestConfigurationModule} + */ +export const fakeConfigurationService = new FakeConfigurationService(); + +/** + * The {@link TestConfigurationModule} should be used whenever you want to + * override the values provided by {@link NestConfigurationService} + * + * Example: + * Test.createTestingModule({ imports: [ModuleA, TestConfigurationModule]}).compile(); + * + * This will create a TestModule which uses the implementation of ModuleA but + * overrides the real Configuration Module with a fake one – {@link fakeConfigurationService} + */ +@Global() +@Module({ + providers: [ + { provide: IConfigurationService, useValue: fakeConfigurationService }, + ], + exports: [IConfigurationService], +}) +export class TestConfigurationModule {} diff --git a/src/common/config/configuration.module.spec.ts b/src/common/config/configuration.module.spec.ts new file mode 100644 index 0000000000..77897498eb --- /dev/null +++ b/src/common/config/configuration.module.spec.ts @@ -0,0 +1,14 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { ConfigurationModule } from './configuration.module'; + +describe('ConfigurationModule', () => { + it(`ConfigurationModule is successfully created`, async () => { + const moduleFixture: TestingModule = await Test.createTestingModule({ + imports: [ConfigurationModule], + }).compile(); + + const app = moduleFixture.createNestApplication(); + await app.init(); + await app.close(); + }); +}); diff --git a/src/common/config/configuration.module.ts b/src/common/config/configuration.module.ts new file mode 100644 index 0000000000..ccbaedb169 --- /dev/null +++ b/src/common/config/configuration.module.ts @@ -0,0 +1,26 @@ +import { Global, Module } from '@nestjs/common'; +import { ConfigModule } from '@nestjs/config'; +import { IConfigurationService } from './configuration.service.interface'; +import { NestConfigurationService } from './nest.configuration.service'; +import configuration from './entities/configuration'; + +/** + * A {@link Global} Module which provides local configuration support via {@link IConfigurationService} + * Feature Modules don't need to import this module directly in order to inject + * the {@link IConfigurationService}. + * + * This module should be included in the "root" application module + */ +@Global() +@Module({ + imports: [ + ConfigModule.forRoot({ + load: [configuration], + }), + ], + providers: [ + { provide: IConfigurationService, useClass: NestConfigurationService }, + ], + exports: [IConfigurationService], +}) +export class ConfigurationModule {} diff --git a/src/common/config/configuration.service.interface.ts b/src/common/config/configuration.service.interface.ts new file mode 100644 index 0000000000..5cb4602c0c --- /dev/null +++ b/src/common/config/configuration.service.interface.ts @@ -0,0 +1,6 @@ +export const IConfigurationService = Symbol('IConfigurationService'); + +export interface IConfigurationService { + get(key: string): T | undefined; + getOrThrow(key: string): T; +} diff --git a/src/config/configuration.ts b/src/common/config/entities/configuration.ts similarity index 100% rename from src/config/configuration.ts rename to src/common/config/entities/configuration.ts diff --git a/src/common/config/nest.configuration.service.ts b/src/common/config/nest.configuration.service.ts new file mode 100644 index 0000000000..39804c8d4b --- /dev/null +++ b/src/common/config/nest.configuration.service.ts @@ -0,0 +1,16 @@ +import { Injectable } from '@nestjs/common'; +import { ConfigService as NestConfigService } from '@nestjs/config'; +import { IConfigurationService } from './configuration.service.interface'; + +@Injectable() +export class NestConfigurationService implements IConfigurationService { + constructor(private readonly configService: NestConfigService) {} + + get(key: string): T | undefined { + return this.configService.get(key); + } + + getOrThrow(key: string): T { + return this.configService.getOrThrow(key); + } +} diff --git a/src/services/config-service/config-service.module.ts b/src/services/config-service/config-service.module.ts index db030722c5..7b30f3817d 100644 --- a/src/services/config-service/config-service.module.ts +++ b/src/services/config-service/config-service.module.ts @@ -1,15 +1,8 @@ import { Module } from '@nestjs/common'; import { HttpErrorHandler } from '../errors/http-error-handler'; import { ConfigService } from './config-service.service'; -import { ConfigModule } from '@nestjs/config'; -import configuration from '../../config/configuration'; @Module({ - imports: [ - ConfigModule.forRoot({ - load: [configuration], - }), - ], providers: [ConfigService, HttpErrorHandler], exports: [ConfigService], }) diff --git a/src/services/config-service/config-service.service.ts b/src/services/config-service/config-service.service.ts index d0c26851cf..4b03c54781 100644 --- a/src/services/config-service/config-service.service.ts +++ b/src/services/config-service/config-service.service.ts @@ -2,23 +2,25 @@ import { Injectable } from '@nestjs/common'; import { Page } from './entities/page.entity'; import { Chain } from './entities/chain.entity'; import { HttpErrorHandler } from '../errors/http-error-handler'; -import { ConfigService as NestConfigService } from '@nestjs/config'; import { Inject } from '@nestjs/common'; import { INetworkService, NetworkService, } from '../../common/network/network.service.interface'; +import { IConfigurationService } from '../../common/config/configuration.service.interface'; @Injectable() export class ConfigService { private readonly baseUri: string; constructor( - private readonly nestConfigService: NestConfigService, + @Inject(IConfigurationService) + private readonly configurationService: IConfigurationService, @Inject(NetworkService) private readonly networkService: INetworkService, private readonly httpErrorHandler: HttpErrorHandler, ) { - this.baseUri = nestConfigService.getOrThrow('safeConfig.baseUri'); + this.baseUri = + configurationService.getOrThrow('safeConfig.baseUri'); } async getChains(): Promise> { diff --git a/src/services/exchange/exchange.module.ts b/src/services/exchange/exchange.module.ts index 5c2b616fb2..06a2b34c48 100644 --- a/src/services/exchange/exchange.module.ts +++ b/src/services/exchange/exchange.module.ts @@ -1,15 +1,8 @@ import { Module } from '@nestjs/common'; -import { ConfigModule } from '@nestjs/config'; -import configuration from '../../config/configuration'; import { HttpErrorHandler } from '../errors/http-error-handler'; import { ExchangeService } from './exchange.service'; @Module({ - imports: [ - ConfigModule.forRoot({ - load: [configuration], - }), - ], providers: [HttpErrorHandler, ExchangeService], exports: [ExchangeService], }) diff --git a/src/services/exchange/exchange.service.ts b/src/services/exchange/exchange.service.ts index 7d730a358c..33451693f3 100644 --- a/src/services/exchange/exchange.service.ts +++ b/src/services/exchange/exchange.service.ts @@ -3,19 +3,20 @@ import { Injectable, InternalServerErrorException, } from '@nestjs/common'; -import { ConfigService } from '@nestjs/config'; import { HttpErrorHandler } from '../errors/http-error-handler'; import { ExchangeResult } from './entities/exchange.entity'; import { INetworkService, NetworkService, } from '../../common/network/network.service.interface'; +import { IConfigurationService } from '../../common/config/configuration.service.interface'; @Injectable() export class ExchangeService { // TODO can we depend on the base url instead? constructor( - private readonly configService: ConfigService, + @Inject(IConfigurationService) + private readonly configurationService: IConfigurationService, @Inject(NetworkService) private readonly networkService: INetworkService, private readonly httpErrorHandler: HttpErrorHandler, ) {} @@ -41,8 +42,8 @@ export class ExchangeService { } private async getExchangeResult(): Promise { - const baseUrl = this.configService.get('exchange.baseUri'); - const apiKey = this.configService.get('exchange.apiKey'); + const baseUrl = this.configurationService.get('exchange.baseUri'); + const apiKey = this.configurationService.get('exchange.apiKey'); try { const { data } = await this.networkService.get(baseUrl, {