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

MARXAN - 291 Users should be able to upload shapefiles of scenario cost surfaces - update cost in database #206

Merged
merged 3 commits into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions api/src/modules/analysis/analysis.module.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { TypeOrmModule } from '@nestjs/typeorm';
import { Module } from '@nestjs/common';
import { PlanningUnitsModule } from '../planning-units/planning-units.module';
import { ScenariosPlanningUnitModule } from '../scenarios-planning-unit/scenarios-planning-unit.module';
import { DbConnections } from '../../ormconfig.connections';

import { ScenariosPlanningUnitModule } from '../scenarios-planning-unit/scenarios-planning-unit.module';
import { AdjustCostSurface } from './entry-points/adjust-cost-surface';

import { AdjustPlanningUnits } from './entry-points/adjust-planning-units';
import { GetScenarioStatus } from './entry-points/get-scenario-status';

import { UpdateCostSurfaceService } from './providers/cost-surface/update-cost-surface.service';
import { ArePuidsAllowedAdapter } from './providers/shared/adapters/are-puids-allowed-adapter';
import { ArePuidsAllowedPort } from './providers/shared/are-puids-allowed.port';
Expand All @@ -14,12 +16,17 @@ import { ScenarioStatusService } from './providers/status/scenario-status.servic
import { RequestJobPort } from './providers/planning-units/request-job.port';
import { AsyncJobsAdapter } from './providers/planning-units/adapters/async-jobs-adapter';
import { CostSurfaceRepo } from './providers/cost-surface/cost-surface-repo';
import { BaseAppCostSurface } from './providers/cost-surface/adapters/base-app-cost-surface';
import { TypeormCostSurface } from './providers/cost-surface/adapters/typeorm-cost-surface';
import { QueueModule } from '../queue/queue.module';
import { queueName } from './queue-name';
import { ScenariosPuCostDataGeo } from './providers/cost-surface/adapters/scenarios-pu-cost-data.geo.entity';

