Merge pull request #944 from myrotvorets/refactoring

Refactoring

mocha.setup.mjs

@@ -2,7 +2,6 @@ import { dirname, join } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { use } from 'chai';
 import chaiAsPromised from 'chai-as-promised';
-import { reset } from 'testdouble';
 
 use(chaiAsPromised);
 
@@ -17,10 +16,6 @@ process.env = {
 
 /** @type {import('mocha').RootHookObject} */
 export const mochaHooks = {
-    /** @returns {void} */
-    afterEach() {
-        reset();
-    },
     afterAll() {
         process.env = { ...env };
     },

package.json

@@ -24,16 +24,15 @@
     "@myrotvorets/express-microservice-middlewares": "^2.2.0",
     "@myrotvorets/express-request-logger": "^1.2.0",
     "@myrotvorets/oav-installer": "^5.0.0",
-    "@myrotvorets/opentelemetry-configurator": "^7.0.0",
-    "@myrotvorets/otel-utils": "^0.0.10",
+    "@myrotvorets/opentelemetry-configurator": "^7.1.0",
+    "@myrotvorets/otel-utils": "^1.0.0",
     "@opentelemetry/api": "^1.6.0",
     "@opentelemetry/core": "^1.17.1",
     "@opentelemetry/semantic-conventions": "^1.17.1",
     "awilix": "^9.0.0",
     "envalid": "^8.0.0",
     "express": "^4.18.2",
-    "express-openapi-validator": "^5.0.6",
-    "maxmind": "^4.3.16"
+    "mmdb-lib": "^2.0.2"
   },
   "devDependencies": {
     "@myrotvorets/eslint-config-myrotvorets-ts": "^2.22.5",
@@ -58,7 +57,6 @@
     "nodemon": "^3.0.1",
     "rimraf": "^5.0.5",
     "supertest": "^6.3.3",
-    "testdouble": "^3.20.0",
     "ts-node": "^10.9.1",
     "typescript": "^5.2.2"
   },

src/lib/container.mts

@@ -1,15 +1,20 @@
-import { AwilixContainer, asFunction, asValue, createContainer } from 'awilix';
+import { readFileSync } from 'node:fs';
+import { AwilixContainer, asClass, asFunction, asValue, createContainer } from 'awilix';
 import type { NextFunction, Request, Response } from 'express';
 import { type Logger, type Tracer, getLogger, getTracer } from '@myrotvorets/otel-utils';
 import { environment } from './environment.mjs';
+import type { GeoIPServiceInterface } from '../services/geoipserviceinterface.mjs';
+import type { CityResponse, IspResponse, MMDBReaderServiceInterface } from '../services/mmdbreaderserviceinterface.mjs';
 import { MeteredGeoIPService } from '../services/meteredgeoipservice.mjs';
-import { GeoIPServiceInterface } from '../services/geoipserviceinterface.mjs';
+import { MMDBReaderService } from '../services/mmdbreaderservice.mjs';
 
 export interface Container {
     geoIPService: GeoIPServiceInterface;
     environment: ReturnType<typeof environment>;
     tracer: Tracer;
     logger: Logger;
+    cityReader: MMDBReaderServiceInterface<CityResponse>;
+    ispReader: MMDBReaderServiceInterface<IspResponse>;
 }
 
 export interface RequestContainer {
@@ -39,21 +44,30 @@ function createTracer(): Tracer {
     return getTracer();
 }
 
-function createGeoIPService({ environment, tracer }: Container): GeoIPServiceInterface {
-    const service = new MeteredGeoIPService({ tracer });
-    service.setCityDatabase(environment.GEOIP_CITY_FILE);
-    service.setISPDatabase(environment.GEOIP_ISP_FILE);
-    return service;
+function createCityReader({ environment }: Container): MMDBReaderServiceInterface<CityResponse> {
+    const reader = new MMDBReaderService<CityResponse>();
+    const buf = readFileSync(environment.GEOIP_CITY_FILE);
+    reader.load(buf);
+    return reader;
+}
+
+function createIspReader({ environment }: Container): MMDBReaderServiceInterface<IspResponse> {
+    const reader = new MMDBReaderService<IspResponse>();
+    const buf = readFileSync(environment.GEOIP_ISP_FILE);
+    reader.load(buf);
+    return reader;
 }
 
 export type LocalsWithContainer = Record<'container', AwilixContainer<RequestContainer & Container>>;
 
 export function initializeContainer(): typeof container {
     container.register({
-        geoIPService: asFunction(createGeoIPService).singleton(),
+        geoIPService: asClass(MeteredGeoIPService).singleton(),
         environment: asFunction(createEnvironment).singleton(),
         logger: asFunction(createLogger).scoped(),
         tracer: asFunction(createTracer).singleton(),
+        cityReader: asFunction(createCityReader).singleton(),
+        ispReader: asFunction(createIspReader).singleton(),
     });
 
     return container;

src/server.mts

@@ -52,10 +52,7 @@ export function createApp(): Express {
 /* c8 ignore start */
 export async function run(): Promise<void> {
     const app = createApp();
-    const container = configureApp(app);
-    const env = container.resolve('environment');
-
-    const server = await createServer(app);
-    server.listen(env.PORT);
+    configureApp(app);
+    await createServer(app);
 }
 /* c8 ignore stop */

src/services/geoipservice.mts

@@ -1,32 +1,18 @@
-import { readFileSync } from 'node:fs';
-import { type CityResponse, type IspResponse, Reader, type Response } from 'maxmind';
 import type { GeoCityResponse, GeoIPServiceInterface, GeoIspResponse, GeoResponse } from './geoipserviceinterface.mjs';
+import type { CityResponse, IspResponse, MMDBReaderServiceInterface } from './mmdbreaderserviceinterface.mjs';
 
-export class GeoIPService implements GeoIPServiceInterface {
-    protected _city: Reader<CityResponse> | undefined;
-    protected _isp: Reader<IspResponse> | undefined;
-
-    public setCityDatabase(file: string): boolean {
-        this._city = GeoIPService.readDatabaseSync(file);
-        return this._city !== undefined;
-    }
-
-    public setISPDatabase(file: string): boolean {
-        this._isp = GeoIPService.readDatabaseSync(file);
-        return this._isp !== undefined;
-    }
+export interface GeoIPServiceOptions {
+    cityReader: MMDBReaderServiceInterface<CityResponse>;
+    ispReader: MMDBReaderServiceInterface<IspResponse>;
+}
 
-    private static readDatabaseSync<T extends Response>(file: string): Reader<T> | undefined {
-        if (file) {
-            try {
-                const buf = readFileSync(file);
-                return new Reader(buf);
-            } catch {
-                /* Do nothing */
-            }
-        }
+export class GeoIPService implements GeoIPServiceInterface {
+    protected _city: MMDBReaderServiceInterface<CityResponse>;
+    protected _isp: MMDBReaderServiceInterface<IspResponse>;
 
-        return undefined;
+    public constructor({ cityReader, ispReader }: GeoIPServiceOptions) {
+        this._city = cityReader;
+        this._isp = ispReader;
     }
 
     public geolocate(ip: string): GeoResponse {
@@ -42,11 +28,11 @@ export class GeoIPService implements GeoIPServiceInterface {
     }
 
     protected geolocateCity(ip: string): [CityResponse | null, number] {
-        return this._city?.getWithPrefixLength(ip) ?? [null, 0];
+        return this._city.getWithPrefixLength(ip);
     }
 
     protected geolocateISP(ip: string): [IspResponse | null, number] {
-        return this._isp?.getWithPrefixLength(ip) ?? [null, 0];
+        return this._isp.getWithPrefixLength(ip);
     }
 
     protected adaptCityResponse(response: CityResponse | null): GeoCityResponse {

src/services/geoipserviceinterface.mts

@@ -20,7 +20,5 @@ export interface GeoPrefixes {
 export type GeoResponse = GeoCityResponse & GeoIspResponse & GeoPrefixes;
 
 export interface GeoIPServiceInterface {
-    setCityDatabase(file: string): boolean;
-    setISPDatabase(file: string): boolean;
     geolocate(ip: string): GeoResponse;
 }

src/services/meteredgeoipservice.mts

@@ -1,20 +1,20 @@
-import type { CityResponse, IspResponse } from 'maxmind';
 import { type Tracer, recordErrorToSpan } from '@myrotvorets/otel-utils';
 import { trace } from '@opentelemetry/api';
 import { cityLookupHistogram, countryCounter, ispLookupHistogram } from '../lib/metrics.mjs';
 import { observe } from '../lib/utils.mjs';
-import { GeoIPService } from './geoipservice.mjs';
+import { GeoIPService, type GeoIPServiceOptions } from './geoipservice.mjs';
 import type { GeoCityResponse, GeoIspResponse, GeoResponse } from './geoipserviceinterface.mjs';
 
-interface MeteredGeoIPServiceOptions {
+interface MeteredGeoIPServiceOptions extends GeoIPServiceOptions {
     tracer: Tracer;
 }
+
 export class MeteredGeoIPService extends GeoIPService {
     private readonly _tracer: Tracer;
 
-    public constructor({ tracer }: MeteredGeoIPServiceOptions) {
-        super();
-        this._tracer = tracer;
+    public constructor(opts: MeteredGeoIPServiceOptions) {
+        super(opts);
+        this._tracer = opts.tracer;
     }
 
     public override geolocate(ip: string): GeoResponse {
@@ -29,34 +29,32 @@ export class MeteredGeoIPService extends GeoIPService {
         });
     }
 
-    protected override geolocateCity(ip: string): [CityResponse | null, number] {
-        let city: CityResponse | null = null;
-        let prefix = 0;
+    protected override geolocateCity(ip: string): ReturnType<GeoIPService['geolocateCity']> {
+        let result: ReturnType<GeoIPService['geolocateCity']> = [null, 0];
 
         cityLookupHistogram.record(
             observe(() => {
-                [city, prefix] = super.geolocateCity(ip);
+                result = super.geolocateCity(ip);
             }),
         );
 
-        return [city, prefix];
+        return result;
     }
 
-    protected override geolocateISP(ip: string): [IspResponse | null, number] {
-        let isp: IspResponse | null = null;
-        let prefix = 0;
+    protected override geolocateISP(ip: string): ReturnType<GeoIPService['geolocateISP']> {
+        let result: ReturnType<GeoIPService['geolocateISP']> = [null, 0];
 
         ispLookupHistogram.record(
             observe(() => {
-                [isp, prefix] = super.geolocateISP(ip);
+                result = super.geolocateISP(ip);
             }),
         );
 
-        return [isp, prefix];
+        return result;
     }
 
-    protected override adaptCityResponse(response: CityResponse | null): GeoCityResponse {
-        const result = super.adaptCityResponse(response);
+    protected override adaptCityResponse(...params: Parameters<GeoIPService['adaptCityResponse']>): GeoCityResponse {
+        const result = super.adaptCityResponse(...params);
         trace
             .getActiveSpan()
             /* c8 ignore next */
@@ -66,8 +64,8 @@ export class MeteredGeoIPService extends GeoIPService {
         return result;
     }
 
-    protected override adaptIspResponse(response: IspResponse | null): GeoIspResponse {
-        const result = super.adaptIspResponse(response);
+    protected override adaptIspResponse(...params: Parameters<GeoIPService['adaptIspResponse']>): GeoIspResponse {
+        const result = super.adaptIspResponse(...params);
         trace
             .getActiveSpan()
             /* c8 ignore start */

src/services/mmdbreaderservice.mts

@@ -0,0 +1,14 @@
+import { Reader } from 'mmdb-lib';
+import { MMDBReaderServiceInterface, Response } from './mmdbreaderserviceinterface.mjs';
+
+export class MMDBReaderService<T extends Response> implements MMDBReaderServiceInterface<T> {
+    #reader: Reader<T> | null = null;
+
+    public load(db: Buffer): void {
+        this.#reader = new Reader(db);
+    }
+
+    public getWithPrefixLength(ip: string): [T | null, number] {
+        return this.#reader?.getWithPrefixLength(ip) ?? [null, 0];
+    }
+}

src/services/mmdbreaderserviceinterface.mts

@@ -0,0 +1,8 @@
+import type { Response } from 'mmdb-lib';
+
+export type { Response, CityResponse, IspResponse } from 'mmdb-lib';
+
+export interface MMDBReaderServiceInterface<T extends Response = Response> {
+    load(db: Buffer): void;
+    getWithPrefixLength(ipAddress: string): [T | null, number];
+}

test/functional/controller/geoip.test.mts

@@ -11,7 +11,7 @@ describe('GeoIPController', function () {
     before(async function () {
         await container.dispose();
         app = createApp();
-        return configureApp(app);
+        configureApp(app);
     });
 
     describe('Error handling', function () {

test/unit/lib/container.test.mts

@@ -22,6 +22,9 @@ describe('Container', function () {
                 .to.be.an('object')
                 .that.has.property('startActiveSpan')
                 .that.is.a('function');
+
+            expect(container.resolve('cityReader')).to.be.an('object').that.has.property('load').that.is.a('function');
+            expect(container.resolve('ispReader')).to.be.an('object').that.has.property('load').that.is.a('function');
         });
     });
 });

test/unit/services/fakemmdbreader.mts

@@ -0,0 +1,12 @@
+import { Mock, mock } from 'node:test';
+import type { MMDBReaderServiceInterface, Response } from '../../../src/services/mmdbreaderserviceinterface.mjs';
+
+export class FakeMMDBReader<T extends Response> implements MMDBReaderServiceInterface<T> {
+    public load(_db: Buffer): void {
+        // Do nothing.
+    }
+
+    public getWithPrefixLength: Mock<MMDBReaderServiceInterface<T>['getWithPrefixLength']> = mock.fn<
+        MMDBReaderServiceInterface<T>['getWithPrefixLength']
+    >(() => [null, 0]);
+}