Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(geoprocessing): scenario planning units: use integrity rule #229

Merged
merged 8 commits into from
Jun 1, 2021
Original file line number Diff line number Diff line change
@@ -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 AvailablePlanningUnitsRepository
implements GetAvailablePlanningUnits {
constructor(
@InjectRepository(ScenariosPlanningUnitGeoEntity)
private readonly repo: Repository<ScenariosPlanningUnitGeoEntity>,
) {}

get(scenarioId: string): Promise<{ ids: string[] }> {
return this.repo
.find({
where: {
scenarioId,
},
})
.then((rows) => ({
ids: rows.map((row) => row.id),
}));
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { PlanningUnitCost } from '../../ports/planning-unit-cost';

export const getCost = (): PlanningUnitCost[] => [
{
export const getCostByPlanningUnit = (
planningUnitsIds: string[],
): PlanningUnitCost[] =>
planningUnitsIds.map((pu) => ({
cost: 200,
planningUnitId: `puid-1`,
},
{
cost: 400,
planningUnitId: `puid-2`,
},
];
planningUnitId: pu,
}));
Original file line number Diff line number Diff line change
@@ -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<MultiPolygon | Polygon, PlanningUnitCost> => ({
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<
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Promise<{ ids: string[] }>> = jest.fn();

get(scenarioId: string): Promise<{ ids: string[] }> {
return this.mock(scenarioId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,28 @@ 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';
import { getGeoJson } from './__mocks__/geojson';
import { getCost } from './__mocks__/cost';
import { getCostByPlanningUnit } from './__mocks__/cost';

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: [
Expand All @@ -37,8 +40,8 @@ beforeEach(async () => {
useClass: PuExtractorFake,
},
{
provide: ArePuidsAllowedPort,
useClass: ArePuidsAllowedFake,
provide: GetAvailablePlanningUnits,
useClass: GetAvailablePuidsFake,
},
{
provide: ShapefileConverterPort,
Expand All @@ -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);
});

Expand All @@ -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`, () => {
Expand All @@ -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 = getCostByPlanningUnit(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);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
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 { 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';

@Injectable()
Expand All @@ -15,16 +17,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<CostSurfaceJobInput, true>): Promise<true> {
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 scenarioPlanningUnitIds = (
await this.availablePlanningUnits.get(job.data.scenarioId)
).ids;

const { errors } = canPlanningUnitsBeLocked(
surfaceCosts.map((cost) => cost.planningUnitId),
scenarioPlanningUnitIds,
);
if (errors.length > 0) {
throw new Error(errors.join('.'));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export abstract class GetAvailablePlanningUnits {
abstract get(
scenarioId: string,
): Promise<{
ids: string[];
}>;
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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 { AvailablePlanningUnitsRepository } from './adapters/available-planning-units-repository';

@Module({
imports: [
Expand All @@ -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: [
Expand All @@ -42,8 +43,8 @@ import { PuCostExtractor } from './adapters/pu-cost-extractor';
useClass: TypeormCostSurface,
},
{
provide: ArePuidsAllowedPort,
useValue: {},
provide: GetAvailablePlanningUnits,
useClass: AvailablePlanningUnitsRepository,
},
{
provide: PuExtractorPort,
Expand Down
Original file line number Diff line number Diff line change
@@ -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<TestCase>([
[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);
},
);
Loading