From 2554942deba5df73f3f35109d696608b09f9538c Mon Sep 17 00:00:00 2001 From: ronenk1 Date: Sun, 26 Mar 2023 11:52:44 +0300 Subject: [PATCH 1/4] chore: API changes for integration + helm improvement --- helm/templates/deployment.yaml | 1 + helm/values.yaml | 1 + openapi3.yaml | 27 +++++++-- src/clients/jobManagerWrapper.ts | 8 ++- src/common/interfaces.ts | 21 +++++-- .../controllers/createPackageController.ts | 5 +- .../models/createPackageManager.ts | 58 +++++++++++-------- src/tasks/models/tasksManager.ts | 29 +++++----- .../createPackage/createExportPackage.spec.ts | 10 ++-- tests/mocks/data.ts | 2 +- tests/mocks/data/mockJob.ts | 2 +- tests/unit/clients/jobManagerClient.spec.ts | 6 +- .../models/createPackageModel.spec.ts | 9 +-- .../createPackage/models/tasksModel.spec.ts | 19 +++--- tests/unit/finalizationManager.spec.ts | 6 +- 15 files changed, 129 insertions(+), 75 deletions(-) diff --git a/helm/templates/deployment.yaml b/helm/templates/deployment.yaml index a922b7f..71481d1 100644 --- a/helm/templates/deployment.yaml +++ b/helm/templates/deployment.yaml @@ -60,6 +60,7 @@ spec: - name: http containerPort: {{ .Values.env.targetPort }} protocol: TCP + imagePullPolicy: {{ .Values.imagePullPolicy }} livenessProbe: initialDelaySeconds: {{ .Values.initialDelaySeconds }} httpGet: diff --git a/helm/values.yaml b/helm/values.yaml index b08e452..1286b9f 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -35,6 +35,7 @@ replicaCount: 1 initialDelaySeconds: 60 nodePort: 30030 #for minikube deployment only resetOnConfigChange: true +imagePullPolicy: Always cloudProvider: dockerRegistryUrl: my-registry-url.io diff --git a/openapi3.yaml b/openapi3.yaml index b90d84b..8ae558e 100644 --- a/openapi3.yaml +++ b/openapi3.yaml @@ -87,7 +87,7 @@ paths: application/json: schema: oneOf: - - $ref: '#/components/schemas/createGpkgJobResponse' + - $ref: '#/components/schemas/createRoiGpkgJobResponse' - $ref: '#/components/schemas/exportNaiveCacheJobResponse' discriminator: propertyName: response @@ -258,9 +258,11 @@ components: description: The priority of the record. Maximum priority = most urgent. minimum: 0 maximum: 999999999 + description: + type: string + description: free test to describe the requested export required: - dbId - - callbackURLs example: dbId: ef03ca54-c68e-4ca8-8432-50ae5ad7a7f8 roi: @@ -287,6 +289,7 @@ components: - http://example.getmap.com/callback2 crs: EPSG:4326 priority: 0 + description: This is roi exporting example createGpkgJobResponse: allOf: - $ref: '#/components/schemas/CommonResponse' @@ -303,6 +306,22 @@ components: required: - id - taskIds + createRoiGpkgJobResponse: + allOf: + - $ref: '#/components/schemas/CommonResponse' + type: object + properties: + jobId: + type: string + format: uuid + taskIds: + type: array + items: + type: string + format: uuid + required: + - jobId + - taskIds naiveCacheJobResponse: allOf: - $ref: '#/components/schemas/CommonResponse' @@ -363,7 +382,7 @@ components: format: uuid roi: $ref: '#/components/schemas/FeatureCollection' - requestJobId: + jobId: type: string format: uuid required: @@ -372,7 +391,7 @@ components: - fileSize - recordCatalogId - roi - - requestJobId + - jobId - status error: type: object diff --git a/src/clients/jobManagerWrapper.ts b/src/clients/jobManagerWrapper.ts index e54d51a..83618f9 100644 --- a/src/clients/jobManagerWrapper.ts +++ b/src/clients/jobManagerWrapper.ts @@ -11,6 +11,7 @@ import { CreateJobBody, ExportVersion, ICreateJobResponse, + ICreateExportJobResponse, IJobExportParameters, IJobParameters, ITaskParameters, @@ -93,7 +94,7 @@ export class JobManagerWrapper extends JobManagerClient { }; } - public async createExport(data: IWorkerExportInput): Promise { + public async createExport(data: IWorkerExportInput): Promise { const expirationDate = new Date(); expirationDate.setDate(expirationDate.getDate() + this.expirationDays); @@ -117,6 +118,7 @@ export class JobManagerWrapper extends JobManagerClient { productType: data.productType, productName: data.cswProductId, priority: data.priority, + description: data.description, status: OperationStatus.IN_PROGRESS, additionalIdentifiers: data.relativeDirectoryPath, tasks: [ @@ -130,8 +132,8 @@ export class JobManagerWrapper extends JobManagerClient { ], }; const res = await this.createJob(createJobRequest); - const createJobResponse: ICreateJobResponse = { - id: res.id, + const createJobResponse: ICreateExportJobResponse = { + jobId: res.id, taskIds: res.taskIds, status: OperationStatus.IN_PROGRESS, }; diff --git a/src/common/interfaces.ts b/src/common/interfaces.ts index 62c0edc..0ae186c 100644 --- a/src/common/interfaces.ts +++ b/src/common/interfaces.ts @@ -16,7 +16,6 @@ export interface OpenApiConfig { export interface IBaseCreatePackage { dbId: string; - callbackURLs: string[]; crs?: string; priority?: number; } @@ -32,10 +31,13 @@ export interface ICleanupData { export interface ICreatePackage extends IBaseCreatePackage { targetResolution?: number; bbox?: BBox | Polygon | MultiPolygon; + callbackURLs: string[]; } export interface ICreatePackageRoi extends IBaseCreatePackage { roi?: FeatureCollection; + callbackURLs?: string[]; + description?: string; } export interface ICallbackBase { @@ -79,21 +81,31 @@ export interface IWorkerInput extends IWorkerInputBase { } export interface IWorkerExportInput extends IWorkerInputBase { - callbacks: ICallbackTargetExport[]; + callbacks?: ICallbackTargetExport[]; roi: FeatureCollection; fileNamesTemplates: ILinkDefinition; + description?: string; } export interface IBasicResponse { message: string; } +/** + * @deprecated GetMap API - will be deprecated on future + */ export interface ICreateJobResponse { id: string; taskIds: string[]; status: OperationStatus.IN_PROGRESS | OperationStatus.COMPLETED; } +export interface ICreateExportJobResponse { + jobId: string; + taskIds: string[]; + status: OperationStatus.IN_PROGRESS | OperationStatus.COMPLETED; +} + /** * @deprecated GetMap API - will be deprecated on future */ @@ -114,8 +126,9 @@ export interface ICallbackDataExportBase { expirationTime: Date; fileSize: number; recordCatalogId: string; - requestJobId: string; + jobId: string; errorReason?: string; + description?: string; } /** @@ -187,7 +200,7 @@ export interface IJobExportParameters { crs: string; exportVersion: ExportVersion; roi: FeatureCollection; - callbacks: ICallbackTargetExport[]; + callbacks?: ICallbackTargetExport[]; callbackParams?: ICallbackExportResponse; fileNamesTemplates: ILinkDefinition; gpkgEstimatedSize?: number; diff --git a/src/createPackage/controllers/createPackageController.ts b/src/createPackage/controllers/createPackageController.ts index 94d3a37..952e9d6 100644 --- a/src/createPackage/controllers/createPackageController.ts +++ b/src/createPackage/controllers/createPackageController.ts @@ -12,11 +12,12 @@ import { ICallbackResposne, ICreatePackageRoi, ICallbackExportResponse, + ICreateExportJobResponse, } from '../../common/interfaces'; type CreatePackageHandler = RequestHandler< undefined, - IBasicResponse | ICreateJobResponse | ICallbackResposne | ICallbackExportResponse, + IBasicResponse | ICreateJobResponse | ICreateExportJobResponse | ICallbackResposne | ICallbackExportResponse, ICreatePackage | ICreatePackageRoi >; @@ -29,7 +30,7 @@ export class CreatePackageController { ) {} public create: CreatePackageHandler = async (req, res, next) => { - const userInput: ICreatePackage = req.body; + const userInput: ICreatePackage = req.body as ICreatePackage; try { this.logger.debug(userInput, `Creating package with user input`); const jobCreated = await this.manager.createPackage(userInput); diff --git a/src/createPackage/models/createPackageManager.ts b/src/createPackage/models/createPackageManager.ts index 68e379d..47daddb 100644 --- a/src/createPackage/models/createPackageManager.ts +++ b/src/createPackage/models/createPackageManager.ts @@ -35,6 +35,7 @@ import { ICallbackExportResponse, ICallbackTargetExport, IConfig, + ICreateExportJobResponse, ICreatePackageRoi, IGeometryRecord, IJobExportParameters, @@ -206,8 +207,8 @@ export class CreatePackageManager { return jobCreated; } - public async createPackageRoi(userInput: ICreatePackageRoi): Promise { - const { dbId, crs, priority, callbackURLs } = userInput; + public async createPackageRoi(userInput: ICreatePackageRoi): Promise { + const { dbId, crs, priority, callbackURLs, description } = userInput; let roi = userInput.roi; const layer = await this.rasterCatalogManager.findLayer(userInput.dbId); const layerMetadata = layer.metadata; @@ -267,20 +268,20 @@ export class CreatePackageManager { crs: crs ?? DEFAULT_CRS, }; - const callbacks = callbackURLs.map((url) => { url, roi }); + const callbacks = callbackURLs ? callbackURLs.map((url) => { url, roi }) : undefined; const duplicationExist = await this.checkForExportDuplicate(dupParams, callbacks); if (duplicationExist && duplicationExist.status === OperationStatus.COMPLETED) { const callbackParam = duplicationExist as ICallbackExportResponse; this.logger.info({ jobStatus: callbackParam.status, - jobId: callbackParam.requestJobId, + jobId: callbackParam.jobId, catalogId: callbackParam.recordCatalogId, msg: `Found relevant cache for export request`, }); return duplicationExist; } else if (duplicationExist) { - const jobResponse = duplicationExist as ICreateJobResponse; - this.logger.info({ jobId: jobResponse.id, status: jobResponse.status, msg: `Found exists relevant In-Progress job for export request` }); + const jobResponse = duplicationExist as ICreateExportJobResponse; + this.logger.info({ jobId: jobResponse.jobId, status: jobResponse.status, msg: `Found exists relevant In-Progress job for export request` }); return duplicationExist; } @@ -358,6 +359,7 @@ export class CreatePackageManager { priority: priority ?? DEFAULT_PRIORITY, callbacks: callbacks, gpkgEstimatedSize: estimatesGpkgSize, + description, }; const jobCreated = await this.jobManagerClient.createExport(workerInput); return jobCreated; @@ -610,8 +612,8 @@ export class CreatePackageManager { private async checkForExportDuplicate( dupParams: JobExportDuplicationParams, - callbackUrls: ICallbackTargetExport[] - ): Promise { + callbackUrls: ICallbackTargetExport[] | undefined + ): Promise { let completedExists = await this.checkForExportCompleted(dupParams); if (completedExists) { return completedExists; @@ -675,16 +677,18 @@ export class CreatePackageManager { private async checkForExportProcessing( dupParams: JobExportDuplicationParams, - newCallbacks: ICallbackTargetExport[] - ): Promise { + newCallbacks: ICallbackTargetExport[] | undefined + ): Promise { this.logger.info({ ...dupParams, roi: undefined, msg: `Checking for PROCESSING duplications with parameters` }); const processingJob = (await this.jobManagerClient.findExportJob(OperationStatus.IN_PROGRESS, dupParams, true)) ?? (await this.jobManagerClient.findExportJob(OperationStatus.PENDING, dupParams, true)); if (processingJob) { - await this.updateExportCallbackURLs(processingJob, newCallbacks); + if (newCallbacks) { + await this.updateExportCallbackURLs(processingJob, newCallbacks); + } return { - id: processingJob.id, + jobId: processingJob.id, taskIds: (processingJob.tasks as unknown as IJobResponse[]).map((t) => t.id), status: OperationStatus.IN_PROGRESS, }; @@ -730,20 +734,24 @@ export class CreatePackageManager { } private async updateExportCallbackURLs(processingJob: JobExportResponse, newCallbacks: ICallbackTargetExport[]): Promise { - const callbacks = processingJob.parameters.callbacks; - for (const newCallback of newCallbacks) { - const hasCallback = callbacks.findIndex((callback) => { - const exist = callback.url === newCallback.url; - if (!exist) { - return false; - } + if (!processingJob.parameters.callbacks) { + processingJob.parameters.callbacks = newCallbacks; + } else { + const callbacks = processingJob.parameters.callbacks; + for (const newCallback of newCallbacks) { + const hasCallback = callbacks.findIndex((callback) => { + const exist = callback.url === newCallback.url; + if (!exist) { + return false; + } - const sameROI = featureCollectionBooleanEqual(callback.roi, newCallback.roi); - return sameROI; - }); - // eslint-disable-next-line @typescript-eslint/no-magic-numbers - if (hasCallback === -1) { - callbacks.push(newCallback); + const sameROI = featureCollectionBooleanEqual(callback.roi, newCallback.roi); + return sameROI; + }); + // eslint-disable-next-line @typescript-eslint/no-magic-numbers + if (hasCallback === -1) { + callbacks.push(newCallback); + } } } await this.jobManagerClient.updateJob(processingJob.id, { diff --git a/src/tasks/models/tasksManager.ts b/src/tasks/models/tasksManager.ts index 8eec8eb..1560962 100644 --- a/src/tasks/models/tasksManager.ts +++ b/src/tasks/models/tasksManager.ts @@ -153,19 +153,21 @@ export class TasksManager { try { this.logger.info({ jobId: job.id, callbacks: job.parameters.callbacks, msg: `Sending callback for job: ${job.id}` }); const targetCallbacks = job.parameters.callbacks; - const callbackPromises: Promise[] = []; - for (const target of targetCallbacks) { - const params: ICallbackExportData = { ...callbackParams, roi: job.parameters.roi }; - callbackPromises.push(this.callbackClient.send(target.url, params)); - } - - const promisesResponse = await Promise.allSettled(callbackPromises); - promisesResponse.forEach((response, index) => { - if (response.status === 'rejected') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - this.logger.error({ reason: response.reason, url: targetCallbacks[index].url, jobId: job.id, msg: `Failed to send callback to url` }); + if (targetCallbacks) { + const callbackPromises: Promise[] = []; + for (const target of targetCallbacks) { + const params: ICallbackExportData = { ...callbackParams, roi: job.parameters.roi }; + callbackPromises.push(this.callbackClient.send(target.url, params)); } - }); + + const promisesResponse = await Promise.allSettled(callbackPromises); + promisesResponse.forEach((response, index) => { + if (response.status === 'rejected') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + this.logger.error({ reason: response.reason, url: targetCallbacks[index].url, jobId: job.id, msg: `Failed to send callback to url` }); + } + }); + } } catch (error) { this.logger.error({ err: error, callbacksUrls: job.parameters.callbacks, jobId: job.id, msg: `Sending callback has failed` }); } @@ -338,8 +340,9 @@ export class TasksManager { expirationTime: expirationDate, fileSize, recordCatalogId: job.internalId as string, - requestJobId: job.id, + jobId: job.id, errorReason, + description: job.description, }; this.logger.info({ links: callbackParams.links, diff --git a/tests/integration/createPackage/createExportPackage.spec.ts b/tests/integration/createPackage/createExportPackage.spec.ts index dbf868f..c897714 100644 --- a/tests/integration/createPackage/createExportPackage.spec.ts +++ b/tests/integration/createPackage/createExportPackage.spec.ts @@ -4,7 +4,7 @@ import { feature, featureCollection, Geometry } from '@turf/turf'; import { getApp } from '../../../src/app'; import { RasterCatalogManagerClient } from '../../../src/clients/rasterCatalogManagerClient'; import { getContainerConfig, resetContainer } from '../testContainerConfig'; -import { ICreateJobResponse, ICreatePackageRoi, JobExportDuplicationParams } from '../../../src/common/interfaces'; +import { ICreateExportJobResponse, ICreatePackageRoi, JobExportDuplicationParams } from '../../../src/common/interfaces'; import { layerFromCatalog, fc1, fcNoMaxResolutionDeg, fcNoIntersection, fcTooHighResolution } from '../../mocks/data'; import { JobManagerWrapper } from '../../../src/clients/jobManagerWrapper'; import { CreatePackageManager } from '../../../src/createPackage/models/createPackageManager'; @@ -136,7 +136,7 @@ describe('Export by ROI', function () { }, status: OperationStatus.COMPLETED, fileSize: 10, - requestJobId: 'afbdd5e6-25db-4567-a81f-71e0e7d30761', + jobId: 'afbdd5e6-25db-4567-a81f-71e0e7d30761', expirationTime: expirationTime, recordCatalogId: layerFromCatalog.id, }; @@ -151,7 +151,7 @@ describe('Export by ROI', function () { }, status: OperationStatus.COMPLETED, fileSize: 10, - requestJobId: 'afbdd5e6-25db-4567-a81f-71e0e7d30761', + jobId: 'afbdd5e6-25db-4567-a81f-71e0e7d30761', expirationTime: expirationTime, recordCatalogId: layerFromCatalog.id, }; @@ -183,8 +183,8 @@ describe('Export by ROI', function () { priority: 0, }; - const inProgressJobResonse: ICreateJobResponse = { - id: 'b1c59730-c31d-4e44-9c67-4dbbb3b1c812', + const inProgressJobResonse: ICreateExportJobResponse = { + jobId: 'b1c59730-c31d-4e44-9c67-4dbbb3b1c812', taskIds: ['6556896a-113c-4397-a48b-0cb2c99658f5'], status: OperationStatus.IN_PROGRESS, }; diff --git a/tests/mocks/data.ts b/tests/mocks/data.ts index 7bff57e..3cee2ea 100644 --- a/tests/mocks/data.ts +++ b/tests/mocks/data.ts @@ -694,7 +694,7 @@ const completedExportJob: IJobResponse = }, status: OperationStatus.COMPLETED, fileSize: 1773568, - requestJobId: 'afbdd5e6-25db-4567-a81f-71e0e7d30761', + jobId: 'afbdd5e6-25db-4567-a81f-71e0e7d30761', expirationTime: new Date(), recordCatalogId: 'b0b19b88-aecb-4e74-b694-dfa7eada8bf7', }, diff --git a/tests/mocks/data/mockJob.ts b/tests/mocks/data/mockJob.ts index c0e5c9f..ce31ccb 100644 --- a/tests/mocks/data/mockJob.ts +++ b/tests/mocks/data/mockJob.ts @@ -61,7 +61,7 @@ export const mockCompletedJob: JobFinalizeResponse = { domain: 'testDomain', version: '1.0', type: 'rasterTilesExporter', - description: '', + description: 'test job', parameters: { crs: 'EPSG:4326', roi: fc1, diff --git a/tests/unit/clients/jobManagerClient.spec.ts b/tests/unit/clients/jobManagerClient.spec.ts index 8277295..344b93b 100644 --- a/tests/unit/clients/jobManagerClient.spec.ts +++ b/tests/unit/clients/jobManagerClient.spec.ts @@ -2,7 +2,7 @@ import jsLogger from '@map-colonies/js-logger'; import { IFindJobsRequest, OperationStatus } from '@map-colonies/mc-priority-queue'; import { getUTCDate } from '@map-colonies/mc-utils'; import { JobManagerWrapper } from '../../../src/clients/jobManagerWrapper'; -import { JobResponse, ICreateJobResponse as JobInProgressResponse, JobExportDuplicationParams } from '../../../src/common/interfaces'; +import { JobResponse, JobExportDuplicationParams, ICreateExportJobResponse } from '../../../src/common/interfaces'; import { configMock, registerDefaultConfig } from '../../mocks/config'; import { completedExportJob, @@ -176,8 +176,8 @@ describe('JobManagerClient', () => { describe('RoiExport', () => { describe('Export Job Creation', () => { it('should create Export job successfully', async () => { - const inProgressJobIds = { id: '123', taskIds: ['123'] }; - const expectedResponse: JobInProgressResponse = { + const inProgressJobIds = { jobId: '123', taskIds: ['123'] }; + const expectedResponse: ICreateExportJobResponse = { ...inProgressJobIds, status: OperationStatus.IN_PROGRESS, }; diff --git a/tests/unit/createPackage/models/createPackageModel.spec.ts b/tests/unit/createPackage/models/createPackageModel.spec.ts index 65aa943..1899c95 100644 --- a/tests/unit/createPackage/models/createPackageModel.spec.ts +++ b/tests/unit/createPackage/models/createPackageModel.spec.ts @@ -21,6 +21,7 @@ import { import { catalogManagerMock, findLayerMock } from '../../../mocks/clients/catalogManagerClient'; import { ExportVersion, + ICreateExportJobResponse, ICreateJobResponse, ICreatePackage, ICreatePackageRoi, @@ -586,7 +587,7 @@ describe('CreatePackageManager', () => { expect(findExportJobMock.mock.calls[3]).toEqual([OperationStatus.COMPLETED, jobDupParams]); expect(createExportMock).toHaveBeenCalledTimes(0); expect(res).toStrictEqual({ - id: inProgressExportJob.id, + jobId: inProgressExportJob.id, taskIds: ['1f765695-338b-4752-b182-a8cbae3c610e'], status: OperationStatus.IN_PROGRESS, }); @@ -625,7 +626,7 @@ describe('CreatePackageManager', () => { expect(findExportJobMock.mock.calls[2]).toEqual([OperationStatus.COMPLETED, jobDupParams]); expect(createExportMock).toHaveBeenCalledTimes(0); expect(res).toStrictEqual({ - id: inProgressExportJob.id, + jobId: inProgressExportJob.id, taskIds: ['1f765695-338b-4752-b182-a8cbae3c610e'], status: OperationStatus.IN_PROGRESS, }); @@ -672,8 +673,8 @@ describe('CreatePackageManager', () => { }, }; const res = await createPackageManager.createPackageRoi({ ...req, callbackURLs: ['http://new-added-callback-url.com'] }); - const expectedReturn: ICreateJobResponse = { - id: inProgressExportJob.id, + const expectedReturn: ICreateExportJobResponse = { + jobId: inProgressExportJob.id, taskIds: ['1f765695-338b-4752-b182-a8cbae3c610e'], status: OperationStatus.IN_PROGRESS, }; diff --git a/tests/unit/createPackage/models/tasksModel.spec.ts b/tests/unit/createPackage/models/tasksModel.spec.ts index c59ee09..8cb9eee 100644 --- a/tests/unit/createPackage/models/tasksModel.spec.ts +++ b/tests/unit/createPackage/models/tasksModel.spec.ts @@ -7,6 +7,7 @@ import { CreateFinalizeTaskBody, ICallbackDataBase, ICallbackDataExportBase, + ICallbackTargetExport, ITaskParameters, JobExportResponse, JobFinalizeResponse, @@ -387,17 +388,18 @@ describe('TasksManager', () => { metadataURI: 'http://download-service/downloads/test${sep}test.json', }, recordCatalogId: '880a9316-0f10-4874-92e2-a62d587a1169', - requestJobId: 'b729f0e0-af64-4c2c-ba4e-e799e2f3df0f', + jobId: 'b729f0e0-af64-4c2c-ba4e-e799e2f3df0f', expirationTime: expirationTime, fileSize: 2000, errorReason: undefined, }; - const actualCallBackUrls = mockCompletedJob.parameters.callbacks.map((callback) => callback.url); + const expectedCallbacksData = mockCompletedJob.parameters.callbacks as unknown as ICallbackTargetExport[]; + const actualCallBackUrls = expectedCallbacksData.map((callback) => callback.url); await tasksManager.sendExportCallbacks(mockCompletedJob, callbackData); expect(sendMock).toHaveBeenCalledTimes(2); - expect(sendMock.mock.calls).toHaveLength(mockCompletedJob.parameters.callbacks.length); + expect(sendMock.mock.calls).toHaveLength(expectedCallbacksData.length); const receviedCallbacks = sendMock.mock.calls; // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return const urlsArr = receviedCallbacks.map((call) => call[0]); @@ -413,7 +415,7 @@ describe('TasksManager', () => { metadataURI: 'http://download-service/downloads/test${sep}test.json', }, recordCatalogId: '880a9316-0f10-4874-92e2-a62d587a1169', - requestJobId: 'b729f0e0-af64-4c2c-ba4e-e799e2f3df0f', + jobId: 'b729f0e0-af64-4c2c-ba4e-e799e2f3df0f', expirationTime: expirationTime, fileSize: 2000, errorReason: undefined, @@ -489,8 +491,9 @@ describe('TasksManager', () => { metadataURI: `${downloadUrl}/downloads/${mockCompletedJob.parameters.relativeDirectoryPath}/${mockCompletedJob.parameters.fileNamesTemplates.metadataURI}`, }, recordCatalogId: mockCompletedJob.internalId as string, - requestJobId: mockCompletedJob.id, + jobId: mockCompletedJob.id, errorReason: undefined, + description: 'test job', }; const expectedUpdateRequest = { @@ -527,8 +530,9 @@ describe('TasksManager', () => { metadataURI: `${mockCompletedJob.parameters.fileNamesTemplates.metadataURI}`, }, recordCatalogId: mockCompletedJob.internalId as string, - requestJobId: mockCompletedJob.id, + jobId: mockCompletedJob.id, errorReason: 'Failed on metadata.json creation', + description: 'test job', }; const expectedUpdateRequest = { @@ -569,8 +573,9 @@ describe('TasksManager', () => { metadataURI: `${mockCompletedJob.parameters.fileNamesTemplates.metadataURI}`, }, recordCatalogId: mockCompletedJob.internalId as string, - requestJobId: mockCompletedJob.id, + jobId: mockCompletedJob.id, errorReason: exportingErrorMsg, + description: 'test job', }; const expectedUpdateRequest = { diff --git a/tests/unit/finalizationManager.spec.ts b/tests/unit/finalizationManager.spec.ts index 4ede23d..ad42e05 100644 --- a/tests/unit/finalizationManager.spec.ts +++ b/tests/unit/finalizationManager.spec.ts @@ -117,7 +117,7 @@ describe('FinalizationManager', () => { expirationTime: expirationDate, recordCatalogId: completedExportJob.internalId as string, fileSize: 1000, - requestJobId: completedExportJob.id, + jobId: completedExportJob.id, }, }, }; @@ -168,7 +168,7 @@ describe('FinalizationManager', () => { recordCatalogId: completedExportJob.internalId as string, fileSize: 0, errorReason: 'Failed on GPKG creation', - requestJobId: completedExportJob.id, + jobId: completedExportJob.id, }, }, }; @@ -277,7 +277,7 @@ describe('FinalizationManager', () => { expirationTime: expirationDate, recordCatalogId: completedExportJob.internalId as string, fileSize: 1000, - requestJobId: completedExportJob.id, + jobId: completedExportJob.id, }, }, }; From c7fab2fc2405e24a0606e041ea79fb4f85374804 Mon Sep 17 00:00:00 2001 From: ronenk1 Date: Sun, 26 Mar 2023 16:39:25 +0300 Subject: [PATCH 2/4] fix: adding description to naive cache response spec --- openapi3.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openapi3.yaml b/openapi3.yaml index 8ae558e..6dd2125 100644 --- a/openapi3.yaml +++ b/openapi3.yaml @@ -380,6 +380,8 @@ components: recordCatalogId: type: string format: uuid + description: + type: string roi: $ref: '#/components/schemas/FeatureCollection' jobId: From 44e0b12b2050ead8b5ed80fe83f8fde638bede29 Mon Sep 17 00:00:00 2001 From: ronenk1 Date: Sun, 26 Mar 2023 17:18:34 +0300 Subject: [PATCH 3/4] fix: pr notes --- src/createPackage/models/createPackageManager.ts | 14 ++++++++------ src/tasks/models/tasksManager.ts | 4 +++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/createPackage/models/createPackageManager.ts b/src/createPackage/models/createPackageManager.ts index 47daddb..ba08c7c 100644 --- a/src/createPackage/models/createPackageManager.ts +++ b/src/createPackage/models/createPackageManager.ts @@ -612,7 +612,7 @@ export class CreatePackageManager { private async checkForExportDuplicate( dupParams: JobExportDuplicationParams, - callbackUrls: ICallbackTargetExport[] | undefined + callbackUrls?: ICallbackTargetExport[] ): Promise { let completedExists = await this.checkForExportCompleted(dupParams); if (completedExists) { @@ -677,16 +677,14 @@ export class CreatePackageManager { private async checkForExportProcessing( dupParams: JobExportDuplicationParams, - newCallbacks: ICallbackTargetExport[] | undefined + newCallbacks?: ICallbackTargetExport[] ): Promise { this.logger.info({ ...dupParams, roi: undefined, msg: `Checking for PROCESSING duplications with parameters` }); const processingJob = (await this.jobManagerClient.findExportJob(OperationStatus.IN_PROGRESS, dupParams, true)) ?? (await this.jobManagerClient.findExportJob(OperationStatus.PENDING, dupParams, true)); if (processingJob) { - if (newCallbacks) { - await this.updateExportCallbackURLs(processingJob, newCallbacks); - } + await this.updateExportCallbackURLs(processingJob, newCallbacks); return { jobId: processingJob.id, taskIds: (processingJob.tasks as unknown as IJobResponse[]).map((t) => t.id), @@ -733,7 +731,11 @@ export class CreatePackageManager { }); } - private async updateExportCallbackURLs(processingJob: JobExportResponse, newCallbacks: ICallbackTargetExport[]): Promise { + private async updateExportCallbackURLs(processingJob: JobExportResponse, newCallbacks?: ICallbackTargetExport[]): Promise { + if (!newCallbacks) { + return; + } + if (!processingJob.parameters.callbacks) { processingJob.parameters.callbacks = newCallbacks; } else { diff --git a/src/tasks/models/tasksManager.ts b/src/tasks/models/tasksManager.ts index 1560962..7a27e45 100644 --- a/src/tasks/models/tasksManager.ts +++ b/src/tasks/models/tasksManager.ts @@ -153,7 +153,9 @@ export class TasksManager { try { this.logger.info({ jobId: job.id, callbacks: job.parameters.callbacks, msg: `Sending callback for job: ${job.id}` }); const targetCallbacks = job.parameters.callbacks; - if (targetCallbacks) { + if (!targetCallbacks) { + return; + } else { const callbackPromises: Promise[] = []; for (const target of targetCallbacks) { const params: ICallbackExportData = { ...callbackParams, roi: job.parameters.roi }; From dc074066d30f108bde59f8f46c0b89567c3d66a0 Mon Sep 17 00:00:00 2001 From: ronenk1 Date: Sun, 26 Mar 2023 17:59:03 +0300 Subject: [PATCH 4/4] fix: pr notes #2 --- src/tasks/models/tasksManager.ts | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/tasks/models/tasksManager.ts b/src/tasks/models/tasksManager.ts index 7a27e45..53dd4b6 100644 --- a/src/tasks/models/tasksManager.ts +++ b/src/tasks/models/tasksManager.ts @@ -155,21 +155,20 @@ export class TasksManager { const targetCallbacks = job.parameters.callbacks; if (!targetCallbacks) { return; - } else { - const callbackPromises: Promise[] = []; - for (const target of targetCallbacks) { - const params: ICallbackExportData = { ...callbackParams, roi: job.parameters.roi }; - callbackPromises.push(this.callbackClient.send(target.url, params)); - } - - const promisesResponse = await Promise.allSettled(callbackPromises); - promisesResponse.forEach((response, index) => { - if (response.status === 'rejected') { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - this.logger.error({ reason: response.reason, url: targetCallbacks[index].url, jobId: job.id, msg: `Failed to send callback to url` }); - } - }); } + const callbackPromises: Promise[] = []; + for (const target of targetCallbacks) { + const params: ICallbackExportData = { ...callbackParams, roi: job.parameters.roi }; + callbackPromises.push(this.callbackClient.send(target.url, params)); + } + + const promisesResponse = await Promise.allSettled(callbackPromises); + promisesResponse.forEach((response, index) => { + if (response.status === 'rejected') { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + this.logger.error({ reason: response.reason, url: targetCallbacks[index].url, jobId: job.id, msg: `Failed to send callback to url` }); + } + }); } catch (error) { this.logger.error({ err: error, callbacksUrls: job.parameters.callbacks, jobId: job.id, msg: `Sending callback has failed` }); }