From 9b2f428d6399aa87de585ca31c217c9f8b0fb779 Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 17:44:10 +0200 Subject: [PATCH 01/10] fix: changed from Map to Record. Change init name to update. added latLonTableTemp instead of overriding directly on latLonTable --- src/latLon/DAL/latLonDAL.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/latLon/DAL/latLonDAL.ts b/src/latLon/DAL/latLonDAL.ts index 5b17fc08..332f74a1 100644 --- a/src/latLon/DAL/latLonDAL.ts +++ b/src/latLon/DAL/latLonDAL.ts @@ -16,7 +16,7 @@ let latLonDALInstance: LatLonDAL | null = null; @injectable() export class LatLonDAL { - private readonly latLonMap: Map; + private latLonTable: Record; private onGoingUpdate: boolean; private dataLoad: | { @@ -26,17 +26,18 @@ export class LatLonDAL { } | undefined; private dataLoadError: boolean; + private latLonTableTemp: Record | null = null; public constructor( @inject(SERVICES.LOGGER) private readonly logger: Logger, @inject(S3_REPOSITORY_SYMBOL) private readonly latLonRepository: S3Repository ) { - this.latLonMap = new Map(); + this.latLonTable = {}; this.onGoingUpdate = true; this.dataLoad = undefined; this.dataLoadError = false; - this.init().catch((error: Error) => { + this.update().catch((error: Error) => { this.logger.error({ msg: 'Failed to initialize lat-lon data', error }); this.dataLoadError = true; }); @@ -52,7 +53,7 @@ export class LatLonDAL { } /* istanbul ignore end */ - public async init(): Promise { + public async update(): Promise { try { const dataLoadPromise = new Promise((resolve, reject) => { this.dataLoadError = false; @@ -87,27 +88,30 @@ export class LatLonDAL { throw new InternalServerError('Lat-lon to tile data currently not available'); } await this.dataLoad?.promise; - return this.latLonMap.get(`${x},${y},${zone}`); + return this.latLonTable[`${x},${y},${zone}`]; } - private clearLatLonMap(): void { - this.logger.debug('Clearing latLon data'); - this.latLonMap.clear(); + public getLatLonTable(): Record { + return this.latLonTable; } private async loadLatLonData(): Promise { this.logger.debug('Loading latLon data'); - this.clearLatLonMap(); + this.latLonTableTemp = {}; const latLonDataPath = await this.latLonRepository.downloadFile('latLonConvertionTable'); const { items: latLonData } = JSON.parse(await fs.promises.readFile(latLonDataPath, 'utf8')) as { items: LatLon[] }; latLonData.forEach((latLon) => { - this.latLonMap.set(`${latLon.min_x},${latLon.min_y},${latLon.zone}`, latLon); + this.latLonTableTemp![`${latLon.min_x},${latLon.min_y},${latLon.zone}`] = latLon; }); + this.latLonTable = this.latLonTableTemp; + this.latLonTableTemp = null; + Object.freeze(this.latLonTable); + try { await fs.promises.unlink(latLonDataPath); } catch (error) { @@ -146,7 +150,7 @@ export const cronLoadTileLatLonDataFactory: FactoryFunction scheduledTask = cron.schedule(cronPattern, () => { if (!latLonDAL.getOnGoingUpdate()) { logger.info('cronLoadTileLatLonData: starting update'); - latLonDAL.init().catch((error: Error) => { + latLonDAL.update().catch((error: Error) => { logger.error({ msg: 'cronLoadTileLatLonData: update failed', error }); }); } else { From a5bce93ecbaa1589025e995343e215caf3abf6ff Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 17:45:06 +0200 Subject: [PATCH 02/10] feat(configManager): manager created --- src/config/models/configManager.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/config/models/configManager.ts diff --git a/src/config/models/configManager.ts b/src/config/models/configManager.ts new file mode 100644 index 00000000..5c14b3d3 --- /dev/null +++ b/src/config/models/configManager.ts @@ -0,0 +1,12 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import { inject, injectable } from 'tsyringe'; +import { LatLonDAL, latLonDalSymbol } from '../../latLon/DAL/latLonDAL'; + +@injectable() +export class ConfigManager { + public constructor(@inject(latLonDalSymbol) private readonly latLonDAL: LatLonDAL) {} + + public getControlTable(): ReturnType { + return this.latLonDAL.getLatLonTable(); + } +} From 1b1f5c9dc8f18e5b881aa780f972ae7160847a50 Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 17:45:30 +0200 Subject: [PATCH 03/10] feat(configController): controller created --- src/config/controllers/configController.ts | 43 ++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 src/config/controllers/configController.ts diff --git a/src/config/controllers/configController.ts b/src/config/controllers/configController.ts new file mode 100644 index 00000000..178c36a3 --- /dev/null +++ b/src/config/controllers/configController.ts @@ -0,0 +1,43 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import { Logger } from '@map-colonies/js-logger'; +import { BoundCounter, Meter } from '@opentelemetry/api-metrics'; +import { RequestHandler } from 'express'; +import httpStatus from 'http-status-codes'; +import { injectable, inject } from 'tsyringe'; +import { SERVICES } from '../../common/constants'; +import { ConfigManager } from '../models/configManager'; +import { LatLonDAL } from '../../latLon/DAL/latLonDAL'; + +type GetTilesHandler = RequestHandler< + undefined, + | ReturnType + | { + type: string; + message: string; + }, + undefined, + undefined +>; + +@injectable() +export class ConfigController { + private readonly createdResourceCounter: BoundCounter; + + public constructor( + @inject(SERVICES.LOGGER) private readonly logger: Logger, + @inject(ConfigManager) private readonly manager: ConfigManager, + @inject(SERVICES.METER) private readonly meter: Meter + ) { + this.createdResourceCounter = meter.createCounter('config_created_resource'); + } + + public getControlTable: GetTilesHandler = (_, res, next) => { + try { + const response = this.manager.getControlTable(); + return res.status(httpStatus.OK).json(response); + } catch (error: unknown) { + this.logger.error({ msg: 'ConfigController.getControlTable error while trying to return control table', error }); + next(error); + } + }; +} From 9bb88e256eb7acbf8c72d8bda01f3b8588b872a2 Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 17:45:47 +0200 Subject: [PATCH 04/10] feat(configRouter): router created --- src/config/routes/configRouter.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 src/config/routes/configRouter.ts diff --git a/src/config/routes/configRouter.ts b/src/config/routes/configRouter.ts new file mode 100644 index 00000000..f3949c51 --- /dev/null +++ b/src/config/routes/configRouter.ts @@ -0,0 +1,16 @@ +import { Router } from 'express'; +import { FactoryFunction } from 'tsyringe'; +import { ConfigController } from '../controllers/configController'; + +const configRouterFactory: FactoryFunction = (dependencyContainer) => { + const router = Router(); + const controller = dependencyContainer.resolve(ConfigController); + + router.get('/control/table', controller.getControlTable); + + return router; +}; + +export const CONFIG_ROUTER_SYMBOL = Symbol('configRouterFactory'); + +export { configRouterFactory }; From f603ac3bd9dc80a705911349e83d1893bad8f149 Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 17:46:10 +0200 Subject: [PATCH 05/10] feat(containerConfig): added CONFIG_ROUTER_SYMBOL, configRouterFactory injection --- src/containerConfig.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/containerConfig.ts b/src/containerConfig.ts index 8c3109e9..dee60328 100644 --- a/src/containerConfig.ts +++ b/src/containerConfig.ts @@ -28,6 +28,7 @@ import { s3ClientFactory } from './common/s3'; import { S3_REPOSITORY_SYMBOL, s3RepositoryFactory } from './common/s3/s3Repository'; import { healthCheckFactory } from './common/utils'; import { MGRS_ROUTER_SYMBOL, mgrsRouterFactory } from './mgrs/routes/mgrsRouter'; +import { CONFIG_ROUTER_SYMBOL, configRouterFactory } from './config/routes/configRouter'; export interface RegisterOptions { override?: InjectionObject[]; @@ -164,6 +165,7 @@ export const registerExternalValues = async (options?: RegisterOptions): Promise } }, }, + { token: CONFIG_ROUTER_SYMBOL, provider: { useFactory: configRouterFactory } }, ]; const container = await registerDependencies(dependencies, options?.override, options?.useChild); return container; From 983285181eed20b3102138cb63719f5000b1b6cf Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 17:46:25 +0200 Subject: [PATCH 06/10] feat(serverBuilder): init /config route --- src/serverBuilder.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/serverBuilder.ts b/src/serverBuilder.ts index 3bf7bb5d..ce4105ab 100644 --- a/src/serverBuilder.ts +++ b/src/serverBuilder.ts @@ -15,9 +15,9 @@ import { ITEM_ROUTER_SYMBOL } from './control/item/routes/itemRouter'; import { ROUTE_ROUTER_SYMBOL } from './control/route/routes/routeRouter'; import { LAT_LON_ROUTER_SYMBOL } from './latLon/routes/latLonRouter'; import { GEOTEXT_SEARCH_ROUTER_SYMBOL } from './location/routes/locationRouter'; -import { cronLoadTileLatLonDataSymbol } from './latLon/DAL/latLonDAL'; import { FeedbackApiMiddlewareManager } from './common/middlewares/feedbackApi.middleware'; import { MGRS_ROUTER_SYMBOL } from './mgrs/routes/mgrsRouter'; +import { CONFIG_ROUTER_SYMBOL } from './config/routes/configRouter'; @injectable() export class ServerBuilder { @@ -31,7 +31,7 @@ export class ServerBuilder { @inject(ROUTE_ROUTER_SYMBOL) private readonly routeRouter: Router, @inject(LAT_LON_ROUTER_SYMBOL) private readonly latLonRouter: Router, @inject(GEOTEXT_SEARCH_ROUTER_SYMBOL) private readonly geotextRouter: Router, - @inject(cronLoadTileLatLonDataSymbol) private readonly cronLoadTileLatLonData: void, + @inject(CONFIG_ROUTER_SYMBOL) private readonly configRouter: Router, @inject(FeedbackApiMiddlewareManager) private readonly feedbackApiMiddleware: FeedbackApiMiddlewareManager, @inject(MGRS_ROUTER_SYMBOL) private readonly mgrsRouter: Router ) { @@ -68,6 +68,7 @@ export class ServerBuilder { this.serverInstance.use('/search', router); this.serverInstance.use('/lookup', this.latLonRouter); + this.serverInstance.use('/config', this.configRouter); } private buildControlRoutes(): Router { From 513cba27a2fccbd944e99dee0c3b3789b9a9c5b7 Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 17:47:57 +0200 Subject: [PATCH 07/10] feat(openapi): added /config/control/table route --- openapi3.yaml | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/openapi3.yaml b/openapi3.yaml index 15fecabb..da021432 100644 --- a/openapi3.yaml +++ b/openapi3.yaml @@ -469,7 +469,59 @@ paths: security: - x-api-key: [] x-user-id: [] + /config/control/table: + get: + operationId: configGetControlTable + tags: + - Control + summary: 'Get Control Grid table' + responses: + 200: + description: 'OK' + content: + application/json: + schema: + type: object + description: 'An object with dynamic keys following the pattern `${latLon.min_x},${latLon.min_y},${latLon.zone}`. Each key represents a coordinate and zone identifier.' + additionalProperties: + type: object + properties: + tile_name: + type: string + example: 'BRN' + zone: + type: string + example: '33' + min_x: + type: string + example: '360000' + min_y: + type: string + example: 5820000 + ext_min_x: + type: number + example: 360000 + ext_min_y: + type: number + example: 5820000 + ext_max_x: + type: number + example: 370000 + ext_max_y: + type: number + example: 5830000 + 400: + '$ref': '#/components/responses/BadRequest' + 401: + '$ref': '#/components/responses/Unauthorized' + 403: + '$ref': '#/components/responses/Forbidden' + 500: + '$ref': '#/components/responses/InternalError' + security: + - x-api-key: [] + x-user-id: [] components: responses: BadRequest: From 1695da3c04e8cdef9d4f87fb8cf62deaffd6f5fc Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 18:44:32 +0200 Subject: [PATCH 08/10] fix: added await if data update is ongoing --- src/config/controllers/configController.ts | 12 +++++++----- src/config/models/configManager.ts | 5 +++-- src/latLon/DAL/latLonDAL.ts | 3 ++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/config/controllers/configController.ts b/src/config/controllers/configController.ts index 178c36a3..3dda97dc 100644 --- a/src/config/controllers/configController.ts +++ b/src/config/controllers/configController.ts @@ -6,11 +6,12 @@ import httpStatus from 'http-status-codes'; import { injectable, inject } from 'tsyringe'; import { SERVICES } from '../../common/constants'; import { ConfigManager } from '../models/configManager'; -import { LatLonDAL } from '../../latLon/DAL/latLonDAL'; +import { ConvertCamelToSnakeCase } from '../../common/utils'; +import { LatLon } from '../../latLon/models/latLon'; type GetTilesHandler = RequestHandler< undefined, - | ReturnType + | Record> | { type: string; message: string; @@ -31,11 +32,12 @@ export class ConfigController { this.createdResourceCounter = meter.createCounter('config_created_resource'); } - public getControlTable: GetTilesHandler = (_, res, next) => { + public getControlTable: GetTilesHandler = async (_, res, next) => { try { - const response = this.manager.getControlTable(); + const response = await this.manager.getControlTable(); return res.status(httpStatus.OK).json(response); - } catch (error: unknown) { + } catch (error: unknown) /* istanbul ignore next */ { + // Ignore next in code coverage as we don't expect an error to be thrown but it might happen. this.logger.error({ msg: 'ConfigController.getControlTable error while trying to return control table', error }); next(error); } diff --git a/src/config/models/configManager.ts b/src/config/models/configManager.ts index 5c14b3d3..6057dcab 100644 --- a/src/config/models/configManager.ts +++ b/src/config/models/configManager.ts @@ -6,7 +6,8 @@ import { LatLonDAL, latLonDalSymbol } from '../../latLon/DAL/latLonDAL'; export class ConfigManager { public constructor(@inject(latLonDalSymbol) private readonly latLonDAL: LatLonDAL) {} - public getControlTable(): ReturnType { - return this.latLonDAL.getLatLonTable(); + public async getControlTable(): ReturnType { + const response = await this.latLonDAL.getLatLonTable(); + return response; } } diff --git a/src/latLon/DAL/latLonDAL.ts b/src/latLon/DAL/latLonDAL.ts index 332f74a1..f0301e2a 100644 --- a/src/latLon/DAL/latLonDAL.ts +++ b/src/latLon/DAL/latLonDAL.ts @@ -91,7 +91,8 @@ export class LatLonDAL { return this.latLonTable[`${x},${y},${zone}`]; } - public getLatLonTable(): Record { + public async getLatLonTable(): Promise> { + await this.dataLoad?.promise; return this.latLonTable; } From 97b8d64331c8c15c8f3f0e7dcad541efe3dd8caa Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 18:44:54 +0200 Subject: [PATCH 09/10] feat(integration/config): added test-case --- tests/integration/config/config.spec.ts | 73 +++++++++++++++++++ .../config/helpers/requestSender.ts | 14 ++++ 2 files changed, 87 insertions(+) create mode 100644 tests/integration/config/config.spec.ts create mode 100644 tests/integration/config/helpers/requestSender.ts diff --git a/tests/integration/config/config.spec.ts b/tests/integration/config/config.spec.ts new file mode 100644 index 00000000..34b37c69 --- /dev/null +++ b/tests/integration/config/config.spec.ts @@ -0,0 +1,73 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import 'jest-openapi'; +import { DependencyContainer } from 'tsyringe'; +import { Application } from 'express'; +import { CleanupRegistry } from '@map-colonies/cleanup-registry'; +import jsLogger from '@map-colonies/js-logger'; +import { trace } from '@opentelemetry/api'; +import httpStatusCodes from 'http-status-codes'; +import { getApp } from '../../../src/app'; +import { SERVICES } from '../../../src/common/constants'; +import { cronLoadTileLatLonDataSymbol, LatLonDAL, latLonDalSymbol, latLonSignletonFactory } from '../../../src/latLon/DAL/latLonDAL'; +import { LatLon } from '../../../src/latLon/models/latLon'; +import { ConvertCamelToSnakeCase } from '../../../src/common/utils'; +import { ConfigRequestSender } from './helpers/requestSender'; + +describe('/config/control/table', function () { + let requestSender: ConfigRequestSender; + let app: { app: Application; container: DependencyContainer }; + + beforeEach(async function () { + app = await getApp({ + override: [ + { token: SERVICES.LOGGER, provider: { useValue: jsLogger({ enabled: false }) } }, + { token: SERVICES.TRACER, provider: { useValue: trace.getTracer('testTracer') } }, + { token: cronLoadTileLatLonDataSymbol, provider: { useValue: {} } }, + { token: SERVICES.ELASTIC_CLIENTS, provider: { useValue: {} } }, + { token: latLonDalSymbol, provider: { useFactory: latLonSignletonFactory } }, + ], + useChild: true, + }); + + requestSender = new ConfigRequestSender(app.app); + }); + + afterAll(async function () { + const cleanupRegistry = app.container.resolve(SERVICES.CLEANUP_REGISTRY); + await cleanupRegistry.trigger(); + app.container.reset(); + + jest.clearAllTimers(); + }); + + describe('Happy Path', function () { + it('should return 200 status code and Control Grid table', async function () { + const response = await requestSender.getControlTable(); + + expect(response.status).toBe(httpStatusCodes.OK); + expect(response).toSatisfyApiSpec(); + expect(response.body).toEqual>>({ + '360000,5820000,33': { + tile_name: 'BRN', + zone: '33', + min_x: '360000', + min_y: '5820000', + ext_min_x: 360000, + ext_min_y: 5820000, + ext_max_x: 370000, + ext_max_y: 5830000, + }, + '480000,5880000,32': { + tile_name: 'BMN', + zone: '32', + min_x: '480000', + min_y: '5880000', + ext_min_x: 480000, + ext_min_y: 5880000, + ext_max_x: 490000, + ext_max_y: 5890000, + }, + }); + }); + }); +}); diff --git a/tests/integration/config/helpers/requestSender.ts b/tests/integration/config/helpers/requestSender.ts new file mode 100644 index 00000000..626bf1f9 --- /dev/null +++ b/tests/integration/config/helpers/requestSender.ts @@ -0,0 +1,14 @@ +import * as supertest from 'supertest'; + +export class ConfigRequestSender { + public constructor(private readonly app: Express.Application) {} + + public async getControlTable(): Promise { + return supertest + .agent(this.app) + .get('/config/control/table') + .set('Content-Type', 'application/json') + .set('x-api-key', 'abc123') + .set('x-user-id', 'abc123'); + } +} From 396e3f09099dd1f38f32a329bb2d2b5d2e7cbd12 Mon Sep 17 00:00:00 2001 From: Niv Greenstein Date: Sun, 27 Oct 2024 18:45:15 +0200 Subject: [PATCH 10/10] feat(unit/config): added test-case --- tests/unit/config/config.spec.ts | 70 ++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 tests/unit/config/config.spec.ts diff --git a/tests/unit/config/config.spec.ts b/tests/unit/config/config.spec.ts new file mode 100644 index 00000000..742d3e88 --- /dev/null +++ b/tests/unit/config/config.spec.ts @@ -0,0 +1,70 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import { InternalServerError } from '../../../src/common/errors'; +import { ConfigManager } from '../../../src/config/models/configManager'; +import { LatLonDAL } from '../../../src/latLon/DAL/latLonDAL'; + +let configManager: ConfigManager; +describe('#ConfigManager', () => { + beforeEach(() => { + const latLonDAL = { + getLatLonTable: () => ({ + '360000,5820000,33': { + tile_name: 'BRN', + zone: '33', + min_x: '360000', + min_y: '5820000', + ext_min_x: 360000, + ext_min_y: 5820000, + ext_max_x: 370000, + ext_max_y: 5830000, + }, + '480000,5880000,32': { + tile_name: 'BMN', + zone: '32', + min_x: '480000', + min_y: '5880000', + ext_min_x: 480000, + ext_min_y: 5880000, + ext_max_x: 490000, + ext_max_y: 5890000, + }, + }), + } as unknown as LatLonDAL; + configManager = new ConfigManager(latLonDAL); + }); + + describe('happy path', () => { + it('should return Control Grid Table data', async () => { + const result = await configManager.getControlTable(); + expect(result).toEqual({ + '360000,5820000,33': { + tile_name: 'BRN', + zone: '33', + min_x: '360000', + min_y: '5820000', + ext_min_x: 360000, + ext_min_y: 5820000, + ext_max_x: 370000, + ext_max_y: 5830000, + }, + '480000,5880000,32': { + tile_name: 'BMN', + zone: '32', + min_x: '480000', + min_y: '5880000', + ext_min_x: 480000, + ext_min_y: 5880000, + ext_max_x: 490000, + ext_max_y: 5890000, + }, + }); + }); + }); + + describe('sad path', () => { + it('should return error', async () => { + jest.spyOn(configManager, 'getControlTable').mockRejectedValue(new InternalServerError('Error')); + await expect(configManager.getControlTable()).rejects.toThrow('Error'); + }); + }); +});