From 3c79e91831d222254ddcf35480fd6cff2b71f770 Mon Sep 17 00:00:00 2001 From: Kamil Gajowy Date: Mon, 31 May 2021 09:28:31 +0200 Subject: [PATCH 1/8] refactor: create library for scenario planning units --- .../src/scenarios-planning-unit.geo.entity.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/libs/scenarios-planning-unit/src/scenarios-planning-unit.geo.entity.ts b/api/libs/scenarios-planning-unit/src/scenarios-planning-unit.geo.entity.ts index 801a46c0c5..35ab7928a1 100644 --- a/api/libs/scenarios-planning-unit/src/scenarios-planning-unit.geo.entity.ts +++ b/api/libs/scenarios-planning-unit/src/scenarios-planning-unit.geo.entity.ts @@ -1,5 +1,5 @@ import { Column, Entity, PrimaryGeneratedColumn } from 'typeorm'; -import { LockStatus } from './lock-status.enum'; +import { LockStatus } from '@marxan/scenarios-planning-unit'; const scenariosPuDataEntityName = 'scenarios_pu_data'; From 83a6099ea6d5d1e93397dcc119557b84555b157e Mon Sep 17 00:00:00 2001 From: Kamil Gajowy Date: Mon, 31 May 2021 10:32:06 +0200 Subject: [PATCH 2/8] refactor(geoprocessing): move scenarios planning unit entity to lib --- .../src/scenarios-planning-unit.geo.entity.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/libs/scenarios-planning-unit/src/scenarios-planning-unit.geo.entity.ts b/api/libs/scenarios-planning-unit/src/scenarios-planning-unit.geo.entity.ts index 35ab7928a1..801a46c0c5 100644 --- a/api/libs/scenarios-planning-unit/src/scenarios-planning-unit.geo.entity.ts +++ b/api/libs/scenarios-planning-unit/src/scenarios-planning-unit.geo.entity.ts @@ -1,5 +1,5 @@ import { Column, Entity, PrimaryGeneratedColumn } from 'typeorm'; -import { LockStatus } from '@marxan/scenarios-planning-unit'; +import { LockStatus } from './lock-status.enum'; const scenariosPuDataEntityName = 'scenarios_pu_data'; From a21257bcbd22a27c76ccc12cce98dbd123cc8b8f Mon Sep 17 00:00:00 2001 From: Kamil Gajowy Date: Mon, 31 May 2021 12:24:51 +0200 Subject: [PATCH 3/8] feat(api): scenario planning units: integrity rule --- .../src/domain/index.ts | 1 + .../src/domain/lock-planning-units.spec.ts | 22 +++++++++++++++++++ .../src/domain/lock-planning-units.ts | 11 ++++++++++ api/libs/scenarios-planning-unit/src/index.ts | 1 + 4 files changed, 35 insertions(+) create mode 100644 api/libs/scenarios-planning-unit/src/domain/index.ts create mode 100644 api/libs/scenarios-planning-unit/src/domain/lock-planning-units.spec.ts create mode 100644 api/libs/scenarios-planning-unit/src/domain/lock-planning-units.ts diff --git a/api/libs/scenarios-planning-unit/src/domain/index.ts b/api/libs/scenarios-planning-unit/src/domain/index.ts new file mode 100644 index 0000000000..bc78d0c9a8 --- /dev/null +++ b/api/libs/scenarios-planning-unit/src/domain/index.ts @@ -0,0 +1 @@ +export { lockPlanningUnits } from './lock-planning-units'; diff --git a/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.spec.ts b/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.spec.ts new file mode 100644 index 0000000000..f908f3ed99 --- /dev/null +++ b/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.spec.ts @@ -0,0 +1,22 @@ +import { lockPlanningUnits } from './lock-planning-units'; + +const fixtures = { + availablePlanningUnits: ['1', '2', '3', '4'], + someOfTheAvailable: ['1', '4'], + withNonAvailable: ['1', '5'], +}; + +test.each([ + [fixtures.availablePlanningUnits, fixtures.availablePlanningUnits, 0], + [fixtures.someOfTheAvailable, fixtures.availablePlanningUnits, 0], + [fixtures.withNonAvailable, fixtures.availablePlanningUnits, 1], + [[], fixtures.availablePlanningUnits, 0], + [fixtures.someOfTheAvailable, [], fixtures.someOfTheAvailable.length], +])( + `when changing %p out of %p, it returns %p errors`, + (unitsToChange, availableUnits, errorsCount) => { + expect( + lockPlanningUnits(unitsToChange, availableUnits).errors.length, + ).toEqual(errorsCount); + }, +); diff --git a/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.ts b/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.ts new file mode 100644 index 0000000000..74a4127d81 --- /dev/null +++ b/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.ts @@ -0,0 +1,11 @@ +import { differenceWith } from 'lodash'; + +export const lockPlanningUnits = ( + planningUnitsIds: string[], + availablePlanningUnitsIds: string[], +): { errors: string[] } => { + const diff = differenceWith(planningUnitsIds, availablePlanningUnitsIds); + return { + errors: diff.map((missingId) => `Missing ${missingId}`), + }; +}; diff --git a/api/libs/scenarios-planning-unit/src/index.ts b/api/libs/scenarios-planning-unit/src/index.ts index e5c84cb4a9..f44a119ae4 100644 --- a/api/libs/scenarios-planning-unit/src/index.ts +++ b/api/libs/scenarios-planning-unit/src/index.ts @@ -1,2 +1,3 @@ export { LockStatus } from './lock-status.enum'; export { ScenariosPlanningUnitGeoEntity } from './scenarios-planning-unit.geo.entity'; +export * from './domain'; From 40e5835934412ac20eaac1d230390bbf4a7527ed Mon Sep 17 00:00:00 2001 From: Kamil Gajowy Date: Tue, 1 Jun 2021 11:15:24 +0200 Subject: [PATCH 4/8] chore(libs): scenarios-planning-unit: domain method name & more test cases --- .../can-planning-units-be-locked.spec.ts | 38 +++++++++++++++++++ .../domain/can-planning-units-be-locked.ts | 14 +++++++ .../src/domain/index.ts | 2 +- .../src/domain/lock-planning-units.spec.ts | 22 ----------- .../src/domain/lock-planning-units.ts | 11 ------ 5 files changed, 53 insertions(+), 34 deletions(-) create mode 100644 api/libs/scenarios-planning-unit/src/domain/can-planning-units-be-locked.spec.ts create mode 100644 api/libs/scenarios-planning-unit/src/domain/can-planning-units-be-locked.ts delete mode 100644 api/libs/scenarios-planning-unit/src/domain/lock-planning-units.spec.ts delete mode 100644 api/libs/scenarios-planning-unit/src/domain/lock-planning-units.ts diff --git a/api/libs/scenarios-planning-unit/src/domain/can-planning-units-be-locked.spec.ts b/api/libs/scenarios-planning-unit/src/domain/can-planning-units-be-locked.spec.ts new file mode 100644 index 0000000000..e860f2d4c7 --- /dev/null +++ b/api/libs/scenarios-planning-unit/src/domain/can-planning-units-be-locked.spec.ts @@ -0,0 +1,38 @@ +import { canPlanningUnitsBeLocked } from './can-planning-units-be-locked'; +import { v4 } from 'uuid'; + +const availablePlanningUnits = ['1', '2', '3', '4']; + +const fixtures = { + availablePlanningUnits, + someOfTheAvailable: [availablePlanningUnits[0], availablePlanningUnits[2]], + withNonAvailable: [v4(), v4()], + everyAvailable: availablePlanningUnits, + allAvailableAndOneMissing: [...availablePlanningUnits, v4()], +}; + +type UnitsToChange = string[]; +type AvailablePlanningUnits = string[]; +type ErrorsCount = number; +type TestCase = [UnitsToChange, AvailablePlanningUnits, ErrorsCount]; + +test.each([ + [fixtures.availablePlanningUnits, fixtures.availablePlanningUnits, 0], + [fixtures.someOfTheAvailable, fixtures.availablePlanningUnits, 0], + [ + fixtures.withNonAvailable, + fixtures.availablePlanningUnits, + fixtures.withNonAvailable.length, + ], + [[], fixtures.availablePlanningUnits, 0], + [fixtures.someOfTheAvailable, [], fixtures.someOfTheAvailable.length], + [fixtures.everyAvailable, fixtures.availablePlanningUnits, 0], + [fixtures.allAvailableAndOneMissing, fixtures.availablePlanningUnits, 1], +])( + `when changing %p out of %p, it returns %p errors count`, + (unitsToChange, availableUnits, errorsCount) => { + expect( + canPlanningUnitsBeLocked(unitsToChange, availableUnits).errors.length, + ).toEqual(errorsCount); + }, +); diff --git a/api/libs/scenarios-planning-unit/src/domain/can-planning-units-be-locked.ts b/api/libs/scenarios-planning-unit/src/domain/can-planning-units-be-locked.ts new file mode 100644 index 0000000000..edff9cc805 --- /dev/null +++ b/api/libs/scenarios-planning-unit/src/domain/can-planning-units-be-locked.ts @@ -0,0 +1,14 @@ +import { difference } from 'lodash'; + +export const canPlanningUnitsBeLocked = ( + planningUnitsIds: string[], + availablePlanningUnitsIds: string[], +): { errors: string[] } => { + const diff = difference(planningUnitsIds, availablePlanningUnitsIds); + return { + errors: diff.map( + (missingId) => + `Planning Unit with id ${missingId} is not a part of this project`, + ), + }; +}; diff --git a/api/libs/scenarios-planning-unit/src/domain/index.ts b/api/libs/scenarios-planning-unit/src/domain/index.ts index bc78d0c9a8..c27a32390b 100644 --- a/api/libs/scenarios-planning-unit/src/domain/index.ts +++ b/api/libs/scenarios-planning-unit/src/domain/index.ts @@ -1 +1 @@ -export { lockPlanningUnits } from './lock-planning-units'; +export { canPlanningUnitsBeLocked } from './can-planning-units-be-locked'; diff --git a/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.spec.ts b/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.spec.ts deleted file mode 100644 index f908f3ed99..0000000000 --- a/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.spec.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { lockPlanningUnits } from './lock-planning-units'; - -const fixtures = { - availablePlanningUnits: ['1', '2', '3', '4'], - someOfTheAvailable: ['1', '4'], - withNonAvailable: ['1', '5'], -}; - -test.each([ - [fixtures.availablePlanningUnits, fixtures.availablePlanningUnits, 0], - [fixtures.someOfTheAvailable, fixtures.availablePlanningUnits, 0], - [fixtures.withNonAvailable, fixtures.availablePlanningUnits, 1], - [[], fixtures.availablePlanningUnits, 0], - [fixtures.someOfTheAvailable, [], fixtures.someOfTheAvailable.length], -])( - `when changing %p out of %p, it returns %p errors`, - (unitsToChange, availableUnits, errorsCount) => { - expect( - lockPlanningUnits(unitsToChange, availableUnits).errors.length, - ).toEqual(errorsCount); - }, -); diff --git a/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.ts b/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.ts deleted file mode 100644 index 74a4127d81..0000000000 --- a/api/libs/scenarios-planning-unit/src/domain/lock-planning-units.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { differenceWith } from 'lodash'; - -export const lockPlanningUnits = ( - planningUnitsIds: string[], - availablePlanningUnitsIds: string[], -): { errors: string[] } => { - const diff = differenceWith(planningUnitsIds, availablePlanningUnitsIds); - return { - errors: diff.map((missingId) => `Missing ${missingId}`), - }; -}; From b0cd880168fae768e9811ee3e94870bce5e2866f Mon Sep 17 00:00:00 2001 From: Kamil Gajowy Date: Mon, 31 May 2021 12:55:02 +0200 Subject: [PATCH 5/8] feat(geoprocessing): scenario planning units: use integrity rule --- .../typeorm-available-planning-units.ts | 28 +++++++++++++ .../__mocks__/are-puids-allowed.fake.ts | 14 ------- .../application/__mocks__/cost.ts | 13 ++---- .../application/__mocks__/geojson.ts | 38 ++++++----------- .../__mocks__/get-available-puids.fake.ts | 11 +++++ .../surface-cost-processor.spec.ts | 41 ++++++++++--------- .../application/surface-cost-processor.ts | 13 ++++-- .../get-available-planning-units.ts | 7 ++++ .../pu-validator/are-puuids-allowed.port.ts | 8 ---- .../surface-cost/surface-cost.module.ts | 11 ++--- 10 files changed, 99 insertions(+), 85 deletions(-) create mode 100644 api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-available-planning-units.ts delete mode 100644 api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/are-puids-allowed.fake.ts create mode 100644 api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/get-available-puids.fake.ts create mode 100644 api/apps/geoprocessing/src/modules/surface-cost/ports/available-planning-units/get-available-planning-units.ts delete mode 100644 api/apps/geoprocessing/src/modules/surface-cost/ports/pu-validator/are-puuids-allowed.port.ts diff --git a/api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-available-planning-units.ts b/api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-available-planning-units.ts new file mode 100644 index 0000000000..35b3cc1800 --- /dev/null +++ b/api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-available-planning-units.ts @@ -0,0 +1,28 @@ +import { Repository } from 'typeorm'; +import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; + +import { ScenariosPlanningUnitGeoEntity } from '@marxan/scenarios-planning-unit'; + +import { GetAvailablePlanningUnits } from '../ports/available-planning-units/get-available-planning-units'; + +@Injectable() +export class TypeormAvailablePlanningUnits + implements GetAvailablePlanningUnits { + constructor( + @InjectRepository(ScenariosPlanningUnitGeoEntity) + private readonly repo: Repository, + ) {} + + get(scenarioId: string): Promise<{ ids: string[] }> { + return this.repo + .find({ + where: { + scenarioId, + }, + }) + .then((rows) => ({ + ids: rows.map((row) => row.id), + })); + } +} diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/are-puids-allowed.fake.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/are-puids-allowed.fake.ts deleted file mode 100644 index 873911927f..0000000000 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/are-puids-allowed.fake.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { Injectable } from '@nestjs/common'; -import { ArePuidsAllowedPort } from '../../ports/pu-validator/are-puuids-allowed.port'; - -@Injectable() -export class ArePuidsAllowedFake implements ArePuidsAllowedPort { - mock: jest.Mock> = jest.fn(); - - async validate( - scenarioId: string, - puIds: string[], - ): Promise<{ errors: unknown[] }> { - return this.mock(scenarioId, puIds); - } -} diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/cost.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/cost.ts index 700fe7ecce..36eebbae11 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/cost.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/cost.ts @@ -1,12 +1,7 @@ import { PlanningUnitCost } from '../../ports/planning-unit-cost'; -export const getCost = (): PlanningUnitCost[] => [ - { +export const getCost = (planningUnitsIds: string[]): PlanningUnitCost[] => + planningUnitsIds.map((pu) => ({ cost: 200, - planningUnitId: `puid-1`, - }, - { - cost: 400, - planningUnitId: `puid-2`, - }, -]; + planningUnitId: pu, + })); diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/geojson.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/geojson.ts index 0541ab97f4..425b71d0ef 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/geojson.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/geojson.ts @@ -1,35 +1,21 @@ import { FeatureCollection, MultiPolygon, Polygon } from 'geojson'; import { PlanningUnitCost } from '../../ports/planning-unit-cost'; -export const getGeoJson = (): FeatureCollection< - MultiPolygon | Polygon, - PlanningUnitCost -> => ({ +export const getGeoJson = ( + planningUnitsIds: string[], +): FeatureCollection => ({ type: 'FeatureCollection', - features: [ - { - properties: { - cost: 200, - planningUnitId: 'uuid-1', - }, - type: 'Feature', - geometry: { - type: 'MultiPolygon', - coordinates: [], - }, + features: planningUnitsIds.map((id) => ({ + properties: { + cost: 200, + planningUnitId: id, }, - { - properties: { - cost: 100, - planningUnitId: 'uuid-2', - }, - type: 'Feature', - geometry: { - type: 'Polygon', - coordinates: [], - }, + type: 'Feature', + geometry: { + type: 'MultiPolygon', + coordinates: [], }, - ], + })), }); export const getGeoJsonWithMissingCost = (): FeatureCollection< diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/get-available-puids.fake.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/get-available-puids.fake.ts new file mode 100644 index 0000000000..4ebdc4efa2 --- /dev/null +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/get-available-puids.fake.ts @@ -0,0 +1,11 @@ +import { Injectable } from '@nestjs/common'; +import { GetAvailablePlanningUnits } from '../../ports/available-planning-units/get-available-planning-units'; + +@Injectable() +export class GetAvailablePuidsFake implements GetAvailablePlanningUnits { + mock: jest.Mock> = jest.fn(); + + get(scenarioId: string): Promise<{ ids: string[] }> { + return this.mock(scenarioId); + } +} diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.spec.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.spec.ts index a3ddba661e..6aefb75895 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.spec.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.spec.ts @@ -5,12 +5,12 @@ import { SurfaceCostProcessor } from './surface-cost-processor'; import { CostSurfacePersistencePort } from '../ports/persistence/cost-surface-persistence.port'; import { PuExtractorPort } from '../ports/pu-extractor/pu-extractor.port'; -import { ArePuidsAllowedPort } from '../ports/pu-validator/are-puuids-allowed.port'; +import { GetAvailablePlanningUnits } from '../ports/available-planning-units/get-available-planning-units'; import { ShapefileConverterPort } from '../ports/shapefile-converter/shapefile-converter.port'; import { ShapefileConverterFake } from './__mocks__/shapefile-converter.fake'; import { PuExtractorFake } from './__mocks__/pu-extractor.fake'; -import { ArePuidsAllowedFake } from './__mocks__/are-puids-allowed.fake'; +import { GetAvailablePuidsFake } from './__mocks__/get-available-puids.fake'; import { CostSurfaceRepoFake } from './__mocks__/cost-surface-repo.fake'; import { getJob } from './__mocks__/job.data'; @@ -21,9 +21,12 @@ let sut: SurfaceCostProcessor; let fileConverter: ShapefileConverterFake; let puExtractor: PuExtractorFake; -let puValidator: ArePuidsAllowedFake; +let puRepo: GetAvailablePuidsFake; let repo: CostSurfaceRepoFake; +const availablePlanningUnitIds = ['1', '2', '3']; +const missingPlanningUnitIds = ['1', '4']; + beforeEach(async () => { const sandbox = await Test.createTestingModule({ providers: [ @@ -37,8 +40,8 @@ beforeEach(async () => { useClass: PuExtractorFake, }, { - provide: ArePuidsAllowedPort, - useClass: ArePuidsAllowedFake, + provide: GetAvailablePlanningUnits, + useClass: GetAvailablePuidsFake, }, { provide: ShapefileConverterPort, @@ -50,7 +53,7 @@ beforeEach(async () => { sut = sandbox.get(SurfaceCostProcessor); fileConverter = sandbox.get(ShapefileConverterPort); puExtractor = sandbox.get(PuExtractorPort); - puValidator = sandbox.get(ArePuidsAllowedPort); + puRepo = sandbox.get(GetAvailablePlanningUnits); repo = sandbox.get(CostSurfacePersistencePort); }); @@ -70,7 +73,7 @@ describe(`when shapefile couldn't be converted`, () => { describe(`when shapefile was converted to geojson`, () => { beforeEach(() => { - fileConverter.mock.mockResolvedValue(getGeoJson()); + fileConverter.mock.mockResolvedValue(getGeoJson(availablePlanningUnitIds)); }); describe(`when cost is missing in properties`, () => { @@ -82,43 +85,43 @@ describe(`when shapefile was converted to geojson`, () => { it(`should throw`, async () => { await expect(sut.process(getJob(scenarioId))).rejects.toThrow(/Missing/); - expect(puExtractor.mock).toHaveBeenCalledWith(getGeoJson()); + expect(puExtractor.mock).toHaveBeenCalledWith( + getGeoJson(availablePlanningUnitIds), + ); }); }); describe(`when cost was resolved`, () => { + const cost = getCost(availablePlanningUnitIds); beforeEach(() => { - puExtractor.mock.mockReturnValue(getCost()); + puExtractor.mock.mockReturnValue(cost); }); describe(`when provided PUs do not belong to given scenario`, () => { beforeEach(() => { - puValidator.mock.mockResolvedValue({ - errors: ['You shall not pass!'], + puRepo.mock.mockResolvedValue({ + ids: missingPlanningUnitIds, }); }); it(`should throw`, async () => { await expect(sut.process(getJob(scenarioId))).rejects.toThrow( - /shall not/, - ); - expect(puValidator.mock).toHaveBeenCalledWith( - scenarioId, - getCost().map((c) => c.planningUnitId), + /Missing/, ); + expect(puRepo.mock).toHaveBeenCalledWith(scenarioId); }); }); describe(`when provided PUs belong to given scenario`, () => { beforeEach(() => { - puValidator.mock.mockResolvedValue({ - errors: [], + puRepo.mock.mockResolvedValue({ + ids: availablePlanningUnitIds, }); }); it(`should persist the results`, async () => { expect(await sut.process(getJob(scenarioId))).toEqual(true); - expect(repo.saveMock).toHaveBeenCalledWith(scenarioId, getCost()); + expect(repo.saveMock).toHaveBeenCalledWith(scenarioId, cost); }); }); }); diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts index 2c248983cb..2ea708f501 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts @@ -6,8 +6,9 @@ import { CostSurfaceJobInput } from '../cost-surface-job-input'; import { CostSurfacePersistencePort } from '../ports/persistence/cost-surface-persistence.port'; import { PuExtractorPort } from '../ports/pu-extractor/pu-extractor.port'; -import { ArePuidsAllowedPort } from '../ports/pu-validator/are-puuids-allowed.port'; +import { GetAvailablePlanningUnits } from '../ports/available-planning-units/get-available-planning-units'; import { ShapefileConverterPort } from '../ports/shapefile-converter/shapefile-converter.port'; +import { lockPlanningUnits } from '@marxan/scenarios-planning-unit'; @Injectable() export class SurfaceCostProcessor @@ -15,16 +16,20 @@ export class SurfaceCostProcessor constructor( private readonly repo: CostSurfacePersistencePort, private readonly puExtractor: PuExtractorPort, - private readonly puValidator: ArePuidsAllowedPort, + private readonly availablePlanningUnits: GetAvailablePlanningUnits, private readonly shapefileConverter: ShapefileConverterPort, ) {} async process(job: Job): Promise { const geoJson = await this.shapefileConverter.convert(job.data.shapefile); const surfaceCosts = this.puExtractor.extract(geoJson); - const { errors } = await this.puValidator.validate( - job.data.scenarioId, + const planningUnitsIds = ( + await this.availablePlanningUnits.get(job.data.scenarioId) + ).ids; + + const { errors } = lockPlanningUnits( surfaceCosts.map((cost) => cost.planningUnitId), + planningUnitsIds, ); if (errors.length > 0) { throw new Error(errors.join('.')); diff --git a/api/apps/geoprocessing/src/modules/surface-cost/ports/available-planning-units/get-available-planning-units.ts b/api/apps/geoprocessing/src/modules/surface-cost/ports/available-planning-units/get-available-planning-units.ts new file mode 100644 index 0000000000..c3c9728f5d --- /dev/null +++ b/api/apps/geoprocessing/src/modules/surface-cost/ports/available-planning-units/get-available-planning-units.ts @@ -0,0 +1,7 @@ +export abstract class GetAvailablePlanningUnits { + abstract get( + scenarioId: string, + ): Promise<{ + ids: string[]; + }>; +} diff --git a/api/apps/geoprocessing/src/modules/surface-cost/ports/pu-validator/are-puuids-allowed.port.ts b/api/apps/geoprocessing/src/modules/surface-cost/ports/pu-validator/are-puuids-allowed.port.ts deleted file mode 100644 index ae573ac81f..0000000000 --- a/api/apps/geoprocessing/src/modules/surface-cost/ports/pu-validator/are-puuids-allowed.port.ts +++ /dev/null @@ -1,8 +0,0 @@ -export abstract class ArePuidsAllowedPort { - abstract validate( - scenarioId: string, - puIds: string[], - ): Promise<{ - errors: unknown[]; - }>; -} diff --git a/api/apps/geoprocessing/src/modules/surface-cost/surface-cost.module.ts b/api/apps/geoprocessing/src/modules/surface-cost/surface-cost.module.ts index 1a139694bc..b6bcba2551 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/surface-cost.module.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/surface-cost.module.ts @@ -1,25 +1,26 @@ import { Module } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; +import { ScenariosPlanningUnitGeoEntity } from '@marxan/scenarios-planning-unit'; import { CqrsModule } from '@nestjs/cqrs'; import { WorkerModule, WorkerProcessor, } from '@marxan-geoprocessing/modules/worker'; import { ShapefilesModule } from '@marxan-geoprocessing/modules/shapefiles/shapefiles.module'; -import { ScenariosPlanningUnitGeoEntity } from '@marxan/scenarios-planning-unit'; import { SurfaceCostProcessor } from './application/surface-cost-processor'; import { SurfaceCostWorker } from './application/surface-cost-worker'; import { CostSurfacePersistencePort } from './ports/persistence/cost-surface-persistence.port'; import { PuExtractorPort } from './ports/pu-extractor/pu-extractor.port'; -import { ArePuidsAllowedPort } from './ports/pu-validator/are-puuids-allowed.port'; +import { GetAvailablePlanningUnits } from './ports/available-planning-units/get-available-planning-units'; import { ShapefileConverterPort } from './ports/shapefile-converter/shapefile-converter.port'; import { TypeormCostSurface } from './adapters/typeorm-cost-surface'; import { ShapefileConverter } from './adapters/shapefile-converter'; import { ScenariosPuCostDataGeo } from '../scenarios/scenarios-pu-cost-data.geo.entity'; import { PuCostExtractor } from './adapters/pu-cost-extractor'; +import { TypeormAvailablePlanningUnits } from './adapters/typeorm-available-planning-units'; @Module({ imports: [ @@ -28,7 +29,7 @@ import { PuCostExtractor } from './adapters/pu-cost-extractor'; CqrsModule, TypeOrmModule.forFeature([ ScenariosPuCostDataGeo, - ScenariosPlanningUnitGeoEntity, // not used but has to imported somewhere + ScenariosPlanningUnitGeoEntity, ]), ], providers: [ @@ -42,8 +43,8 @@ import { PuCostExtractor } from './adapters/pu-cost-extractor'; useClass: TypeormCostSurface, }, { - provide: ArePuidsAllowedPort, - useValue: {}, + provide: GetAvailablePlanningUnits, + useClass: TypeormAvailablePlanningUnits, }, { provide: PuExtractorPort, From d1526cdb5feebd0920faaedd219c7fbe98e77725 Mon Sep 17 00:00:00 2001 From: Kamil Gajowy Date: Tue, 1 Jun 2021 11:28:10 +0200 Subject: [PATCH 6/8] refactor: rebase and adjust to domain change --- .../surface-cost/application/surface-cost-processor.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts index 2ea708f501..c83622613c 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts @@ -1,14 +1,15 @@ import { Injectable } from '@nestjs/common'; import { Job } from 'bullmq'; -import { WorkerProcessor } from '../../worker'; +import { canPlanningUnitsBeLocked } from '@marxan/scenarios-planning-unit'; +import { WorkerProcessor } from '@marxan-geoprocessing/modules/worker'; + import { CostSurfaceJobInput } from '../cost-surface-job-input'; import { CostSurfacePersistencePort } from '../ports/persistence/cost-surface-persistence.port'; import { PuExtractorPort } from '../ports/pu-extractor/pu-extractor.port'; import { GetAvailablePlanningUnits } from '../ports/available-planning-units/get-available-planning-units'; import { ShapefileConverterPort } from '../ports/shapefile-converter/shapefile-converter.port'; -import { lockPlanningUnits } from '@marxan/scenarios-planning-unit'; @Injectable() export class SurfaceCostProcessor @@ -27,7 +28,7 @@ export class SurfaceCostProcessor await this.availablePlanningUnits.get(job.data.scenarioId) ).ids; - const { errors } = lockPlanningUnits( + const { errors } = canPlanningUnitsBeLocked( surfaceCosts.map((cost) => cost.planningUnitId), planningUnitsIds, ); From 818cda97701d8f98a93dfed10a5848a175568292 Mon Sep 17 00:00:00 2001 From: Kamil Gajowy Date: Tue, 1 Jun 2021 14:25:26 +0200 Subject: [PATCH 7/8] chore: rename framework-specific adapter to just repository --- ...anning-units.ts => available-planning-units-repository.ts} | 2 +- .../src/modules/surface-cost/application/__mocks__/cost.ts | 4 +++- .../surface-cost/application/surface-cost-processor.spec.ts | 4 ++-- .../src/modules/surface-cost/surface-cost.module.ts | 4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) rename api/apps/geoprocessing/src/modules/surface-cost/adapters/{typeorm-available-planning-units.ts => available-planning-units-repository.ts} (94%) diff --git a/api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-available-planning-units.ts b/api/apps/geoprocessing/src/modules/surface-cost/adapters/available-planning-units-repository.ts similarity index 94% rename from api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-available-planning-units.ts rename to api/apps/geoprocessing/src/modules/surface-cost/adapters/available-planning-units-repository.ts index 35b3cc1800..6000404e70 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-available-planning-units.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/adapters/available-planning-units-repository.ts @@ -7,7 +7,7 @@ import { ScenariosPlanningUnitGeoEntity } from '@marxan/scenarios-planning-unit' import { GetAvailablePlanningUnits } from '../ports/available-planning-units/get-available-planning-units'; @Injectable() -export class TypeormAvailablePlanningUnits +export class AvailablePlanningUnitsRepository implements GetAvailablePlanningUnits { constructor( @InjectRepository(ScenariosPlanningUnitGeoEntity) diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/cost.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/cost.ts index 36eebbae11..33469b3740 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/cost.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/__mocks__/cost.ts @@ -1,6 +1,8 @@ import { PlanningUnitCost } from '../../ports/planning-unit-cost'; -export const getCost = (planningUnitsIds: string[]): PlanningUnitCost[] => +export const getCostByPlanningUnit = ( + planningUnitsIds: string[], +): PlanningUnitCost[] => planningUnitsIds.map((pu) => ({ cost: 200, planningUnitId: pu, diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.spec.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.spec.ts index 6aefb75895..d8462a5208 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.spec.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.spec.ts @@ -15,7 +15,7 @@ import { CostSurfaceRepoFake } from './__mocks__/cost-surface-repo.fake'; import { getJob } from './__mocks__/job.data'; import { getGeoJson } from './__mocks__/geojson'; -import { getCost } from './__mocks__/cost'; +import { getCostByPlanningUnit } from './__mocks__/cost'; let sut: SurfaceCostProcessor; @@ -92,7 +92,7 @@ describe(`when shapefile was converted to geojson`, () => { }); describe(`when cost was resolved`, () => { - const cost = getCost(availablePlanningUnitIds); + const cost = getCostByPlanningUnit(availablePlanningUnitIds); beforeEach(() => { puExtractor.mock.mockReturnValue(cost); }); diff --git a/api/apps/geoprocessing/src/modules/surface-cost/surface-cost.module.ts b/api/apps/geoprocessing/src/modules/surface-cost/surface-cost.module.ts index b6bcba2551..3cf9153379 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/surface-cost.module.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/surface-cost.module.ts @@ -20,7 +20,7 @@ import { TypeormCostSurface } from './adapters/typeorm-cost-surface'; import { ShapefileConverter } from './adapters/shapefile-converter'; import { ScenariosPuCostDataGeo } from '../scenarios/scenarios-pu-cost-data.geo.entity'; import { PuCostExtractor } from './adapters/pu-cost-extractor'; -import { TypeormAvailablePlanningUnits } from './adapters/typeorm-available-planning-units'; +import { AvailablePlanningUnitsRepository } from './adapters/available-planning-units-repository'; @Module({ imports: [ @@ -44,7 +44,7 @@ import { TypeormAvailablePlanningUnits } from './adapters/typeorm-available-plan }, { provide: GetAvailablePlanningUnits, - useClass: TypeormAvailablePlanningUnits, + useClass: AvailablePlanningUnitsRepository, }, { provide: PuExtractorPort, From 804aeaa2a7765b48e895bae4c552bb8759599ea1 Mon Sep 17 00:00:00 2001 From: Kamil Gajowy Date: Tue, 1 Jun 2021 14:47:50 +0200 Subject: [PATCH 8/8] chore: rename cost surface internals --- .../surface-cost/application/surface-cost-processor.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts index c83622613c..fd0c824bb7 100644 --- a/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts +++ b/api/apps/geoprocessing/src/modules/surface-cost/application/surface-cost-processor.ts @@ -24,13 +24,13 @@ export class SurfaceCostProcessor async process(job: Job): Promise { const geoJson = await this.shapefileConverter.convert(job.data.shapefile); const surfaceCosts = this.puExtractor.extract(geoJson); - const planningUnitsIds = ( + const scenarioPlanningUnitIds = ( await this.availablePlanningUnits.get(job.data.scenarioId) ).ids; const { errors } = canPlanningUnitsBeLocked( surfaceCosts.map((cost) => cost.planningUnitId), - planningUnitsIds, + scenarioPlanningUnitIds, ); if (errors.length > 0) { throw new Error(errors.join('.'));