diff --git a/README.md b/README.md index 015dd76..8d631a5 100644 --- a/README.md +++ b/README.md @@ -63,22 +63,26 @@ npm run test:watch #### Integration tests -`npm run test:integration` needs an S3-compatible server. By default it starts a Minio -[testcontainer](https://testcontainers.com/) automatically. - The image is pinned to the release deployed in our Azure environment; bump it in -`tests/integration/helpers/minioContainer.ts` when that environment moves. - -To run against an already-running Minio instead, set `TEST_MINIO_ENDPOINT`: - -| Variable | Default | Purpose | -| ------------------------ | ------------ | ------------------------------------------------------------------ | -| `TEST_MINIO_ENDPOINT` | _(unset)_ | Point the suite at an existing Minio. Unset means start a container. | -| `TEST_MINIO_ACCESS_KEY` | `minioadmin` | Access key for that server. | -| `TEST_MINIO_SECRET_KEY` | `minioadmin` | Secret key for that server. | - -> **The suite creates and deletes buckets on whichever endpoint it is given.** Never point -> `TEST_MINIO_ENDPOINT` at a shared or deployed environment, and beware of leaving it exported in a -> shell profile. Each run prints which mode it selected and against which endpoint. +`npm run test:integration` needs an S3-compatible server and a Redis server. By default it starts +an S3-compatible and a Redis [testcontainer](https://testcontainers.com/) automatically. +The images are pinned to the releases deployed in our environments; bump them in +`tests/integration/helpers/s3Container.ts` and `tests/integration/helpers/redisContainer.ts` +when those environments move. + +To run against already-running servers instead, set the `TEST_*` variables: + +| Variable | Default | Purpose | +| -------------------- | ------------ | ------------------------------------------------------------------------ | +| `TEST_S3_ENDPOINT` | _(unset)_ | Point the suite at an existing S3 server. Unset means start a container. | +| `TEST_S3_ACCESS_KEY` | `minioadmin` | Access key for that server. | +| `TEST_S3_SECRET_KEY` | `minioadmin` | Secret key for that server. | +| `TEST_REDIS_HOST` | _(unset)_ | Point the suite at an existing Redis. Unset means start a container. | +| `TEST_REDIS_PORT` | `6379` | Port for that server. | + +> **The suite creates and deletes buckets on whichever S3 server it is given, and calls `FLUSHDB` on +> whichever Redis it is given.** Never point `TEST_S3_ENDPOINT` or `TEST_REDIS_HOST` at a shared +> or deployed environment, and beware of leaving them exported in a shell profile. Each run prints +> which mode it selected and against which server. ## Customizing the Boilerplate diff --git a/config/custom-environment-variables.json b/config/custom-environment-variables.json index 67aa92f..a9a5deb 100644 --- a/config/custom-environment-variables.json +++ b/config/custom-environment-variables.json @@ -110,6 +110,33 @@ "__name": "FS_SUB_PATHS", "__format": "json" } + }, + "redis": { + "delete": { + "batchSize": { + "__name": "REDIS_DELETE_BATCH_SIZE", + "__format": "number" + }, + "scanCount": { + "__name": "REDIS_SCAN_COUNT", + "__format": "number" + } + }, + "host": "REDIS_HOST", + "port": { + "__name": "REDIS_PORT", + "__format": "number" + }, + "db": { + "__name": "REDIS_DB", + "__format": "number" + }, + "username": "REDIS_USERNAME", + "password": "REDIS_PASSWORD", + "tlsEnabled": { + "__name": "REDIS_TLS_ENABLED", + "__format": "boolean" + } } }, "strategies": { diff --git a/config/default.json b/config/default.json index 96a6b10..4d379c7 100644 --- a/config/default.json +++ b/config/default.json @@ -34,6 +34,14 @@ { "job": "Delete_Layer", "task": "artifacts-deletion" + }, + { + "job": "Update_Delete_Cache", + "task": "tiles-deletion" + }, + { + "job": "Swap_Delete_Cache", + "task": "tiles-deletion" } ] } @@ -58,6 +66,12 @@ }, "deleteLayer": { "type": "Delete_Layer" + }, + "updateCacheDeletion": { + "type": "Update_Delete_Cache" + }, + "swapCacheDeletion": { + "type": "Swap_Delete_Cache" } }, "tasks": { @@ -77,7 +91,7 @@ "shouldResetTimeout": true }, "storage": { - "cleanupStorageProviders": ["FS", "S3"], + "cleanupStorageProviders": ["FS", "S3", "REDIS"], "s3": { "delete": { "batchSize": 1000 @@ -97,6 +111,15 @@ "subPaths": { "tilesSubPath": "tiles" } + }, + "redis": { + "delete": { + "batchSize": 1000, + "scanCount": 1000 + }, + "host": "localhost", + "port": 6379, + "db": 0 } }, "strategies": { diff --git a/helm/templates/_tplValues.tpl b/helm/templates/_tplValues.tpl index 55da75a..6ef84be 100644 --- a/helm/templates/_tplValues.tpl +++ b/helm/templates/_tplValues.tpl @@ -57,6 +57,11 @@ Custom definitions {{- include "common.tplvalues.merge" ( dict "values" ( list .Values.storage .Values.global.storage ) "context" . ) }} {{- end -}} +{{/* storage.redis, then global.storage.redis, then the shared global.redis block */}} +{{- define "common.redis.merged" -}} +{{- include "common.tplvalues.merge" ( dict "values" ( list ((.Values.storage).redis | default dict) ((.Values.global.storage).redis | default dict) (.Values.global.redis | default dict) ) "context" . ) }} +{{- end -}} + {{- define "common.ca.merged" -}} {{- include "common.tplvalues.merge" ( dict "values" ( list .Values.ca .Values.global.ca ) "context" . ) }} {{- end -}} diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index 95bc4c1..792b211 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -3,6 +3,7 @@ {{- $storage := fromYaml (include "common.storage.merged" .) -}} {{- $s3 := ($storage.s3) | default dict -}} {{- $fs := ($storage.fs) | default dict -}} +{{- $redis := fromYaml (include "common.redis.merged" .) -}} {{- $internalPvc := (($fs).internalPvc) | default dict -}} {{- $fsBasePath := clean (printf "/%s" $internalPvc.mountPath) -}} {{- if .Values.enabled -}} @@ -74,6 +75,22 @@ data: {{- end }} FS_SUB_PATHS: {{ $subPaths | toJson | quote }} {{- end }} + {{- if has "REDIS" $storage.cleanupStorageProviders }} + REDIS_DELETE_BATCH_SIZE: {{ $redis.delete.batchSize | default 1000 | quote }} + REDIS_HOST: {{ $redis.host | quote }} + REDIS_PORT: {{ $redis.port | default 6379 | quote }} + REDIS_DB: {{ $redis.db | default 0 | quote }} + REDIS_SCAN_COUNT: {{ $redis.delete.scanCount | default 1000 | quote }} + REDIS_TLS_ENABLED: {{ $redis.tlsEnabled | default false | quote }} + {{- with $redis.auth }} + {{- if .enabled }} + {{- if .username }} + REDIS_USERNAME: {{ .username | quote }} + {{- end }} + REDIS_PASSWORD: {{ .password | quote }} + {{- end }} + {{- end }} + {{- end }} {{- with .Values.env.strategies.tilesDeletion }} TILES_DELETION_BATCH_SIZE: {{ .batchSize | default 1000 | quote }} TILES_DELETION_CONCURRENCY: {{ .concurrency | default 10 | quote }} diff --git a/helm/values.yaml b/helm/values.yaml index 70f01e9..9a779e9 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -4,6 +4,7 @@ global: metrics: {} jobDefinitions: {} storage: {} + redis: {} serviceUrls: {} ca: {} @@ -13,7 +14,7 @@ serviceUrls: jobTracker: "" storage: - cleanupStorageProviders: {} + cleanupStorageProviders: [] # any of "FS", "S3", "REDIS" s3: delete: batchSize: 1000 @@ -30,6 +31,19 @@ storage: name: "" mountPath: "" tilesSubPath: "" # e.g. folder/tiles + redis: + host: "" + port: 6379 + db: 0 + # Same nesting as global.redis.auth; username is sent only when non-empty + auth: + enabled: false + username: "" + password: "" + tlsEnabled: false + delete: + batchSize: 1000 + scanCount: 1000 mclabels: component: backend @@ -115,6 +129,9 @@ env: queue: heartbeatIntervalMs: 1000 dequeueIntervalMs: 3000 + jobnik: + worker: + concurrency: 1 worker: capabilities: pairs: @@ -122,6 +139,10 @@ env: task: "tiles-deletion" - job: "Ingestion_Swap_Update" task: "tiles-deletion" + - job: "Update_Delete_Cache" + task: "tiles-deletion" + - job: "Swap_Delete_Cache" + task: "tiles-deletion" httpRetry: attempts: 3 delay: "exponential" diff --git a/package-lock.json b/package-lock.json index d97d035..06af458 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,13 +16,14 @@ "@map-colonies/js-logger": "^5.0.0", "@map-colonies/mc-priority-queue": "^9.1.2", "@map-colonies/mc-utils": "^6.0.1", - "@map-colonies/raster-shared": "^9.0.0-alpha.0", + "@map-colonies/raster-shared": "^9.0.0", "@map-colonies/read-pkg": "^1.0.0", "@map-colonies/schemas": "^1.20.0", "@map-colonies/telemetry": "^10.0.1", "@opentelemetry/api": "^1.9.0", "compression": "^1.8.0", "express": "^4.21.2", + "ioredis": "^6.0.0", "prom-client": "^15.1.3", "reflect-metadata": "^0.2.2", "tsyringe": "^4.8.0", @@ -2080,6 +2081,12 @@ "url": "https://github.com/sponsors/nzakas" } }, + "node_modules/@ioredis/commands": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@ioredis/commands/-/commands-2.0.0.tgz", + "integrity": "sha512-vrx0AE/T0h7cRZwfo1M39Cr+ZhZrkf0V8mQN75wucKCxCLD9l/VX6no3gFvrLqD1IlG/1LtzWovqEw3t0Vr9zg==", + "license": "MIT" + }, "node_modules/@isaacs/balanced-match": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/@isaacs/balanced-match/-/balanced-match-4.0.1.tgz", @@ -2561,9 +2568,9 @@ "license": "ISC" }, "node_modules/@map-colonies/raster-shared": { - "version": "9.0.0-alpha.0", - "resolved": "https://registry.npmjs.org/@map-colonies/raster-shared/-/raster-shared-9.0.0-alpha.0.tgz", - "integrity": "sha512-7NUVOMnlGaeSDYZu84wEXI25SyL/DatD/LHVBiGVP+CYWZ/8KjEe9WMH84EIPZS4VAlNpRbNZ770582AJNRjxQ==", + "version": "9.0.0", + "resolved": "https://registry.npmjs.org/@map-colonies/raster-shared/-/raster-shared-9.0.0.tgz", + "integrity": "sha512-RBrcPmVDm6i0/+odACjw/4HJYRuqWXwlJWnNXDm2sqamx74r+IxuTUYtRJHKktFwJc3r+g1YuT60VBeW9Lcaww==", "license": "ISC", "dependencies": { "@map-colonies/mc-priority-queue": "^9.1.0", @@ -9051,6 +9058,15 @@ "node": ">=12" } }, + "node_modules/cluster-key-slot": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/cluster-key-slot/-/cluster-key-slot-1.1.1.tgz", + "integrity": "sha512-rwHwUfXL40Chm1r08yrhU3qpUvdVlgkKNeyeGPOxnW8/SyVDvgRaed/Uz54AqWNaTCAThlj6QAs3TZcKI0xDEw==", + "license": "Apache-2.0", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/code-block-writer": { "version": "12.0.0", "resolved": "https://registry.npmjs.org/code-block-writer/-/code-block-writer-12.0.0.tgz", @@ -9709,6 +9725,15 @@ "node": ">=0.4.0" } }, + "node_modules/denque": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/denque/-/denque-2.1.0.tgz", + "integrity": "sha512-HVQE3AAb/pxF8fQAoiqpvg9i3evqug3hoiwakOyZAwJm+6vZehbkYXZ0l4JxS+I3QxM97v5aaRNhj8v5oBhekw==", + "license": "Apache-2.0", + "engines": { + "node": ">=0.10" + } + }, "node_modules/density-clustering": { "version": "1.3.0", "resolved": "https://registry.npmjs.org/density-clustering/-/density-clustering-1.3.0.tgz", @@ -11592,6 +11617,27 @@ "node": "^14.17.0 || ^16.13.0 || >=18.0.0" } }, + "node_modules/ioredis": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/ioredis/-/ioredis-6.0.0.tgz", + "integrity": "sha512-f+Dtubxfpf6KYFq7WVXJoOLn0bk4TJrMrN9SzeE+jrWrCWj7XX3fA6vkryafhADX+GMymRxgDJDOI33COkJc0w==", + "license": "MIT", + "dependencies": { + "@ioredis/commands": "2.0.0", + "cluster-key-slot": "1.1.1", + "debug": "4.4.3", + "denque": "2.1.0", + "redis-errors": "1.2.0", + "standard-as-callback": "2.1.0" + }, + "engines": { + "node": ">=20.0.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/ioredis" + } + }, "node_modules/ipaddr.js": { "version": "1.9.1", "resolved": "https://registry.npmjs.org/ipaddr.js/-/ipaddr.js-1.9.1.tgz", @@ -13851,6 +13897,15 @@ "node": ">= 12.13.0" } }, + "node_modules/redis-errors": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/redis-errors/-/redis-errors-1.2.0.tgz", + "integrity": "sha512-1qny3OExCf0UvUV/5wpYKf2YwPcOqXzkwKKSmKHiE6ZMQs5heeE/c8eXK+PNllPvmjgAbfnsbpkGZWy8cBpn9w==", + "license": "MIT", + "engines": { + "node": ">=4" + } + }, "node_modules/reflect-metadata": { "version": "0.2.2", "resolved": "https://registry.npmjs.org/reflect-metadata/-/reflect-metadata-0.2.2.tgz", @@ -14537,6 +14592,12 @@ "dev": true, "license": "MIT" }, + "node_modules/standard-as-callback": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/standard-as-callback/-/standard-as-callback-2.1.0.tgz", + "integrity": "sha512-qoRRSyROncaz1z0mvYqIE4lCd9p2R90i6GxW3uZv5ucSu8tU7B5HXUP1gG8pVZsYNVaXjk8ClXHPttLyxAL48A==", + "license": "MIT" + }, "node_modules/statuses": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", diff --git a/package.json b/package.json index 61e1672..df4beb8 100644 --- a/package.json +++ b/package.json @@ -39,13 +39,14 @@ "@map-colonies/js-logger": "^5.0.0", "@map-colonies/mc-priority-queue": "^9.1.2", "@map-colonies/mc-utils": "^6.0.1", - "@map-colonies/raster-shared": "^9.0.0-alpha.0", + "@map-colonies/raster-shared": "^9.0.0", "@map-colonies/read-pkg": "^1.0.0", "@map-colonies/schemas": "^1.20.0", "@map-colonies/telemetry": "^10.0.1", "@opentelemetry/api": "^1.9.0", "compression": "^1.8.0", "express": "^4.21.2", + "ioredis": "^6.0.0", "prom-client": "^15.1.3", "reflect-metadata": "^0.2.2", "tsyringe": "^4.8.0", diff --git a/src/cleaner/clients/index.ts b/src/cleaner/clients/index.ts new file mode 100644 index 0000000..9f80cd8 --- /dev/null +++ b/src/cleaner/clients/index.ts @@ -0,0 +1,2 @@ +export { createRedisConnection } from './redisClient'; +export { createS3Client } from './s3Client'; diff --git a/src/cleaner/clients/redisClient.ts b/src/cleaner/clients/redisClient.ts new file mode 100644 index 0000000..ef4d4e0 --- /dev/null +++ b/src/cleaner/clients/redisClient.ts @@ -0,0 +1,29 @@ +import type { Logger } from '@map-colonies/js-logger'; +// eslint-disable-next-line @typescript-eslint/naming-convention -- ioredis' default export is a class +import Redis from 'ioredis'; +import type { RedisStorageConfig } from '../storageProviders/storageConfig'; + +/** + * Connects to a standalone Redis. + * + * The Redis connection is a long-lived socket: it is opened here at startup so a bad host or + * credential fails the service at boot rather than on the first task, and closed by the + * `onSignal` shutdown hook. + */ +export async function createRedisConnection(config: RedisStorageConfig, logger: Logger): Promise { + const client = new Redis({ + host: config.host, + port: config.port, + db: config.db, + username: config.username, + password: config.password, + lazyConnect: true, + ...(config.tlsEnabled === true && { tls: {} }), + }); + + await client.connect(); + + logger.info({ msg: 'Connected to Redis', host: config.host, port: config.port, db: config.db }); + + return client; +} diff --git a/src/cleaner/clients/s3Client.ts b/src/cleaner/clients/s3Client.ts new file mode 100644 index 0000000..5258471 --- /dev/null +++ b/src/cleaner/clients/s3Client.ts @@ -0,0 +1,19 @@ +import { S3Client } from '@aws-sdk/client-s3'; +import type { S3StorageConfig } from '../storageProviders/storageConfig'; + +/** + * Builds the S3 client shared by every resolution of the S3 storage provider. + * Its sockets are released by the `onSignal` shutdown hook. + */ +export function createS3Client(config: S3StorageConfig): S3Client { + return new S3Client({ + endpoint: config.endpoint, + credentials: { + accessKeyId: config.accessKeyId, + secretAccessKey: config.secretAccessKey, + }, + forcePathStyle: config.forcePathStyle, + region: config.region, + tls: config.sslEnabled, + }); +} diff --git a/src/cleaner/storageProviders/failuresHandling.ts b/src/cleaner/storageProviders/failuresHandling.ts index 19c1d1d..1dc0cdf 100644 --- a/src/cleaner/storageProviders/failuresHandling.ts +++ b/src/cleaner/storageProviders/failuresHandling.ts @@ -13,6 +13,14 @@ export interface DeleteFailureSummary { samples: string[]; } +export const countFailures = (failures: DeleteFailure): number => { + let count = 0; + for (const { count: reasonCount } of failures.values()) { + count += reasonCount; + } + return count; +}; + export const mergeFailures = ({ source, target }: { source: DeleteFailure; target: DeleteFailure }): DeleteFailure => { const failures: DeleteFailure = structuredClone(target); @@ -24,17 +32,9 @@ export const mergeFailures = ({ source, target }: { source: DeleteFailure; targe return failures; }; -/** - * Reduces a list of provider delete failures into a compact, log-friendly - * shape. Lives alongside `IStorageProvider` because it operates purely on - * `DeleteResult` — any caller of `provider.delete()` can use it, regardless - * of which storage backend produced the failures. - */ -export function summarizeDeleteFailures({ failures }: DeleteResult): DeleteFailureSummary { - let failuresCount = 0; - for (const { count } of failures.values()) { - failuresCount += count; - } +/** Reduces delete failures into a compact, log-friendly shape, independent of the storage backend. */ +export function summarizeDeleteFailures({ failures }: Pick): DeleteFailureSummary { + const failuresCount = countFailures(failures); const sortedFailures = Array.from(failures.entries()).sort(([, { count: a }], [, { count: b }]) => b - a); const summary = sortedFailures.map(([reason, { count }]) => `${reason}=${count}`).join(', '); diff --git a/src/cleaner/storageProviders/fsStorageProvider.ts b/src/cleaner/storageProviders/fsStorageProvider.ts index d6667de..d46ec20 100644 --- a/src/cleaner/storageProviders/fsStorageProvider.ts +++ b/src/cleaner/storageProviders/fsStorageProvider.ts @@ -3,7 +3,14 @@ import { join } from 'node:path'; import type { Logger } from '@map-colonies/js-logger'; import type { DeleteStoredResourcesParams } from '@map-colonies/raster-shared'; import { inject, injectable } from 'tsyringe'; -import { mergeFailures, type DeleteFailure, type DeleteResult, type IStorageProvider, type StorageProvider } from '@src/cleaner/storageProviders'; +import { + countFailures, + mergeFailures, + type DeleteFailure, + type DeleteResult, + type IStorageProvider, + type StorageProvider, +} from '@src/cleaner/storageProviders'; import { getChunk, isPathWithinAllowedSubPaths, resolveAbsolutePath } from '@src/cleaner/utils'; import { SERVICES } from '@common/constants'; import { describeError, UnrecoverableError } from '../errors'; @@ -68,7 +75,7 @@ export class FsStorageProvider implements IStorageProvider<'FS'> { await this.cleanupEmptyDirs(paths, targetPath); - return { failures }; + return { failures, deletedCount: paths.length - countFailures(failures) }; } public async deleteResources({ @@ -103,8 +110,7 @@ export class FsStorageProvider implements IStorageProvider<'FS'> { } failures = mergeFailures({ source: chunkFailures, target: failures }); - let failedPathsCount = 0; - chunkFailures.forEach((chunkFailure) => (failedPathsCount += chunkFailure.count)); + const failedPathsCount = countFailures(chunkFailures); const deletedPathsCount = relativePathsChunk.length - failedPathsCount; totalDeletedPathsCount += deletedPathsCount; totalFailedPathsCount += failedPathsCount; @@ -119,7 +125,7 @@ export class FsStorageProvider implements IStorageProvider<'FS'> { await this.cleanupEmptyDirs(paths, resolveAbsolutePath(join(this.fsConfig.basePath, subPath))); - return { failures }; + return { failures, deletedCount: totalDeletedPathsCount }; } /** diff --git a/src/cleaner/storageProviders/iStorageProvider.ts b/src/cleaner/storageProviders/iStorageProvider.ts index 2afd85c..f8e68aa 100644 --- a/src/cleaner/storageProviders/iStorageProvider.ts +++ b/src/cleaner/storageProviders/iStorageProvider.ts @@ -5,8 +5,15 @@ import type { DeleteStoredResourcesParams, Storage } from '@map-colonies/raster- */ export type DeleteFailure = Map; +/** Where a deletion operates. Redis keys are flat under the prefix, so it has no relativePath. */ +export interface StorageTarget { + storageTarget: string; + relativePath?: string; +} + export interface DeleteResult { failures: DeleteFailure; + deletedCount: number; } export type StorageProvider = Storage['storageProvider']; @@ -17,9 +24,6 @@ export interface IStorageProvider { * - S3: storageTarget = bucket name; paths are object keys * - FS: storageTarget = sub path of the configured base path; the provider joins its own * base path and rejects anything falling outside the configured deletion sub paths - * - * Returns an object including delete failures aggregation with one entry per failed reason. - * "Not found" is reported as a failure with additional metadata on failure - count and sample */ delete: (storageTarget: string, paths: string[]) => Promise; @@ -35,8 +39,9 @@ export interface IStorageProvider { * - S3: storageTarget = bucket, relativePath = key prefix — lists objects (KeyCount > 0) * - FS: storageTarget = sub path of the configured base path, relativePath = subdirectory * below it — checks fs.stat, subject to the same sub path validation as `delete` + * Optional: a cache store (Redis) cannot tell a missing target from a cold cache, so it omits it. */ - targetExists: (storageTarget: string, relativePath: string) => Promise; + targetExists?: (storageTarget: string, relativePath: string) => Promise; } export type StorageProviders = { diff --git a/src/cleaner/storageProviders/index.ts b/src/cleaner/storageProviders/index.ts index 43f390e..d23f357 100644 --- a/src/cleaner/storageProviders/index.ts +++ b/src/cleaner/storageProviders/index.ts @@ -1,12 +1,24 @@ -export { mergeFailures, summarizeDeleteFailures, type DeleteFailureSummary } from './failuresHandling'; +export { countFailures, mergeFailures, summarizeDeleteFailures, type DeleteFailureSummary } from './failuresHandling'; export { FsStorageProvider } from './fsStorageProvider'; -export type { DeleteFailure, DeleteResult, IStorageProvider, StorageProvider, StorageProviders } from './iStorageProvider'; +export type { + DeleteFailure, + DeleteResult, + IStorageProvider, + ResolvedStorageProvider, + StorageProvider, + StorageProviders, + StorageTarget, +} from './iStorageProvider'; +export { RedisStorageProvider } from './redisStorageProvider'; export { S3StorageProvider } from './s3StorageProvider'; export { buildFsStorageConfig, + buildRedisStorageConfig, buildS3StorageConfig, type FsConfig, type FsStorageConfig, + type RedisConfig, + type RedisStorageConfig, type S3Config, type S3StorageConfig, } from './storageConfig'; diff --git a/src/cleaner/storageProviders/redisStorageProvider.ts b/src/cleaner/storageProviders/redisStorageProvider.ts new file mode 100644 index 0000000..d1776da --- /dev/null +++ b/src/cleaner/storageProviders/redisStorageProvider.ts @@ -0,0 +1,116 @@ +import type { Logger } from '@map-colonies/js-logger'; +import type { DeleteStoredResourcesParams } from '@map-colonies/raster-shared'; +// eslint-disable-next-line @typescript-eslint/naming-convention -- ioredis' default export is a class +import type Redis from 'ioredis'; +import { inject, injectable } from 'tsyringe'; +import { SERVICES } from '@common/constants'; +import { getChunk } from '@src/cleaner/utils'; +import { describeError } from '../errors'; +import { countFailures, mergeFailures } from './failuresHandling'; +import type { DeleteFailure, DeleteResult, IStorageProvider, StorageProvider } from './iStorageProvider'; +import type { RedisStorageConfig } from './storageConfig'; + +type RedisStorageProviderType = Extract; + +@injectable() +export class RedisStorageProvider implements IStorageProvider { + public constructor( + @inject(SERVICES.REDIS_STORAGE_CONFIG) private readonly redisConfig: RedisStorageConfig, + @inject(SERVICES.REDIS_CONNECTION) private readonly client: Redis, + @inject(SERVICES.LOGGER) private readonly logger: Logger + ) { + this.logger.debug({ + msg: 'Loaded Redis storage provider', + host: redisConfig.host, + port: redisConfig.port, + batchSize: redisConfig.batchSize, + }); + } + + public async delete(prefix: string, keys: string[]): Promise { + if (keys.length === 0) { + return { failures: new Map(), deletedCount: 0 }; + } + + const result = await this.unlinkInBatches(keys); + this.logger.debug({ + msg: 'Deleted keys from Redis', + prefix, + requestedCount: keys.length, + deletedCount: result.deletedCount, + failedCount: countFailures(result.failures), + }); + return result; + } + + public async deleteResources(params: Extract): Promise { + const pattern = this.matchPattern(params.prefix); + this.logger.info({ msg: 'Starting Redis prefix wipe', prefix: params.prefix, pattern }); + + let failures: DeleteFailure = new Map(); + let deletedCount = 0; + // May slightly overcount: SCAN can return a key twice while Redis is resizing its keyspace + let foundCount = 0; + + for await (const keys of this.scanKeys(pattern)) { + if (keys.length === 0) { + continue; + } + foundCount += keys.length; + const result = await this.unlinkInBatches(keys); + failures = mergeFailures({ source: result.failures, target: failures }); + deletedCount += result.deletedCount; + } + + this.logger.info({ + msg: `Completed Redis prefix wipe for prefix ${params.prefix} deleted ${deletedCount}/${foundCount} keys`, + prefix: params.prefix, + failedCount: countFailures(failures), + failedReasons: failures.size, + }); + return { failures, deletedCount }; + } + + private matchPattern(prefix: string): string { + return `${prefix}-*`; + } + + /** + * Walks the keyspace a page at a time, so nothing is buffered whole. + * + * `MATCH` filters after retrieval, so a page can come back empty while keys still remain. + * The loop therefore ends on the cursor returning to '0', never on an empty page. + */ + private async *scanKeys(pattern: string): AsyncGenerator { + let cursor = '0'; + do { + const [nextCursor, keys] = await this.client.scan(cursor, 'MATCH', pattern, 'COUNT', this.redisConfig.scanCount); + cursor = nextCursor; + yield keys; + } while (cursor !== '0'); + } + + /** + * One multi-key `UNLINK` per chunk, summing what Redis reports it removed. + * + * A chunk that throws is recorded whole against its reason: the command is atomic, so + * nothing in it was removed, and Redis gives no per-key attribution to do better. + */ + private async unlinkInBatches(keys: string[]): Promise> { + const failures: DeleteFailure = new Map(); + let deletedCount = 0; + + for (const chunk of getChunk(keys, this.redisConfig.batchSize)) { + try { + deletedCount += await this.client.unlink(...chunk); + } catch (error) { + const reason = describeError(error); + this.logger.error({ msg: 'Unlink chunk failed', reason, keysCount: chunk.length, err: error }); + const failure = failures.get(reason); + failures.set(reason, { count: (failure?.count ?? 0) + chunk.length, sample: failure?.sample ?? chunk[0]! }); + } + } + + return { failures, deletedCount }; + } +} diff --git a/src/cleaner/storageProviders/s3StorageProvider.ts b/src/cleaner/storageProviders/s3StorageProvider.ts index b1aa1fe..88f74ec 100644 --- a/src/cleaner/storageProviders/s3StorageProvider.ts +++ b/src/cleaner/storageProviders/s3StorageProvider.ts @@ -7,8 +7,8 @@ import { NoSuchBucket, NotFound, paginateListObjectsV2, - S3Client, S3ServiceException, + type S3Client, type _Error, type _Object, } from '@aws-sdk/client-s3'; @@ -16,7 +16,14 @@ import type { Logger } from '@map-colonies/js-logger'; import type { DeleteStoredResourcesParams } from '@map-colonies/raster-shared'; import { inject, injectable } from 'tsyringe'; import { SERVICES } from '@common/constants'; -import { mergeFailures, type DeleteFailure, type DeleteResult, type IStorageProvider, type StorageProvider } from '@src/cleaner/storageProviders'; +import { + countFailures, + mergeFailures, + type DeleteFailure, + type DeleteResult, + type IStorageProvider, + type StorageProvider, +} from '@src/cleaner/storageProviders'; import { getChunk, normalizeFolderPath } from '@src/cleaner/utils'; import { describeError, UnrecoverableError } from '../errors'; import type { S3StorageConfig } from './storageConfig'; @@ -25,23 +32,11 @@ type S3StorageProviderType = Extract; @injectable() export class S3StorageProvider implements IStorageProvider { - private readonly s3Client: S3Client; - public constructor( @inject(SERVICES.S3_STORAGE_CONFIG) private readonly s3Config: S3StorageConfig, + @inject(SERVICES.S3_CLIENT) private readonly s3Client: S3Client, @inject(SERVICES.LOGGER) private readonly logger: Logger ) { - // TODO: move client to a singleton resolution since - this.s3Client = new S3Client({ - endpoint: s3Config.endpoint, - credentials: { - accessKeyId: s3Config.accessKeyId, - secretAccessKey: s3Config.secretAccessKey, - }, - forcePathStyle: s3Config.forcePathStyle, - region: s3Config.region, - tls: s3Config.sslEnabled, - }); this.logger.debug({ msg: 'Loaded S3 storage provider', endpoint: s3Config.endpoint, batchSize: this.s3Config.batchSize }); } @@ -54,7 +49,7 @@ export class S3StorageProvider implements IStorageProvider): Promise { this.logger.debug({ msg: `Starting S3 resources deletion`, bucket, pathsCount: paths.length }); let failures: DeleteFailure = new Map(); + let deletedCount = 0; - if (paths.length === 0) return { failures }; + if (paths.length === 0) return { failures, deletedCount }; if (paths.some((path) => path.length === 0)) throw new UnrecoverableError('Cannot delete resources directly under root path of the bucket'); // Prevent root deletion const exists = await this.bucketExists(bucket); @@ -73,11 +69,12 @@ export class S3StorageProvider implements IStorageProvider { @@ -146,7 +143,7 @@ export class S3StorageProvider implements IStorageProvider { + private async deleteResource({ bucket, path }: { bucket: string; path: string }): Promise { this.logger.debug({ msg: 'Deleting a resource', bucket, path }); let failures: DeleteFailure = new Map(); let totalDeletedObjectsCount = 0, @@ -177,8 +174,7 @@ export class S3StorageProvider implements IStorageProvider (failedObjectsCount += chunkFailure.count)); + const failedObjectsCount = countFailures(chunkFailures); const deletedObjectsCount = keys.length - failedObjectsCount; totalDeletedObjectsCount += deletedObjectsCount; totalFailedObjectsCount += failedObjectsCount; @@ -192,7 +188,7 @@ export class S3StorageProvider implements IStorageProvider { + this.logger.info({ msg: 'Waiting for the reload window before deleting', prefix, delaySeconds }); + await setTimeout(delaySeconds * MS_PER_SECOND); + } + private resolveStorageProvider(storageProvider: K): IStorageProvider { this.logger.debug({ msg: `Resolving storage provider`, provider: storageProvider, providers: Object.keys(this.storageProviders) }); const provider = this.storageProviders[storageProvider]; diff --git a/src/cleaner/strategies/tilesDeletionStrategy.ts b/src/cleaner/strategies/tilesDeletionStrategy.ts index 839abfc..2706725 100644 --- a/src/cleaner/strategies/tilesDeletionStrategy.ts +++ b/src/cleaner/strategies/tilesDeletionStrategy.ts @@ -5,18 +5,22 @@ import { StorageProvider, TilesDeletionParams, tilesDeletionParamsSchema } from import { inject, injectable } from 'tsyringe'; import type { ConfigType } from '@common/config'; import { PERCENTAGE_COMPLETE, SERVICES } from '@common/constants'; -import { mergeFailures, summarizeDeleteFailures, type DeleteFailure, type StorageProviders } from '@src/cleaner/storageProviders'; +import { + countFailures, + mergeFailures, + summarizeDeleteFailures, + type DeleteFailure, + type ResolvedStorageProvider, + type StorageProviders, + type StorageTarget, +} from '@src/cleaner/storageProviders'; import { RecoverableError, UnrecoverableError, describeError } from '../errors'; -import { ResolvedStorageProvider } from '../storageProviders/iStorageProvider'; import { resolveTileKeyGenerator, validateSchema } from '../utils'; import type { TaskContext } from './strategyFactory'; import type { ITaskStrategy } from './taskStrategy'; const NOT_FOUND_REASONS = new Set([NoSuchKey.name, 'ENOENT']); -/** Redis tiles deletion is not implemented yet (MAPCO-11263). */ -type SupportedTilesDeletionParams = Exclude; - @injectable() export class TilesDeletionStrategy implements ITaskStrategy { private readonly batchSize: number; @@ -39,15 +43,8 @@ export class TilesDeletionStrategy implements ITaskStrategy } public async execute(params: TilesDeletionParams): Promise { - if (params.storageProvider === StorageProvider.REDIS) { - throw new UnrecoverableError(`Tiles deletion is not implemented for ${StorageProvider.REDIS} storage`); - } - - const { provider, storageTarget } = this.resolveStorageProvider(params); - - if (!(await provider.targetExists(storageTarget, params.tilesRelativePath))) { - throw new UnrecoverableError(`${params.storageProvider} storage target does not exist: ${storageTarget}/${params.tilesRelativePath}`); - } + const { provider, storageTarget, relativePath } = this.resolveStorageProvider(params); + await this.assertTargetExists(provider, storageTarget, relativePath); const totalTiles = this.countTiles(params); @@ -59,8 +56,8 @@ export class TilesDeletionStrategy implements ITaskStrategy totalTiles, }); - const failures = await this.deleteTiles(provider, storageTarget, params, totalTiles); - this.reportOutcome(failures, totalTiles); + const { failures, deletedCount } = await this.deleteTiles(provider, storageTarget, params, totalTiles); + this.reportOutcome(failures, totalTiles, deletedCount); } /** @@ -70,18 +67,13 @@ export class TilesDeletionStrategy implements ITaskStrategy * matches the desired end-state) but counted separately for visibility. * Terminal progress to 100% is handled by the queue's task-ack — no explicit call needed here. */ - private reportOutcome(failures: DeleteFailure, totalTiles: number): void { + private reportOutcome(failures: DeleteFailure, totalTiles: number, deletedCount: number): void { const retryable: DeleteFailure = new Map(); const notFound: DeleteFailure = new Map(); - for (const failure of failures) { - (NOT_FOUND_REASONS.has(failure[0]) ? notFound : retryable).set(failure[0], failure[1]); + for (const [reason, failure] of failures) { + (NOT_FOUND_REASONS.has(reason) ? notFound : retryable).set(reason, failure); } - - let retryableCount = 0; - retryable.forEach((retryableFailure) => (retryableCount += retryableFailure.count)); - let notFoundCount = 0; - notFound.forEach((notFoundFailure) => (notFoundCount += notFoundFailure.count)); - const deletedCount = totalTiles - retryableCount - notFoundCount; + const notFoundCount = countFailures(notFound); if (retryable.size > 0) { const { failuresCount, samples, summary } = summarizeDeleteFailures({ failures: retryable }); @@ -102,36 +94,57 @@ export class TilesDeletionStrategy implements ITaskStrategy this.logger.warn({ msg: 'Tiles deletion completed with missing tiles', totalTiles, - notFoundCount: notFound.size, + notFoundCount, deletedCount, - allTilesMissing: notFound.size === totalTiles, + allTilesMissing: notFoundCount === totalTiles, }); return; } - this.logger.info({ msg: 'Tiles deletion completed successfully', deletedCount: totalTiles }); + this.logger.info({ msg: 'Tiles deletion completed successfully', totalTiles, deletedCount }); } - private resolveStorageProvider(params: SupportedTilesDeletionParams): { provider: ResolvedStorageProvider; storageTarget: string } { + private resolveStorageProvider(params: TilesDeletionParams): StorageTarget & { provider: ResolvedStorageProvider } { // eslint-disable-next-line @typescript-eslint/naming-convention const storageProvider = this.storageProviders[params.storageProvider]; if (storageProvider === undefined) throw new UnrecoverableError(`Unsupported storage provider ${params.storageProvider}`); - const storageTarget = params.storageProvider === StorageProvider.S3 ? params.bucket : params.subPath; - this.logger.debug({ msg: `Using ${params.storageProvider} provider`, storageTarget }); - return { provider: storageProvider, storageTarget }; + const target = this.resolveTarget(params); + this.logger.debug({ msg: `Using ${params.storageProvider} provider`, ...target }); + return { provider: storageProvider, ...target }; + } + + private resolveTarget(params: TilesDeletionParams): StorageTarget { + switch (params.storageProvider) { + case StorageProvider.S3: + return { storageTarget: params.bucket, relativePath: params.tilesRelativePath }; + case StorageProvider.FS: + return { storageTarget: params.subPath, relativePath: params.tilesRelativePath }; + case StorageProvider.REDIS: + return { storageTarget: params.prefix }; + } + } + + private async assertTargetExists(provider: ResolvedStorageProvider, storageTarget: string, relativePath?: string): Promise { + if (provider.targetExists === undefined || relativePath === undefined) { + return; + } + if (!(await provider.targetExists(storageTarget, relativePath))) { + throw new UnrecoverableError(`Tiles storage target does not exist: ${storageTarget}/${relativePath}`); + } } private async deleteTiles( provider: ResolvedStorageProvider, storageTarget: string, - params: SupportedTilesDeletionParams, + params: TilesDeletionParams, totalTiles: number - ): Promise { + ): Promise<{ failures: DeleteFailure; deletedCount: number }> { const { jobId, taskId } = this.taskContext; let failures: DeleteFailure = new Map(); const pendingBatches: string[][] = []; let batch: string[] = []; let processedTiles = 0; + let deletedCount = 0; for (const tileKey of this.generateTileKeys(params)) { batch.push(tileKey); @@ -139,9 +152,10 @@ export class TilesDeletionStrategy implements ITaskStrategy pendingBatches.push(batch); batch = []; if (pendingBatches.length === this.concurrency) { - const { batchFailures, processedTilesCount } = await this.flushBatches(provider, storageTarget, pendingBatches); - processedTiles += processedTilesCount; - failures = mergeFailures({ source: batchFailures, target: failures }); + const flushed = await this.flushBatches(provider, storageTarget, pendingBatches); + processedTiles += flushed.processedTilesCount; + deletedCount += flushed.deletedCount; + failures = mergeFailures({ source: flushed.batchFailures, target: failures }); const percentage = Math.round((processedTiles / totalTiles) * PERCENTAGE_COMPLETE); await this.queueClient.updateProgress(jobId, taskId, percentage); this.logger.info({ msg: 'Tiles deletion progress', deletionProgress: `${processedTiles}/${totalTiles}`, failedTiles: failures.size }); @@ -153,13 +167,14 @@ export class TilesDeletionStrategy implements ITaskStrategy pendingBatches.push(batch); } if (pendingBatches.length > 0) { - const { batchFailures, processedTilesCount } = await this.flushBatches(provider, storageTarget, pendingBatches); - processedTiles += processedTilesCount; - failures = mergeFailures({ source: batchFailures, target: failures }); + const flushed = await this.flushBatches(provider, storageTarget, pendingBatches); + processedTiles += flushed.processedTilesCount; + deletedCount += flushed.deletedCount; + failures = mergeFailures({ source: flushed.batchFailures, target: failures }); this.logger.info({ msg: 'Tiles deletion progress', deletionProgress: `${processedTiles}/${totalTiles}`, failedTiles: failures.size }); } - return failures; + return { failures, deletedCount }; } /** @@ -169,21 +184,21 @@ export class TilesDeletionStrategy implements ITaskStrategy * the reason) are both collected so nothing is silently lost and every failed path * carries a cause the caller can surface in the task rejection reason. * pendingBatches is cleared in-place for reuse. - * - * @returns Total tile paths attempted (not necessarily deleted). */ private async flushBatches( provider: ResolvedStorageProvider, storageTarget: string, pendingBatches: string[][] - ): Promise<{ batchFailures: DeleteFailure; processedTilesCount: number }> { + ): Promise<{ batchFailures: DeleteFailure; processedTilesCount: number; deletedCount: number }> { let failures: DeleteFailure = new Map(); + let deletedCount = 0; const processedTilesCount = pendingBatches.reduce((sum, b) => sum + b.length, 0); const results = await Promise.allSettled(pendingBatches.map(async (batch) => provider.delete(storageTarget, batch))); for (const [index, result] of results.entries()) { if (result.status === 'fulfilled') { failures = mergeFailures({ source: result.value.failures, target: failures }); + deletedCount += result.value.deletedCount; } else { const error: unknown = result.reason; const reason = describeError(error); @@ -194,7 +209,7 @@ export class TilesDeletionStrategy implements ITaskStrategy } } pendingBatches.length = 0; - return { batchFailures: failures, processedTilesCount }; + return { batchFailures: failures, processedTilesCount, deletedCount }; } /** @@ -202,11 +217,11 @@ export class TilesDeletionStrategy implements ITaskStrategy * For each range, the tile count is the product of the width (maxX - minX + 1) * and height (maxY - minY + 1) of the range grid. */ - private countTiles(params: SupportedTilesDeletionParams): number { + private countTiles(params: TilesDeletionParams): number { return params.ranges.reduce((sum, r) => sum + (r.maxX - r.minX + 1) * (r.maxY - r.minY + 1), 0); } - private *generateTileKeys(params: SupportedTilesDeletionParams): Generator { + private *generateTileKeys(params: TilesDeletionParams): Generator { const toKeys = resolveTileKeyGenerator(params); for (const range of params.ranges) { diff --git a/src/common/constants.ts b/src/common/constants.ts index a9be61f..dc12f0b 100644 --- a/src/common/constants.ts +++ b/src/common/constants.ts @@ -21,6 +21,9 @@ export const SERVICES = { STORAGE_PROVIDERS: Symbol('StorageProviders'), FS_STORAGE_CONFIG: Symbol('FsStorageConfig'), S3_STORAGE_CONFIG: Symbol('S3StorageConfig'), + S3_CLIENT: Symbol('S3Client'), + REDIS_STORAGE_CONFIG: Symbol('RedisStorageConfig'), + REDIS_CONNECTION: Symbol('RedisConnection'), TASK_CONTEXT: Symbol('TaskContext'), JOB_TRACKER_CLIENT: Symbol('JobTrackerClient'), // ============================================================================= @@ -32,3 +35,4 @@ export const SERVICES = { } satisfies Record; export const PERCENTAGE_COMPLETE = 100; +export const MS_PER_SECOND = 1000; diff --git a/src/containerConfig.ts b/src/containerConfig.ts index f7826d3..1f372bf 100644 --- a/src/containerConfig.ts +++ b/src/containerConfig.ts @@ -1,7 +1,7 @@ import { IWorker, JobnikSDK } from '@map-colonies/jobnik-sdk'; import { jsLogger, type Logger } from '@map-colonies/js-logger'; import { TaskHandler as QueueClient } from '@map-colonies/mc-priority-queue'; -import { SourceType } from '@map-colonies/raster-shared'; +import { SourceType, StorageProvider } from '@map-colonies/raster-shared'; import { getOtelMixin } from '@map-colonies/telemetry'; import { trace } from '@opentelemetry/api'; import { Registry } from 'prom-client'; @@ -11,9 +11,17 @@ import { SERVICE_NAME, SERVICES } from '@common/constants'; import { getJobAndTaskToken, InjectionObject, registerDependencies } from '@common/dependencyRegistration'; import { getTracing } from '@common/tracing'; import type { StorageProviders } from '@src/cleaner/storageProviders'; +import { createRedisConnection, createS3Client } from './cleaner/clients'; import { ErrorHandler } from './cleaner/errors'; import { JobTrackerClient } from './cleaner/httpClients'; -import { buildFsStorageConfig, buildS3StorageConfig, FsStorageProvider, S3StorageProvider } from './cleaner/storageProviders'; +import { + buildFsStorageConfig, + buildRedisStorageConfig, + buildS3StorageConfig, + FsStorageProvider, + RedisStorageProvider, + S3StorageProvider, +} from './cleaner/storageProviders'; import { DeleteStoredResourcesStrategy, StrategyFactory, TilesDeletionStrategy } from './cleaner/strategies'; import type { QueueConfig } from './cleaner/types'; import { ConfigType, getConfig } from './common/config'; @@ -39,6 +47,9 @@ export const registerExternalValues = async (options?: RegisterOptions): Promise const cleanupStorageProviders = configInstance.get('storage.cleanupStorageProviders') as unknown as string[]; const fsStorageConfig = cleanupStorageProviders.includes(SourceType.FS) ? buildFsStorageConfig(configInstance, logger) : undefined; const s3StorageConfig = cleanupStorageProviders.includes(SourceType.S3) ? buildS3StorageConfig(configInstance, logger) : undefined; + const redisStorageConfig = cleanupStorageProviders.includes(StorageProvider.REDIS) ? buildRedisStorageConfig(configInstance, logger) : undefined; + const s3Client = s3StorageConfig ? createS3Client(s3StorageConfig) : undefined; + const redisConnection = redisStorageConfig ? await createRedisConnection(redisStorageConfig, logger) : undefined; const dependencies: InjectionObject[] = [ { token: SERVICES.CONFIG, provider: { useValue: configInstance } }, @@ -105,13 +116,17 @@ export const registerExternalValues = async (options?: RegisterOptions): Promise }, ...(fsStorageConfig ? [{ token: SERVICES.FS_STORAGE_CONFIG, provider: { useValue: fsStorageConfig } }] : []), ...(s3StorageConfig ? [{ token: SERVICES.S3_STORAGE_CONFIG, provider: { useValue: s3StorageConfig } }] : []), + ...(s3Client ? [{ token: SERVICES.S3_CLIENT, provider: { useValue: s3Client } }] : []), + ...(redisStorageConfig ? [{ token: SERVICES.REDIS_STORAGE_CONFIG, provider: { useValue: redisStorageConfig } }] : []), + ...(redisConnection ? [{ token: SERVICES.REDIS_CONNECTION, provider: { useValue: redisConnection } }] : []), { token: SERVICES.STORAGE_PROVIDERS, provider: { useFactory: instancePerContainerCachingFactory((container) => { const providers = { - ...(s3StorageConfig && { [SourceType.S3]: container.resolve(S3StorageProvider) }), + ...(s3Client && { [SourceType.S3]: container.resolve(S3StorageProvider) }), ...(fsStorageConfig && { [SourceType.FS]: container.resolve(FsStorageProvider) }), + ...(redisConnection && { [StorageProvider.REDIS]: container.resolve(RedisStorageProvider) }), }; return providers; }), @@ -158,13 +173,36 @@ export const registerExternalValues = async (options?: RegisterOptions): Promise useClass: DeleteStoredResourcesStrategy, }, }, + // Redis cache invalidation created by overseer after an ingestion finalizes: an update deletes the + // tile ranges of the ingested footprint, a swap wipes the whole cache prefix. + { + token: getJobAndTaskToken({ + //TODO: when we create worker config schema we can move this to a constant and remove the cast + jobType: configInstance.get('jobDefinitions.jobs.updateCacheDeletion.type') as unknown as string, + taskType: configInstance.get('jobDefinitions.tasks.tilesDeletion.type') as unknown as string, + }), + provider: { + useClass: TilesDeletionStrategy, + }, + }, + { + token: getJobAndTaskToken({ + //TODO: when we create worker config schema we can move this to a constant and remove the cast + jobType: configInstance.get('jobDefinitions.jobs.swapCacheDeletion.type') as unknown as string, + taskType: configInstance.get('jobDefinitions.tasks.tilesDeletion.type') as unknown as string, + }), + provider: { + useClass: DeleteStoredResourcesStrategy, + }, + }, { token: 'onSignal', provider: { useFactory: (container) => { const worker = container.resolve(SERVICES.WORKER); return async (): Promise => { - await Promise.all([getTracing().stop(), worker.stop()]); + await Promise.all([getTracing().stop(), worker.stop(), redisConnection?.quit()]); + s3Client?.destroy(); }; }, }, diff --git a/tests/clients/redisClient.spec.ts b/tests/clients/redisClient.spec.ts new file mode 100644 index 0000000..6b7476a --- /dev/null +++ b/tests/clients/redisClient.spec.ts @@ -0,0 +1,65 @@ +import type { Logger } from '@map-colonies/js-logger'; +// eslint-disable-next-line @typescript-eslint/naming-convention -- ioredis' default export is a class +import Redis from 'ioredis'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { createRedisConnection } from '@src/cleaner/clients/redisClient'; +import { createMockLogger, createRedisStorageConfig } from '../helpers/mocks'; + +const { redisConstructor, connect } = vi.hoisted(() => ({ + redisConstructor: vi.fn(), + connect: vi.fn(), +})); + +vi.mock('ioredis', () => ({ + default: class { + public connect = connect; + public constructor(options: unknown) { + redisConstructor(options); + } + }, +})); + +describe('createRedisConnection', () => { + let mockLogger: Logger; + + beforeEach(() => { + vi.clearAllMocks(); + mockLogger = createMockLogger(); + connect.mockResolvedValue(undefined); + }); + + describe('connecting', () => { + it('should connect once at startup and return the connected Redis client', async () => { + const client = await createRedisConnection(createRedisStorageConfig(), mockLogger); + + expect(connect).toHaveBeenCalledTimes(1); + expect(client).toBeInstanceOf(Redis); + }); + + it('should build the client lazily with the configured connection details', async () => { + await createRedisConnection(createRedisStorageConfig({ host: 'redis.local', port: 6380, db: 2 }), mockLogger); + + expect(redisConstructor).toHaveBeenCalledWith(expect.objectContaining({ host: 'redis.local', port: 6380, db: 2, lazyConnect: true })); + }); + + it('should pass credentials through to the client', async () => { + await createRedisConnection(createRedisStorageConfig({ username: 'user', password: 'secret' }), mockLogger); + + expect(redisConstructor).toHaveBeenCalledWith(expect.objectContaining({ username: 'user', password: 'secret' })); + }); + + it('should enable tls when configured', async () => { + await createRedisConnection(createRedisStorageConfig({ tlsEnabled: true }), mockLogger); + + expect(redisConstructor).toHaveBeenCalledWith(expect.objectContaining({ tls: {} })); + }); + }); + + describe('connection failure', () => { + it('should propagate the error so startup fails fast rather than on the first task', async () => { + connect.mockRejectedValue(new Error('ECONNREFUSED')); + + await expect(createRedisConnection(createRedisStorageConfig(), mockLogger)).rejects.toThrow('ECONNREFUSED'); + }); + }); +}); diff --git a/tests/clients/s3Client.spec.ts b/tests/clients/s3Client.spec.ts new file mode 100644 index 0000000..b4c522a --- /dev/null +++ b/tests/clients/s3Client.spec.ts @@ -0,0 +1,29 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import { S3Client } from '@aws-sdk/client-s3'; +import { describe, expect, it, vi } from 'vitest'; +import { createS3Client } from '@src/cleaner/clients/s3Client'; +import { createS3StorageConfig, S3_VALIDATED_CONFIG_DEFAULTS } from '../helpers/mocks'; + +vi.mock(import('@aws-sdk/client-s3'), async (importOriginal) => { + const originModule = await importOriginal(); + return { ...originModule, S3Client: vi.fn() as unknown as typeof S3Client }; +}); + +describe('createS3Client', () => { + it('should construct S3Client with config values', () => { + createS3Client(createS3StorageConfig()); + + expect(S3Client).toHaveBeenCalledWith( + expect.objectContaining({ + credentials: { + accessKeyId: S3_VALIDATED_CONFIG_DEFAULTS.accessKeyId, + secretAccessKey: S3_VALIDATED_CONFIG_DEFAULTS.secretAccessKey, + }, + endpoint: S3_VALIDATED_CONFIG_DEFAULTS.endpoint, + forcePathStyle: S3_VALIDATED_CONFIG_DEFAULTS.forcePathStyle, + region: S3_VALIDATED_CONFIG_DEFAULTS.region, + tls: S3_VALIDATED_CONFIG_DEFAULTS.sslEnabled, + }) + ); + }); +}); diff --git a/tests/helpers/fakes/tilesDeletionFakes.ts b/tests/helpers/fakes/tilesDeletionFakes.ts index a290e1b..650c031 100644 --- a/tests/helpers/fakes/tilesDeletionFakes.ts +++ b/tests/helpers/fakes/tilesDeletionFakes.ts @@ -1,5 +1,11 @@ import { faker } from '@faker-js/faker'; -import { StorageProvider, type FsTilesDeletionParams, type S3TilesDeletionParams, type TileRange } from '@map-colonies/raster-shared'; +import { + StorageProvider, + type FsTilesDeletionParams, + type RedisTilesDeletionParams, + type S3TilesDeletionParams, + type TileRange, +} from '@map-colonies/raster-shared'; const EXTENSIONS = ['png', 'jpeg'] as const; @@ -25,6 +31,11 @@ function buildTilesRelativePath(): string { return `${faker.word.noun().toLowerCase()}/${faker.string.alphanumeric({ length: 6, casing: 'lower' })}`; } +/** Mirrors the observed `{layer}-{productType}-{grid}` cache prefix, dashes included. */ +function buildRedisPrefix(): string { + return `${faker.string.alphanumeric({ length: 8, casing: 'lower' })}-Orthophoto-WorldCRS84`; +} + function buildTilesDeletionCommon(overrides: Partial): TilesDeletionCommon { return { tilesRelativePath: overrides.tilesRelativePath ?? buildTilesRelativePath(), @@ -49,5 +60,20 @@ function buildFsTilesDeletionParams(overrides: Partial = }; } -export { buildTileRange, buildTilesRelativePath, buildS3TilesDeletionParams, buildFsTilesDeletionParams }; +function buildRedisTilesDeletionParams(overrides: Partial = {}): RedisTilesDeletionParams { + return { + storageProvider: StorageProvider.REDIS, + prefix: overrides.prefix ?? buildRedisPrefix(), + ranges: overrides.ranges ?? [buildTileRange()], + }; +} + +export { + buildTileRange, + buildTilesRelativePath, + buildRedisPrefix, + buildS3TilesDeletionParams, + buildFsTilesDeletionParams, + buildRedisTilesDeletionParams, +}; export type { TilesDeletionCommon }; diff --git a/tests/helpers/mocks.ts b/tests/helpers/mocks.ts index 9f14b46..8395a57 100644 --- a/tests/helpers/mocks.ts +++ b/tests/helpers/mocks.ts @@ -1,8 +1,17 @@ import type { Logger } from '@map-colonies/js-logger'; import type { TaskHandler as QueueClient } from '@map-colonies/mc-priority-queue'; -import { vi } from 'vitest'; +// eslint-disable-next-line @typescript-eslint/naming-convention -- ioredis' default export is a class +import type Redis from 'ioredis'; +import { vi, type Mock } from 'vitest'; import type { IStorageProvider, StorageProvider } from '@src/cleaner/storageProviders/iStorageProvider'; -import type { FsConfig, FsStorageConfig, S3Config, S3StorageConfig } from '@src/cleaner/storageProviders/storageConfig'; +import type { + FsConfig, + FsStorageConfig, + RedisConfig, + RedisStorageConfig, + S3Config, + S3StorageConfig, +} from '@src/cleaner/storageProviders/storageConfig'; import type { ErrorHandler } from '../../src/cleaner/errors'; import type { JobTrackerClient } from '../../src/cleaner/httpClients'; import type { ITaskStrategy, StrategyFactory } from '../../src/cleaner/strategies'; @@ -67,11 +76,13 @@ export function createMockErrorHandler(defaultDecision: ErrorDecision = { should // ─── StorageProvider ───────────────────────────────────────────────────────── -export function createMockStorageProvider(): IStorageProvider { +export function createMockStorageProvider(): Required>; +export function createMockStorageProvider(options: { targetExists: false }): IStorageProvider; +export function createMockStorageProvider({ targetExists = true } = {}): IStorageProvider { return { - delete: vi.fn().mockResolvedValue({ failures: new Map() }), - deleteResources: vi.fn().mockResolvedValue({ failures: new Map() }), - targetExists: vi.fn().mockResolvedValue(true), + delete: vi.fn().mockResolvedValue({ failures: new Map(), deletedCount: 0 }), + deleteResources: vi.fn().mockResolvedValue({ failures: new Map(), deletedCount: 0 }), + ...(targetExists && { targetExists: vi.fn().mockResolvedValue(true) }), }; } @@ -168,6 +179,79 @@ export function createFsStorageConfig(overrides: Partial = {}): return { ...FS_VALIDATED_CONFIG_DEFAULTS, ...overrides }; } +// ─── Redis Storage Config (RedisStorageProvider) ───────────────────────────── + +export const REDIS_STORAGE_CONFIG_DEFAULTS = { + delete: { + batchSize: 3, + scanCount: 10, + }, + host: 'localhost', + port: 6379, + db: 0, +} as const satisfies RedisConfig; + +export function createMockRedisConfig(overrides: Record = {}): ConfigType { + return { + get: vi.fn().mockReturnValue({ ...REDIS_STORAGE_CONFIG_DEFAULTS, ...overrides }), + } as unknown as ConfigType; +} + +// ─── Redis Validated Storage Config ────────────────────────────────────────── + +export const REDIS_VALIDATED_CONFIG_DEFAULTS = { + host: REDIS_STORAGE_CONFIG_DEFAULTS.host, + port: REDIS_STORAGE_CONFIG_DEFAULTS.port, + db: REDIS_STORAGE_CONFIG_DEFAULTS.db, + scanCount: REDIS_STORAGE_CONFIG_DEFAULTS.delete.scanCount, + batchSize: REDIS_STORAGE_CONFIG_DEFAULTS.delete.batchSize, +} as const satisfies RedisStorageConfig; + +export function createRedisStorageConfig(overrides: Partial = {}): RedisStorageConfig { + return { ...REDIS_VALIDATED_CONFIG_DEFAULTS, ...overrides }; +} + +// ─── Redis Client (RedisStorageProvider) ───────────────────────────────────── + +/** + * A mock Redis client recording every key it was asked to unlink. Cast at the boundary with `asRedis`; + * RedisStorageProvider only ever touches `scan` and `unlink`. + */ +export interface MockRedisClient { + unlinked: string[]; + scan: Mock; + unlink: Mock; +} + +export function createMockRedisClient(): MockRedisClient { + const unlinked: string[] = []; + return { + unlinked, + scan: vi.fn().mockResolvedValue(['0', []]), + unlink: vi.fn().mockImplementation(async (...keys: string[]) => { + unlinked.push(...keys); + return Promise.resolve(keys.length); + }), + }; +} + +export function asRedis(client: MockRedisClient): Redis { + return client as unknown as Redis; +} + +/** A client whose SCAN walks `pages` in order, returning to cursor '0' only on the last one. */ +export function createMockScanningRedisClient(pages: string[][]): MockRedisClient { + const client = createMockRedisClient(); + let call = 0; + client.scan = vi.fn().mockImplementation(async () => { + const page = pages[call] ?? []; + call += 1; + const cursor = call >= pages.length ? '0' : String(call); + return Promise.resolve([cursor, page] as [string, string[]]); + }); + return client; +} + // ─── JobTrackerClient ───────────────────────────────────────────────────────── export function createMockJobTrackerClient(): JobTrackerClient { diff --git a/tests/integration/deleteStoredResourcesStrategy.integration.spec.ts b/tests/integration/deleteStoredResourcesStrategy.integration.spec.ts new file mode 100644 index 0000000..d49180a --- /dev/null +++ b/tests/integration/deleteStoredResourcesStrategy.integration.spec.ts @@ -0,0 +1,74 @@ +import { faker } from '@faker-js/faker'; +import { describe, expect, it } from 'vitest'; +import { useStorageBackends } from './helpers/backendFixtures'; +import { sadPathProviders } from './helpers/storageBackends'; +import { storedResourcesBackends, type ResourceBackend } from './helpers/storedResourcesBackends'; +import { + expectTaskAcked, + expectTaskRejectedUnrecoverable, + runTask, + storedResourcesDeletionUnderTest, + type RunTaskParams, +} from './helpers/testPoller'; + +const ZOOM = 10; + +const layerName = (): string => `layer-${faker.string.alphanumeric({ length: 8, casing: 'lower' })}`; +const layerTiles = (backend: ResourceBackend, layer: string, count: number): string[] => + Array.from({ length: count }, (_, i) => backend.tileKey(layer, ZOOM, i, i + 1)); + +describe('stored resources deletion E2E (polling → strategy → real provider → ack)', () => { + const harness = useStorageBackends(); + const { storageContext } = harness; + + describe.each(storedResourcesBackends(harness))('$storageProvider provider', (backend) => { + const strategyUnderTest = storedResourcesDeletionUnderTest(backend.storageProvider); + const runStoredResourcesDeletion = async (params: unknown, overrides: Partial = {}): ReturnType => + runTask({ strategyUnderTest, providers: storageContext().providers, params, ...overrides }); + + describe('happy path', () => { + it('polls the task, wipes the whole layer, and acks while leaving other layers intact', async () => { + const target = layerName(); + const survivors = layerTiles(backend, layerName(), 3); + // 25 tiles is enough to cross the small Redis SCAN and UNLINK page sizes the fixtures configure. + await backend.seed([...layerTiles(backend, target, 25), ...survivors]); + + const run = await runStoredResourcesDeletion(backend.buildParams([target])); + + expect(await backend.list()).toEqual([...survivors].sort()); + expectTaskAcked(run); + }); + + it('acks a task for a layer that holds nothing, so redelivery is idempotent', async () => { + const survivors = layerTiles(backend, layerName(), 2); + await backend.seed(survivors); + + const run = await runStoredResourcesDeletion(backend.buildParams([layerName()])); + + expect(await backend.list()).toEqual([...survivors].sort()); + expectTaskAcked(run); + }); + + it.runIf(backend.supportsMultipleLayers)('wipes every listed layer in one task and nothing else', async () => { + const targets = [layerName(), layerName()]; + const survivors = layerTiles(backend, layerName(), 2); + await backend.seed([...targets.flatMap((layer) => layerTiles(backend, layer, 4)), ...survivors]); + + const run = await runStoredResourcesDeletion(backend.buildParams(targets)); + + expect(await backend.list()).toEqual([...survivors].sort()); + expectTaskAcked(run); + }); + }); + + describe('sad path', () => { + it('rejects the task as unrecoverable without acking', async () => { + const { params, reason } = backend.sadPath; + + const run = await runStoredResourcesDeletion(params(), { providers: sadPathProviders(backend.sadPath, storageContext()) }); + + expectTaskRejectedUnrecoverable(run, reason); + }); + }); + }); +}); diff --git a/tests/integration/helpers/backendFixtures.ts b/tests/integration/helpers/backendFixtures.ts index 1206b35..7a3bc58 100644 --- a/tests/integration/helpers/backendFixtures.ts +++ b/tests/integration/helpers/backendFixtures.ts @@ -1,18 +1,25 @@ -import { join } from 'node:path'; import { faker } from '@faker-js/faker'; import { container } from 'tsyringe'; -import { StorageProvider, type FsTilesDeletionParams, type S3TilesDeletionParams } from '@map-colonies/raster-shared'; +import { afterAll, afterEach, beforeAll, beforeEach } from 'vitest'; +import { StorageProvider } from '@map-colonies/raster-shared'; import type { S3Client } from '@aws-sdk/client-s3'; -import { S3StorageProvider, FsStorageProvider, type FsStorageConfig, type StorageProviders } from '@src/cleaner/storageProviders'; -import { buildFsTilesDeletionParams, buildS3TilesDeletionParams, type TilesDeletionCommon } from '../../helpers/fakes/tilesDeletionFakes'; +// eslint-disable-next-line @typescript-eslint/naming-convention -- ioredis' default export is a class +import type Redis from 'ioredis'; +import { createRedisConnection } from '@src/cleaner/clients'; +import { + FsStorageProvider, + RedisStorageProvider, + S3StorageProvider, + type FsStorageConfig, + type RedisStorageConfig, + type StorageProviders, +} from '@src/cleaner/storageProviders'; import { createMockLogger } from '../../helpers/mocks'; -import { startMinio, type MinioHandle } from './minioContainer'; -import { buildS3StorageConfigForMinio, createTestS3Client, deleteBucket, ensureBucket, listAllKeys, putManyTiles } from './s3TestKit'; -import { listAllFiles, makeTempFsBase, rmBase, writeManyTiles } from './fsTestKit'; - -/** The storage providers a tiles-deletion task can actually be routed to today (REDIS is not implemented). */ -type PathAddressedProvider = Exclude; -type PathAddressedParams = S3TilesDeletionParams | FsTilesDeletionParams; +import { startS3, type S3Handle } from './s3Container'; +import { buildS3StorageConfig, createTestS3Client, deleteBucket, ensureBucket } from './s3TestKit'; +import { makeTempFsBase, rmBase } from './fsTestKit'; +import { startRedis, type RedisHandle } from './redisContainer'; +import { createTestRedisClient, flush } from './redisTestKit'; /** * The only sub path FS deletion is allowed under, mirroring `storage.fs.subPaths` in config. @@ -20,10 +27,17 @@ type PathAddressedParams = S3TilesDeletionParams | FsTilesDeletionParams; */ const FS_ALLOWED_SUB_PATH = 'artifacts/tiles'; const FS_DELETE_BATCH_SIZE = 100; +// Small paging values so a modest seed crosses both SCAN and UNLINK boundaries +const REDIS_SCAN_COUNT = 10; +const REDIS_DELETE_BATCH_SIZE = 4; interface BackendHandles { - minio: MinioHandle; + s3: S3Handle; s3Client: S3Client; + redis: RedisHandle; + /** Seeding and listing client; the provider gets its own connection. */ + redisClient: Redis; + redisConnection: Redis; } interface TestStorageContext { @@ -36,15 +50,23 @@ interface TestStorageContext { fsSubPath: string; } +function buildRedisStorageConfig(redis: RedisHandle, batchSize = REDIS_DELETE_BATCH_SIZE): RedisStorageConfig { + return { host: redis.host, port: redis.port, db: 0, scanCount: REDIS_SCAN_COUNT, batchSize }; +} + async function startBackends(): Promise { - const minio = await startMinio(); - const s3Client = createTestS3Client(minio); - return { minio, s3Client }; + const [s3, redis] = await Promise.all([startS3(), startRedis()]); + const s3Client = createTestS3Client(s3); + const redisClient = createTestRedisClient(redis); + const redisConnection = await createRedisConnection(buildRedisStorageConfig(redis), createMockLogger()); + return { s3, s3Client, redis, redisClient, redisConnection }; } -async function stopBackends({ minio, s3Client }: BackendHandles): Promise { +async function stopBackends({ s3, s3Client, redis, redisClient, redisConnection }: BackendHandles): Promise { s3Client.destroy(); - await minio.stop(); + await redisConnection.quit(); + redisClient.disconnect(); + await Promise.all([s3.stop(), redis.stop()]); } /** Per-provider delete batch sizes; each falls back to the production-shaped default. */ @@ -56,28 +78,39 @@ interface ProviderBatchSizes { * instead of chunking, so tiles deletion is unaffected by it. */ fs?: number; + /** Keys per `UNLINK` the Redis provider chunks its input into. */ + redis?: number; } -function buildProviders(minio: MinioHandle, fsBasePath: string, batchSizes: ProviderBatchSizes = {}): StorageProviders { +function buildProviders( + { s3, s3Client, redis, redisConnection }: BackendHandles, + fsBasePath: string, + batchSizes: ProviderBatchSizes = {} +): StorageProviders { const fsStorageConfig: FsStorageConfig = { basePath: fsBasePath, subPaths: [FS_ALLOWED_SUB_PATH], batchSize: batchSizes.fs ?? FS_DELETE_BATCH_SIZE, }; - const s3StorageConfig = buildS3StorageConfigForMinio(minio); + const s3StorageConfig = buildS3StorageConfig(s3); return { - [StorageProvider.S3]: new S3StorageProvider({ ...s3StorageConfig, batchSize: batchSizes.s3 ?? s3StorageConfig.batchSize }, createMockLogger()), + [StorageProvider.S3]: new S3StorageProvider( + { ...s3StorageConfig, batchSize: batchSizes.s3 ?? s3StorageConfig.batchSize }, + s3Client, + createMockLogger() + ), [StorageProvider.FS]: new FsStorageProvider(fsStorageConfig, createMockLogger()), + [StorageProvider.REDIS]: new RedisStorageProvider(buildRedisStorageConfig(redis, batchSizes.redis), redisConnection, createMockLogger()), }; } -async function setupTestStorageContext({ minio, s3Client }: BackendHandles): Promise { +async function setupTestStorageContext(handles: BackendHandles): Promise { const bucket = `test-${faker.string.alphanumeric({ length: 16, casing: 'lower' })}`; - await ensureBucket(s3Client, bucket); + await ensureBucket(handles.s3Client, bucket); const fsBasePath = await makeTempFsBase(); - return { providers: buildProviders(minio, fsBasePath), bucket, fsBasePath, fsSubPath: FS_ALLOWED_SUB_PATH }; + return { providers: buildProviders(handles, fsBasePath), bucket, fsBasePath, fsSubPath: FS_ALLOWED_SUB_PATH }; } /** @@ -86,58 +119,50 @@ async function setupTestStorageContext({ minio, s3Client }: BackendHandles): Pro * providers point at the same bucket and base path, so seeding stays unchanged. */ function providersWithBatchSizes(handles: BackendHandles, storageContext: TestStorageContext, batchSizes: ProviderBatchSizes): StorageProviders { - return buildProviders(handles.minio, storageContext.fsBasePath, batchSizes); + return buildProviders(handles, storageContext.fsBasePath, batchSizes); } async function teardownTestStorageContext(handles: BackendHandles, storageContext: TestStorageContext): Promise { try { - await Promise.all([deleteBucket(handles.s3Client, storageContext.bucket), rmBase(storageContext.fsBasePath)]); + await Promise.all([deleteBucket(handles.s3Client, storageContext.bucket), rmBase(storageContext.fsBasePath), flush(handles.redisClient)]); } finally { + // The poller helpers register into the global container; start every test from a clean one. container.reset(); } } -interface ProviderBackend { - storageProvider: PathAddressedProvider; - /** - * Task params carrying this backend's own storage locator — bucket for S3, sub path for FS. - * `overrides` pins the otherwise-faked tile fields, e.g. explicit `ranges`. - */ - buildParams: (tilesRelativePath: string, overrides?: Partial) => PathAddressedParams; - /** Seeds tiles at paths relative to the storage target. */ - seed: (paths: string[]) => Promise; - /** Lists surviving tiles as paths relative to the storage target. */ - list: (prefix: string) => Promise; +/** Lazy views over the suite's backends, safe to capture at `describe.each` collection time. */ +interface StorageBackendsHarness { + handles: () => BackendHandles; + storageContext: () => TestStorageContext; } -function s3Backend(handles: () => BackendHandles, perTest: () => TestStorageContext): ProviderBackend { - return { - storageProvider: StorageProvider.S3, - buildParams: (tilesRelativePath, overrides) => buildS3TilesDeletionParams({ ...overrides, bucket: perTest().bucket, tilesRelativePath }), - seed: async (paths) => putManyTiles(handles().s3Client, perTest().bucket, paths), - list: async (prefix) => listAllKeys(handles().s3Client, perTest().bucket, prefix), - }; -} +/** + * Registers the whole backend lifecycle for a suite: containers once per file, a fresh bucket, + * FS base path and empty Redis DB per test. Call inside the top-level `describe`. + */ +function useStorageBackends(): StorageBackendsHarness { + let handles: BackendHandles; + let storageContext: TestStorageContext; -function fsBackend(perTest: () => TestStorageContext): ProviderBackend { - // The provider joins base path + sub path itself, so the test seeds and reads the same root. - const targetRoot = (): string => join(perTest().fsBasePath, perTest().fsSubPath); - return { - storageProvider: StorageProvider.FS, - buildParams: (tilesRelativePath, overrides) => buildFsTilesDeletionParams({ ...overrides, subPath: perTest().fsSubPath, tilesRelativePath }), - seed: async (paths) => writeManyTiles(targetRoot(), paths), - list: async (prefix) => (await listAllFiles(targetRoot())).filter((path) => path.startsWith(prefix)), - }; + beforeAll(async () => { + handles = await startBackends(); + }); + + afterAll(async () => { + await stopBackends(handles); + }); + + beforeEach(async () => { + storageContext = await setupTestStorageContext(handles); + }); + + afterEach(async () => { + await teardownTestStorageContext(handles, storageContext); + }); + + return { handles: () => handles, storageContext: () => storageContext }; } -export { - FS_ALLOWED_SUB_PATH, - startBackends, - stopBackends, - setupTestStorageContext, - teardownTestStorageContext, - providersWithBatchSizes, - s3Backend, - fsBackend, -}; -export type { BackendHandles, TestStorageContext, ProviderBackend, PathAddressedParams, ProviderBatchSizes }; +export { FS_ALLOWED_SUB_PATH, useStorageBackends, providersWithBatchSizes }; +export type { BackendHandles, TestStorageContext, ProviderBatchSizes, StorageBackendsHarness }; diff --git a/tests/integration/helpers/minioContainer.ts b/tests/integration/helpers/minioContainer.ts deleted file mode 100644 index 97772be..0000000 --- a/tests/integration/helpers/minioContainer.ts +++ /dev/null @@ -1,57 +0,0 @@ -/* eslint-disable @typescript-eslint/naming-convention */ -import { GenericContainer, type StartedTestContainer } from 'testcontainers'; - -interface MinioHandle { - endpoint: string; - accessKeyId: string; - secretAccessKey: string; - stop: () => Promise; -} - -/** - * Pinned to the release running on our Azure deployment, so the suite exercises the same server - * behavior we deploy against. - */ -const MINIO_IMAGE = 'minio/minio:RELEASE.2025-07-23T15-54-02Z'; -const MINIO_PORT = 9000; -const DEFAULT_USER = 'minioadmin'; -const DEFAULT_PASSWORD = 'minioadmin'; - -async function startMinio(): Promise { - const externalEndpoint = process.env.TEST_MINIO_ENDPOINT; - if (externalEndpoint !== undefined && externalEndpoint !== '') { - console.warn(`Minio: using EXTERNAL server at ${externalEndpoint} (TEST_MINIO_ENDPOINT is set).`); - console.warn('Minio: this suite CREATES AND DELETES buckets on that server. Unset TEST_MINIO_ENDPOINT to use a testcontainer.'); - return { - endpoint: externalEndpoint, - accessKeyId: process.env.TEST_MINIO_ACCESS_KEY ?? DEFAULT_USER, - secretAccessKey: process.env.TEST_MINIO_SECRET_KEY ?? DEFAULT_PASSWORD, - stop: async (): Promise => Promise.resolve(), - }; - } - console.log(`Minio: TEST_MINIO_ENDPOINT is not set, starting testcontainer from ${MINIO_IMAGE}`); - const container: StartedTestContainer = await new GenericContainer(MINIO_IMAGE) - .withCommand(['server', '/data']) - .withEnvironment({ - MINIO_ROOT_USER: DEFAULT_USER, - MINIO_ROOT_PASSWORD: DEFAULT_PASSWORD, - }) - .withExposedPorts(MINIO_PORT) - .start(); - - return { - endpoint: `http://${container.getHost()}:${container.getMappedPort(MINIO_PORT)}`, - accessKeyId: DEFAULT_USER, - secretAccessKey: DEFAULT_PASSWORD, - stop: async (): Promise => { - try { - await container.stop(); - } catch (error) { - // Non-fatal: Ryuk reaps the container on test process exit, unless it is disabled. - console.warn('Failed to stop Minio container; relying on Ryuk to reap it', error); - } - }, - }; -} - -export { startMinio, type MinioHandle }; diff --git a/tests/integration/helpers/redisContainer.ts b/tests/integration/helpers/redisContainer.ts new file mode 100644 index 0000000..d39802c --- /dev/null +++ b/tests/integration/helpers/redisContainer.ts @@ -0,0 +1,46 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import { GenericContainer, type StartedTestContainer } from 'testcontainers'; + +interface RedisHandle { + host: string; + port: number; + stop: () => Promise; +} + +/** Pinned to the image deployed in our environments; bump it when they move. */ +const REDIS_IMAGE = 'docker.io/bitnamilegacy/redis:7.2.1'; +const REDIS_PORT = 6379; + +async function startRedis(): Promise { + const externalHost = process.env.TEST_REDIS_HOST; + if (externalHost !== undefined && externalHost !== '') { + console.warn(`Redis: using EXTERNAL server at ${externalHost} (TEST_REDIS_HOST is set).`); + console.warn('Redis: this suite FLUSHES THE DB on that server. Unset TEST_REDIS_HOST to use a testcontainer.'); + return { + host: externalHost, + port: Number(process.env.TEST_REDIS_PORT ?? REDIS_PORT), + stop: async (): Promise => Promise.resolve(), + }; + } + + console.log(`Redis: TEST_REDIS_HOST is not set, starting testcontainer from ${REDIS_IMAGE}`); + const container: StartedTestContainer = await new GenericContainer(REDIS_IMAGE) + .withEnvironment({ ALLOW_EMPTY_PASSWORD: 'yes' }) + .withExposedPorts(REDIS_PORT) + .start(); + + return { + host: container.getHost(), + port: container.getMappedPort(REDIS_PORT), + stop: async (): Promise => { + try { + await container.stop(); + } catch (error) { + // Non-fatal: Ryuk reaps the container on test process exit, unless it is disabled. + console.warn('Failed to stop Redis container; relying on Ryuk to reap it', error); + } + }, + }; +} + +export { startRedis, type RedisHandle }; diff --git a/tests/integration/helpers/redisTestKit.ts b/tests/integration/helpers/redisTestKit.ts new file mode 100644 index 0000000..da3e6e1 --- /dev/null +++ b/tests/integration/helpers/redisTestKit.ts @@ -0,0 +1,27 @@ +// eslint-disable-next-line @typescript-eslint/naming-convention -- ioredis' default export is a class +import Redis from 'ioredis'; +import type { RedisHandle } from './redisContainer'; +import { TINY_TILE_BODY } from './tileFixtures'; + +function createTestRedisClient(handle: RedisHandle): Redis { + return new Redis({ host: handle.host, port: handle.port }); +} + +async function seedKeys(client: Redis, keys: string[]): Promise { + const pipeline = client.pipeline(); + for (const key of keys) { + pipeline.set(key, TINY_TILE_BODY); + } + await pipeline.exec(); +} + +async function listAllKeys(client: Redis): Promise { + const keys = await client.keys('*'); + return keys.sort(); +} + +async function flush(client: Redis): Promise { + await client.flushdb(); +} + +export { createTestRedisClient, seedKeys, listAllKeys, flush }; diff --git a/tests/integration/helpers/s3Container.ts b/tests/integration/helpers/s3Container.ts new file mode 100644 index 0000000..e60682d --- /dev/null +++ b/tests/integration/helpers/s3Container.ts @@ -0,0 +1,58 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import { GenericContainer, type StartedTestContainer } from 'testcontainers'; + +interface S3Handle { + endpoint: string; + accessKeyId: string; + secretAccessKey: string; + stop: () => Promise; +} + +/** + * Any S3-compatible server works here. Pinned to the image our infra deploys (currently SeaweedFS's + * S3 gateway), so the suite exercises the same server behavior. + */ +const S3_IMAGE = 'docker.io/chrislusf/seaweedfs:4.47'; +const S3_PORT = 8333; +const DEFAULT_USER = 'minioadmin'; +const DEFAULT_PASSWORD = 'minioadmin'; + +async function startS3(): Promise { + const externalEndpoint = process.env.TEST_S3_ENDPOINT; + if (externalEndpoint !== undefined && externalEndpoint !== '') { + console.warn(`S3: using EXTERNAL server at ${externalEndpoint} (TEST_S3_ENDPOINT is set).`); + console.warn('S3: this suite CREATES AND DELETES buckets on that server. Unset TEST_S3_ENDPOINT to use a testcontainer.'); + return { + endpoint: externalEndpoint, + accessKeyId: process.env.TEST_S3_ACCESS_KEY ?? DEFAULT_USER, + secretAccessKey: process.env.TEST_S3_SECRET_KEY ?? DEFAULT_PASSWORD, + stop: async (): Promise => Promise.resolve(), + }; + } + console.log(`S3: TEST_S3_ENDPOINT is not set, starting testcontainer from ${S3_IMAGE}`); + const container: StartedTestContainer = await new GenericContainer(S3_IMAGE) + .withCommand(['server', '-s3', '-dir=/data']) + // SeaweedFS registers these as the S3 admin identity + .withEnvironment({ + AWS_ACCESS_KEY_ID: DEFAULT_USER, + AWS_SECRET_ACCESS_KEY: DEFAULT_PASSWORD, + }) + .withExposedPorts(S3_PORT) + .start(); + + return { + endpoint: `http://${container.getHost()}:${container.getMappedPort(S3_PORT)}`, + accessKeyId: DEFAULT_USER, + secretAccessKey: DEFAULT_PASSWORD, + stop: async (): Promise => { + try { + await container.stop(); + } catch (error) { + // Non-fatal: Ryuk reaps the container on test process exit, unless it is disabled. + console.warn('Failed to stop S3 container; relying on Ryuk to reap it', error); + } + }, + }; +} + +export { startS3, type S3Handle }; diff --git a/tests/integration/helpers/s3TestKit.ts b/tests/integration/helpers/s3TestKit.ts index 2fb72fe..9ee77fa 100644 --- a/tests/integration/helpers/s3TestKit.ts +++ b/tests/integration/helpers/s3TestKit.ts @@ -10,7 +10,7 @@ import { BucketAlreadyExists, } from '@aws-sdk/client-s3'; import type { S3StorageConfig } from '@src/cleaner/storageProviders'; -import type { MinioHandle } from './minioContainer'; +import type { S3Handle } from './s3Container'; import { TINY_TILE_BODY } from './tileFixtures'; // S3/MinIO reject a DeleteObjects request carrying more than 1000 keys. Only bucket @@ -18,7 +18,7 @@ import { TINY_TILE_BODY } from './tileFixtures'; const S3_DELETE_OBJECTS_MAX_KEYS = 1000; const TEST_REGION = 'us-east-1'; -function createTestS3Client(handle: MinioHandle): S3Client { +function createTestS3Client(handle: S3Handle): S3Client { return new S3Client({ endpoint: handle.endpoint, credentials: { @@ -36,7 +36,7 @@ function createTestS3Client(handle: MinioHandle): S3Client { * built here straight from the container handle, so the test skips `buildS3StorageConfig` * (and with it the `storage.s3` config lookup) but exercises the real provider. */ -function buildS3StorageConfigForMinio(handle: MinioHandle): S3StorageConfig { +function buildS3StorageConfig(handle: S3Handle): S3StorageConfig { return { endpoint: handle.endpoint, accessKeyId: handle.accessKeyId, @@ -103,4 +103,4 @@ async function putManyTiles(client: S3Client, bucket: string, keys: string[]): P } } -export { createTestS3Client, buildS3StorageConfigForMinio, ensureBucket, deleteBucket, emptyBucket, putTile, putManyTiles, listAllKeys }; +export { createTestS3Client, buildS3StorageConfig, ensureBucket, deleteBucket, emptyBucket, putTile, putManyTiles, listAllKeys }; diff --git a/tests/integration/helpers/storageBackends.ts b/tests/integration/helpers/storageBackends.ts new file mode 100644 index 0000000..5ca608e --- /dev/null +++ b/tests/integration/helpers/storageBackends.ts @@ -0,0 +1,88 @@ +import { join } from 'node:path'; +import { StorageProvider } from '@map-colonies/raster-shared'; +import type { StorageProviders } from '@src/cleaner/storageProviders'; +import type { BackendHandles, TestStorageContext } from './backendFixtures'; +import { listAllFiles, writeManyTiles } from './fsTestKit'; +import { listAllKeys, putManyTiles } from './s3TestKit'; +import { listAllKeys as listAllRedisKeys, seedKeys } from './redisTestKit'; + +const DEFAULT_TILE_EXTENSION = 'png'; + +/** + * Raw access to one real storage backend, independent of any strategy: what a test seeds + * before a task runs and reads back afterwards. Strategy suites layer their own task-shaped + * vocabulary on top of this. + */ +interface StorageBackend { + storageProvider: StorageProvider; + /** + * Names one tile of `layer` the way this backend stores it, relative to the storage target — + * exactly as the strategies generate the keys they hand the provider. + */ + tileKey: (layer: string, zoom: number, x: number, y: number, extension?: string) => string; + /** Seeds tiles at keys relative to the storage target. */ + seed: (keys: string[]) => Promise; + /** Sorted keys under the storage target, narrowed to those starting with `prefix` when given. */ + list: (prefix?: string) => Promise; +} + +/** A task the strategy or provider must refuse as a producer bug rather than retry. */ +interface SadPath { + params: () => Params; + /** Replaces the context's providers, e.g. to leave this backend unregistered. */ + providers?: (storageContext: TestStorageContext) => StorageProviders; + /** Substring of the rejection reason. */ + reason: string; +} + +/** `{layer}/{z}/{x}/{y}.{ext}` — the S3 object key and the FS path under the sub path. */ +const pathTileKey: StorageBackend['tileKey'] = (layer, zoom, x, y, extension = DEFAULT_TILE_EXTENSION) => `${layer}/${zoom}/${x}/${y}.${extension}`; + +/** `{prefix}-{z}-{x}-{y}` — the mapproxy cache key, no extension. */ +const redisTileKey: StorageBackend['tileKey'] = (prefix, zoom, x, y) => `${prefix}-${zoom}-${x}-${y}`; + +/** The context's providers minus `storageProvider`, so a task addressed to it is unsupported. */ +function providersWithout(storageProvider: StorageProvider): (storageContext: TestStorageContext) => StorageProviders { + return ({ providers }) => { + const rest = { ...providers }; + delete rest[storageProvider]; + return rest; + }; +} + +/** The providers a sad-path task should run against: its own override, else the context's. */ +function sadPathProviders({ providers }: SadPath, storageContext: TestStorageContext): StorageProviders { + return providers ? providers(storageContext) : storageContext.providers; +} + +function s3StorageBackend(handles: () => BackendHandles, perTest: () => TestStorageContext): StorageBackend { + return { + storageProvider: StorageProvider.S3, + tileKey: pathTileKey, + seed: async (keys) => putManyTiles(handles().s3Client, perTest().bucket, keys), + list: async (prefix) => listAllKeys(handles().s3Client, perTest().bucket, prefix), + }; +} + +function fsStorageBackend(perTest: () => TestStorageContext): StorageBackend { + // The provider joins base path + sub path itself, so the test seeds and reads the same root. + const targetRoot = (): string => join(perTest().fsBasePath, perTest().fsSubPath); + return { + storageProvider: StorageProvider.FS, + tileKey: pathTileKey, + seed: async (keys) => writeManyTiles(targetRoot(), keys), + list: async (prefix = '') => (await listAllFiles(targetRoot())).filter((path) => path.startsWith(prefix)), + }; +} + +function redisStorageBackend(handles: () => BackendHandles): StorageBackend { + return { + storageProvider: StorageProvider.REDIS, + tileKey: redisTileKey, + seed: async (keys) => seedKeys(handles().redisClient, keys), + list: async (prefix = '') => (await listAllRedisKeys(handles().redisClient)).filter((key) => key.startsWith(prefix)), + }; +} + +export { s3StorageBackend, fsStorageBackend, redisStorageBackend, providersWithout, sadPathProviders }; +export type { StorageBackend, SadPath }; diff --git a/tests/integration/helpers/storedResourcesBackends.ts b/tests/integration/helpers/storedResourcesBackends.ts new file mode 100644 index 0000000..77d3232 --- /dev/null +++ b/tests/integration/helpers/storedResourcesBackends.ts @@ -0,0 +1,58 @@ +import { StorageProvider, type DeleteStoredResourcesParams } from '@map-colonies/raster-shared'; +import type { StorageBackendsHarness } from './backendFixtures'; +import { fsStorageBackend, providersWithout, redisStorageBackend, s3StorageBackend, type SadPath, type StorageBackend } from './storageBackends'; + +/** Adapts one storage backend to the layer-shaped vocabulary of the stored-resources E2E suite. */ +interface ResourceBackend extends StorageBackend { + /** Every path/prefix travels in the task params; each backend names its own locator. */ + buildParams: (layers: string[]) => DeleteStoredResourcesParams; + /** Redis wipes exactly one prefix per task, so multi-layer scenarios skip it. */ + supportsMultipleLayers: boolean; + sadPath: SadPath; +} + +function s3ResourceBackend({ handles, storageContext }: StorageBackendsHarness): ResourceBackend { + return { + ...s3StorageBackend(handles, storageContext), + buildParams: (layers) => ({ storageProvider: StorageProvider.S3, bucket: storageContext().bucket, paths: layers }), + supportsMultipleLayers: true, + sadPath: { + params: () => ({ storageProvider: StorageProvider.S3, bucket: 'no-such-bucket', paths: ['layer'] }), + reason: 'Bucket does not exist', + }, + }; +} + +function fsResourceBackend({ storageContext }: StorageBackendsHarness): ResourceBackend { + return { + ...fsStorageBackend(storageContext), + buildParams: (layers) => ({ storageProvider: StorageProvider.FS, subPath: storageContext().fsSubPath, paths: layers }), + supportsMultipleLayers: true, + sadPath: { + // Climbs out of the allowed sub path, which the FS provider refuses outright + params: () => ({ storageProvider: StorageProvider.FS, subPath: storageContext().fsSubPath, paths: ['../../escaped'] }), + reason: 'Cannot delete paths outside the configured sub paths', + }, + }; +} + +function redisResourceBackend({ handles }: StorageBackendsHarness): ResourceBackend { + return { + ...redisStorageBackend(handles), + buildParams: ([prefix]) => ({ storageProvider: StorageProvider.REDIS, prefix: prefix! }), + supportsMultipleLayers: false, + sadPath: { + params: () => ({ storageProvider: StorageProvider.REDIS, prefix: 'layer' }), + providers: providersWithout(StorageProvider.REDIS), + reason: 'Unsupported storage provider REDIS', + }, + }; +} + +/** One adapter per real backend, for `describe.each`. */ +function storedResourcesBackends(harness: StorageBackendsHarness): ResourceBackend[] { + return [s3ResourceBackend(harness), fsResourceBackend(harness), redisResourceBackend(harness)]; +} + +export { storedResourcesBackends }; +export type { ResourceBackend }; diff --git a/tests/integration/helpers/testPoller.ts b/tests/integration/helpers/testPoller.ts index c0cd0f0..b2c550e 100644 --- a/tests/integration/helpers/testPoller.ts +++ b/tests/integration/helpers/testPoller.ts @@ -1,33 +1,58 @@ /* eslint-disable @typescript-eslint/unbound-method */ -import { vi } from 'vitest'; +import { expect, vi } from 'vitest'; import { container } from 'tsyringe'; import type { ITaskResponse, TaskHandler as QueueClient } from '@map-colonies/mc-priority-queue'; +import { StorageProvider } from '@map-colonies/raster-shared'; +import type { constructor } from 'tsyringe/dist/typings/types'; import { SERVICES } from '@src/common/constants'; import { getJobAndTaskToken } from '@src/common/dependencyRegistration'; import { TaskPoller } from '@src/worker/taskPoller'; import { ErrorHandler } from '@src/cleaner/errors'; -import { StrategyFactory, TilesDeletionStrategy } from '@src/cleaner/strategies'; +import { DeleteStoredResourcesStrategy, StrategyFactory, TilesDeletionStrategy } from '@src/cleaner/strategies'; +import type { ITaskStrategy } from '@src/cleaner/strategies/taskStrategy'; import type { StorageProviders } from '@src/cleaner/storageProviders'; import type { JobTrackerClient } from '@src/cleaner/httpClients'; import type { PollingPairConfig } from '@src/cleaner/types'; +import { buildTask } from '../../helpers/fakes/taskFakes'; import { createMockLogger, createMockQueueClient, createMockStrategyConfig, createMockJobTrackerClient } from '../../helpers/mocks'; -const TASK_TYPE = 'tiles-deletion'; -const JOB_TYPE = 'Ingestion_Update'; -const POLLING_PAIR: PollingPairConfig = { jobType: JOB_TYPE, taskType: TASK_TYPE, maxAttempts: 3 }; const POLLER_WATCHDOG_MS = 30_000; +/** A strategy together with the job+task pair it is registered under in `containerConfig`. */ +interface StrategyUnderTest { + pollingPair: PollingPairConfig; + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- any params shape; ITaskStrategy is generic over it + strategy: constructor>; +} + +const MAX_ATTEMPTS = 3; +const TILES_DELETION_TASK = 'tiles-deletion'; + +/** + * The job each strategy is registered under in `containerConfig`, per storage kind: path stores + * are cleaned on ingestion and layer deletion, the Redis cache on the overseer's cache-deletion jobs. + */ +function tilesDeletionUnderTest(storageProvider: StorageProvider): StrategyUnderTest { + const jobType = storageProvider === StorageProvider.REDIS ? 'Update_Delete_Cache' : 'Ingestion_Update'; + return { pollingPair: { jobType, taskType: TILES_DELETION_TASK, maxAttempts: MAX_ATTEMPTS }, strategy: TilesDeletionStrategy }; +} + +function storedResourcesDeletionUnderTest(storageProvider: StorageProvider): StrategyUnderTest { + const jobType = storageProvider === StorageProvider.REDIS ? 'Swap_Delete_Cache' : 'Delete_Layer'; + return { pollingPair: { jobType, taskType: TILES_DELETION_TASK, maxAttempts: MAX_ATTEMPTS }, strategy: DeleteStoredResourcesStrategy }; +} + /** * Returns a queue client whose `dequeue` hands out `task` exactly once for the * matching pair, then null forever. `ack` / `reject` invoke `onTerminal()` so * the caller can stop the poller and assert. */ -function buildSingleShotQueue(task: ITaskResponse, onTerminal: () => void): QueueClient { +function buildSingleShotQueue(task: ITaskResponse, onTerminal: () => void, pair: PollingPairConfig): QueueClient { const queue = createMockQueueClient(); let handedOut = false; vi.mocked(queue.dequeue).mockImplementation(((jobType: string, taskType: string) => { - if (handedOut || jobType !== JOB_TYPE || taskType !== TASK_TYPE) { + if (handedOut || jobType !== pair.jobType || taskType !== pair.taskType) { return null; } handedOut = true; @@ -50,6 +75,7 @@ interface TestPollerParams { task: ITaskResponse; /** Config keys merged over the strategy defaults, e.g. the batching knobs. */ configOverrides?: Record; + strategyUnderTest?: StrategyUnderTest; } interface TestPoller { @@ -59,33 +85,43 @@ interface TestPoller { } /** - * Wires the real TaskPoller → StrategyFactory → TilesDeletionStrategy pipeline, - * with real S3/FS providers (supplied by the caller) and a single-shot fake - * QueueClient. The poller terminates as soon as ack/reject fires. + * Wires the real TaskPoller → StrategyFactory → strategy pipeline, with real + * providers (supplied by the caller) and a single-shot fake QueueClient. + * The poller terminates as soon as ack/reject fires. * - * The strategy reads nothing but batching knobs from config — every storage locator + * The strategies read nothing but batching knobs from config — every storage locator * travels in the task params — so the mock config carries no bucket or base path. */ -function buildPoller({ providers, task, configOverrides = {} }: TestPollerParams): TestPoller { +function buildPoller({ + providers, + task, + configOverrides = {}, + strategyUnderTest = tilesDeletionUnderTest(StorageProvider.S3), +}: TestPollerParams): TestPoller { + const { pollingPair, strategy } = strategyUnderTest; const config = createMockStrategyConfig({ 'queue.dequeueIntervalMs': 0, ...configOverrides }); container.register(SERVICES.LOGGER, { useValue: createMockLogger() }); container.register(SERVICES.CONFIG, { useValue: config }); container.register(SERVICES.STORAGE_PROVIDERS, { useValue: providers }); - const queueClient = buildSingleShotQueue(task, () => { - void poller.stop(); - }); + const queueClient = buildSingleShotQueue( + task, + () => { + void poller.stop(); + }, + pollingPair + ); container.register(SERVICES.QUEUE_CLIENT, { useValue: queueClient }); // StrategyFactory resolves strategies by the combined job+task token, not the task type alone. - container.register(getJobAndTaskToken(POLLING_PAIR), { useClass: TilesDeletionStrategy }); + container.register(getJobAndTaskToken(pollingPair), { useClass: strategy }); const jobTrackerClient = createMockJobTrackerClient(); container.register(SERVICES.JOB_TRACKER_CLIENT, { useValue: jobTrackerClient }); const strategyFactory = container.resolve(StrategyFactory); const errorHandler = container.resolve(ErrorHandler); - const poller = new TaskPoller(createMockLogger(), config, queueClient, strategyFactory, errorHandler, [POLLING_PAIR], jobTrackerClient); + const poller = new TaskPoller(createMockLogger(), config, queueClient, strategyFactory, errorHandler, [pollingPair], jobTrackerClient); const runSingleTask = async (): Promise => { const startPromise = poller.start(); @@ -99,5 +135,46 @@ function buildPoller({ providers, task, configOverrides = {} }: TestPollerParams return { queueClient, jobTrackerClient, runSingleTask }; } -export { TASK_TYPE, JOB_TYPE, POLLING_PAIR, buildSingleShotQueue, buildPoller }; -export type { TestPoller, TestPollerParams }; +interface RunTaskParams extends Omit { + strategyUnderTest: StrategyUnderTest; + params: unknown; +} + +interface TaskRun { + task: ITaskResponse; + queueClient: QueueClient; + jobTrackerClient: JobTrackerClient; +} + +/** Builds a task carrying `params`, drives it once through the poller and hands back what to assert on. */ +async function runTask({ strategyUnderTest, params, ...pollerParams }: RunTaskParams): Promise { + const task = buildTask({ type: strategyUnderTest.pollingPair.taskType, parameters: params }); + const { runSingleTask, ...clients } = buildPoller({ ...pollerParams, task, strategyUnderTest }); + await runSingleTask(); + return { task, ...clients }; +} + +/** The task reached the queue's happy terminal state and the job tracker heard about it. */ +function expectTaskAcked({ task, queueClient, jobTrackerClient }: TaskRun): void { + expect(queueClient.ack).toHaveBeenCalledWith(task.jobId, task.id); + expect(queueClient.reject).not.toHaveBeenCalled(); + expect(jobTrackerClient.notify).toHaveBeenCalledWith(task.id); +} + +/** The task was rejected with `shouldRetry=false` and a reason containing `reason`. */ +function expectTaskRejectedUnrecoverable({ task, queueClient, jobTrackerClient }: TaskRun, reason: string): void { + expect(queueClient.reject).toHaveBeenCalledWith(task.jobId, task.id, false, expect.stringContaining(reason)); + expect(queueClient.ack).not.toHaveBeenCalled(); + expect(jobTrackerClient.notify).toHaveBeenCalledWith(task.id); +} + +export { + tilesDeletionUnderTest, + storedResourcesDeletionUnderTest, + buildSingleShotQueue, + buildPoller, + runTask, + expectTaskAcked, + expectTaskRejectedUnrecoverable, +}; +export type { StrategyUnderTest, TestPoller, TestPollerParams, RunTaskParams, TaskRun }; diff --git a/tests/integration/helpers/tileFixtures.ts b/tests/integration/helpers/tileFixtures.ts index 105d485..d473581 100644 --- a/tests/integration/helpers/tileFixtures.ts +++ b/tests/integration/helpers/tileFixtures.ts @@ -1,26 +1,24 @@ import type { TileRange } from '@map-colonies/raster-shared'; -import type { TilesDeletionCommon } from '../../helpers/fakes/tilesDeletionFakes'; // First 4 bytes of a PNG file header (\x89 P N G). Content is arbitrary; // eslint-disable-next-line @typescript-eslint/no-magic-numbers const TINY_TILE_BODY = Buffer.from([0x89, 0x50, 0x4e, 0x47]); -/** - * Every path returned here is relative to the storage target — the bucket for S3, - * the task's sub path for FS — exactly as `TilesDeletionStrategy` generates them. - */ -function tilePathsForRange(range: TileRange, tilesRelativePath: string, fileExtension: string): string[] { - const paths: string[] = []; +/** Names one tile of a fixed layer, relative to the storage target; see `StorageBackend.tileKey`. */ +type TileKeyFormat = (zoom: number, x: number, y: number) => string; + +function tileKeysForRange(range: TileRange, format: TileKeyFormat): string[] { + const keys: string[] = []; for (let x = range.minX; x <= range.maxX; x++) { for (let y = range.minY; y <= range.maxY; y++) { - paths.push(`${tilesRelativePath}/${range.zoom}/${x}/${y}.${fileExtension}`); + keys.push(format(range.zoom, x, y)); } } - return paths; + return keys; } -function tilePathsForRanges(params: TilesDeletionCommon): string[] { - return params.ranges.flatMap((range) => tilePathsForRange(range, params.tilesRelativePath, params.fileExtension)); +function tileKeysForRanges(ranges: TileRange[], format: TileKeyFormat): string[] { + return ranges.flatMap((range) => tileKeysForRange(range, format)); } function paddedRange(range: TileRange, padding: number): TileRange { @@ -35,20 +33,20 @@ function paddedRange(range: TileRange, padding: number): TileRange { /** * Builds the set of tiles inside the `paddedRange` but **outside** the original - * `params.ranges`. These tiles are seeded into the backend before the strategy + * `ranges`. These tiles are seeded into the backend before the strategy * runs and MUST survive — they prove "only the supplied ranges are deleted". * * Deduped, because with several ranges the padded regions can overlap and yield - * the same bystander path twice. + * the same bystander key twice. */ -function extraTilePathsAroundRanges(params: TilesDeletionCommon, padding: number): string[] { - const targetSet = new Set(tilePathsForRanges(params)); +function extraTileKeysAroundRanges(ranges: TileRange[], format: TileKeyFormat, padding: number): string[] { + const targetSet = new Set(tileKeysForRanges(ranges, format)); const extras = new Set(); - for (const range of params.ranges) { + for (const range of ranges) { const wider = paddedRange(range, padding); - for (const path of tilePathsForRange(wider, params.tilesRelativePath, params.fileExtension)) { - if (!targetSet.has(path)) { - extras.add(path); + for (const key of tileKeysForRange(wider, format)) { + if (!targetSet.has(key)) { + extras.add(key); } } } @@ -56,28 +54,55 @@ function extraTilePathsAroundRanges(params: TilesDeletionCommon, padding: number } /** - * Returns one tile path per (range × zoomDelta) pair, placed at a zoom level + * Returns one tile key per (range × zoomDelta) pair, placed at a zoom level * adjacent to each range's zoom. * - * Like `extraTilePathsAroundRanges`, the returned paths are seeded before the + * Like `extraTileKeysAroundRanges`, the returned keys are seeded before the * test and must survive after the strategy runs. * */ -function extraTilePathsAtAdjacentZooms(params: TilesDeletionCommon, zoomDeltas: number[]): string[] { - const targetSet = new Set(tilePathsForRanges(params)); +function extraTileKeysAtAdjacentZooms(ranges: TileRange[], format: TileKeyFormat, zoomDeltas: number[]): string[] { + const targetSet = new Set(tileKeysForRanges(ranges, format)); const extras = new Set(); - for (const range of params.ranges) { + for (const range of ranges) { for (const delta of zoomDeltas) { const zoom = range.zoom + delta; if (zoom < 0) continue; - const path = `${params.tilesRelativePath}/${zoom}/${range.minX}/${range.minY}.${params.fileExtension}`; + const key = format(zoom, range.minX, range.minY); // With several ranges, one range's neighbouring zoom can be another range's own zoom, - // where the path is a deletion target rather than a bystander that must survive. - if (!targetSet.has(path)) { - extras.add(path); + // where the key is a deletion target rather than a bystander that must survive. + if (!targetSet.has(key)) { + extras.add(key); } } } return [...extras]; } -export { tilePathsForRange, tilePathsForRanges, paddedRange, extraTilePathsAroundRanges, extraTilePathsAtAdjacentZooms, TINY_TILE_BODY }; +const BYSTANDER_PADDING = 1; +/** One zoom below and one above each range. */ +// eslint-disable-next-line @typescript-eslint/no-magic-numbers +const BYSTANDER_ZOOM_DELTAS = [-1, 1]; + +/** + * Tiles that neighbour `ranges` in X/Y and at the adjacent zooms, deduped. Seeded alongside the + * targets, they must all survive the run — the one assertion every deletion scenario shares. + */ +function bystanderTileKeys(ranges: TileRange[], format: TileKeyFormat): string[] { + return [ + ...new Set([ + ...extraTileKeysAroundRanges(ranges, format, BYSTANDER_PADDING), + ...extraTileKeysAtAdjacentZooms(ranges, format, BYSTANDER_ZOOM_DELTAS), + ]), + ]; +} + +export { + tileKeysForRange, + tileKeysForRanges, + paddedRange, + extraTileKeysAroundRanges, + extraTileKeysAtAdjacentZooms, + bystanderTileKeys, + TINY_TILE_BODY, +}; +export type { TileKeyFormat }; diff --git a/tests/integration/helpers/tilesDeletionBackends.ts b/tests/integration/helpers/tilesDeletionBackends.ts new file mode 100644 index 0000000..5eab90e --- /dev/null +++ b/tests/integration/helpers/tilesDeletionBackends.ts @@ -0,0 +1,98 @@ +import { faker } from '@faker-js/faker'; +import { StorageProvider, type TileRange, type TilesDeletionParams } from '@map-colonies/raster-shared'; +import { buildFsTilesDeletionParams, buildRedisTilesDeletionParams, buildS3TilesDeletionParams } from '../../helpers/fakes/tilesDeletionFakes'; +import type { ProviderBatchSizes, StorageBackendsHarness } from './backendFixtures'; +import { fsStorageBackend, providersWithout, redisStorageBackend, s3StorageBackend, type SadPath, type StorageBackend } from './storageBackends'; +import type { TileKeyFormat } from './tileFixtures'; + +type ParamsOf

