From fbaca4f7d1674e1b8734f05efab1d6be589f8f36 Mon Sep 17 00:00:00 2001 From: razbroc Date: Sun, 23 Aug 2026 14:42:39 +0300 Subject: [PATCH 1/2] fix: type the mapproxy caches section as a map of caches IMapProxyJsonDocument.caches was typed as a single IMapProxyCache rather than a map of them, so every caches[name] lookup resolved to any through the index signature, and the casts around those lookups were unchecked. --- src/common/interfaces.ts | 2 +- src/layers/models/layersManager.ts | 1 - .../unit/layers/models/layersManager.spec.ts | 20 +++++++------------ 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/common/interfaces.ts b/src/common/interfaces.ts index 51195d9..3568747 100644 --- a/src/common/interfaces.ts +++ b/src/common/interfaces.ts @@ -82,7 +82,7 @@ export interface IRedisConfig { export interface IMapProxyJsonDocument { services: JsonObject; layers: IMapProxyLayer[]; - caches: IMapProxyCache; + caches: Record; grids: JsonObject; globals: IMapProxyGlobalConfig; } diff --git a/src/layers/models/layersManager.ts b/src/layers/models/layersManager.ts index c39c6ec..8a51719 100644 --- a/src/layers/models/layersManager.ts +++ b/src/layers/models/layersManager.ts @@ -66,7 +66,6 @@ class LayersManager { // our current only real cache layer, other caches cases are known as the source layers const cacheName = isSourceType(cacheType) && cacheType === SourceTypes.REDIS ? getRedisCacheName(layerName) : layerName; - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const currentSourceCache: IMapProxyCache | undefined = configJson.caches[cacheName]; if (currentSourceCache === undefined) { diff --git a/tests/unit/layers/models/layersManager.spec.ts b/tests/unit/layers/models/layersManager.spec.ts index b1cf06d..79621b3 100644 --- a/tests/unit/layers/models/layersManager.spec.ts +++ b/tests/unit/layers/models/layersManager.spec.ts @@ -4,7 +4,7 @@ import { container } from 'tsyringe'; import { jsLogger, type Logger } from '@map-colonies/js-logger'; import { BadRequestError, ConflictError, NotFoundError, NotImplementedError } from '@map-colonies/error-types'; import { lookup as mimeLookup, TilesMimeFormat } from '@map-colonies/types'; -import { ILayerPostRequest, IMapProxyCache, IMapProxyConfig, IRedisConfig } from '../../../../src/common/interfaces'; +import { ILayerPostRequest, IMapProxyCache, IMapProxyConfig, IRedisConfig, IS3Source } from '../../../../src/common/interfaces'; import { LayersManager } from '../../../../src/layers/models/layersManager'; import { mockLayerNameAlreadyExists } from '../../mock/mockLayerNameAlreadyExists'; import { mockLayerNameIsNotExists } from '../../mock/mockLayerNameIsNotExists'; @@ -247,8 +247,7 @@ describe('layersManager', () => { await expect(layersManager.addLayer(mockLayerNameIsNotExists)).toResolve(); const resultJson = await MockConfigProvider.getJson(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - expect(resultJson.caches[mockLayerNameIsNotExists.name].cache.use_http_get).toBe(true); + expect((resultJson.caches[mockLayerNameIsNotExists.name]?.cache as IS3Source).use_http_get).toBe(true); expect(updateJsonMock).toHaveBeenCalledTimes(1); }); @@ -267,8 +266,7 @@ describe('layersManager', () => { await expect(layersManager.addLayer(mockLayerNameIsNotExists)).toResolve(); const resultJson = await MockConfigProvider.getJson(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - expect(resultJson.caches[mockLayerNameIsNotExists.name].cache.use_http_get).toBe(false); + expect((resultJson.caches[mockLayerNameIsNotExists.name]?.cache as IS3Source).use_http_get).toBe(false); expect(updateJsonMock).toHaveBeenCalledTimes(1); }); }); @@ -368,10 +366,8 @@ describe('layersManager', () => { jest.spyOn(configManager, 'getConfig').mockResolvedValue(mockData()); //check data const data = await MockConfigProvider.getJson(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - expect(data.caches[mockLayerName].format).toBe(expectedTileMimeFormatPng); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - expect(data.caches[mockRedisLayerName].format).toBe(expectedTileMimeFormatPng); + expect(data.caches[mockLayerName]?.format).toBe(expectedTileMimeFormatPng); + expect(data.caches[mockRedisLayerName]?.format).toBe(expectedTileMimeFormatPng); // action const action = layersManager.updateLayer(mockLayerName, mockUpdateLayerRequest); @@ -380,10 +376,8 @@ describe('layersManager', () => { expect.assertions(6); await expect(action).toResolve(); const result = await MockConfigProvider.getJson(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - expect(result.caches[mockLayerName].format).toBe(expectedTileMimeFormatJpeg); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - expect(result.caches[mockRedisLayerName].format).toBe(expectedTileMimeFormatJpeg); + expect(result.caches[mockLayerName]?.format).toBe(expectedTileMimeFormatJpeg); + expect(result.caches[mockRedisLayerName]?.format).toBe(expectedTileMimeFormatJpeg); expect(updateJsonMock).toHaveBeenCalledTimes(1); }); From 9e421cbb45c9f34fea3fdd323aab4297059eeff2 Mon Sep 17 00:00:00 2001 From: razbroc Date: Sun, 23 Aug 2026 14:43:22 +0300 Subject: [PATCH 2/2] feat: return the whole cache from GET /layer/{layerName}/{cacheType} The endpoint returned only the cache source, the inner 'cache' section of a mapproxy cache. It now returns the whole cache as it is written in the configuration, alongside its name. The cache source stays at the same 'cache' key, so existing consumers read it unchanged and the new fields are additive. The cache is spread verbatim, so mapproxy options we do not model are returned too. Also: - ICacheObject and the unused ICacheName become IGetCacheResponse, which the request handler now actually declares as its response type. - a caches entry that is not an object, or holds no cache source, raises the existing NotFoundError instead of a 500 or a corrupt response. - adds the missing gpkgCache response schema, since geopackage is an accepted cacheType, with mapproxy's own option set and defaults. --- openapi3.yaml | 84 +++++++++++++++++++ src/common/interfaces.ts | 7 +- src/common/utils.ts | 16 +++- src/layers/controllers/layersController.ts | 4 +- src/layers/models/layersManager.ts | 24 ++---- .../integration/layers/layersManager.spec.ts | 16 +++- .../unit/layers/models/layersManager.spec.ts | 29 ++++++- tests/unit/mock/mockJson.json | 5 ++ 8 files changed, 155 insertions(+), 30 deletions(-) diff --git a/openapi3.yaml b/openapi3.yaml index 0af14d4..c282c92 100644 --- a/openapi3.yaml +++ b/openapi3.yaml @@ -246,6 +246,12 @@ paths: directory: >- /f9f92153-96e1-46fb-a44b-5c1b0955639c/c67ef7fb-a08c-41f8-ba4b-f2ba9a4b9a2a/ directory_layout: tms + sources: [] + grids: + - epsg4326dir + format: image/png + upscale_tiles: 18 + minimize_meta_requests: true s3: summary: s3 cache value: @@ -256,6 +262,12 @@ paths: /5eedfc75-861c-42fc-81a9-ab2c0b95c274/d8527f15-5377-4a28-b5ce-92891d897aec/ directory_layout: tms bucket_name: bucket-name + sources: [] + grids: + - epsg4326dir + format: image/png + upscale_tiles: 18 + minimize_meta_requests: true redis: summary: redis cache value: @@ -267,6 +279,25 @@ paths: password: password username: '' default_ttl: 86400 + sources: + - example-layer + grids: + - epsg4326dir + format: image/png + geopackage: + summary: geopackage cache + value: + cacheName: example-layer + cache: + type: geopackage + filename: /path/to/tiles.gpkg + table_name: tiles + sources: [] + grids: + - epsg4326dir + format: image/png + upscale_tiles: 18 + minimize_meta_requests: true '400': description: Not valid request content: @@ -423,25 +454,78 @@ components: default_ttl: type: integer example: 86400 + gpkgCache: + type: object + required: + - type + properties: + type: + type: string + enum: + - geopackage + filename: + type: string + description: >- + Path to the geopackage file. Defaults to cachename.gpkg. Ignored + when levels is true. + example: /path/to/tiles.gpkg + table_name: + type: string + description: >- + Table the tiles are stored in. Defaults to cachename_gridname. + example: tiles + levels: + type: boolean + description: >- + Store each level in a separate geopackage under directory. Defaults + to false. + directory: + type: string + description: Directory of the geopackage files, used only when levels is true. getCacheResponse: type: object + description: >- + A mapproxy cache, as it is written in the configuration, alongside its + name. Properties other than cacheName are the cache entry itself, so + options that are not listed here may also be present. required: - cacheName - cache properties: cacheName: type: string + example: example-layer cache: oneOf: - $ref: '#/components/schemas/fileCache' - $ref: '#/components/schemas/s3Cache' - $ref: '#/components/schemas/redisCache' + - $ref: '#/components/schemas/gpkgCache' discriminator: propertyName: type mapping: file: '#/components/schemas/fileCache' s3: '#/components/schemas/s3Cache' redis: '#/components/schemas/redisCache' + geopackage: '#/components/schemas/gpkgCache' + sources: + type: array + items: + type: string + grids: + type: array + items: + type: string + example: + - epsg4326dir + format: + type: string + example: image/png + upscale_tiles: + type: number + example: 18 + minimize_meta_requests: + type: boolean getConfigResponse: type: object properties: diff --git a/src/common/interfaces.ts b/src/common/interfaces.ts index 3568747..3c7387a 100644 --- a/src/common/interfaces.ts +++ b/src/common/interfaces.ts @@ -116,15 +116,10 @@ export interface IRedisSource extends ICacheSource { default_ttl: number; } -export interface ICacheName { +export interface IGetCacheResponse extends IMapProxyCache { cacheName: string; } -export interface ICacheObject { - cacheName: string; - cache: IRedisSource | IS3Source | IFSSource; -} - export interface IGpkgSource extends ICacheSource { filename: string; table_name: string; diff --git a/src/common/utils.ts b/src/common/utils.ts index b104dcf..a726260 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -3,7 +3,7 @@ import { promises as fsp } from 'node:fs'; import { safeLoad, safeDump, YAMLException } from 'js-yaml'; import { container } from 'tsyringe'; import { SERVICES } from '../common/constants'; -import { IFSConfig, IMapProxyJsonDocument } from './interfaces'; +import { IFSConfig, IMapProxyCache, IMapProxyJsonDocument } from './interfaces'; import { SourceTypes } from './enums'; // read mapproxy yaml config file and convert it into a json object @@ -61,6 +61,20 @@ export function isLayerNameSuffixRedis(layerName: string): boolean { return layerName.endsWith('-redis'); } +/** + * Check if an entry of the mapproxy 'caches' section is a well formed cache, holding a cache source. + * Hand edited configurations may hold entries that are not objects at all, or objects with no 'cache' section. + * @param cache entry taken from the mapproxy 'caches' section + * @return boolean - if the entry can be treated as a cache holding a cache source. + */ +export function isMapProxyCache(cache: unknown): cache is IMapProxyCache { + if (typeof cache !== 'object' || cache === null) { + return false; + } + const cacheSource: unknown = (cache as Record).cache; + return typeof cacheSource === 'object' && cacheSource !== null; +} + export function adjustTilesPath(tilesPath: string, cacheSource: SourceTypes): string { const fsConfig = container.resolve(SERVICES.FS); switch (cacheSource) { diff --git a/src/layers/controllers/layersController.ts b/src/layers/controllers/layersController.ts index ac1dc15..9f62bb1 100644 --- a/src/layers/controllers/layersController.ts +++ b/src/layers/controllers/layersController.ts @@ -3,12 +3,12 @@ import type { RequestHandler } from 'express'; import httpStatus from 'http-status-codes'; import { injectable, inject } from 'tsyringe'; import { SERVICES } from '../../common/constants'; -import type { ICacheName, ILayerPostRequest, IMapProxyCache } from '../../common/interfaces'; +import type { IGetCacheResponse, ILayerPostRequest, IMapProxyCache } from '../../common/interfaces'; import { LayersManager } from '../models/layersManager'; type CreateLayerHandler = RequestHandler; type GetLayerHandler = RequestHandler<{ name: string }, IMapProxyCache, IMapProxyCache>; -type GetCacheHandler = RequestHandler<{ layerName: string; cacheType: string }, ICacheName>; +type GetCacheHandler = RequestHandler<{ layerName: string; cacheType: string }, IGetCacheResponse>; type UpdateLayerHandler = RequestHandler<{ name: string }, ILayerPostRequest, ILayerPostRequest>; type DeleteLayerHandler = RequestHandler; @injectable() diff --git a/src/layers/models/layersManager.ts b/src/layers/models/layersManager.ts index 8a51719..1f544af 100644 --- a/src/layers/models/layersManager.ts +++ b/src/layers/models/layersManager.ts @@ -17,10 +17,7 @@ import type { ICacheProvider, ICacheSource, IRedisConfig, - ICacheObject, - IRedisSource, - IS3Source, - IFSSource, + IGetCacheResponse, } from '../../common/interfaces'; import { isLayerNameExists } from '../../common/validations/isLayerNameExists'; import { S3Source } from '../../common/cacheProviders/S3Source'; @@ -29,7 +26,7 @@ import { FSSource } from '../../common/cacheProviders/fsSource'; import { isSourceType, SourceTypes, sourceTypeValues } from '../../common/enums'; import { RedisSource } from '../../common/cacheProviders/redisSource'; import { ConfigsManager } from '../../configs/models/configsManager'; -import { getRedisCacheName, getRedisCacheOriginalName, isLayerNameSuffixRedis } from '../../common/utils'; +import { getRedisCacheName, getRedisCacheOriginalName, isLayerNameSuffixRedis, isMapProxyCache } from '../../common/utils'; @injectable() class LayersManager { @@ -54,7 +51,7 @@ class LayersManager { } @withSpanAsyncV4 - public async getCacheByNameAndType(layerName: string, cacheType: string): Promise { + public async getCacheByNameAndType(layerName: string, cacheType: string): Promise { const configJson = await this.configProvider.getJson(); const requestedLayer = configJson.layers.find((layer) => layer.name === layerName); @@ -66,25 +63,22 @@ class LayersManager { // our current only real cache layer, other caches cases are known as the source layers const cacheName = isSourceType(cacheType) && cacheType === SourceTypes.REDIS ? getRedisCacheName(layerName) : layerName; - const currentSourceCache: IMapProxyCache | undefined = configJson.caches[cacheName]; + const requestedCache = configJson.caches[cacheName]; - if (currentSourceCache === undefined) { + // a missing entry, an entry that is not an object, or an entry with no cache source, are all unusable as a cache + if (!isMapProxyCache(requestedCache)) { const errorMsg = `cache not found for ${layerName} layer`; this.logger.warn({ msg: errorMsg, layerName, cacheType }); throw new NotFoundError(errorMsg); } - if (currentSourceCache.cache.type !== cacheType) { + if (requestedCache.cache.type !== cacheType) { const errorMsg = `${layerName} layer cache not found with requested cache type: ${cacheType}`; this.logger.warn({ msg: errorMsg, layerName, cacheType }); throw new BadRequestError(errorMsg); } - type AvailableSources = IRedisSource | IS3Source | IFSSource; - - return { - cacheName: cacheName, - cache: currentSourceCache.cache as AvailableSources, - }; + // the whole cache is returned as it is written in the configuration, the cache name is ours and always wins + return { ...requestedCache, cacheName }; } @withSpanAsyncV4 diff --git a/tests/integration/layers/layersManager.spec.ts b/tests/integration/layers/layersManager.spec.ts index c061fc6..d96f10e 100644 --- a/tests/integration/layers/layersManager.spec.ts +++ b/tests/integration/layers/layersManager.spec.ts @@ -1,7 +1,7 @@ import { promises as fsp } from 'node:fs'; import httpStatusCodes from 'http-status-codes'; import { container } from 'tsyringe'; -import { ICacheName, ILayerPostRequest, IMapProxyCache } from '../../../src/common/interfaces'; +import { IGetCacheResponse, ILayerPostRequest, IMapProxyCache } from '../../../src/common/interfaces'; import { mockLayerNameIsNotExists } from '../../unit/mock/mockLayerNameIsNotExists'; import { mockLayerNameAlreadyExists } from '../../unit/mock/mockLayerNameAlreadyExists'; import { init as configProviderInit, updateJsonMock } from '../../unit/mock/mockConfigProvider'; @@ -65,14 +65,24 @@ describe('layerManager', () => { }); describe('#getLayersCache', () => { - it('Happy Path - should return status 200 and the cacheName', async () => { + it('Happy Path - should return status 200 and the whole cache', async () => { const response = await requestSender.getLayersCache('mockLayerNameExists', 's3'); expect(response.status).toBe(httpStatusCodes.OK); - const resource = response.body as ICacheName; + const resource = response.body as IGetCacheResponse; expect(response).toSatisfyApiSpec(); expect(resource.cacheName).toBe('mockLayerNameExists'); + expect(resource).toStrictEqual({ + cacheName: 'mockLayerNameExists', + sources: [], + grids: ['epsg4326dir'], + format: 'image/png', + // eslint-disable-next-line @typescript-eslint/naming-convention + upscale_tiles: 18, + // eslint-disable-next-line @typescript-eslint/naming-convention + cache: { type: 's3', directory: '/path/to/s3/directory/tile', directory_layout: 'tms' }, + }); }); it('Sad Path - should fail with response status 404 Not Found and layer name is not exists', async () => { diff --git a/tests/unit/layers/models/layersManager.spec.ts b/tests/unit/layers/models/layersManager.spec.ts index 79621b3..60741fc 100644 --- a/tests/unit/layers/models/layersManager.spec.ts +++ b/tests/unit/layers/models/layersManager.spec.ts @@ -105,12 +105,17 @@ describe('layersManager', () => { }); describe('#getCacheByNameAndType', () => { - it('should successfully return the cache name', async () => { + it('should successfully return the whole cache alongside its name', async () => { + /* eslint-disable @typescript-eslint/naming-convention */ const expectedCache = { cacheName: 'mockLayerNameExists', - // eslint-disable-next-line @typescript-eslint/naming-convention + sources: [], + grids: ['epsg4326dir'], + format: 'image/png', + upscale_tiles: 18, cache: { directory: '/path/to/s3/directory/tile', directory_layout: 'tms', type: 's3' }, }; + /* eslint-enable @typescript-eslint/naming-convention */ // action expect.assertions(2); @@ -137,12 +142,30 @@ describe('layersManager', () => { // expectation; await expect(action).rejects.toThrow(new NotFoundError(`cache not found for ${layerName} layer`)); }); + it('should fail with not found for a cache that holds no cache source', async () => { + // action + const layerName = 'combined_layers'; + expect.assertions(1); + const action = layersManager.getCacheByNameAndType(layerName, 's3'); + // expectation; + await expect(action).rejects.toThrow(new NotFoundError(`cache not found for ${layerName} layer`)); + }); + + it('should fail with not found for a cache that is not an object', async () => { + // action + const layerName = 'mock'; + expect.assertions(1); + const action = layersManager.getCacheByNameAndType(layerName, 's3'); + // expectation; + await expect(action).rejects.toThrow(new NotFoundError(`cache not found for ${layerName} layer`)); + }); + it('should fail with not valid source type', async () => { // action expect.assertions(1); const action = layersManager.getCacheByNameAndType('mockLayerNameExists', 'notValidType'); // expectation; - await expect(action).rejects.toThrow(new NotFoundError(`mockLayerNameExists layer cache not found with requested cache type: notValidType`)); + await expect(action).rejects.toThrow(new BadRequestError(`mockLayerNameExists layer cache not found with requested cache type: notValidType`)); }); }); diff --git a/tests/unit/mock/mockJson.json b/tests/unit/mock/mockJson.json index 701c912..2c108ed 100644 --- a/tests/unit/mock/mockJson.json +++ b/tests/unit/mock/mockJson.json @@ -100,6 +100,11 @@ "name": "amsterdam_5cm", "title": "amsterdam 5m layer discription", "sources": ["amsterdam_5cm"] + }, + { + "name": "combined_layers", + "title": "title", + "sources": ["mock"] } ] }