Skip to content

Commit

Permalink
feat(api): prevent creating scenario if project is not ready
Browse files Browse the repository at this point in the history
  • Loading branch information
Dyostiq committed Oct 1, 2021
1 parent be26aae commit 3e9ec96
Show file tree
Hide file tree
Showing 9 changed files with 183 additions and 20 deletions.
8 changes: 2 additions & 6 deletions api/apps/api/src/modules/api-events/api-events.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ import {

import { DeleteResult } from 'typeorm';

import {
ApiEvent,
ApiEventResult,
QualifiedEventTopic,
} from './api-event.api.entity';
import { ApiEvent, ApiEventResult } from './api-event.api.entity';
import { ApiEventsService } from './api-events.service';
import { CreateApiEventDTO } from './dto/create.api-event.dto';
import { API_EVENT_KINDS } from '@marxan/api-events';
Expand Down Expand Up @@ -88,6 +84,6 @@ export class ApiEventsController {
@Param('kind') kind: API_EVENT_KINDS,
@Param('topic') topic: string,
): Promise<DeleteResult> {
return await this.service.purgeAll({ kind, topic } as QualifiedEventTopic);
return await this.service.purgeAll({ kind, topic });
}
}
26 changes: 20 additions & 6 deletions api/apps/api/src/modules/api-events/api-events.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm';

import { DeleteResult, Repository } from 'typeorm';
import { Either, left, right } from 'fp-ts/lib/Either';
import { FindOperator } from 'typeorm/find-options/FindOperator';