= Extract; +type PathParams = ParamsOf<'S3' | 'FS'>; + +/** + * Adapts one storage backend to the range-shaped vocabulary of the tiles-deletion E2E suite. + * Params-typed members are method signatures (bivariant) rather than function properties, so a + * `TilesDeletionBackend` still fits the `describe.each` list typed on the whole union. + */ +/* eslint-disable @typescript-eslint/method-signature-style */ +interface TilesDeletionBackend extends StorageBackend { + /** Batch sizes that make this backend's provider re-chunk a strategy batch of `chunkSize`. */ + chunkedBatchSizes: (chunkSize: number) => ProviderBatchSizes; + sadPath: SadPath; + /** + * Task params carrying a fresh locator for this backend — bucket + relative path for S3, + * sub path + relative path for FS, key prefix for Redis. `overrides` pins the otherwise-faked + * tile fields, e.g. explicit `ranges`. + */ + buildParams(overrides?: { ranges?: TileRange[] }): Params; + /** Names one tile of `params` the way this backend stores it — what `seed` and `list` speak. */ + keyFormat(params: Params): TileKeyFormat; + /** Surviving tiles under the locator of `params`. */ + listTarget(params: Params): Promise; +} +/* eslint-enable @typescript-eslint/method-signature-style */ + +const freshRelativePath = (): string => `${faker.string.uuid()}/${faker.string.uuid()}`; + +/** + * S3 and FS both address tiles as `{tilesRelativePath}/{z}/{x}/{y}.{ext}` under their locator + * and both probe the target before deleting, so they differ only in how params are built and + * whether the provider re-chunks a batch. + */ +function pathTilesBackend( + storage: StorageBackend, + buildParams: TilesDeletionBackend['buildParams'], + chunkedBatchSizes: TilesDeletionBackend['chunkedBatchSizes'] +): TilesDeletionBackend { + return { + ...storage, + buildParams, + keyFormat: (params) => (zoom, x, y) => storage.tileKey(params.tilesRelativePath, zoom, x, y, params.fileExtension), + listTarget: async (params) => storage.list(`${params.tilesRelativePath}/`), + chunkedBatchSizes, + // Nothing is seeded, so the path the task points at was never created on the backend. + sadPath: { params: buildParams, reason: 'storage target does not exist' }, + }; +} + +function s3TilesBackend({ handles, storageContext }: StorageBackendsHarness): TilesDeletionBackend> { + return pathTilesBackend( + s3StorageBackend(handles, storageContext), + (overrides) => buildS3TilesDeletionParams({ ...overrides, bucket: storageContext().bucket, tilesRelativePath: freshRelativePath() }), + (chunkSize) => ({ s3: chunkSize }) + ); +} + +function fsTilesBackend({ storageContext }: StorageBackendsHarness): TilesDeletionBackend> { + return pathTilesBackend( + fsStorageBackend(storageContext), + (overrides) => buildFsTilesDeletionParams({ ...overrides, subPath: storageContext().fsSubPath, tilesRelativePath: freshRelativePath() }), + // `delete` fans every path out at once, so no FS batch size re-chunks a strategy batch. + () => ({}) + ); +} + +function redisTilesBackend({ handles }: StorageBackendsHarness): TilesDeletionBackend> { + const storage = redisStorageBackend(handles); + return { + ...storage, + buildParams: (overrides) => buildRedisTilesDeletionParams(overrides), + keyFormat: (params) => (zoom, x, y) => storage.tileKey(params.prefix, zoom, x, y), + listTarget: async (params) => storage.list(`${params.prefix}-`), + chunkedBatchSizes: (chunkSize) => ({ redis: chunkSize }), + // Redis has no target to probe — a cold prefix is a clean no-op — so the only unrecoverable + // path is a task addressed to a provider this instance was not configured with. + sadPath: { + params: () => buildRedisTilesDeletionParams(), + providers: providersWithout(StorageProvider.REDIS), + reason: 'Unsupported storage provider REDIS', + }, + }; +} + +/** One adapter per real backend, for `describe.each`. */ +function tilesDeletionBackends(harness: StorageBackendsHarness): TilesDeletionBackend[] { + return [s3TilesBackend(harness), fsTilesBackend(harness), redisTilesBackend(harness)]; +} + +export { tilesDeletionBackends }; +export type { TilesDeletionBackend }; diff --git a/tests/integration/helpers/tilesDeletionStrategyScenarios.ts b/tests/integration/helpers/tilesDeletionStrategyScenarios.ts index 3d8d615..826acf6 100644 --- a/tests/integration/helpers/tilesDeletionStrategyScenarios.ts +++ b/tests/integration/helpers/tilesDeletionStrategyScenarios.ts @@ -3,16 +3,16 @@ import type { TileRange } from '@map-colonies/raster-shared'; /** * Sized so one run crosses every batching boundary: 180 tiles at `strategyBatchSize` 40 is * four full batches flushed in two groups of `strategyConcurrency`, followed by a 20-tile - * trailing partial batch. Each 40-key batch then re-chunks into 25 + 15 inside the S3 - * provider's own delete loop, which a production-sized batch never reaches — its cap is 1000 - * keys, well above the strategy's batch size. + * trailing partial batch. Each 40-key batch then re-chunks into 25 + 15 inside the provider's + * own delete loop (S3's `DeleteObjects`, Redis' `UNLINK`), which a production-sized batch never + * reaches — the caps are far above the strategy's batch size. */ const BATCHING_SCENARIO = { range: { zoom: 12, minX: 0, maxX: 11, minY: 0, maxY: 14 } satisfies TileRange, tileCount: 180, strategyBatchSize: 40, strategyConcurrency: 2, - s3ChunkSize: 25, + providerChunkSize: 25, /** One `updateProgress` call per flush of `strategyConcurrency` full batches. */ expectedFlushCount: 2, /** Math.round((80 / 180) * 100) and Math.round((160 / 180) * 100) — one per flush. */ diff --git a/tests/integration/tilesDeletionStrategy.integration.spec.ts b/tests/integration/tilesDeletionStrategy.integration.spec.ts index 8c679f5..1700feb 100644 --- a/tests/integration/tilesDeletionStrategy.integration.spec.ts +++ b/tests/integration/tilesDeletionStrategy.integration.spec.ts @@ -1,121 +1,73 @@ /* eslint-disable @typescript-eslint/unbound-method */ -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest'; -import { faker } from '@faker-js/faker'; -import { buildTask } from '../helpers/fakes/taskFakes'; -import { - fsBackend, - providersWithBatchSizes, - s3Backend, - setupTestStorageContext, - startBackends, - stopBackends, - teardownTestStorageContext, - type BackendHandles, - type PathAddressedParams, - type TestStorageContext, -} from './helpers/backendFixtures'; -import { buildPoller, TASK_TYPE } from './helpers/testPoller'; -import { extraTilePathsAroundRanges, extraTilePathsAtAdjacentZooms, tilePathsForRanges } from './helpers/tileFixtures'; +import { describe, expect, it } from 'vitest'; +import { providersWithBatchSizes, useStorageBackends } from './helpers/backendFixtures'; +import { sadPathProviders } from './helpers/storageBackends'; +import { expectTaskAcked, expectTaskRejectedUnrecoverable, runTask, tilesDeletionUnderTest, type RunTaskParams } from './helpers/testPoller'; +import { tilesDeletionBackends } from './helpers/tilesDeletionBackends'; +import { bystanderTileKeys, tileKeysForRanges } from './helpers/tileFixtures'; import { BATCHING_SCENARIO, MULTIPLE_RANGES_SCENARIO } from './helpers/tilesDeletionStrategyScenarios'; describe('tiles deletion E2E (polling → strategy → real provider → ack)', () => { - let handles: BackendHandles; - let storageContext: TestStorageContext; + const harness = useStorageBackends(); + const { handles, storageContext } = harness; - beforeAll(async () => { - handles = await startBackends(); - }); - - afterAll(async () => { - await stopBackends(handles); - }); - - beforeEach(async () => { - storageContext = await setupTestStorageContext(handles); - }); - - afterEach(async () => { - await teardownTestStorageContext(handles, storageContext); - }); + describe.each(tilesDeletionBackends(harness))('$storageProvider provider', (backend) => { + const strategyUnderTest = tilesDeletionUnderTest(backend.storageProvider); + const runTilesDeletion = async (params: unknown, overrides: Partial = {}): ReturnType => + runTask({ strategyUnderTest, providers: storageContext().providers, params, ...overrides }); - describe.each([ - s3Backend( - () => handles, - () => storageContext - ), - fsBackend(() => storageContext), - ])('$storageProvider provider', (backend) => { describe('happy path', () => { it('polls the task, runs the strategy against the chosen provider, and acks after deleting only the requested tiles', async () => { - const tilesRelativePath = `${faker.string.uuid()}/${faker.string.uuid()}`; - const params: PathAddressedParams = backend.buildParams(tilesRelativePath); - const target = tilePathsForRanges(params); - const extras = [...extraTilePathsAroundRanges(params, 2), ...extraTilePathsAtAdjacentZooms(params, [-1, 1])]; - await backend.seed([...target, ...extras]); - const task = buildTask({ type: TASK_TYPE, parameters: params }); - - const { runSingleTask, queueClient, jobTrackerClient } = buildPoller({ providers: storageContext.providers, task }); - await runSingleTask(); - const remaining = await backend.list(`${tilesRelativePath}/`); - - expect(remaining.sort()).toEqual([...extras].sort()); - expect(queueClient.ack).toHaveBeenCalledWith(task.jobId, task.id); - expect(queueClient.reject).not.toHaveBeenCalled(); - expect(jobTrackerClient.notify).toHaveBeenCalledWith(task.id); + const params = backend.buildParams(); + const format = backend.keyFormat(params); + const bystanders = bystanderTileKeys(params.ranges, format); + await backend.seed([...tileKeysForRanges(params.ranges, format), ...bystanders]); + + const run = await runTilesDeletion(params); + + expect(await backend.listTarget(params)).toEqual([...bystanders].sort()); + expectTaskAcked(run); }); it('acks a redelivered task whose tiles are already gone, leaving unrelated tiles intact', async () => { - const tilesRelativePath = `${faker.string.uuid()}/${faker.string.uuid()}`; - const params: PathAddressedParams = backend.buildParams(tilesRelativePath); - const extras = [...extraTilePathsAroundRanges(params, 2), ...extraTilePathsAtAdjacentZooms(params, [-1, 1])]; - await backend.seed([...tilePathsForRanges(params), ...extras]); - - const firstTask = buildTask({ type: TASK_TYPE, parameters: params }); - await buildPoller({ providers: storageContext.providers, task: firstTask }).runSingleTask(); + const params = backend.buildParams(); + const format = backend.keyFormat(params); + const bystanders = bystanderTileKeys(params.ranges, format); + await backend.seed([...tileKeysForRanges(params.ranges, format), ...bystanders]); + await runTilesDeletion(params); // The queue delivers at least once, so the same task can come back after a crash or a // visibility timeout. Deleting an already-deleted tile must still be a success. - const redeliveredTask = buildTask({ type: TASK_TYPE, parameters: params }); - const { runSingleTask, queueClient, jobTrackerClient } = buildPoller({ providers: storageContext.providers, task: redeliveredTask }); - await runSingleTask(); - const remaining = await backend.list(`${tilesRelativePath}/`); - - expect(queueClient.ack).toHaveBeenCalledWith(redeliveredTask.jobId, redeliveredTask.id); - expect(queueClient.reject).not.toHaveBeenCalled(); - expect(jobTrackerClient.notify).toHaveBeenCalledWith(redeliveredTask.id); - expect(remaining.sort()).toEqual([...extras].sort()); + const redelivered = await runTilesDeletion(params); + + expect(await backend.listTarget(params)).toEqual([...bystanders].sort()); + expectTaskAcked(redelivered); }); }); describe('batching', () => { it('flushes full batches concurrently, chunks each inside the provider, and reports progress per flush', async () => { - const tilesRelativePath = `${faker.string.uuid()}/${faker.string.uuid()}`; - const params: PathAddressedParams = backend.buildParams(tilesRelativePath, { ranges: [BATCHING_SCENARIO.range] }); - const target = tilePathsForRanges(params); + const params = backend.buildParams({ ranges: [BATCHING_SCENARIO.range] }); + const format = backend.keyFormat(params); + const target = tileKeysForRanges(params.ranges, format); // Guards the arithmetic the progress percentages below are derived from. expect(target).toHaveLength(BATCHING_SCENARIO.tileCount); + const bystanders = bystanderTileKeys(params.ranges, format); + await backend.seed([...target, ...bystanders]); - const extras = extraTilePathsAroundRanges(params, 1); - await backend.seed([...target, ...extras]); - const task = buildTask({ type: TASK_TYPE, parameters: params }); - - const { runSingleTask, queueClient, jobTrackerClient } = buildPoller({ - providers: providersWithBatchSizes(handles, storageContext, { s3: BATCHING_SCENARIO.s3ChunkSize }), - task, + const run = await runTilesDeletion(params, { + providers: providersWithBatchSizes(handles(), storageContext(), backend.chunkedBatchSizes(BATCHING_SCENARIO.providerChunkSize)), configOverrides: { 'strategies.tilesDeletion.batchSize': BATCHING_SCENARIO.strategyBatchSize, 'strategies.tilesDeletion.concurrency': BATCHING_SCENARIO.strategyConcurrency, }, }); - await runSingleTask(); - const remaining = await backend.list(`${tilesRelativePath}/`); - expect(remaining.sort()).toEqual([...extras].sort()); - expect(queueClient.ack).toHaveBeenCalledWith(task.jobId, task.id); - expect(queueClient.reject).not.toHaveBeenCalled(); - expect(jobTrackerClient.notify).toHaveBeenCalledWith(task.id); + expect(await backend.listTarget(params)).toEqual([...bystanders].sort()); + expectTaskAcked(run); // One update per flush of `concurrency` full batches. The trailing partial batch reports // nothing, and the terminal 100% is the queue's task-ack rather than the strategy's job. + const { task, queueClient } = run; expect(queueClient.updateProgress).toHaveBeenCalledTimes(BATCHING_SCENARIO.expectedFlushCount); expect(queueClient.updateProgress).toHaveBeenNthCalledWith(1, task.jobId, task.id, BATCHING_SCENARIO.firstFlushPercentage); expect(queueClient.updateProgress).toHaveBeenNthCalledWith(2, task.jobId, task.id, BATCHING_SCENARIO.secondFlushPercentage); @@ -124,42 +76,32 @@ describe('tiles deletion E2E (polling → strategy → real provider → ack)', describe('multiple ranges', () => { it('deletes every tile of every range, leaving neighbouring tiles and zooms of each intact', async () => { - const tilesRelativePath = `${faker.string.uuid()}/${faker.string.uuid()}`; - const params: PathAddressedParams = backend.buildParams(tilesRelativePath, { ranges: MULTIPLE_RANGES_SCENARIO.ranges }); - const target = tilePathsForRanges(params); + const params = backend.buildParams({ ranges: MULTIPLE_RANGES_SCENARIO.ranges }); + const format = backend.keyFormat(params); + const target = tileKeysForRanges(params.ranges, format); // Guards against a range set that silently collapses to fewer tiles than intended. expect(target).toHaveLength(MULTIPLE_RANGES_SCENARIO.tileCount); + const bystanders = bystanderTileKeys(params.ranges, format); + await backend.seed([...target, ...bystanders]); - const extras = [...new Set([...extraTilePathsAroundRanges(params, 1), ...extraTilePathsAtAdjacentZooms(params, [-1, 1])])]; - await backend.seed([...target, ...extras]); - const task = buildTask({ type: TASK_TYPE, parameters: params }); - - const { runSingleTask, queueClient, jobTrackerClient } = buildPoller({ providers: storageContext.providers, task }); - await runSingleTask(); - const remaining = await backend.list(`${tilesRelativePath}/`); + const run = await runTilesDeletion(params); // Every range contributes deletions, and nothing outside them is touched — so a // regression that only walked the first range would leave survivors here. - expect(remaining.sort()).toEqual([...extras].sort()); - expect(queueClient.ack).toHaveBeenCalledWith(task.jobId, task.id); - expect(queueClient.reject).not.toHaveBeenCalled(); - expect(jobTrackerClient.notify).toHaveBeenCalledWith(task.id); + expect(await backend.listTarget(params)).toEqual([...bystanders].sort()); + expectTaskAcked(run); }); }); describe('sad path', () => { - it('rejects the task as unrecoverable without acking when the storage target does not exist', async () => { - // Nothing is seeded, so the path the task points at was never created on the backend. - const params: PathAddressedParams = backend.buildParams(`${faker.string.uuid()}/${faker.string.uuid()}`); - const task = buildTask({ type: TASK_TYPE, parameters: params }); - - const { runSingleTask, queueClient, jobTrackerClient } = buildPoller({ providers: storageContext.providers, task }); - await runSingleTask(); - - // shouldRetry=false is the whole point: a missing target is a producer bug, not a blip. - expect(queueClient.reject).toHaveBeenCalledWith(task.jobId, task.id, false, expect.stringContaining('storage target does not exist')); - expect(queueClient.ack).not.toHaveBeenCalled(); - expect(jobTrackerClient.notify).toHaveBeenCalledWith(task.id); + it('rejects the task as unrecoverable without acking', async () => { + const { params, reason } = backend.sadPath; + + const run = await runTilesDeletion(params(), { providers: sadPathProviders(backend.sadPath, storageContext()) }); + + // shouldRetry=false is the whole point: a missing target or an unknown provider is a + // producer bug, not a blip. + expectTaskRejectedUnrecoverable(run, reason); }); }); }); diff --git a/tests/storageProviders/fsStorageProvider.spec.ts b/tests/storageProviders/fsStorageProvider.spec.ts index ab4db25..51a7814 100644 --- a/tests/storageProviders/fsStorageProvider.spec.ts +++ b/tests/storageProviders/fsStorageProvider.spec.ts @@ -89,7 +89,7 @@ describe('FsStorageProvider', () => { describe('#delete', () => { it('should return empty failures map for empty input', async () => { const result = await provider.delete(SUB_PATH, []); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); expect(unlink).not.toHaveBeenCalled(); }); @@ -118,7 +118,7 @@ describe('FsStorageProvider', () => { it('should return empty failures map when all unlinks succeed', async () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png', 'tile/10/0/1.png']); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); }); it('should treat ENOENT as a failed deletion tagged with ENOENT reason', async () => { @@ -127,7 +127,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png']); - expect(result).toEqual({ failures: new Map([['ENOENT', { count: 1, sample: 'tile/10/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['ENOENT', { count: 1, sample: 'tile/10/0/0.png' }]]), deletedCount: 0 }); }); it('should return failed path with reason for non-ENOENT errors', async () => { @@ -136,7 +136,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png']); - expect(result).toEqual({ failures: new Map([['EACCES', { count: 1, sample: 'tile/10/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['EACCES', { count: 1, sample: 'tile/10/0/0.png' }]]), deletedCount: 0 }); }); it('should fall back to error message when error has no errno code', async () => { @@ -144,7 +144,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png']); - expect(result).toEqual({ failures: new Map([['disk on fire', { count: 1, sample: 'tile/10/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['disk on fire', { count: 1, sample: 'tile/10/0/0.png' }]]), deletedCount: 0 }); }); it('should fall back to "Unknown" when error has neither errno code nor message', async () => { @@ -152,7 +152,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png']); - expect(result).toEqual({ failures: new Map([['Unknown', { count: 1, sample: 'tile/10/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['Unknown', { count: 1, sample: 'tile/10/0/0.png' }]]), deletedCount: 0 }); }); it('should tag failures with the stringified value when a non-Error is thrown', async () => { @@ -160,7 +160,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png']); - expect(result).toEqual({ failures: new Map([['raw string failure', { count: 1, sample: 'tile/10/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['raw string failure', { count: 1, sample: 'tile/10/0/0.png' }]]), deletedCount: 0 }); }); it('should fall back to "Unknown" when a non-Error empty value is thrown', async () => { @@ -168,7 +168,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png']); - expect(result).toEqual({ failures: new Map([['Unknown', { count: 1, sample: 'tile/10/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['Unknown', { count: 1, sample: 'tile/10/0/0.png' }]]), deletedCount: 0 }); }); it('should fall back to a generic reason when the thrown value cannot be stringified', async () => { @@ -180,7 +180,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png']); - expect(result).toEqual({ failures: new Map([['non-serializable thrown value', { count: 1, sample: 'tile/10/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['non-serializable thrown value', { count: 1, sample: 'tile/10/0/0.png' }]]), deletedCount: 0 }); }); it('should unlink every path of a large input', async () => { @@ -201,7 +201,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, paths); - expect(result).toEqual({ failures: new Map([['EACCES', { count: 7, sample: 'tile/10/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['EACCES', { count: 7, sample: 'tile/10/0/0.png' }]]), deletedCount: 0 }); }); it('should handle mixed success, ENOENT and real errors', async () => { @@ -221,6 +221,7 @@ describe('FsStorageProvider', () => { ['ENOENT', { count: 1, sample: 'tile/10/0/1.png' }], ['EACCES', { count: 1, sample: 'tile/10/0/2.png' }], ]), + deletedCount: 1, }); }); @@ -231,7 +232,7 @@ describe('FsStorageProvider', () => { const relativePath = 'layer/v1/10/5/3.png'; const result = await provider.delete(SUB_PATH, [relativePath]); - expect(result).toEqual({ failures: new Map([['EACCES', { count: 1, sample: relativePath }]]) }); + expect(result).toEqual({ failures: new Map([['EACCES', { count: 1, sample: relativePath }]]), deletedCount: 0 }); expect(Array.from(result.failures.values())[0]?.sample).not.toMatch(`^${BASE_PATH}*`); }); @@ -268,7 +269,7 @@ describe('FsStorageProvider', () => { const result = await provider.delete(SUB_PATH, ['tile/10/0/0.png']); // Should not throw and should return correct failed paths - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 1 }); }); it('should not call rmdir when input is empty', async () => { @@ -318,7 +319,7 @@ describe('FsStorageProvider', () => { it('should successfully return without failures for empty paths', async () => { const result = await provider.deleteResources({ paths: [], subPath: FS_SUB_PATH, storageProvider: 'FS' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); expect(rm).not.toHaveBeenCalled(); expect(rmdir).not.toHaveBeenCalled(); }); @@ -326,7 +327,7 @@ describe('FsStorageProvider', () => { it('should successfully call delete all files and return without failures', async () => { const result = await provider.deleteResources({ paths: [RELATIVE_PATH], subPath: FS_SUB_PATH, storageProvider: 'FS' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 1 }); expect(rm).toHaveBeenCalledWith(join(BASE_PATH, FS_SUB_PATH, RELATIVE_PATH), { recursive: true, force: true }); expect(rmdir).toHaveBeenCalledWith(join(BASE_PATH, FS_SUB_PATH, 'layer')); }); @@ -334,7 +335,7 @@ describe('FsStorageProvider', () => { it('should successfully call delete all files and return without failures for multiple paths', async () => { const result = await provider.deleteResources({ paths: [RELATIVE_PATH, RELATIVE_PATH], subPath: FS_SUB_PATH, storageProvider: 'FS' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); expect(rm).toHaveBeenCalledWith(join(BASE_PATH, FS_SUB_PATH, RELATIVE_PATH), { recursive: true, force: true }); expect(rmdir).toHaveBeenCalledWith(join(BASE_PATH, FS_SUB_PATH, 'layer')); }); @@ -342,7 +343,7 @@ describe('FsStorageProvider', () => { it('should successfully call delete all files and return without failures for nested paths', async () => { const result = await provider.deleteResources({ paths: [RELATIVE_PATH, `${RELATIVE_PATH}/old`], subPath: FS_SUB_PATH, storageProvider: 'FS' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); expect(rm).toHaveBeenCalledWith(join(BASE_PATH, FS_SUB_PATH, RELATIVE_PATH), { recursive: true, force: true }); expect(rmdir).toHaveBeenNthCalledWith(1, join(BASE_PATH, FS_SUB_PATH, 'layer')); expect(rmdir).toHaveBeenNthCalledWith(2, join(BASE_PATH, FS_SUB_PATH, RELATIVE_PATH)); @@ -387,6 +388,7 @@ describe('FsStorageProvider', () => { expect(result).toEqual({ failures: new Map([['EACCES', { count: 1, sample: join(BASE_PATH, FS_SUB_PATH, RELATIVE_PATH) }]]), + deletedCount: 0, }); }); @@ -418,6 +420,7 @@ describe('FsStorageProvider', () => { expect(result).toEqual({ failures: new Map([['EACCES', { count: 7, sample: join(BASE_PATH, FS_SUB_PATH, 'layer/v0') }]]), + deletedCount: 0, }); }); @@ -437,6 +440,7 @@ describe('FsStorageProvider', () => { ['EACCES', { count: 2, sample: join(BASE_PATH, FS_SUB_PATH, 'layer/v0') }], ['EBUSY', { count: 1, sample: join(BASE_PATH, FS_SUB_PATH, 'layer/v2') }], ]), + deletedCount: 1, }); }); @@ -521,7 +525,7 @@ describe('FsStorageProvider', () => { const result = await provider.deleteResources({ paths: ['layer/v1/old'], subPath: FS_SUB_PATH, storageProvider: 'FS' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 1 }); }); it('should still attempt cleanup when rm rejects, without adding cleanup errors to the failures', async () => { @@ -533,6 +537,7 @@ describe('FsStorageProvider', () => { expect(rmdir).toHaveBeenCalledWith(join(SUB_PATH_ROOT, 'layer/v1')); expect(result).toEqual({ failures: new Map([['EACCES', { count: 1, sample: join(SUB_PATH_ROOT, 'layer/v1/old') }]]), + deletedCount: 0, }); }); }); diff --git a/tests/storageProviders/redisStorageProvider.spec.ts b/tests/storageProviders/redisStorageProvider.spec.ts new file mode 100644 index 0000000..4fcf4aa --- /dev/null +++ b/tests/storageProviders/redisStorageProvider.spec.ts @@ -0,0 +1,149 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { RedisStorageProvider } from '@src/cleaner/storageProviders/redisStorageProvider'; +import { + asRedis, + createMockLogger, + createMockRedisClient, + createMockScanningRedisClient, + createRedisStorageConfig, + type MockRedisClient, +} from '../helpers/mocks'; + +describe('RedisStorageProvider', () => { + const config = createRedisStorageConfig({ batchSize: 3, scanCount: 2 }); + let client: MockRedisClient; + let provider: RedisStorageProvider; + + beforeEach(() => { + client = createMockRedisClient(); + provider = new RedisStorageProvider(config, asRedis(client), createMockLogger()); + }); + + describe('#delete', () => { + it('should unlink every key and report how many were removed', async () => { + const keys = ['p-1-1-1', 'p-1-1-2', 'p-1-2-1']; + + const result = await provider.delete('p', keys); + + expect(client.unlinked).toEqual(keys); + expect(result.deletedCount).toBe(3); + expect(result.failures.size).toBe(0); + }); + + it('should send one unlink per batchSize chunk', async () => { + await provider.delete('p', ['a', 'b', 'c', 'd', 'e']); + + // batchSize is 3, so 5 keys means two commands. + expect(client.unlink).toHaveBeenCalledTimes(2); + expect(client.unlink).toHaveBeenNthCalledWith(1, 'a', 'b', 'c'); + expect(client.unlink).toHaveBeenNthCalledWith(2, 'd', 'e'); + }); + + it('should report deletedCount 0 when the keys were already gone, rather than a failure', async () => { + client.unlink = vi.fn().mockResolvedValue(0); + + const result = await provider.delete('p', ['missing-1-1-1']); + + expect(result.deletedCount).toBe(0); + expect(result.failures.size).toBe(0); + }); + + it('should record a failed chunk against its reason without losing the rest', async () => { + client.unlink = vi + .fn() + .mockRejectedValueOnce(new Error('READONLY')) + .mockImplementation(async (...keys: string[]) => Promise.resolve(keys.length)); + + const result = await provider.delete('p', ['a', 'b', 'c', 'd']); + + expect(result.failures.get('READONLY')).toEqual({ count: 3, sample: 'a' }); + expect(result.deletedCount).toBe(1); + }); + + it('should keep the first sample when two chunks fail for the same reason', async () => { + client.unlink = vi.fn().mockRejectedValue(new Error('READONLY')); + + const result = await provider.delete('p', ['a', 'b', 'c', 'd']); + + expect(result.failures.get('READONLY')).toEqual({ count: 4, sample: 'a' }); + expect(result.deletedCount).toBe(0); + }); + + it('should return an empty result without touching Redis when given no keys', async () => { + const result = await provider.delete('p', []); + + expect(client.unlink).not.toHaveBeenCalled(); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); + }); + }); + + describe('#deleteResources', () => { + it('should scan the prefix and unlink everything it finds', async () => { + client = createMockScanningRedisClient([['p-1-1-1', 'p-1-1-2'], ['p-2-1-1']]); + provider = new RedisStorageProvider(config, asRedis(client), createMockLogger()); + + const result = await provider.deleteResources({ storageProvider: 'REDIS', prefix: 'p' }); + + expect([...client.unlinked].sort()).toEqual(['p-1-1-1', 'p-1-1-2', 'p-2-1-1']); + expect(result.deletedCount).toBe(3); + expect(result.failures.size).toBe(0); + }); + + it('should follow the cursor until it returns to 0', async () => { + client = createMockScanningRedisClient([['a'], ['b'], ['c']]); + provider = new RedisStorageProvider(config, asRedis(client), createMockLogger()); + + await provider.deleteResources({ storageProvider: 'REDIS', prefix: 'p' }); + + expect(client.scan).toHaveBeenCalledTimes(3); + }); + + it('should keep scanning past an empty page, because MATCH filters after retrieval', async () => { + client = createMockScanningRedisClient([[], ['found']]); + provider = new RedisStorageProvider(config, asRedis(client), createMockLogger()); + + const result = await provider.deleteResources({ storageProvider: 'REDIS', prefix: 'p' }); + + expect(client.unlinked).toEqual(['found']); + expect(result.deletedCount).toBe(1); + }); + + it('should not unlink at all for an empty page', async () => { + client = createMockScanningRedisClient([[], []]); + provider = new RedisStorageProvider(config, asRedis(client), createMockLogger()); + + await provider.deleteResources({ storageProvider: 'REDIS', prefix: 'p' }); + + expect(client.unlink).not.toHaveBeenCalled(); + }); + + it('should scan with the prefix and a trailing dash wildcard', async () => { + client = createMockScanningRedisClient([[]]); + provider = new RedisStorageProvider(config, asRedis(client), createMockLogger()); + + await provider.deleteResources({ storageProvider: 'REDIS', prefix: 'layer-redis_WorldCRS84' }); + + expect(client.scan).toHaveBeenCalledWith('0', 'MATCH', 'layer-redis_WorldCRS84-*', 'COUNT', config.scanCount); + }); + + it('should report deletedCount 0 for a cold cache rather than failing', async () => { + client = createMockScanningRedisClient([[]]); + provider = new RedisStorageProvider(config, asRedis(client), createMockLogger()); + + const result = await provider.deleteResources({ storageProvider: 'REDIS', prefix: 'p' }); + + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); + }); + + it('should accumulate failures across pages without losing the count', async () => { + client = createMockScanningRedisClient([['a'], ['b']]); + client.unlink = vi.fn().mockRejectedValue(new Error('READONLY')); + provider = new RedisStorageProvider(config, asRedis(client), createMockLogger()); + + const result = await provider.deleteResources({ storageProvider: 'REDIS', prefix: 'p' }); + + expect(result.failures.get('READONLY')).toEqual({ count: 2, sample: 'a' }); + expect(result.deletedCount).toBe(0); + }); + }); +}); diff --git a/tests/storageProviders/s3StorageProvider.spec.ts b/tests/storageProviders/s3StorageProvider.spec.ts index f0a168d..c96adf8 100644 --- a/tests/storageProviders/s3StorageProvider.spec.ts +++ b/tests/storageProviders/s3StorageProvider.spec.ts @@ -5,8 +5,8 @@ import { NoSuchBucket, NotFound, paginateListObjectsV2, - S3Client, S3ServiceException, + type S3Client, type DeleteObjectsCommandOutput, } from '@aws-sdk/client-s3'; import { faker } from '@faker-js/faker'; @@ -14,7 +14,7 @@ import type { Logger } from '@map-colonies/js-logger'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { UnrecoverableError } from '@src/cleaner/errors'; import { S3StorageProvider } from '@src/cleaner/storageProviders/s3StorageProvider'; -import { createMockLogger, createS3StorageConfig, S3_VALIDATED_CONFIG_DEFAULTS } from '../helpers/mocks'; +import { createMockLogger, createS3StorageConfig } from '../helpers/mocks'; const mockSend = vi.fn(); const mockPaginateListObjectsV2Next = vi.fn(); @@ -33,7 +33,6 @@ vi.mock(import('@aws-sdk/client-s3'), async (importOriginal) => { const originModule = await importOriginal(); return { ...originModule, - S3Client: vi.fn(() => ({ send: mockSend })) as unknown as typeof S3Client, DeleteObjectsCommand: vi.fn((input: unknown) => input) as unknown as typeof DeleteObjectsCommand, ListObjectsV2Command: vi.fn((input: unknown) => input) as unknown as typeof ListObjectsV2Command, paginateListObjectsV2: vi.fn(() => mockPaginateListObjectsV2) as unknown as typeof paginateListObjectsV2, @@ -41,6 +40,7 @@ vi.mock(import('@aws-sdk/client-s3'), async (importOriginal) => { }); const BUCKET = 'test-bucket'; +const mockS3Client = { send: mockSend } as unknown as S3Client; describe('S3StorageProvider', () => { let provider: S3StorageProvider; @@ -49,26 +49,7 @@ describe('S3StorageProvider', () => { beforeEach(() => { vi.clearAllMocks(); mockLogger = createMockLogger(); - provider = new S3StorageProvider(createS3StorageConfig(), mockLogger); - }); - - describe('#constructor', () => { - it('should construct S3Client with config values', () => { - const provider = new S3StorageProvider(createS3StorageConfig(), mockLogger); - expect(S3Client).toHaveBeenCalledWith( - expect.objectContaining({ - credentials: { - accessKeyId: S3_VALIDATED_CONFIG_DEFAULTS.accessKeyId, - secretAccessKey: S3_VALIDATED_CONFIG_DEFAULTS.secretAccessKey, - }, - endpoint: S3_VALIDATED_CONFIG_DEFAULTS.endpoint, - forcePathStyle: S3_VALIDATED_CONFIG_DEFAULTS.forcePathStyle, - region: S3_VALIDATED_CONFIG_DEFAULTS.region, - tls: S3_VALIDATED_CONFIG_DEFAULTS.sslEnabled, - }) - ); - expect(provider).toBeInstanceOf(S3StorageProvider); - }); + provider = new S3StorageProvider(createS3StorageConfig(), mockS3Client, mockLogger); }); describe('#delete', () => { @@ -78,7 +59,7 @@ describe('S3StorageProvider', () => { it('should return empty failures map for empty input', async () => { const result = await provider.delete(BUCKET, []); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); expect(mockSend).not.toHaveBeenCalled(); }); @@ -99,7 +80,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, paths); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); }); it('should return failed paths tagged with the S3 error Code', async () => { @@ -110,7 +91,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, paths); - expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 1, sample: 'a.txt' }]]) }); + expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 1, sample: 'a.txt' }]]), deletedCount: 1 }); }); it('should treat NoSuchKey as a failed deletion tagged with NoSuchKey reason', async () => { @@ -121,7 +102,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, paths); - expect(result).toEqual({ failures: new Map([['NoSuchKey', { count: 1, sample: 'missing.txt' }]]) }); + expect(result).toEqual({ failures: new Map([['NoSuchKey', { count: 1, sample: 'missing.txt' }]]), deletedCount: 0 }); }); it('should fall back to Message when error has no Code', async () => { @@ -131,7 +112,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, ['a.txt']); - expect(result).toEqual({ failures: new Map([['Something bad', { count: 1, sample: 'a.txt' }]]) }); + expect(result).toEqual({ failures: new Map([['Something bad', { count: 1, sample: 'a.txt' }]]), deletedCount: 0 }); }); it('should fall back to "Unknown" when error has neither Code nor Message', async () => { @@ -141,7 +122,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, ['a.txt']); - expect(result).toEqual({ failures: new Map([['Unknown', { count: 1, sample: 'a.txt' }]]) }); + expect(result).toEqual({ failures: new Map([['Unknown', { count: 1, sample: 'a.txt' }]]), deletedCount: 0 }); }); it('should return all errors including NoSuchKey with their codes', async () => { @@ -162,12 +143,13 @@ describe('S3StorageProvider', () => { ['AccessDenied', { count: 1, sample: 'b.txt' }], ['InternalError', { count: 1, sample: 'd.txt' }], ]), + deletedCount: 0, }); }); it('should batch paths into chunks of the configured batch size', async () => { const paths = Array.from({ length: 1500 }, (_, i) => `object-${i}.txt`); - provider = new S3StorageProvider(createS3StorageConfig({ batchSize: 1000 }), mockLogger); + provider = new S3StorageProvider(createS3StorageConfig({ batchSize: 1000 }), mockS3Client, mockLogger); await provider.delete(BUCKET, paths); @@ -184,7 +166,7 @@ describe('S3StorageProvider', () => { it('should never exceed the S3 max keys limit per request for the maximum allowed batch size', async () => { const paths = Array.from({ length: 2500 }, (_, i) => `object-${i}.txt`); - provider = new S3StorageProvider(createS3StorageConfig({ batchSize: 1000 }), mockLogger); + provider = new S3StorageProvider(createS3StorageConfig({ batchSize: 1000 }), mockS3Client, mockLogger); await provider.delete(BUCKET, paths); @@ -202,7 +184,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, paths); - expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 2, sample: 'object-0.txt' }]]) }); + expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 2, sample: 'object-0.txt' }]]), deletedCount: 1498 }); }); it('should ignore returned errors that carry no Key', async () => { @@ -212,7 +194,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, ['a.txt', 'b.txt']); - expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 1, sample: 'b.txt' }]]) }); + expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 1, sample: 'b.txt' }]]), deletedCount: 1 }); }); it('should return an empty failures map when every returned error carries no Key', async () => { @@ -220,7 +202,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, ['a.txt']); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 1 }); }); it('should tag the chunk with the stringified value when send rejects with a non-Error', async () => { @@ -228,7 +210,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, ['a.txt', 'b.txt']); - expect(result).toEqual({ failures: new Map([['connection reset', { count: 2, sample: 'a.txt' }]]) }); + expect(result).toEqual({ failures: new Map([['connection reset', { count: 2, sample: 'a.txt' }]]), deletedCount: 0 }); }); it('should add entire chunk to failures tagged with the thrown error when send rejects', async () => { @@ -237,7 +219,7 @@ describe('S3StorageProvider', () => { const result = await provider.delete(BUCKET, paths); - expect(result).toEqual({ failures: new Map([['Network error', { count: 2, sample: 'a.txt' }]]) }); + expect(result).toEqual({ failures: new Map([['Network error', { count: 2, sample: 'a.txt' }]]), deletedCount: 0 }); }); }); @@ -303,7 +285,7 @@ describe('S3StorageProvider', () => { it('should return empty result when input paths is empty', async () => { const result = await provider.deleteResources({ paths: [], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -318,7 +300,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(1); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -349,7 +331,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [path], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(1); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -368,7 +350,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(2); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -387,7 +369,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(2); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -409,7 +391,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(3); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -430,7 +412,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 1 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(3); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -452,7 +434,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [objectKey], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 1 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(3); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -484,7 +466,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH1, PATH2], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 4 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(6); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -504,7 +486,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 3 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(2); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -519,7 +501,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(2); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -538,7 +520,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); expect(DeleteObjectsCommand).toHaveBeenCalledWith({ Bucket: BUCKET, Delete: { Objects: keys.map((Key) => ({ Key })) } }); expect(mockSend).toHaveBeenCalledTimes(3); }); @@ -553,7 +535,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 0 }); expect(DeleteObjectsCommand).not.toHaveBeenCalled(); expect(mockSend).toHaveBeenCalledTimes(2); }); @@ -571,7 +553,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 2, sample: 'layer/v1/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 2, sample: 'layer/v1/0/0.png' }]]), deletedCount: 0 }); }); it('should accumulate failures of the same reason across paths keeping the first sample', async () => { @@ -589,11 +571,11 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: ['layer/v1', 'layer/v2'], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 2, sample: 'layer/v1/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 2, sample: 'layer/v1/0/0.png' }]]), deletedCount: 0 }); }); it('should request pages sized by the configured batch size', async () => { - provider = new S3StorageProvider(createS3StorageConfig({ batchSize: 500 }), mockLogger); + provider = new S3StorageProvider(createS3StorageConfig({ batchSize: 500 }), mockS3Client, mockLogger); mockSend .mockResolvedValueOnce(undefined) // bucket exists .mockRejectedValueOnce(new NotFound({ $metadata: {}, message: 'not found' })); // no listing found for single object @@ -616,7 +598,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(2); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -635,7 +617,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map() }); + expect(result).toEqual({ failures: new Map(), deletedCount: 2 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(2); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -706,7 +688,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 1, sample: 'layer/v1/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['AccessDenied', { count: 1, sample: 'layer/v1/0/0.png' }]]), deletedCount: 1 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(3); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); @@ -728,7 +710,7 @@ describe('S3StorageProvider', () => { const result = await provider.deleteResources({ paths: [PATH], bucket: BUCKET, storageProvider: 'S3' }); - expect(result).toEqual({ failures: new Map([['NetworkError', { count: 1, sample: 'layer/v1/0/0.png' }]]) }); + expect(result).toEqual({ failures: new Map([['NetworkError', { count: 1, sample: 'layer/v1/0/0.png' }]]), deletedCount: 1 }); expect(mockPaginateListObjectsV2Next).toHaveBeenCalledTimes(3); expect(mockPaginateListObjectsV2Return).toHaveBeenCalledTimes(0); expect(mockPaginateListObjectsV2Throw).toHaveBeenCalledTimes(0); diff --git a/tests/storageProviders/storageConfig.spec.ts b/tests/storageProviders/storageConfig.spec.ts index a427b7e..cada7c2 100644 --- a/tests/storageProviders/storageConfig.spec.ts +++ b/tests/storageProviders/storageConfig.spec.ts @@ -2,16 +2,19 @@ import type { Logger } from '@map-colonies/js-logger'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { faker } from '@faker-js/faker'; import { ConfigurationError } from '@src/cleaner/errors'; -import { buildFsStorageConfig, buildS3StorageConfig, type FsConfig } from '@src/cleaner/storageProviders/storageConfig'; +import { buildFsStorageConfig, buildRedisStorageConfig, buildS3StorageConfig, type FsConfig } from '@src/cleaner/storageProviders/storageConfig'; import { assertCanDeleteFromFolder } from '@src/cleaner/utils/fs'; import type { ConfigType } from '@src/common/config'; import { createFsStorageConfig, createMockFsConfig, createMockLogger, + createMockRedisConfig, createMockS3Config, + createRedisStorageConfig, createS3StorageConfig, FS_STORAGE_CONFIG_DEFAULTS, + REDIS_STORAGE_CONFIG_DEFAULTS, } from '../helpers/mocks'; vi.mock('@src/cleaner/utils/fs', () => ({ @@ -133,4 +136,46 @@ describe('storageConfig', () => { expect(() => buildS3StorageConfig(config, mockLogger)).toThrow(ConfigurationError); }); }); + + describe('#buildRedisStorageConfig', () => { + it('should return the validated config with the delete batch size hoisted', () => { + const config = createMockRedisConfig(); + + const result = buildRedisStorageConfig(config, mockLogger); + + expect(result).toStrictEqual(createRedisStorageConfig()); + }); + + it('should carry optional credentials and tls through', () => { + const config = createMockRedisConfig({ username: 'user', password: 'secret', tlsEnabled: true }); + + const result = buildRedisStorageConfig(config, mockLogger); + + expect(result).toMatchObject({ username: 'user', password: 'secret', tlsEnabled: true }); + }); + + it('should throw ConfigurationError when batchSize is less than or equal to 0', () => { + const config = createMockRedisConfig({ + delete: { ...REDIS_STORAGE_CONFIG_DEFAULTS.delete, batchSize: faker.number.int({ max: 0, min: -Number.MAX_SAFE_INTEGER }) }, + }); + + expect(() => buildRedisStorageConfig(config, mockLogger)).toThrow(ConfigurationError); + }); + + it('should throw ConfigurationError when scanCount is less than or equal to 0', () => { + const config = createMockRedisConfig({ + delete: { ...REDIS_STORAGE_CONFIG_DEFAULTS.delete, scanCount: faker.number.int({ max: 0, min: -Number.MAX_SAFE_INTEGER }) }, + }); + + expect(() => buildRedisStorageConfig(config, mockLogger)).toThrow(ConfigurationError); + }); + + it('should not log the password, so a secret never reaches the logs', () => { + const config = createMockRedisConfig({ password: 'secret' }); + + buildRedisStorageConfig(config, mockLogger); + + expect(JSON.stringify(vi.mocked(mockLogger.info).mock.calls)).not.toContain('secret'); + }); + }); }); diff --git a/tests/strategies/deleteStoredResourcesStrategy.spec.ts b/tests/strategies/deleteStoredResourcesStrategy.spec.ts index 950b40f..a115fa7 100644 --- a/tests/strategies/deleteStoredResourcesStrategy.spec.ts +++ b/tests/strategies/deleteStoredResourcesStrategy.spec.ts @@ -1,3 +1,4 @@ +import { setTimeout as sleep } from 'node:timers/promises'; import type { Logger } from '@map-colonies/js-logger'; import { RedisDeleteStoredResourcesParams, @@ -6,7 +7,7 @@ import { type FsDeleteStoredResourcesParams, type S3DeleteStoredResourcesParams, } from '@map-colonies/raster-shared'; -import { beforeEach, describe, expect, it, type vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { RecoverableError, UnrecoverableError, ValidationError } from '@src/cleaner/errors'; import type { IStorageProvider, StorageProviders } from '@src/cleaner/storageProviders'; import { DeleteStoredResourcesStrategy } from '@src/cleaner/strategies/deleteStoredResourcesStrategy'; @@ -16,6 +17,10 @@ import { createMockStoredResourcesDeletionStrategyConfig, createMockLogger, crea const S3_BUCKET = 'test-bucket'; const FS_SUB_PATH = 'test/artifacts/tiles'; const PREFIX = 'layer-redis_WorldCRS84'; +const RELOAD_WINDOW_SECONDS = 308; +const MS_PER_SECOND = 1000; + +vi.mock('node:timers/promises', () => ({ setTimeout: vi.fn().mockResolvedValue(undefined) })); const s3Params: S3DeleteStoredResourcesParams = { storageProvider: StorageProvider.S3, paths: ['layer1'], bucket: S3_BUCKET }; const fsParams: FsDeleteStoredResourcesParams = { storageProvider: StorageProvider.FS, paths: ['layer2'], subPath: FS_SUB_PATH }; @@ -27,18 +32,23 @@ describe('DeleteStoredResourcesStrategy', () => { let mockS3Provider: IStorageProvider<'S3'>; // eslint-disable-next-line @typescript-eslint/naming-convention let mockFsProvider: IStorageProvider<'FS'>; + // eslint-disable-next-line @typescript-eslint/naming-convention + let mockRedisProvider: IStorageProvider<'REDIS'>; let mockLogger: Logger; let mockConfig: ConfigType; beforeEach(() => { mockS3Provider = createMockStorageProvider(); mockFsProvider = createMockStorageProvider(); + mockRedisProvider = createMockStorageProvider({ targetExists: false }); mockLogger = createMockLogger(); + vi.mocked(sleep).mockClear(); mockConfig = createMockStoredResourcesDeletionStrategyConfig(); const storageProviders: StorageProviders = { [SourceType.FS]: mockFsProvider, [SourceType.S3]: mockS3Provider, + [StorageProvider.REDIS]: mockRedisProvider, }; strategy = new DeleteStoredResourcesStrategy(mockLogger, mockConfig, storageProviders); @@ -178,6 +188,26 @@ describe('DeleteStoredResourcesStrategy', () => { expect(mockS3Provider.deleteResources).toHaveBeenCalledWith({ paths: [], bucket: S3_BUCKET, storageProvider: 'S3' }); }); + describe('REDIS provider', () => { + it('should wipe the prefix without a reload window when the task carries no delay', async () => { + await strategy.execute(redisParams); + + expect(sleep).toHaveBeenCalledExactlyOnceWith(0); + expect(mockRedisProvider.deleteResources).toHaveBeenCalledWith(redisParams); + expect(mockS3Provider.deleteResources).not.toHaveBeenCalled(); + }); + + it('should wait out the reload window before wiping when the task carries delaySeconds', async () => { + const params: RedisDeleteStoredResourcesParams = { ...redisParams, delaySeconds: RELOAD_WINDOW_SECONDS }; + + await strategy.execute(params); + + expect(sleep).toHaveBeenCalledExactlyOnceWith(RELOAD_WINDOW_SECONDS * MS_PER_SECOND); + expect(vi.mocked(sleep).mock.invocationCallOrder[0]).toBeLessThan(vi.mocked(mockRedisProvider.deleteResources).mock.invocationCallOrder[0]!); + expect(mockRedisProvider.deleteResources).toHaveBeenCalledWith(params); + }); + }); + it('should rethrow error thrown by deleteResources', async () => { const expectedError = new Error('Custom'); (mockS3Provider.deleteResources as ReturnType).mockRejectedValueOnce(expectedError); diff --git a/tests/strategies/tilesDeletionStrategy.spec.ts b/tests/strategies/tilesDeletionStrategy.spec.ts index 5d1f0e8..7c52f6a 100644 --- a/tests/strategies/tilesDeletionStrategy.spec.ts +++ b/tests/strategies/tilesDeletionStrategy.spec.ts @@ -1,6 +1,12 @@ import { faker } from '@faker-js/faker'; import type { TaskHandler as QueueClient } from '@map-colonies/mc-priority-queue'; -import { type FsTilesDeletionParams, type S3TilesDeletionParams, SourceType } from '@map-colonies/raster-shared'; +import { + type FsTilesDeletionParams, + type RedisTilesDeletionParams, + type S3TilesDeletionParams, + SourceType, + StorageProvider, +} from '@map-colonies/raster-shared'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { RecoverableError, UnrecoverableError, ValidationError } from '@src/cleaner/errors'; import type { IStorageProvider, StorageProviders } from '@src/cleaner/storageProviders'; @@ -36,8 +42,8 @@ const tilePath = (z: number, x: number, y: number): string => `${s3Params.tilesR describe('TilesDeletionStrategy', () => { let strategy: TilesDeletionStrategy; - let MockS3Provider: IStorageProvider<'S3'>; - let MockFsProvider: IStorageProvider<'FS'>; + let MockS3Provider: Required>; + let MockFsProvider: Required>; let mockUpdateProgress: ReturnType; beforeEach(() => { @@ -157,14 +163,6 @@ describe('TilesDeletionStrategy', () => { expect(MockS3Provider.delete).not.toHaveBeenCalled(); }); - it('should throw UnrecoverableError for REDIS params, whose tiles are not path addressed', async () => { - const redisParams = { storageProvider: 'REDIS', prefix: 'layer-redis_WorldCRS84', ranges: s3Params.ranges }; - - await expect(strategy.execute(strategy.validate(redisParams))).rejects.toThrow(UnrecoverableError); - expect(MockS3Provider.delete).not.toHaveBeenCalled(); - expect(MockFsProvider.delete).not.toHaveBeenCalled(); - }); - it('should throw UnrecoverableError for unknown provider', async () => { const unknownParams = { ...s3Params, storageProvider: 'UNKNOWN' } as unknown as S3TilesDeletionParams; @@ -238,6 +236,60 @@ describe('TilesDeletionStrategy', () => { }); }); + describe('REDIS provider', () => { + const redisParams: RedisTilesDeletionParams = { + storageProvider: StorageProvider.REDIS, + prefix: 'test-Orthophoto-redis_WorldCRS84', + ranges: [{ zoom: 3, minX: 1, maxX: 2, minY: 5, maxY: 6 }], + }; + let MockRedisProvider: IStorageProvider<'REDIS'>; + + const buildStrategy = (storageProviders: StorageProviders): TilesDeletionStrategy => { + const queueClient = { updateProgress: mockUpdateProgress } as unknown as QueueClient; + return new TilesDeletionStrategy(createMockLogger(), createMockStrategyConfig(), storageProviders, queueClient, TASK_CONTEXT); + }; + + beforeEach(() => { + MockRedisProvider = createMockStorageProvider({ targetExists: false }); + }); + + it('should delete redis tile keys with the prefix as storage target', async () => { + vi.mocked(MockRedisProvider.delete).mockResolvedValue({ failures: new Map(), deletedCount: 4 }); + const redisStrategy = buildStrategy({ [StorageProvider.REDIS]: MockRedisProvider }); + + await expect(redisStrategy.execute(redisParams)).resolves.toBeUndefined(); + + expect(MockRedisProvider.delete).toHaveBeenCalledWith(redisParams.prefix, [ + `${redisParams.prefix}-3-1-5`, + `${redisParams.prefix}-3-1-6`, + `${redisParams.prefix}-3-2-5`, + `${redisParams.prefix}-3-2-6`, + ]); + }); + + it('should skip the target existence check when the provider does not implement it', async () => { + vi.mocked(MockRedisProvider.delete).mockResolvedValue({ failures: new Map(), deletedCount: 0 }); + const redisStrategy = buildStrategy({ [StorageProvider.REDIS]: MockRedisProvider }); + + await expect(redisStrategy.execute(redisParams)).resolves.toBeUndefined(); + + expect(MockRedisProvider.delete).toHaveBeenCalledTimes(1); + }); + + it('should still enforce targetExists for S3', async () => { + vi.mocked(MockS3Provider.targetExists).mockResolvedValue(false); + const s3Strategy = buildStrategy({ [StorageProvider.S3]: MockS3Provider }); + + await expect(s3Strategy.execute(s3Params)).rejects.toThrow(UnrecoverableError); + }); + + it('should throw UnrecoverableError when the redis provider is not registered', async () => { + const redisStrategy = buildStrategy({}); + + await expect(redisStrategy.execute(redisParams)).rejects.toThrow(UnrecoverableError); + }); + }); + describe('progress reporting', () => { it('should call updateProgress mid-stream for large tile sets without ever setting 100', async () => { // batchSize=100, concurrency=2 → flush after 200 tiles, then final flush for remainder @@ -311,7 +363,10 @@ describe('TilesDeletionStrategy', () => { }); it('should not call updateProgress when retryable failures occur', async () => { - vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map([['AccessDenied', { count: 1, sample: tilePath(10, 0, 0) }]]) }); + vi.mocked(MockS3Provider.delete).mockResolvedValue({ + failures: new Map([['AccessDenied', { count: 1, sample: tilePath(10, 0, 0) }]]), + deletedCount: 0, + }); await expect(strategy.execute(s3Params)).rejects.toThrow(RecoverableError); @@ -319,7 +374,10 @@ describe('TilesDeletionStrategy', () => { }); it('should not call updateProgress when only not-found failures occur', async () => { - vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map([['NoSuchKey', { count: 1, sample: tilePath(10, 0, 0) }]]) }); + vi.mocked(MockS3Provider.delete).mockResolvedValue({ + failures: new Map([['NoSuchKey', { count: 1, sample: tilePath(10, 0, 0) }]]), + deletedCount: 0, + }); await expect(strategy.execute(s3Params)).resolves.toBeUndefined(); expect(mockUpdateProgress).not.toHaveBeenCalled(); @@ -328,13 +386,19 @@ describe('TilesDeletionStrategy', () => { describe('failure handling', () => { it('should throw RecoverableError when provider returns fatal failed paths', async () => { - vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map([['AccessDenied', { count: 1, sample: tilePath(10, 0, 0) }]]) }); + vi.mocked(MockS3Provider.delete).mockResolvedValue({ + failures: new Map([['AccessDenied', { count: 1, sample: tilePath(10, 0, 0) }]]), + deletedCount: 0, + }); await expect(strategy.execute(s3Params)).rejects.toThrow(RecoverableError); }); it('should include fatal failed count in RecoverableError message', async () => { - vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map([['AccessDenied', { count: 2, sample: tilePath(10, 0, 0) }]]) }); + vi.mocked(MockS3Provider.delete).mockResolvedValue({ + failures: new Map([['AccessDenied', { count: 2, sample: tilePath(10, 0, 0) }]]), + deletedCount: 0, + }); await expect(strategy.execute(s3Params)).rejects.toThrow(/Failed to delete 2/); }); @@ -345,6 +409,7 @@ describe('TilesDeletionStrategy', () => { ['EACCES', { count: 2, sample: tilePath(10, 0, 0) }], ['AccessDenied', { count: 1, sample: tilePath(10, 1, 0) }], ]), + deletedCount: 0, }); await expect(strategy.execute(s3Params)).rejects.toThrow(/Reasons: EACCES=2, AccessDenied=1/); @@ -356,13 +421,17 @@ describe('TilesDeletionStrategy', () => { ['NoSuchKey', { count: 1, sample: tilePath(10, 0, 0) }], ['AccessDenied', { count: 1, sample: tilePath(10, 0, 1) }], ]), + deletedCount: 0, }); await expect(strategy.execute(s3Params)).rejects.toThrow(/Failed to delete 1.*Reasons: AccessDenied=1/); }); it('should include path and reason in the failure sample', async () => { - vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map([['AccessDenied', { count: 1, sample: tilePath(10, 0, 0) }]]) }); + vi.mocked(MockS3Provider.delete).mockResolvedValue({ + failures: new Map([['AccessDenied', { count: 1, sample: tilePath(10, 0, 0) }]]), + deletedCount: 0, + }); await expect(strategy.execute(s3Params)).rejects.toThrow(/layer\/v1\/10\/0\/0\.png \(AccessDenied\)/); }); @@ -370,19 +439,23 @@ describe('TilesDeletionStrategy', () => { it('should resolve successfully when all failures are not-found (ENOENT)', async () => { vi.mocked(MockFsProvider.delete).mockResolvedValue({ failures: new Map([['ENOENT', { count: 1, sample: tilePath(10, 0, 0) }]]), + deletedCount: 3, }); await expect(strategy.execute(fsParams)).resolves.toBeUndefined(); }); it('should resolve successfully when all failures are not-found (NoSuchKey)', async () => { - vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map([['NoSuchKey', { count: 1, sample: tilePath(10, 0, 0) }]]) }); + vi.mocked(MockS3Provider.delete).mockResolvedValue({ + failures: new Map([['NoSuchKey', { count: 1, sample: tilePath(10, 0, 0) }]]), + deletedCount: 0, + }); await expect(strategy.execute(s3Params)).resolves.toBeUndefined(); }); it('should resolve successfully when provider returns no failed paths', async () => { - vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map() }); + vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map(), deletedCount: 4 }); await expect(strategy.execute(s3Params)).resolves.toBeUndefined(); }); @@ -436,7 +509,7 @@ describe('TilesDeletionStrategy', () => { ranges: [{ zoom: 5, minX: 0, maxX: 9, minY: 0, maxY: 19 }], }; vi.mocked(MockS3Provider.delete) - .mockResolvedValueOnce({ failures: new Map([['AccessDenied', { count: 3, sample: tilePath(5, 0, 0) }]]) }) + .mockResolvedValueOnce({ failures: new Map([['AccessDenied', { count: 3, sample: tilePath(5, 0, 0) }]]), deletedCount: 0 }) .mockRejectedValueOnce(new Error('S3 connection lost')); // reasons are ordered by descending count @@ -455,5 +528,91 @@ describe('TilesDeletionStrategy', () => { await expect(strategy.execute(fsParams)).resolves.toBeUndefined(); }); }); + + describe('deleted count reporting', () => { + let mockLogger: ReturnType; + + function buildStrategyWithLogger(batchSize = 100): TilesDeletionStrategy { + mockLogger = createMockLogger(); + const storageProviders: StorageProviders = { [SourceType.S3]: MockS3Provider }; + const queueClient = { updateProgress: mockUpdateProgress } as unknown as QueueClient; + const config = createMockStrategyConfig({ + 'strategies.tilesDeletion.batchSize': batchSize, + 'strategies.tilesDeletion.concurrency': 2, + }); + return new TilesDeletionStrategy(mockLogger, config, storageProviders, queueClient, TASK_CONTEXT); + } + + it('should report the count the provider observed rather than the number of tiles attempted', async () => { + // s3Params spans 4 tiles; the store reports only 2 existed + vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map(), deletedCount: 2 }); + + await buildStrategyWithLogger().execute(s3Params); + + expect(mockLogger.info).toHaveBeenCalledWith(expect.objectContaining({ msg: 'Tiles deletion completed successfully', deletedCount: 2 })); + }); + + it('should sum the observed count across batches', async () => { + // batchSize 1 over a 2x2 range → four delete calls + vi.mocked(MockS3Provider.delete) + .mockResolvedValueOnce({ failures: new Map(), deletedCount: 1 }) + .mockResolvedValueOnce({ failures: new Map(), deletedCount: 1 }) + .mockResolvedValue({ failures: new Map(), deletedCount: 0 }); + + await buildStrategyWithLogger(1).execute(s3Params); + + expect(vi.mocked(MockS3Provider.delete)).toHaveBeenCalledTimes(4); + expect(mockLogger.info).toHaveBeenCalledWith(expect.objectContaining({ msg: 'Tiles deletion completed successfully', deletedCount: 2 })); + }); + + it('should report zero deletions when the store found nothing, which is the wrong-prefix signal', async () => { + vi.mocked(MockS3Provider.delete).mockResolvedValue({ failures: new Map(), deletedCount: 0 }); + + await buildStrategyWithLogger().execute(s3Params); + + expect(mockLogger.info).toHaveBeenCalledWith(expect.objectContaining({ msg: 'Tiles deletion completed successfully', deletedCount: 0 })); + }); + + it('should count a hard-rejected batch as zero deletions', async () => { + // batchSize 1, concurrency 2: the first window has one success and one rejection. + vi.mocked(MockS3Provider.delete) + .mockResolvedValueOnce({ failures: new Map(), deletedCount: 1 }) + .mockRejectedValueOnce(new Error('S3 connection lost')) + .mockResolvedValue({ failures: new Map(), deletedCount: 1 }); + + await expect(buildStrategyWithLogger(1).execute(s3Params)).rejects.toThrow(RecoverableError); + + expect(mockLogger.error).toHaveBeenCalledWith(expect.objectContaining({ msg: 'Tiles deletion partially failed', deletedCount: 3 })); + }); + + it('should report the observed count in the partial-failure log too', async () => { + vi.mocked(MockS3Provider.delete) + .mockResolvedValueOnce({ failures: new Map(), deletedCount: 0 }) + .mockResolvedValue({ failures: new Map([['AccessDenied', { count: 1, sample: tilePath(10, 0, 0) }]]), deletedCount: 0 }); + + await expect(buildStrategyWithLogger(1).execute(s3Params)).rejects.toThrow(RecoverableError); + + expect(mockLogger.error).toHaveBeenCalledWith(expect.objectContaining({ msg: 'Tiles deletion partially failed', deletedCount: 0 })); + }); + + it('should report the not-found tile count, not the number of not-found reasons', async () => { + vi.mocked(MockS3Provider.delete).mockResolvedValue({ + failures: new Map([['NoSuchKey', { count: 3, sample: tilePath(10, 0, 0) }]]), + deletedCount: 1, + }); + + await buildStrategyWithLogger().execute(s3Params); + + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.objectContaining({ + msg: 'Tiles deletion completed with missing tiles', + totalTiles: 4, + notFoundCount: 3, + deletedCount: 1, + allTilesMissing: false, + }) + ); + }); + }); }); });