Skip to content

Commit

Permalink
delete dangling feature_amounts_per_planning_unit data straight away …
Browse files Browse the repository at this point in the history
…when deleting a feature

This is a fast operation, so it can be done straight away. Other more
expensive db deletions are still left to the garbage collector.
  • Loading branch information
hotzevzl committed Feb 16, 2024
1 parent 3c38e28 commit 413bf67
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 5 deletions.
71 changes: 66 additions & 5 deletions api/apps/api/src/modules/geo-features/geo-features.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,12 @@ export class GeoFeaturesService extends AppBaseService<
data: UploadShapefileDTO,
features: Record<string, any>[],
): Promise<Either<Error, GeoFeature>> {
/**
* @debt Avoid duplicating transaction scaffolding in multiple sites
* within this class: this should be wrapped in a utility method, and the
* code to be executed within transactions, as well as error handling
* (`catch`) and cleanup (`finally`) should be passed to the utility method.
*/
const apiQueryRunner = this.apiDataSource.createQueryRunner();
const geoQueryRunner = this.geoDataSource.createQueryRunner();

Expand Down Expand Up @@ -478,6 +484,16 @@ export class GeoFeaturesService extends AppBaseService<
);
}

private async deleteFeatureAmountsPerPlanningUnit(
geoEntityManager: EntityManager,
featureId: string,
): Promise<void> {
const repo = geoEntityManager.getRepository(
FeatureAmountsPerPlanningUnitEntity,
);
await repo.delete({ featureId });
}

public async updateFeatureForProject(
userId: string,
featureId: string,
Expand Down Expand Up @@ -573,11 +589,56 @@ export class GeoFeaturesService extends AppBaseService<
return left(featureNotDeletable);
}

// NOTE
// This deletes the feature on the API DB but not on the GEO DB. That task is left up to the CleanupTasks
// cronjob; the cronjob is not activated manually because it may take a non trivial amount of time because
// of other data to be garbage collected
await this.repository.delete(featureId);
/**
* @debt Avoid duplicating transaction scaffolding in multiple sites within
* this class: this should be wrapped in a utility method, and the code to
* be executed within transactions, as well as error handling (`catch`) and
* cleanup (`finally`) should be passed to the utility method.
*/
const apiQueryRunner = this.apiDataSource.createQueryRunner();
const geoQueryRunner = this.geoDataSource.createQueryRunner();

await apiQueryRunner.connect();
await geoQueryRunner.connect();

await apiQueryRunner.startTransaction();
await geoQueryRunner.startTransaction();

try {
/**
* Delete the feature, as well as its associated amount per planning unit
* data.
*
* This is fast, and leaving amount per planning unit data behind until it
* is eventually garbage-collected by a scheduled cleanup task may cause
* issues with piece exporters/importers, so it is ok to delete this
* associated data straight away.
*
* Other feature data (such as spatial data) can be left to the cleanup
* tasks to delete when suitable, as doing so synchronously at this stage
* would be a potentially expensive operation.
*/
await apiQueryRunner.manager.delete(GeoFeature, { id: featureId });
await this.deleteFeatureAmountsPerPlanningUnit(
geoQueryRunner.manager,
featureId,
);
await apiQueryRunner.commitTransaction();
await geoQueryRunner.commitTransaction();
} catch (err) {
await apiQueryRunner.rollbackTransaction();
await geoQueryRunner.rollbackTransaction();

this.logger.error(
`An error occurred while deleting feature with id ${featureId} or any of its related data (changes have been rolled back)`,
String(err),
);
throw err;
} finally {
// you need to release a queryRunner which was manually instantiated
await apiQueryRunner.release();
await geoQueryRunner.release();
}

return right(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,13 @@ export class CleanupTasksService implements CleanupTasks {
SELECT df.feature_id FROM dangling_features df
);`,
);

await this.geoEntityManager.query(
`DELETE FROM feature_amounts_per_planning_unit fappu
WHERE fappu.feature_id IN (
SELECT df.feature_id FROM dangling_features df
);`,
);
}

async deleteDanglingCostSurfacesIdsInGeoDb() {
Expand Down

0 comments on commit 413bf67

Please sign in to comment.