From 9c506e586616ac58073cdc5ac7ffa49b1be8c6bb Mon Sep 17 00:00:00 2001 From: kgajowy Date: Mon, 11 Oct 2021 11:09:11 +0200 Subject: [PATCH 1/3] refactor(geo-features): remove unused code --- .../geo-features/geo-feature-set.service.ts | 182 +----------------- .../geo-features/geo-features.module.ts | 11 +- .../modules/scenarios/scenarios.controller.ts | 31 --- 3 files changed, 4 insertions(+), 220 deletions(-) diff --git a/api/apps/api/src/modules/geo-features/geo-feature-set.service.ts b/api/apps/api/src/modules/geo-features/geo-feature-set.service.ts index 04ef3fc1f6..7b959a37e0 100644 --- a/api/apps/api/src/modules/geo-features/geo-feature-set.service.ts +++ b/api/apps/api/src/modules/geo-features/geo-feature-set.service.ts @@ -2,43 +2,13 @@ import { JSONAPISerializerConfig, PaginationMeta, } from '@marxan-api/utils/app-base.service'; -import { Inject, Injectable, Logger } from '@nestjs/common'; -import { - GeoFeatureSetSpecification, - SpecForGeofeature, - SpecForGeoFeatureWithGeoprocessing, - SpecForPlainGeoFeature, -} from './dto/geo-feature-set-specification.dto'; +import { Injectable } from '@nestjs/common'; +import { GeoFeatureSetSpecification } from './dto/geo-feature-set-specification.dto'; import * as JSONAPISerializer from 'jsonapi-serializer'; import { GeoFeatureSetResult } from './geo-feature-set.api.entity'; -import { InjectRepository } from '@nestjs/typeorm'; -import { Scenario } from '../scenarios/scenario.api.entity'; -import { EntityManager, Repository } from 'typeorm'; -import { GeoFeaturePropertySetService } from './geo-feature-property-sets.service'; -import { ScenarioFeaturesData } from '@marxan/features'; -import { flattenDeep } from 'lodash'; -import { GeoprocessingOpSplitV1 } from './types/geo-feature.geoprocessing-operations.type'; -import { RemoteFeaturesData } from '../scenarios-features/entities/remote-features-data.geo.entity'; -import { plainToClass } from 'class-transformer'; - -export const EntityManagerToken = Symbol(); - -export const MarxanFeaturesMetadata = { - defaults: { - fpf: 1, - }, -}; @Injectable() export class GeoFeatureSetService { - constructor( - @InjectRepository(Scenario) - private readonly scenarioRepository: Repository, - private readonly geoFeaturePropertySetService: GeoFeaturePropertySetService, - @Inject(EntityManagerToken) - private readonly entityManager: EntityManager, - ) {} - get serializerConfig(): JSONAPISerializerConfig { return { attributes: ['status', 'features'], @@ -60,152 +30,4 @@ export class GeoFeatureSetService { return serializer.serialize(entities); } - - /** - * Create or replace the set of features linked to a scenario. - */ - async createOrReplaceFeatureSet( - id: string, - dto: GeoFeatureSetSpecification, - ): Promise { - const scenario = await this.scenarioRepository.findOneOrFail(id); - await this.scenarioRepository.update(id, { featureSet: dto }); - // @todo: move to async job - this was just for simple tests - await this.createFeaturesForScenario(id, dto.features); - return await this.geoFeaturePropertySetService.extendGeoFeatureProcessingSpecification( - dto, - scenario, - ); - } - - /** - * Given a specification for features to be linked to a scenario, compute - * features defined via geoprocessing operations and link plain features - * and features from geoprocessing to the scenario. - */ - async createFeaturesForScenario( - scenarioId: string, - featureSpecification: SpecForGeofeature[], - ): Promise { - await this.entityManager.transaction(async (transactionalEntityManager) => { - const repository = transactionalEntityManager.getRepository( - ScenarioFeaturesData, - ); - await repository.delete({ scenarioId }); - // First process features which can be used as they are (plain) - await this.createPlainFeaturesForScenario( - transactionalEntityManager, - scenarioId, - featureSpecification, - ); - // Then process features from geoprocessing operations of kind `split/v1` - // TODO - // Then process features from geoprocessing operations of kind `stratification/v1` - // TODO - }); - } - - /** - * Given a specification for features to be linked to a scenario, select plain - * features (i.e. no geoprocessing required) and link them to the scenario as - * part of an ongoing db transaction. - * - * @todo fix typing - */ - async createPlainFeaturesForScenario( - transactionalEntityManager: EntityManager, - scenarioId: string, - featureSpecification: SpecForGeofeature[], - ): Promise< - { - scenarioId: string; - featuresDataId: string; - fpf?: number; - prop?: number; - }[][] - > { - const scenarioFeaturesData = Promise.all( - featureSpecification - .filter( - (feature): feature is SpecForPlainGeoFeature => - !('geoprocessingOperations' in feature), - ) - .map(async (feature) => { - const featuresData: Partial[] = await transactionalEntityManager - .find(RemoteFeaturesData, { featureId: feature.featureId }) - .then((result) => - result.map((fd) => ({ - scenarioId, - featuresDataId: fd.id, - fpf: - feature.marxanSettings?.fpf ?? - MarxanFeaturesMetadata.defaults.fpf, - prop: feature.marxanSettings?.prop, - })), - ); - - return transactionalEntityManager.save( - plainToClass(ScenarioFeaturesData, featuresData), - ); - }), - ); - return scenarioFeaturesData; - } - - /** - * Given a specification for features to be linked to a scenario, select plain - * features (i.e. no geoprocessing required) and link them to the scenario as - * part of an ongoing db transaction. - * - * @todo fix typing - */ - async createGeoprocessedFeaturesForScenarioWithSplitV1( - transactionalEntityManager: EntityManager, - scenarioId: string, - featureSpecification: SpecForGeofeature[], - ): Promise { - // ({ - // scenarioId: string; - // featuresDataId: string; - // fpf: number; - // prop: number | undefined; - // } & RemoteScenarioFeaturesData)[] - const _repository = transactionalEntityManager.getRepository( - ScenarioFeaturesData, - ); - return Promise.all( - featureSpecification - .filter( - (feature): feature is SpecForGeoFeatureWithGeoprocessing => - 'geoprocessingOperations' in feature, - ) - .map((feature) => { - const splitOperations = flattenDeep( - feature.geoprocessingOperations - ?.filter( - (operation): operation is GeoprocessingOpSplitV1 => - operation.kind === 'split/v1', - ) - .map((operation) => - operation.splits.map((split) => ({ - featureId: feature.featureId, - splitByProperty: operation.splitByProperty, - value: split.value, - marxanSettings: split.marxanSettings, - })), - ), - ); - - Logger.debug(splitOperations); - // return repository.save({ - // scenarioId, - // featuresDataId: feature.featureId, - // fpf: - // feature.marxanSettings?.fpf ?? - // MarxanFeaturesMetadata.defaults.fpf, - // prop: feature.marxanSettings?.prop, - // }); - }), - ); - } } 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 d65e719c1a..57d2246bab 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 @@ -1,5 +1,5 @@ import { Module } from '@nestjs/common'; -import { getEntityManagerToken, TypeOrmModule } from '@nestjs/typeorm'; +import { TypeOrmModule } from '@nestjs/typeorm'; import { Project } from '@marxan-api/modules/projects/project.api.entity'; import { GeoFeature } from './geo-feature.api.entity'; import { @@ -12,10 +12,7 @@ import { GeoFeaturesService } from './geo-features.service'; import { ProxyService } from '@marxan-api/modules/proxy/proxy.service'; import { Scenario } from '../scenarios/scenario.api.entity'; import { GeoFeatureSetSerializer } from './geo-feature-set.serializer'; -import { - EntityManagerToken, - GeoFeatureSetService, -} from './geo-feature-set.service'; +import { GeoFeatureSetService } from './geo-feature-set.service'; import { ScenarioFeaturesData } from '@marxan/features'; import { GeoFeaturePropertySetService } from './geo-feature-property-sets.service'; import { ProcessingModule } from './processing'; @@ -36,10 +33,6 @@ import { DbConnections } from '@marxan-api/ormconfig.connections'; GeoFeatureSetService, GeoFeaturePropertySetService, ProxyService, - { - provide: EntityManagerToken, - useExisting: getEntityManagerToken(DbConnections.geoprocessingDB), - }, ], controllers: [GeoFeaturesController], exports: [ diff --git a/api/apps/api/src/modules/scenarios/scenarios.controller.ts b/api/apps/api/src/modules/scenarios/scenarios.controller.ts index 6d8976e860..0d094148fd 100644 --- a/api/apps/api/src/modules/scenarios/scenarios.controller.ts +++ b/api/apps/api/src/modules/scenarios/scenarios.controller.ts @@ -71,9 +71,7 @@ import { ProxyService } from '@marxan-api/modules/proxy/proxy.service'; import { ZipFilesSerializer } from './dto/zip-files.serializer'; import { ScenarioPlanningUnitSerializer } from './dto/scenario-planning-unit.serializer'; import { GeoFeatureSetSerializer } from '../geo-features/geo-feature-set.serializer'; -import { UpdateGeoFeatureSetDTO } from '../geo-features/dto/update.geo-feature-set.dto'; import { CreateGeoFeatureSetDTO } from '../geo-features/dto/create.geo-feature-set.dto'; -import { GeoFeatureSetService } from '../geo-features/geo-feature-set.service'; import { ScenarioPlanningUnitDto } from './dto/scenario-planning-unit.dto'; import { isLeft } from 'fp-ts/Either'; import { ScenarioFeaturesGapDataService } from '../scenarios-features/scenario-features-gap-data.service'; @@ -104,7 +102,6 @@ export class ScenariosController { private readonly scenarioFeaturesGapDataService: ScenarioFeaturesGapDataService, private readonly scenarioFeaturesOutputGapDataService: ScenarioFeaturesOutputGapDataService, private readonly geoFeatureSetSerializer: GeoFeatureSetSerializer, - private readonly geoFeatureSetService: GeoFeatureSetService, private readonly scenarioSerializer: ScenarioSerializer, private readonly scenarioFeaturesGapData: ScenarioFeaturesGapDataSerializer, private readonly scenarioFeaturesOutputGapData: ScenarioFeaturesOutputGapDataSerializer, @@ -250,34 +247,6 @@ export class ScenariosController { ); } - @ApiOperation({ description: 'Create feature set for scenario' }) - @ApiCreatedResponse({ type: GeoFeatureSetResult }) - @Post(':id/features/specification') - async createFeatureSetFor( - @Body(new ValidationPipe()) dto: CreateGeoFeatureSetDTO, - @Param('id', ParseUUIDPipe) id: string, - ): Promise { - // TODO once `copy` is in place, replace with implementation as in - // id/features/specification/v2 - return await this.geoFeatureSetSerializer.serialize( - await this.geoFeatureSetService.createOrReplaceFeatureSet(id, dto), - ); - } - - @ApiOperation({ description: 'Update feature set for scenario' }) - @ApiOkResponse({ type: GeoFeatureSetResult }) - @Put(':id/features/specification') - async updateFeatureSetFor( - @Body(new ValidationPipe()) dto: UpdateGeoFeatureSetDTO, - @Param('id', ParseUUIDPipe) id: string, - ): Promise { - // TODO once `copy` is in place, replace with implementation as in - // id/features/specification/v2 - return await this.geoFeatureSetSerializer.serialize( - await this.geoFeatureSetService.createOrReplaceFeatureSet(id, dto), - ); - } - @ApiOperation({ description: 'Get feature set for scenario' }) @ApiOkResponse({ type: GeoFeatureSetResult }) @Get(':id/features/specification') From 361347385208905f70cebbb59030c12fe1b2bdb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Barrenechea=20S=C3=A1nchez?= Date: Thu, 14 Oct 2021 10:11:01 +0200 Subject: [PATCH 2/3] refactor(geo-features): remove v2 from endpoint --- app/hooks/features/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/hooks/features/index.ts b/app/hooks/features/index.ts index 31cb26303d..56178f9d72 100644 --- a/app/hooks/features/index.ts +++ b/app/hooks/features/index.ts @@ -477,7 +477,7 @@ export function useSaveSelectedFeatures({ const saveFeature = ({ id, data }: SaveSelectedFeaturesProps) => { return SCENARIOS.request({ - url: `/${id}/features/specification/v2`, + url: `/${id}/features/specification`, data, headers: { Authorization: `Bearer ${session.accessToken}`, From 419b5fd3bbc8e7e85a37a0d2585a93da0c6bcb51 Mon Sep 17 00:00:00 2001 From: kgajowy Date: Thu, 14 Oct 2021 10:45:17 +0200 Subject: [PATCH 3/3] refactor(scenarios): replace temporary specification-v2 with v1 --- api/apps/api/src/modules/scenarios/scenarios.controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/apps/api/src/modules/scenarios/scenarios.controller.ts b/api/apps/api/src/modules/scenarios/scenarios.controller.ts index 0d094148fd..4bf923dcd6 100644 --- a/api/apps/api/src/modules/scenarios/scenarios.controller.ts +++ b/api/apps/api/src/modules/scenarios/scenarios.controller.ts @@ -231,7 +231,7 @@ export class ScenariosController { @ApiOperation({ description: 'Create feature set for scenario' }) @ApiTags(asyncJobTag) - @Post(':id/features/specification/v2') + @Post(':id/features/specification') async createSpecification( @Body(new ValidationPipe()) dto: CreateGeoFeatureSetDTO, @Param('id', ParseUUIDPipe) id: string,