From 186cadb2bc2a66228c09b1a679801b575c7127b3 Mon Sep 17 00:00:00 2001 From: Kevin Date: Wed, 21 Feb 2024 13:56:50 +0100 Subject: [PATCH] fix(Features): Makes the upload of features (shapefile and csv) asynchronous --- ...8099064649-AddFeatureShapefileApiEvents.ts | 32 ++++ .../geo-feature-tags.service.ts | 7 +- .../feature-shapefile-upload-api-events.ts | 37 ++++ .../adapters/geo-features-adapters.module.ts | 16 ++ .../geo-features/geo-features-request-info.ts | 1 + .../geo-features/geo-features.module.ts | 8 +- .../geo-features/geo-features.service.ts | 171 +++++++++++++++--- ...events.ts => feature-csv-import.events.ts} | 24 +-- .../import/features-amounts-upload.service.ts | 94 ++++++++-- .../feature-shapefile-import-events-port.ts | 13 ++ .../block-guard/marxan-block-guard.service.ts | 6 + .../marxan-project-checker.service.ts | 23 +++ .../project-checker/project-checker.module.ts | 5 +- .../project-checker.service.spec.ts | 9 + .../project-checker.service.ts | 4 + .../projects.project-features.controller.ts | 68 +++---- api/apps/api/src/utils/acl.utils.ts | 39 +--- api/apps/api/test/geo-features.e2e-spec.ts | 74 +++++++- .../geo-features/geo-feature-tags.e2e-spec.ts | 2 +- .../geo-features/geo-feature-tags.fixtures.ts | 9 +- .../update-project-feature-name.e2e-spec.ts | 39 +++- .../project-tags.e2e-spec.ts | 70 ++++++- .../project-tags.fixtures.ts | 9 +- .../import-files/no_features_upload.csv | 8 +- .../upload-feature-csv.e2e-spec.ts | 72 ++++++-- .../upload-feature/upload-feature.e2e-spec.ts | 42 +++-- .../upload-feature/upload-feature.fixtures.ts | 140 ++++++++++---- .../utils/project-checker.service-fake.ts | 18 ++ .../test/utils/test-client/seed/features.ts | 21 ++- .../wait-for-feature-to-be-ready.utils.ts | 30 +++ .../test/integration/cloning/fixtures.ts | 2 +- .../api-events/src/api-event-kinds.enum.ts | 3 + 32 files changed, 862 insertions(+), 234 deletions(-) create mode 100644 api/apps/api/src/migrations/api/1708099064649-AddFeatureShapefileApiEvents.ts create mode 100644 api/apps/api/src/modules/geo-features/adapters/feature-shapefile-upload-api-events.ts create mode 100644 api/apps/api/src/modules/geo-features/adapters/geo-features-adapters.module.ts rename api/apps/api/src/modules/geo-features/import/{feature-import.events.ts => feature-csv-import.events.ts} (56%) create mode 100644 api/apps/api/src/modules/geo-features/ports/feature-shapefile-import-events-port.ts diff --git a/api/apps/api/src/migrations/api/1708099064649-AddFeatureShapefileApiEvents.ts b/api/apps/api/src/migrations/api/1708099064649-AddFeatureShapefileApiEvents.ts new file mode 100644 index 0000000000..8d96af5173 --- /dev/null +++ b/api/apps/api/src/migrations/api/1708099064649-AddFeatureShapefileApiEvents.ts @@ -0,0 +1,32 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +export class AddFeatureShapefileApiEvents1708099064649 + implements MigrationInterface +{ + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(` + INSERT INTO api_event_kinds (id) values + ('features.shapefile.import.submitted/v1/alpha'), + ('features.shapefile.import.finished/v1/alpha'), + ('features.shapefile.import.failed/v1/alpha') + `); + + await queryRunner.query(` + UPDATE features SET creation_status = 'created' + WHERE creation_status = 'done'; + `); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query(` + DELETE FROM api_event_kinds WHERE id IN ( + 'features.shapefile.import.submitted/v1/alpha', + 'features.shapefile.import.finished/v1/alpha', + 'features.shapefile.import.failed/v1/alpha') + `); + await queryRunner.query(` + UPDATE features SET creation_status = 'done' + WHERE creation_status = 'created'; + `); + } +} diff --git a/api/apps/api/src/modules/geo-feature-tags/geo-feature-tags.service.ts b/api/apps/api/src/modules/geo-feature-tags/geo-feature-tags.service.ts index 018836d141..f9de1e149c 100644 --- a/api/apps/api/src/modules/geo-feature-tags/geo-feature-tags.service.ts +++ b/api/apps/api/src/modules/geo-feature-tags/geo-feature-tags.service.ts @@ -12,6 +12,7 @@ import { projectNotVisible, } from '@marxan-api/modules/projects/projects.service'; import { UpdateProjectTagDTO } from '@marxan-api/modules/projects/dto/update-project-tag.dto'; +import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; export const featureNotFoundWithinProject = Symbol( 'feature not found within project', @@ -57,7 +58,11 @@ export class GeoFeatureTagsService { .createQueryBuilder('feature_tag') .select('feature_tag.tag', 'tag') .distinct(true) - .where({ projectId }); + .innerJoin(GeoFeature, 'f', 'feature_tag.featureId = f.id') + .where( + 'feature_tag.projectId = :projectId and f.creationStatus = :creationStatus', + { projectId, creationStatus: JobStatus.created }, + ); /** @debt * Even tho this is a highly focused endpoint returning only a list of, it should conform to JSON:API standards as well diff --git a/api/apps/api/src/modules/geo-features/adapters/feature-shapefile-upload-api-events.ts b/api/apps/api/src/modules/geo-features/adapters/feature-shapefile-upload-api-events.ts new file mode 100644 index 0000000000..6c271d6451 --- /dev/null +++ b/api/apps/api/src/modules/geo-features/adapters/feature-shapefile-upload-api-events.ts @@ -0,0 +1,37 @@ +import { Injectable } from '@nestjs/common'; +import { API_EVENT_KINDS } from '@marxan/api-events'; +import { ApiEventsService } from '@marxan-api/modules/api-events'; +import { + FeatureShapefileImportEventsPort, + FeatureShapefileImportState, +} from '@marxan-api/modules/geo-features/ports/feature-shapefile-import-events-port'; + +@Injectable() +export class FeatureShapefileImportApiEvents + extends ApiEventsService + implements FeatureShapefileImportEventsPort +{ + private readonly eventsMap: Record< + FeatureShapefileImportState, + API_EVENT_KINDS + > = { + [FeatureShapefileImportState.FeatureShapefileSubmitted]: + API_EVENT_KINDS.features__shapefile__import__submitted__v1__alpha, + [FeatureShapefileImportState.FeatureShapefileFinished]: + API_EVENT_KINDS.features__shapefile__import__finished__v1__alpha, + [FeatureShapefileImportState.FeatureShapefileFailed]: + API_EVENT_KINDS.features__shapefile__import__failed__v1__alpha, + }; + + async event( + projectId: string, + state: FeatureShapefileImportState, + context?: Record, + ): Promise { + await this.create({ + data: context ?? {}, + topic: projectId, + kind: this.eventsMap[state], + }); + } +} diff --git a/api/apps/api/src/modules/geo-features/adapters/geo-features-adapters.module.ts b/api/apps/api/src/modules/geo-features/adapters/geo-features-adapters.module.ts new file mode 100644 index 0000000000..9ec7628fa3 --- /dev/null +++ b/api/apps/api/src/modules/geo-features/adapters/geo-features-adapters.module.ts @@ -0,0 +1,16 @@ +import { Module } from '@nestjs/common'; +import { ApiEventsModule } from '../../api-events'; +import { FeatureShapefileImportEventsPort } from '@marxan-api/modules/geo-features/ports/feature-shapefile-import-events-port'; +import { FeatureShapefileImportApiEvents } from '@marxan-api/modules/geo-features/adapters/feature-shapefile-upload-api-events'; + +@Module({ + imports: [ApiEventsModule], + providers: [ + { + provide: FeatureShapefileImportEventsPort, + useClass: FeatureShapefileImportApiEvents, + }, + ], + exports: [FeatureShapefileImportEventsPort], +}) +export class GeoFeaturesAdaptersModule {} diff --git a/api/apps/api/src/modules/geo-features/geo-features-request-info.ts b/api/apps/api/src/modules/geo-features/geo-features-request-info.ts index 9999b2d418..13a09f2d76 100644 --- a/api/apps/api/src/modules/geo-features/geo-features-request-info.ts +++ b/api/apps/api/src/modules/geo-features/geo-features-request-info.ts @@ -7,5 +7,6 @@ export interface GeoFeaturesRequestInfo extends AppInfoDTO { projectId?: string; bbox?: BBox; ids?: string[]; + includeInProgress?: boolean; }; } diff --git a/api/apps/api/src/modules/geo-features/geo-features.module.ts b/api/apps/api/src/modules/geo-features/geo-features.module.ts index 6e53777506..e577d42cee 100644 --- a/api/apps/api/src/modules/geo-features/geo-features.module.ts +++ b/api/apps/api/src/modules/geo-features/geo-features.module.ts @@ -23,8 +23,10 @@ import { FeatureAmountUploadRegistry } from '@marxan-api/modules/geo-features/im import { UploadedFeatureAmount } from '@marxan-api/modules/geo-features/import/features-amounts-data.api.entity'; import { FeatureAmountUploadService } from '@marxan-api/modules/geo-features/import/features-amounts-upload.service'; import { ApiEventsModule } from '@marxan-api/modules/api-events'; -import { FeatureImportEventsService } from '@marxan-api/modules/geo-features/import/feature-import.events'; import { FeatureAmountsPerPlanningUnitModule } from '@marxan/feature-amounts-per-planning-unit'; +import { ShapefilesModule } from '@marxan/shapefile-converter'; +import { GeoFeaturesAdaptersModule } from '@marxan-api/modules/geo-features/adapters/geo-features-adapters.module'; +import { FeatureCSVImportEventsService } from '@marxan-api/modules/geo-features/import/feature-csv-import.events'; @Module({ imports: [ @@ -45,7 +47,9 @@ import { FeatureAmountsPerPlanningUnitModule } from '@marxan/feature-amounts-per forwardRef(() => ScenarioFeaturesModule), ApiEventsModule, GeoFeatureTagsModule, + ShapefilesModule, FeatureAmountsPerPlanningUnitModule.for(DbConnections.geoprocessingDB), + GeoFeaturesAdaptersModule, ], providers: [ GeoFeaturesService, @@ -54,7 +58,7 @@ import { FeatureAmountsPerPlanningUnitModule } from '@marxan/feature-amounts-per GeoFeaturePropertySetService, ProxyService, FeatureAmountUploadService, - FeatureImportEventsService, + FeatureCSVImportEventsService, ], controllers: [GeoFeaturesController], exports: [ diff --git a/api/apps/api/src/modules/geo-features/geo-features.service.ts b/api/apps/api/src/modules/geo-features/geo-features.service.ts index 6a103ce238..cb1fe903e5 100644 --- a/api/apps/api/src/modules/geo-features/geo-features.service.ts +++ b/api/apps/api/src/modules/geo-features/geo-features.service.ts @@ -1,4 +1,9 @@ -import { forwardRef, Inject, Injectable } from '@nestjs/common'; +import { + BadRequestException, + forwardRef, + Inject, + Injectable, +} from '@nestjs/common'; import { InjectDataSource, InjectEntityManager, @@ -6,6 +11,7 @@ import { } from '@nestjs/typeorm'; import { DeepReadonly } from 'utility-types'; import { + Brackets, DataSource, EntityManager, Not, @@ -16,7 +22,7 @@ import { GeoFeatureGeometry, GeometrySource } from '@marxan/geofeatures'; import { geoFeatureResource } from './geo-feature.geo.entity'; import { GeoFeatureSetSpecification } from './dto/geo-feature-set-specification.dto'; -import { BBox, Geometry } from 'geojson'; +import { BBox, GeoJSON, Geometry } from 'geojson'; import { AppBaseService, JSONAPISerializerConfig, @@ -32,7 +38,11 @@ import { DbConnections } from '@marxan-api/ormconfig.connections'; import { v4 } from 'uuid'; import { UploadShapefileDTO } from '../projects/dto/upload-shapefile.dto'; import { GeoFeaturesRequestInfo } from './geo-features-request-info'; -import { antimeridianBbox, nominatim2bbox } from '@marxan/utils/geo'; +import { + antimeridianBbox, + isFeatureCollection, + nominatim2bbox, +} from '@marxan/utils/geo'; import { Either, left, right } from 'fp-ts/lib/Either'; import { ProjectAclService } from '@marxan-api/modules/access-control/projects-acl/project-acl.service'; import { @@ -54,6 +64,12 @@ import { } from '@marxan/feature-amounts-per-planning-unit'; import { ComputeFeatureAmountPerPlanningUnit } from '@marxan/feature-amounts-per-planning-unit/feature-amounts-per-planning-units.service'; import { CHUNK_SIZE_FOR_BATCH_APIDB_OPERATIONS } from '@marxan-api/utils/chunk-size-for-batch-apidb-operations'; +import { ensureShapefileHasRequiredFiles } from '@marxan-api/utils/file-uploads.utils'; +import { ShapefileService } from '@marxan/shapefile-converter'; +import { + FeatureShapefileImportEventsPort, + FeatureShapefileImportState, +} from '@marxan-api/modules/geo-features/ports/feature-shapefile-import-events-port'; const geoFeatureFilterKeyNames = [ 'featureClassName', @@ -91,6 +107,8 @@ export const missingPuidColumnInFeatureAmountCsvUpload = Symbol( export const unknownPuidsInFeatureAmountCsvUpload = Symbol( 'there are unknown PUids in feature amount csv upload', ); +export const onlyFeatureCollectionShapefileSupported = + 'only feature collection shapefile supported'; export type FindResult = { data: (Partial | undefined)[]; @@ -128,6 +146,8 @@ export class GeoFeaturesService extends AppBaseService< private readonly projectAclService: ProjectAclService, private readonly featureAmountUploads: FeatureAmountUploadService, private readonly featureAmountsPerPlanningUnitService: FeatureAmountsPerPlanningUnitService, + private readonly shapefileService: ShapefileService, + private readonly featureShapefileImportEvents: FeatureShapefileImportEventsPort, ) { super( geoFeaturesRepository, @@ -153,6 +173,7 @@ export class GeoFeaturesService extends AppBaseService< 'tag', 'scenarioUsageCount', 'amountRange', + 'creationStatus', ], keyForAttribute: 'camelCase', }; @@ -235,13 +256,37 @@ export class GeoFeaturesService extends AppBaseService< // Only apply narrowing by intersection with project bbox if there are // features falling within said bbox; otherwise return an empty set // by short-circuiting the query. + + // Of all three possible creation status, (fail, running, created) we must also include failure and running (if + // indicated in the request) even if they don't intersect with the bounding box (because in those non created + // states there is no geospatial data in the geoDB to intersect with) + const creationStatusSelection = info?.params?.includeInProgress + ? [JobStatus.failure, JobStatus.running] + : []; if (geoFeaturesWithinProjectBbox?.length > 0) { query.andWhere( - `${this.alias}.id IN (:...geoFeaturesWithinProjectBbox)`, - { geoFeaturesWithinProjectBbox }, + new Brackets((qb) => { + qb.where( + `${this.alias}.id IN (:...geoFeaturesWithinProjectBbox)`, + { + geoFeaturesWithinProjectBbox, + }, + ); + if (creationStatusSelection.length > 0) { + qb.orWhere( + `${this.alias}.creationStatus IN (:...creationStatusSelection)`, + { creationStatusSelection }, + ); + } + }), ); } else { - query.andWhere('false'); + if (creationStatusSelection.length > 0) { + query.andWhere( + `${this.alias}.creationStatus IN (:...creationStatusSelection)`, + { creationStatusSelection }, + ); + } } } @@ -379,10 +424,51 @@ export class GeoFeaturesService extends AppBaseService< return extendedResult; } + public async uploadFeatureFromShapefile( + projectId: string, + dto: UploadShapefileDTO, + shapefile: Express.Multer.File, + ): Promise> { + await ensureShapefileHasRequiredFiles(shapefile); + + let data: GeoJSON; + try { + const geoJSON = await this.shapefileService.transformToGeoJson( + shapefile, + { + allowOverlaps: true, + }, + ); + data = geoJSON.data; + } catch (e) { + throw new BadRequestException(e); + } + + if (!isFeatureCollection(data)) { + return left(onlyFeatureCollectionShapefileSupported); + } + + this.createFeaturesForShapefileAsync(projectId, dto, data.features); + + return right(void 0); + } + + public createFeaturesForShapefileAsync( + projectId: string, + data: UploadShapefileDTO, + featureDatas: Record[], + ) { + try { + setTimeout(async () => { + await this.createFeaturesForShapefile(projectId, data, featureDatas); + }); + } catch (e) {} + } + public async createFeaturesForShapefile( projectId: string, data: UploadShapefileDTO, - features: Record[], + featureDatas: Record[], ): Promise> { /** * @debt Avoid duplicating transaction scaffolding in multiple sites @@ -393,19 +479,29 @@ export class GeoFeaturesService extends AppBaseService< const apiQueryRunner = this.apiDataSource.createQueryRunner(); const geoQueryRunner = this.geoDataSource.createQueryRunner(); + // For shapefile uploads, the feature metadata remains persisted after failure (with creatinStatus failed) and + // everything else is rolled back (tags, feature data) + const geoFeature = await this.createFeature( + apiQueryRunner.manager, + projectId, + data, + ); + await apiQueryRunner.connect(); await geoQueryRunner.connect(); await apiQueryRunner.startTransaction(); await geoQueryRunner.startTransaction(); - let geoFeature: GeoFeature; try { - // Create single row in features - geoFeature = await this.createFeature( - apiQueryRunner.manager, + await this.featureShapefileImportEvents.event( projectId, - data, + FeatureShapefileImportState.FeatureShapefileSubmitted, + { + projectId, + id: geoFeature.id, + dto: data, + }, ); //Create Tag if provided @@ -415,14 +511,13 @@ export class GeoFeaturesService extends AppBaseService< geoFeature.id, data.tagName, ); - // Store geometries in features_data table - for (const feature of features) { + for (const featureData of featureDatas) { await this.createFeatureData( geoQueryRunner.manager, geoFeature.id, - feature.geometry, - feature.properties, + featureData.geometry, + featureData.properties, ); } @@ -445,8 +540,17 @@ export class GeoFeaturesService extends AppBaseService< geoQueryRunner.manager, ); + // set status created + geoFeature.creationStatus = JobStatus.created; + await apiQueryRunner.manager.getRepository(GeoFeature).save(geoFeature); + await apiQueryRunner.commitTransaction(); await geoQueryRunner.commitTransaction(); + + await this.featureShapefileImportEvents.event( + projectId, + FeatureShapefileImportState.FeatureShapefileFinished, + ); } catch (err) { await apiQueryRunner.rollbackTransaction(); await geoQueryRunner.rollbackTransaction(); @@ -455,7 +559,18 @@ export class GeoFeaturesService extends AppBaseService< 'An error occurred creating features for shapefile (changes have been rolled back)', String(err), ); - throw err; + + // set status to fail + geoFeature.creationStatus = JobStatus.failure; + await apiQueryRunner.manager.getRepository(GeoFeature).save(geoFeature); + + await this.featureShapefileImportEvents.event( + projectId, + FeatureShapefileImportState.FeatureShapefileFailed, + err, + ); + + return left(err); } finally { // you need to release a queryRunner which was manually instantiated await apiQueryRunner.release(); @@ -509,7 +624,7 @@ export class GeoFeaturesService extends AppBaseService< const feature = await this.geoFeaturesRepository.findOne({ where: { id: featureId }, }); - if (!feature) { + if (!feature || feature.creationStatus !== JobStatus.created) { return left(featureNotFound); } if (!feature.isCustom || !feature.projectId) { @@ -566,7 +681,11 @@ export class GeoFeaturesService extends AppBaseService< const feature = await this.geoFeaturesRepository.findOne({ where: { id: featureId }, }); - if (!feature) { + if ( + !feature || + (feature.creationStatus !== JobStatus.created && + feature.creationStatus !== JobStatus.failure) + ) { return left(featureNotFound); } @@ -802,7 +921,7 @@ export class GeoFeaturesService extends AppBaseService< featureClassName: data.name, description: data.description, projectId, - creationStatus: JobStatus.created, + creationStatus: JobStatus.running, }), ); } @@ -852,11 +971,8 @@ export class GeoFeaturesService extends AppBaseService< userId: string, ): Promise< Either< - | typeof importedFeatureNameAlreadyExist - | typeof unknownPuidsInFeatureAmountCsvUpload - | typeof projectNotFound - | typeof featureDataCannotBeUploadedWithCsv, - GeoFeature[] + typeof projectNotFound | typeof featureDataCannotBeUploadedWithCsv, + void > > { const project = await this.projectRepository.findOne({ @@ -875,11 +991,12 @@ export class GeoFeaturesService extends AppBaseService< return left(featureDataCannotBeUploadedWithCsv); } - return await this.featureAmountUploads.uploadFeatureFromCsv({ + this.featureAmountUploads.uploadFeatureFromCSVAsync( fileBuffer, projectId, userId, - }); + ); + return right(void 0); } private async extendFindAllGeoFeatureWithScenarioUsage( diff --git a/api/apps/api/src/modules/geo-features/import/feature-import.events.ts b/api/apps/api/src/modules/geo-features/import/feature-csv-import.events.ts similarity index 56% rename from api/apps/api/src/modules/geo-features/import/feature-import.events.ts rename to api/apps/api/src/modules/geo-features/import/feature-csv-import.events.ts index 103a4c1b1a..154d4190f4 100644 --- a/api/apps/api/src/modules/geo-features/import/feature-import.events.ts +++ b/api/apps/api/src/modules/geo-features/import/feature-csv-import.events.ts @@ -4,7 +4,7 @@ import { API_EVENT_KINDS } from '@marxan/api-events'; // @todo: Refactor this once we design the uniform solution for events management @Injectable() -export class FeatureImportEventsService { +export class FeatureCSVImportEventsService { eventId!: string; submit = () => API_EVENT_KINDS.features__csv__import__submitted__v1__alpha; @@ -13,25 +13,15 @@ export class FeatureImportEventsService { constructor(private readonly apiEvents: ApiEventsService) {} - async createEvent(data: any) { - this.eventId = await this.apiEvents - .create({ - kind: this.submit(), - topic: data.userId, - }) - .then(({ id }) => id); + async submittedEvent(topic: string, data: any) { + await this.apiEvents.create({ topic, kind: this.submit(), data }); } - async finishEvent() { - await this.apiEvents.update(this.eventId, { - kind: this.finish(), - }); + async finishEvent(topic: string) { + await this.apiEvents.create({ topic, kind: this.finish() }); } - async failEvent(data: any) { - await this.apiEvents.update(this.eventId, { - kind: this.fail(), - data: data, - }); + async failEvent(topic: string, data: any) { + await this.apiEvents.create({ topic, kind: this.fail(), data: data }); } } diff --git a/api/apps/api/src/modules/geo-features/import/features-amounts-upload.service.ts b/api/apps/api/src/modules/geo-features/import/features-amounts-upload.service.ts index d91c534a50..7e59aa0d15 100644 --- a/api/apps/api/src/modules/geo-features/import/features-amounts-upload.service.ts +++ b/api/apps/api/src/modules/geo-features/import/features-amounts-upload.service.ts @@ -8,22 +8,28 @@ import { import { DbConnections } from '@marxan-api/ormconfig.connections'; import { DataSource, EntityManager, QueryRunner } from 'typeorm'; import { InjectDataSource, InjectEntityManager } from '@nestjs/typeorm'; -import { featureAmountCsvParser } from '@marxan-api/modules/geo-features/import/csv.parser'; +import { + duplicateHeadersInFeatureAmountCsvUpload, + duplicatePuidsInFeatureAmountCsvUpload, + featureAmountCsvParser, + noFeaturesFoundInInFeatureAmountCsvUpload, +} from '@marxan-api/modules/geo-features/import/csv.parser'; import { FeatureAmountCSVDto } from '@marxan-api/modules/geo-features/dto/feature-amount-csv.dto'; import { FeatureAmountUploadRegistry } from '@marxan-api/modules/geo-features/import/features-amounts-upload-registry.api.entity'; import { GeoFeaturesService, importedFeatureNameAlreadyExist, + missingPuidColumnInFeatureAmountCsvUpload, unknownPuidsInFeatureAmountCsvUpload, } from '@marxan-api/modules/geo-features/geo-features.service'; import { isLeft, left, Right, right } from 'fp-ts/Either'; -import { FeatureImportEventsService } from '@marxan-api/modules/geo-features/import/feature-import.events'; import { GeoFeature } from '@marxan-api/modules/geo-features/geo-feature.api.entity'; import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; import { chunk } from 'lodash'; import { Left } from 'fp-ts/lib/Either'; import { CHUNK_SIZE_FOR_BATCH_APIDB_OPERATIONS } from '@marxan-api/utils/chunk-size-for-batch-apidb-operations'; import { UploadedFeatureAmount } from '@marxan-api/modules/geo-features/import/features-amounts-data.api.entity'; +import { FeatureCSVImportEventsService } from '@marxan-api/modules/geo-features/import/feature-csv-import.events'; @Injectable() export class FeatureAmountUploadService { @@ -38,11 +44,29 @@ export class FeatureAmountUploadService { private readonly geoEntityManager: EntityManager, @InjectEntityManager(DbConnections.default) private readonly apiEntityManager: EntityManager, - private readonly events: FeatureImportEventsService, + private readonly events: FeatureCSVImportEventsService, @Inject(forwardRef(() => GeoFeaturesService)) private readonly geoFeaturesService: GeoFeaturesService, ) {} + uploadFeatureFromCSVAsync( + fileBuffer: Buffer, + projectId: string, + userId: string, + ) { + setTimeout(async () => { + try { + await this.uploadFeatureFromCsv({ + fileBuffer, + projectId, + userId, + }); + } catch (e) { + this.logger.error(e); + } + }); + } + async uploadFeatureFromCsv(data: { fileBuffer: Buffer; projectId: string; @@ -62,7 +86,7 @@ export class FeatureAmountUploadService { this.logger.log( `Starting process of parsing csv file and saving amount data to temporary storage`, ); - await this.events.createEvent(data); + // saving feature data to temporary table const featuresRegistry: Left | Right = await this.saveCsvToRegistry(data, apiQueryRunner); @@ -70,20 +94,21 @@ export class FeatureAmountUploadService { if (isLeft(featuresRegistry)) { // Some validations done while parsing csv in stream return nested Left object when being rejected as left // Todo: make validations during csv parse more unified - if (featuresRegistry.left.left) { - return featuresRegistry.left; - } - return featuresRegistry; + const errorCode = featuresRegistry.left.left ?? featuresRegistry.left; + + throw this.csvErrorMapper(errorCode); } // Saving new features to apiDB 'features' table this.logger.log(`Saving new features to (apiBD).features table...`); + //features entities are created here newFeaturesFromCsvUpload = await this.saveNewFeaturesFromCsvUpload( apiQueryRunner, featuresRegistry.right.id, data.projectId, ); this.logger.log(`New features saved in (apiBD).features table`); + // Saving new features amounts and geoms to geoDB 'features_amounts' table this.logger.log( `Starting the process of saving new features amounts and geoms to (geoDB).features_amounts table`, @@ -115,25 +140,36 @@ export class FeatureAmountUploadService { geoQueryRunner.manager, ); + for (const feature of newFeaturesFromCsvUpload) { + await apiQueryRunner.manager + .getRepository(GeoFeature) + .update({ id: feature.id }, { creationStatus: JobStatus.created }); + } + this.logger.log(`Csv file upload process finished successfully`); // Committing transaction await apiQueryRunner.commitTransaction(); await geoQueryRunner.commitTransaction(); + + // This is done, for now, in order to avoid unnecessary polling from FE + await this.events.submittedEvent(data.projectId, data); + await this.events.finishEvent(data.projectId); } catch (err) { - await this.events.failEvent(err); await apiQueryRunner.rollbackTransaction(); await geoQueryRunner.rollbackTransaction(); + // This is done, for now, in order to avoid unnecessary polling from FE + await this.events.submittedEvent(data.projectId, data); + await this.events.failEvent(data.projectId, err); this.logger.error( 'An error occurred creating features and saving amounts from csv (changes have been rolled back)', String(err), ); - throw new BadRequestException(err); + throw err; } finally { // you need to release a queryRunner which was manually instantiated await apiQueryRunner.release(); await geoQueryRunner.release(); - await this.events.finishEvent(); } return right(newFeaturesFromCsvUpload); } @@ -180,7 +216,7 @@ export class FeatureAmountUploadService { if (isLeft(e)) { return left(e); } - throw e; + throw new BadRequestException(e); } } @@ -215,6 +251,38 @@ export class FeatureAmountUploadService { return newUpload; } + private csvErrorMapper( + error: + | typeof importedFeatureNameAlreadyExist + | typeof unknownPuidsInFeatureAmountCsvUpload + | typeof missingPuidColumnInFeatureAmountCsvUpload + | typeof duplicatePuidsInFeatureAmountCsvUpload + | typeof duplicateHeadersInFeatureAmountCsvUpload + | typeof noFeaturesFoundInInFeatureAmountCsvUpload + | Error, + ) { + switch (error) { + case importedFeatureNameAlreadyExist: + return new BadRequestException('Imported Features already present'); + case unknownPuidsInFeatureAmountCsvUpload: + return new BadRequestException('Unknown PUIDs'); + case missingPuidColumnInFeatureAmountCsvUpload: + return new BadRequestException('Missing PUID column'); + case noFeaturesFoundInInFeatureAmountCsvUpload: + return new BadRequestException( + 'No features found in feature amount CSV upload', + ); + case duplicateHeadersInFeatureAmountCsvUpload: + return new BadRequestException( + 'Duplicate headers in feature amount CSV upload', + ); + case duplicatePuidsInFeatureAmountCsvUpload: + return new BadRequestException( + 'Duplicate PUIDs in feature amount CSV upload', + ); + } + } + private async saveNewFeaturesFromCsvUpload( queryRunner: QueryRunner, uploadId: string, @@ -232,7 +300,7 @@ export class FeatureAmountUploadService { return { featureClassName: feature.feature_name, projectId: projectId, - creationStatus: JobStatus.created, + creationStatus: JobStatus.running, isLegacy: true, }; }); diff --git a/api/apps/api/src/modules/geo-features/ports/feature-shapefile-import-events-port.ts b/api/apps/api/src/modules/geo-features/ports/feature-shapefile-import-events-port.ts new file mode 100644 index 0000000000..b6823b5d81 --- /dev/null +++ b/api/apps/api/src/modules/geo-features/ports/feature-shapefile-import-events-port.ts @@ -0,0 +1,13 @@ +export enum FeatureShapefileImportState { + FeatureShapefileSubmitted = 'feature-shapefile-upload-submitted', + FeatureShapefileFinished = 'feature-shapefile-upload-finished', + FeatureShapefileFailed = 'feature-shapefile-upload-failed', +} + +export abstract class FeatureShapefileImportEventsPort { + abstract event( + projectId: string, + state: FeatureShapefileImportState, + context?: Record | Error, + ): Promise; +} diff --git a/api/apps/api/src/modules/projects/block-guard/marxan-block-guard.service.ts b/api/apps/api/src/modules/projects/block-guard/marxan-block-guard.service.ts index 29b232f59a..9d7ac5048b 100644 --- a/api/apps/api/src/modules/projects/block-guard/marxan-block-guard.service.ts +++ b/api/apps/api/src/modules/projects/block-guard/marxan-block-guard.service.ts @@ -41,12 +41,14 @@ export class MarxanBlockGuard implements BlockGuard { hasPendingImports, hasPendingBlmCalibration, hasPendingMarxanRun, + hasPendingFeatures, hasImportedLegacyProject, ] = await Promise.all([ this.projectChecker.hasPendingExports(projectId), this.projectChecker.hasPendingImports(projectId), this.projectChecker.hasPendingBlmCalibration(projectId), this.projectChecker.hasPendingMarxanRun(projectId), + this.projectChecker.hasPendingFeatures(projectId), this.legacyProjectImportChecker.isLegacyProjectImportCompletedFor( projectId, ), @@ -75,6 +77,10 @@ export class MarxanBlockGuard implements BlockGuard { throw new BadRequestException( `Project ${projectId} editing is blocked because of pending legacy project import`, ); + if (isRight(hasPendingFeatures) && hasPendingFeatures.right) + throw new BadRequestException( + `Project ${projectId} editing is blocked because of pending features`, + ); } async ensureThatScenarioIsNotBlocked(scenarioId: string): Promise { diff --git a/api/apps/api/src/modules/projects/project-checker/marxan-project-checker.service.ts b/api/apps/api/src/modules/projects/project-checker/marxan-project-checker.service.ts index b8cc2721d3..3f1474353c 100644 --- a/api/apps/api/src/modules/projects/project-checker/marxan-project-checker.service.ts +++ b/api/apps/api/src/modules/projects/project-checker/marxan-project-checker.service.ts @@ -14,6 +14,8 @@ import { InjectRepository } from '@nestjs/typeorm'; import { Either, left, right } from 'fp-ts/Either'; import { isRight } from 'fp-ts/lib/These'; import { In, Repository } from 'typeorm'; +import { GeoFeature } from '@marxan-api/modules/geo-features/geo-feature.api.entity'; +import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; @Injectable() export class MarxanProjectChecker implements ProjectChecker { @@ -21,6 +23,8 @@ export class MarxanProjectChecker implements ProjectChecker { private readonly apiEvents: ApiEventsService, @InjectRepository(Project) private readonly repository: Repository, + @InjectRepository(GeoFeature) + private readonly featureRepo: Repository, private readonly planningAreas: PlanningAreasService, private readonly scenarioChecker: ScenarioChecker, ) {} @@ -163,6 +167,25 @@ export class MarxanProjectChecker implements ProjectChecker { return right(results.some((hasPendingMarxanRun) => hasPendingMarxanRun)); } + async hasPendingFeatures( + projectId: string, + ): Promise> { + const project = await this.repository.findOne({ + where: { id: projectId }, + relations: { scenarios: true }, + }); + + if (!project) { + return left(doesntExist); + } + + const pendingFeatures = await this.featureRepo.count({ + where: { projectId, creationStatus: JobStatus.running }, + }); + + return right(pendingFeatures > 0); + } + async isProjectReady( projectId: string, ): Promise> { diff --git a/api/apps/api/src/modules/projects/project-checker/project-checker.module.ts b/api/apps/api/src/modules/projects/project-checker/project-checker.module.ts index 9abff87662..d368b94fe6 100644 --- a/api/apps/api/src/modules/projects/project-checker/project-checker.module.ts +++ b/api/apps/api/src/modules/projects/project-checker/project-checker.module.ts @@ -6,14 +6,15 @@ import { PublishedProject } from '@marxan-api/modules/published-project/entities import { ScenarioCheckerModule } from '@marxan-api/modules/scenarios/scenario-checker/scenario-checker.module'; import { Module } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; -import { Project } from '../project.api.entity'; +import { Project } from '@marxan-api/modules/projects/project.api.entity'; +import { GeoFeature } from '@marxan-api/modules/geo-features/geo-feature.api.entity'; @Module({ imports: [ ApiEventsModule, ScenarioCheckerModule, PlanningAreasModule, - TypeOrmModule.forFeature([Project, PublishedProject]), + TypeOrmModule.forFeature([Project, GeoFeature, PublishedProject]), ], providers: [ { diff --git a/api/apps/api/src/modules/projects/project-checker/project-checker.service.spec.ts b/api/apps/api/src/modules/projects/project-checker/project-checker.service.spec.ts index 28b99e4a1c..223e5fb932 100644 --- a/api/apps/api/src/modules/projects/project-checker/project-checker.service.spec.ts +++ b/api/apps/api/src/modules/projects/project-checker/project-checker.service.spec.ts @@ -16,6 +16,7 @@ import { isEqual } from 'lodash'; import { FindOneOptions, In, Repository } from 'typeorm'; import { ScenarioCheckerFake } from '../../../../../api/test/utils/scenario-checker.service-fake'; import { PlanningAreasService } from '@marxan-api/modules/planning-areas'; +import { GeoFeature } from '@marxan-api/modules/geo-features/geo-feature.api.entity'; let fixtures: FixtureType; @@ -415,6 +416,10 @@ async function getFixtures() { findOne: jest.fn((_: any) => Promise.resolve({} as Scenario)), }; + const fakeFeaturesRepo: jest.Mocked, 'count'>> = { + count: jest.fn((_: any) => Promise.resolve(1)), + }; + const fakePlaningAreaFacade = { locatePlanningAreaEntity: jest.fn(), }; @@ -432,6 +437,10 @@ async function getFixtures() { provide: getRepositoryToken(Scenario), useValue: fakeScenariosRepo, }, + { + provide: getRepositoryToken(GeoFeature), + useValue: fakeFeaturesRepo, + }, { provide: PlanningAreasService, useValue: fakePlaningAreaFacade, diff --git a/api/apps/api/src/modules/projects/project-checker/project-checker.service.ts b/api/apps/api/src/modules/projects/project-checker/project-checker.service.ts index d18ee1226e..51e871a52c 100644 --- a/api/apps/api/src/modules/projects/project-checker/project-checker.service.ts +++ b/api/apps/api/src/modules/projects/project-checker/project-checker.service.ts @@ -23,4 +23,8 @@ export abstract class ProjectChecker { abstract isProjectReady( projectId: string, ): Promise>; + + abstract hasPendingFeatures( + projectId: string, + ): Promise>; } diff --git a/api/apps/api/src/modules/projects/projects.project-features.controller.ts b/api/apps/api/src/modules/projects/projects.project-features.controller.ts index 6d0175cf32..5909c67aad 100644 --- a/api/apps/api/src/modules/projects/projects.project-features.controller.ts +++ b/api/apps/api/src/modules/projects/projects.project-features.controller.ts @@ -1,11 +1,9 @@ import { - BadRequestException, Body, Controller, Delete, ForbiddenException, Get, - InternalServerErrorException, NotFoundException, Param, ParseUUIDPipe, @@ -34,10 +32,7 @@ import { import { apiGlobalPrefixes } from '@marxan-api/api.config'; import { JwtAuthGuard } from '@marxan-api/guards/jwt-auth.guard'; import { FileInterceptor } from '@nestjs/platform-express'; -import { - ensureShapefileHasRequiredFiles, - uploadOptions, -} from '@marxan-api/utils/file-uploads.utils'; +import { uploadOptions } from '@marxan-api/utils/file-uploads.utils'; import { JSONAPIQueryParams } from '@marxan-api/decorators/json-api-parameters.decorator'; import { RequestWithAuthenticatedUser } from '@marxan-api/app.controller'; @@ -55,8 +50,6 @@ import { GeoFeatureSerializer } from './dto/geo-feature.serializer'; import { isLeft } from 'fp-ts/Either'; import { UploadShapefileDTO } from './dto/upload-shapefile.dto'; import { GeoFeaturesService } from '@marxan-api/modules/geo-features'; -import { ShapefileService } from '@marxan/shapefile-converter'; -import { isFeatureCollection } from '@marxan/utils'; import { inlineJobTag } from '@marxan-api/dto/inline-job-tag'; import { GeometryFileInterceptor, @@ -77,7 +70,6 @@ import { UpdateGeoFeatureTagDTO } from '@marxan-api/modules/geo-feature-tags/dto import { GeoFeatureTagsService } from '@marxan-api/modules/geo-feature-tags/geo-feature-tags.service'; import { GetProjectTagsResponseDto } from '@marxan-api/modules/projects/dto/get-project-tags-response.dto'; import { UpdateProjectTagDTO } from '@marxan-api/modules/projects/dto/update-project-tag.dto'; -import { isNil } from 'lodash'; import { Response } from 'express'; import { ProxyService } from '@marxan-api/modules/proxy/proxy.service'; @@ -91,7 +83,6 @@ export class ProjectFeaturesController { private readonly geoFeatureSerializer: GeoFeatureSerializer, private readonly geoFeatureService: GeoFeaturesService, private readonly geoFeatureTagsService: GeoFeatureTagsService, - private readonly shapefileService: ShapefileService, private readonly proxyService: ProxyService, ) {} @@ -111,6 +102,7 @@ export class ProjectFeaturesController { @Param('projectId', ParseUUIDPipe) projectId: string, @Req() req: RequestWithAuthenticatedUser, @Query('q') featureClassAndAliasFilter?: string, + @Query('includeInProgress') includeInProgress?: boolean, ): Promise { const result = await this.projectsService.findAllGeoFeatures( fetchSpecification, @@ -118,7 +110,8 @@ export class ProjectFeaturesController { authenticatedUser: req.user, params: { projectId: projectId, - featureClassAndAliasFilter: featureClassAndAliasFilter, + featureClassAndAliasFilter, + includeInProgress, }, }, ); @@ -133,9 +126,9 @@ export class ProjectFeaturesController { @IsMissingAclImplementation() @ApiConsumesShapefile({ withGeoJsonResponse: false }) @ApiOperation({ - description: `Upload shapefiles of species or bioregional features`, + description: `Upload shapefiles of species or bioregional features asynchronously. Returns ok response after all validations, and then it is processed in the background`, }) - @ApiOkResponse({ type: GeoFeature }) + @ApiOkResponse() @ApiTags(inlineJobTag) @ApiBody({ description: 'Shapefile to upload', @@ -147,49 +140,33 @@ export class ProjectFeaturesController { @Param('id') projectId: string, @UploadedFile() shapefile: Express.Multer.File, @Body() body: UploadShapefileDTO, - ): Promise { - await ensureShapefileHasRequiredFiles(shapefile); - - const { data } = await this.shapefileService.transformToGeoJson(shapefile, { - allowOverlaps: true, - }); - - if (!isFeatureCollection(data)) { - throw new BadRequestException(`Only FeatureCollection is supported.`); - } + @Req() req: RequestWithAuthenticatedUser, + ): Promise { + const result = await this.geoFeatureService.uploadFeatureFromShapefile( + projectId, + body, + shapefile, + ); - const newFeatureOrError = - await this.geoFeatureService.createFeaturesForShapefile( + if (isLeft(result)) { + throw mapAclDomainToHttpError(result.left, { + userId: req.user.id, projectId, - body, - data.features, - ); - - if (isLeft(newFeatureOrError)) { - // @debt Use mapDomainToHttpException() instead - throw new InternalServerErrorException(newFeatureOrError.left); - } else { - const result = await this.geoFeatureService.getById( - newFeatureOrError.right.id, - ); - if (isNil(result)) { - // @debt Use mapDomainToHttpException() instead - throw new NotFoundException(); - } - - return this.geoFeatureSerializer.serialize(result); + resourceType: projectResource.name.plural, + }); } } @ImplementsAcl() @ApiConsumesCsv({ - description: 'Upload a csv with feature amounts for each puid', + description: + 'Upload a csv with feature amounts for each puid asynchronously. Returns ok response after all validations, and then it is processed in the background', }) @ApiParam({ name: 'projectId', description: 'Id of the Project the feature is part of', }) - @ApiOkResponse({ type: GeoFeature, isArray: true }) + @ApiOkResponse() @UseInterceptors( FileInterceptor('file', { limits: uploadOptions(50 * 1024 ** 2).limits }), ) @@ -198,7 +175,7 @@ export class ProjectFeaturesController { @Param('projectId', ParseUUIDPipe) projectId: string, @UploadedFile() file: Express.Multer.File, @Req() req: RequestWithAuthenticatedUser, - ): Promise { + ): Promise { const result = await this.geoFeatureService.saveFeaturesFromCsv( file.buffer, projectId, @@ -212,7 +189,6 @@ export class ProjectFeaturesController { resourceType: projectResource.name.plural, }); } - return result.right; } @ImplementsAcl() diff --git a/api/apps/api/src/utils/acl.utils.ts b/api/apps/api/src/utils/acl.utils.ts index 40a2669208..85fcb57417 100644 --- a/api/apps/api/src/utils/acl.utils.ts +++ b/api/apps/api/src/utils/acl.utils.ts @@ -87,15 +87,8 @@ import { featureNameAlreadyInUse, featureNotEditable, featureNotFound, - importedFeatureNameAlreadyExist, - missingPuidColumnInFeatureAmountCsvUpload, - unknownPuidsInFeatureAmountCsvUpload, + onlyFeatureCollectionShapefileSupported, } from '@marxan-api/modules/geo-features/geo-features.service'; -import { - duplicateHeadersInFeatureAmountCsvUpload, - duplicatePuidsInFeatureAmountCsvUpload, - noFeaturesFoundInInFeatureAmountCsvUpload, -} from '@marxan-api/modules/geo-features/import/csv.parser'; import { featureNotEditableByUserWithinProject, featureNotFoundWithinProject, @@ -187,12 +180,6 @@ export const mapAclDomainToHttpError = ( | typeof projectNotVisible | typeof tagNotFoundForProject | typeof scenarioNotCreated - | typeof importedFeatureNameAlreadyExist - | typeof unknownPuidsInFeatureAmountCsvUpload - | typeof missingPuidColumnInFeatureAmountCsvUpload - | typeof duplicatePuidsInFeatureAmountCsvUpload - | typeof duplicateHeadersInFeatureAmountCsvUpload - | typeof noFeaturesFoundInInFeatureAmountCsvUpload | ImportProjectError | typeof featureDataCannotBeUploadedWithCsv | typeof outputProjectSummaryNotFound @@ -209,7 +196,8 @@ export const mapAclDomainToHttpError = ( | typeof cannotDeleteDefaultCostSurface | typeof deleteCostSurfaceFailed | typeof scenarioNotEditable - | typeof linkCostSurfaceToScenarioFailed, + | typeof linkCostSurfaceToScenarioFailed + | typeof onlyFeatureCollectionShapefileSupported, options?: ErrorHandlerOptions, ) => { switch (errorToCheck) { @@ -421,24 +409,7 @@ export const mapAclDomainToHttpError = ( throw new NotFoundException( `Scenario for Project with id ${options?.projectId} could not be created`, ); - case importedFeatureNameAlreadyExist: - return new BadRequestException('Imported Features already present'); - case unknownPuidsInFeatureAmountCsvUpload: - return new BadRequestException('Unknown PUIDs'); - case missingPuidColumnInFeatureAmountCsvUpload: - return new BadRequestException('Missing PUID column'); - case noFeaturesFoundInInFeatureAmountCsvUpload: - return new BadRequestException( - 'No features found in feature amount CSV upload', - ); - case duplicateHeadersInFeatureAmountCsvUpload: - return new BadRequestException( - 'Duplicate headers in feature amount CSV upload', - ); - case duplicatePuidsInFeatureAmountCsvUpload: - return new BadRequestException( - 'Duplicate PUIDs in feature amount CSV upload', - ); + case featureDataCannotBeUploadedWithCsv: throw new ForbiddenException( `User with id ${options?.userId} cannot upload feature data with csv for project with id ${options?.projectId}`, @@ -475,6 +446,8 @@ export const mapAclDomainToHttpError = ( throw new BadRequestException( `Linking Cost Surface to Scenario ${options?.scenarioId} failed`, ); + case onlyFeatureCollectionShapefileSupported: + throw new BadRequestException(`Only FeatureCollection is supported.`); default: const _exhaustiveCheck: never = errorToCheck; diff --git a/api/apps/api/test/geo-features.e2e-spec.ts b/api/apps/api/test/geo-features.e2e-spec.ts index 6993ed8f0d..0ee03ea70a 100644 --- a/api/apps/api/test/geo-features.e2e-spec.ts +++ b/api/apps/api/test/geo-features.e2e-spec.ts @@ -18,7 +18,10 @@ import { getEntityManagerToken, getRepositoryToken } from '@nestjs/typeorm'; import { DbConnections } from '@marxan-api/ormconfig.connections'; import { range } from 'lodash'; import { GeoFeatureGeometry } from '@marxan/geofeatures'; -import { Scenario } from '@marxan-api/modules/scenarios/scenario.api.entity'; +import { + JobStatus, + Scenario, +} from '@marxan-api/modules/scenarios/scenario.api.entity'; import { FixtureType } from '@marxan/utils/tests/fixture-type'; import * as fs from 'fs/promises'; import * as path from 'path'; @@ -55,19 +58,42 @@ describe('GeoFeaturesModule (e2e)', () => { /** * https://www.figma.com/file/hq0BZNB9fzyFSbEUgQIHdK/Marxan-Visual_V02?node-id=2991%3A2492 */ - test('As a user, I should be able to retrieve a list of features available within a project', async () => { + test('As a user, I should be able to retrieve a list features available within a project (excluding currently running and failed ones)', async () => { const response = await request(app.getHttpServer()) .get(`/api/v1/projects/${world.projectWithCountry}/features`) .set('Authorization', `Bearer ${jwtToken}`) .expect(HttpStatus.OK); - const geoFeaturesForProject: GeoFeature[] = response.body.data; + const geoFeaturesForProject: JSONAPIGeoFeaturesData[] = response.body.data; expect(geoFeaturesForProject.length).toBeGreaterThan(0); expect(response.body.data[0].type).toBe(geoFeatureResource.name.plural); expect(response.body.data[0].attributes.amountRange).toEqual({ min: null, max: null, }); + const statuses = geoFeaturesForProject.map( + (feat) => feat.attributes.creationStatus, + ); + expect(statuses).not.toContain(JobStatus.failure); + expect(statuses).not.toContain(JobStatus.running); + }); + + test('As a user, I should be able to retrieve a list features available within a project, including currently running and failed ones if indicated in the request', async () => { + const response = await request(app.getHttpServer()) + .get(`/api/v1/projects/${world.projectWithCountry}/features`) + .query({ includeInProgress: true }) + .set('Authorization', `Bearer ${jwtToken}`) + .expect(HttpStatus.OK); + + const geoFeaturesForProject: JSONAPIGeoFeaturesData[] = response.body.data; + expect(geoFeaturesForProject.length).toBeGreaterThan(0); + expect(response.body.data[0].type).toBe(geoFeatureResource.name.plural); + + const statuses = geoFeaturesForProject.map( + (feat) => feat.attributes.creationStatus, + ); + expect(statuses).toContain(JobStatus.failure); + expect(statuses).toContain(JobStatus.running); }); test('should include the min and max amount of the features in the proper units of measure', async () => { @@ -221,6 +247,33 @@ describe('GeoFeaturesModule (e2e)', () => { geoFeaturesFilters.cheeta.featureClassName, ); expect(response.body.data[0].attributes.isCustom).toEqual(false); + + const response2 = await request(app.getHttpServer()) + .get(`/api/v1/projects/${world.projectWithCountry}/features`) + .query({ q: 'failed_feature' }) + .set('Authorization', `Bearer ${jwtToken}`) + .expect(HttpStatus.OK); + + expect(response2.body.data).toHaveLength(0); + + const response3 = await request(app.getHttpServer()) + .get(`/api/v1/projects/${world.projectWithCountry}/features`) + .query({ q: 'running_feature' }) + .set('Authorization', `Bearer ${jwtToken}`) + .expect(HttpStatus.OK); + + expect(response3.body.data).toHaveLength(0); + + const response4 = await request(app.getHttpServer()) + .get(`/api/v1/projects/${world.projectWithCountry}/features`) + .query({ q: 'running_feature', includeInProgress: true }) + .set('Authorization', `Bearer ${jwtToken}`) + .expect(HttpStatus.OK); + + expect(response4.body.data).toHaveLength(1); + expect(response4.body.data[0].attributes.featureClassName).toEqual( + 'running_feature', + ); }); test.skip('should return a single result of geo-features whose alias property matches a given filter', async () => { @@ -260,6 +313,17 @@ describe('GeoFeaturesModule (e2e)', () => { response.body.data.map((feature: JSONAPIGeoFeaturesData) => { expect(feature.attributes.isCustom).toEqual(false); }); + + const response2 = await request(app.getHttpServer()) + .get(`/api/v1/projects/${world.projectWithCountry}/features?q=`) + .query({ includeInProgress: true }) + .set('Authorization', `Bearer ${jwtToken}`) + .expect(HttpStatus.OK); + + expect(response2.body.data).toHaveLength(11); + response2.body.data.map((feature: JSONAPIGeoFeaturesData) => { + expect(feature.attributes.isCustom).toEqual(false); + }); }); }); @@ -312,7 +376,9 @@ export const getGeoFeatureFixtures = async (app: INestApplication) => { VALUES ('${name}', '${name}', null,'${ projectId ? projectId : 'null' - }', ' name', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')) + }', ' name', null, '${ + JobStatus.created + }', (SELECT id FROM users WHERE email = 'aa@example.com')) RETURNING feature_class_name, id; `); diff --git a/api/apps/api/test/geo-features/geo-feature-tags.e2e-spec.ts b/api/apps/api/test/geo-features/geo-feature-tags.e2e-spec.ts index ba3dcc2826..929c7e39ad 100644 --- a/api/apps/api/test/geo-features/geo-feature-tags.e2e-spec.ts +++ b/api/apps/api/test/geo-features/geo-feature-tags.e2e-spec.ts @@ -15,7 +15,7 @@ describe('GeoFeatureTag DELETE (e2e)', () => { await fixtures?.cleanup(); }); - test('should return error if feature is not found or not related to project', async () => { + test('should return error if feature is not found, not related to project, or not fully created', async () => { //ARRANGE const randomProjectId = v4(); const randomFeatureId = v4(); diff --git a/api/apps/api/test/geo-features/geo-feature-tags.fixtures.ts b/api/apps/api/test/geo-features/geo-feature-tags.fixtures.ts index 83823c8e41..029fc8c803 100644 --- a/api/apps/api/test/geo-features/geo-feature-tags.fixtures.ts +++ b/api/apps/api/test/geo-features/geo-feature-tags.fixtures.ts @@ -15,6 +15,7 @@ import { ProjectRoles } from '@marxan-api/modules/access-control/projects-acl/dt import { UsersProjectsApiEntity } from '@marxan-api/modules/access-control/projects-acl/entity/users-projects.api.entity'; import { GivenUserExists } from '../steps/given-user-exists'; import { tagMaxlength } from '@marxan-api/modules/geo-feature-tags/dto/update-geo-feature-tag.dto'; +import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; export const getGeoFeatureTagsFixtures = async () => { const app = await bootstrapApplication(); @@ -85,13 +86,17 @@ export const getGeoFeatureTagsFixtures = async () => { return projectResult.data.id; }, - GivenFeatureOnProject: async (projectId: string, featureName: string) => { + GivenFeatureOnProject: async ( + projectId: string, + featureName: string, + creationStatus = JobStatus.created, + ) => { const results: { id: string; }[] = await geoFeatureRepo.query(`INSERT INTO features (feature_class_name, alias, description, property_name, intersection, project_id, creation_status, created_by) VALUES - ('${featureName}', 'alias_${featureName}', null, ' name', null, '${projectId}', 'created', (SELECT id FROM users WHERE email = 'aa@example.com')) + ('${featureName}', 'alias_${featureName}', null, ' name', null, '${projectId}', '${creationStatus}', (SELECT id FROM users WHERE email = 'aa@example.com')) RETURNING id; `); diff --git a/api/apps/api/test/geo-features/update-project-feature-name.e2e-spec.ts b/api/apps/api/test/geo-features/update-project-feature-name.e2e-spec.ts index a13b6165c6..fec21c6833 100644 --- a/api/apps/api/test/geo-features/update-project-feature-name.e2e-spec.ts +++ b/api/apps/api/test/geo-features/update-project-feature-name.e2e-spec.ts @@ -33,6 +33,37 @@ describe('Project - update feature Name', () => { await fixtures.ThenFeatureWasNotFound(result, projectId, featureId); }); + test('should return NotFound error when feature is not fully created', async () => { + //ARRANGE + const projectId = fixtures.anotherProjectId; + const runningId = await fixtures.GivenBaseFeature( + 'running', + projectId, + JobStatus.running, + ); + const failureId = await fixtures.GivenBaseFeature( + 'failure', + projectId, + JobStatus.failure, + ); + + //ACT + const response1 = await fixtures.WhenUpdatingFeatureForProject( + runningId, + 'forbidden update', + ); + const response2 = await fixtures.WhenUpdatingFeatureForProject( + failureId, + 'forbidden update', + ); + + //ASSERT + await fixtures.ThenFeatureWasNotFound(response1, projectId, runningId); + await fixtures.ThenGeoFeatureHasName(runningId, 'running'); + await fixtures.ThenFeatureWasNotFound(response2, projectId, failureId); + await fixtures.ThenGeoFeatureHasName(failureId, 'failure'); + }); + test('should not permit updating a given feature when the feature is associated to a different project', async () => { const originalName = 'someName'; const anotherProjectId = fixtures.anotherProjectId; @@ -215,12 +246,16 @@ const getFixtures = async () => { }, // ARRANGE - GivenBaseFeature: async (featureClassName: string, projectId?: string) => { + GivenBaseFeature: async ( + featureClassName: string, + projectId?: string, + creationStatus = JobStatus.created, + ) => { const baseFeature = await geoFeaturesApiRepo.save({ id: v4(), featureClassName, projectId, - creationStatus: JobStatus.created, + creationStatus, }); return baseFeature.id; }, diff --git a/api/apps/api/test/projects/project-feature-tags/project-tags.e2e-spec.ts b/api/apps/api/test/projects/project-feature-tags/project-tags.e2e-spec.ts index a535092d31..b20dcd7646 100644 --- a/api/apps/api/test/projects/project-feature-tags/project-tags.e2e-spec.ts +++ b/api/apps/api/test/projects/project-feature-tags/project-tags.e2e-spec.ts @@ -3,6 +3,7 @@ import { v4 } from 'uuid'; import { HttpStatus } from '@nestjs/common'; import { getProjectTagsFixtures } from './project-tags.fixtures'; import { tagMaxlength } from '@marxan-api/modules/geo-feature-tags/dto/update-geo-feature-tag.dto'; +import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; let fixtures: FixtureType; @@ -109,7 +110,7 @@ describe('Projects Tag GET (e2e)', () => { expect(response2.body.data).toEqual(['AnotherTag', 'another-repeated-tag']); }); - test('should return all available distinct tags for the given project if no tag query param is provided', async () => { + test('should return all available distinct tags for the features of given project that have creationStatus created, if no tag query param is provided', async () => { const projectId1 = await fixtures.GivenProject('someProject'); const featureId11 = await fixtures.GivenFeatureOnProject( projectId1, @@ -123,9 +124,30 @@ describe('Projects Tag GET (e2e)', () => { projectId1, 'name13', ); + const featureIdRunning = await fixtures.GivenFeatureOnProject( + projectId1, + 'running', + JobStatus.running, + ); + const featureIdFailure = await fixtures.GivenFeatureOnProject( + projectId1, + 'failure', + JobStatus.failure, + ); + await fixtures.GivenTagOnFeature(projectId1, featureId11, 'OneRepeatedTag'); await fixtures.GivenTagOnFeature(projectId1, featureId12, 'OneRepeatedTag'); await fixtures.GivenTagOnFeature(projectId1, featureId13, 'AnotherTag'); + await fixtures.GivenTagOnFeature( + projectId1, + featureIdRunning, + 'Irrelevant', + ); + await fixtures.GivenTagOnFeature( + projectId1, + featureIdFailure, + 'Irrelevant', + ); // ACT const response = await fixtures.WhenGettingProjectTags( @@ -210,7 +232,7 @@ describe('Projects Tag PATCH (e2e)', () => { fixtures.ThenTagNotFoundErrorWasReturned(response, projectId); }); - test('should update all feature tag rows that match exactly with the tag to be updated, for the given Project', async () => { + test('should update all feature tag rows that match exactly with the tag to be updated, for the features in the given Project, that have a creationStatus of created only ', async () => { // ARRANGE const projectId1 = await fixtures.GivenProject('someProject'); const projectId2 = await fixtures.GivenProject('someProject2'); @@ -219,12 +241,32 @@ describe('Projects Tag PATCH (e2e)', () => { const featureId13 = await fixtures.GivenFeatureOnProject(projectId1, 'f13'); const featureId14 = await fixtures.GivenFeatureOnProject(projectId1, 'f14'); const featureId21 = await fixtures.GivenFeatureOnProject(projectId2, 'f21'); + const featureIdRunning = await fixtures.GivenFeatureOnProject( + projectId1, + 'running', + JobStatus.running, + ); + const featureIdFailure = await fixtures.GivenFeatureOnProject( + projectId1, + 'failure', + JobStatus.failure, + ); await fixtures.GivenTagOnFeature(projectId1, featureId11, 'toBeUpdated'); await fixtures.GivenTagOnFeature(projectId1, featureId12, 'notupdated'); await fixtures.GivenTagOnFeature(projectId1, featureId13, 'NOTupdated'); await fixtures.GivenTagOnFeature(projectId1, featureId14, 'TOBEUPDATED'); await fixtures.GivenTagOnFeature(projectId2, featureId21, 'toBeUpdated'); + await fixtures.GivenTagOnFeature( + projectId1, + featureIdRunning, + 'toBeUpdated', + ); + await fixtures.GivenTagOnFeature( + projectId1, + featureIdFailure, + 'toBeUpdated', + ); //ACT const response = await fixtures.WhenPatchingAProjectTag( @@ -237,7 +279,16 @@ describe('Projects Tag PATCH (e2e)', () => { expect(response.status).toBe(HttpStatus.OK); await fixtures.ThenFeatureHasTag(projectId1, featureId12, 'notupdated'); await fixtures.ThenFeatureHasTag(projectId1, featureId13, 'notupdated'); - await fixtures.ThenFeatureHasTag(projectId2, featureId21, 'toBeUpdated'); + await fixtures.ThenFeatureHasTag( + projectId1, + featureIdRunning, + 'updatedTAG', + ); + await fixtures.ThenFeatureHasTag( + projectId1, + featureIdFailure, + 'updatedTAG', + ); await fixtures.ThenFeatureHasTag(projectId1, featureId11, 'updatedTAG'); await fixtures.ThenFeatureHasTag(projectId1, featureId14, 'updatedTAG'); @@ -347,6 +398,16 @@ describe('Projects Tag GET Features (e2e)', () => { const featureId13 = await fixtures.GivenFeatureOnProject(projectId1, 'f13'); const featureId14 = await fixtures.GivenFeatureOnProject(projectId1, 'f14'); const featureId21 = await fixtures.GivenFeatureOnProject(projectId2, 'f21'); + const featureIdRunning = await fixtures.GivenFeatureOnProject( + projectId1, + 'running', + JobStatus.running, + ); + const featureIdFailure = await fixtures.GivenFeatureOnProject( + projectId1, + 'failure', + JobStatus.failure, + ); const mockedIntersectingProjectFeature = fixtures.GivenGeoFeatureServiceIntersectingFeaturesMock([ @@ -362,6 +423,9 @@ describe('Projects Tag GET Features (e2e)', () => { await fixtures.GivenTagOnFeature(projectId1, featureId14, 'wind'); await fixtures.GivenTagOnFeature(projectId2, featureId21, 'fire'); + await fixtures.GivenTagOnFeature(projectId1, featureIdRunning, 'fire'); + await fixtures.GivenTagOnFeature(projectId1, featureIdFailure, 'EARTH'); + // ACT const response1 = await fixtures.WhenGettingFeaturesFromProject( projectId1, diff --git a/api/apps/api/test/projects/project-feature-tags/project-tags.fixtures.ts b/api/apps/api/test/projects/project-feature-tags/project-tags.fixtures.ts index fc1e94368c..7d996dd834 100644 --- a/api/apps/api/test/projects/project-feature-tags/project-tags.fixtures.ts +++ b/api/apps/api/test/projects/project-feature-tags/project-tags.fixtures.ts @@ -16,6 +16,7 @@ import { UsersProjectsApiEntity } from '@marxan-api/modules/access-control/proje import { GivenUserExists } from '../../steps/given-user-exists'; import { tagMaxlength } from '@marxan-api/modules/geo-feature-tags/dto/update-geo-feature-tag.dto'; import { GeoFeaturesService } from '@marxan-api/modules/geo-features'; +import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; export const getProjectTagsFixtures = async () => { const app = await bootstrapApplication(); @@ -94,13 +95,17 @@ export const getProjectTagsFixtures = async () => { return projectResult.data.id; }, - GivenFeatureOnProject: async (projectId: string, featureName: string) => { + GivenFeatureOnProject: async ( + projectId: string, + featureName: string, + creationStatus = JobStatus.created, + ) => { const results: { id: string; }[] = await geoFeatureRepo.query(`INSERT INTO features (feature_class_name, alias, description, property_name, intersection, project_id, creation_status, created_by) VALUES - ('${featureName}', 'alias_${featureName}', null, ' name', null, '${projectId}', 'created', (SELECT id FROM users WHERE email = 'aa@example.com')) + ('${featureName}', 'alias_${featureName}', null, ' name', null, '${projectId}', '${creationStatus}', (SELECT id FROM users WHERE email = 'aa@example.com')) RETURNING id; `); diff --git a/api/apps/api/test/upload-feature/import-files/no_features_upload.csv b/api/apps/api/test/upload-feature/import-files/no_features_upload.csv index f5f8ad2bbf..16621b66fc 100644 --- a/api/apps/api/test/upload-feature/import-files/no_features_upload.csv +++ b/api/apps/api/test/upload-feature/import-files/no_features_upload.csv @@ -1,4 +1,4 @@ -puid, -1, -2, -3, +puid +1 +2 +3 diff --git a/api/apps/api/test/upload-feature/upload-feature-csv.e2e-spec.ts b/api/apps/api/test/upload-feature/upload-feature-csv.e2e-spec.ts index b6f88777b1..b495a3c485 100644 --- a/api/apps/api/test/upload-feature/upload-feature-csv.e2e-spec.ts +++ b/api/apps/api/test/upload-feature/upload-feature-csv.e2e-spec.ts @@ -1,6 +1,7 @@ import { FixtureType } from '@marxan/utils/tests/fixture-type'; import { getFixtures } from './upload-feature.fixtures'; import { v4 } from 'uuid'; +import { API_EVENT_KINDS } from '@marxan/api-events'; let fixtures: FixtureType; @@ -13,9 +14,12 @@ afterEach(async () => { }); test(`custom feature csv upload`, async () => { - const result = await fixtures.WhenUploadingCustomFeatureFromCSV(); + // ARRANGE / ACT + await fixtures.WhenUploadingCustomFeatureFromCSV(); - expect(result.body).toHaveLength(2); + // ASSERT + await fixtures.ThenCSVImportSubmitEventWasSubmitted(fixtures.projectId); + await fixtures.ThenCSVImportFinishedEventWasSubmitted(fixtures.projectId); await fixtures.ThenNewFeaturesAreCreated(); await fixtures.ThenNewFeaturesAmountsAreCreated(); await fixtures.ThenFeatureAmountPerPlanningUnitAreCreated(); @@ -26,36 +30,76 @@ test('custom feature csv upload when project not found', async () => { const falseProjectId = v4(); const response = await fixtures.WhenUploadingCsvWhenProjectNotFound(falseProjectId); - await fixtures.ThenProjectNotFoundErrorIsReturned(response, falseProjectId); + fixtures.ThenProjectNotFoundErrorIsReturned(response, falseProjectId); await fixtures.AndNoFeatureUploadIsRegistered(); }); test('custom feature csv upload with missing puids', async () => { - const response = await fixtures.WhenUploadingCsvWithMissingPUIDColumn(); - await fixtures.ThenMissingPUIDErrorIsReturned(response); + // ARRANGE / ACT + await fixtures.WhenUploadingCsvWithMissingPUIDColumn(); + + // ASSERT + await fixtures.ThenCSVImportSubmitEventWasSubmitted(fixtures.projectId); + const event = await fixtures.ThenWaitForApiEvent( + fixtures.projectId, + API_EVENT_KINDS.features__csv__import__failed__v1__alpha, + ); + fixtures.ThenMissingPUIDErrorIsReturned(event); await fixtures.AndNoFeatureUploadIsRegistered(); }); test('custom feature csv upload with no features', async () => { - const response = await fixtures.WhenUploadingCsvWithNoFeatures(); - await fixtures.ThenNoFeaturesInCsvFileErrorIsReturned(response); + // ARRANGE / ACT + await fixtures.WhenUploadingCsvWithNoFeatures(); + + // ASSERT + await fixtures.ThenCSVImportSubmitEventWasSubmitted(fixtures.projectId); + const event = await fixtures.ThenWaitForApiEvent( + fixtures.projectId, + API_EVENT_KINDS.features__csv__import__failed__v1__alpha, + ); + fixtures.ThenNoFeaturesInCsvFileErrorIsReturned(event); await fixtures.AndNoFeatureUploadIsRegistered(); }); test('custom feature csv upload with duplicated puids', async () => { - const response = await fixtures.WhenUploadingCsvWithDuplicatedPUIDs(); - await fixtures.ThenDuplicatedPUIDErrorIsReturned(response); + // ARRANGE / ACT + await fixtures.WhenUploadingCsvWithDuplicatedPUIDs(); + + // ASSERT + await fixtures.ThenCSVImportSubmitEventWasSubmitted(fixtures.projectId); + const event = await fixtures.ThenWaitForApiEvent( + fixtures.projectId, + API_EVENT_KINDS.features__csv__import__failed__v1__alpha, + ); + fixtures.ThenDuplicatedPUIDErrorIsReturned(event); await fixtures.AndNoFeatureUploadIsRegistered(); }); test('custom feature csv with puids not present in the project', async () => { - const response = - await fixtures.WhenUploadingCsvWithPuidsNotPresentITheProject(); - await fixtures.ThenPuidsNotPresentErrorIsReturned(response); + // ARRANGE / ACT + await fixtures.WhenUploadingCsvWithPuidsNotPresentITheProject(); + + // ASSERT + await fixtures.ThenCSVImportSubmitEventWasSubmitted(fixtures.projectId); + const event = await fixtures.ThenWaitForApiEvent( + fixtures.projectId, + API_EVENT_KINDS.features__csv__import__failed__v1__alpha, + ); + fixtures.ThenPuidsNotPresentErrorIsReturned(event); await fixtures.AndNoFeatureUploadIsRegistered(); }); test('custom feature csv upload with duplicated header', async () => { - const response = await fixtures.WhenUploadingACSVWithDuplicatedHeaders(); - await fixtures.ThenDuplicatedHeaderErrorIsReturned(response); + // ARRANGE / ACT + await fixtures.WhenUploadingACSVWithDuplicatedHeaders(); + + // ASSERT + await fixtures.ThenCSVImportSubmitEventWasSubmitted(fixtures.projectId); + const event = await fixtures.ThenWaitForApiEvent( + fixtures.projectId, + API_EVENT_KINDS.features__csv__import__failed__v1__alpha, + ); + + fixtures.ThenDuplicatedHeaderErrorIsReturned(event); await fixtures.AndNoFeatureUploadIsRegistered(); }); diff --git a/api/apps/api/test/upload-feature/upload-feature.e2e-spec.ts b/api/apps/api/test/upload-feature/upload-feature.e2e-spec.ts index 7190918739..4bff91d981 100644 --- a/api/apps/api/test/upload-feature/upload-feature.e2e-spec.ts +++ b/api/apps/api/test/upload-feature/upload-feature.e2e-spec.ts @@ -14,12 +14,21 @@ afterEach(async () => { }); test(`custom feature upload`, async () => { + // ARRANGE const name = 'someFeature'; const description = 'someDescrip'; await fixtures.GivenProjectPusWithGeometryForProject(); + // ACT const result = await fixtures.WhenUploadingCustomFeature(name, description); + await fixtures.ThenShapefileImportSubmittedEventWasSubmitted( + fixtures.projectId, + ); + await fixtures.ThenShapefileImportFinishedEventWasSubmitted( + fixtures.projectId, + ); + // ASSERT await fixtures.ThenGeoFeaturesAreCreated(result, name, description); await fixtures.ThenFeatureAmountsFromShapefileAreCreated(name); }); @@ -37,7 +46,6 @@ test(`if tagging info is included in DTO but invalid, then error is returned and name, description, invalidTag1, - { skipWaitingForFeatureToBeReady: true }, // needed as this feature will never be actually created ); expect(result1.status).toBe(HttpStatus.BAD_REQUEST); fixtures.ThenInvalidTagErrorWasReturned(result1); @@ -47,7 +55,6 @@ test(`if tagging info is included in DTO but invalid, then error is returned and name, description, invalidTag2, - { skipWaitingForFeatureToBeReady: true }, // needed as this feature will never be actually created ); expect(result2.status).toBe(HttpStatus.BAD_REQUEST); fixtures.ThenMaxLengthErrorWasReturned(result2); @@ -67,9 +74,12 @@ test(`if tagging info is included in DTO and valid, created feature should be ta description, tag, ); + await fixtures.ThenShapefileImportFinishedEventWasSubmitted( + fixtures.projectId, + ); // ASSERT - await fixtures.ThenGeoFeaturesAreCreated(result, name, description, tag); + await fixtures.ThenGeoFeaturesAreCreated(result, name, description); await fixtures.ThenGeoFeatureTagIsCreated(name, tag); }); @@ -86,14 +96,12 @@ test(`if tagging info is included in, the feature's tag should be trimmed down o description, paddedTag, ); + await fixtures.ThenShapefileImportFinishedEventWasSubmitted( + fixtures.projectId, + ); // ASSERT - await fixtures.ThenGeoFeaturesAreCreated( - result, - name, - description, - paddedTag.trim(), - ); + await fixtures.ThenGeoFeaturesAreCreated(result, name, description); await fixtures.ThenGeoFeatureTagIsCreated(name, paddedTag.trim()); }); @@ -115,25 +123,29 @@ test(`if there is already an existing feature with a tag that has equivalent cap description, 'SomE-TAG', ); + await fixtures.ThenShapefileImportFinishedEventWasSubmitted( + fixtures.projectId, + ); // ASSERT - await fixtures.ThenGeoFeaturesAreCreated( - result, - name, - description, - equivalentTag, - ); + await fixtures.ThenGeoFeaturesAreCreated(result, name, description); await fixtures.ThenGeoFeatureTagIsCreated(name, equivalentTag); // TODO Check for update of last_modified_at for all affected tag rows for project when implemented }); test('should delete feature_amounts_per_planning_unit data related to a feature when this is deleted', async () => { + // ARRANGE const name = 'someFeature'; const description = 'someDescrip'; await fixtures.GivenProjectPusWithGeometryForProject(); + // ACT const result = await fixtures.WhenUploadingCustomFeature(name, description); + await fixtures.ThenShapefileImportFinishedEventWasSubmitted( + fixtures.projectId, + ); + // ASSERT await fixtures.ThenFeatureAmountsFromShapefileAreCreated(name); await fixtures.WhenDeletingFeatureForProject(name); await fixtures.ThenFeatureAmountsPerPlanningUnitDataIsDeletedForFeatureWithGivenId( diff --git a/api/apps/api/test/upload-feature/upload-feature.fixtures.ts b/api/apps/api/test/upload-feature/upload-feature.fixtures.ts index 3122875bf6..2971d8a005 100644 --- a/api/apps/api/test/upload-feature/upload-feature.fixtures.ts +++ b/api/apps/api/test/upload-feature/upload-feature.fixtures.ts @@ -10,7 +10,7 @@ import { GeoFeatureGeometry } from '@marxan/geofeatures'; import { DbConnections } from '@marxan-api/ormconfig.connections'; import { FeatureAmountUploadRegistry } from '@marxan-api/modules/geo-features/import/features-amounts-upload-registry.api.entity'; import { GivenProjectsPuExists } from '../../../geoprocessing/test/steps/given-projects-pu-exists'; -import { HttpStatus } from '@nestjs/common'; +import { BadRequestException, HttpStatus } from '@nestjs/common'; import { GeoFeatureTag } from '@marxan-api/modules/geo-feature-tags/geo-feature-tag.api.entity'; import { tagMaxlength } from '@marxan-api/modules/geo-feature-tags/dto/update-geo-feature-tag.dto'; import { FeatureAmountsPerPlanningUnitEntity } from '@marxan/feature-amounts-per-planning-unit'; @@ -20,11 +20,21 @@ import { } from '@marxan-jobs/planning-unit-geometry'; import { PlanningUnitGridShape } from '@marxan/scenarios-planning-unit'; import { GivenPuSquareGridGeometryExists } from '../../../geoprocessing/test/steps/given-pu-geometries-exists'; -import { waitForFeatureToBeReady } from '../utils/wait-for-feature-to-be-ready.utils'; +import { EventBusTestUtils } from '../utils/event-bus.test.utils'; +import { API_EVENT_KINDS } from '@marxan/api-events'; +import { ApiEventsService } from '@marxan-api/modules/api-events'; +import { GivenUserExists } from '../steps/given-user-exists'; +import { ApiEventByTopicAndKind } from '@marxan-api/modules/api-events/api-event.topic+kind.api.entity'; +import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; +import { waitForEvent } from '../utils/wait-for-feature-to-be-ready.utils'; export const getFixtures = async () => { - const app = await bootstrapApplication(); + const app = await bootstrapApplication([], [EventBusTestUtils]); + const eventBusUtils = app.get(EventBusTestUtils); + eventBusUtils.startInspectingEvents(); + const apiEventService = app.get(ApiEventsService); const token = await GivenUserIsLoggedIn(app); + const userId = await GivenUserExists(app); const { projectId, cleanup } = await GivenProjectExists( app, token, @@ -77,6 +87,8 @@ export const getFixtures = async () => { ); return { + userId, + projectId, cleanup: async () => { const feature = await geoFeaturesApiRepo.findOne({ where: { @@ -93,7 +105,9 @@ export const getFixtures = async () => { }); await planningUnitsRepo.delete({}); await projectsPuRepo.delete({}); + await apiEventService.purgeAll(); await cleanup(); + eventBusUtils.stopInspectingEvents(); await app.close(); }, @@ -146,9 +160,6 @@ export const getFixtures = async () => { name: string, description: string, tagName?: string, - options?: { - skipWaitingForFeatureToBeReady?: boolean; - }, ) => { const dto: any = { name, @@ -162,13 +173,6 @@ export const getFixtures = async () => { .set('Authorization', `Bearer ${token}`) .attach(`file`, __dirname + `/import-files/wetlands.zip`) .field(dto); - if (!options?.skipWaitingForFeatureToBeReady) { - expect(response.body.data.id).toBeDefined(); - await waitForFeatureToBeReady( - geoFeaturesApiRepo, - response.body.data.id, - ); - } return response; }, WhenUploadingCustomFeatureFromCSV: async () => { @@ -264,6 +268,72 @@ export const getFixtures = async () => { .send(); }, // ASSERT + ThenWaitForApiEvent: (topic: string, kind: API_EVENT_KINDS) => { + return new Promise((resolve, reject) => { + const findApiEvent = setInterval(async () => { + try { + const event = await apiEventService.getLatestEventForTopic({ + topic, + kind, + }); + clearInterval(findApiEvent); + + resolve(event); + } catch (e) { + console.error(e); + } + }, 150); + + setTimeout(async () => { + clearInterval(findApiEvent); + reject(); + }, 6000); + }); + }, + ThenCSVImportSubmitEventWasSubmitted: async (topic: string) => { + await waitForEvent( + apiEventService, + topic, + API_EVENT_KINDS.features__csv__import__submitted__v1__alpha, + ); + }, + ThenCSVImportFinishedEventWasSubmitted: async (topic: string) => { + await waitForEvent( + apiEventService, + topic, + API_EVENT_KINDS.features__csv__import__finished__v1__alpha, + ); + }, + ThenCSVImportFailedEventWasSubmitted: async (topic: string) => { + await waitForEvent( + apiEventService, + topic, + API_EVENT_KINDS.features__csv__import__failed__v1__alpha, + ); + }, + + ThenShapefileImportSubmittedEventWasSubmitted: async (topic: string) => { + await waitForEvent( + apiEventService, + topic, + API_EVENT_KINDS.features__shapefile__import__submitted__v1__alpha, + ); + }, + ThenShapefileImportFinishedEventWasSubmitted: async (topic: string) => { + await waitForEvent( + apiEventService, + topic, + API_EVENT_KINDS.features__shapefile__import__finished__v1__alpha, + ); + }, + ThenShapefileImportFailedEventWasSubmitted: async (topic: string) => { + await waitForEvent( + apiEventService, + topic, + API_EVENT_KINDS.features__shapefile__import__failed__v1__alpha, + ); + }, + ThenMaxLengthErrorWasReturned: (response: request.Response) => { const error: any = response.body.errors[0].meta.rawError.response.message[0]; @@ -309,13 +379,9 @@ export const getFixtures = async () => { result: request.Response, name: string, description: string, - tag?: string, ) => { // Check response payload, in JSON:API format - expect(result.body?.data?.type).toEqual('geo_features'); - expect(result.body?.data?.attributes?.isCustom).toEqual(true); - expect(result.body?.data?.attributes?.featureClassName).toEqual(name); - expect(result.body?.data?.attributes?.tag).toEqual(tag); + expect(result.status).toBe(HttpStatus.CREATED); const features = await geoFeaturesApiRepo.find({ where: { @@ -333,7 +399,7 @@ export const getFixtures = async () => { amountMin: 820348505.9774874, propertyName: null, intersection: null, - creationStatus: 'created', + creationStatus: JobStatus.created, projectId, isCustom: true, isLegacy: false, @@ -382,6 +448,7 @@ export const getFixtures = async () => { expect(newFeaturesAdded[1].isLegacy).toBe(true); expect(newFeaturesAdded[0].amountMin).toEqual(3.245387225); expect(newFeaturesAdded[0].amountMax).toEqual(4.245387225); + expect(newFeaturesAdded[0].creationStatus).toEqual(JobStatus.created); }, ThenNewFeaturesAmountsAreCreated: async () => { @@ -465,22 +532,23 @@ export const getFixtures = async () => { expect(featureImportRegistryRecord?.projectId).toBeUndefined(); expect(featureImportRegistryRecord?.uploadedFeatures).toBeUndefined(); }, - ThenMissingPUIDErrorIsReturned: async (result: request.Response) => { - expect(result.body.errors[0].status).toEqual(HttpStatus.BAD_REQUEST); - expect(result.body.errors[0].title).toEqual('Missing PUID column'); + ThenMissingPUIDErrorIsReturned: (event: ApiEventByTopicAndKind) => { + expect(event.data?.name).toEqual(BadRequestException.name); + expect(event.data?.message).toEqual('Missing PUID column'); }, - ThenNoFeaturesInCsvFileErrorIsReturned: async ( - result: request.Response, - ) => { - expect(result.body.errors[0].status).toEqual(HttpStatus.BAD_REQUEST); + ThenNoFeaturesInCsvFileErrorIsReturned: (event: ApiEventByTopicAndKind) => { + expect(event.data?.name).toEqual(BadRequestException.name); + expect(event.data?.message).toEqual( + 'No features found in feature amount CSV upload', + ); }, - ThenDuplicatedPUIDErrorIsReturned: async (result: request.Response) => { - expect(result.body.errors[0].status).toEqual(HttpStatus.BAD_REQUEST); - expect(result.body.errors[0].title).toEqual( + ThenDuplicatedPUIDErrorIsReturned: (event: ApiEventByTopicAndKind) => { + expect(event.data?.name).toEqual(BadRequestException.name); + expect(event.data?.message).toEqual( 'Duplicate PUIDs in feature amount CSV upload', ); }, - ThenProjectNotFoundErrorIsReturned: async ( + ThenProjectNotFoundErrorIsReturned: ( result: request.Response, falseProjectId: string, ) => { @@ -489,15 +557,15 @@ export const getFixtures = async () => { `Project with id ${falseProjectId} not found`, ); }, - ThenDuplicatedHeaderErrorIsReturned: async (result: request.Response) => { - expect(result.body.errors[0].status).toEqual(HttpStatus.BAD_REQUEST); - expect(result.body.errors[0].title).toEqual( + ThenDuplicatedHeaderErrorIsReturned: (event: ApiEventByTopicAndKind) => { + expect(event.data?.name).toEqual(BadRequestException.name); + expect(event.data?.message).toEqual( 'Duplicate headers found ["feat_1d666bd"]', ); }, - ThenPuidsNotPresentErrorIsReturned: async (result: request.Response) => { - expect(result.body.errors[0].status).toEqual(HttpStatus.BAD_REQUEST); - expect(result.body.errors[0].title).toEqual('Unknown PUIDs'); + ThenPuidsNotPresentErrorIsReturned: (event: ApiEventByTopicAndKind) => { + expect(event.data?.name).toEqual(BadRequestException.name); + expect(event.data?.message).toEqual('Unknown PUIDs'); }, AndNoFeatureUploadIsRegistered: async () => { const featureImportRegistryRecord = await featureImportRegistry.findOne({ diff --git a/api/apps/api/test/utils/project-checker.service-fake.ts b/api/apps/api/test/utils/project-checker.service-fake.ts index e38e3ef45a..5ccd90096c 100644 --- a/api/apps/api/test/utils/project-checker.service-fake.ts +++ b/api/apps/api/test/utils/project-checker.service-fake.ts @@ -15,6 +15,7 @@ export class ProjectCheckerFake implements ProjectChecker { private projectsWithPendingExports: string[]; private projectsThatAreNotReady: string[]; private projectsWithPendingImports: string[]; + private projectsWithPendingFeatures: string[]; constructor( @InjectRepository(Project) @@ -24,6 +25,7 @@ export class ProjectCheckerFake implements ProjectChecker { this.projectsWithPendingExports = []; this.projectsThatAreNotReady = []; this.projectsWithPendingImports = []; + this.projectsWithPendingFeatures = []; } async hasPendingImports( @@ -130,6 +132,17 @@ export class ProjectCheckerFake implements ProjectChecker { return right(results.some((pendingMarxanRun) => pendingMarxanRun)); } + async hasPendingFeatures( + projectId: string, + ): Promise> { + const project = await this.projectRepo.findOne({ + where: { id: projectId }, + }); + if (!project) return left(doesntExist); + + return right(this.projectsWithPendingFeatures.includes(projectId)); + } + async isProjectReady( projectId: string, ): Promise> { @@ -146,9 +159,14 @@ export class ProjectCheckerFake implements ProjectChecker { this.projectsWithPendingImports.push(projectId); } + addPendingFeatureForProject(projectId: string) { + this.projectsWithPendingFeatures.push(projectId); + } + clear() { this.projectsThatAreNotReady = []; this.projectsWithPendingExports = []; this.projectsWithPendingImports = []; + this.projectsWithPendingFeatures = []; } } diff --git a/api/apps/api/test/utils/test-client/seed/features.ts b/api/apps/api/test/utils/test-client/seed/features.ts index 77420ebb1e..e7a011d967 100644 --- a/api/apps/api/test/utils/test-client/seed/features.ts +++ b/api/apps/api/test/utils/test-client/seed/features.ts @@ -1,6 +1,7 @@ import { Connection } from 'typeorm'; import * as fs from 'fs/promises'; import * as path from 'path'; +import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; // TODO: Is this only needed for Project and GeoFeatures tests? export const insertFeatures = async ( @@ -13,15 +14,17 @@ export const insertFeatures = async ( }[] = await apiConnection.query(`INSERT INTO features (feature_class_name, alias, description, property_name, intersection, creation_status, created_by) VALUES - ('demo_acinonyx_jubatus', 'Acinonyx_jubatus', null, ' name', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')), - ('demo_bucorvus_leadbeateri', 'Bucorvus_leadbeateri', null, ' id', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')), - ('demo_ceratotherium_simum', 'Ceratotherium_simum', null, ' name', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')), - ('demo_civettictis_civetta', 'Civettictis_civetta', null, ' id', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')), - ('demo_diceros_bicornis', 'Diceros_bicornis', null, ' name', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')), - ('demo_equus_quagga', 'Equus_quagga', null, ' name', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')), - ('demo_giraffa_camelopardalis', 'Giraffa_camelopardalis', null, ' name', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')), - ('demo_kobus_leche', 'Kobus_leche', null, ' name', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')), - ('demo_panthera_pardus', 'Panthera_pardus', null, ' name', null, 'created', (SELECT id FROM users WHERE email = 'aa@example.com')) + ('demo_acinonyx_jubatus', 'Acinonyx_jubatus', null, ' name', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('demo_bucorvus_leadbeateri', 'Bucorvus_leadbeateri', null, ' id', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('demo_ceratotherium_simum', 'Ceratotherium_simum', null, ' name', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('demo_civettictis_civetta', 'Civettictis_civetta', null, ' id', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('demo_diceros_bicornis', 'Diceros_bicornis', null, ' name', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('demo_equus_quagga', 'Equus_quagga', null, ' name', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('demo_giraffa_camelopardalis', 'Giraffa_camelopardalis', null, ' name', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('demo_kobus_leche', 'Kobus_leche', null, ' name', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('demo_panthera_pardus', 'Panthera_pardus', null, ' name', null, '${JobStatus.created}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('failed_feature', 'FAILED Feature', null, ' name', null, '${JobStatus.failure}', (SELECT id FROM users WHERE email = 'aa@example.com')), + ('running_feature', 'RUNNING Feature', null, ' name', null, '${JobStatus.running}', (SELECT id FROM users WHERE email = 'aa@example.com')) RETURNING feature_class_name, id; `); diff --git a/api/apps/api/test/utils/wait-for-feature-to-be-ready.utils.ts b/api/apps/api/test/utils/wait-for-feature-to-be-ready.utils.ts index 879f0a4140..d81cb1a18c 100644 --- a/api/apps/api/test/utils/wait-for-feature-to-be-ready.utils.ts +++ b/api/apps/api/test/utils/wait-for-feature-to-be-ready.utils.ts @@ -2,7 +2,37 @@ import { JobStatus } from '@marxan-api/modules/scenarios/scenario.api.entity'; import { GeoFeature } from '@marxan-api/modules/geo-features/geo-feature.api.entity'; import { Repository } from 'typeorm'; import { waitFor } from './wait-for.utils'; +import { ApiEventsService } from '@marxan-api/modules/api-events'; +import { API_EVENT_KINDS } from '@marxan/api-events'; +export const waitForEvent = async ( + eventsService: ApiEventsService, + topic: string, + kind: API_EVENT_KINDS, +) => { + const checkFn = async () => { + try { + await eventsService.getLatestEventForTopic({ + topic, + kind, + }); + return true; + } catch (e) { + return false; + } + }; + + await waitFor( + { + description: 'event to appear', + fn: checkFn, + }, + { + maxTries: 10, + intervalMs: 200, + }, + ); +}; export const waitForFeatureToBeReady = async ( geoFeaturesApiRepo: Repository, featureId: string, diff --git a/api/apps/geoprocessing/test/integration/cloning/fixtures.ts b/api/apps/geoprocessing/test/integration/cloning/fixtures.ts index 5d32fcea35..e20c7b428a 100644 --- a/api/apps/geoprocessing/test/integration/cloning/fixtures.ts +++ b/api/apps/geoprocessing/test/integration/cloning/fixtures.ts @@ -468,7 +468,7 @@ export async function GivenFeatures( .map((_, index) => ({ id: v4(), feature_class_name: `custom-${projectId}-${index + 1}`, - creation_status: 'done', + creation_status: 'created', project_id: projectId, is_legacy: isLegacy, })); diff --git a/api/libs/api-events/src/api-event-kinds.enum.ts b/api/libs/api-events/src/api-event-kinds.enum.ts index 0fdb7e0ad0..fe9e86d155 100644 --- a/api/libs/api-events/src/api-event-kinds.enum.ts +++ b/api/libs/api-events/src/api-event-kinds.enum.ts @@ -104,6 +104,9 @@ export enum API_EVENT_KINDS { features__csv__import__submitted__v1__alpha = 'features.csv.import.submitted/v1/alpha', features__csv__import__finished__v1__alpha = 'features.csv.import.finished/v1/alpha', features__csv__import__failed__v1__alpha = 'features.csv.import.failed/v1/alpha', + features__shapefile__import__submitted__v1__alpha = 'features.shapefile.import.submitted/v1/alpha', + features__shapefile__import__finished__v1__alpha = 'features.shapefile.import.finished/v1/alpha', + features__shapefile__import__failed__v1__alpha = 'features.shapefile.import.failed/v1/alpha', } export type ProjectEvents = Pick<