From b83e95d190e89054f8ba95a932d73e3b35a84c2c Mon Sep 17 00:00:00 2001 From: Roi Cohen <213414225+roicohen326@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:38:57 +0300 Subject: [PATCH 01/11] feat: handle Delete_Layer tasks flow Add a DeleteLayerJobHandler for the new Delete_Layer job type, wiring its task flow (delete -> tiles-deletion -> artifacts-deletion -> finalize) through config, helm, and the job handler factory. The tiles-deletion and artifacts-deletion tasks are excluded from auto-creation since another service creates them with real per-layer parameters job-tracker doesn't have. Bumps @map-colonies/raster-shared to 8.3.0-alpha.0 (exact pin, matching ingestion-trigger) since the deletion types are only available on the alpha track. --- config/custom-environment-variables.json | 11 +- config/default.json | 11 +- helm/templates/configmap.yaml | 4 + helm/values.yaml | 11 + package-lock.json | 55 ++--- package.json | 2 +- src/common/interfaces.ts | 3 + src/common/mappers.ts | 1 + .../deleteLayer/deleteLayerHandler.ts | 29 +++ src/tasks/handlers/jobHandlerFactory.ts | 4 + .../tasks/deleteLayer/taskManager.spec.ts | 195 ++++++++++++++++++ tests/mocks/configMock.ts | 5 + tests/mocks/jobMocks.ts | 33 +++ tests/unit/tasks/handlers/jobHandler.spec.ts | 1 + .../tasks/handlers/jobHandlerFactory.spec.ts | 15 ++ 15 files changed, 337 insertions(+), 43 deletions(-) create mode 100644 src/tasks/handlers/deleteLayer/deleteLayerHandler.ts create mode 100644 tests/integration/tasks/deleteLayer/taskManager.spec.ts diff --git a/config/custom-environment-variables.json b/config/custom-environment-variables.json index 03fbc73..4820e7b 100644 --- a/config/custom-environment-variables.json +++ b/config/custom-environment-variables.json @@ -68,7 +68,8 @@ "update": "JOB_DEFINITIONS_JOB_UPDATE", "swapUpdate": "JOB_DEFINITIONS_JOB_SWAP_UPDATE", "export": "JOB_DEFINITIONS_JOB_EXPORT", - "seed": "JOB_DEFINITIONS_JOB_SEED" + "seed": "JOB_DEFINITIONS_JOB_SEED", + "deleteLayer": "JOB_DEFINITIONS_JOB_DELETE_LAYER" }, "tasks": { "validation": "JOB_DEFINITIONS_TASK_VALIDATION", @@ -79,7 +80,9 @@ "polygonParts": "JOB_DEFINITIONS_TASK_POLYGON_PARTS", "export": "JOB_DEFINITIONS_TASK_EXPORT", "finalize": "JOB_DEFINITIONS_TASK_FINALIZE", - "seed": "JOB_DEFINITIONS_TASK_SEED" + "seed": "JOB_DEFINITIONS_TASK_SEED", + "delete": "JOB_DEFINITIONS_TASK_DELETE", + "artifactsDeletion": "JOB_DEFINITIONS_TASK_ARTIFACTS_DELETION" }, "suspendingTaskTypes": { "__name": "JOB_DEFINITIONS_SUSPENDING_TASKS", @@ -98,6 +101,10 @@ "seedTasksFlow": { "__name": "SEED_TASKS_FLOW", "__format": "json" + }, + "deleteLayerTasksFlow": { + "__name": "DELETE_LAYER_TASKS_FLOW", + "__format": "json" } } } diff --git a/config/default.json b/config/default.json index 7d9eae3..e0de6ee 100644 --- a/config/default.json +++ b/config/default.json @@ -53,7 +53,8 @@ "update": "Ingestion_Update", "swapUpdate": "Ingestion_Swap_Update", "export": "Export", - "seed": "TilesSeeding" + "seed": "TilesSeeding", + "deleteLayer": "Delete_Layer" }, "tasks": { "validation": "validation", @@ -63,13 +64,17 @@ "createTasks": "create-tasks", "export": "tilesExporting", "finalize": "finalize", - "seed": "TilesSeeding" + "seed": "TilesSeeding", + "tilesDeletion": "tiles-deletion", + "delete": "delete", + "artifactsDeletion": "artifacts-deletion" }, "suspendingTaskTypes": ["polygon-parts"] }, "taskFlowManager": { "ingestionTasksFlow": ["validation", "create-merge-tasks", "tilesMerging", "finalize"], "exportTasksFlow": ["init", "tilesExporting", "polygon-parts", "finalize"], - "seedTasksFlow": ["TilesSeeding"] + "seedTasksFlow": ["TilesSeeding"], + "deleteLayerTasksFlow": ["delete", "tiles-deletion", "artifacts-deletion", "finalize"] } } diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index 4c7c231..11ef2ad 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -33,6 +33,7 @@ data: JOB_DEFINITIONS_JOB_SWAP_UPDATE: {{ $jobDefinitions.jobs.swapUpdate.type | quote }} JOB_DEFINITIONS_JOB_EXPORT: {{ $jobDefinitions.jobs.export.type | quote }} JOB_DEFINITIONS_JOB_SEED: {{ $jobDefinitions.jobs.seed.type | quote }} + JOB_DEFINITIONS_JOB_DELETE_LAYER: {{ $jobDefinitions.jobs.deleteLayer.type | quote }} JOB_DEFINITIONS_TASK_INIT: {{ $jobDefinitions.tasks.init.type | quote }} JOB_DEFINITIONS_TASK_VALIDATION: {{ $jobDefinitions.tasks.validation.type | quote }} JOB_DEFINITIONS_TASK_MERGE: {{ $jobDefinitions.tasks.merge.type | quote }} @@ -42,10 +43,13 @@ data: JOB_DEFINITIONS_TASK_EXPORT: {{ $jobDefinitions.tasks.export.type | quote }} JOB_DEFINITIONS_TASK_FINALIZE: {{ $jobDefinitions.tasks.finalize.type | quote }} JOB_DEFINITIONS_TASK_SEED: {{ $jobDefinitions.tasks.seed.type | quote }} + JOB_DEFINITIONS_TASK_DELETE: {{ $jobDefinitions.tasks.delete.type | quote }} + JOB_DEFINITIONS_TASK_ARTIFACTS_DELETION: {{ $jobDefinitions.tasks.artifactsDeletion.type | quote }} JOB_DEFINITIONS_SUSPENDING_TASKS: {{ $suspendingTaskTypes | quote }} INGESTION_TASKS_FLOW: {{ .Values.taskFlowManager.ingestionTasksFlow | toJson | quote }} EXPORT_TASKS_FLOW: {{ .Values.taskFlowManager.exportTasksFlow | toJson | quote }} SEED_TASKS_FLOW: {{ .Values.taskFlowManager.seedTasksFlow | toJson | quote }} + DELETE_LAYER_TASKS_FLOW: {{ .Values.taskFlowManager.deleteLayerTasksFlow | toJson | quote }} {{- with .Values.configManagement }} CONFIG_NAME: {{ .name | quote }} CONFIG_VERSION: {{ .version | quote }} diff --git a/helm/values.yaml b/helm/values.yaml index 20b566c..0efd7be 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -95,6 +95,8 @@ jobDefinitions: type: '' seed: type: '' + deleteLayer: + type: '' tasks: validation: type: '' @@ -115,6 +117,10 @@ jobDefinitions: type: '' seed: type: '' + delete: + type: '' + artifactsDeletion: + type: '' taskFlowManager: ingestionTasksFlow: @@ -130,6 +136,11 @@ taskFlowManager: - finalize seedTasksFlow: - TilesSeeding + deleteLayerTasksFlow: + - delete + - tiles-deletion + - artifacts-deletion + - finalize env: port: 80 targetPort: 8080 diff --git a/package-lock.json b/package-lock.json index 6a25788..974f6c1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,7 @@ "@map-colonies/mc-utils": "^5.1.0", "@map-colonies/openapi-express-viewer": "^3.0.0", "@map-colonies/prometheus": "^1.0.0", - "@map-colonies/raster-shared": "^7.10.2", + "@map-colonies/raster-shared": "8.3.0-alpha.0", "@map-colonies/read-pkg": "0.0.1", "@map-colonies/schemas": "^1.18.0", "@map-colonies/telemetry": "^6.0.0", @@ -161,7 +161,6 @@ "resolved": "https://registry.npmjs.org/@babel/core/-/core-7.18.2.tgz", "integrity": "sha512-A8pri1YJiC5UnkdrWcmfZTJTV85b4UXTAfImGmCfYmax4TR9Cw8sDS0MOk++Gp2mE/BefVJ5nwy5yzqNJbP/DQ==", "dev": true, - "peer": true, "dependencies": { "@ampproject/remapping": "^2.1.0", "@babel/code-frame": "^7.16.7", @@ -3287,7 +3286,6 @@ "resolved": "https://registry.npmjs.org/@map-colonies/mc-utils/-/mc-utils-5.1.0.tgz", "integrity": "sha512-4eL4ykgJFrFSPP18AbeJUJklwmzJieormWIvEc8c8PFRXU5eN1lRUal60xAhghpmzEQnudRG1kV3NWIibUjw2w==", "license": "ISC", - "peer": true, "dependencies": { "@map-colonies/types": "^1.9.0", "@turf/turf": "^6.5.0", @@ -3414,9 +3412,9 @@ } }, "node_modules/@map-colonies/raster-shared": { - "version": "7.10.2", - "resolved": "https://registry.npmjs.org/@map-colonies/raster-shared/-/raster-shared-7.10.2.tgz", - "integrity": "sha512-EKPtETrKPWOZTpVQ0CEs2CFaC2cU9FlXa+Ylss+Jg7wmFEaN4BKR2hUw5kjtsgD69lqL9fyFCWQysBFc7h4iTQ==", + "version": "8.3.0-alpha.0", + "resolved": "https://registry.npmjs.org/@map-colonies/raster-shared/-/raster-shared-8.3.0-alpha.0.tgz", + "integrity": "sha512-xcleqPf+KSOObCqHuKm1nng7WQRAZ0JrX12qe1U8pX8iijZ7pS7QMrHnPM7DFU5cxMoxkgnRC8hVzd1SMLiErQ==", "license": "ISC", "dependencies": { "@map-colonies/mc-priority-queue": "^9.1.0", @@ -3440,8 +3438,7 @@ "version": "1.18.0", "resolved": "https://registry.npmjs.org/@map-colonies/schemas/-/schemas-1.18.0.tgz", "integrity": "sha512-G6OB/xogqzcLrDU+sXok8ECFbdzQDjI9AE+eMZP6Bcl8BZArkkFI/wNz14HCD2NE81p4SNaeAKUuyGZBqzRJyg==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@map-colonies/telemetry": { "version": "6.0.0", @@ -3467,7 +3464,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.7.0.tgz", "integrity": "sha512-AdY5wvN0P2vXBi3b29hxZgSFvdhdxPB9+f0B6s//P9Q8nibRWeA3cHm8UmLpio9ABigkVHJ5NMPk+Mz8VCCyrw==", "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=8.0.0" } @@ -3772,7 +3768,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.6.0.tgz", "integrity": "sha512-HLM1v2cbZ4TgYN6KEOj+Bbj8rAKriOdkF9Ed3tG25FoprSiQl7kYc+RRT6fUZGOvx0oMi5U67GoFdT+XUn8zEg==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/semantic-conventions": "^1.29.0" }, @@ -4945,7 +4940,6 @@ "resolved": "https://registry.npmjs.org/@map-colonies/types/-/types-1.9.0.tgz", "integrity": "sha512-TUM82wWLCV49vFTs16h4OYo15FjlnhMeMGLE922CeA0QNJyEkyaJG7IHvrM0LG793Ep4qeBMeB1izJQaN2fVYQ==", "license": "ISC", - "peer": true, "dependencies": { "@types/geojson": "^7946.0.16", "@types/mime-types": "^2.1.1", @@ -5022,7 +5016,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=8.0.0" } @@ -5031,7 +5024,6 @@ "version": "0.46.0", "resolved": "https://registry.npmjs.org/@opentelemetry/api-logs/-/api-logs-0.46.0.tgz", "integrity": "sha512-+9BcqfiEDGPXEIo+o3tso/aqGM5dGbGwAkGVp3FPpZ8GlkK1YlaKRd9gMVyPaeRATwvO5wYGGnCsAc/sMMM9Qw==", - "peer": true, "dependencies": { "@opentelemetry/api": "^1.0.0" }, @@ -9626,6 +9618,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">=10" } @@ -9642,6 +9635,7 @@ "os": [ "darwin" ], + "peer": true, "engines": { "node": ">=10" } @@ -9658,6 +9652,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=10" } @@ -9674,6 +9669,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=10" } @@ -9690,6 +9686,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=10" } @@ -9706,6 +9703,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=10" } @@ -9722,6 +9720,7 @@ "os": [ "linux" ], + "peer": true, "engines": { "node": ">=10" } @@ -9738,6 +9737,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=10" } @@ -9754,6 +9754,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=10" } @@ -9770,6 +9771,7 @@ "os": [ "win32" ], + "peer": true, "engines": { "node": ">=10" } @@ -9802,6 +9804,7 @@ "resolved": "https://registry.npmjs.org/@swc/types/-/types-0.1.25.tgz", "integrity": "sha512-iAoY/qRhNH8a/hBvm3zKj9qQ4oc2+3w1unPJa2XvTK3XjeLXtzcCingVPw/9e5mn1+0yPqxcBGp9Jf0pkfMb1g==", "license": "Apache-2.0", + "peer": true, "dependencies": { "@swc/counter": "^0.1.3" } @@ -11824,8 +11827,7 @@ "node_modules/@types/json-schema": { "version": "7.0.15", "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.15.tgz", - "integrity": "sha512-5+fP8P8MFNC+AyZCDxrB2pkZFPGzqQWUzpSeuuVLvm8VMcorNYavBqoFcxK8bQz4Qsbn4oUEEem4wDLfcysGHA==", - "peer": true + "integrity": "sha512-5+fP8P8MFNC+AyZCDxrB2pkZFPGzqQWUzpSeuuVLvm8VMcorNYavBqoFcxK8bQz4Qsbn4oUEEem4wDLfcysGHA==" }, "node_modules/@types/keygrip": { "version": "1.0.6", @@ -11940,8 +11942,7 @@ "version": "20.5.1", "resolved": "https://registry.npmjs.org/@types/node/-/node-20.5.1.tgz", "integrity": "sha512-4tT2UrL5LBqDwoed9wZ6N3umC4Yhz3W3FloMmiiG4JwmUJWpie0c7lcnUNd4gtMKuDEO4wRVS8B6Xa0uMRsMKg==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@types/normalize-package-data": { "version": "2.4.1", @@ -12145,7 +12146,6 @@ "integrity": "sha512-klQbnPAAiGYFyI02+znpBRLyjL4/BrBd0nyWkdC0s/6xFLkXYQ8OoRrSkqacS1ddVxf/LDyODIKbQ5TgKAf/Fg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.56.1", "@typescript-eslint/types": "8.56.1", @@ -12681,7 +12681,6 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.16.0.tgz", "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -12745,7 +12744,6 @@ "resolved": "https://registry.npmjs.org/ajv/-/ajv-8.18.0.tgz", "integrity": "sha512-PlXPeEWMXMZ7sPYOHqmDyCJzcfNrUr3fGNKtezX14ykXOEIvyK81d+qydx89KY5O71FKMPaQ2vBfBFI5NHR63A==", "license": "MIT", - "peer": true, "dependencies": { "fast-deep-equal": "^3.1.3", "fast-uri": "^3.0.1", @@ -12914,7 +12912,6 @@ "resolved": "https://registry.npmjs.org/axios/-/axios-0.21.4.tgz", "integrity": "sha512-ut5vewkiu8jjGBdqpM44XxjuCjq9LAKeHVmoVfHVzy8eHgxxq8SbAVQNovDA8mVi05kP0Ea/n/UzcSHcTJQfNg==", "license": "MIT", - "peer": true, "dependencies": { "follow-redirects": "^1.14.0" } @@ -13341,7 +13338,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -14213,7 +14209,6 @@ "integrity": "sha512-kcZ6+W5QzcJ3P1Mt+83OUv/oHFqZHIx8DuxG6eZ5RGMERoLqp4BuGjhHLYGK+Kf5XVkQvqBSmAy/nGWN3qDgEA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "import-fresh": "^3.3.0", "js-yaml": "^4.1.0", @@ -15096,7 +15091,6 @@ "integrity": "sha512-hr4ihw+DBqcvrsEDioRO31Z17x71pUYoNe/4h6Z0wB72p7MU7/9gH8Q3s12NFhHPfYBBOV3qyfUxmr/Yn3shnQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "env-paths": "^2.2.1", "import-fresh": "^3.3.0", @@ -15929,7 +15923,6 @@ "integrity": "sha512-vPZZsiOKaBAIATpFE2uMI4w5IRwdv/FpQ+qZZMR4E+PeOcM4OeoEbqxRMnywdxP19TyB/3h6QBB0EWon7letSQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/types": "^8.35.0", "comment-parser": "^1.4.1", @@ -16006,7 +15999,6 @@ "integrity": "sha512-ZCGr7vTH2WSo2hrK5oM2RULFmMruQ7W3cX7YfwoTiPfzTGTFBMmrVIz45jZHd++cGKj/kWf02li/RhTGcANJSA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/utils": "^8.0.0" }, @@ -16519,7 +16511,6 @@ "resolved": "https://registry.npmjs.org/express/-/express-4.22.1.tgz", "integrity": "sha512-F2X8g9P1X7uCPZMA3MVf9wcTqlyNp7IhH5qPCI0izhaOIYXaW9L535tGA3qmjRzpH+bZczqq7hVKxTR4NWnu+g==", "license": "MIT", - "peer": true, "dependencies": { "accepts": "~1.3.8", "array-flatten": "1.1.1", @@ -18383,7 +18374,6 @@ "resolved": "https://registry.npmjs.org/jest/-/jest-29.5.0.tgz", "integrity": "sha512-juMg3he2uru1QoXX078zTa7pO85QyB9xajZc6bU+d9yEGwrKX6+vGmJQ3UdVZsvTEUARIdObzH68QItim6OSSQ==", "dev": true, - "peer": true, "dependencies": { "@jest/core": "^29.5.0", "@jest/types": "^29.5.0", @@ -21050,7 +21040,6 @@ "integrity": "sha512-UczzB+0nnwGotYSgllfARAqWCJ5e/skuV2K/l+Zyck/H6pJIhLXuBnz+6vn2i211o7DtbE78HQtsYEKICHGI+g==", "dev": true, "license": "MIT", - "peer": true, "funding": { "type": "opencollective", "url": "https://opencollective.com/mobx" @@ -22342,7 +22331,6 @@ "resolved": "https://registry.npmjs.org/pino/-/pino-10.3.1.tgz", "integrity": "sha512-r34yH/GlQpKZbU1BvFFqOjhISRo1MNx1tWYsYvmj6KIRHSPMT2+yHOEb1SG6NMvRoHRF0a07kCOox/9yakl1vg==", "license": "MIT", - "peer": true, "dependencies": { "@pinojs/redact": "^0.4.0", "atomic-sleep": "^1.0.0", @@ -22613,7 +22601,6 @@ "integrity": "sha512-UOnG6LftzbdaHZcKoPFtOcCKztrQ57WkHDeRD9t/PTQtmT0NHSeWWepj6pS0z/N7+08BHFDQVUrfmfMRcZwbMg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -22766,7 +22753,6 @@ "resolved": "https://registry.npmjs.org/prom-client/-/prom-client-15.1.3.tgz", "integrity": "sha512-6ZiOBfCywsD4k1BN9IX0uZhF+tJkV8q8llP64G5Hajs4JOeVLPCwpPVcpXy3BwYiUGgyJzsJJQeOIv7+hDSq8g==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/api": "^1.4.0", "tdigest": "^0.1.1" @@ -23008,7 +22994,6 @@ "integrity": "sha512-Ku/hhYbVjOQnXDZFv2+RibmLFGwFdeeKHFcOTlrt7xplBnya5OGn/hIRDsqDiSUcfORsDC7MPxwork8jBwsIWA==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -23019,7 +23004,6 @@ "integrity": "sha512-yELu4WmLPw5Mr/lmeEpox5rw3RETacE++JgHqQzd2dg+YbJuat3jH4ingc+WPZhxaoFzdv9y33G+F7Nl5O0GBg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -24377,7 +24361,6 @@ "integrity": "sha512-J72R4ltw0UBVUlEjTzI0gg2STOqlI9JBhQOL4Dxt7aJOnnSesy0qJDn4PYfMCafk9cWOaVg129Pesl5o+DIh0Q==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@emotion/is-prop-valid": "1.4.0", "@emotion/unitless": "0.10.0", @@ -24687,7 +24670,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -24890,7 +24872,6 @@ "resolved": "https://registry.npmjs.org/ts-node/-/ts-node-10.9.1.tgz", "integrity": "sha512-NtVysVPkxxrwFGUUxGYhfux8k78pQB3JqYBXlLRZgdGUqTO5wU/UyHop5p70iEbGhB7q5KmiZiU0Y3KlJrScEw==", "dev": true, - "peer": true, "dependencies": { "@cspotcode/source-map-support": "^0.8.0", "@tsconfig/node10": "^1.0.7", diff --git a/package.json b/package.json index c1f43af..a12bc0e 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "@map-colonies/mc-utils": "^5.1.0", "@map-colonies/openapi-express-viewer": "^3.0.0", "@map-colonies/prometheus": "^1.0.0", - "@map-colonies/raster-shared": "^7.10.2", + "@map-colonies/raster-shared": "8.3.0-alpha.0", "@map-colonies/read-pkg": "0.0.1", "@map-colonies/schemas": "^1.18.0", "@map-colonies/telemetry": "^6.0.0", diff --git a/src/common/interfaces.ts b/src/common/interfaces.ts index 7d4e60e..dbcd3d5 100644 --- a/src/common/interfaces.ts +++ b/src/common/interfaces.ts @@ -22,6 +22,7 @@ export interface IJobDefinitionsConfig { swapUpdate: string; export: string; seed: string; + deleteLayer: string; }; tasks: { polygonParts: string; @@ -33,6 +34,8 @@ export interface IJobDefinitionsConfig { export: string; seed: string; tilesDeletion: string; + delete: string; + artifactsDeletion: string; }; suspendingTaskTypes: string[]; } diff --git a/src/common/mappers.ts b/src/common/mappers.ts index 5c24d89..24a5ef5 100644 --- a/src/common/mappers.ts +++ b/src/common/mappers.ts @@ -46,5 +46,6 @@ export const createTaskParametersMapper = (jobDefinitions: IJobDefinitionsConfig callbacksSent: false, } satisfies ExportFinalizeFullProcessingParams, ], + [`${jobDefinitions.jobs.deleteLayer}_${jobDefinitions.tasks.finalize}`, {}], ]); }; diff --git a/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts b/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts new file mode 100644 index 0000000..7cd7b34 --- /dev/null +++ b/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts @@ -0,0 +1,29 @@ +import type { Logger } from '@map-colonies/js-logger'; +import type { IJobResponse, ITaskResponse, JobManagerClient } from '@map-colonies/mc-priority-queue'; +import { injectable, inject } from 'tsyringe'; +import type { ConfigType } from '@src/common/config'; +import type { TaskTypes } from '../../../common/interfaces'; +import { SERVICES } from '../../../common/constants'; +import { JobHandler } from '../jobHandler'; + +@injectable() +export class DeleteLayerJobHandler extends JobHandler { + protected readonly tasksFlow: TaskTypes; + protected readonly excludedTypes: TaskTypes; + protected readonly blockedDuplicationTypes: TaskTypes; + + public constructor( + @inject(SERVICES.LOGGER) logger: Logger, + @inject(SERVICES.CONFIG) config: ConfigType, + jobManagerClient: JobManagerClient, + job: IJobResponse, + task: ITaskResponse + ) { + super(logger, config, jobManagerClient, job, task); + this.tasksFlow = this.config.get('taskFlowManager.deleteLayerTasksFlow') as unknown as TaskTypes; + this.excludedTypes = [this.jobDefinitions.tasks.tilesDeletion, this.jobDefinitions.tasks.artifactsDeletion]; + this.blockedDuplicationTypes = [this.jobDefinitions.tasks.finalize]; + + this.initializeTaskOperations(); + } +} diff --git a/src/tasks/handlers/jobHandlerFactory.ts b/src/tasks/handlers/jobHandlerFactory.ts index 143e4c8..47c9fa8 100644 --- a/src/tasks/handlers/jobHandlerFactory.ts +++ b/src/tasks/handlers/jobHandlerFactory.ts @@ -7,6 +7,7 @@ import type { JobHandler } from './jobHandler'; import { IngestionJobHandler } from './ingestion/ingestionHandler'; import { ExportJobHandler } from './export/exportHandler'; import { SeedJobHandler } from './seed/seedHandler'; +import { DeleteLayerJobHandler } from './deleteLayer/deleteLayerHandler'; export function getJobHandler( jobHandlerType: string, @@ -31,6 +32,9 @@ export function getJobHandler( case jobDefinitions.jobs.seed: { return new SeedJobHandler(logger, config, jobManagerClient, job, task); } + case jobDefinitions.jobs.deleteLayer: { + return new DeleteLayerJobHandler(logger, config, jobManagerClient, job, task); + } default: throw new BadRequestError(`${jobHandlerType} job type is invalid`); } diff --git a/tests/integration/tasks/deleteLayer/taskManager.spec.ts b/tests/integration/tasks/deleteLayer/taskManager.spec.ts new file mode 100644 index 0000000..db7ac01 --- /dev/null +++ b/tests/integration/tasks/deleteLayer/taskManager.spec.ts @@ -0,0 +1,195 @@ +import nock, { cleanAll, isDone, pendingMocks } from 'nock'; +import { OperationStatus } from '@map-colonies/mc-priority-queue'; +import { StatusCodes as httpStatusCodes } from 'http-status-codes'; +import { trace } from '@opentelemetry/api'; +import { jsLogger } from '@map-colonies/js-logger'; +import { initConfig } from '../../../../src/common/config'; +import { configMock } from '../../../mocks/configMock'; +import { getApp } from '../../../../src/app'; +import type { IJobManagerConfig, IJobDefinitionsConfig } from '../../../../src/common/interfaces'; +import { getDeleteLayerJobMock, getTaskMock } from '../../../mocks/jobMocks'; +import { calculateJobPercentage } from '../../../../src/utils/jobUtils'; +import { SERVICES } from '../../../../src/common/constants'; +import { registerExternalValues } from '../../../../src/containerConfig'; +import { TasksRequestSender } from '../helpers/requestSender'; +import { getTestContainerConfig, resetContainer } from '../helpers/containerConfig'; + +describe('tasks', function () { + let requestSender: TasksRequestSender; + let jobManagerConfigMock: IJobManagerConfig; + let jobDefinitionsConfig: IJobDefinitionsConfig; + + beforeAll(async function () { + await initConfig(true); + }); + + beforeEach(async function () { + const [app] = await getApp({ + override: [...(await getTestContainerConfig())], + useChild: true, + }); + + await registerExternalValues({ + override: [ + { token: SERVICES.LOGGER, provider: { useValue: await jsLogger({ enabled: false }) } }, + { token: SERVICES.CONFIG, provider: { useValue: configMock } }, + { token: SERVICES.TRACER, provider: { useValue: trace.getTracer('testTracer') } }, + ], + }); + + requestSender = new TasksRequestSender(app); + jobManagerConfigMock = configMock.get('jobManagement.config') as unknown as IJobManagerConfig; + jobDefinitionsConfig = configMock.get('jobDefinitions') as IJobDefinitionsConfig; + cleanAll(); + }); + + afterEach(function () { + resetContainer(); + jest.restoreAllMocks(); + if (!isDone()) { + throw new Error(`Not all nock interceptors were used: ${JSON.stringify(pendingMocks())}`); + } + }); + + describe('Happy Path', function () { + it('should return 200 and create finalize task when getting completed delete task and no other task is pending', async () => { + // mocks + const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 1, completedTasks: 1 }); + const mockDeleteTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks.delete, + status: OperationStatus.COMPLETED, + }); + const mockFinalizeTaskBody = { + parameters: {}, + type: jobDefinitionsConfig.tasks.finalize, + blockDuplication: true, + }; + + nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockDeleteTask.id }).reply(httpStatusCodes.OK, [mockDeleteTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .get(`/jobs/${mockDeleteLayerJob.id}`) + .query({ shouldReturnTasks: false }) + .reply(httpStatusCodes.OK, mockDeleteLayerJob); + nock(jobManagerConfigMock.jobManagerBaseUrl).post(`/jobs/${mockDeleteLayerJob.id}/tasks`, mockFinalizeTaskBody).reply(httpStatusCodes.CREATED); + const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount + 1); + nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); + + // action + const response = await requestSender.handleTaskNotification(mockDeleteTask.id); + + // expectation + expect(response.status).toBe(httpStatusCodes.OK); + expect(response).toSatisfyApiSpec(); + }); + + it('should return 200 and only update progress when getting completed delete task while other deletion tasks are still pending', async () => { + // mocks + const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 3, completedTasks: 1 }); + const mockDeleteTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks.delete, + status: OperationStatus.COMPLETED, + }); + + nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockDeleteTask.id }).reply(httpStatusCodes.OK, [mockDeleteTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .get(`/jobs/${mockDeleteLayerJob.id}`) + .query({ shouldReturnTasks: false }) + .reply(httpStatusCodes.OK, mockDeleteLayerJob); + const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount); + nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); + + // action + const response = await requestSender.handleTaskNotification(mockDeleteTask.id); + + // expectation + expect(response.status).toBe(httpStatusCodes.OK); + expect(response).toSatisfyApiSpec(); + }); + + it.each([{ taskTypeKey: 'tilesDeletion' as const }, { taskTypeKey: 'artifactsDeletion' as const }])( + 'should return 200 and create finalize task when getting completed excluded "$taskTypeKey" task and no other task is pending', + async ({ taskTypeKey }) => { + // mocks + const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 3, completedTasks: 3 }); + const mockExcludedTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks[taskTypeKey], + status: OperationStatus.COMPLETED, + }); + const mockFinalizeTaskBody = { + parameters: {}, + type: jobDefinitionsConfig.tasks.finalize, + blockDuplication: true, + }; + + nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockExcludedTask.id }).reply(httpStatusCodes.OK, [mockExcludedTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .get(`/jobs/${mockDeleteLayerJob.id}`) + .query({ shouldReturnTasks: false }) + .reply(httpStatusCodes.OK, mockDeleteLayerJob); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .post(`/jobs/${mockDeleteLayerJob.id}/tasks`, mockFinalizeTaskBody) + .reply(httpStatusCodes.CREATED); + const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount + 1); + nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); + + // action + const response = await requestSender.handleTaskNotification(mockExcludedTask.id); + + // expectation + expect(response.status).toBe(httpStatusCodes.OK); + expect(response).toSatisfyApiSpec(); + } + ); + + it('should return 200 and complete job when getting completed finalize task', async () => { + // mocks + const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 4, completedTasks: 4 }); + const mockFinalizeTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks.finalize, + status: OperationStatus.COMPLETED, + }); + + nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockFinalizeTask.id }).reply(httpStatusCodes.OK, [mockFinalizeTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .get(`/jobs/${mockDeleteLayerJob.id}`) + .query({ shouldReturnTasks: false }) + .reply(httpStatusCodes.OK, mockDeleteLayerJob); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.COMPLETED, percentage: 100 }) + .reply(httpStatusCodes.OK); + + // action + const response = await requestSender.handleTaskNotification(mockFinalizeTask.id); + + // expectation + expect(response.status).toBe(httpStatusCodes.OK); + expect(response).toSatisfyApiSpec(); + }); + + it('should return 200 and fail job when getting failed delete task', async () => { + // mocks + const mockDeleteLayerJob = getDeleteLayerJobMock(); + const mockDeleteTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks.delete, + status: OperationStatus.FAILED, + reason: 'delete task failed', + }); + + nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockDeleteTask.id }).reply(httpStatusCodes.OK, [mockDeleteTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .get(`/jobs/${mockDeleteLayerJob.id}`) + .query({ shouldReturnTasks: false }) + .reply(httpStatusCodes.OK, mockDeleteLayerJob); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.FAILED, reason: mockDeleteTask.reason }) + .reply(httpStatusCodes.OK); + + // action + const response = await requestSender.handleTaskNotification(mockDeleteTask.id); + + // expectation + expect(response.status).toBe(httpStatusCodes.OK); + expect(response).toSatisfyApiSpec(); + }); + }); +}); diff --git a/tests/mocks/configMock.ts b/tests/mocks/configMock.ts index 05685e2..ad73312 100644 --- a/tests/mocks/configMock.ts +++ b/tests/mocks/configMock.ts @@ -83,6 +83,7 @@ const registerDefaultConfig = (): void => { swapUpdate: 'Ingestion_Swap_Update', export: 'Export', seed: 'TilesSeeding', + deleteLayer: 'Delete_Layer', }, tasks: { validation: 'validation', @@ -93,6 +94,9 @@ const registerDefaultConfig = (): void => { finalize: 'finalize', export: 'tilesExporting', seed: 'TilesSeeding', + tilesDeletion: 'tiles-deletion', + delete: 'delete', + artifactsDeletion: 'artifacts-deletion', }, suspendingTaskTypes: ['validation'], }, @@ -100,6 +104,7 @@ const registerDefaultConfig = (): void => { ingestionTasksFlow: ['validation', 'create-tasks', 'tilesMerging', 'finalize'], exportTasksFlow: ['init', 'tilesExporting', 'polygon-parts', 'finalize'], seedTasksFlow: ['TilesSeeding'], + deleteLayerTasksFlow: ['delete', 'tiles-deletion', 'artifacts-deletion', 'finalize'], }, }; diff --git a/tests/mocks/jobMocks.ts b/tests/mocks/jobMocks.ts index f1c0737..6b7e4d0 100644 --- a/tests/mocks/jobMocks.ts +++ b/tests/mocks/jobMocks.ts @@ -109,6 +109,39 @@ export const getSeedingJobMock = (override?: Partial>): IJobResponse => { + const defaultJobMock = { + id: faker.string.uuid(), + resourceId: 'test', + version: '1.0', + type: 'Delete_Layer', + description: '', + status: OperationStatus.IN_PROGRESS, + percentage: 0, + reason: '', + domain: 'RASTER', + isCleaned: false, + priority: 0, + parameters: { approver: 'test-approver' }, + expirationDate: undefined, + internalId: faker.string.uuid(), + producerName: undefined, + productName: 'test', + productType: 'Orthophoto', + additionalIdentifiers: '', + taskCount: 1, + completedTasks: 0, + failedTasks: 0, + expiredTasks: 0, + pendingTasks: 0, + inProgressTasks: 0, + abortedTasks: 0, + created: faker.date.anytime().toString(), + updated: faker.date.anytime().toString(), + }; + return { ...defaultJobMock, ...override }; +}; + export const getTaskMock = (jobId: string, override?: Partial, 'jobId'>>): ITaskResponse => { const defaultTaskMock = { id: faker.string.uuid(), diff --git a/tests/unit/tasks/handlers/jobHandler.spec.ts b/tests/unit/tasks/handlers/jobHandler.spec.ts index ddddb3f..e0cfb93 100644 --- a/tests/unit/tasks/handlers/jobHandler.spec.ts +++ b/tests/unit/tasks/handlers/jobHandler.spec.ts @@ -23,6 +23,7 @@ describe('JobHandler', () => { { mockJob: createTestJob(jobDefinitionsConfig.jobs.swapUpdate), taskType: jobDefinitionsConfig.tasks.merge }, { mockJob: createTestJob(jobDefinitionsConfig.jobs.export), taskType: jobDefinitionsConfig.tasks.export }, { mockJob: createTestJob(jobDefinitionsConfig.jobs.seed), taskType: jobDefinitionsConfig.tasks.seed }, + { mockJob: createTestJob(jobDefinitionsConfig.jobs.deleteLayer), taskType: jobDefinitionsConfig.tasks.delete }, ]; const testCaseHandlerLog = '$mockJob.type handler'; diff --git a/tests/unit/tasks/handlers/jobHandlerFactory.spec.ts b/tests/unit/tasks/handlers/jobHandlerFactory.spec.ts index 84e53f6..580ebbd 100644 --- a/tests/unit/tasks/handlers/jobHandlerFactory.spec.ts +++ b/tests/unit/tasks/handlers/jobHandlerFactory.spec.ts @@ -7,6 +7,7 @@ import { getJobHandler } from '../../../../src/tasks/handlers/jobHandlerFactory' import type { IJobDefinitionsConfig } from '../../../../src/common/interfaces'; import { IngestionJobHandler } from '../../../../src/tasks/handlers/ingestion/ingestionHandler'; import { ExportJobHandler } from '../../../../src/tasks/handlers/export/exportHandler'; +import { DeleteLayerJobHandler } from '../../../../src/tasks/handlers/deleteLayer/deleteLayerHandler'; // Test helper functions const createTestLogger = async (): Promise => jsLogger({ enabled: false }); @@ -87,6 +88,20 @@ describe('jobHandlerFactory', () => { // Then: should create ExportJobHandler expect(handler).toBeInstanceOf(ExportJobHandler); }); + + it('should create DeleteLayerJobHandler for deleteLayer job type', () => { + // Given: deleteLayer job and task + const jobType = jobDefinitionsConfig.jobs.deleteLayer; + const mockJob = createTestJob(jobType); + const mockTask = getTaskMock(mockJob.id); + const mockQueueClient = createMockQueueClient(); + + // When: creating handler for deleteLayer job type + const handler = getJobHandler(jobType, jobDefinitionsConfig, logger, mockQueueClient, configMock, mockJob, mockTask); + + // Then: should create DeleteLayerJobHandler + expect(handler).toBeInstanceOf(DeleteLayerJobHandler); + }); }); describe('Error handling', () => { From 07f7d23ccba0e9d461eb498f444fb15c4c6a30a0 Mon Sep 17 00:00:00 2001 From: Roi Cohen <213414225+roicohen326@users.noreply.github.com> Date: Sun, 19 Jul 2026 13:32:50 +0300 Subject: [PATCH 02/11] docs: add in-depth Delete_Layer tasks flow documentation Cover the cross-repo context, the generic job/task engine, the design rationale behind the new handler, config wiring across all four layers, the raster-shared alpha-pin situation, and the test strategy. --- README.md | 3 + docs/delete-layer-tasks-flow.md | 432 ++++++++++++++++++++++++++++++++ 2 files changed, 435 insertions(+) create mode 100644 docs/delete-layer-tasks-flow.md diff --git a/README.md b/README.md index abe1eae..192e984 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,9 @@ Services notifies Job Tracker on completed tasks, and Job Tracker handles the re ## API Check out the OpenAPI spec [here](/openapi3.yaml) + +## Job type deep-dives +- [Delete_Layer tasks flow](docs/delete-layer-tasks-flow.md) — architecture, design rationale, and test strategy for the `Delete_Layer` job type. #### Get all items ```http diff --git a/docs/delete-layer-tasks-flow.md b/docs/delete-layer-tasks-flow.md new file mode 100644 index 0000000..ee97e65 --- /dev/null +++ b/docs/delete-layer-tasks-flow.md @@ -0,0 +1,432 @@ +# Delete_Layer Tasks Flow + +Deep-dive documentation for the `Delete_Layer` job type support added to job-tracker. +This complements the top-level [README](../README.md) — read that first for the general +service overview, then come here for the full design rationale, architecture, and test +strategy behind this specific feature. + +## Table of contents + +- [1. Why this exists](#1-why-this-exists) +- [2. Cross-repo context](#2-cross-repo-context) +- [3. job-tracker's generic engine](#3-job-trackers-generic-engine) +- [4. The Delete_Layer design](#4-the-delete_layer-design) +- [5. Every file changed, and why](#5-every-file-changed-and-why) +- [6. Config wiring, end to end](#6-config-wiring-end-to-end) +- [7. The raster-shared dependency problem](#7-the-raster-shared-dependency-problem) +- [8. Test strategy](#8-test-strategy) +- [9. Verification performed](#9-verification-performed) +- [10. Comparison matrix: all four job handlers](#10-comparison-matrix-all-four-job-handlers) +- [11. Known limitations & follow-ups](#11-known-limitations--follow-ups) + +--- + +## 1. Why this exists + +MapColonies' raster stack supports deleting an entire layer (tiles, catalog record, +mapproxy/geoserver registration, artifacts). That operation is modeled as its own job +type, `Delete_Layer`, running through the same generic job/task orchestration that every +other job type (`Ingestion_New`, `Ingestion_Update`, `Ingestion_Swap_Update`, `Export`, +`TilesSeeding`) already uses. job-tracker is the service that owns that orchestration: it +reacts to task-completion webhooks and decides what happens next in a job's lifecycle. + +Before this change, job-tracker had **no knowledge of `Delete_Layer` at all** — a task +notification for a deletion job would have hit the `default` branch of the job handler +factory and thrown a `BadRequestError`. This document describes what was added to make +job-tracker a first-class participant in that flow. + +## 2. Cross-repo context + +This feature spans several repositories, and understanding job-tracker's slice requires +knowing what the others already do (as of when this was implemented): + +| Repo | Role | State found | +| ------------------- | ------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `raster-shared` | Shared types/schemas/constants (npm package) | Defines `DeletionJobTypes.Delete_Layer`, `DeletionTaskTypes.{Delete, LayerTilesDeletion, ArtifactsDeletion}`, and the Zod schemas for each task's parameters — but **only on the `alpha` branch/dist-tag**, not in any stable release. | +| `ingestion-trigger` | Initiates ingestion/deletion jobs | Already pinned to `raster-shared@8.3.0-alpha.0`. Its `IngestionManager.deleteLayer()` creates a `Delete_Layer` job with **exactly one task**, type `delete`, params `{ deleteFromCatalog: false, deleteFromMapproxy: false, deleteFromGeoserver: false, deletePolygonParts: false }` (all flags currently hardcoded `false` — this part of the pipeline is itself still incomplete upstream). | +| `cleaner` | Worker that dequeues and executes deletion tasks | Only has a `tilesDeletionStrategy`, and its `worker.capabilities.pairs` config currently only pairs `tiles-deletion` with `Ingestion_Update` / `Ingestion_Swap_Update` — **not** with `Delete_Layer`. No strategy exists yet for `delete` or `artifacts-deletion`. | +| `job-tracker` | Orchestrates the task flow (this repo) | **Nothing** — the subject of this change. | + +The practical implication: as of this change, nothing in the ecosystem actually _executes_ +the `delete`, `tiles-deletion`, or `artifacts-deletion` tasks for a `Delete_Layer` job yet. +job-tracker's job is to have the routing skeleton correctly in place so that when the rest +of the ecosystem catches up (cleaner adds a `Delete_Layer` pairing, something starts +consuming `delete`/`artifacts-deletion`), the sequencing already works without further +changes here. + +## 3. job-tracker's generic engine + +Before describing what's new, here's the mechanism every job handler (including the new +one) plugs into. This is pre-existing code, but understanding it precisely is essential to +understanding why `Delete_Layer` was wired the way it was. + +### Request flow + +``` +POST /tasks/:taskId/notify + → TasksController.handleTaskNotification + → TasksManager.handleTaskNotification(taskId) + 1. findTask(taskId) — must be COMPLETED or FAILED, else 428 + 2. jobManager.getJob(task.jobId) + 3. getJobHandler(job.type, ...) — factory picks the concrete handler + 4. handler.handleFailedTask() — if task failed + handler.handleCompletedNotification() — if task completed +``` + +### The handler hierarchy + +``` +IJobHandler (interface) + └─ BaseJobHandler (abstract) — completeJob / failJob / suspendJob / updateJobProgress + └─ JobHandler (abstract) — task-flow-aware: tasksFlow, excludedTypes, blockedDuplicationTypes + ├─ IngestionJobHandler + ├─ ExportJobHandler + ├─ SeedJobHandler + └─ DeleteLayerJobHandler ← new +``` + +`JobHandler` composes a `TaskHandler` (in [`taskHandler.ts`](../src/tasks/handlers/taskHandler.ts)) +that does the actual sequencing math: + +```ts +public getNextTaskType(): TaskTypeItem | undefined { + const indexOfCurrentTask = this.tasksFlow.indexOf(this.task.type); + let nextTaskTypeIndex = indexOfCurrentTask + 1; + + // walk forward past any task type marked as "excluded" (skip) + while (nextTaskTypeIndex < this.tasksFlow.length) { + const nextTaskType = this.tasksFlow[nextTaskTypeIndex]; + if (nextTaskType === undefined || !this.shouldSkipTaskCreation(nextTaskType)) break; + nextTaskTypeIndex++; + } + + if (nextTaskTypeIndex >= this.tasksFlow.length) return undefined; + return this.tasksFlow[nextTaskTypeIndex]; +} + +public shouldSkipTaskCreation(taskType: string): boolean { + return this.excludedTypes.includes(taskType); +} +``` + +And [`jobHandler.ts`](../src/tasks/handlers/jobHandler.ts)'s `handleCompletedNotification` +decides what to do with that result: + +```ts +public async handleCompletedNotification(): Promise { + const nextTaskType = this.taskWorker?.getNextTaskType(); + + if (nextTaskType == undefined) { // end of the flow + await this.handleNoNextTask(); // → completeJob() or updateJobProgress() + return; + } + if (!this.isProceedable()) { // per-task-type business rule (e.g. validation) + await this.suspendJob(this.task.reason); + return; + } + if (!this.isReadyForNextTask() || this.shouldSkipTaskCreation(nextTaskType)) { + await this.handleSkipTask(nextTaskType); // → updateJobProgress() only + return; + } + await this.createNewTask(nextTaskType); // → jobManager.createTaskForJob(...) +} +``` + +Two concepts matter most for `Delete_Layer`: + +- **`excludedTypes`** — task types this handler will _never actively create_ via + `createTaskForJob`, because it doesn't have the real, per-instance parameters needed + (e.g. a `catalogId` or a list of S3 paths). Something else creates those tasks directly. + The handler still processes completion webhooks for them (`shouldSkipTaskCreation` + causes `getNextTaskType` to walk _past_ them when computing what to create next, but the + webhook for their own completion is handled exactly like any other task). +- **`isReadyForNextTask()`** — `job.completedTasks === job.taskCount`. This is what + prevents job-tracker from racing ahead and creating the next task before _every_ + currently-existing task for the job (including ones it didn't create itself) has + finished. + +### Static task parameters + +Whenever job-tracker _does_ create a task, it needs static parameters for it, resolved by +job-type + task-type from a fixed map in [`mappers.ts`](../src/common/mappers.ts): + +```ts +const key: JobAndTask = `${jobType}_${taskType}`; +const parameters = this.taskParametersMapper.get(key); +if (parameters === undefined) throw new BadRequestError(`task parameters for ${key} do not exist`); +``` + +This is a hard constraint: **job-tracker can only auto-create a task type if its +parameters are the same every time** (or empty). This is precisely why `tiles-deletion` +and `artifacts-deletion` had to be excluded rather than auto-created — their parameters +(`catalogId`, `paths`, `sourceProvider`, `bucket`) are per-layer, dynamic data that +job-tracker has no way to produce generically. + +## 4. The Delete_Layer design + +### The flow + +```mermaid +flowchart LR + A["delete\n(created externally,\nby ingestion-trigger)"] -->|completed| B{"next in flow?"} + B -->|"tiles-deletion\n(excluded → skip)"| C{"next in flow?"} + C -->|"artifacts-deletion\n(excluded → skip)"| D{"next in flow?"} + D -->|"finalize\n(not excluded)"| E["job-tracker creates\nfinalize task"] + E -->|completed| F["job COMPLETED"] + + A -.->|"tiles-deletion completes\n(webhook received)"| C + A -.->|"artifacts-deletion completes\n(webhook received)"| D +``` + +Configured as (`taskFlowManager.deleteLayerTasksFlow`): + +```json +["delete", "tiles-deletion", "artifacts-deletion", "finalize"] +``` + +Walking through what actually happens for each possible completion notification, given +`excludedTypes = [tiles-deletion, artifacts-deletion]`: + +| Task that completed | `getNextTaskType()` result | Condition | Behavior | +| -------------------- | ---------------------------------------------------------- | ------------------------------------------------------------------- | -------------------------------------------------------------------------- | +| `delete` | skips `tiles-deletion` + `artifacts-deletion` → `finalize` | `completedTasks === taskCount` | creates `finalize` | +| `delete` | same | `completedTasks !== taskCount` (other deletion tasks still pending) | progress update only | +| `tiles-deletion` | skips `artifacts-deletion` → `finalize` | ready | creates `finalize` | +| `artifacts-deletion` | → `finalize` | ready | creates `finalize` | +| `finalize` | `undefined` (end of flow) | `taskType === Finalize` in `BaseJobHandler.isJobCompleted` | job **COMPLETED** | +| any task, `FAILED` | — | `delete` is not in `suspendingTaskTypes` | job **FAILED** (default `JobHandler.handleFailedTask`, no override needed) | + +Why `delete`, `tiles-deletion`, and `artifacts-deletion` can be created **in any order +relative to each other** from job-tracker's point of view: it never creates them itself +(all three are either the pre-existing first task or excluded), so whichever one's +completion webhook arrives, `isReadyForNextTask()` is the real gate — job-tracker only +moves to `finalize` once every task that exists for the job (regardless of who created it +or in what order) is done. + +### The handler implementation + +[`src/tasks/handlers/deleteLayer/deleteLayerHandler.ts`](../src/tasks/handlers/deleteLayer/deleteLayerHandler.ts): + +```ts +@injectable() +export class DeleteLayerJobHandler extends JobHandler { + protected readonly tasksFlow: TaskTypes; + protected readonly excludedTypes: TaskTypes; + protected readonly blockedDuplicationTypes: TaskTypes; + + public constructor(/* … */) { + super(logger, config, jobManagerClient, job, task); + this.tasksFlow = this.config.get('taskFlowManager.deleteLayerTasksFlow') as unknown as TaskTypes; + this.excludedTypes = [this.jobDefinitions.tasks.tilesDeletion, this.jobDefinitions.tasks.artifactsDeletion]; + this.blockedDuplicationTypes = [this.jobDefinitions.tasks.finalize]; + + this.initializeTaskOperations(); + } +} +``` + +Design choices explained: + +- **No `isJobCompleted` override.** Unlike `SeedJobHandler` (which overrides it because a + seed job has no `finalize` step at all), `Delete_Layer` ends in `finalize` just like + ingestion/export, so the inherited `BaseJobHandler.isJobCompleted` (`completedTasks === +taskCount && taskType === TaskTypes.Finalize`) is correct as-is. +- **No `isProceedable` override / proceed rule.** Ingestion registers a + `ValidationProceedRule` because a validation task can be "completed but invalid" and + needs to suspend the job instead of proceeding. Nothing in the `Delete_Layer` flow has + an equivalent conditional-proceed semantic, so the default (`isProceedable` always + returns `true` unless a rule is registered) is correct. +- **No `handleFailedTask` override.** Export overrides this to special-case a "create an + error-callback finalize task instead of just failing" behavior. Nothing in the ticket + scope called for that for deletion, so the default `JobHandler.handleFailedTask` (fail + the job, unless the task type is in `suspendingTaskTypes`) applies as-is. +- **`blockedDuplicationTypes = [finalize]`** rather than mirroring `ExportJobHandler` + (which blocks duplication on the excluded type itself). The rationale: `blockDuplication` + only has any effect on task types job-tracker actually creates via `createTaskForJob` — + and in this handler that's only ever `finalize`. Protecting the one task type this + handler actually creates from double-creation (e.g. if two completion webhooks for + different upstream tasks arrive close together) is the meaningful protection here. + +## 5. Every file changed, and why + +| File | Change | +| --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `package.json` | `@map-colonies/raster-shared` bumped `^7.10.2` → `8.3.0-alpha.0` (exact pin). See [§7](#7-the-raster-shared-dependency-problem). | +| `package-lock.json` | Regenerated by `npm install` for the above. | +| `src/common/interfaces.ts` | Added `jobs.deleteLayer`, `tasks.delete`, `tasks.artifactsDeletion` to `IJobDefinitionsConfig`. (`tasks.tilesDeletion` already existed in the interface but had never been populated in `config/default.json` — that gap is now also closed.) | +| `src/common/mappers.ts` | Added a `${jobs.deleteLayer}_${tasks.finalize} → {}` entry — required because job-tracker creates the `finalize` task itself and the mapper throws if no entry exists for a job+task combination it tries to create. | +| `src/tasks/handlers/deleteLayer/deleteLayerHandler.ts` | **New.** The handler itself — see [§4](#4-the-delete_layer-design). | +| `src/tasks/handlers/jobHandlerFactory.ts` | Added the `jobDefinitions.jobs.deleteLayer` case to the factory switch. | +| `config/default.json` | Added `jobDefinitions.jobs.deleteLayer`, the three new/completed task entries, and `taskFlowManager.deleteLayerTasksFlow`. This is the single source of truth for local dev and tests (`config/test.json` and `config/production.json` are both `{}` — everything is either defaulted here or overridden by env vars in prod). | +| `config/custom-environment-variables.json` | Env var names for every new/completed config key, so helm can inject real values in each deployed environment. | +| `helm/values.yaml` | Chart defaults for the new job/task types and the new flow array. | +| `helm/templates/configmap.yaml` | Renders the new env vars (`JOB_DEFINITIONS_JOB_DELETE_LAYER`, `JOB_DEFINITIONS_TASK_DELETE`, `JOB_DEFINITIONS_TASK_ARTIFACTS_DELETION`, `DELETE_LAYER_TASKS_FLOW`) into the ConfigMap. | +| `tests/mocks/configMock.ts` | Test-side mirror of `config/default.json` — same additions, plus the previously-missing `tilesDeletion`/`delete`/`artifactsDeletion` task entries needed for any test to reference them. | +| `tests/mocks/jobMocks.ts` | New `getDeleteLayerJobMock()`, mirroring the existing `getExportJobMock`/`getSeedingJobMock` pattern. | +| `tests/unit/tasks/handlers/jobHandler.spec.ts` | Added a `Delete_Layer` case to the generic cross-handler `testCases` array — see [§8](#8-test-strategy). | +| `tests/unit/tasks/handlers/jobHandlerFactory.spec.ts` | New test: factory returns a `DeleteLayerJobHandler` instance for the `deleteLayer` job type. | +| `tests/integration/tasks/deleteLayer/taskManager.spec.ts` | **New.** Full HTTP-level integration suite — see [§8](#8-test-strategy). | + +## 6. Config wiring, end to end + +Every new job/task identifier and the new flow array had to be threaded through **four** +layers consistently, since a mismatch at any layer means the wrong value (or `undefined`) +in some environment: + +``` +config/default.json (local dev / test fallback values) + │ +config/custom-environment-variables.json (maps config keys → env var names) + │ +helm/values.yaml (chart default values, per-environment overridable) + │ +helm/templates/configmap.yaml (renders the actual env vars injected at deploy time) +``` + +Concretely, for the job type: + +| Layer | Value | +| ----------------------------------- | ---------------------------------------------------------------------------------------- | +| `config/default.json` | `jobDefinitions.jobs.deleteLayer = "Delete_Layer"` | +| `custom-environment-variables.json` | `jobDefinitions.jobs.deleteLayer = "JOB_DEFINITIONS_JOB_DELETE_LAYER"` | +| `helm/values.yaml` | `jobDefinitions.jobs.deleteLayer.type: ''` (filled in per-environment values override) | +| `helm/templates/configmap.yaml` | `JOB_DEFINITIONS_JOB_DELETE_LAYER: {{ $jobDefinitions.jobs.deleteLayer.type \| quote }}` | + +And the same four-layer pattern for `tasks.delete`, `tasks.artifactsDeletion`, and +`taskFlowManager.deleteLayerTasksFlow` (the last one is a JSON-encoded array end-to-end: +`DELETE_LAYER_TASKS_FLOW` env var → parsed back into an array by the `@map-colonies/config` +library via the `__format: "json"` directive in `custom-environment-variables.json`). + +This was validated by rendering the actual chart (see [§9](#9-verification-performed)) — +not just by eyeballing the four files — since Helm's Go-templating syntax isn't valid +standalone YAML and a plain YAML linter can't confirm correctness (the IDE's inline +diagnostics on `configmap.yaml` during this work were exactly that false-positive: they +flagged the _new_ lines using the identical `{{ }}` pattern already used everywhere else +in the file). + +## 7. The raster-shared dependency problem + +This was the most consequential discovery during investigation, because it blocks the +feature at the type level, not just the routing level. + +- `raster-shared`'s `DeletionJobTypes`/`DeletionTaskTypes`/`DeleteLayerJobParams`/ + `DeleteTaskParams` were added in commit `8d50c52` ("feat: add layer deletion schemas, + types and constants (MAPCO-7285)"), which landed on the **`alpha` branch**. +- Checked the CHANGELOG entry ("add tile deletion schemas and types", #191) — it appears + under `8.1.0-alpha.0`, which _looks_ like it should have rolled into the stable `8.1.0` + release. It did not: `git ls-tree` on the commit that produced the published `8.2.0` + package (`6aad051`, "bump version from 8.1.0 to 8.2.0") confirms **no `deletion` + directory exists in that commit**. The alpha and stable release trains have diverged. +- The only place these types exist in a published package is the `alpha` dist-tag, + currently `8.3.0-alpha.1` (confirmed via `npm pack @map-colonies/raster-shared@alpha`). +- `ingestion-trigger` already depends on `raster-shared@8.3.0-alpha.0` (exact pin, not a + caret range) — this repo's `package.json` now matches that exact version for + consistency between the two services consuming the same in-flight schema. +- Confirmed no breaking change from the `8.0.0-alpha.0` release (which _did_ have a + genuine breaking change — "change polygon part validation error to new format, + MAPCO-10506") affects job-tracker: this repo only imports `TaskTypes`, + `ExportFinalizeType`, the ingestion/export finalize param types, and + `ingestionValidationTaskParamsSchema` (just `.pick({isValid: true})` off it) — none of + which touch the polygon-parts validation error shape. `tsc --noEmit` after the bump + confirmed this empirically. + +**Action item for whoever owns this long-term:** once `raster-shared`'s deletion +feature ships in a stable release, both `job-tracker` and `ingestion-trigger` need to move +off the alpha pin together, to a matching stable version. + +## 8. Test strategy + +### Unit tests + +`tests/unit/tasks/handlers/jobHandler.spec.ts` runs a shared battery of generic behavior +assertions across **every** job handler type, driven by one `testCases` array. Adding +`{ mockJob: createTestJob(jobs.deleteLayer), taskType: tasks.delete }` to that array gets +`Delete_Layer` covered, for free, by: + +- `handleFailedTask` → fails the job with the task's reason. +- `handleCompletedNotification` on `finalize`, all tasks done → completes the job at 100%. +- `handleCompletedNotification` when `isProceedable()` is mocked `false` → suspends the job. +- `handleCompletedNotification` when the task is still `IN_PROGRESS` → progress update only, no task created. +- `handleCompletedNotification` when other tasks in the job are still incomplete → progress update only, no task created. + +`tests/unit/tasks/handlers/jobHandlerFactory.spec.ts` adds one targeted test: the factory +returns a `DeleteLayerJobHandler` instance for `jobDefinitions.jobs.deleteLayer`. + +### Integration tests + +`tests/integration/tasks/deleteLayer/taskManager.spec.ts` drives the actual Express app +through `supertest`, with `nock` mocking every job-manager HTTP call, validating each +response against the OpenAPI spec (`toSatisfyApiSpec()`). Unlike the unit tests, this +exercises the **real** `TaskHandler.getNextTaskType()` / `shouldSkipTaskCreation()` logic +against the real `deleteLayerTasksFlow` array — nothing here is mocked at that layer, so +these tests are what actually prove the skip-over-excluded-types sequencing works, not +just that the handler wires up. + +| # | Scenario | Asserts | +| --- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | `delete` completes, `completedTasks === taskCount` (nothing else pending) | `finalize` task created with `blockDuplication: true`, `parameters: {}`; job progress updated | +| 2 | `delete` completes, `completedTasks < taskCount` (other deletion tasks still pending) | **no** task created; progress updated only | +| 3a | `tiles-deletion` completes, ready | correctly walks past the (already-passed) excluded type, creates `finalize` | +| 3b | `artifacts-deletion` completes, ready | same, one step later in the flow | +| 4 | `finalize` completes | job transitions to `COMPLETED` at 100% | +| 5 | `delete` fails | job transitions to `FAILED` with the task's reason (default `handleFailedTask`, no suspend — `delete` isn't in `suspendingTaskTypes`) | + +### Results + +``` +Unit: 134/134 passing, 100% statements/branches/functions/lines on the new handler +Integration: 96/96 passing (5 new scenarios, one parameterized over 2 excluded task types) +``` + +## 9. Verification performed + +Beyond the automated test suites: + +- `tsc --noEmit` — clean after the `raster-shared` bump (confirms no breaking-change fallout). +- `eslint .` — clean. +- `prettier --check .` — clean (one formatting fix applied to the new integration spec). +- **Helm chart rendering** — since the chart has an external OCI dependency (`mclabels`) + not resolvable offline, the chart was copied to a scratch directory, the dependency + block stripped, and a minimal local stub for the `mclabels.labels`/`mclabels. +selectorLabels` template functions was added just to satisfy `helm template`'s parser. + Rendering `templates/configmap.yaml` confirmed every new env var (including the + JSON-encoded `DELETE_LAYER_TASKS_FLOW`) renders correctly. This is real evidence, not + just visual inspection — Helm's Go-templating means a file can look syntactically odd to + a plain-YAML linter while still being perfectly valid, and the only way to know for sure + is to actually render it. + +## 10. Comparison matrix: all four job handlers + +| | `IngestionJobHandler` | `ExportJobHandler` | `SeedJobHandler` | `DeleteLayerJobHandler` | +| ------------------------------------ | ------------------------------------- | -------------------------------------------- | ------------------------------- | ---------------------------------------- | +| Ends in `finalize`? | ✅ | ✅ | ❌ (overrides `isJobCompleted`) | ✅ | +| `excludedTypes` | `[merge, tilesDeletion]` | `[export]` (the heavy `tilesExporting` step) | `[seed]` (its only task) | `[tilesDeletion, artifactsDeletion]` | +| `blockedDuplicationTypes` | `[validation, createTasks, finalize]` | `[export]` | `[]` | `[finalize]` | +| Custom `isProceedable`/proceed rule? | ✅ `ValidationProceedRule` | ❌ | ❌ | ❌ | +| Custom `handleFailedTask`? | ❌ (default) | ✅ (error-callback finalize task) | ❌ (default) | ❌ (default) | +| First task created by | job-tracker itself (`validation`) | external initiator | external initiator | external initiator (`ingestion-trigger`) | + +This table is a useful reference the next time a new job type needs to be added — it +makes explicit which knobs actually vary between handlers versus which ones are +essentially always the same. + +## 11. Known limitations & follow-ups + +- **`raster-shared` alpha pin** — see [§7](#7-the-raster-shared-dependency-problem). This + is a real, tracked risk: alpha packages can be republished or diverge without the usual + semver guarantees. +- **Nothing consumes `delete` or `artifacts-deletion` yet**, and `cleaner` doesn't have a + `Delete_Layer` → `tiles-deletion` pairing configured. job-tracker's routing is correct + and tested in isolation, but the end-to-end feature isn't runnable in a real environment + until those other services catch up — this doc's scope was explicitly job-tracker's + slice only (per your confirmation to go with the full 4-stage flow). +- **`ingestion-trigger` currently hardcodes all four `delete` task flags to `false`** + (`deleteFromCatalog/Mapproxy/Geoserver/PolygonParts`), so even once a worker consumes + the `delete` task, it wouldn't currently be told to delete anything from those + subsystems. Out of scope for job-tracker, but worth knowing if the flow doesn't seem to + "do" anything end-to-end yet. +- **No dedicated finalize-parameters schema exists in `raster-shared` for deletion** (unlike + export, which has `ExportFinalizeFullProcessingParams`/`ExportFinalizeErrorCallbackParams`). + The `mappers.ts` entry therefore uses an empty object. If a future `raster-shared` + release adds a typed finalize schema for deletion, this mapper entry should be updated + to use it instead of `{}`. From 1d7c5008b3ba07e4b3617a253685c6ba117d69e4 Mon Sep 17 00:00:00 2001 From: Roi Cohen <213414225+roicohen326@users.noreply.github.com> Date: Mon, 20 Jul 2026 12:12:50 +0300 Subject: [PATCH 03/11] docs: add a plain-language explainer for the Delete_Layer flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A separate, jargon-free doc explaining what job-tracker does, what a "handler" is, and what changed for Delete_Layer — for anyone who wants the gist without the code-level deep-dive. --- README.md | 1 + docs/delete-layer-simple-explainer.md | 92 +++++++++++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 docs/delete-layer-simple-explainer.md diff --git a/README.md b/README.md index 192e984..9598f2b 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,7 @@ Services notifies Job Tracker on completed tasks, and Job Tracker handles the re Check out the OpenAPI spec [here](/openapi3.yaml) ## Job type deep-dives +- [Delete_Layer — the simple explanation](docs/delete-layer-simple-explainer.md) — start here if you just want the plain-language version. - [Delete_Layer tasks flow](docs/delete-layer-tasks-flow.md) — architecture, design rationale, and test strategy for the `Delete_Layer` job type. #### Get all items diff --git a/docs/delete-layer-simple-explainer.md b/docs/delete-layer-simple-explainer.md new file mode 100644 index 0000000..a11c644 --- /dev/null +++ b/docs/delete-layer-simple-explainer.md @@ -0,0 +1,92 @@ +# Delete_Layer — The Simple Explanation + +This is the plain-language version. For the full technical deep-dive (code, config +wiring, tests), see [delete-layer-tasks-flow.md](delete-layer-tasks-flow.md) — but you +shouldn't need it just to understand what we built and why. + +## What does job-tracker actually do? + +Think of a **job** as a to-do list, and job-tracker as the person checking items off that +list. + +Example — "ingest a new layer" is not one action, it's a sequence: + +``` +1. validate the input +2. merge the tiles +3. finalize (send callbacks, update the catalog, etc.) +``` + +Other services do the _actual work_ of each step (validating, merging, etc.). When a +service finishes a step, it tells job-tracker: **"hey, step X is done."** job-tracker's +entire job is to answer one question every time that happens: + +> **"Given that this step just finished, what's the next step — and is the whole list done?"** + +That's it. job-tracker doesn't validate anything, doesn't merge anything, doesn't delete +anything itself. It's the checklist-keeper, not a worker. + +## So what is a "handler"? + +Different jobs have different checklists: + +| Job type | Checklist | +| ------------------------ | --------------------------------------------------- | +| Ingest a new layer | validate → merge tiles → finalize | +| Export a layer | init → export tiles → split into parts → finalize | +| Seed tiles | (one step, nothing after) | +| **Delete a layer (new)** | delete → delete tiles → delete artifacts → finalize | + +A **handler** is just: _the piece of code that knows one of these checklists._ When +job-tracker gets a "step X is done" notification, it looks at which job that step belongs +to, grabs the matching handler for that job's checklist, and asks it "what's next?" + +There's one handler class per row in that table. We added the last one. + +## What we actually built + +We added a **new checklist** (`Delete_Layer`) and a **new handler** that knows it: + +``` + delete → delete tiles → delete artifacts → finalize → done! + ↑ ↑ ↑ + created by someone else someone else + ingestion- creates this creates this + trigger task directly task directly +``` + +Two important details baked into the handler: + +- **"delete tiles" and "delete artifacts" are not created by job-tracker.** Those two + steps need real, specific information (which exact files to delete) that job-tracker + doesn't have and shouldn't need to know. Some other service creates those steps + directly, with the real data. job-tracker just knows to expect them, skip past them + when deciding "what's next," and wait for them to finish before moving to `finalize`. +- **`finalize` is the only step job-tracker creates itself**, and it's the step that, + once done, marks the whole job as complete. + +## Why does this matter across multiple repos? + +This deletion feature isn't finished everywhere yet — think of it as a relay race where +not every runner is in position: + +- `ingestion-trigger` already knows how to _start_ a deletion job (creates the `delete` + step). +- `cleaner` (the service that would actually delete tiles) isn't wired up yet to do this + for deletion jobs specifically. +- `job-tracker` (this repo) now knows the full checklist and will correctly sequence + through it — that's what this change adds. + +So right now, if someone kicks off a deletion job, job-tracker will correctly track it +and know what should happen next at each point — but a couple of the other services still +need their own follow-up work before the whole thing runs start-to-finish. That's +expected and outside this repo's scope. + +## One more thing: the version pin + +We had to point job-tracker at a slightly newer version of a shared library +(`raster-shared`) to get the new `Delete_Layer` definitions. That version is currently a +**pre-release ("alpha")** build, not a final stable one — because the stable release +with this feature hasn't been published yet. We're using the exact same pre-release +version that `ingestion-trigger` already uses, so at least the two repos agree. Once a +real stable release comes out, both repos should switch to it together. From fe19b8346b6751309e2010e4ce45383909e48e1c Mon Sep 17 00:00:00 2001 From: Roi Cohen <213414225+roicohen326@users.noreply.github.com> Date: Mon, 20 Jul 2026 15:47:38 +0300 Subject: [PATCH 04/11] Revert "docs: add a plain-language explainer for the Delete_Layer flow" This reverts commit 1d7c5008b3ba07e4b3617a253685c6ba117d69e4. --- README.md | 1 - docs/delete-layer-simple-explainer.md | 92 --------------------------- 2 files changed, 93 deletions(-) delete mode 100644 docs/delete-layer-simple-explainer.md diff --git a/README.md b/README.md index 9598f2b..192e984 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,6 @@ Services notifies Job Tracker on completed tasks, and Job Tracker handles the re Check out the OpenAPI spec [here](/openapi3.yaml) ## Job type deep-dives -- [Delete_Layer — the simple explanation](docs/delete-layer-simple-explainer.md) — start here if you just want the plain-language version. - [Delete_Layer tasks flow](docs/delete-layer-tasks-flow.md) — architecture, design rationale, and test strategy for the `Delete_Layer` job type. #### Get all items diff --git a/docs/delete-layer-simple-explainer.md b/docs/delete-layer-simple-explainer.md deleted file mode 100644 index a11c644..0000000 --- a/docs/delete-layer-simple-explainer.md +++ /dev/null @@ -1,92 +0,0 @@ -# Delete_Layer — The Simple Explanation - -This is the plain-language version. For the full technical deep-dive (code, config -wiring, tests), see [delete-layer-tasks-flow.md](delete-layer-tasks-flow.md) — but you -shouldn't need it just to understand what we built and why. - -## What does job-tracker actually do? - -Think of a **job** as a to-do list, and job-tracker as the person checking items off that -list. - -Example — "ingest a new layer" is not one action, it's a sequence: - -``` -1. validate the input -2. merge the tiles -3. finalize (send callbacks, update the catalog, etc.) -``` - -Other services do the _actual work_ of each step (validating, merging, etc.). When a -service finishes a step, it tells job-tracker: **"hey, step X is done."** job-tracker's -entire job is to answer one question every time that happens: - -> **"Given that this step just finished, what's the next step — and is the whole list done?"** - -That's it. job-tracker doesn't validate anything, doesn't merge anything, doesn't delete -anything itself. It's the checklist-keeper, not a worker. - -## So what is a "handler"? - -Different jobs have different checklists: - -| Job type | Checklist | -| ------------------------ | --------------------------------------------------- | -| Ingest a new layer | validate → merge tiles → finalize | -| Export a layer | init → export tiles → split into parts → finalize | -| Seed tiles | (one step, nothing after) | -| **Delete a layer (new)** | delete → delete tiles → delete artifacts → finalize | - -A **handler** is just: _the piece of code that knows one of these checklists._ When -job-tracker gets a "step X is done" notification, it looks at which job that step belongs -to, grabs the matching handler for that job's checklist, and asks it "what's next?" - -There's one handler class per row in that table. We added the last one. - -## What we actually built - -We added a **new checklist** (`Delete_Layer`) and a **new handler** that knows it: - -``` - delete → delete tiles → delete artifacts → finalize → done! - ↑ ↑ ↑ - created by someone else someone else - ingestion- creates this creates this - trigger task directly task directly -``` - -Two important details baked into the handler: - -- **"delete tiles" and "delete artifacts" are not created by job-tracker.** Those two - steps need real, specific information (which exact files to delete) that job-tracker - doesn't have and shouldn't need to know. Some other service creates those steps - directly, with the real data. job-tracker just knows to expect them, skip past them - when deciding "what's next," and wait for them to finish before moving to `finalize`. -- **`finalize` is the only step job-tracker creates itself**, and it's the step that, - once done, marks the whole job as complete. - -## Why does this matter across multiple repos? - -This deletion feature isn't finished everywhere yet — think of it as a relay race where -not every runner is in position: - -- `ingestion-trigger` already knows how to _start_ a deletion job (creates the `delete` - step). -- `cleaner` (the service that would actually delete tiles) isn't wired up yet to do this - for deletion jobs specifically. -- `job-tracker` (this repo) now knows the full checklist and will correctly sequence - through it — that's what this change adds. - -So right now, if someone kicks off a deletion job, job-tracker will correctly track it -and know what should happen next at each point — but a couple of the other services still -need their own follow-up work before the whole thing runs start-to-finish. That's -expected and outside this repo's scope. - -## One more thing: the version pin - -We had to point job-tracker at a slightly newer version of a shared library -(`raster-shared`) to get the new `Delete_Layer` definitions. That version is currently a -**pre-release ("alpha")** build, not a final stable one — because the stable release -with this feature hasn't been published yet. We're using the exact same pre-release -version that `ingestion-trigger` already uses, so at least the two repos agree. Once a -real stable release comes out, both repos should switch to it together. From 665c798afdd86c843dc41d26106c43986f93ac60 Mon Sep 17 00:00:00 2001 From: shlomiko Date: Sun, 26 Jul 2026 11:40:51 +0300 Subject: [PATCH 05/11] refactor: remove artifacts-deletion task from delete layer flow --- config/custom-environment-variables.json | 3 +- config/default.json | 5 +- docs/delete-layer-tasks-flow.md | 64 +++++++++--------- helm/templates/configmap.yaml | 1 - helm/values.yaml | 3 - src/common/interfaces.ts | 1 - .../deleteLayer/deleteLayerHandler.ts | 2 +- .../tasks/deleteLayer/taskManager.spec.ts | 65 +++++++++---------- tests/mocks/configMock.ts | 3 +- 9 files changed, 66 insertions(+), 81 deletions(-) diff --git a/config/custom-environment-variables.json b/config/custom-environment-variables.json index 4820e7b..308dad2 100644 --- a/config/custom-environment-variables.json +++ b/config/custom-environment-variables.json @@ -81,8 +81,7 @@ "export": "JOB_DEFINITIONS_TASK_EXPORT", "finalize": "JOB_DEFINITIONS_TASK_FINALIZE", "seed": "JOB_DEFINITIONS_TASK_SEED", - "delete": "JOB_DEFINITIONS_TASK_DELETE", - "artifactsDeletion": "JOB_DEFINITIONS_TASK_ARTIFACTS_DELETION" + "delete": "JOB_DEFINITIONS_TASK_DELETE" }, "suspendingTaskTypes": { "__name": "JOB_DEFINITIONS_SUSPENDING_TASKS", diff --git a/config/default.json b/config/default.json index e0de6ee..0ae8a03 100644 --- a/config/default.json +++ b/config/default.json @@ -66,8 +66,7 @@ "finalize": "finalize", "seed": "TilesSeeding", "tilesDeletion": "tiles-deletion", - "delete": "delete", - "artifactsDeletion": "artifacts-deletion" + "delete": "delete" }, "suspendingTaskTypes": ["polygon-parts"] }, @@ -75,6 +74,6 @@ "ingestionTasksFlow": ["validation", "create-merge-tasks", "tilesMerging", "finalize"], "exportTasksFlow": ["init", "tilesExporting", "polygon-parts", "finalize"], "seedTasksFlow": ["TilesSeeding"], - "deleteLayerTasksFlow": ["delete", "tiles-deletion", "artifacts-deletion", "finalize"] + "deleteLayerTasksFlow": ["delete", "tiles-deletion", "finalize"] } } diff --git a/docs/delete-layer-tasks-flow.md b/docs/delete-layer-tasks-flow.md index ee97e65..feafc20 100644 --- a/docs/delete-layer-tasks-flow.md +++ b/docs/delete-layer-tasks-flow.md @@ -42,16 +42,16 @@ knowing what the others already do (as of when this was implemented): | Repo | Role | State found | | ------------------- | ------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `raster-shared` | Shared types/schemas/constants (npm package) | Defines `DeletionJobTypes.Delete_Layer`, `DeletionTaskTypes.{Delete, LayerTilesDeletion, ArtifactsDeletion}`, and the Zod schemas for each task's parameters — but **only on the `alpha` branch/dist-tag**, not in any stable release. | +| `raster-shared` | Shared types/schemas/constants (npm package) | Defines `DeletionJobTypes.Delete_Layer`, `DeletionTaskTypes.{Delete, LayerTilesDeletion}`, and the Zod schemas for each task's parameters — but **only on the `alpha` branch/dist-tag**, not in any stable release. | | `ingestion-trigger` | Initiates ingestion/deletion jobs | Already pinned to `raster-shared@8.3.0-alpha.0`. Its `IngestionManager.deleteLayer()` creates a `Delete_Layer` job with **exactly one task**, type `delete`, params `{ deleteFromCatalog: false, deleteFromMapproxy: false, deleteFromGeoserver: false, deletePolygonParts: false }` (all flags currently hardcoded `false` — this part of the pipeline is itself still incomplete upstream). | -| `cleaner` | Worker that dequeues and executes deletion tasks | Only has a `tilesDeletionStrategy`, and its `worker.capabilities.pairs` config currently only pairs `tiles-deletion` with `Ingestion_Update` / `Ingestion_Swap_Update` — **not** with `Delete_Layer`. No strategy exists yet for `delete` or `artifacts-deletion`. | +| `cleaner` | Worker that dequeues and executes deletion tasks | Only has a `tilesDeletionStrategy`, and its `worker.capabilities.pairs` config currently only pairs `tiles-deletion` with `Ingestion_Update` / `Ingestion_Swap_Update` — **not** with `Delete_Layer`. No strategy exists yet for `delete`. | | `job-tracker` | Orchestrates the task flow (this repo) | **Nothing** — the subject of this change. | The practical implication: as of this change, nothing in the ecosystem actually _executes_ -the `delete`, `tiles-deletion`, or `artifacts-deletion` tasks for a `Delete_Layer` job yet. +the `delete` or `tiles-deletion` tasks for a `Delete_Layer` job yet. job-tracker's job is to have the routing skeleton correctly in place so that when the rest of the ecosystem catches up (cleaner adds a `Delete_Layer` pairing, something starts -consuming `delete`/`artifacts-deletion`), the sequencing already works without further +consuming `delete`), the sequencing already works without further changes here. ## 3. job-tracker's generic engine @@ -158,7 +158,7 @@ if (parameters === undefined) throw new BadRequestError(`task parameters for ${k This is a hard constraint: **job-tracker can only auto-create a task type if its parameters are the same every time** (or empty). This is precisely why `tiles-deletion` -and `artifacts-deletion` had to be excluded rather than auto-created — their parameters +had to be excluded rather than auto-created — its parameters (`catalogId`, `paths`, `sourceProvider`, `bucket`) are per-layer, dynamic data that job-tracker has no way to produce generically. @@ -170,35 +170,32 @@ job-tracker has no way to produce generically. flowchart LR A["delete\n(created externally,\nby ingestion-trigger)"] -->|completed| B{"next in flow?"} B -->|"tiles-deletion\n(excluded → skip)"| C{"next in flow?"} - C -->|"artifacts-deletion\n(excluded → skip)"| D{"next in flow?"} - D -->|"finalize\n(not excluded)"| E["job-tracker creates\nfinalize task"] + C -->|"finalize\n(not excluded)"| E["job-tracker creates\nfinalize task"] E -->|completed| F["job COMPLETED"] A -.->|"tiles-deletion completes\n(webhook received)"| C - A -.->|"artifacts-deletion completes\n(webhook received)"| D ``` Configured as (`taskFlowManager.deleteLayerTasksFlow`): ```json -["delete", "tiles-deletion", "artifacts-deletion", "finalize"] +["delete", "tiles-deletion", "finalize"] ``` Walking through what actually happens for each possible completion notification, given -`excludedTypes = [tiles-deletion, artifacts-deletion]`: - -| Task that completed | `getNextTaskType()` result | Condition | Behavior | -| -------------------- | ---------------------------------------------------------- | ------------------------------------------------------------------- | -------------------------------------------------------------------------- | -| `delete` | skips `tiles-deletion` + `artifacts-deletion` → `finalize` | `completedTasks === taskCount` | creates `finalize` | -| `delete` | same | `completedTasks !== taskCount` (other deletion tasks still pending) | progress update only | -| `tiles-deletion` | skips `artifacts-deletion` → `finalize` | ready | creates `finalize` | -| `artifacts-deletion` | → `finalize` | ready | creates `finalize` | -| `finalize` | `undefined` (end of flow) | `taskType === Finalize` in `BaseJobHandler.isJobCompleted` | job **COMPLETED** | -| any task, `FAILED` | — | `delete` is not in `suspendingTaskTypes` | job **FAILED** (default `JobHandler.handleFailedTask`, no override needed) | - -Why `delete`, `tiles-deletion`, and `artifacts-deletion` can be created **in any order +`excludedTypes = [tiles-deletion]`: + +| Task that completed | `getNextTaskType()` result | Condition | Behavior | +| ------------------- | ------------------------------------ | ------------------------------------------------------------------- | -------------------------------------------------------------------------- | +| `delete` | skips `tiles-deletion` → `finalize` | `completedTasks === taskCount` | creates `finalize` | +| `delete` | same | `completedTasks !== taskCount` (other deletion tasks still pending) | progress update only | +| `tiles-deletion` | → `finalize` | ready | creates `finalize` | +| `finalize` | `undefined` (end of flow) | `taskType === Finalize` in `BaseJobHandler.isJobCompleted` | job **COMPLETED** | +| any task, `FAILED` | — | `delete` is not in `suspendingTaskTypes` | job **FAILED** (default `JobHandler.handleFailedTask`, no override needed) | + +Why `delete` and `tiles-deletion` can be created **in any order relative to each other** from job-tracker's point of view: it never creates them itself -(all three are either the pre-existing first task or excluded), so whichever one's +(both are either the pre-existing first task or excluded), so whichever one's completion webhook arrives, `isReadyForNextTask()` is the real gate — job-tracker only moves to `finalize` once every task that exists for the job (regardless of who created it or in what order) is done. @@ -217,7 +214,7 @@ export class DeleteLayerJobHandler extends JobHandler { public constructor(/* … */) { super(logger, config, jobManagerClient, job, task); this.tasksFlow = this.config.get('taskFlowManager.deleteLayerTasksFlow') as unknown as TaskTypes; - this.excludedTypes = [this.jobDefinitions.tasks.tilesDeletion, this.jobDefinitions.tasks.artifactsDeletion]; + this.excludedTypes = [this.jobDefinitions.tasks.tilesDeletion]; this.blockedDuplicationTypes = [this.jobDefinitions.tasks.finalize]; this.initializeTaskOperations(); @@ -253,15 +250,15 @@ taskCount && taskType === TaskTypes.Finalize`) is correct as-is. | --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `package.json` | `@map-colonies/raster-shared` bumped `^7.10.2` → `8.3.0-alpha.0` (exact pin). See [§7](#7-the-raster-shared-dependency-problem). | | `package-lock.json` | Regenerated by `npm install` for the above. | -| `src/common/interfaces.ts` | Added `jobs.deleteLayer`, `tasks.delete`, `tasks.artifactsDeletion` to `IJobDefinitionsConfig`. (`tasks.tilesDeletion` already existed in the interface but had never been populated in `config/default.json` — that gap is now also closed.) | +| `src/common/interfaces.ts` | Added `jobs.deleteLayer` and `tasks.delete` to `IJobDefinitionsConfig`. (`tasks.tilesDeletion` already existed in the interface but had never been populated in `config/default.json` — that gap is now also closed.) | | `src/common/mappers.ts` | Added a `${jobs.deleteLayer}_${tasks.finalize} → {}` entry — required because job-tracker creates the `finalize` task itself and the mapper throws if no entry exists for a job+task combination it tries to create. | | `src/tasks/handlers/deleteLayer/deleteLayerHandler.ts` | **New.** The handler itself — see [§4](#4-the-delete_layer-design). | | `src/tasks/handlers/jobHandlerFactory.ts` | Added the `jobDefinitions.jobs.deleteLayer` case to the factory switch. | -| `config/default.json` | Added `jobDefinitions.jobs.deleteLayer`, the three new/completed task entries, and `taskFlowManager.deleteLayerTasksFlow`. This is the single source of truth for local dev and tests (`config/test.json` and `config/production.json` are both `{}` — everything is either defaulted here or overridden by env vars in prod). | +| `config/default.json` | Added `jobDefinitions.jobs.deleteLayer`, the new/completed task entries, and `taskFlowManager.deleteLayerTasksFlow`. This is the single source of truth for local dev and tests (`config/test.json` and `config/production.json` are both `{}` — everything is either defaulted here or overridden by env vars in prod). | | `config/custom-environment-variables.json` | Env var names for every new/completed config key, so helm can inject real values in each deployed environment. | | `helm/values.yaml` | Chart defaults for the new job/task types and the new flow array. | -| `helm/templates/configmap.yaml` | Renders the new env vars (`JOB_DEFINITIONS_JOB_DELETE_LAYER`, `JOB_DEFINITIONS_TASK_DELETE`, `JOB_DEFINITIONS_TASK_ARTIFACTS_DELETION`, `DELETE_LAYER_TASKS_FLOW`) into the ConfigMap. | -| `tests/mocks/configMock.ts` | Test-side mirror of `config/default.json` — same additions, plus the previously-missing `tilesDeletion`/`delete`/`artifactsDeletion` task entries needed for any test to reference them. | +| `helm/templates/configmap.yaml` | Renders the new env vars (`JOB_DEFINITIONS_JOB_DELETE_LAYER`, `JOB_DEFINITIONS_TASK_DELETE`, `DELETE_LAYER_TASKS_FLOW`) into the ConfigMap. | +| `tests/mocks/configMock.ts` | Test-side mirror of `config/default.json` — same additions, plus the previously-missing `tilesDeletion`/`delete` task entries needed for any test to reference them. | | `tests/mocks/jobMocks.ts` | New `getDeleteLayerJobMock()`, mirroring the existing `getExportJobMock`/`getSeedingJobMock` pattern. | | `tests/unit/tasks/handlers/jobHandler.spec.ts` | Added a `Delete_Layer` case to the generic cross-handler `testCases` array — see [§8](#8-test-strategy). | | `tests/unit/tasks/handlers/jobHandlerFactory.spec.ts` | New test: factory returns a `DeleteLayerJobHandler` instance for the `deleteLayer` job type. | @@ -292,7 +289,7 @@ Concretely, for the job type: | `helm/values.yaml` | `jobDefinitions.jobs.deleteLayer.type: ''` (filled in per-environment values override) | | `helm/templates/configmap.yaml` | `JOB_DEFINITIONS_JOB_DELETE_LAYER: {{ $jobDefinitions.jobs.deleteLayer.type \| quote }}` | -And the same four-layer pattern for `tasks.delete`, `tasks.artifactsDeletion`, and +And the same four-layer pattern for `tasks.delete` and `taskFlowManager.deleteLayerTasksFlow` (the last one is a JSON-encoded array end-to-end: `DELETE_LAYER_TASKS_FLOW` env var → parsed back into an array by the `@map-colonies/config` library via the `__format: "json"` directive in `custom-environment-variables.json`). @@ -366,8 +363,7 @@ just that the handler wires up. | --- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | | 1 | `delete` completes, `completedTasks === taskCount` (nothing else pending) | `finalize` task created with `blockDuplication: true`, `parameters: {}`; job progress updated | | 2 | `delete` completes, `completedTasks < taskCount` (other deletion tasks still pending) | **no** task created; progress updated only | -| 3a | `tiles-deletion` completes, ready | correctly walks past the (already-passed) excluded type, creates `finalize` | -| 3b | `artifacts-deletion` completes, ready | same, one step later in the flow | +| 3 | `tiles-deletion` completes, ready | correctly walks past the (already-passed) excluded type, creates `finalize` | | 4 | `finalize` completes | job transitions to `COMPLETED` at 100% | | 5 | `delete` fails | job transitions to `FAILED` with the task's reason (default `handleFailedTask`, no suspend — `delete` isn't in `suspendingTaskTypes`) | @@ -375,7 +371,7 @@ just that the handler wires up. ``` Unit: 134/134 passing, 100% statements/branches/functions/lines on the new handler -Integration: 96/96 passing (5 new scenarios, one parameterized over 2 excluded task types) +Integration: 95/95 passing (5 new scenarios) ``` ## 9. Verification performed @@ -400,7 +396,7 @@ selectorLabels` template functions was added just to satisfy `helm template`'s p | | `IngestionJobHandler` | `ExportJobHandler` | `SeedJobHandler` | `DeleteLayerJobHandler` | | ------------------------------------ | ------------------------------------- | -------------------------------------------- | ------------------------------- | ---------------------------------------- | | Ends in `finalize`? | ✅ | ✅ | ❌ (overrides `isJobCompleted`) | ✅ | -| `excludedTypes` | `[merge, tilesDeletion]` | `[export]` (the heavy `tilesExporting` step) | `[seed]` (its only task) | `[tilesDeletion, artifactsDeletion]` | +| `excludedTypes` | `[merge, tilesDeletion]` | `[export]` (the heavy `tilesExporting` step) | `[seed]` (its only task) | `[tilesDeletion]` | | `blockedDuplicationTypes` | `[validation, createTasks, finalize]` | `[export]` | `[]` | `[finalize]` | | Custom `isProceedable`/proceed rule? | ✅ `ValidationProceedRule` | ❌ | ❌ | ❌ | | Custom `handleFailedTask`? | ❌ (default) | ✅ (error-callback finalize task) | ❌ (default) | ❌ (default) | @@ -415,11 +411,11 @@ essentially always the same. - **`raster-shared` alpha pin** — see [§7](#7-the-raster-shared-dependency-problem). This is a real, tracked risk: alpha packages can be republished or diverge without the usual semver guarantees. -- **Nothing consumes `delete` or `artifacts-deletion` yet**, and `cleaner` doesn't have a +- **Nothing consumes `delete` yet**, and `cleaner` doesn't have a `Delete_Layer` → `tiles-deletion` pairing configured. job-tracker's routing is correct and tested in isolation, but the end-to-end feature isn't runnable in a real environment until those other services catch up — this doc's scope was explicitly job-tracker's - slice only (per your confirmation to go with the full 4-stage flow). + slice only. - **`ingestion-trigger` currently hardcodes all four `delete` task flags to `false`** (`deleteFromCatalog/Mapproxy/Geoserver/PolygonParts`), so even once a worker consumes the `delete` task, it wouldn't currently be told to delete anything from those diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index 11ef2ad..f00dc7b 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -44,7 +44,6 @@ data: JOB_DEFINITIONS_TASK_FINALIZE: {{ $jobDefinitions.tasks.finalize.type | quote }} JOB_DEFINITIONS_TASK_SEED: {{ $jobDefinitions.tasks.seed.type | quote }} JOB_DEFINITIONS_TASK_DELETE: {{ $jobDefinitions.tasks.delete.type | quote }} - JOB_DEFINITIONS_TASK_ARTIFACTS_DELETION: {{ $jobDefinitions.tasks.artifactsDeletion.type | quote }} JOB_DEFINITIONS_SUSPENDING_TASKS: {{ $suspendingTaskTypes | quote }} INGESTION_TASKS_FLOW: {{ .Values.taskFlowManager.ingestionTasksFlow | toJson | quote }} EXPORT_TASKS_FLOW: {{ .Values.taskFlowManager.exportTasksFlow | toJson | quote }} diff --git a/helm/values.yaml b/helm/values.yaml index 0efd7be..de9023e 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -119,8 +119,6 @@ jobDefinitions: type: '' delete: type: '' - artifactsDeletion: - type: '' taskFlowManager: ingestionTasksFlow: @@ -139,7 +137,6 @@ taskFlowManager: deleteLayerTasksFlow: - delete - tiles-deletion - - artifacts-deletion - finalize env: port: 80 diff --git a/src/common/interfaces.ts b/src/common/interfaces.ts index dbcd3d5..73d8e26 100644 --- a/src/common/interfaces.ts +++ b/src/common/interfaces.ts @@ -35,7 +35,6 @@ export interface IJobDefinitionsConfig { seed: string; tilesDeletion: string; delete: string; - artifactsDeletion: string; }; suspendingTaskTypes: string[]; } diff --git a/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts b/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts index 7cd7b34..9377510 100644 --- a/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts +++ b/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts @@ -21,7 +21,7 @@ export class DeleteLayerJobHandler extends JobHandler { ) { super(logger, config, jobManagerClient, job, task); this.tasksFlow = this.config.get('taskFlowManager.deleteLayerTasksFlow') as unknown as TaskTypes; - this.excludedTypes = [this.jobDefinitions.tasks.tilesDeletion, this.jobDefinitions.tasks.artifactsDeletion]; + this.excludedTypes = [this.jobDefinitions.tasks.tilesDeletion]; this.blockedDuplicationTypes = [this.jobDefinitions.tasks.finalize]; this.initializeTaskOperations(); diff --git a/tests/integration/tasks/deleteLayer/taskManager.spec.ts b/tests/integration/tasks/deleteLayer/taskManager.spec.ts index db7ac01..8904aff 100644 --- a/tests/integration/tasks/deleteLayer/taskManager.spec.ts +++ b/tests/integration/tasks/deleteLayer/taskManager.spec.ts @@ -106,40 +106,37 @@ describe('tasks', function () { expect(response).toSatisfyApiSpec(); }); - it.each([{ taskTypeKey: 'tilesDeletion' as const }, { taskTypeKey: 'artifactsDeletion' as const }])( - 'should return 200 and create finalize task when getting completed excluded "$taskTypeKey" task and no other task is pending', - async ({ taskTypeKey }) => { - // mocks - const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 3, completedTasks: 3 }); - const mockExcludedTask = getTaskMock(mockDeleteLayerJob.id, { - type: jobDefinitionsConfig.tasks[taskTypeKey], - status: OperationStatus.COMPLETED, - }); - const mockFinalizeTaskBody = { - parameters: {}, - type: jobDefinitionsConfig.tasks.finalize, - blockDuplication: true, - }; - - nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockExcludedTask.id }).reply(httpStatusCodes.OK, [mockExcludedTask]); - nock(jobManagerConfigMock.jobManagerBaseUrl) - .get(`/jobs/${mockDeleteLayerJob.id}`) - .query({ shouldReturnTasks: false }) - .reply(httpStatusCodes.OK, mockDeleteLayerJob); - nock(jobManagerConfigMock.jobManagerBaseUrl) - .post(`/jobs/${mockDeleteLayerJob.id}/tasks`, mockFinalizeTaskBody) - .reply(httpStatusCodes.CREATED); - const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount + 1); - nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); - - // action - const response = await requestSender.handleTaskNotification(mockExcludedTask.id); - - // expectation - expect(response.status).toBe(httpStatusCodes.OK); - expect(response).toSatisfyApiSpec(); - } - ); + it('should return 200 and create finalize task when getting completed excluded "tilesDeletion" task and no other task is pending', async () => { + // mocks + const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 3, completedTasks: 3 }); + const mockExcludedTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks.tilesDeletion, + status: OperationStatus.COMPLETED, + }); + const mockFinalizeTaskBody = { + parameters: {}, + type: jobDefinitionsConfig.tasks.finalize, + blockDuplication: true, + }; + + nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockExcludedTask.id }).reply(httpStatusCodes.OK, [mockExcludedTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .get(`/jobs/${mockDeleteLayerJob.id}`) + .query({ shouldReturnTasks: false }) + .reply(httpStatusCodes.OK, mockDeleteLayerJob); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .post(`/jobs/${mockDeleteLayerJob.id}/tasks`, mockFinalizeTaskBody) + .reply(httpStatusCodes.CREATED); + const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount + 1); + nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); + + // action + const response = await requestSender.handleTaskNotification(mockExcludedTask.id); + + // expectation + expect(response.status).toBe(httpStatusCodes.OK); + expect(response).toSatisfyApiSpec(); + }); it('should return 200 and complete job when getting completed finalize task', async () => { // mocks diff --git a/tests/mocks/configMock.ts b/tests/mocks/configMock.ts index ad73312..841c756 100644 --- a/tests/mocks/configMock.ts +++ b/tests/mocks/configMock.ts @@ -96,7 +96,6 @@ const registerDefaultConfig = (): void => { seed: 'TilesSeeding', tilesDeletion: 'tiles-deletion', delete: 'delete', - artifactsDeletion: 'artifacts-deletion', }, suspendingTaskTypes: ['validation'], }, @@ -104,7 +103,7 @@ const registerDefaultConfig = (): void => { ingestionTasksFlow: ['validation', 'create-tasks', 'tilesMerging', 'finalize'], exportTasksFlow: ['init', 'tilesExporting', 'polygon-parts', 'finalize'], seedTasksFlow: ['TilesSeeding'], - deleteLayerTasksFlow: ['delete', 'tiles-deletion', 'artifacts-deletion', 'finalize'], + deleteLayerTasksFlow: ['delete', 'tiles-deletion', 'finalize'], }, }; From 101ece943c2416a4ab0596d4b84c1d889bb5d18c Mon Sep 17 00:00:00 2001 From: shlomiko Date: Sun, 26 Jul 2026 14:14:06 +0300 Subject: [PATCH 06/11] refactor: remove finalize task from delete layer flow Delete_Layer tasks are all created externally - job-tracker no longer creates any task for this job type. The job completes when all tasks are done (isJobCompleted override, like the seed handler). --- config/default.json | 2 +- docs/delete-layer-tasks-flow.md | 109 +++++++++--------- helm/values.yaml | 1 - src/common/mappers.ts | 1 - .../deleteLayer/deleteLayerHandler.ts | 8 +- .../tasks/deleteLayer/taskManager.spec.ts | 49 +------- tests/mocks/configMock.ts | 2 +- tests/unit/tasks/handlers/jobHandler.spec.ts | 6 +- 8 files changed, 70 insertions(+), 108 deletions(-) diff --git a/config/default.json b/config/default.json index 0ae8a03..8e57929 100644 --- a/config/default.json +++ b/config/default.json @@ -74,6 +74,6 @@ "ingestionTasksFlow": ["validation", "create-merge-tasks", "tilesMerging", "finalize"], "exportTasksFlow": ["init", "tilesExporting", "polygon-parts", "finalize"], "seedTasksFlow": ["TilesSeeding"], - "deleteLayerTasksFlow": ["delete", "tiles-deletion", "finalize"] + "deleteLayerTasksFlow": ["delete", "tiles-deletion"] } } diff --git a/docs/delete-layer-tasks-flow.md b/docs/delete-layer-tasks-flow.md index feafc20..87ca58c 100644 --- a/docs/delete-layer-tasks-flow.md +++ b/docs/delete-layer-tasks-flow.md @@ -42,9 +42,9 @@ knowing what the others already do (as of when this was implemented): | Repo | Role | State found | | ------------------- | ------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `raster-shared` | Shared types/schemas/constants (npm package) | Defines `DeletionJobTypes.Delete_Layer`, `DeletionTaskTypes.{Delete, LayerTilesDeletion}`, and the Zod schemas for each task's parameters — but **only on the `alpha` branch/dist-tag**, not in any stable release. | +| `raster-shared` | Shared types/schemas/constants (npm package) | Defines `DeletionJobTypes.Delete_Layer`, `DeletionTaskTypes.{Delete, LayerTilesDeletion}`, and the Zod schemas for each task's parameters — but **only on the `alpha` branch/dist-tag**, not in any stable release. | | `ingestion-trigger` | Initiates ingestion/deletion jobs | Already pinned to `raster-shared@8.3.0-alpha.0`. Its `IngestionManager.deleteLayer()` creates a `Delete_Layer` job with **exactly one task**, type `delete`, params `{ deleteFromCatalog: false, deleteFromMapproxy: false, deleteFromGeoserver: false, deletePolygonParts: false }` (all flags currently hardcoded `false` — this part of the pipeline is itself still incomplete upstream). | -| `cleaner` | Worker that dequeues and executes deletion tasks | Only has a `tilesDeletionStrategy`, and its `worker.capabilities.pairs` config currently only pairs `tiles-deletion` with `Ingestion_Update` / `Ingestion_Swap_Update` — **not** with `Delete_Layer`. No strategy exists yet for `delete`. | +| `cleaner` | Worker that dequeues and executes deletion tasks | Only has a `tilesDeletionStrategy`, and its `worker.capabilities.pairs` config currently only pairs `tiles-deletion` with `Ingestion_Update` / `Ingestion_Swap_Update` — **not** with `Delete_Layer`. No strategy exists yet for `delete`. | | `job-tracker` | Orchestrates the task flow (this repo) | **Nothing** — the subject of this change. | The practical implication: as of this change, nothing in the ecosystem actually _executes_ @@ -169,9 +169,8 @@ job-tracker has no way to produce generically. ```mermaid flowchart LR A["delete\n(created externally,\nby ingestion-trigger)"] -->|completed| B{"next in flow?"} - B -->|"tiles-deletion\n(excluded → skip)"| C{"next in flow?"} - C -->|"finalize\n(not excluded)"| E["job-tracker creates\nfinalize task"] - E -->|completed| F["job COMPLETED"] + B -->|"tiles-deletion\n(excluded → skip)"| C{"end of flow"} + C -->|"all tasks completed"| F["job COMPLETED"] A -.->|"tiles-deletion completes\n(webhook received)"| C ``` @@ -179,25 +178,24 @@ flowchart LR Configured as (`taskFlowManager.deleteLayerTasksFlow`): ```json -["delete", "tiles-deletion", "finalize"] +["delete", "tiles-deletion"] ``` Walking through what actually happens for each possible completion notification, given -`excludedTypes = [tiles-deletion]`: +`excludedTypes = [delete, tiles-deletion]`: -| Task that completed | `getNextTaskType()` result | Condition | Behavior | -| ------------------- | ------------------------------------ | ------------------------------------------------------------------- | -------------------------------------------------------------------------- | -| `delete` | skips `tiles-deletion` → `finalize` | `completedTasks === taskCount` | creates `finalize` | -| `delete` | same | `completedTasks !== taskCount` (other deletion tasks still pending) | progress update only | -| `tiles-deletion` | → `finalize` | ready | creates `finalize` | -| `finalize` | `undefined` (end of flow) | `taskType === Finalize` in `BaseJobHandler.isJobCompleted` | job **COMPLETED** | -| any task, `FAILED` | — | `delete` is not in `suspendingTaskTypes` | job **FAILED** (default `JobHandler.handleFailedTask`, no override needed) | +| Task that completed | `getNextTaskType()` result | Condition | Behavior | +| ------------------- | -------------------------------------------------- | ------------------------------------------------------------------- | -------------------------------------------------------------------------- | +| `delete` | skips `tiles-deletion` → `undefined` (end of flow) | `completedTasks === taskCount` | job **COMPLETED** (overridden `isJobCompleted`) | +| `delete` | same | `completedTasks !== taskCount` (other deletion tasks still pending) | progress update only | +| `tiles-deletion` | `undefined` (end of flow) | `completedTasks === taskCount` | job **COMPLETED** | +| any task, `FAILED` | — | `delete` is not in `suspendingTaskTypes` | job **FAILED** (default `JobHandler.handleFailedTask`, no override needed) | Why `delete` and `tiles-deletion` can be created **in any order relative to each other** from job-tracker's point of view: it never creates them itself (both are either the pre-existing first task or excluded), so whichever one's completion webhook arrives, `isReadyForNextTask()` is the real gate — job-tracker only -moves to `finalize` once every task that exists for the job (regardless of who created it +completes the job once every task that exists for the job (regardless of who created it or in what order) is done. ### The handler implementation @@ -214,20 +212,24 @@ export class DeleteLayerJobHandler extends JobHandler { public constructor(/* … */) { super(logger, config, jobManagerClient, job, task); this.tasksFlow = this.config.get('taskFlowManager.deleteLayerTasksFlow') as unknown as TaskTypes; - this.excludedTypes = [this.jobDefinitions.tasks.tilesDeletion]; - this.blockedDuplicationTypes = [this.jobDefinitions.tasks.finalize]; + this.excludedTypes = [this.jobDefinitions.tasks.delete, this.jobDefinitions.tasks.tilesDeletion]; + this.blockedDuplicationTypes = []; this.initializeTaskOperations(); } + + public override isJobCompleted = (): boolean => { + return this.job.completedTasks === this.job.taskCount; + }; } ``` Design choices explained: -- **No `isJobCompleted` override.** Unlike `SeedJobHandler` (which overrides it because a - seed job has no `finalize` step at all), `Delete_Layer` ends in `finalize` just like - ingestion/export, so the inherited `BaseJobHandler.isJobCompleted` (`completedTasks === -taskCount && taskType === TaskTypes.Finalize`) is correct as-is. +- **`isJobCompleted` override.** Like `SeedJobHandler`, `Delete_Layer` has no `finalize` + step at all, so the inherited `BaseJobHandler.isJobCompleted` (which additionally + requires `taskType === TaskTypes.Finalize`) would never fire. The override completes the + job as soon as `completedTasks === taskCount`. - **No `isProceedable` override / proceed rule.** Ingestion registers a `ValidationProceedRule` because a validation task can be "completed but invalid" and needs to suspend the job instead of proceeding. Nothing in the `Delete_Layer` flow has @@ -237,32 +239,32 @@ taskCount && taskType === TaskTypes.Finalize`) is correct as-is. error-callback finalize task instead of just failing" behavior. Nothing in the ticket scope called for that for deletion, so the default `JobHandler.handleFailedTask` (fail the job, unless the task type is in `suspendingTaskTypes`) applies as-is. -- **`blockedDuplicationTypes = [finalize]`** rather than mirroring `ExportJobHandler` - (which blocks duplication on the excluded type itself). The rationale: `blockDuplication` - only has any effect on task types job-tracker actually creates via `createTaskForJob` — - and in this handler that's only ever `finalize`. Protecting the one task type this - handler actually creates from double-creation (e.g. if two completion webhooks for - different upstream tasks arrive close together) is the meaningful protection here. +- **`excludedTypes = [delete, tilesDeletion]`** — job-tracker is not in charge of + creating any of the delete-layer tasks; both are created externally, so both are + excluded from auto-creation. With no `finalize` in the flow, job-tracker never creates + any task for this job type — it only tracks progress and completes/fails the job. +- **`blockedDuplicationTypes = []`** — mirroring `SeedJobHandler`; no duplication + blocking is applied to the tasks this handler creates. ## 5. Every file changed, and why -| File | Change | -| --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `package.json` | `@map-colonies/raster-shared` bumped `^7.10.2` → `8.3.0-alpha.0` (exact pin). See [§7](#7-the-raster-shared-dependency-problem). | -| `package-lock.json` | Regenerated by `npm install` for the above. | -| `src/common/interfaces.ts` | Added `jobs.deleteLayer` and `tasks.delete` to `IJobDefinitionsConfig`. (`tasks.tilesDeletion` already existed in the interface but had never been populated in `config/default.json` — that gap is now also closed.) | -| `src/common/mappers.ts` | Added a `${jobs.deleteLayer}_${tasks.finalize} → {}` entry — required because job-tracker creates the `finalize` task itself and the mapper throws if no entry exists for a job+task combination it tries to create. | -| `src/tasks/handlers/deleteLayer/deleteLayerHandler.ts` | **New.** The handler itself — see [§4](#4-the-delete_layer-design). | -| `src/tasks/handlers/jobHandlerFactory.ts` | Added the `jobDefinitions.jobs.deleteLayer` case to the factory switch. | +| File | Change | +| --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `package.json` | `@map-colonies/raster-shared` bumped `^7.10.2` → `8.3.0-alpha.0` (exact pin). See [§7](#7-the-raster-shared-dependency-problem). | +| `package-lock.json` | Regenerated by `npm install` for the above. | +| `src/common/interfaces.ts` | Added `jobs.deleteLayer` and `tasks.delete` to `IJobDefinitionsConfig`. (`tasks.tilesDeletion` already existed in the interface but had never been populated in `config/default.json` — that gap is now also closed.) | +| `src/common/mappers.ts` | No `Delete_Layer` entry — job-tracker never creates a task for this job type (all tasks are externally created and there is no `finalize` step), so no static task parameters are needed. | +| `src/tasks/handlers/deleteLayer/deleteLayerHandler.ts` | **New.** The handler itself — see [§4](#4-the-delete_layer-design). | +| `src/tasks/handlers/jobHandlerFactory.ts` | Added the `jobDefinitions.jobs.deleteLayer` case to the factory switch. | | `config/default.json` | Added `jobDefinitions.jobs.deleteLayer`, the new/completed task entries, and `taskFlowManager.deleteLayerTasksFlow`. This is the single source of truth for local dev and tests (`config/test.json` and `config/production.json` are both `{}` — everything is either defaulted here or overridden by env vars in prod). | -| `config/custom-environment-variables.json` | Env var names for every new/completed config key, so helm can inject real values in each deployed environment. | -| `helm/values.yaml` | Chart defaults for the new job/task types and the new flow array. | -| `helm/templates/configmap.yaml` | Renders the new env vars (`JOB_DEFINITIONS_JOB_DELETE_LAYER`, `JOB_DEFINITIONS_TASK_DELETE`, `DELETE_LAYER_TASKS_FLOW`) into the ConfigMap. | -| `tests/mocks/configMock.ts` | Test-side mirror of `config/default.json` — same additions, plus the previously-missing `tilesDeletion`/`delete` task entries needed for any test to reference them. | -| `tests/mocks/jobMocks.ts` | New `getDeleteLayerJobMock()`, mirroring the existing `getExportJobMock`/`getSeedingJobMock` pattern. | -| `tests/unit/tasks/handlers/jobHandler.spec.ts` | Added a `Delete_Layer` case to the generic cross-handler `testCases` array — see [§8](#8-test-strategy). | -| `tests/unit/tasks/handlers/jobHandlerFactory.spec.ts` | New test: factory returns a `DeleteLayerJobHandler` instance for the `deleteLayer` job type. | -| `tests/integration/tasks/deleteLayer/taskManager.spec.ts` | **New.** Full HTTP-level integration suite — see [§8](#8-test-strategy). | +| `config/custom-environment-variables.json` | Env var names for every new/completed config key, so helm can inject real values in each deployed environment. | +| `helm/values.yaml` | Chart defaults for the new job/task types and the new flow array. | +| `helm/templates/configmap.yaml` | Renders the new env vars (`JOB_DEFINITIONS_JOB_DELETE_LAYER`, `JOB_DEFINITIONS_TASK_DELETE`, `DELETE_LAYER_TASKS_FLOW`) into the ConfigMap. | +| `tests/mocks/configMock.ts` | Test-side mirror of `config/default.json` — same additions, plus the previously-missing `tilesDeletion`/`delete` task entries needed for any test to reference them. | +| `tests/mocks/jobMocks.ts` | New `getDeleteLayerJobMock()`, mirroring the existing `getExportJobMock`/`getSeedingJobMock` pattern. | +| `tests/unit/tasks/handlers/jobHandler.spec.ts` | Added a `Delete_Layer` case to the generic cross-handler `testCases` array — see [§8](#8-test-strategy). | +| `tests/unit/tasks/handlers/jobHandlerFactory.spec.ts` | New test: factory returns a `DeleteLayerJobHandler` instance for the `deleteLayer` job type. | +| `tests/integration/tasks/deleteLayer/taskManager.spec.ts` | **New.** Full HTTP-level integration suite — see [§8](#8-test-strategy). | ## 6. Config wiring, end to end @@ -341,7 +343,6 @@ assertions across **every** job handler type, driven by one `testCases` array. A `Delete_Layer` covered, for free, by: - `handleFailedTask` → fails the job with the task's reason. -- `handleCompletedNotification` on `finalize`, all tasks done → completes the job at 100%. - `handleCompletedNotification` when `isProceedable()` is mocked `false` → suspends the job. - `handleCompletedNotification` when the task is still `IN_PROGRESS` → progress update only, no task created. - `handleCompletedNotification` when other tasks in the job are still incomplete → progress update only, no task created. @@ -361,17 +362,16 @@ just that the handler wires up. | # | Scenario | Asserts | | --- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | `delete` completes, `completedTasks === taskCount` (nothing else pending) | `finalize` task created with `blockDuplication: true`, `parameters: {}`; job progress updated | +| 1 | `delete` completes, `completedTasks === taskCount` (nothing else pending) | job transitions to `COMPLETED` at 100% — no task created | | 2 | `delete` completes, `completedTasks < taskCount` (other deletion tasks still pending) | **no** task created; progress updated only | -| 3 | `tiles-deletion` completes, ready | correctly walks past the (already-passed) excluded type, creates `finalize` | -| 4 | `finalize` completes | job transitions to `COMPLETED` at 100% | -| 5 | `delete` fails | job transitions to `FAILED` with the task's reason (default `handleFailedTask`, no suspend — `delete` isn't in `suspendingTaskTypes`) | +| 3 | `tiles-deletion` completes, all tasks done | job transitions to `COMPLETED` at 100% | +| 4 | `delete` fails | job transitions to `FAILED` with the task's reason (default `handleFailedTask`, no suspend — `delete` isn't in `suspendingTaskTypes`) | ### Results ``` Unit: 134/134 passing, 100% statements/branches/functions/lines on the new handler -Integration: 95/95 passing (5 new scenarios) +Integration: 94/94 passing (4 new scenarios) ``` ## 9. Verification performed @@ -395,9 +395,9 @@ selectorLabels` template functions was added just to satisfy `helm template`'s p | | `IngestionJobHandler` | `ExportJobHandler` | `SeedJobHandler` | `DeleteLayerJobHandler` | | ------------------------------------ | ------------------------------------- | -------------------------------------------- | ------------------------------- | ---------------------------------------- | -| Ends in `finalize`? | ✅ | ✅ | ❌ (overrides `isJobCompleted`) | ✅ | -| `excludedTypes` | `[merge, tilesDeletion]` | `[export]` (the heavy `tilesExporting` step) | `[seed]` (its only task) | `[tilesDeletion]` | -| `blockedDuplicationTypes` | `[validation, createTasks, finalize]` | `[export]` | `[]` | `[finalize]` | +| Ends in `finalize`? | ✅ | ✅ | ❌ (overrides `isJobCompleted`) | ❌ (overrides `isJobCompleted`) | +| `excludedTypes` | `[merge, tilesDeletion]` | `[export]` (the heavy `tilesExporting` step) | `[seed]` (its only task) | `[delete, tilesDeletion]` | +| `blockedDuplicationTypes` | `[validation, createTasks, finalize]` | `[export]` | `[]` | `[]` | | Custom `isProceedable`/proceed rule? | ✅ `ValidationProceedRule` | ❌ | ❌ | ❌ | | Custom `handleFailedTask`? | ❌ (default) | ✅ (error-callback finalize task) | ❌ (default) | ❌ (default) | | First task created by | job-tracker itself (`validation`) | external initiator | external initiator | external initiator (`ingestion-trigger`) | @@ -421,8 +421,3 @@ essentially always the same. the `delete` task, it wouldn't currently be told to delete anything from those subsystems. Out of scope for job-tracker, but worth knowing if the flow doesn't seem to "do" anything end-to-end yet. -- **No dedicated finalize-parameters schema exists in `raster-shared` for deletion** (unlike - export, which has `ExportFinalizeFullProcessingParams`/`ExportFinalizeErrorCallbackParams`). - The `mappers.ts` entry therefore uses an empty object. If a future `raster-shared` - release adds a typed finalize schema for deletion, this mapper entry should be updated - to use it instead of `{}`. diff --git a/helm/values.yaml b/helm/values.yaml index de9023e..36a3138 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -137,7 +137,6 @@ taskFlowManager: deleteLayerTasksFlow: - delete - tiles-deletion - - finalize env: port: 80 targetPort: 8080 diff --git a/src/common/mappers.ts b/src/common/mappers.ts index 24a5ef5..5c24d89 100644 --- a/src/common/mappers.ts +++ b/src/common/mappers.ts @@ -46,6 +46,5 @@ export const createTaskParametersMapper = (jobDefinitions: IJobDefinitionsConfig callbacksSent: false, } satisfies ExportFinalizeFullProcessingParams, ], - [`${jobDefinitions.jobs.deleteLayer}_${jobDefinitions.tasks.finalize}`, {}], ]); }; diff --git a/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts b/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts index 9377510..5955ea2 100644 --- a/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts +++ b/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts @@ -21,9 +21,13 @@ export class DeleteLayerJobHandler extends JobHandler { ) { super(logger, config, jobManagerClient, job, task); this.tasksFlow = this.config.get('taskFlowManager.deleteLayerTasksFlow') as unknown as TaskTypes; - this.excludedTypes = [this.jobDefinitions.tasks.tilesDeletion]; - this.blockedDuplicationTypes = [this.jobDefinitions.tasks.finalize]; + this.excludedTypes = [this.jobDefinitions.tasks.delete, this.jobDefinitions.tasks.tilesDeletion]; + this.blockedDuplicationTypes = []; this.initializeTaskOperations(); } + + public override isJobCompleted = (): boolean => { + return this.job.completedTasks === this.job.taskCount; + }; } diff --git a/tests/integration/tasks/deleteLayer/taskManager.spec.ts b/tests/integration/tasks/deleteLayer/taskManager.spec.ts index 8904aff..d941226 100644 --- a/tests/integration/tasks/deleteLayer/taskManager.spec.ts +++ b/tests/integration/tasks/deleteLayer/taskManager.spec.ts @@ -52,27 +52,22 @@ describe('tasks', function () { }); describe('Happy Path', function () { - it('should return 200 and create finalize task when getting completed delete task and no other task is pending', async () => { + it('should return 200 and complete job when getting completed delete task and no other task is pending', async () => { // mocks const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 1, completedTasks: 1 }); const mockDeleteTask = getTaskMock(mockDeleteLayerJob.id, { type: jobDefinitionsConfig.tasks.delete, status: OperationStatus.COMPLETED, }); - const mockFinalizeTaskBody = { - parameters: {}, - type: jobDefinitionsConfig.tasks.finalize, - blockDuplication: true, - }; nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockDeleteTask.id }).reply(httpStatusCodes.OK, [mockDeleteTask]); nock(jobManagerConfigMock.jobManagerBaseUrl) .get(`/jobs/${mockDeleteLayerJob.id}`) .query({ shouldReturnTasks: false }) .reply(httpStatusCodes.OK, mockDeleteLayerJob); - nock(jobManagerConfigMock.jobManagerBaseUrl).post(`/jobs/${mockDeleteLayerJob.id}/tasks`, mockFinalizeTaskBody).reply(httpStatusCodes.CREATED); - const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount + 1); - nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.COMPLETED, percentage: 100 }) + .reply(httpStatusCodes.OK); // action const response = await requestSender.handleTaskNotification(mockDeleteTask.id); @@ -106,47 +101,15 @@ describe('tasks', function () { expect(response).toSatisfyApiSpec(); }); - it('should return 200 and create finalize task when getting completed excluded "tilesDeletion" task and no other task is pending', async () => { + it('should return 200 and complete job when getting completed excluded "tilesDeletion" task and no other task is pending', async () => { // mocks const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 3, completedTasks: 3 }); const mockExcludedTask = getTaskMock(mockDeleteLayerJob.id, { type: jobDefinitionsConfig.tasks.tilesDeletion, status: OperationStatus.COMPLETED, }); - const mockFinalizeTaskBody = { - parameters: {}, - type: jobDefinitionsConfig.tasks.finalize, - blockDuplication: true, - }; nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockExcludedTask.id }).reply(httpStatusCodes.OK, [mockExcludedTask]); - nock(jobManagerConfigMock.jobManagerBaseUrl) - .get(`/jobs/${mockDeleteLayerJob.id}`) - .query({ shouldReturnTasks: false }) - .reply(httpStatusCodes.OK, mockDeleteLayerJob); - nock(jobManagerConfigMock.jobManagerBaseUrl) - .post(`/jobs/${mockDeleteLayerJob.id}/tasks`, mockFinalizeTaskBody) - .reply(httpStatusCodes.CREATED); - const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount + 1); - nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); - - // action - const response = await requestSender.handleTaskNotification(mockExcludedTask.id); - - // expectation - expect(response.status).toBe(httpStatusCodes.OK); - expect(response).toSatisfyApiSpec(); - }); - - it('should return 200 and complete job when getting completed finalize task', async () => { - // mocks - const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 4, completedTasks: 4 }); - const mockFinalizeTask = getTaskMock(mockDeleteLayerJob.id, { - type: jobDefinitionsConfig.tasks.finalize, - status: OperationStatus.COMPLETED, - }); - - nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockFinalizeTask.id }).reply(httpStatusCodes.OK, [mockFinalizeTask]); nock(jobManagerConfigMock.jobManagerBaseUrl) .get(`/jobs/${mockDeleteLayerJob.id}`) .query({ shouldReturnTasks: false }) @@ -156,7 +119,7 @@ describe('tasks', function () { .reply(httpStatusCodes.OK); // action - const response = await requestSender.handleTaskNotification(mockFinalizeTask.id); + const response = await requestSender.handleTaskNotification(mockExcludedTask.id); // expectation expect(response.status).toBe(httpStatusCodes.OK); diff --git a/tests/mocks/configMock.ts b/tests/mocks/configMock.ts index 841c756..40845a1 100644 --- a/tests/mocks/configMock.ts +++ b/tests/mocks/configMock.ts @@ -103,7 +103,7 @@ const registerDefaultConfig = (): void => { ingestionTasksFlow: ['validation', 'create-tasks', 'tilesMerging', 'finalize'], exportTasksFlow: ['init', 'tilesExporting', 'polygon-parts', 'finalize'], seedTasksFlow: ['TilesSeeding'], - deleteLayerTasksFlow: ['delete', 'tiles-deletion', 'finalize'], + deleteLayerTasksFlow: ['delete', 'tiles-deletion'], }, }; diff --git a/tests/unit/tasks/handlers/jobHandler.spec.ts b/tests/unit/tasks/handlers/jobHandler.spec.ts index e0cfb93..9f28438 100644 --- a/tests/unit/tasks/handlers/jobHandler.spec.ts +++ b/tests/unit/tasks/handlers/jobHandler.spec.ts @@ -59,9 +59,11 @@ describe('JobHandler', () => { }); describe('handleCompletedNotification', () => { - const nonSeedCases = testCases.filter(({ mockJob }) => mockJob.type !== jobDefinitionsConfig.jobs.seed); // removing seed job test case as finalize task type is not handled there + const finalizeCases = testCases.filter( + ({ mockJob }) => mockJob.type !== jobDefinitionsConfig.jobs.seed && mockJob.type !== jobDefinitionsConfig.jobs.deleteLayer + ); // removing seed and deleteLayer job test cases as finalize task type is not handled there - it.each(nonSeedCases)( + it.each(finalizeCases)( `should complete job when all of the task are completed and task type is "finalize" - ${testCaseHandlerLog}`, async ({ mockJob }) => { mockJob = { ...mockJob, completedTasks: 10, taskCount: 10 }; From f72f66c3eccf53e54432d59c5ec04998e2dfc1e4 Mon Sep 17 00:00:00 2001 From: shlomiko Date: Sun, 26 Jul 2026 14:22:17 +0300 Subject: [PATCH 07/11] refactor: gate delete layer job completion on tiles-deletion task A tiles-deletion task always follows delete, so the job completes only when all tasks are done and the completed notification is for tiles-deletion - a completed delete alone never completes the job. --- docs/delete-layer-tasks-flow.md | 27 ++++++++++--------- .../deleteLayer/deleteLayerHandler.ts | 6 ++--- .../tasks/deleteLayer/taskManager.spec.ts | 7 +++-- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docs/delete-layer-tasks-flow.md b/docs/delete-layer-tasks-flow.md index 87ca58c..a3a1480 100644 --- a/docs/delete-layer-tasks-flow.md +++ b/docs/delete-layer-tasks-flow.md @@ -184,12 +184,11 @@ Configured as (`taskFlowManager.deleteLayerTasksFlow`): Walking through what actually happens for each possible completion notification, given `excludedTypes = [delete, tiles-deletion]`: -| Task that completed | `getNextTaskType()` result | Condition | Behavior | -| ------------------- | -------------------------------------------------- | ------------------------------------------------------------------- | -------------------------------------------------------------------------- | -| `delete` | skips `tiles-deletion` → `undefined` (end of flow) | `completedTasks === taskCount` | job **COMPLETED** (overridden `isJobCompleted`) | -| `delete` | same | `completedTasks !== taskCount` (other deletion tasks still pending) | progress update only | -| `tiles-deletion` | `undefined` (end of flow) | `completedTasks === taskCount` | job **COMPLETED** | -| any task, `FAILED` | — | `delete` is not in `suspendingTaskTypes` | job **FAILED** (default `JobHandler.handleFailedTask`, no override needed) | +| Task that completed | `getNextTaskType()` result | Condition | Behavior | +| ------------------- | -------------------------------------------------- | ---------------------------------------- | -------------------------------------------------------------------------- | +| `delete` | skips `tiles-deletion` → `undefined` (end of flow) | any | progress update only — completion is gated on `tiles-deletion` | +| `tiles-deletion` | `undefined` (end of flow) | `completedTasks === taskCount` | job **COMPLETED** (overridden `isJobCompleted`) | +| any task, `FAILED` | — | `delete` is not in `suspendingTaskTypes` | job **FAILED** (default `JobHandler.handleFailedTask`, no override needed) | Why `delete` and `tiles-deletion` can be created **in any order relative to each other** from job-tracker's point of view: it never creates them itself @@ -218,18 +217,20 @@ export class DeleteLayerJobHandler extends JobHandler { this.initializeTaskOperations(); } - public override isJobCompleted = (): boolean => { - return this.job.completedTasks === this.job.taskCount; + public override isJobCompleted = (taskType: TaskType): boolean => { + return this.job.completedTasks === this.job.taskCount && taskType === this.jobDefinitions.tasks.tilesDeletion; }; } ``` Design choices explained: -- **`isJobCompleted` override.** Like `SeedJobHandler`, `Delete_Layer` has no `finalize` - step at all, so the inherited `BaseJobHandler.isJobCompleted` (which additionally - requires `taskType === TaskTypes.Finalize`) would never fire. The override completes the - job as soon as `completedTasks === taskCount`. +- **`isJobCompleted` override.** `Delete_Layer` has no `finalize` step at all, so the + inherited `BaseJobHandler.isJobCompleted` (which requires `taskType === +TaskTypes.Finalize`) would never fire. Since a `tiles-deletion` task always follows + `delete`, the override gates completion on the `tiles-deletion` notification: the job + completes only when `completedTasks === taskCount` **and** the completed task is + `tiles-deletion` — a completed `delete` alone never completes the job. - **No `isProceedable` override / proceed rule.** Ingestion registers a `ValidationProceedRule` because a validation task can be "completed but invalid" and needs to suspend the job instead of proceeding. Nothing in the `Delete_Layer` flow has @@ -362,7 +363,7 @@ just that the handler wires up. | # | Scenario | Asserts | | --- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | `delete` completes, `completedTasks === taskCount` (nothing else pending) | job transitions to `COMPLETED` at 100% — no task created | +| 1 | `delete` completes, `completedTasks === taskCount` (nothing else pending) | progress updated only — job is **not** completed; completion is gated on `tiles-deletion` | | 2 | `delete` completes, `completedTasks < taskCount` (other deletion tasks still pending) | **no** task created; progress updated only | | 3 | `tiles-deletion` completes, all tasks done | job transitions to `COMPLETED` at 100% | | 4 | `delete` fails | job transitions to `FAILED` with the task's reason (default `handleFailedTask`, no suspend — `delete` isn't in `suspendingTaskTypes`) | diff --git a/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts b/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts index 5955ea2..74d919b 100644 --- a/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts +++ b/src/tasks/handlers/deleteLayer/deleteLayerHandler.ts @@ -2,7 +2,7 @@ import type { Logger } from '@map-colonies/js-logger'; import type { IJobResponse, ITaskResponse, JobManagerClient } from '@map-colonies/mc-priority-queue'; import { injectable, inject } from 'tsyringe'; import type { ConfigType } from '@src/common/config'; -import type { TaskTypes } from '../../../common/interfaces'; +import type { TaskType, TaskTypes } from '../../../common/interfaces'; import { SERVICES } from '../../../common/constants'; import { JobHandler } from '../jobHandler'; @@ -27,7 +27,7 @@ export class DeleteLayerJobHandler extends JobHandler { this.initializeTaskOperations(); } - public override isJobCompleted = (): boolean => { - return this.job.completedTasks === this.job.taskCount; + public override isJobCompleted = (taskType: TaskType): boolean => { + return this.job.completedTasks === this.job.taskCount && taskType === this.jobDefinitions.tasks.tilesDeletion; }; } diff --git a/tests/integration/tasks/deleteLayer/taskManager.spec.ts b/tests/integration/tasks/deleteLayer/taskManager.spec.ts index d941226..31dafe6 100644 --- a/tests/integration/tasks/deleteLayer/taskManager.spec.ts +++ b/tests/integration/tasks/deleteLayer/taskManager.spec.ts @@ -52,7 +52,7 @@ describe('tasks', function () { }); describe('Happy Path', function () { - it('should return 200 and complete job when getting completed delete task and no other task is pending', async () => { + it('should return 200 and only update progress when getting completed delete task and no other task is pending', async () => { // mocks const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 1, completedTasks: 1 }); const mockDeleteTask = getTaskMock(mockDeleteLayerJob.id, { @@ -65,9 +65,8 @@ describe('tasks', function () { .get(`/jobs/${mockDeleteLayerJob.id}`) .query({ shouldReturnTasks: false }) .reply(httpStatusCodes.OK, mockDeleteLayerJob); - nock(jobManagerConfigMock.jobManagerBaseUrl) - .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.COMPLETED, percentage: 100 }) - .reply(httpStatusCodes.OK); + const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount); + nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); // action const response = await requestSender.handleTaskNotification(mockDeleteTask.id); From 7c18b1ce21c8b478d5690c80b975785989298ca6 Mon Sep 17 00:00:00 2001 From: shlomiko Date: Sun, 26 Jul 2026 14:23:11 +0300 Subject: [PATCH 08/11] docs: remove delete-layer-tasks-flow document --- docs/delete-layer-tasks-flow.md | 424 -------------------------------- 1 file changed, 424 deletions(-) delete mode 100644 docs/delete-layer-tasks-flow.md diff --git a/docs/delete-layer-tasks-flow.md b/docs/delete-layer-tasks-flow.md deleted file mode 100644 index a3a1480..0000000 --- a/docs/delete-layer-tasks-flow.md +++ /dev/null @@ -1,424 +0,0 @@ -# Delete_Layer Tasks Flow - -Deep-dive documentation for the `Delete_Layer` job type support added to job-tracker. -This complements the top-level [README](../README.md) — read that first for the general -service overview, then come here for the full design rationale, architecture, and test -strategy behind this specific feature. - -## Table of contents - -- [1. Why this exists](#1-why-this-exists) -- [2. Cross-repo context](#2-cross-repo-context) -- [3. job-tracker's generic engine](#3-job-trackers-generic-engine) -- [4. The Delete_Layer design](#4-the-delete_layer-design) -- [5. Every file changed, and why](#5-every-file-changed-and-why) -- [6. Config wiring, end to end](#6-config-wiring-end-to-end) -- [7. The raster-shared dependency problem](#7-the-raster-shared-dependency-problem) -- [8. Test strategy](#8-test-strategy) -- [9. Verification performed](#9-verification-performed) -- [10. Comparison matrix: all four job handlers](#10-comparison-matrix-all-four-job-handlers) -- [11. Known limitations & follow-ups](#11-known-limitations--follow-ups) - ---- - -## 1. Why this exists - -MapColonies' raster stack supports deleting an entire layer (tiles, catalog record, -mapproxy/geoserver registration, artifacts). That operation is modeled as its own job -type, `Delete_Layer`, running through the same generic job/task orchestration that every -other job type (`Ingestion_New`, `Ingestion_Update`, `Ingestion_Swap_Update`, `Export`, -`TilesSeeding`) already uses. job-tracker is the service that owns that orchestration: it -reacts to task-completion webhooks and decides what happens next in a job's lifecycle. - -Before this change, job-tracker had **no knowledge of `Delete_Layer` at all** — a task -notification for a deletion job would have hit the `default` branch of the job handler -factory and thrown a `BadRequestError`. This document describes what was added to make -job-tracker a first-class participant in that flow. - -## 2. Cross-repo context - -This feature spans several repositories, and understanding job-tracker's slice requires -knowing what the others already do (as of when this was implemented): - -| Repo | Role | State found | -| ------------------- | ------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `raster-shared` | Shared types/schemas/constants (npm package) | Defines `DeletionJobTypes.Delete_Layer`, `DeletionTaskTypes.{Delete, LayerTilesDeletion}`, and the Zod schemas for each task's parameters — but **only on the `alpha` branch/dist-tag**, not in any stable release. | -| `ingestion-trigger` | Initiates ingestion/deletion jobs | Already pinned to `raster-shared@8.3.0-alpha.0`. Its `IngestionManager.deleteLayer()` creates a `Delete_Layer` job with **exactly one task**, type `delete`, params `{ deleteFromCatalog: false, deleteFromMapproxy: false, deleteFromGeoserver: false, deletePolygonParts: false }` (all flags currently hardcoded `false` — this part of the pipeline is itself still incomplete upstream). | -| `cleaner` | Worker that dequeues and executes deletion tasks | Only has a `tilesDeletionStrategy`, and its `worker.capabilities.pairs` config currently only pairs `tiles-deletion` with `Ingestion_Update` / `Ingestion_Swap_Update` — **not** with `Delete_Layer`. No strategy exists yet for `delete`. | -| `job-tracker` | Orchestrates the task flow (this repo) | **Nothing** — the subject of this change. | - -The practical implication: as of this change, nothing in the ecosystem actually _executes_ -the `delete` or `tiles-deletion` tasks for a `Delete_Layer` job yet. -job-tracker's job is to have the routing skeleton correctly in place so that when the rest -of the ecosystem catches up (cleaner adds a `Delete_Layer` pairing, something starts -consuming `delete`), the sequencing already works without further -changes here. - -## 3. job-tracker's generic engine - -Before describing what's new, here's the mechanism every job handler (including the new -one) plugs into. This is pre-existing code, but understanding it precisely is essential to -understanding why `Delete_Layer` was wired the way it was. - -### Request flow - -``` -POST /tasks/:taskId/notify - → TasksController.handleTaskNotification - → TasksManager.handleTaskNotification(taskId) - 1. findTask(taskId) — must be COMPLETED or FAILED, else 428 - 2. jobManager.getJob(task.jobId) - 3. getJobHandler(job.type, ...) — factory picks the concrete handler - 4. handler.handleFailedTask() — if task failed - handler.handleCompletedNotification() — if task completed -``` - -### The handler hierarchy - -``` -IJobHandler (interface) - └─ BaseJobHandler (abstract) — completeJob / failJob / suspendJob / updateJobProgress - └─ JobHandler (abstract) — task-flow-aware: tasksFlow, excludedTypes, blockedDuplicationTypes - ├─ IngestionJobHandler - ├─ ExportJobHandler - ├─ SeedJobHandler - └─ DeleteLayerJobHandler ← new -``` - -`JobHandler` composes a `TaskHandler` (in [`taskHandler.ts`](../src/tasks/handlers/taskHandler.ts)) -that does the actual sequencing math: - -```ts -public getNextTaskType(): TaskTypeItem | undefined { - const indexOfCurrentTask = this.tasksFlow.indexOf(this.task.type); - let nextTaskTypeIndex = indexOfCurrentTask + 1; - - // walk forward past any task type marked as "excluded" (skip) - while (nextTaskTypeIndex < this.tasksFlow.length) { - const nextTaskType = this.tasksFlow[nextTaskTypeIndex]; - if (nextTaskType === undefined || !this.shouldSkipTaskCreation(nextTaskType)) break; - nextTaskTypeIndex++; - } - - if (nextTaskTypeIndex >= this.tasksFlow.length) return undefined; - return this.tasksFlow[nextTaskTypeIndex]; -} - -public shouldSkipTaskCreation(taskType: string): boolean { - return this.excludedTypes.includes(taskType); -} -``` - -And [`jobHandler.ts`](../src/tasks/handlers/jobHandler.ts)'s `handleCompletedNotification` -decides what to do with that result: - -```ts -public async handleCompletedNotification(): Promise { - const nextTaskType = this.taskWorker?.getNextTaskType(); - - if (nextTaskType == undefined) { // end of the flow - await this.handleNoNextTask(); // → completeJob() or updateJobProgress() - return; - } - if (!this.isProceedable()) { // per-task-type business rule (e.g. validation) - await this.suspendJob(this.task.reason); - return; - } - if (!this.isReadyForNextTask() || this.shouldSkipTaskCreation(nextTaskType)) { - await this.handleSkipTask(nextTaskType); // → updateJobProgress() only - return; - } - await this.createNewTask(nextTaskType); // → jobManager.createTaskForJob(...) -} -``` - -Two concepts matter most for `Delete_Layer`: - -- **`excludedTypes`** — task types this handler will _never actively create_ via - `createTaskForJob`, because it doesn't have the real, per-instance parameters needed - (e.g. a `catalogId` or a list of S3 paths). Something else creates those tasks directly. - The handler still processes completion webhooks for them (`shouldSkipTaskCreation` - causes `getNextTaskType` to walk _past_ them when computing what to create next, but the - webhook for their own completion is handled exactly like any other task). -- **`isReadyForNextTask()`** — `job.completedTasks === job.taskCount`. This is what - prevents job-tracker from racing ahead and creating the next task before _every_ - currently-existing task for the job (including ones it didn't create itself) has - finished. - -### Static task parameters - -Whenever job-tracker _does_ create a task, it needs static parameters for it, resolved by -job-type + task-type from a fixed map in [`mappers.ts`](../src/common/mappers.ts): - -```ts -const key: JobAndTask = `${jobType}_${taskType}`; -const parameters = this.taskParametersMapper.get(key); -if (parameters === undefined) throw new BadRequestError(`task parameters for ${key} do not exist`); -``` - -This is a hard constraint: **job-tracker can only auto-create a task type if its -parameters are the same every time** (or empty). This is precisely why `tiles-deletion` -had to be excluded rather than auto-created — its parameters -(`catalogId`, `paths`, `sourceProvider`, `bucket`) are per-layer, dynamic data that -job-tracker has no way to produce generically. - -## 4. The Delete_Layer design - -### The flow - -```mermaid -flowchart LR - A["delete\n(created externally,\nby ingestion-trigger)"] -->|completed| B{"next in flow?"} - B -->|"tiles-deletion\n(excluded → skip)"| C{"end of flow"} - C -->|"all tasks completed"| F["job COMPLETED"] - - A -.->|"tiles-deletion completes\n(webhook received)"| C -``` - -Configured as (`taskFlowManager.deleteLayerTasksFlow`): - -```json -["delete", "tiles-deletion"] -``` - -Walking through what actually happens for each possible completion notification, given -`excludedTypes = [delete, tiles-deletion]`: - -| Task that completed | `getNextTaskType()` result | Condition | Behavior | -| ------------------- | -------------------------------------------------- | ---------------------------------------- | -------------------------------------------------------------------------- | -| `delete` | skips `tiles-deletion` → `undefined` (end of flow) | any | progress update only — completion is gated on `tiles-deletion` | -| `tiles-deletion` | `undefined` (end of flow) | `completedTasks === taskCount` | job **COMPLETED** (overridden `isJobCompleted`) | -| any task, `FAILED` | — | `delete` is not in `suspendingTaskTypes` | job **FAILED** (default `JobHandler.handleFailedTask`, no override needed) | - -Why `delete` and `tiles-deletion` can be created **in any order -relative to each other** from job-tracker's point of view: it never creates them itself -(both are either the pre-existing first task or excluded), so whichever one's -completion webhook arrives, `isReadyForNextTask()` is the real gate — job-tracker only -completes the job once every task that exists for the job (regardless of who created it -or in what order) is done. - -### The handler implementation - -[`src/tasks/handlers/deleteLayer/deleteLayerHandler.ts`](../src/tasks/handlers/deleteLayer/deleteLayerHandler.ts): - -```ts -@injectable() -export class DeleteLayerJobHandler extends JobHandler { - protected readonly tasksFlow: TaskTypes; - protected readonly excludedTypes: TaskTypes; - protected readonly blockedDuplicationTypes: TaskTypes; - - public constructor(/* … */) { - super(logger, config, jobManagerClient, job, task); - this.tasksFlow = this.config.get('taskFlowManager.deleteLayerTasksFlow') as unknown as TaskTypes; - this.excludedTypes = [this.jobDefinitions.tasks.delete, this.jobDefinitions.tasks.tilesDeletion]; - this.blockedDuplicationTypes = []; - - this.initializeTaskOperations(); - } - - public override isJobCompleted = (taskType: TaskType): boolean => { - return this.job.completedTasks === this.job.taskCount && taskType === this.jobDefinitions.tasks.tilesDeletion; - }; -} -``` - -Design choices explained: - -- **`isJobCompleted` override.** `Delete_Layer` has no `finalize` step at all, so the - inherited `BaseJobHandler.isJobCompleted` (which requires `taskType === -TaskTypes.Finalize`) would never fire. Since a `tiles-deletion` task always follows - `delete`, the override gates completion on the `tiles-deletion` notification: the job - completes only when `completedTasks === taskCount` **and** the completed task is - `tiles-deletion` — a completed `delete` alone never completes the job. -- **No `isProceedable` override / proceed rule.** Ingestion registers a - `ValidationProceedRule` because a validation task can be "completed but invalid" and - needs to suspend the job instead of proceeding. Nothing in the `Delete_Layer` flow has - an equivalent conditional-proceed semantic, so the default (`isProceedable` always - returns `true` unless a rule is registered) is correct. -- **No `handleFailedTask` override.** Export overrides this to special-case a "create an - error-callback finalize task instead of just failing" behavior. Nothing in the ticket - scope called for that for deletion, so the default `JobHandler.handleFailedTask` (fail - the job, unless the task type is in `suspendingTaskTypes`) applies as-is. -- **`excludedTypes = [delete, tilesDeletion]`** — job-tracker is not in charge of - creating any of the delete-layer tasks; both are created externally, so both are - excluded from auto-creation. With no `finalize` in the flow, job-tracker never creates - any task for this job type — it only tracks progress and completes/fails the job. -- **`blockedDuplicationTypes = []`** — mirroring `SeedJobHandler`; no duplication - blocking is applied to the tasks this handler creates. - -## 5. Every file changed, and why - -| File | Change | -| --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `package.json` | `@map-colonies/raster-shared` bumped `^7.10.2` → `8.3.0-alpha.0` (exact pin). See [§7](#7-the-raster-shared-dependency-problem). | -| `package-lock.json` | Regenerated by `npm install` for the above. | -| `src/common/interfaces.ts` | Added `jobs.deleteLayer` and `tasks.delete` to `IJobDefinitionsConfig`. (`tasks.tilesDeletion` already existed in the interface but had never been populated in `config/default.json` — that gap is now also closed.) | -| `src/common/mappers.ts` | No `Delete_Layer` entry — job-tracker never creates a task for this job type (all tasks are externally created and there is no `finalize` step), so no static task parameters are needed. | -| `src/tasks/handlers/deleteLayer/deleteLayerHandler.ts` | **New.** The handler itself — see [§4](#4-the-delete_layer-design). | -| `src/tasks/handlers/jobHandlerFactory.ts` | Added the `jobDefinitions.jobs.deleteLayer` case to the factory switch. | -| `config/default.json` | Added `jobDefinitions.jobs.deleteLayer`, the new/completed task entries, and `taskFlowManager.deleteLayerTasksFlow`. This is the single source of truth for local dev and tests (`config/test.json` and `config/production.json` are both `{}` — everything is either defaulted here or overridden by env vars in prod). | -| `config/custom-environment-variables.json` | Env var names for every new/completed config key, so helm can inject real values in each deployed environment. | -| `helm/values.yaml` | Chart defaults for the new job/task types and the new flow array. | -| `helm/templates/configmap.yaml` | Renders the new env vars (`JOB_DEFINITIONS_JOB_DELETE_LAYER`, `JOB_DEFINITIONS_TASK_DELETE`, `DELETE_LAYER_TASKS_FLOW`) into the ConfigMap. | -| `tests/mocks/configMock.ts` | Test-side mirror of `config/default.json` — same additions, plus the previously-missing `tilesDeletion`/`delete` task entries needed for any test to reference them. | -| `tests/mocks/jobMocks.ts` | New `getDeleteLayerJobMock()`, mirroring the existing `getExportJobMock`/`getSeedingJobMock` pattern. | -| `tests/unit/tasks/handlers/jobHandler.spec.ts` | Added a `Delete_Layer` case to the generic cross-handler `testCases` array — see [§8](#8-test-strategy). | -| `tests/unit/tasks/handlers/jobHandlerFactory.spec.ts` | New test: factory returns a `DeleteLayerJobHandler` instance for the `deleteLayer` job type. | -| `tests/integration/tasks/deleteLayer/taskManager.spec.ts` | **New.** Full HTTP-level integration suite — see [§8](#8-test-strategy). | - -## 6. Config wiring, end to end - -Every new job/task identifier and the new flow array had to be threaded through **four** -layers consistently, since a mismatch at any layer means the wrong value (or `undefined`) -in some environment: - -``` -config/default.json (local dev / test fallback values) - │ -config/custom-environment-variables.json (maps config keys → env var names) - │ -helm/values.yaml (chart default values, per-environment overridable) - │ -helm/templates/configmap.yaml (renders the actual env vars injected at deploy time) -``` - -Concretely, for the job type: - -| Layer | Value | -| ----------------------------------- | ---------------------------------------------------------------------------------------- | -| `config/default.json` | `jobDefinitions.jobs.deleteLayer = "Delete_Layer"` | -| `custom-environment-variables.json` | `jobDefinitions.jobs.deleteLayer = "JOB_DEFINITIONS_JOB_DELETE_LAYER"` | -| `helm/values.yaml` | `jobDefinitions.jobs.deleteLayer.type: ''` (filled in per-environment values override) | -| `helm/templates/configmap.yaml` | `JOB_DEFINITIONS_JOB_DELETE_LAYER: {{ $jobDefinitions.jobs.deleteLayer.type \| quote }}` | - -And the same four-layer pattern for `tasks.delete` and -`taskFlowManager.deleteLayerTasksFlow` (the last one is a JSON-encoded array end-to-end: -`DELETE_LAYER_TASKS_FLOW` env var → parsed back into an array by the `@map-colonies/config` -library via the `__format: "json"` directive in `custom-environment-variables.json`). - -This was validated by rendering the actual chart (see [§9](#9-verification-performed)) — -not just by eyeballing the four files — since Helm's Go-templating syntax isn't valid -standalone YAML and a plain YAML linter can't confirm correctness (the IDE's inline -diagnostics on `configmap.yaml` during this work were exactly that false-positive: they -flagged the _new_ lines using the identical `{{ }}` pattern already used everywhere else -in the file). - -## 7. The raster-shared dependency problem - -This was the most consequential discovery during investigation, because it blocks the -feature at the type level, not just the routing level. - -- `raster-shared`'s `DeletionJobTypes`/`DeletionTaskTypes`/`DeleteLayerJobParams`/ - `DeleteTaskParams` were added in commit `8d50c52` ("feat: add layer deletion schemas, - types and constants (MAPCO-7285)"), which landed on the **`alpha` branch**. -- Checked the CHANGELOG entry ("add tile deletion schemas and types", #191) — it appears - under `8.1.0-alpha.0`, which _looks_ like it should have rolled into the stable `8.1.0` - release. It did not: `git ls-tree` on the commit that produced the published `8.2.0` - package (`6aad051`, "bump version from 8.1.0 to 8.2.0") confirms **no `deletion` - directory exists in that commit**. The alpha and stable release trains have diverged. -- The only place these types exist in a published package is the `alpha` dist-tag, - currently `8.3.0-alpha.1` (confirmed via `npm pack @map-colonies/raster-shared@alpha`). -- `ingestion-trigger` already depends on `raster-shared@8.3.0-alpha.0` (exact pin, not a - caret range) — this repo's `package.json` now matches that exact version for - consistency between the two services consuming the same in-flight schema. -- Confirmed no breaking change from the `8.0.0-alpha.0` release (which _did_ have a - genuine breaking change — "change polygon part validation error to new format, - MAPCO-10506") affects job-tracker: this repo only imports `TaskTypes`, - `ExportFinalizeType`, the ingestion/export finalize param types, and - `ingestionValidationTaskParamsSchema` (just `.pick({isValid: true})` off it) — none of - which touch the polygon-parts validation error shape. `tsc --noEmit` after the bump - confirmed this empirically. - -**Action item for whoever owns this long-term:** once `raster-shared`'s deletion -feature ships in a stable release, both `job-tracker` and `ingestion-trigger` need to move -off the alpha pin together, to a matching stable version. - -## 8. Test strategy - -### Unit tests - -`tests/unit/tasks/handlers/jobHandler.spec.ts` runs a shared battery of generic behavior -assertions across **every** job handler type, driven by one `testCases` array. Adding -`{ mockJob: createTestJob(jobs.deleteLayer), taskType: tasks.delete }` to that array gets -`Delete_Layer` covered, for free, by: - -- `handleFailedTask` → fails the job with the task's reason. -- `handleCompletedNotification` when `isProceedable()` is mocked `false` → suspends the job. -- `handleCompletedNotification` when the task is still `IN_PROGRESS` → progress update only, no task created. -- `handleCompletedNotification` when other tasks in the job are still incomplete → progress update only, no task created. - -`tests/unit/tasks/handlers/jobHandlerFactory.spec.ts` adds one targeted test: the factory -returns a `DeleteLayerJobHandler` instance for `jobDefinitions.jobs.deleteLayer`. - -### Integration tests - -`tests/integration/tasks/deleteLayer/taskManager.spec.ts` drives the actual Express app -through `supertest`, with `nock` mocking every job-manager HTTP call, validating each -response against the OpenAPI spec (`toSatisfyApiSpec()`). Unlike the unit tests, this -exercises the **real** `TaskHandler.getNextTaskType()` / `shouldSkipTaskCreation()` logic -against the real `deleteLayerTasksFlow` array — nothing here is mocked at that layer, so -these tests are what actually prove the skip-over-excluded-types sequencing works, not -just that the handler wires up. - -| # | Scenario | Asserts | -| --- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | `delete` completes, `completedTasks === taskCount` (nothing else pending) | progress updated only — job is **not** completed; completion is gated on `tiles-deletion` | -| 2 | `delete` completes, `completedTasks < taskCount` (other deletion tasks still pending) | **no** task created; progress updated only | -| 3 | `tiles-deletion` completes, all tasks done | job transitions to `COMPLETED` at 100% | -| 4 | `delete` fails | job transitions to `FAILED` with the task's reason (default `handleFailedTask`, no suspend — `delete` isn't in `suspendingTaskTypes`) | - -### Results - -``` -Unit: 134/134 passing, 100% statements/branches/functions/lines on the new handler -Integration: 94/94 passing (4 new scenarios) -``` - -## 9. Verification performed - -Beyond the automated test suites: - -- `tsc --noEmit` — clean after the `raster-shared` bump (confirms no breaking-change fallout). -- `eslint .` — clean. -- `prettier --check .` — clean (one formatting fix applied to the new integration spec). -- **Helm chart rendering** — since the chart has an external OCI dependency (`mclabels`) - not resolvable offline, the chart was copied to a scratch directory, the dependency - block stripped, and a minimal local stub for the `mclabels.labels`/`mclabels. -selectorLabels` template functions was added just to satisfy `helm template`'s parser. - Rendering `templates/configmap.yaml` confirmed every new env var (including the - JSON-encoded `DELETE_LAYER_TASKS_FLOW`) renders correctly. This is real evidence, not - just visual inspection — Helm's Go-templating means a file can look syntactically odd to - a plain-YAML linter while still being perfectly valid, and the only way to know for sure - is to actually render it. - -## 10. Comparison matrix: all four job handlers - -| | `IngestionJobHandler` | `ExportJobHandler` | `SeedJobHandler` | `DeleteLayerJobHandler` | -| ------------------------------------ | ------------------------------------- | -------------------------------------------- | ------------------------------- | ---------------------------------------- | -| Ends in `finalize`? | ✅ | ✅ | ❌ (overrides `isJobCompleted`) | ❌ (overrides `isJobCompleted`) | -| `excludedTypes` | `[merge, tilesDeletion]` | `[export]` (the heavy `tilesExporting` step) | `[seed]` (its only task) | `[delete, tilesDeletion]` | -| `blockedDuplicationTypes` | `[validation, createTasks, finalize]` | `[export]` | `[]` | `[]` | -| Custom `isProceedable`/proceed rule? | ✅ `ValidationProceedRule` | ❌ | ❌ | ❌ | -| Custom `handleFailedTask`? | ❌ (default) | ✅ (error-callback finalize task) | ❌ (default) | ❌ (default) | -| First task created by | job-tracker itself (`validation`) | external initiator | external initiator | external initiator (`ingestion-trigger`) | - -This table is a useful reference the next time a new job type needs to be added — it -makes explicit which knobs actually vary between handlers versus which ones are -essentially always the same. - -## 11. Known limitations & follow-ups - -- **`raster-shared` alpha pin** — see [§7](#7-the-raster-shared-dependency-problem). This - is a real, tracked risk: alpha packages can be republished or diverge without the usual - semver guarantees. -- **Nothing consumes `delete` yet**, and `cleaner` doesn't have a - `Delete_Layer` → `tiles-deletion` pairing configured. job-tracker's routing is correct - and tested in isolation, but the end-to-end feature isn't runnable in a real environment - until those other services catch up — this doc's scope was explicitly job-tracker's - slice only. -- **`ingestion-trigger` currently hardcodes all four `delete` task flags to `false`** - (`deleteFromCatalog/Mapproxy/Geoserver/PolygonParts`), so even once a worker consumes - the `delete` task, it wouldn't currently be told to delete anything from those - subsystems. Out of scope for job-tracker, but worth knowing if the flow doesn't seem to - "do" anything end-to-end yet. From 631d06af6448cf49d2383b0b658ed9c24c704b65 Mon Sep 17 00:00:00 2001 From: shlomiko Date: Sun, 26 Jul 2026 14:23:32 +0300 Subject: [PATCH 09/11] docs: remove dangling link to deleted delete-layer-tasks-flow doc --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index 192e984..dade23d 100644 --- a/README.md +++ b/README.md @@ -7,8 +7,6 @@ Services notifies Job Tracker on completed tasks, and Job Tracker handles the re ## API Check out the OpenAPI spec [here](/openapi3.yaml) -## Job type deep-dives -- [Delete_Layer tasks flow](docs/delete-layer-tasks-flow.md) — architecture, design rationale, and test strategy for the `Delete_Layer` job type. #### Get all items ```http From 9a173d42e8ae7d7f2cbb6a8deed251c6c7207c85 Mon Sep 17 00:00:00 2001 From: shlomiko Date: Sun, 26 Jul 2026 14:36:01 +0300 Subject: [PATCH 10/11] refactor: rename tiles-deletion env var to JOB_DEFINITIONS_TASK_TILES_DELETION JOB_DEFINITIONS_TASK_DELETION was easily confused with JOB_DEFINITIONS_TASK_DELETE (the delete task). --- README.md | 2 +- config/custom-environment-variables.json | 2 +- helm/templates/configmap.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index dade23d..64663d1 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ Start the server |`JOB_DEFINITIONS_JOB_SWAP_UPDATE` |Job name. |Ingestion_Swap_Update | |`JOB_DEFINITIONS_TASK_INIT` |Task name. |init | |`JOB_DEFINITIONS_TASK_MERGE` |Task name. |merge | -|`JOB_DEFINITIONS_TASK_DELETION` |Task name. |merge | +|`JOB_DEFINITIONS_TASK_TILES_DELETION`|Task name. |tiles-deletion | |`JOB_DEFINITIONS_TASK_POLYGON_PARTS`|Task name. |polygon-parts | |`JOB_DEFINITIONS_TASK_FINALIZE` |Task name. |finalize | |`HTTP_RETRY_ATTEMPTS` |How many retries should the service make if a request fails. |5 | diff --git a/config/custom-environment-variables.json b/config/custom-environment-variables.json index 308dad2..e527ae7 100644 --- a/config/custom-environment-variables.json +++ b/config/custom-environment-variables.json @@ -76,7 +76,7 @@ "init": "JOB_DEFINITIONS_TASK_INIT", "createTasks": "JOB_DEFINITIONS_TASK_CREATE_TASKS", "merge": "JOB_DEFINITIONS_TASK_MERGE", - "tilesDeletion": "JOB_DEFINITIONS_TASK_DELETION", + "tilesDeletion": "JOB_DEFINITIONS_TASK_TILES_DELETION", "polygonParts": "JOB_DEFINITIONS_TASK_POLYGON_PARTS", "export": "JOB_DEFINITIONS_TASK_EXPORT", "finalize": "JOB_DEFINITIONS_TASK_FINALIZE", diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index f00dc7b..cb9dfea 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -37,7 +37,7 @@ data: JOB_DEFINITIONS_TASK_INIT: {{ $jobDefinitions.tasks.init.type | quote }} JOB_DEFINITIONS_TASK_VALIDATION: {{ $jobDefinitions.tasks.validation.type | quote }} JOB_DEFINITIONS_TASK_MERGE: {{ $jobDefinitions.tasks.merge.type | quote }} - JOB_DEFINITIONS_TASK_DELETION: {{ $jobDefinitions.tasks.tilesDeletion.type | quote }} + JOB_DEFINITIONS_TASK_TILES_DELETION: {{ $jobDefinitions.tasks.tilesDeletion.type | quote }} JOB_DEFINITIONS_TASK_POLYGON_PARTS: {{ $jobDefinitions.tasks.polygonParts.type | quote }} JOB_DEFINITIONS_TASK_CREATE_TASKS: {{ $jobDefinitions.tasks.createTasks.type | quote }} JOB_DEFINITIONS_TASK_EXPORT: {{ $jobDefinitions.tasks.export.type | quote }} From 7f6a74ad6ec2a77e8d231f5d3f7af05853481ae7 Mon Sep 17 00:00:00 2001 From: shlomiko Date: Sun, 26 Jul 2026 14:48:45 +0300 Subject: [PATCH 11/11] test: adjust delete layer integration coverage Remove redundant pending-deletion-tasks progress test (only one delete task ever exists) and add a failed tiles-deletion task scenario. --- .../tasks/deleteLayer/taskManager.spec.ts | 50 ++++++++++--------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/tests/integration/tasks/deleteLayer/taskManager.spec.ts b/tests/integration/tasks/deleteLayer/taskManager.spec.ts index 31dafe6..09f6058 100644 --- a/tests/integration/tasks/deleteLayer/taskManager.spec.ts +++ b/tests/integration/tasks/deleteLayer/taskManager.spec.ts @@ -76,75 +76,79 @@ describe('tasks', function () { expect(response).toSatisfyApiSpec(); }); - it('should return 200 and only update progress when getting completed delete task while other deletion tasks are still pending', async () => { + it('should return 200 and complete job when getting completed excluded "tilesDeletion" task and no other task is pending', async () => { // mocks - const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 3, completedTasks: 1 }); - const mockDeleteTask = getTaskMock(mockDeleteLayerJob.id, { - type: jobDefinitionsConfig.tasks.delete, + const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 3, completedTasks: 3 }); + const mockExcludedTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks.tilesDeletion, status: OperationStatus.COMPLETED, }); - nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockDeleteTask.id }).reply(httpStatusCodes.OK, [mockDeleteTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockExcludedTask.id }).reply(httpStatusCodes.OK, [mockExcludedTask]); nock(jobManagerConfigMock.jobManagerBaseUrl) .get(`/jobs/${mockDeleteLayerJob.id}`) .query({ shouldReturnTasks: false }) .reply(httpStatusCodes.OK, mockDeleteLayerJob); - const taskPercentage = calculateJobPercentage(mockDeleteLayerJob.completedTasks, mockDeleteLayerJob.taskCount); - nock(jobManagerConfigMock.jobManagerBaseUrl).put(`/jobs/${mockDeleteLayerJob.id}`, { percentage: taskPercentage }).reply(httpStatusCodes.OK); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.COMPLETED, percentage: 100 }) + .reply(httpStatusCodes.OK); // action - const response = await requestSender.handleTaskNotification(mockDeleteTask.id); + const response = await requestSender.handleTaskNotification(mockExcludedTask.id); // expectation expect(response.status).toBe(httpStatusCodes.OK); expect(response).toSatisfyApiSpec(); }); - it('should return 200 and complete job when getting completed excluded "tilesDeletion" task and no other task is pending', async () => { + it('should return 200 and fail job when getting failed delete task', async () => { // mocks - const mockDeleteLayerJob = getDeleteLayerJobMock({ taskCount: 3, completedTasks: 3 }); - const mockExcludedTask = getTaskMock(mockDeleteLayerJob.id, { - type: jobDefinitionsConfig.tasks.tilesDeletion, - status: OperationStatus.COMPLETED, + const mockDeleteLayerJob = getDeleteLayerJobMock(); + const mockDeleteTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks.delete, + status: OperationStatus.FAILED, + reason: 'delete task failed', }); - nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockExcludedTask.id }).reply(httpStatusCodes.OK, [mockExcludedTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockDeleteTask.id }).reply(httpStatusCodes.OK, [mockDeleteTask]); nock(jobManagerConfigMock.jobManagerBaseUrl) .get(`/jobs/${mockDeleteLayerJob.id}`) .query({ shouldReturnTasks: false }) .reply(httpStatusCodes.OK, mockDeleteLayerJob); nock(jobManagerConfigMock.jobManagerBaseUrl) - .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.COMPLETED, percentage: 100 }) + .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.FAILED, reason: mockDeleteTask.reason }) .reply(httpStatusCodes.OK); // action - const response = await requestSender.handleTaskNotification(mockExcludedTask.id); + const response = await requestSender.handleTaskNotification(mockDeleteTask.id); // expectation expect(response.status).toBe(httpStatusCodes.OK); expect(response).toSatisfyApiSpec(); }); - it('should return 200 and fail job when getting failed delete task', async () => { + it('should return 200 and fail job when getting failed tiles-deletion task', async () => { // mocks const mockDeleteLayerJob = getDeleteLayerJobMock(); - const mockDeleteTask = getTaskMock(mockDeleteLayerJob.id, { - type: jobDefinitionsConfig.tasks.delete, + const mockTilesDeletionTask = getTaskMock(mockDeleteLayerJob.id, { + type: jobDefinitionsConfig.tasks.tilesDeletion, status: OperationStatus.FAILED, - reason: 'delete task failed', + reason: 'tiles-deletion task failed', }); - nock(jobManagerConfigMock.jobManagerBaseUrl).post('/tasks/find', { id: mockDeleteTask.id }).reply(httpStatusCodes.OK, [mockDeleteTask]); + nock(jobManagerConfigMock.jobManagerBaseUrl) + .post('/tasks/find', { id: mockTilesDeletionTask.id }) + .reply(httpStatusCodes.OK, [mockTilesDeletionTask]); nock(jobManagerConfigMock.jobManagerBaseUrl) .get(`/jobs/${mockDeleteLayerJob.id}`) .query({ shouldReturnTasks: false }) .reply(httpStatusCodes.OK, mockDeleteLayerJob); nock(jobManagerConfigMock.jobManagerBaseUrl) - .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.FAILED, reason: mockDeleteTask.reason }) + .put(`/jobs/${mockDeleteLayerJob.id}`, { status: OperationStatus.FAILED, reason: mockTilesDeletionTask.reason }) .reply(httpStatusCodes.OK); // action - const response = await requestSender.handleTaskNotification(mockDeleteTask.id); + const response = await requestSender.handleTaskNotification(mockTilesDeletionTask.id); // expectation expect(response.status).toBe(httpStatusCodes.OK);