@Module({
imports: [
TypeOrmModule.forFeature(
[ScenariosPuCostDataGeo],
DbConnections.geoprocessingDB,
),
ScenariosPlanningUnitModule,
PlanningUnitsModule,
QueueModule.register({
Expand Down Expand Up @@ -52,7 +59,7 @@ import { queueName } from './queue-name';
},
{
provide: CostSurfaceRepo,
useClass: BaseAppCostSurface,
useClass: TypeormCostSurface,
},
],
exports: [AdjustCostSurface, AdjustPlanningUnits, GetScenarioStatus],
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import {
Column,
Entity,
JoinColumn,
ManyToOne,
PrimaryGeneratedColumn,
RelationId,
} from 'typeorm';
import { ScenariosPlanningUnitGeoEntity } from '../../../../scenarios-planning-unit/entities/scenarios-planning-unit.geo.entity';

@Entity({
name: 'scenarios_pu_cost_data',
})
export class ScenariosPuCostDataGeo {
@PrimaryGeneratedColumn('uuid')
id!: string;

@Column({
type: 'float8',
nullable: false,
name: 'output_results_data_id',
})
planningUnitId!: string;

@Column({
type: 'float8',
nullable: false,
default: 0,
comment: `By default we will set them as unitary based on equal area`,
})
cost!: number;

@ManyToOne(() => ScenariosPlanningUnitGeoEntity)
@JoinColumn({
referencedColumnName: 'id',
name: 'scenarios_pu_data_id',
})
scenariosPlanningUnit?: ScenariosPlanningUnitGeoEntity | null;

@RelationId((spud: ScenariosPuCostDataGeo) => spud.scenariosPlanningUnit)
scenariosPuDataId!: string;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { flatten, Injectable } from '@nestjs/common';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though the end result is the same, I think this is a more-or-less internal TypeORM function that happens to be exported - should we use the lodash one for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in af3773c

import { CostSurfaceRepo } from '../cost-surface-repo';
import { CostSurfaceInputDto } from '../../../entry-points/adjust-cost-surface-input';
import { InjectRepository } from '@nestjs/typeorm';
import { ScenariosPuCostDataGeo } from './scenarios-pu-cost-data.geo.entity';
import { DbConnections } from '../../../../../ormconfig.connections';
import { Repository } from 'typeorm';

type Success = true;

@Injectable()
export class TypeormCostSurface implements CostSurfaceRepo {
constructor(
@InjectRepository(ScenariosPuCostDataGeo, DbConnections.geoprocessingDB)
private readonly costs: Repository<ScenariosPuCostDataGeo>,
) {
//
}

async applyCostSurface(
_: string,
values: CostSurfaceInputDto['planningUnits'],
): Promise<Success> {
const pairs = values.map<[string, number]>((pair) => [pair.id, pair.cost]);
await this.costs.query(
`
UPDATE scenarios_pu_cost_data as spd
set "cost" = pucd.new_cost
from (values
${this.generateParametrizedValues(pairs)}
) as pucd(output_results_data_id, new_cost)
where pucd.output_results_data_id = spd.output_results_data_id
`,
flatten(pairs),
);
return true;
}

/**
*
* generates parametrized input for:
* ('a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11'::uuid, 5000::float)
*
* in form of:
* ($1::uuid, $2::float),
* ($3::uuid, $4::float),
* ($5::uuid, $6::float),
* ...
*
*/
private generateParametrizedValues(pairs: [string, number][]): string {
return pairs
.map(
(_, index) =>
`($${(index + 1) * 2 - 1}::uuid, $${(index + 1) * 2}::float)`,
)
.join(',');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { INestApplication } from '@nestjs/common';
import { TypeormCostSurface } from '../../../src/modules/analysis/providers/cost-surface/adapters/typeorm-cost-surface';
import { bootstrapApplication } from '../../utils/api-application';
import { CostSurfaceUpdateWorld, createWorld } from './world';
import { CostSurfaceRepo } from '../../../src/modules/analysis/providers/cost-surface/cost-surface-repo';

let app: INestApplication;
let sut: TypeormCostSurface;
let world: CostSurfaceUpdateWorld;

beforeAll(async () => {
app = await bootstrapApplication();
world = await createWorld(app);
sut = app.get(CostSurfaceRepo);
});

afterAll(async () => {
await world.cleanup();
await app.close();
});

describe(`when updating some of the costs`, () => {
let puCostDataIds: string[];
beforeEach(async () => {
puCostDataIds = await world.GivenPuCostDataExists();
});

it(`applies new costs to given PU`, async () => {
const costOf9999Id = puCostDataIds[0];
const costOf1Id = puCostDataIds[1];
const sameCostId = puCostDataIds[2];

await sut.applyCostSurface(world.scenarioId, [
{
cost: 9999,
id: costOf9999Id,
},
{
cost: 1,
id: costOf1Id,
},
]);

const afterChanges = await world.GetPuCostsData(world.scenarioId);

expect(afterChanges).toContainEqual({
scenario_id: world.scenarioId,
cost: 9999,
pu_id: costOf9999Id,
});

expect(afterChanges).toContainEqual({
scenario_id: world.scenarioId,
cost: 1,
pu_id: costOf1Id,
});

expect(afterChanges).toContainEqual({
scenario_id: world.scenarioId,
cost: expect.any(Number),
pu_id: sameCostId,
});
});
});
80 changes: 80 additions & 0 deletions api/test/integration/cost-surface-repo/world.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { INestApplication } from '@nestjs/common';
import { ScenariosPlanningUnitGeoEntity } from '../../../src/modules/scenarios-planning-unit/entities/scenarios-planning-unit.geo.entity';
import { GivenScenarioPuDataExists } from '../../steps/given-scenario-pu-data-exists';
import { getRepositoryToken } from '@nestjs/typeorm';
import { ScenariosPuCostDataGeo } from '../../../src/modules/analysis/providers/cost-surface/adapters/scenarios-pu-cost-data.geo.entity';
import { DbConnections } from '../../../src/ormconfig.connections';
import { In, Repository } from 'typeorm';
import { v4 } from 'uuid';

export interface CostSurfaceUpdateWorld {
cleanup: () => Promise<void>;
scenarioId: string;
planningUnitsIds: string[];
GivenPuCostDataExists: () => Promise<string[]>;
GetPuCostsData: (
scenarioId: string,
) => Promise<{ scenario_id: string; cost: number; pu_id: string }[]>;
}

export const createWorld = async (
app: INestApplication,
): Promise<CostSurfaceUpdateWorld> => {
const scenarioId = v4();
const puCostRepoToken = getRepositoryToken(
ScenariosPuCostDataGeo,
DbConnections.geoprocessingDB,
);
const puDataRepoToken = getRepositoryToken(
ScenariosPlanningUnitGeoEntity,
DbConnections.geoprocessingDB,
);
const puDataRepo: Repository<ScenariosPlanningUnitGeoEntity> = app.get(
puDataRepoToken,
);
const puCostDataRepo: Repository<ScenariosPuCostDataGeo> = app.get(
puCostRepoToken,
);
const scenarioPuData = await GivenScenarioPuDataExists(
puDataRepo,
scenarioId,
);

const puDataIds = scenarioPuData.rows.map((row) => row.id);
const puIds = scenarioPuData.rows.map((row) => row.puGeometryId);

return {
GetPuCostsData: async (
scenarioId: string,
): Promise<{ scenario_id: string; cost: number; pu_id: string }[]> =>
puCostDataRepo.query(`
select spud.scenario_id, spucd."cost", spucd.output_results_data_id as pu_id from scenarios_pu_data as spud join scenarios_pu_cost_data as spucd on (spud."id" = spucd.scenarios_pu_data_id)
where spud.scenario_id = '${scenarioId}'
`),
Comment on lines +50 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the reason behind using a raw query?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a sake of simplicity and missing relations :(

GivenPuCostDataExists: async () =>
puCostDataRepo
.save(
scenarioPuData.rows.map((scenarioPuData) =>
puCostDataRepo.create({
scenariosPuDataId: scenarioPuData.id,
cost: 300,
planningUnitId: scenarioPuData.puGeometryId,
scenariosPlanningUnit: scenarioPuData,
}),
),
)
.then((rows) => rows.map((row) => row.planningUnitId)),
planningUnitsIds: puIds,
scenarioId,
cleanup: async () => {
await puCostDataRepo.delete({
scenariosPlanningUnit: {
id: In(puDataIds),
},
});
await puDataRepo.delete({
scenarioId,
});
},
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

export class ScenariosPuCostData1621843948965 implements MigrationInterface {
name = 'ScenariosPuCostData1621843948965';

public async up(queryRunner: QueryRunner): Promise<void> {
/**
* backward compatibility; can probably removed in another migration
*/
await queryRunner.query(
`ALTER TABLE "scenarios_pu_cost_data" ALTER COLUMN "cost" SET DEFAULT '0'`,
);
await queryRunner.query(
`ALTER TABLE "scenarios_pu_cost_data" ALTER COLUMN "cost" SET NOT NULL`,
);
await queryRunner.query(
`COMMENT ON COLUMN "scenarios_pu_cost_data"."cost" IS 'By default we will set them as unitary based on equal area'`,
);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "scenarios_pu_cost_data" ALTER COLUMN "cost" DROP NOT NULL`,
);
await queryRunner.query(
`ALTER TABLE "scenarios_pu_cost_data" ALTER COLUMN "cost" DROP DEFAULT`,
);
await queryRunner.query(
`COMMENT ON COLUMN "scenarios_pu_cost_data"."cost" IS NULL`,
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { MigrationInterface, QueryRunner } from 'typeorm';

export class ScenariosPuCostDataId1621847467456 implements MigrationInterface {
name = 'ScenariosPuCostDataId1621847467456';

public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "scenarios_pu_cost_data" ADD "scenarios_pu_data_id" uuid`,
);
await queryRunner.query(
`ALTER TABLE "scenarios_pu_cost_data" ADD CONSTRAINT "FK_21454fad6e954ba771262974ae7" FOREIGN KEY ("scenarios_pu_data_id") REFERENCES "scenarios_pu_data"("id") ON DELETE NO ACTION ON UPDATE NO ACTION`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine you have discussed this with Alicia - just cross checking that the absence of cascades here is intentional. I can't think of a reason to leave cost data behind in case we delete scenario PUs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I missed it - thanks for catching this out!
fixed in af3773c

);
}

public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "scenarios_pu_cost_data" DROP CONSTRAINT "FK_21454fad6e954ba771262974ae7"`,
);
await queryRunner.query(
`ALTER TABLE "scenarios_pu_cost_data" DROP COLUMN "scenarios_pu_data_id"`,
);
}
}
9 changes: 9 additions & 0 deletions geoprocessing/src/modules/scenarios/lock-status.enum.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export enum LockStatus {
Unstated = 'unstated',

/**
* is always guaranteed to be tagged as included in the planning solution
*/
LockedIn = 'locked-in',
LockedOut = 'locked-out',
}
Loading