From 7afd7ba60a316e5893c1c86d85c9dafe670f187e Mon Sep 17 00:00:00 2001 From: liran Date: Mon, 23 Feb 2026 11:09:04 +0200 Subject: [PATCH 1/5] feat: added validation against the extractable service --- config/custom-environment-variables.json | 9 ++ config/default.json | 6 ++ helm/templates/configmap.yaml | 2 + .../extractable-management/extractableCall.ts | 62 +++++++++++ .../extractable-management/interfaces.ts | 10 ++ src/metadata/models/metadataManager.ts | 2 + src/validator/validationManager.ts | 12 +++ tests/helpers/mockCreator.ts | 4 + .../metadata/metadataController.spec.ts | 102 ++++++++++++++++-- .../extractable/requestCall.spec.ts | 69 ++++++++++++ .../unit/validator/validationManager.spec.ts | 24 ++++- 11 files changed, 295 insertions(+), 7 deletions(-) create mode 100644 src/externalServices/extractable-management/extractableCall.ts create mode 100644 src/externalServices/extractable-management/interfaces.ts create mode 100644 tests/unit/externalServices/extractable/requestCall.spec.ts diff --git a/config/custom-environment-variables.json b/config/custom-environment-variables.json index 441a17e..27fe5b8 100644 --- a/config/custom-environment-variables.json +++ b/config/custom-environment-variables.json @@ -45,6 +45,7 @@ "externalServices": { "storeTrigger": "STORE_TRIGGER_URL", "catalog": "CATALOG_URL", + "extractable": "EXTRACTABLE_URL", "lookupTables": { "url": "LOOKUP_TABLES_URL", "subUrl": "LOOKUP_TABLES_SUB_URL" @@ -76,5 +77,13 @@ "__name": "S3_DEST_MAX_ATTEMPTS", "__format": "number" } + }, + "enableServices": { + "extractable": { + "extractableManagement": { + "__name": "ENABLE_EXTRACTABLE_MANAGEMENT", + "__format": "boolean" + } + } } } diff --git a/config/default.json b/config/default.json index c40b1bf..18d07d1 100644 --- a/config/default.json +++ b/config/default.json @@ -36,6 +36,7 @@ "externalServices": { "storeTrigger": "http://127.0.0.1:8080", "catalog": "http://127.0.0.1:8080", + "extractable": "http://127.0.0.1:8080", "lookupTables": { "url": "http://127.0.0.1:8080", "subUrl": "lookup-tables/lookupData" @@ -58,5 +59,10 @@ "forcePathStyle": true, "sslEnabled": false, "maxAttempts": 3 + }, + "enableServices": { + "extractable": { + "extractableManagement": true + } } } diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index 3e5e715..6df47a5 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -29,6 +29,8 @@ data: SERVER_PORT: {{ .Values.env.targetPort | quote }} STORE_TRIGGER_URL: {{ .Values.env.storeTrigger.url | default (printf "http://%s-store-trigger" .Release.Name) }} CATALOG_URL: {{ .Values.env.catalog.url | default (printf "http://%s-catalog" .Release.Name) }} + EXTRACTABLE_URL: {{ .Values.env.extractable.url | default (printf "http://%s-extractable" .Release.Name) }} + ENABLE_EXTRACTABLE_MANAGEMENT: {{ .Values.env.enableServices.extractable.extractableManagement | quote }} LOOKUP_TABLES_URL: {{ .Values.validations.lookupTables.url | quote }} LOOKUP_TABLES_SUB_URL: {{ .Values.validations.lookupTables.subUrl | quote }} {{ if eq $providers.source "NFS" }} diff --git a/src/externalServices/extractable-management/extractableCall.ts b/src/externalServices/extractable-management/extractableCall.ts new file mode 100644 index 0000000..1f9a8e5 --- /dev/null +++ b/src/externalServices/extractable-management/extractableCall.ts @@ -0,0 +1,62 @@ +import axios from 'axios'; +import { inject, injectable } from 'tsyringe'; +import { Logger } from '@map-colonies/js-logger'; +import { StatusCodes } from 'http-status-codes'; +import { Tracer } from '@opentelemetry/api'; +import { withSpanAsyncV4 } from '@map-colonies/telemetry'; +import { SERVICES } from '../../common/constants'; +import { AppError } from '../../common/appError'; +import { IConfig, LogContext } from '../../common/interfaces'; + +@injectable() +export class ExtractableCall { + private readonly logContext: LogContext; + private readonly extractable: string; + + public constructor( + @inject(SERVICES.CONFIG) private readonly config: IConfig, + @inject(SERVICES.LOGGER) private readonly logger: Logger, + @inject(SERVICES.TRACER) public readonly tracer: Tracer + ) { + this.extractable = this.config.get('externalServices.extractable'); + this.logContext = { + fileName: __filename, + class: ExtractableCall.name, + }; + } + + @withSpanAsyncV4 + public async isExtractableRecordExists(recordName: string): Promise { + const logContext = { ...this.logContext, function: this.isExtractableRecordExists.name }; + this.logger.debug({ msg: `Checking record '${recordName}' in extractable service`, logContext }); + + try { + const response = await axios.get(`${this.extractable}/records/${recordName}`, + { + validateStatus: () => true + }); + + if (response.status === StatusCodes.OK) { + this.logger.debug({ msg: `Record '${recordName}' exists in extractable`, logContext }); + return true; + } + + if (response.status === StatusCodes.NOT_FOUND) { + this.logger.debug({ msg: `Record '${recordName}' does not exist in extractable`, logContext }); + return false; + } + + this.logger.error({ msg: `Unexpected status from extractable: ${response.status}`, logContext, status: response.status }); + + throw new AppError('extractable', StatusCodes.INTERNAL_SERVER_ERROR, 'Unexpected response from extractable service', true); + } catch (error) { + this.logger.error({ msg: 'Error occurred during isExtractableRecordExists call', recordName, logContext, err: error }); + + if (error instanceof AppError) { + throw error; + } + + throw new AppError('extractable', StatusCodes.INTERNAL_SERVER_ERROR, 'Failed to query extractable service', true); + } + } +} diff --git a/src/externalServices/extractable-management/interfaces.ts b/src/externalServices/extractable-management/interfaces.ts new file mode 100644 index 0000000..46ff138 --- /dev/null +++ b/src/externalServices/extractable-management/interfaces.ts @@ -0,0 +1,10 @@ +import { Layer3DMetadata } from '@map-colonies/mc-model-types'; + +export interface MetadataParams { + identifier: string; +} + +export interface Record3D extends Layer3DMetadata { + id: string; + links: string; +} diff --git a/src/metadata/models/metadataManager.ts b/src/metadata/models/metadataManager.ts index 44bda75..a0fd71e 100644 --- a/src/metadata/models/metadataManager.ts +++ b/src/metadata/models/metadataManager.ts @@ -112,6 +112,8 @@ export class MetadataManager { } else if (record3D.productStatus == RecordStatus.BEING_DELETED) { throw new AppError('badRequest', StatusCodes.BAD_REQUEST, `Can't change status of record that is being deleted`, true); } + + // TODO: add validation on the extractable this.logger.info({ msg: 'model validated successfully', logContext, diff --git a/src/validator/validationManager.ts b/src/validator/validationManager.ts index 2ea1829..54e4e51 100644 --- a/src/validator/validationManager.ts +++ b/src/validator/validationManager.ts @@ -16,6 +16,7 @@ import { CatalogCall } from '../externalServices/catalog/catalogCall'; import { convertSphereFromXYZToWGS84, convertRegionFromRadianToDegrees } from './calculatePolygonFromTileset'; import { BoundingRegion, BoundingSphere, TileSetJson } from './interfaces'; import { extractLink } from './extractPathFromLink'; +import { ExtractableCall } from '../externalServices/extractable-management/extractableCall'; export const ERROR_METADATA_DATE = 'sourceStartDate should not be later than sourceEndDate'; export const ERROR_METADATA_RESOLUTION = 'minResolutionMeter should not be bigger than maxResolutionMeter'; @@ -25,6 +26,7 @@ export const ERROR_METADATA_BOX_TILESET = `BoundingVolume of box is not supporte export const ERROR_METADATA_BAD_FORMAT_TILESET = 'Bad tileset format. Should be in 3DTiles format'; export const ERROR_METADATA_ERRORED_TILESET = `File tileset validation failed`; export const ERROR_METADATA_FOOTPRINT_FAR_FROM_MODEL = `Wrong footprint! footprint's coordinates is not even close to the model!`; +export const ERROR_METADATA_PRODUCT_NAME_CONFLICT = `record with this product name exists in extractable service`; export interface FailedReason { outFailedReason: string; @@ -41,6 +43,7 @@ export class ValidationManager { @inject(SERVICES.TRACER) public readonly tracer: Tracer, @inject(LookupTablesCall) private readonly lookupTables: LookupTablesCall, @inject(CatalogCall) private readonly catalog: CatalogCall, + @inject(ExtractableCall) private readonly extractable: ExtractableCall, @inject(SERVICES.PROVIDER) private readonly provider: Provider ) { this.logContext = { @@ -152,6 +155,15 @@ export class ValidationManager { return false; } + const isExtractableManagementEnabled = this.config.get('enableServices.extractable.extractableManagement'); + if (isExtractableManagementEnabled) { + const existsInExtractable = await this.extractable.isExtractableRecordExists(record.producerName!); + if (existsInExtractable) { + refReason.outFailedReason = ERROR_METADATA_PRODUCT_NAME_CONFLICT; + return false; + } + } + if (record.productStatus == RecordStatus.BEING_DELETED) { refReason.outFailedReason = `Can't update record that is being deleted`; return false; diff --git a/tests/helpers/mockCreator.ts b/tests/helpers/mockCreator.ts index c3192d3..cbf5e3e 100644 --- a/tests/helpers/mockCreator.ts +++ b/tests/helpers/mockCreator.ts @@ -52,3 +52,7 @@ export const catalogMock = { patchMetadata: jest.fn(), changeStatus: jest.fn(), }; + +export const extractableMock = { + isExtractableRecordExists: jest.fn(), +} diff --git a/tests/integration/metadata/metadataController.spec.ts b/tests/integration/metadata/metadataController.spec.ts index d0fec96..8cca3a8 100644 --- a/tests/integration/metadata/metadataController.spec.ts +++ b/tests/integration/metadata/metadataController.spec.ts @@ -21,7 +21,7 @@ import { S3Helper } from '../../helpers/s3Helper'; import { S3Config } from '../../../src/common/interfaces'; import { extractLink } from '../../../src/validator/extractPathFromLink'; import { CatalogCall } from '../../../src/externalServices/catalog/catalogCall'; -import { ERROR_METADATA_PRODUCT_NAME_UNIQUE } from '../../../src/validator/validationManager'; +import { ERROR_METADATA_PRODUCT_NAME_CONFLICT, ERROR_METADATA_PRODUCT_NAME_UNIQUE } from '../../../src/validator/validationManager'; import { MetadataRequestSender } from './helpers/requestSender'; describe('MetadataController', function () { @@ -62,6 +62,7 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); @@ -81,6 +82,7 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [record] }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); @@ -101,6 +103,7 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); @@ -149,6 +152,7 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: classification }] as ILookupOption[] }); const response = await requestSender.updateMetadata(identifier, payload); @@ -162,7 +166,9 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); payload.footprint = createWrongFootprintSchema(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + const record = createRecord(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); @@ -178,7 +184,9 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); payload.footprint = createWrongFootprintCoordinates(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + const record = createRecord(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); @@ -194,7 +202,9 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); payload.footprint = createWrongFootprintMixed2D3D(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + const record = createRecord(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); @@ -208,7 +218,9 @@ describe('MetadataController', function () { const payload = createUpdatePayload(); payload.sourceDateEnd = faker.date.past(); payload.sourceDateStart = faker.date.soon(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + const record = createRecord(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); @@ -229,6 +241,22 @@ describe('MetadataController', function () { expect(response).toSatisfyApiSpec(); }); + it(`Should return 400 status code if producer name conflicts with extractable`, async function () { + const identifier = faker.string.uuid(); + const payload = createUpdatePayload(); + const record = createRecord(); + const linkUrl = extractLink(record.links); + await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK }); + + const response = await requestSender.updateMetadata(identifier, payload); + + expect(response.status).toBe(StatusCodes.BAD_REQUEST); + expect(response.body).toHaveProperty('message', ERROR_METADATA_PRODUCT_NAME_CONFLICT); + expect(response).toSatisfyApiSpec(); + }); + it(`Should return 400 status code if record product name already exists in catalog`, async function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); @@ -237,6 +265,7 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const clonedRecordWithSameNameAsPayload = { ...record, productName: payload.productName }; mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [clonedRecordWithSameNameAsPayload] }); @@ -260,8 +289,9 @@ describe('MetadataController', function () { await s3Helper.createFile(linkUrl, true); record.productStatus = RecordStatus.BEING_DELETED; mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const response = await requestSender.updateMetadata(identifier, payload); @@ -280,6 +310,7 @@ describe('MetadataController', function () { mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockRejectedValueOnce(new Error('lookup-tables error')); const response = await requestSender.updateMetadata(identifier, payload); @@ -306,6 +337,7 @@ describe('MetadataController', function () { const payload = createUpdatePayload(); const record = createRecord(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); @@ -324,6 +356,7 @@ describe('MetadataController', function () { const payload = createUpdatePayload(); const record = createRecord(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); @@ -343,6 +376,7 @@ describe('MetadataController', function () { const record = createRecord(); record.links = faker.word.sample(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); @@ -350,6 +384,62 @@ describe('MetadataController', function () { expect(response.body).toHaveProperty('message', `Link extraction failed`); expect(response).toSatisfyApiSpec(); }); + + it(`Should return 500 status code if extractable returns unexpected status`, async function () { + const identifier = faker.string.uuid(); + const payload = createUpdatePayload(); + const record = createRecord(); + const linkUrl = extractLink(record.links); + await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.INTERNAL_SERVER_ERROR }); + + const response = await requestSender.updateMetadata(identifier, payload); + + expect(response.status).toBe(StatusCodes.INTERNAL_SERVER_ERROR); + expect(response.body).toHaveProperty('message', 'Unexpected response from extractable service'); + expect(response).toSatisfyApiSpec(); + }); + + it(`Should return 500 status code if extractable service is not available`, async function () { + const identifier = faker.string.uuid(); + const payload = createUpdatePayload(); + const record = createRecord(); + const linkUrl = extractLink(record.links); + await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockRejectedValueOnce(new Error('extractable is not available')); + + const response = await requestSender.updateMetadata(identifier, payload); + + expect(response.status).toBe(StatusCodes.INTERNAL_SERVER_ERROR); + expect(response.body).toHaveProperty('message', 'Failed to query extractable service'); + expect(response).toSatisfyApiSpec(); + }); + + it(`Should call extractable with validateStatus always true`, async function () { + const identifier = faker.string.uuid(); + const payload = createUpdatePayload(); + const record = createRecord(); + const linkUrl = extractLink(record.links); + await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + + await requestSender.updateMetadata(identifier, payload); + + const extractableCall = mockAxios.get.mock.calls.find(call => call[0]!.includes('/records/')); + expect(extractableCall).toBeDefined(); + const options = extractableCall![1]; + expect(options.validateStatus).toBeDefined(); + const validateStatus = options.validateStatus; + expect(validateStatus(200)).toBe(true); + expect(validateStatus(404)).toBe(true); + expect(validateStatus(500)).toBe(true); + }); }); }); diff --git a/tests/unit/externalServices/extractable/requestCall.spec.ts b/tests/unit/externalServices/extractable/requestCall.spec.ts new file mode 100644 index 0000000..eb99414 --- /dev/null +++ b/tests/unit/externalServices/extractable/requestCall.spec.ts @@ -0,0 +1,69 @@ +import mockAxios from 'jest-mock-axios'; +import config from 'config'; +import jsLogger from '@map-colonies/js-logger'; +import { faker } from '@faker-js/faker'; +import { StatusCodes } from 'http-status-codes'; +import { trace } from '@opentelemetry/api'; +import { ExtractableCall } from '../../../../src/externalServices/extractable-management/extractableCall'; +let extractable: ExtractableCall; +describe('extractableCall tests', () => { + const extractableUrl = config.get('externalServices.extractable'); + beforeEach(() => { + extractable = new ExtractableCall(config, jsLogger({ enabled: false }), trace.getTracer('testTracer')); + }); + afterEach(() => { + jest.clearAllMocks(); + }); + describe('isExtractableRecordExists Function', () => { + it('Returns true when recordName exists in DB', async () => { + const recordName = faker.string.uuid(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK }); + + const response = await extractable.isExtractableRecordExists(recordName); + + expect(mockAxios.get).toHaveBeenCalledWith(`${extractableUrl}/records/${recordName}`, { validateStatus: expect.any(Function) }); + expect(response).toBe(true); + }); + it('Returns false when recordName does not exist in DB', async () => { + const recordName = faker.string.uuid(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); + + const response = await extractable.isExtractableRecordExists(recordName); + + expect(mockAxios.get).toHaveBeenCalledWith(`${extractableUrl}/records/${recordName}`, { validateStatus: expect.any(Function) }); + expect(response).toBe(false); + }); + it('Rejects if got unexpected response from extractable', async () => { + const recordName = faker.string.uuid(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.INTERNAL_SERVER_ERROR }); + + const response = extractable.isExtractableRecordExists(recordName); + + expect(mockAxios.get).toHaveBeenCalledWith(`${extractableUrl}/records/${recordName}`, { validateStatus: expect.any(Function) }); + await expect(response).rejects.toThrow('Unexpected response from extractable service'); + }); + it('rejects if service is not available', async () => { + const recordName = faker.string.uuid(); + mockAxios.get.mockRejectedValueOnce(new Error('extractable is not available')); + + const response = extractable.isExtractableRecordExists(recordName); + + await expect(response).rejects.toThrow('Failed to query extractable service'); + }); + it('uses validateStatus that always returns true', async () => { + const recordName = faker.string.uuid(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK }); + + await extractable.isExtractableRecordExists(recordName); + + const [, options] = mockAxios.get.mock.calls[0]; + const validateStatus = options.validateStatus; + + expect(typeof validateStatus).toBe('function'); + expect(validateStatus(200)).toBe(true); + expect(validateStatus(404)).toBe(true); + expect(validateStatus(500)).toBe(true); + expect(validateStatus(0)).toBe(true); + }); + }); +}); \ No newline at end of file diff --git a/tests/unit/validator/validationManager.spec.ts b/tests/unit/validator/validationManager.spec.ts index b51d54b..e3a3def 100644 --- a/tests/unit/validator/validationManager.spec.ts +++ b/tests/unit/validator/validationManager.spec.ts @@ -13,6 +13,7 @@ import { ERROR_METADATA_DATE, ERROR_METADATA_ERRORED_TILESET, ERROR_METADATA_FOOTPRINT_FAR_FROM_MODEL, + ERROR_METADATA_PRODUCT_NAME_CONFLICT, ERROR_METADATA_PRODUCT_NAME_UNIQUE, ERROR_METADATA_RESOLUTION, FailedReason, @@ -31,7 +32,7 @@ import { getBasePath, createWrongFootprintMixed2D3D, } from '../../helpers/helpers'; -import { configMock, lookupTablesMock, catalogMock, providerMock } from '../../helpers/mockCreator'; +import { configMock, lookupTablesMock, catalogMock, extractableMock, providerMock } from '../../helpers/mockCreator'; import { AppError } from '../../../src/common/appError'; import { FILE_ENCODING } from '../../../src/common/constants'; @@ -45,6 +46,7 @@ describe('ValidationManager', () => { trace.getTracer('testTracer'), lookupTablesMock as never, catalogMock as never, + extractableMock as never, providerMock as never ); }); @@ -207,6 +209,7 @@ describe('ValidationManager', () => { trace.getTracer('testTracer'), lookupTablesMock as never, catalogMock as never, + extractableMock as never, providerMock ); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint('WrongVolume')); @@ -473,6 +476,25 @@ describe('ValidationManager', () => { expect(response).toBe(false); expect(refReason.outFailedReason).toBe(ERROR_METADATA_PRODUCT_NAME_UNIQUE); }); + + it('returns false when extractable management is enabled and record exists in extractable', async () => { + configMock.get.mockReturnValue(true); + const identifier = faker.string.uuid(); + const payload = createUpdatePayload(); + const record = createRecord(); + catalogMock.findRecords.mockResolvedValue([]); + catalogMock.getRecord.mockResolvedValue(record); + lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); + providerMock.getFile.mockResolvedValue(getTileset()); + extractableMock.isExtractableRecordExists.mockResolvedValue(true); + + const refReason: FailedReason = { outFailedReason: '' }; + const response = await validationManager.validateUpdate(identifier, payload, refReason); + + expect(extractableMock.isExtractableRecordExists).toHaveBeenCalledWith(record.producerName); + expect(response).toBe(false); + expect(refReason.outFailedReason).toBe(ERROR_METADATA_PRODUCT_NAME_CONFLICT); + }); }); describe('isPolygonValid', () => { From a2e642ab527049171a9b823a3a4faf07a4ef16c9 Mon Sep 17 00:00:00 2001 From: liran Date: Mon, 23 Feb 2026 17:34:50 +0200 Subject: [PATCH 2/5] fix: finish updateStatus and asaf changes --- bundledApi.yaml | 25 ++-- openapi3.yaml | 12 ++ .../extractable-management/extractableCall.ts | 7 +- .../extractable-management/interfaces.ts | 10 -- src/metadata/models/metadataManager.ts | 16 ++- src/validator/validationManager.ts | 43 ++++-- tests/helpers/mockCreator.ts | 3 +- .../metadata/metadataController.spec.ts | 128 +++++++++++++++--- .../extractable/requestCall.spec.ts | 13 +- .../metadata/models/metadataManager.spec.ts | 58 ++++++++ .../unit/validator/validationManager.spec.ts | 110 ++++++++++++++- 11 files changed, 367 insertions(+), 58 deletions(-) delete mode 100644 src/externalServices/extractable-management/interfaces.ts diff --git a/bundledApi.yaml b/bundledApi.yaml index f2e89ef..22eada7 100644 --- a/bundledApi.yaml +++ b/bundledApi.yaml @@ -166,6 +166,12 @@ paths: application/json: schema: $ref: '#/components/schemas/error' + '409': + description: Conflict + content: + application/json: + schema: + $ref: '#/components/schemas/error' '500': description: Unexpected Error content: @@ -198,6 +204,12 @@ paths: application/json: schema: $ref: '#/components/schemas/error' + '409': + description: Conflict + content: + application/json: + schema: + $ref: '#/components/schemas/error' '500': description: Unexpected Error content: @@ -242,9 +254,7 @@ components: additionalProperties: false payloadForValidation: type: object - description: >- - 3d model payload, if metadta is not provided will validate only the - sources + description: 3d model payload, if metadta is not provided will validate only the sources required: - modelPath - tilesetFilename @@ -637,8 +647,7 @@ components: allOf: - $ref: '#/components/schemas/Geometry' - description: Geographic demarcation of the product - - example: >- - {"type":"Polygon","coordinates":[[[1,2],[3,4],[5,6],[7,8],[1,2]]]} + - example: '{"type":"Polygon","coordinates":[[[1,2],[3,4],[5,6],[7,8],[1,2]]]}' heightRangeFrom: type: number format: double @@ -731,8 +740,7 @@ components: allOf: - $ref: '#/components/schemas/Geometry' - description: Geographic demarcation of the product - - example: >- - {"type":"Polygon","coordinates":[[[1,2],[3,4],[5,6],[7,8],[1,2]]]} + - example: '{"type":"Polygon","coordinates":[[[1,2],[3,4],[5,6],[7,8],[1,2]]]}' minResolutionMeter: type: number format: double @@ -964,8 +972,7 @@ components: allOf: - $ref: '#/components/schemas/Geometry' - description: Geographic demarcation of the product - - example: >- - {"type":"Polygon","coordinates":[[[1,2],[3,4],[5,6],[7,8],[1,2]]]} + - example: '{"type":"Polygon","coordinates":[[[1,2],[3,4],[5,6],[7,8],[1,2]]]}' heightRangeFrom: type: number format: double diff --git a/openapi3.yaml b/openapi3.yaml index 734bd02..2ae7dc9 100644 --- a/openapi3.yaml +++ b/openapi3.yaml @@ -167,6 +167,12 @@ paths: application/json: schema: $ref: '#/components/schemas/error' + '409': + description: Conflict + content: + application/json: + schema: + $ref: '#/components/schemas/error' '500': description: Unexpected Error content: @@ -200,6 +206,12 @@ paths: application/json: schema: $ref: '#/components/schemas/error' + '409': + description: Conflict + content: + application/json: + schema: + $ref: '#/components/schemas/error' '500': description: Unexpected Error content: diff --git a/src/externalServices/extractable-management/extractableCall.ts b/src/externalServices/extractable-management/extractableCall.ts index 1f9a8e5..f1e3fde 100644 --- a/src/externalServices/extractable-management/extractableCall.ts +++ b/src/externalServices/extractable-management/extractableCall.ts @@ -31,12 +31,11 @@ export class ExtractableCall { this.logger.debug({ msg: `Checking record '${recordName}' in extractable service`, logContext }); try { - const response = await axios.get(`${this.extractable}/records/${recordName}`, - { - validateStatus: () => true + const response = await axios.get(`${this.extractable}/records/${recordName}`, { + validateStatus: () => true, }); - if (response.status === StatusCodes.OK) { + if (response.status === StatusCodes.OK) { this.logger.debug({ msg: `Record '${recordName}' exists in extractable`, logContext }); return true; } diff --git a/src/externalServices/extractable-management/interfaces.ts b/src/externalServices/extractable-management/interfaces.ts deleted file mode 100644 index 46ff138..0000000 --- a/src/externalServices/extractable-management/interfaces.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { Layer3DMetadata } from '@map-colonies/mc-model-types'; - -export interface MetadataParams { - identifier: string; -} - -export interface Record3D extends Layer3DMetadata { - id: string; - links: string; -} diff --git a/src/metadata/models/metadataManager.ts b/src/metadata/models/metadataManager.ts index a0fd71e..39e6246 100644 --- a/src/metadata/models/metadataManager.ts +++ b/src/metadata/models/metadataManager.ts @@ -51,10 +51,17 @@ export class MetadataManager { try { const refReason: FailedReason = { outFailedReason: '' }; - const isValid: boolean = await this.validator.validateUpdate(identifier, payload, refReason); + const result = await this.validator.validateUpdate(identifier, payload, refReason); + const isExtractableConflict = Array.isArray(result) && result[1] === true; + const isValid = result === true; + + if (!isValid && isExtractableConflict) { + throw new AppError('conflict', StatusCodes.CONFLICT, refReason.outFailedReason, true); + } if (!isValid) { throw new AppError('badRequest', StatusCodes.BAD_REQUEST, refReason.outFailedReason, true); } + this.logger.info({ msg: 'model validated successfully', logContext, @@ -113,7 +120,12 @@ export class MetadataManager { throw new AppError('badRequest', StatusCodes.BAD_REQUEST, `Can't change status of record that is being deleted`, true); } - // TODO: add validation on the extractable + const refReason: FailedReason = { outFailedReason: '' }; + const isValid: boolean = await this.validator.validateUpdateStatus(record3D, refReason); + if (!isValid) { + throw new AppError('conflict', StatusCodes.CONFLICT, refReason.outFailedReason, true); + } + this.logger.info({ msg: 'model validated successfully', logContext, diff --git a/src/validator/validationManager.ts b/src/validator/validationManager.ts index 54e4e51..78ab26b 100644 --- a/src/validator/validationManager.ts +++ b/src/validator/validationManager.ts @@ -17,6 +17,7 @@ import { convertSphereFromXYZToWGS84, convertRegionFromRadianToDegrees } from '. import { BoundingRegion, BoundingSphere, TileSetJson } from './interfaces'; import { extractLink } from './extractPathFromLink'; import { ExtractableCall } from '../externalServices/extractable-management/extractableCall'; +import { Record3D } from '../externalServices/catalog/interfaces'; export const ERROR_METADATA_DATE = 'sourceStartDate should not be later than sourceEndDate'; export const ERROR_METADATA_RESOLUTION = 'minResolutionMeter should not be bigger than maxResolutionMeter'; @@ -26,7 +27,7 @@ export const ERROR_METADATA_BOX_TILESET = `BoundingVolume of box is not supporte export const ERROR_METADATA_BAD_FORMAT_TILESET = 'Bad tileset format. Should be in 3DTiles format'; export const ERROR_METADATA_ERRORED_TILESET = `File tileset validation failed`; export const ERROR_METADATA_FOOTPRINT_FAR_FROM_MODEL = `Wrong footprint! footprint's coordinates is not even close to the model!`; -export const ERROR_METADATA_PRODUCT_NAME_CONFLICT = `record with this product name exists in extractable service`; +export const ERROR_METADATA_PRODUCT_NAME_CONFLICT = `An external service locks this record`; export interface FailedReason { outFailedReason: string; @@ -36,6 +37,7 @@ export interface FailedReason { export class ValidationManager { private readonly limit: number; private readonly logContext: LogContext; + private readonly isExtractableManagementEnabled: boolean; public constructor( @inject(SERVICES.CONFIG) private readonly config: IConfig, @@ -51,6 +53,8 @@ export class ValidationManager { class: ValidationManager.name, }; this.limit = this.config.get('validation.percentageLimit'); + + this.isExtractableManagementEnabled = this.config.get('enableServices.extractable.extractableManagement'); } @withSpanAsyncV4 @@ -147,7 +151,7 @@ export class ValidationManager { } @withSpanAsyncV4 - public async validateUpdate(identifier: string, payload: UpdatePayload, refReason: FailedReason): Promise { + public async validateUpdate(identifier: string, payload: UpdatePayload, refReason: FailedReason): Promise { const record = await this.catalog.getRecord(identifier); if (record === undefined) { @@ -155,13 +159,9 @@ export class ValidationManager { return false; } - const isExtractableManagementEnabled = this.config.get('enableServices.extractable.extractableManagement'); - if (isExtractableManagementEnabled) { - const existsInExtractable = await this.extractable.isExtractableRecordExists(record.producerName!); - if (existsInExtractable) { - refReason.outFailedReason = ERROR_METADATA_PRODUCT_NAME_CONFLICT; - return false; - } + const isValid = await this.isRecordAbsentFromExtractable(record, refReason); + if (!isValid) { + return [false, true]; } if (record.productStatus == RecordStatus.BEING_DELETED) { @@ -226,6 +226,11 @@ export class ValidationManager { return true; } + @withSpanAsyncV4 + public async validateUpdateStatus(record3D: Record3D, refReason: FailedReason): Promise { + return this.isRecordAbsentFromExtractable(record3D, refReason); + } + @withSpanV4 public isModelPathValid(sourcePath: string, basePath: string): boolean { const logContext = { ...this.logContext, function: this.isModelPathValid.name }; @@ -486,4 +491,24 @@ export class ValidationManager { }); return true; } + + private async isRecordAbsentFromExtractable(record: Record3D, refReason: FailedReason): Promise { + const logContext = { ...this.logContext, function: this.isRecordAbsentFromExtractable.name }; + + if (!this.isExtractableManagementEnabled) { + this.logger.debug({ + msg: 'Extractable validation skipped - service disabled', + logContext, + }); + return true; + } + + const existsInExtractable = await this.extractable.isExtractableRecordExists(record.producerName!); + if (existsInExtractable) { + refReason.outFailedReason = ERROR_METADATA_PRODUCT_NAME_CONFLICT; + return false; + } + + return true; + } } diff --git a/tests/helpers/mockCreator.ts b/tests/helpers/mockCreator.ts index cbf5e3e..64c045d 100644 --- a/tests/helpers/mockCreator.ts +++ b/tests/helpers/mockCreator.ts @@ -25,6 +25,7 @@ export const validationManagerMock = { validateUpdate: jest.fn(), getTilesetModelPolygon: jest.fn(), isPolygonValid: jest.fn(), + validateUpdateStatus: jest.fn(), }; export const configMock = { @@ -55,4 +56,4 @@ export const catalogMock = { export const extractableMock = { isExtractableRecordExists: jest.fn(), -} +}; diff --git a/tests/integration/metadata/metadataController.spec.ts b/tests/integration/metadata/metadataController.spec.ts index 8cca3a8..ca30a5d 100644 --- a/tests/integration/metadata/metadataController.spec.ts +++ b/tests/integration/metadata/metadataController.spec.ts @@ -4,6 +4,7 @@ import { StatusCodes } from 'http-status-codes'; import mockAxios from 'jest-mock-axios'; import { faker } from '@faker-js/faker'; import config from 'config'; +import { register } from 'prom-client'; import { RecordStatus } from '@map-colonies/types'; import { ILookupOption } from '../../../src/externalServices/lookupTables/interfaces'; import { @@ -23,6 +24,7 @@ import { extractLink } from '../../../src/validator/extractPathFromLink'; import { CatalogCall } from '../../../src/externalServices/catalog/catalogCall'; import { ERROR_METADATA_PRODUCT_NAME_CONFLICT, ERROR_METADATA_PRODUCT_NAME_UNIQUE } from '../../../src/validator/validationManager'; import { MetadataRequestSender } from './helpers/requestSender'; +import { IConfig } from '../../../src/common/interfaces'; describe('MetadataController', function () { let requestSender: MetadataRequestSender; @@ -42,6 +44,7 @@ describe('MetadataController', function () { beforeEach(async () => { await s3Helper.initialize(); + register.clear(); }); afterEach(async () => { @@ -134,10 +137,8 @@ describe('MetadataController', function () { const response = await requestSender.updateMetadata(identifier, payload); - /* eslint-disable @typescript-eslint/no-unsafe-assignment */ expect(catalogCallPatchPayloadSpy).toHaveBeenCalledTimes(1); expect(catalogCallPatchPayloadSpy).toHaveBeenCalledWith(expect.any(String), patchMetadataPayload); - expect(response.status).toBe(StatusCodes.OK); expect(response).toSatisfyApiSpec(); }); @@ -241,7 +242,7 @@ describe('MetadataController', function () { expect(response).toSatisfyApiSpec(); }); - it(`Should return 400 status code if producer name conflicts with extractable`, async function () { + it(`Should return 409 status code if product name conflicts with extractable`, async function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); @@ -252,7 +253,7 @@ describe('MetadataController', function () { const response = await requestSender.updateMetadata(identifier, payload); - expect(response.status).toBe(StatusCodes.BAD_REQUEST); + expect(response.status).toBe(StatusCodes.CONFLICT); expect(response.body).toHaveProperty('message', ERROR_METADATA_PRODUCT_NAME_CONFLICT); expect(response).toSatisfyApiSpec(); }); @@ -266,10 +267,8 @@ describe('MetadataController', function () { await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); - const clonedRecordWithSameNameAsPayload = { ...record, productName: payload.productName }; mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [clonedRecordWithSameNameAsPayload] }); - mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); @@ -307,9 +306,9 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockRejectedValueOnce(new Error('lookup-tables error')); @@ -336,13 +335,12 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); + const linkUrl = extractLink(record.links); + await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); - const linkUrl = extractLink(record.links); - await s3Helper.createFile(linkUrl, true); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.BAD_REQUEST, data: [] }); - mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); const response = await requestSender.updateMetadata(identifier, payload); @@ -355,11 +353,11 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); + const linkUrl = extractLink(record.links); + await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); - const linkUrl = extractLink(record.links); - await s3Helper.createFile(linkUrl, true); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.CONFLICT }); @@ -431,7 +429,7 @@ describe('MetadataController', function () { await requestSender.updateMetadata(identifier, payload); - const extractableCall = mockAxios.get.mock.calls.find(call => call[0]!.includes('/records/')); + const extractableCall = mockAxios.get.mock.calls.find((call) => call[0]!.includes('/records/')); expect(extractableCall).toBeDefined(); const options = extractableCall![1]; expect(options.validateStatus).toBeDefined(); @@ -441,6 +439,49 @@ describe('MetadataController', function () { expect(validateStatus(500)).toBe(true); }); }); + + describe('When extractable management is disabled', function () { + let disabledRequestSender: MetadataRequestSender; + + beforeAll(function () { + const disabledConfig: IConfig = { + ...config, + get: (setting: string): T => { + if (setting === 'enableServices.extractable.extractableManagement') { + return false as T; + } + return config.get(setting); + }, + }; + const app = getApp({ + override: [ + { token: SERVICES.LOGGER, provider: { useValue: jsLogger({ enabled: false }) } }, + { token: SERVICES.TRACER, provider: { useValue: trace.getTracer('testTracer') } }, + { token: SERVICES.CONFIG, provider: { useValue: disabledConfig } }, + ], + }); + disabledRequestSender = new MetadataRequestSender(app); + }); + + it(`Should return 200 status code even if product name conflicts with extractable`, async function () { + const identifier = faker.string.uuid(); + const payload = createUpdatePayload(); + const expected = createRecord(); + const record = createRecord(); + const linkUrl = extractLink(record.links); + await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + // no extractable call — service is disabled + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); + + const response = await disabledRequestSender.updateMetadata(identifier, payload); + + expect(response.status).toBe(StatusCodes.OK); + expect(response).toSatisfyApiSpec(); + }); + }); }); describe('PATCH /metadata/status/{identifier}', function () { @@ -449,8 +490,9 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); const expected = createRecord(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); - mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); // getRecord + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); // extractable check + mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); // changeStatus const response = await requestSender.updateStatus(identifier, payload); @@ -471,6 +513,19 @@ describe('MetadataController', function () { expect(response.body).toHaveProperty('message', `Record with identifier: ${identifier} doesn't exist!`); expect(response).toSatisfyApiSpec(); }); + + it(`Should return 409 status code if conflicts with extractable`, async function () { + const identifier = faker.string.uuid(); + const payload = createUpdateStatusPayload(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); // getRecord + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK }); // extractable → found → conflict + + const response = await requestSender.updateStatus(identifier, payload); + + expect(response.status).toBe(StatusCodes.CONFLICT); + expect(response.body).toHaveProperty('message', ERROR_METADATA_PRODUCT_NAME_CONFLICT); + expect(response).toSatisfyApiSpec(); + }); }); describe('Sad Path 😥', function () { @@ -489,8 +544,9 @@ describe('MetadataController', function () { it(`Should return 500 status code if during sending request, catalog didn't return as expected`, async function () { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); - mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.CONFLICT }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); // getRecord + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); // extractable check + mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.CONFLICT }); // changeStatus → unexpected const response = await requestSender.updateStatus(identifier, payload); @@ -499,5 +555,43 @@ describe('MetadataController', function () { expect(response).toSatisfyApiSpec(); }); }); + + describe('When extractable management is disabled', function () { + let disabledRequestSender: MetadataRequestSender; + + beforeAll(function () { + const disabledConfig: IConfig = { + ...config, + get: (setting: string): T => { + if (setting === 'enableServices.extractable.extractableManagement') { + return false as T; + } + return config.get(setting); + }, + }; + const app = getApp({ + override: [ + { token: SERVICES.LOGGER, provider: { useValue: jsLogger({ enabled: false }) } }, + { token: SERVICES.TRACER, provider: { useValue: trace.getTracer('testTracer') } }, + { token: SERVICES.CONFIG, provider: { useValue: disabledConfig } }, + ], + }); + disabledRequestSender = new MetadataRequestSender(app); + }); + + it(`Should return 200 status code even if conflicts with extractable`, async function () { + const identifier = faker.string.uuid(); + const payload = createUpdateStatusPayload(); + const expected = createRecord(); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); // getRecord + // no extractable call — service is disabled + mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); // changeStatus + + const response = await disabledRequestSender.updateStatus(identifier, payload); + + expect(response.status).toBe(StatusCodes.OK); + expect(response).toSatisfyApiSpec(); + }); + }); }); }); diff --git a/tests/unit/externalServices/extractable/requestCall.spec.ts b/tests/unit/externalServices/extractable/requestCall.spec.ts index eb99414..4431124 100644 --- a/tests/unit/externalServices/extractable/requestCall.spec.ts +++ b/tests/unit/externalServices/extractable/requestCall.spec.ts @@ -8,12 +8,15 @@ import { ExtractableCall } from '../../../../src/externalServices/extractable-ma let extractable: ExtractableCall; describe('extractableCall tests', () => { const extractableUrl = config.get('externalServices.extractable'); + beforeEach(() => { extractable = new ExtractableCall(config, jsLogger({ enabled: false }), trace.getTracer('testTracer')); }); + afterEach(() => { jest.clearAllMocks(); }); + describe('isExtractableRecordExists Function', () => { it('Returns true when recordName exists in DB', async () => { const recordName = faker.string.uuid(); @@ -24,6 +27,7 @@ describe('extractableCall tests', () => { expect(mockAxios.get).toHaveBeenCalledWith(`${extractableUrl}/records/${recordName}`, { validateStatus: expect.any(Function) }); expect(response).toBe(true); }); + it('Returns false when recordName does not exist in DB', async () => { const recordName = faker.string.uuid(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); @@ -33,15 +37,17 @@ describe('extractableCall tests', () => { expect(mockAxios.get).toHaveBeenCalledWith(`${extractableUrl}/records/${recordName}`, { validateStatus: expect.any(Function) }); expect(response).toBe(false); }); + it('Rejects if got unexpected response from extractable', async () => { const recordName = faker.string.uuid(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.INTERNAL_SERVER_ERROR }); - + const response = extractable.isExtractableRecordExists(recordName); - + expect(mockAxios.get).toHaveBeenCalledWith(`${extractableUrl}/records/${recordName}`, { validateStatus: expect.any(Function) }); await expect(response).rejects.toThrow('Unexpected response from extractable service'); }); + it('rejects if service is not available', async () => { const recordName = faker.string.uuid(); mockAxios.get.mockRejectedValueOnce(new Error('extractable is not available')); @@ -50,6 +56,7 @@ describe('extractableCall tests', () => { await expect(response).rejects.toThrow('Failed to query extractable service'); }); + it('uses validateStatus that always returns true', async () => { const recordName = faker.string.uuid(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK }); @@ -66,4 +73,4 @@ describe('extractableCall tests', () => { expect(validateStatus(0)).toBe(true); }); }); -}); \ No newline at end of file +}); diff --git a/tests/unit/metadata/models/metadataManager.spec.ts b/tests/unit/metadata/models/metadataManager.spec.ts index 76bc5bb..4470c15 100644 --- a/tests/unit/metadata/models/metadataManager.spec.ts +++ b/tests/unit/metadata/models/metadataManager.spec.ts @@ -1,3 +1,4 @@ +import { StatusCodes } from 'http-status-codes'; import jsLogger from '@map-colonies/js-logger'; import { trace } from '@opentelemetry/api'; import { faker } from '@faker-js/faker'; @@ -12,6 +13,7 @@ let metadataManager: MetadataManager; describe('MetadataManager', () => { beforeEach(() => { + validationManagerMock.validateUpdateStatus = jest.fn(); metadataManager = new MetadataManager( jsLogger({ enabled: false }), trace.getTracer('testTracer'), @@ -65,6 +67,42 @@ describe('MetadataManager', () => { await expect(response).rejects.toThrow(AppError); }); + + it('rejects with conflict if validation failed due to extractable conflict', async () => { + const identifier = faker.string.uuid(); + const payload: UpdatePayload = createUpdatePayload(); + const refReason = { outFailedReason: 'conflict reason' }; + validationManagerMock.validateUpdate.mockImplementation(async (id, pl, ref) => { + ref.outFailedReason = 'conflict reason'; + return [false, true]; + }); + + const response = metadataManager.updateMetadata(identifier, payload); + + await expect(response).rejects.toThrow(AppError); + await expect(response).rejects.toMatchObject({ + status: StatusCodes.CONFLICT, + message: 'conflict reason', + }); + }); + + it('rejects with bad request if validation failed for other reasons', async () => { + const identifier = faker.string.uuid(); + const payload: UpdatePayload = createUpdatePayload(); + const refReason = { outFailedReason: 'bad request reason' }; + validationManagerMock.validateUpdate.mockImplementation(async (id, pl, ref) => { + ref.outFailedReason = 'bad request reason'; + return false; + }); + + const response = metadataManager.updateMetadata(identifier, payload); + + await expect(response).rejects.toThrow(AppError); + await expect(response).rejects.toMatchObject({ + status: StatusCodes.BAD_REQUEST, + message: 'bad request reason', + }); + }); }); describe('updateStatus tests', () => { @@ -72,6 +110,7 @@ describe('MetadataManager', () => { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); catalogMock.getRecord.mockResolvedValue(createRecord()); + validationManagerMock.validateUpdateStatus.mockResolvedValue(true); catalogMock.changeStatus.mockResolvedValue(payload); const response = await metadataManager.updateStatus(identifier, payload); @@ -115,11 +154,30 @@ describe('MetadataManager', () => { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); catalogMock.getRecord.mockResolvedValue(createRecord()); + validationManagerMock.validateUpdateStatus.mockResolvedValue(true); catalogMock.changeStatus.mockRejectedValue(new Error('catalog service is not available')); const response = metadataManager.updateStatus(identifier, payload); await expect(response).rejects.toThrow(AppError); }); + + it('rejects with conflict if validation failed', async () => { + const identifier = faker.string.uuid(); + const payload = createUpdateStatusPayload(); + catalogMock.getRecord.mockResolvedValue(createRecord()); + validationManagerMock.validateUpdateStatus.mockImplementation(async (rec, ref) => { + ref.outFailedReason = 'conflict reason'; + return false; + }); + + const response = metadataManager.updateStatus(identifier, payload); + + await expect(response).rejects.toThrow(AppError); + await expect(response).rejects.toMatchObject({ + status: StatusCodes.CONFLICT, + message: 'conflict reason', + }); + }); }); }); diff --git a/tests/unit/validator/validationManager.spec.ts b/tests/unit/validator/validationManager.spec.ts index e3a3def..e40e81c 100644 --- a/tests/unit/validator/validationManager.spec.ts +++ b/tests/unit/validator/validationManager.spec.ts @@ -65,6 +65,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: true }); }); @@ -80,6 +81,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: ERROR_METADATA_RESOLUTION, @@ -102,6 +104,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: true }); }); @@ -117,6 +120,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: ERROR_METADATA_DATE }); }); @@ -132,6 +136,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: `Invalid polygon provided. Must be in a GeoJson format of a Polygon. Should contain "type", "coordinates" and "BBOX" only. polygon: ${JSON.stringify( @@ -151,6 +156,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: `Wrong polygon: ${JSON.stringify(payload.metadata.footprint)} the first and last coordinates should be equal`, @@ -168,6 +174,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: `Wrong footprint! footprint's coordinates should be all in the same dimension 2D or 3D`, @@ -184,6 +191,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, {} as unknown as Polygon); + expect(response).toStrictEqual({ isValid: false, message: `An error caused during the validation of the intersection` }); }); @@ -195,6 +203,7 @@ describe('ValidationManager', () => { catalogMock.findRecords.mockResolvedValue([]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: ERROR_METADATA_FOOTPRINT_FAR_FROM_MODEL }); }); @@ -212,7 +221,9 @@ describe('ValidationManager', () => { extractableMock as never, providerMock ); + const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint('WrongVolume')); + expect(response.isValid).toBe(false); expect(response.message).toContain('The footprint intersectection with the model'); }); @@ -227,6 +238,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: true }); // For now, the validation will be only warning. so it's true }); @@ -241,6 +253,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: true }); }); @@ -253,6 +266,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: `Record with productId: ${payload.metadata.productId} doesn't exist!`, @@ -268,6 +282,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue(['NonValidClassification']); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: `classification is not a valid value.. Optional values: ${'NonValidClassification'}`, @@ -285,6 +300,7 @@ describe('ValidationManager', () => { lookupTablesMock.getClassifications.mockResolvedValue([payload.metadata.classification]); const response = await validationManager.isMetadataValidForIngestion(payload.metadata, createFootprint()); + expect(response).toStrictEqual({ isValid: false, message: ERROR_METADATA_PRODUCT_NAME_UNIQUE, @@ -300,6 +316,7 @@ describe('ValidationManager', () => { { path: join('nonExistsFolder', createTilesetFileName()), result: false }, ])('should check if sources exists and return true for %p', async (testInput: { path: string; result: boolean }) => { const response = await validationManager.isPathExist(testInput.path); + expect(response).toBe(testInput.result); }); }); @@ -331,7 +348,6 @@ describe('ValidationManager', () => { describe('validateModelPath tests', () => { it('returns true when got valid model path', () => { const modelPath = createModelPath(); - const result = validationManager.isModelPathValid(modelPath, getBasePath()); expect(result).toBe(true); @@ -339,7 +355,6 @@ describe('ValidationManager', () => { it('returns false when model path not in the agreed path', () => { const modelPath = 'some/path'; - const result = validationManager.isModelPathValid(modelPath, getBasePath()); expect(result).toBe(false); @@ -351,6 +366,7 @@ describe('ValidationManager', () => { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); + catalogMock.findRecords.mockResolvedValue([]); catalogMock.getRecord.mockResolvedValue(record); lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); @@ -365,6 +381,7 @@ describe('ValidationManager', () => { it('returns error if catalog dont contain the requested record', async () => { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); + catalogMock.findRecords.mockResolvedValue([]); catalogMock.getRecord.mockResolvedValue(undefined); @@ -380,6 +397,7 @@ describe('ValidationManager', () => { const payload = createUpdatePayload(); const record = createRecord(); record.productStatus = RecordStatus.BEING_DELETED; + catalogMock.findRecords.mockResolvedValue([]); catalogMock.getRecord.mockResolvedValue(record); @@ -393,6 +411,7 @@ describe('ValidationManager', () => { it('throws error when catalog services does not properly responded', async () => { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); + catalogMock.findRecords.mockResolvedValue([]); catalogMock.getRecord.mockRejectedValue(new AppError('error', StatusCodes.INTERNAL_SERVER_ERROR, 'catalog error', true)); @@ -406,6 +425,7 @@ describe('ValidationManager', () => { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); + catalogMock.findRecords.mockResolvedValue([]); catalogMock.getRecord.mockResolvedValue(record); lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); @@ -425,6 +445,7 @@ describe('ValidationManager', () => { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); + catalogMock.findRecords.mockResolvedValue([]); catalogMock.getRecord.mockResolvedValue(record); lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); @@ -448,6 +469,7 @@ describe('ValidationManager', () => { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); + catalogMock.findRecords.mockResolvedValue([]); catalogMock.getRecord.mockResolvedValue(record); lookupTablesMock.getClassifications.mockResolvedValue(['NonValidClassification']); @@ -465,6 +487,7 @@ describe('ValidationManager', () => { const payload = createUpdatePayload(); const record = createRecord(); const clonedRecordWithSameNameAsPayload = { ...record, productName: payload.productName }; + catalogMock.findRecords.mockResolvedValue([clonedRecordWithSameNameAsPayload]); catalogMock.getRecord.mockResolvedValue(record); lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); @@ -478,10 +501,10 @@ describe('ValidationManager', () => { }); it('returns false when extractable management is enabled and record exists in extractable', async () => { - configMock.get.mockReturnValue(true); const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); + catalogMock.findRecords.mockResolvedValue([]); catalogMock.getRecord.mockResolvedValue(record); lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); @@ -491,16 +514,95 @@ describe('ValidationManager', () => { const refReason: FailedReason = { outFailedReason: '' }; const response = await validationManager.validateUpdate(identifier, payload, refReason); + expect(extractableMock.isExtractableRecordExists).toHaveBeenCalledWith(record.producerName); + expect(response).toStrictEqual([false, true]); + expect(refReason.outFailedReason).toBe(ERROR_METADATA_PRODUCT_NAME_CONFLICT); + }); + + it('returns true when extractable management is disabled even if record exists in extractable', async () => { + configMock.get.mockReturnValue(false); + + validationManager = new ValidationManager( + configMock, + jsLogger({ enabled: false }), + trace.getTracer('testTracer'), + lookupTablesMock as never, + catalogMock as never, + extractableMock as never, + providerMock + ); + + const identifier = faker.string.uuid(); + const payload = createUpdatePayload(); + const record = createRecord(); + + catalogMock.findRecords.mockResolvedValue([]); + catalogMock.getRecord.mockResolvedValue(record); + lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); + providerMock.getFile.mockResolvedValue(getTileset()); + extractableMock.isExtractableRecordExists.mockResolvedValue(true); + + const refReason: FailedReason = { outFailedReason: '' }; + const response = await validationManager.validateUpdate(identifier, payload, refReason); + + expect(extractableMock.isExtractableRecordExists).not.toHaveBeenCalled(); + expect(response).toBe(true); + }); + }); + + describe('validateUpdateStatus', () => { + it('returns true when valid', async () => { + const record = createRecord(); + extractableMock.isExtractableRecordExists.mockResolvedValue(false); + + const refReason: FailedReason = { outFailedReason: '' }; + const response = await validationManager.validateUpdateStatus(record, refReason); + + expect(extractableMock.isExtractableRecordExists).toHaveBeenCalledWith(record.producerName); + expect(response).toBe(true); + }); + + it('returns false when extractable exists', async () => { + const record = createRecord(); + extractableMock.isExtractableRecordExists.mockResolvedValue(true); + + const refReason: FailedReason = { outFailedReason: '' }; + const response = await validationManager.validateUpdateStatus(record, refReason); + expect(extractableMock.isExtractableRecordExists).toHaveBeenCalledWith(record.producerName); expect(response).toBe(false); expect(refReason.outFailedReason).toBe(ERROR_METADATA_PRODUCT_NAME_CONFLICT); }); + + it('returns true when disabled', async () => { + configMock.get.mockReturnValue(false); + + validationManager = new ValidationManager( + configMock, + jsLogger({ enabled: false }), + trace.getTracer('testTracer'), + lookupTablesMock as never, + catalogMock as never, + extractableMock as never, + providerMock + ); + + const record = createRecord(); + extractableMock.isExtractableRecordExists.mockResolvedValue(true); + + const refReason: FailedReason = { outFailedReason: '' }; + const response = await validationManager.validateUpdateStatus(record, refReason); + + expect(extractableMock.isExtractableRecordExists).not.toHaveBeenCalled(); + expect(response).toBe(true); + }); }); describe('isPolygonValid', () => { it('returns true when Polygon is valid', () => { const footprint = createFootprint('Region'); const response = validationManager.isPolygonValid(footprint); + expect(response.isValid).toBe(true); }); @@ -508,6 +610,7 @@ describe('ValidationManager', () => { const footprint = createFootprint('Region'); footprint.bbox = [faker.location.longitude(), faker.location.latitude(), faker.location.longitude(), faker.location.latitude()]; const response = validationManager.isPolygonValid(footprint); + expect(response.isValid).toBe(true); }); @@ -515,6 +618,7 @@ describe('ValidationManager', () => { const footprint = createFootprint('Region'); footprint.coordinates = [][0] as unknown as Position[][]; const response = validationManager.isPolygonValid(footprint); + expect(response.isValid).toBe(false); expect(response.message).toContain( `Invalid polygon provided. Must be in a GeoJson format of a Polygon. Should contain "type", "coordinates" and "BBOX" only.` From ce57f3a9a2fa2ef6f1f392a25b6caf132c46881d Mon Sep 17 00:00:00 2001 From: liran Date: Tue, 24 Feb 2026 21:08:35 +0200 Subject: [PATCH 3/5] chore: meanningful name --- config/custom-environment-variables.json | 7 +- helm/templates/configmap.yaml | 2 +- src/externalServices/catalog/catalogCall.ts | 20 ++- src/metadata/models/metadataManager.ts | 17 +-- src/validator/validationManager.ts | 81 +++++------ tests/helpers/mockCreator.ts | 1 + .../metadata/metadataController.spec.ts | 136 ++++++------------ .../metadata/models/metadataManager.spec.ts | 35 ++--- .../unit/validator/validationManager.spec.ts | 86 +++++------ 9 files changed, 164 insertions(+), 221 deletions(-) diff --git a/config/custom-environment-variables.json b/config/custom-environment-variables.json index 27fe5b8..b334488 100644 --- a/config/custom-environment-variables.json +++ b/config/custom-environment-variables.json @@ -80,10 +80,9 @@ }, "enableServices": { "extractable": { - "extractableManagement": { - "__name": "ENABLE_EXTRACTABLE_MANAGEMENT", - "__format": "boolean" - } + "__name": "ENABLE_EXTRACTABLE_MANAGEMENT", + "__format": "boolean" + } } } diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index 6df47a5..0a66462 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -30,7 +30,7 @@ data: STORE_TRIGGER_URL: {{ .Values.env.storeTrigger.url | default (printf "http://%s-store-trigger" .Release.Name) }} CATALOG_URL: {{ .Values.env.catalog.url | default (printf "http://%s-catalog" .Release.Name) }} EXTRACTABLE_URL: {{ .Values.env.extractable.url | default (printf "http://%s-extractable" .Release.Name) }} - ENABLE_EXTRACTABLE_MANAGEMENT: {{ .Values.env.enableServices.extractable.extractableManagement | quote }} + ENABLE_EXTRACTABLE_MANAGEMENT: {{ .Values.env.enableServices.extractable | quote }} LOOKUP_TABLES_URL: {{ .Values.validations.lookupTables.url | quote }} LOOKUP_TABLES_SUB_URL: {{ .Values.validations.lookupTables.subUrl | quote }} {{ if eq $providers.source "NFS" }} diff --git a/src/externalServices/catalog/catalogCall.ts b/src/externalServices/catalog/catalogCall.ts index 747ca16..0f5bd9c 100644 --- a/src/externalServices/catalog/catalogCall.ts +++ b/src/externalServices/catalog/catalogCall.ts @@ -67,12 +67,20 @@ export class CatalogCall { }); return response.data; } else { - this.logger.error({ - msg: `Something went wrong in catalog when tring to find records, service returned ${response.status}`, - logContext, - response, - }); - throw new AppError('catalog', StatusCodes.INTERNAL_SERVER_ERROR, 'Problem with the catalog during Finding Records', true); + /* istanbul ignore next */ + { + this.logger.error({ + msg: `Something went wrong in catalog when tring to find records, service returned ${response.status}`, + logContext, + response, + }); + throw new AppError( + 'catalog', + StatusCodes.INTERNAL_SERVER_ERROR, + 'Problem with the catalog during Finding Records', + true + ); + } } } catch (err) { this.logger.error({ diff --git a/src/metadata/models/metadataManager.ts b/src/metadata/models/metadataManager.ts index 39e6246..3fb7849 100644 --- a/src/metadata/models/metadataManager.ts +++ b/src/metadata/models/metadataManager.ts @@ -51,17 +51,18 @@ export class MetadataManager { try { const refReason: FailedReason = { outFailedReason: '' }; - const result = await this.validator.validateUpdate(identifier, payload, refReason); - const isExtractableConflict = Array.isArray(result) && result[1] === true; - const isValid = result === true; + const isValid = await this.validator.validateUpdate(identifier, payload, refReason); - if (!isValid && isExtractableConflict) { - throw new AppError('conflict', StatusCodes.CONFLICT, refReason.outFailedReason, true); - } if (!isValid) { throw new AppError('badRequest', StatusCodes.BAD_REQUEST, refReason.outFailedReason, true); } + const record = (await this.catalog.getRecord(identifier)) as Record3D; + const doesNotExistInExtractable = await this.validator.isRecordAbsentFromExtractable(record, refReason); + if (!doesNotExistInExtractable) { + throw new AppError('conflict', StatusCodes.CONFLICT, refReason.outFailedReason, true); + } + this.logger.info({ msg: 'model validated successfully', logContext, @@ -121,8 +122,8 @@ export class MetadataManager { } const refReason: FailedReason = { outFailedReason: '' }; - const isValid: boolean = await this.validator.validateUpdateStatus(record3D, refReason); - if (!isValid) { + const doesNotExistInExtractable = await this.validator.isRecordAbsentFromExtractable(record3D, refReason); + if (!doesNotExistInExtractable) { throw new AppError('conflict', StatusCodes.CONFLICT, refReason.outFailedReason, true); } diff --git a/src/validator/validationManager.ts b/src/validator/validationManager.ts index 78ab26b..b2997cc 100644 --- a/src/validator/validationManager.ts +++ b/src/validator/validationManager.ts @@ -54,7 +54,7 @@ export class ValidationManager { }; this.limit = this.config.get('validation.percentageLimit'); - this.isExtractableManagementEnabled = this.config.get('enableServices.extractable.extractableManagement'); + this.isExtractableManagementEnabled = this.config.get('enableServices.extractable'); } @withSpanAsyncV4 @@ -151,7 +151,7 @@ export class ValidationManager { } @withSpanAsyncV4 - public async validateUpdate(identifier: string, payload: UpdatePayload, refReason: FailedReason): Promise { + public async validateUpdate(identifier: string, payload: UpdatePayload, refReason: FailedReason): Promise { const record = await this.catalog.getRecord(identifier); if (record === undefined) { @@ -159,11 +159,6 @@ export class ValidationManager { return false; } - const isValid = await this.isRecordAbsentFromExtractable(record, refReason); - if (!isValid) { - return [false, true]; - } - if (record.productStatus == RecordStatus.BEING_DELETED) { refReason.outFailedReason = `Can't update record that is being deleted`; return false; @@ -226,11 +221,6 @@ export class ValidationManager { return true; } - @withSpanAsyncV4 - public async validateUpdateStatus(record3D: Record3D, refReason: FailedReason): Promise { - return this.isRecordAbsentFromExtractable(record3D, refReason); - } - @withSpanV4 public isModelPathValid(sourcePath: string, basePath: string): boolean { const logContext = { ...this.logContext, function: this.isModelPathValid.name }; @@ -361,6 +351,26 @@ export class ValidationManager { return { isValid: true }; } + public async isRecordAbsentFromExtractable(record: Record3D, refReason: FailedReason): Promise { + const logContext = { ...this.logContext, function: this.isRecordAbsentFromExtractable.name }; + + if (!this.isExtractableManagementEnabled) { + this.logger.debug({ + msg: 'Extractable validation skipped - service disabled', + logContext, + }); + return true; + } + + const existsInExtractable = await this.extractable.isExtractableRecordExists(record.productName!); + if (existsInExtractable) { + refReason.outFailedReason = ERROR_METADATA_PRODUCT_NAME_CONFLICT; + return false; + } + + return true; + } + private validateCoordinates(footprint: Polygon): boolean { const length = footprint.coordinates[0].length; const first = footprint.coordinates[0][0]; @@ -446,18 +456,21 @@ export class ValidationManager { isValid: true, }; } catch (err) { - const msg = `An error caused during the validation of the intersection`; - this.logger.error({ - msg, - logContext, - err, - modelPolygon, - footprint, - }); - return { - isValid: false, - message: msg, - }; + /* istanbul ignore next */ + { + const msg = `An error caused during the validation of the intersection`; + this.logger.error({ + msg, + logContext, + err, + modelPolygon, + footprint, + }); + return { + isValid: false, + message: msg, + }; + } } } @@ -491,24 +504,4 @@ export class ValidationManager { }); return true; } - - private async isRecordAbsentFromExtractable(record: Record3D, refReason: FailedReason): Promise { - const logContext = { ...this.logContext, function: this.isRecordAbsentFromExtractable.name }; - - if (!this.isExtractableManagementEnabled) { - this.logger.debug({ - msg: 'Extractable validation skipped - service disabled', - logContext, - }); - return true; - } - - const existsInExtractable = await this.extractable.isExtractableRecordExists(record.producerName!); - if (existsInExtractable) { - refReason.outFailedReason = ERROR_METADATA_PRODUCT_NAME_CONFLICT; - return false; - } - - return true; - } } diff --git a/tests/helpers/mockCreator.ts b/tests/helpers/mockCreator.ts index 64c045d..a9c3237 100644 --- a/tests/helpers/mockCreator.ts +++ b/tests/helpers/mockCreator.ts @@ -26,6 +26,7 @@ export const validationManagerMock = { getTilesetModelPolygon: jest.fn(), isPolygonValid: jest.fn(), validateUpdateStatus: jest.fn(), + isRecordAbsentFromExtractable: jest.fn(), }; export const configMock = { diff --git a/tests/integration/metadata/metadataController.spec.ts b/tests/integration/metadata/metadataController.spec.ts index ca30a5d..8f937f7 100644 --- a/tests/integration/metadata/metadataController.spec.ts +++ b/tests/integration/metadata/metadataController.spec.ts @@ -65,9 +65,10 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); - mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const response = await requestSender.updateMetadata(identifier, payload); @@ -85,9 +86,10 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); - mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [record] }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const response = await requestSender.updateMetadata(identifier, payload); @@ -106,41 +108,19 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); - mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const catalogCallPatchPayloadSpy = jest.spyOn(CatalogCall.prototype, 'patchMetadata'); - const patchMetadataPayload = { - productName: payload.productName, - sourceDateStart: payload.sourceDateStart?.toISOString(), - sourceDateEnd: payload.sourceDateEnd?.toISOString(), - footprint: expectedFootprint, - description: payload.description, - creationDate: payload.creationDate?.toISOString(), - minResolutionMeter: payload.minResolutionMeter, - maxResolutionMeter: payload.maxResolutionMeter, - maxAccuracyCE90: payload.maxAccuracyCE90, - absoluteAccuracyLE90: payload.absoluteAccuracyLE90, - accuracySE90: payload.accuracySE90, - relativeAccuracySE90: payload.relativeAccuracySE90, - visualAccuracy: payload.visualAccuracy, - heightRangeFrom: payload.heightRangeFrom, - heightRangeTo: payload.heightRangeTo, - classification: payload.classification, - producerName: payload.producerName, - maxFlightAlt: payload.maxFlightAlt, - minFlightAlt: payload.minFlightAlt, - geographicArea: payload.geographicArea, - }; const response = await requestSender.updateMetadata(identifier, payload); - expect(catalogCallPatchPayloadSpy).toHaveBeenCalledTimes(1); - expect(catalogCallPatchPayloadSpy).toHaveBeenCalledWith(expect.any(String), patchMetadataPayload); expect(response.status).toBe(StatusCodes.OK); expect(response).toSatisfyApiSpec(); + expect(catalogCallPatchPayloadSpy).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ footprint: expectedFootprint })); }); }); @@ -153,7 +133,7 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: classification }] as ILookupOption[] }); const response = await requestSender.updateMetadata(identifier, payload); @@ -169,15 +149,10 @@ describe('MetadataController', function () { payload.footprint = createWrongFootprintSchema(); const record = createRecord(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); expect(response.status).toBe(StatusCodes.BAD_REQUEST); - expect(response.body).toHaveProperty( - 'message', - `request/body/footprint/coordinates must NOT have fewer than 2 items, request/body/footprint/type must be equal to one of the allowed values: LineString, request/body/footprint/type must be equal to one of the allowed values: Polygon, request/body/footprint/type must be equal to one of the allowed values: MultiPoint, request/body/footprint/type must be equal to one of the allowed values: MultiLineString, request/body/footprint/type must be equal to one of the allowed values: MultiPolygon, request/body/footprint must match a schema in anyOf` - ); expect(response).toSatisfyApiSpec(); }); @@ -187,15 +162,11 @@ describe('MetadataController', function () { payload.footprint = createWrongFootprintCoordinates(); const record = createRecord(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); expect(response.status).toBe(StatusCodes.BAD_REQUEST); - expect(response.body).toHaveProperty( - 'message', - `Wrong polygon: ${JSON.stringify(payload.footprint)} the first and last coordinates should be equal` - ); + expect(response.body).toHaveProperty('message', `Wrong polygon: ${JSON.stringify(payload.footprint)} the first and last coordinates should be equal`); expect(response).toSatisfyApiSpec(); }); @@ -205,7 +176,6 @@ describe('MetadataController', function () { payload.footprint = createWrongFootprintMixed2D3D(); const record = createRecord(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); @@ -221,7 +191,6 @@ describe('MetadataController', function () { payload.sourceDateStart = faker.date.soon(); const record = createRecord(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); @@ -249,6 +218,9 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK }); const response = await requestSender.updateMetadata(identifier, payload); @@ -261,16 +233,13 @@ describe('MetadataController', function () { it(`Should return 400 status code if record product name already exists in catalog`, async function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); - const expected = createRecord(); const record = createRecord(); const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const clonedRecordWithSameNameAsPayload = { ...record, productName: payload.productName }; + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [clonedRecordWithSameNameAsPayload] }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); - mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const response = await requestSender.updateMetadata(identifier, payload); @@ -282,16 +251,11 @@ describe('MetadataController', function () { it(`Should return 400 status code if record product status is 'Being-Deleted'`, async function () { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); - const expected = createRecord(); const record = createRecord(); + record.productStatus = RecordStatus.BEING_DELETED; const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); - record.productStatus = RecordStatus.BEING_DELETED; mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); - mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); - mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); - mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const response = await requestSender.updateMetadata(identifier, payload); @@ -308,8 +272,9 @@ describe('MetadataController', function () { const record = createRecord(); const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); mockAxios.get.mockRejectedValueOnce(new Error('lookup-tables error')); const response = await requestSender.updateMetadata(identifier, payload); @@ -338,27 +303,10 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); - mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.BAD_REQUEST, data: [] }); - - const response = await requestSender.updateMetadata(identifier, payload); - - expect(response.status).toBe(StatusCodes.INTERNAL_SERVER_ERROR); - expect(response.body).toHaveProperty('message', 'Problem with catalog find'); - expect(response).toSatisfyApiSpec(); - }); - - it(`Should return 500 status code if during sending request, catalog didn't return as expected on find`, async function () { - const identifier = faker.string.uuid(); - const payload = createUpdatePayload(); - const record = createRecord(); - const linkUrl = extractLink(record.links); - await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); - mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); - mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.CONFLICT }); const response = await requestSender.updateMetadata(identifier, payload); @@ -374,7 +322,6 @@ describe('MetadataController', function () { const record = createRecord(); record.links = faker.word.sample(); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); const response = await requestSender.updateMetadata(identifier, payload); @@ -390,6 +337,9 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.INTERNAL_SERVER_ERROR }); const response = await requestSender.updateMetadata(identifier, payload); @@ -406,6 +356,9 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.get.mockRejectedValueOnce(new Error('extractable is not available')); const response = await requestSender.updateMetadata(identifier, payload); @@ -421,10 +374,12 @@ describe('MetadataController', function () { const record = createRecord(); const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); - mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); await requestSender.updateMetadata(identifier, payload); @@ -447,7 +402,7 @@ describe('MetadataController', function () { const disabledConfig: IConfig = { ...config, get: (setting: string): T => { - if (setting === 'enableServices.extractable.extractableManagement') { + if (setting === 'enableServices.extractable') { return false as T; } return config.get(setting); @@ -471,9 +426,9 @@ describe('MetadataController', function () { const linkUrl = extractLink(record.links); await s3Helper.createFile(linkUrl, true); mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); - // no extractable call — service is disabled - mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); mockAxios.post.mockResolvedValueOnce({ status: StatusCodes.OK, data: [] }); + mockAxios.get.mockResolvedValueOnce({ data: [{ value: payload.classification }] as ILookupOption[] }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: record }); mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const response = await disabledRequestSender.updateMetadata(identifier, payload); @@ -490,9 +445,9 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); const expected = createRecord(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); // getRecord - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); // extractable check - mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); // changeStatus + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); + mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const response = await requestSender.updateStatus(identifier, payload); @@ -517,8 +472,9 @@ describe('MetadataController', function () { it(`Should return 409 status code if conflicts with extractable`, async function () { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); // getRecord - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK }); // extractable → found → conflict + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK }); const response = await requestSender.updateStatus(identifier, payload); @@ -544,9 +500,9 @@ describe('MetadataController', function () { it(`Should return 500 status code if during sending request, catalog didn't return as expected`, async function () { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); // getRecord - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); // extractable check - mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.CONFLICT }); // changeStatus → unexpected + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.NOT_FOUND }); + mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.CONFLICT }); const response = await requestSender.updateStatus(identifier, payload); @@ -563,7 +519,7 @@ describe('MetadataController', function () { const disabledConfig: IConfig = { ...config, get: (setting: string): T => { - if (setting === 'enableServices.extractable.extractableManagement') { + if (setting === 'enableServices.extractable') { return false as T; } return config.get(setting); @@ -583,9 +539,9 @@ describe('MetadataController', function () { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); const expected = createRecord(); - mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); // getRecord - // no extractable call — service is disabled - mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); // changeStatus + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + mockAxios.get.mockResolvedValueOnce({ status: StatusCodes.OK, data: createRecord() }); + mockAxios.patch.mockResolvedValueOnce({ status: StatusCodes.OK, data: expected }); const response = await disabledRequestSender.updateStatus(identifier, payload); diff --git a/tests/unit/metadata/models/metadataManager.spec.ts b/tests/unit/metadata/models/metadataManager.spec.ts index 4470c15..45075c1 100644 --- a/tests/unit/metadata/models/metadataManager.spec.ts +++ b/tests/unit/metadata/models/metadataManager.spec.ts @@ -13,7 +13,6 @@ let metadataManager: MetadataManager; describe('MetadataManager', () => { beforeEach(() => { - validationManagerMock.validateUpdateStatus = jest.fn(); metadataManager = new MetadataManager( jsLogger({ enabled: false }), trace.getTracer('testTracer'), @@ -29,7 +28,9 @@ describe('MetadataManager', () => { it('resolves without errors', async () => { const identifier = faker.string.uuid(); const payload: UpdatePayload = createUpdatePayload(); - validationManagerMock.validateUpdate.mockReturnValue(true); + validationManagerMock.validateUpdate.mockResolvedValue(true); + validationManagerMock.isRecordAbsentFromExtractable.mockResolvedValue(true); + catalogMock.getRecord.mockResolvedValue(createRecord()); catalogMock.patchMetadata.mockResolvedValue(payload); const response = await metadataManager.updateMetadata(identifier, payload); @@ -40,7 +41,7 @@ describe('MetadataManager', () => { it(`rejects if update's validation failed`, async () => { const identifier = faker.string.uuid(); const payload: UpdatePayload = createUpdatePayload(); - validationManagerMock.validateUpdate.mockReturnValue(false); + validationManagerMock.validateUpdate.mockResolvedValue(false); const response = metadataManager.updateMetadata(identifier, payload); @@ -60,7 +61,8 @@ describe('MetadataManager', () => { it(`rejects if didn't update metadata in catalog`, async () => { const identifier = faker.string.uuid(); const payload: UpdatePayload = createUpdatePayload(); - validationManagerMock.validateUpdate.mockReturnValue(true); + validationManagerMock.validateUpdate.mockResolvedValue(true); + validationManagerMock.isRecordAbsentFromExtractable.mockResolvedValue(true); catalogMock.patchMetadata.mockRejectedValue(new Error('catalog service is not available')); const response = metadataManager.updateMetadata(identifier, payload); @@ -71,14 +73,14 @@ describe('MetadataManager', () => { it('rejects with conflict if validation failed due to extractable conflict', async () => { const identifier = faker.string.uuid(); const payload: UpdatePayload = createUpdatePayload(); - const refReason = { outFailedReason: 'conflict reason' }; - validationManagerMock.validateUpdate.mockImplementation(async (id, pl, ref) => { + validationManagerMock.validateUpdate.mockResolvedValue(true); + validationManagerMock.isRecordAbsentFromExtractable.mockImplementation(async (_id, ref) => { ref.outFailedReason = 'conflict reason'; - return [false, true]; + return false; }); + catalogMock.getRecord.mockResolvedValue(createRecord()); const response = metadataManager.updateMetadata(identifier, payload); - await expect(response).rejects.toThrow(AppError); await expect(response).rejects.toMatchObject({ status: StatusCodes.CONFLICT, @@ -89,8 +91,7 @@ describe('MetadataManager', () => { it('rejects with bad request if validation failed for other reasons', async () => { const identifier = faker.string.uuid(); const payload: UpdatePayload = createUpdatePayload(); - const refReason = { outFailedReason: 'bad request reason' }; - validationManagerMock.validateUpdate.mockImplementation(async (id, pl, ref) => { + validationManagerMock.validateUpdate.mockImplementation(async (_id, _pl, ref) => { ref.outFailedReason = 'bad request reason'; return false; }); @@ -109,13 +110,14 @@ describe('MetadataManager', () => { it('resolves without errors', async () => { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); - catalogMock.getRecord.mockResolvedValue(createRecord()); - validationManagerMock.validateUpdateStatus.mockResolvedValue(true); - catalogMock.changeStatus.mockResolvedValue(payload); + const record = createRecord(); + catalogMock.getRecord.mockResolvedValue(record); + validationManagerMock.isRecordAbsentFromExtractable.mockResolvedValue(true); + catalogMock.changeStatus.mockResolvedValue(record); const response = await metadataManager.updateStatus(identifier, payload); - expect(response).toMatchObject(payload); + expect(response).toMatchObject(record); }); it(`rejects if update's validation failed with non-existing record`, async () => { @@ -154,7 +156,8 @@ describe('MetadataManager', () => { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); catalogMock.getRecord.mockResolvedValue(createRecord()); - validationManagerMock.validateUpdateStatus.mockResolvedValue(true); + + validationManagerMock.isRecordAbsentFromExtractable.mockResolvedValue(true); catalogMock.changeStatus.mockRejectedValue(new Error('catalog service is not available')); const response = metadataManager.updateStatus(identifier, payload); @@ -166,7 +169,7 @@ describe('MetadataManager', () => { const identifier = faker.string.uuid(); const payload = createUpdateStatusPayload(); catalogMock.getRecord.mockResolvedValue(createRecord()); - validationManagerMock.validateUpdateStatus.mockImplementation(async (rec, ref) => { + validationManagerMock.isRecordAbsentFromExtractable.mockImplementation(async (_id, ref) => { ref.outFailedReason = 'conflict reason'; return false; }); diff --git a/tests/unit/validator/validationManager.spec.ts b/tests/unit/validator/validationManager.spec.ts index e40e81c..793b4e0 100644 --- a/tests/unit/validator/validationManager.spec.ts +++ b/tests/unit/validator/validationManager.spec.ts @@ -500,7 +500,7 @@ describe('ValidationManager', () => { expect(refReason.outFailedReason).toBe(ERROR_METADATA_PRODUCT_NAME_UNIQUE); }); - it('returns false when extractable management is enabled and record exists in extractable', async () => { + it('returns false and sets reason when tileset polygon extraction fails', async () => { const identifier = faker.string.uuid(); const payload = createUpdatePayload(); const record = createRecord(); @@ -509,18 +509,31 @@ describe('ValidationManager', () => { catalogMock.getRecord.mockResolvedValue(record); lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); providerMock.getFile.mockResolvedValue(getTileset()); - extractableMock.isExtractableRecordExists.mockResolvedValue(true); + + const polygonSpy = jest + .spyOn(validationManager as unknown as { getTilesetModelPolygon: (fileContent: string, failedReason: FailedReason) => Polygon | undefined }, 'getTilesetModelPolygon') + .mockImplementation((_fileContent: string, failedReason: FailedReason) => { + failedReason.outFailedReason = 'tileset error'; + return undefined; + }); const refReason: FailedReason = { outFailedReason: '' }; const response = await validationManager.validateUpdate(identifier, payload, refReason); - expect(extractableMock.isExtractableRecordExists).toHaveBeenCalledWith(record.producerName); - expect(response).toStrictEqual([false, true]); - expect(refReason.outFailedReason).toBe(ERROR_METADATA_PRODUCT_NAME_CONFLICT); + expect(response).toBe(false); + expect(refReason.outFailedReason).toBe('tileset error'); + + polygonSpy.mockRestore(); }); + }); - it('returns true when extractable management is disabled even if record exists in extractable', async () => { - configMock.get.mockReturnValue(false); + describe('isRecordAbsentFromExtractable', () => { + it('returns true when extractable management is disabled', async () => { + const record = createRecord(); + configMock.get.mockImplementation((key: string) => { + if (key === 'enableServices.extractable') return false; + return 50; + }); validationManager = new ValidationManager( configMock, @@ -532,69 +545,38 @@ describe('ValidationManager', () => { providerMock ); - const identifier = faker.string.uuid(); - const payload = createUpdatePayload(); - const record = createRecord(); - - catalogMock.findRecords.mockResolvedValue([]); - catalogMock.getRecord.mockResolvedValue(record); - lookupTablesMock.getClassifications.mockResolvedValue([payload.classification]); - providerMock.getFile.mockResolvedValue(getTileset()); - extractableMock.isExtractableRecordExists.mockResolvedValue(true); - const refReason: FailedReason = { outFailedReason: '' }; - const response = await validationManager.validateUpdate(identifier, payload, refReason); + const result = await validationManager.isRecordAbsentFromExtractable(record, refReason); + + expect(result).toBe(true); expect(extractableMock.isExtractableRecordExists).not.toHaveBeenCalled(); - expect(response).toBe(true); }); - }); - describe('validateUpdateStatus', () => { - it('returns true when valid', async () => { + it('returns true when record does not exist in extractable', async () => { const record = createRecord(); + catalogMock.getRecord.mockResolvedValue(record); extractableMock.isExtractableRecordExists.mockResolvedValue(false); const refReason: FailedReason = { outFailedReason: '' }; - const response = await validationManager.validateUpdateStatus(record, refReason); - expect(extractableMock.isExtractableRecordExists).toHaveBeenCalledWith(record.producerName); - expect(response).toBe(true); - }); + const result = await validationManager.isRecordAbsentFromExtractable(record, refReason); - it('returns false when extractable exists', async () => { - const record = createRecord(); - extractableMock.isExtractableRecordExists.mockResolvedValue(true); - - const refReason: FailedReason = { outFailedReason: '' }; - const response = await validationManager.validateUpdateStatus(record, refReason); - - expect(extractableMock.isExtractableRecordExists).toHaveBeenCalledWith(record.producerName); - expect(response).toBe(false); - expect(refReason.outFailedReason).toBe(ERROR_METADATA_PRODUCT_NAME_CONFLICT); + expect(result).toBe(true); + expect(extractableMock.isExtractableRecordExists).toHaveBeenCalledWith(record.productName); }); - it('returns true when disabled', async () => { - configMock.get.mockReturnValue(false); - - validationManager = new ValidationManager( - configMock, - jsLogger({ enabled: false }), - trace.getTracer('testTracer'), - lookupTablesMock as never, - catalogMock as never, - extractableMock as never, - providerMock - ); - + it('returns false and sets reason when record exists in extractable', async () => { const record = createRecord(); + catalogMock.getRecord.mockResolvedValue(record); extractableMock.isExtractableRecordExists.mockResolvedValue(true); const refReason: FailedReason = { outFailedReason: '' }; - const response = await validationManager.validateUpdateStatus(record, refReason); - expect(extractableMock.isExtractableRecordExists).not.toHaveBeenCalled(); - expect(response).toBe(true); + const result = await validationManager.isRecordAbsentFromExtractable(record, refReason); + + expect(result).toBe(false); + expect(refReason.outFailedReason).toBe(ERROR_METADATA_PRODUCT_NAME_CONFLICT); }); }); From 19633a0bfdd31dc836c5c65891929732b731e463 Mon Sep 17 00:00:00 2001 From: liran Date: Wed, 25 Feb 2026 18:12:17 +0200 Subject: [PATCH 4/5] chore: asaf comments --- config/custom-environment-variables.json | 9 +++------ config/default.json | 6 +----- helm/templates/configmap.yaml | 2 +- helm/values.yaml | 7 ++++++- src/validator/validationManager.ts | 2 +- tests/integration/metadata/metadataController.spec.ts | 2 +- tests/unit/validator/validationManager.spec.ts | 2 +- 7 files changed, 14 insertions(+), 16 deletions(-) diff --git a/config/custom-environment-variables.json b/config/custom-environment-variables.json index b334488..8a691a0 100644 --- a/config/custom-environment-variables.json +++ b/config/custom-environment-variables.json @@ -78,11 +78,8 @@ "__format": "number" } }, - "enableServices": { - "extractable": { - "__name": "ENABLE_EXTRACTABLE_MANAGEMENT", - "__format": "boolean" - - } + "isExtractableLogicEnabled": { + "__name": "ENABLE_EXTRACTABLE_MANAGEMENT", + "__format": "boolean" } } diff --git a/config/default.json b/config/default.json index 18d07d1..3ad3532 100644 --- a/config/default.json +++ b/config/default.json @@ -60,9 +60,5 @@ "sslEnabled": false, "maxAttempts": 3 }, - "enableServices": { - "extractable": { - "extractableManagement": true - } - } + "isExtractableLogicEnabled": true } diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index 0a66462..fbd71f0 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -30,7 +30,7 @@ data: STORE_TRIGGER_URL: {{ .Values.env.storeTrigger.url | default (printf "http://%s-store-trigger" .Release.Name) }} CATALOG_URL: {{ .Values.env.catalog.url | default (printf "http://%s-catalog" .Release.Name) }} EXTRACTABLE_URL: {{ .Values.env.extractable.url | default (printf "http://%s-extractable" .Release.Name) }} - ENABLE_EXTRACTABLE_MANAGEMENT: {{ .Values.env.enableServices.extractable | quote }} + ENABLE_EXTRACTABLE_MANAGEMENT: {{ .Values.env.isExtractableLogicEnabled | quote }} LOOKUP_TABLES_URL: {{ .Values.validations.lookupTables.url | quote }} LOOKUP_TABLES_SUB_URL: {{ .Values.validations.lookupTables.subUrl | quote }} {{ if eq $providers.source "NFS" }} diff --git a/helm/values.yaml b/helm/values.yaml index 6cfe963..b8b1932 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -28,7 +28,9 @@ global: name: '' pv_path: '' sub_path: '' - + + isExtractableLogicEnabled: false + cloudProvider: dockerRegistryUrl: flavor: @@ -127,6 +129,9 @@ env: url: catalog: url: + isExtractableLogicEnabled: true + extractable: + url: resources: enabled: true diff --git a/src/validator/validationManager.ts b/src/validator/validationManager.ts index b2997cc..354cd35 100644 --- a/src/validator/validationManager.ts +++ b/src/validator/validationManager.ts @@ -54,7 +54,7 @@ export class ValidationManager { }; this.limit = this.config.get('validation.percentageLimit'); - this.isExtractableManagementEnabled = this.config.get('enableServices.extractable'); + this.isExtractableManagementEnabled = this.config.get('isExtractableLogicEnabled'); } @withSpanAsyncV4 diff --git a/tests/integration/metadata/metadataController.spec.ts b/tests/integration/metadata/metadataController.spec.ts index 8f937f7..8c1f89b 100644 --- a/tests/integration/metadata/metadataController.spec.ts +++ b/tests/integration/metadata/metadataController.spec.ts @@ -519,7 +519,7 @@ describe('MetadataController', function () { const disabledConfig: IConfig = { ...config, get: (setting: string): T => { - if (setting === 'enableServices.extractable') { + if (setting === 'isExtractableLogicEnabled') { return false as T; } return config.get(setting); diff --git a/tests/unit/validator/validationManager.spec.ts b/tests/unit/validator/validationManager.spec.ts index 793b4e0..2a5e341 100644 --- a/tests/unit/validator/validationManager.spec.ts +++ b/tests/unit/validator/validationManager.spec.ts @@ -531,7 +531,7 @@ describe('ValidationManager', () => { it('returns true when extractable management is disabled', async () => { const record = createRecord(); configMock.get.mockImplementation((key: string) => { - if (key === 'enableServices.extractable') return false; + if (key === 'isExtractableLogicEnabled') return false; return 50; }); From 82655a130cbe7d03a394122ac262eee51feeefea Mon Sep 17 00:00:00 2001 From: liran Date: Wed, 25 Feb 2026 20:24:50 +0200 Subject: [PATCH 5/5] refactor: asaf changes --- helm/templates/configmap.yaml | 2 +- helm/values.yaml | 3 +- .../metadata/metadataController.spec.ts | 2 +- .../unit/validator/validationManager.spec.ts | 32 +++++++++++++++++-- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/helm/templates/configmap.yaml b/helm/templates/configmap.yaml index fbd71f0..faa2583 100644 --- a/helm/templates/configmap.yaml +++ b/helm/templates/configmap.yaml @@ -30,7 +30,7 @@ data: STORE_TRIGGER_URL: {{ .Values.env.storeTrigger.url | default (printf "http://%s-store-trigger" .Release.Name) }} CATALOG_URL: {{ .Values.env.catalog.url | default (printf "http://%s-catalog" .Release.Name) }} EXTRACTABLE_URL: {{ .Values.env.extractable.url | default (printf "http://%s-extractable" .Release.Name) }} - ENABLE_EXTRACTABLE_MANAGEMENT: {{ .Values.env.isExtractableLogicEnabled | quote }} + ENABLE_EXTRACTABLE_MANAGEMENT: {{ .Values.global.isExtractableLogicEnabled | quote }} LOOKUP_TABLES_URL: {{ .Values.validations.lookupTables.url | quote }} LOOKUP_TABLES_SUB_URL: {{ .Values.validations.lookupTables.subUrl | quote }} {{ if eq $providers.source "NFS" }} diff --git a/helm/values.yaml b/helm/values.yaml index b8b1932..5607a78 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -29,7 +29,7 @@ global: pv_path: '' sub_path: '' - isExtractableLogicEnabled: false + isExtractableLogicEnabled: cloudProvider: dockerRegistryUrl: @@ -129,7 +129,6 @@ env: url: catalog: url: - isExtractableLogicEnabled: true extractable: url: diff --git a/tests/integration/metadata/metadataController.spec.ts b/tests/integration/metadata/metadataController.spec.ts index 8c1f89b..413f10f 100644 --- a/tests/integration/metadata/metadataController.spec.ts +++ b/tests/integration/metadata/metadataController.spec.ts @@ -402,7 +402,7 @@ describe('MetadataController', function () { const disabledConfig: IConfig = { ...config, get: (setting: string): T => { - if (setting === 'enableServices.extractable') { + if (setting === 'isExtractableLogicEnabled') { return false as T; } return config.get(setting); diff --git a/tests/unit/validator/validationManager.spec.ts b/tests/unit/validator/validationManager.spec.ts index 2a5e341..a728501 100644 --- a/tests/unit/validator/validationManager.spec.ts +++ b/tests/unit/validator/validationManager.spec.ts @@ -555,7 +555,21 @@ describe('ValidationManager', () => { it('returns true when record does not exist in extractable', async () => { const record = createRecord(); - catalogMock.getRecord.mockResolvedValue(record); + configMock.get.mockImplementation((key: string) => { + if (key === 'isExtractableLogicEnabled') return true; + if (key === 'validation.percentageLimit') return 50; + return 50; + }); + + validationManager = new ValidationManager( + configMock, + jsLogger({ enabled: false }), + trace.getTracer('testTracer'), + lookupTablesMock as never, + catalogMock as never, + extractableMock as never, + providerMock + ); extractableMock.isExtractableRecordExists.mockResolvedValue(false); const refReason: FailedReason = { outFailedReason: '' }; @@ -568,7 +582,21 @@ describe('ValidationManager', () => { it('returns false and sets reason when record exists in extractable', async () => { const record = createRecord(); - catalogMock.getRecord.mockResolvedValue(record); + configMock.get.mockImplementation((key: string) => { + if (key === 'isExtractableLogicEnabled') return true; + if (key === 'validation.percentageLimit') return 50; + return 50; + }); + + validationManager = new ValidationManager( + configMock, + jsLogger({ enabled: false }), + trace.getTracer('testTracer'), + lookupTablesMock as never, + catalogMock as never, + extractableMock as never, + providerMock + ); extractableMock.isExtractableRecordExists.mockResolvedValue(true); const refReason: FailedReason = { outFailedReason: '' };