import {
ApiEvent,
Expand All @@ -24,6 +25,12 @@ import { UpdateApiEventDTO } from './dto/update.api-event.dto';
import { AppInfoDTO } from '../../dto/info.dto';
import { AppConfig } from '../../utils/config.utils';

export interface QualifiedEventTopicSearch
extends Omit<QualifiedEventTopic, 'kind'> {
topic: string;
kind: FindOperator<QualifiedEventTopic['kind']> | QualifiedEventTopic['kind'];
}

export const duplicate = Symbol();

@Injectable()
Expand Down Expand Up @@ -57,12 +64,19 @@ export class ApiEventsService extends AppBaseService<
* return the matching event with latest timestamp.
*/
public async getLatestEventForTopic(
qualifiedTopic: QualifiedEventTopic,
qualifiedTopic: QualifiedEventTopicSearch,
): Promise<ApiEventByTopicAndKind | undefined> {
const result = await this.latestEventByTopicAndKindRepo.findOne({
topic: qualifiedTopic.topic,
kind: qualifiedTopic.kind,
});
const result = await this.latestEventByTopicAndKindRepo.findOne(
{
topic: qualifiedTopic.topic,
kind: qualifiedTopic.kind,
},
{
order: {
timestamp: 'DESC',
},
},
);
if (!result) {
throw new NotFoundException(
`No events found for topic ${qualifiedTopic.topic} and kind ${qualifiedTopic.kind}.`,
Expand Down Expand Up @@ -98,7 +112,7 @@ export class ApiEventsService extends AppBaseService<
* `QualifiedEventTopic` (i.e. a topic qualified by `kind` and `apiVersion`).
*/
public async purgeAll(
qualifiedTopic?: QualifiedEventTopic,
qualifiedTopic?: QualifiedEventTopicSearch,
): Promise<DeleteResult> {
if (!isNil(qualifiedTopic)) {
this.logger.log(
Expand Down
91 changes: 91 additions & 0 deletions api/apps/api/src/modules/scenarios/project-checker.service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { In } from 'typeorm';
import { Test } from '@nestjs/testing';
import { FixtureType } from '@marxan/utils/tests/fixture-type';
import { API_EVENT_KINDS } from '@marxan/api-events';
import { ApiEventsService } from '@marxan-api/modules/api-events';
import { ProjectChecker } from './project-checker.service';

let fixtures: FixtureType<typeof getFixtures>;

beforeEach(async () => {
fixtures = await getFixtures();
});

it(`should form valid request`, () => {
// given
const service = fixtures.getService();
// when
service.isProjectReady(`projectId`);
// then
fixtures.ThenShouldAskAllPlanningUnitsStatuses();
});

it.each(
Object.values(API_EVENT_KINDS).filter(
(kind) =>
kind !== API_EVENT_KINDS.project__planningUnits__finished__v1__alpha,
),
)(`should return false for %s`, async (kind) => {
// given
const service = fixtures.getService();
// and
fixtures.fakeApiEventsService.getLatestEventForTopic.mockResolvedValueOnce({
kind,
timestamp: new Date(),
topic: `projectId`,
});
// when
const isReady = await service.isProjectReady(`projectId`);
// then
expect(isReady).toBe(false);
});

it(`should return true for finished`, async () => {
// given
const service = fixtures.getService();
// and
fixtures.fakeApiEventsService.getLatestEventForTopic.mockResolvedValueOnce({
kind: API_EVENT_KINDS.project__planningUnits__finished__v1__alpha as const,
timestamp: new Date(),
topic: `projectId`,
});
// when
const isReady = await service.isProjectReady(`projectId`);
// then
expect(isReady).toBe(true);
});

async function getFixtures() {
const fakeApiEventsService: jest.Mocked<
Pick<ApiEventsService, 'getLatestEventForTopic'>
> = {
getLatestEventForTopic: jest.fn<any, any>(),
};
const testingModule = await Test.createTestingModule({
providers: [
{
provide: ApiEventsService,
useValue: fakeApiEventsService,
},
ProjectChecker,
],
}).compile();

return {
fakeApiEventsService,
getService() {
return testingModule.get(ProjectChecker);
},
ThenShouldAskAllPlanningUnitsStatuses() {
expect(fakeApiEventsService.getLatestEventForTopic).toBeCalledTimes(1);
expect(fakeApiEventsService.getLatestEventForTopic).toBeCalledWith({
topic: `projectId`,
kind: In(
Object.values(API_EVENT_KINDS)
.filter((kind) => kind.startsWith(`project.planningUnits.`))
.sort(),
),
});
},
};
}
25 changes: 25 additions & 0 deletions api/apps/api/src/modules/scenarios/project-checker.service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Injectable } from '@nestjs/common';
import { In } from 'typeorm';

import { API_EVENT_KINDS } from '@marxan/api-events';
import { ApiEventsService } from '@marxan-api/modules/api-events';

@Injectable()
export class ProjectChecker {
constructor(private readonly apiEvents: ApiEventsService) {}

async isProjectReady(projectId: string): Promise<boolean> {
const event = await this.apiEvents.getLatestEventForTopic({
topic: projectId,
kind: In([
API_EVENT_KINDS.project__planningUnits__failed__v1__alpha,
API_EVENT_KINDS.project__planningUnits__finished__v1__alpha,
API_EVENT_KINDS.project__planningUnits__submitted__v1__alpha,
]),
});
return (
event?.kind ===
API_EVENT_KINDS.project__planningUnits__finished__v1__alpha
);
}
}
13 changes: 10 additions & 3 deletions api/apps/api/src/modules/scenarios/scenarios.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
UseGuards,
UseInterceptors,
ValidationPipe,
ConflictException,
} from '@nestjs/common';
import { scenarioResource, ScenarioResult } from './scenario.api.entity';
import { Request, Response } from 'express';
Expand All @@ -31,7 +32,6 @@ import {
ApiBearerAuth,
ApiCreatedResponse,
ApiForbiddenResponse,
ApiNoContentResponse,
ApiOkResponse,
ApiOperation,
ApiParam,
Expand Down Expand Up @@ -61,7 +61,7 @@ import { uploadOptions } from '@marxan-api/utils/file-uploads.utils';
import { ShapefileGeoJSONResponseDTO } from './dto/shapefile.geojson.response.dto';
import { ApiConsumesShapefile } from '@marxan-api/decorators/shapefile.decorator';
import { FileInterceptor } from '@nestjs/platform-express';
import { ScenariosService } from './scenarios.service';
import { ProjectNotReady, ScenariosService } from './scenarios.service';
import { ScenarioSerializer } from './dto/scenario.serializer';
import { ScenarioFeatureSerializer } from './dto/scenario-feature.serializer';
import { ScenarioFeatureResultDto } from './dto/scenario-feature-result.dto';
Expand Down Expand Up @@ -225,8 +225,15 @@ export class ScenariosController {
@Body(new ValidationPipe()) dto: CreateScenarioDTO,
@Req() req: RequestWithAuthenticatedUser,
): Promise<ScenarioResult> {
const result = await this.service.create(dto, {
authenticatedUser: req.user,
});
if (isLeft(result)) {
const _exhaustiveCheck: ProjectNotReady = result.left;
throw new ConflictException();
}
return await this.scenarioSerializer.serialize(
await this.service.create(dto, { authenticatedUser: req.user }),
result.right,
undefined,
true,
);
Expand Down
4 changes: 4 additions & 0 deletions api/apps/api/src/modules/scenarios/scenarios.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { ScenarioFeatureSerializer } from './dto/scenario-feature.serializer';
import { CostSurfaceTemplateModule } from './cost-surface-template';
import { SolutionResultCrudService } from './solutions-result/solution-result-crud.service';
import { DbConnections } from '@marxan-api/ormconfig.connections';
import { ApiEventsModule } from '@marxan-api/modules/api-events';
import {
ScenariosPlanningUnitGeoEntity,
ScenariosPuOutputGeoEntity,
Expand All @@ -43,6 +44,7 @@ import { SpecificationModule } from './specification';
import { ScenarioFeaturesGapDataSerializer } from './dto/scenario-feature-gap-data.serializer';
import { ScenarioFeaturesOutputGapDataSerializer } from './dto/scenario-feature-output-gap-data.serializer';
import { CostRangeService } from './cost-range-service';
import { ProjectChecker } from './project-checker.service';

@Module({
imports: [
Expand Down Expand Up @@ -72,8 +74,10 @@ import { CostRangeService } from './cost-range-service';
OutputFilesModule,
MarxanRunModule,
AdminAreasModule,
ApiEventsModule,
],
providers: [
ProjectChecker,
ScenariosService,
ScenariosCrudService,
ProxyService,
Expand Down
22 changes: 18 additions & 4 deletions api/apps/api/src/modules/scenarios/scenarios.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import { FetchSpecification } from 'nestjs-base-service';
import { classToClass } from 'class-transformer';
import * as stream from 'stream';
import { isLeft } from 'fp-ts/Either';
import { Either, isLeft, left, right } from 'fp-ts/Either';
import { pick } from 'lodash';

import { MarxanInput, MarxanParameters } from '@marxan/marxan-input';
Expand All @@ -33,7 +33,7 @@ import { OutputFilesService } from './output-files/output-files.service';
import { InputFilesArchiverService, InputFilesService } from './input-files';
import { notFound, RunService } from './marxan-run';
import { GeoFeatureSetSpecification } from '../geo-features/dto/geo-feature-set-specification.dto';
import { SimpleJobStatus } from './scenario.api.entity';
import { Scenario, SimpleJobStatus } from './scenario.api.entity';
import { assertDefined } from '@marxan/utils';
import { ScenarioPlanningUnitsProtectedStatusCalculatorService } from '@marxan/scenarios-planning-unit';
import { GeoFeaturePropertySetService } from '../geo-features/geo-feature-property-sets.service';
Expand All @@ -42,13 +42,17 @@ import { ScenarioPlanningUnitsLinkerService } from './planning-units/scenario-pl
import { CreateGeoFeatureSetDTO } from '../geo-features/dto/create.geo-feature-set.dto';
import { SpecificationService } from './specification';
import { CostRange, CostRangeService } from './cost-range-service';
import { ProjectChecker } from '@marxan-api/modules/scenarios/project-checker.service';

/** @debt move to own module */
const EmptyGeoFeaturesSpecification: GeoFeatureSetSpecification = {
status: SimpleJobStatus.draft,
features: [],
};

export const projectNotReady = Symbol('project not ready');
export type ProjectNotReady = typeof projectNotReady;

@Injectable()
export class ScenariosService {
private readonly geoprocessingUrl: string = AppConfig.get(
Expand All @@ -73,6 +77,7 @@ export class ScenariosService {
private readonly planningUnitsStatusCalculatorService: ScenarioPlanningUnitsProtectedStatusCalculatorService,
private readonly specificationService: SpecificationService,
private readonly costService: CostRangeService,
private readonly projectChecker: ProjectChecker,
) {}

async findAllPaginated(
Expand All @@ -91,15 +96,24 @@ export class ScenariosService {
return this.crudService.remove(scenarioId);
}

async create(input: CreateScenarioDTO, info: AppInfoDTO) {
async create(
input: CreateScenarioDTO,
info: AppInfoDTO,
): Promise<Either<ProjectNotReady, Scenario>> {
const validatedMetadata = this.getPayloadWithValidatedMetadata(input);
const isProjectReady = await this.projectChecker.isProjectReady(
input.projectId,
);
if (!isProjectReady) {
return left(projectNotReady);
}
const scenario = await this.crudService.create(validatedMetadata, info);
await this.planningUnitsLinkerService.link(scenario);
await this.planningUnitsStatusCalculatorService.calculatedProtectionStatusForPlanningUnitsIn(
scenario,
input,
);
return scenario;
return right(scenario);
}

async update(scenarioId: string, input: UpdateScenarioDTO) {
Expand Down
7 changes: 6 additions & 1 deletion api/apps/api/test/integration/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ import { v4 } from 'uuid';
import { Test, TestingModule } from '@nestjs/testing';
import { AppModule } from '@marxan-api/app.module';
import { SpecificationRepository } from '@marxan-api/modules/specification/application/specification.repository';
import { ProjectChecker } from '@marxan-api/modules/scenarios/project-checker.service';
import { GivenUserIsLoggedIn } from '../steps/given-user-is-logged-in';
import { GivenProjectExists } from '../steps/given-project';
import { GivenScenarioExists } from '../steps/given-scenario-exists';
import { ScenariosTestUtils } from '../utils/scenarios.test.utils';
import { fakeProjectChecker } from '../utils/api-application';

import { SpecificationAdaptersModule } from '@marxan-api/modules/specification/adapters/specification-adapters.module';
import {
Expand All @@ -16,7 +18,10 @@ import {
export const getFixtures = async () => {
const moduleFixture: TestingModule = await Test.createTestingModule({
imports: [AppModule, SpecificationAdaptersModule],
}).compile();
})
.overrideProvider(ProjectChecker)
.useValue(fakeProjectChecker)
.compile();
const app = await moduleFixture.createNestApplication().init();
const specificationRepository = app.get(SpecificationRepository);

Expand Down
7 changes: 7 additions & 0 deletions api/apps/api/test/utils/api-application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import { AppModule } from '@marxan-api/app.module';
import { QueueToken } from '../../src/modules/queue/queue.tokens';
import { FakeQueue, FakeQueueBuilder } from './queues';
import { QueueBuilder } from '@marxan-api/modules/queue/queue.builder';
import { ProjectChecker } from '@marxan-api/modules/scenarios/project-checker.service';

export const fakeProjectChecker = {
isProjectReady: async () => true,
};

export const bootstrapApplication = async (
imports: ModuleMetadata['imports'] = [],
Expand All @@ -16,6 +21,8 @@ export const bootstrapApplication = async (
.useClass(FakeQueue) // https://github.com/nestjs/nest/issues/2303#issuecomment-507563175
.overrideProvider(QueueBuilder)
.useClass(FakeQueueBuilder)
.overrideProvider(ProjectChecker)
.useValue(fakeProjectChecker)
.compile();

return moduleFixture
Expand Down

0 comments on commit 3e9ec96

Please sign in to comment.