From ac8d06ee44e987b1a1863f8036f4abb158702a21 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 29 May 2024 16:25:17 -0500 Subject: [PATCH 1/3] [neo4j] Delete empty VariantGroup in single query --- .../media/progress-report-media.repository.ts | 18 ++++++++---------- .../media/progress-report-media.service.ts | 4 +--- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/components/progress-report/media/progress-report-media.repository.ts b/src/components/progress-report/media/progress-report-media.repository.ts index 6e5c4e1840..e4cb47dafd 100644 --- a/src/components/progress-report/media/progress-report-media.repository.ts +++ b/src/components/progress-report/media/progress-report-media.repository.ts @@ -14,6 +14,7 @@ import { ACTIVE, createNode, createRelationships, + deleteBaseNode, filter, matchProjectScopedRoles, matchProjectSens, @@ -233,17 +234,14 @@ export class ProgressReportMediaRepository extends DtoRepository< .run(); } - async isVariantGroupEmpty(id: string) { - const hasVariant = await this.db + async deleteVariantGroupIfEmpty(id: string) { + await this.db .query() - .match([ - node('variantGroup', 'VariantGroup', { id }), - relation('out', '', 'child', ACTIVE), - node('variant', 'ProgressReportMedia'), - ]) - .return('variant') - .first(); - return !hasVariant; + .match(node('variantGroup', 'VariantGroup', { id })) + .raw('where not exists((variantGroup)-[:child { active: true }]->())') + .apply(deleteBaseNode('variantGroup')) + .return('*') + .executeAndLogStats(); } protected hydrate(session: Session) { diff --git a/src/components/progress-report/media/progress-report-media.service.ts b/src/components/progress-report/media/progress-report-media.service.ts index 72cfa3bf82..265c46ec1b 100644 --- a/src/components/progress-report/media/progress-report-media.service.ts +++ b/src/components/progress-report/media/progress-report-media.service.ts @@ -126,9 +126,7 @@ export class ProgressReportMediaService { .verifyCan('delete'); await this.repo.deleteNode(id); - if (await this.repo.isVariantGroupEmpty(media.variantGroup)) { - await this.repo.deleteNode(media.variantGroup); - } + await this.repo.deleteVariantGroupIfEmpty(media.variantGroup); return media.report; } From 2344052889e30833c6eb3dbec752150fd04379c5 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 29 May 2024 16:36:26 -0500 Subject: [PATCH 2/3] Change `deleteNode` signature --- src/components/budget/budget.service.ts | 2 +- src/components/engagement/engagement.service.ts | 2 +- src/components/partnership/partnership.service.ts | 2 +- src/core/database/common.repository.ts | 5 ++++- src/core/edgedb/common.repository.ts | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/budget/budget.service.ts b/src/components/budget/budget.service.ts index 22553f668a..9cadb4c059 100644 --- a/src/components/budget/budget.service.ts +++ b/src/components/budget/budget.service.ts @@ -265,7 +265,7 @@ export class BudgetService { async deleteRecord(id: ID, session: Session, changeset?: ID): Promise { try { - await this.budgetRecordsRepo.deleteNode(id, changeset); + await this.budgetRecordsRepo.deleteNode(id, { changeset }); } catch (e) { this.logger.warning('Failed to delete Budget Record', { exception: e, diff --git a/src/components/engagement/engagement.service.ts b/src/components/engagement/engagement.service.ts index 8c7d3671fb..0d4431da9b 100644 --- a/src/components/engagement/engagement.service.ts +++ b/src/components/engagement/engagement.service.ts @@ -367,7 +367,7 @@ export class EngagementService { await this.eventBus.publish(new EngagementWillDeleteEvent(object, session)); try { - await this.repo.deleteNode(object, changeset); + await this.repo.deleteNode(object, { changeset }); } catch (e) { this.logger.warning('Failed to delete Engagement', { exception: e, diff --git a/src/components/partnership/partnership.service.ts b/src/components/partnership/partnership.service.ts index e7ab6a7088..890548e596 100644 --- a/src/components/partnership/partnership.service.ts +++ b/src/components/partnership/partnership.service.ts @@ -224,7 +224,7 @@ export class PartnershipService { ); try { - await this.repo.deleteNode(object, changeset); + await this.repo.deleteNode(object, { changeset }); } catch (exception) { this.logger.error('Failed to delete', { id, exception }); throw new ServerException('Failed to delete', exception); diff --git a/src/core/database/common.repository.ts b/src/core/database/common.repository.ts index dd2d418718..f0bc01676d 100644 --- a/src/core/database/common.repository.ts +++ b/src/core/database/common.repository.ts @@ -130,7 +130,10 @@ export class CommonRepository { return res.stats; } - async deleteNode(objectOrId: { id: ID } | ID, changeset?: ID) { + async deleteNode( + objectOrId: { id: ID } | ID, + { changeset }: { changeset?: ID } = {}, + ) { if (!changeset) { await this.db.deleteNode(objectOrId); return; diff --git a/src/core/edgedb/common.repository.ts b/src/core/edgedb/common.repository.ts index 7663cb0111..efeed40baf 100644 --- a/src/core/edgedb/common.repository.ts +++ b/src/core/edgedb/common.repository.ts @@ -61,7 +61,7 @@ export class CommonRepository implements PublicOf { ); } - async deleteNode(objectOrId: { id: ID } | ID, _changeset?: ID) { + async deleteNode(objectOrId: { id: ID } | ID, _: { changeset?: ID } = {}) { const id = isIdLike(objectOrId) ? objectOrId : objectOrId.id; const query = e.delete(e.Object, () => ({ filter_single: { id }, From 76aed54634fa7bf48650aa46431788892dabd9e7 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 29 May 2024 16:53:47 -0500 Subject: [PATCH 3/3] Limit `DtoRepo.deleteNode` to filter to the resource type by default This is a safety thing. Makes it hard to delete something legit but of the wrong type. --- src/core/database/common.repository.ts | 25 +++++++++++++++++-------- src/core/database/dto.repository.ts | 11 +++++++++++ src/core/edgedb/common.repository.ts | 8 ++++++-- src/core/edgedb/dto.repository.ts | 10 ++++++++++ 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/core/database/common.repository.ts b/src/core/database/common.repository.ts index f0bc01676d..eae12be5a4 100644 --- a/src/core/database/common.repository.ts +++ b/src/core/database/common.repository.ts @@ -13,9 +13,10 @@ import { ResourceShape, ServerException, } from '~/common'; +import { ResourceLike, ResourcesHost } from '../resources'; import { DatabaseService } from './database.service'; import { createUniqueConstraint } from './indexer'; -import { ACTIVE, updateRelationList } from './query'; +import { ACTIVE, deleteBaseNode, updateRelationList } from './query'; import { BaseNode } from './results'; /** @@ -23,8 +24,8 @@ import { BaseNode } from './results'; */ @Injectable() export class CommonRepository { - @Inject(DatabaseService) - protected db: DatabaseService; + @Inject() protected db: DatabaseService; + @Inject() protected readonly resources: ResourcesHost; async getBaseNode( id: ID, @@ -132,18 +133,26 @@ export class CommonRepository { async deleteNode( objectOrId: { id: ID } | ID, - { changeset }: { changeset?: ID } = {}, + { changeset, resource }: { changeset?: ID; resource?: ResourceLike } = {}, ) { + const id = isIdLike(objectOrId) ? objectOrId : objectOrId.id; + const label = resource + ? this.resources.enhance(resource).dbLabel + : 'BaseNode'; if (!changeset) { - await this.db.deleteNode(objectOrId); + await this.db + .query() + .matchNode('node', label, { id }) + .apply(deleteBaseNode('node')) + .return('*') + .run(); return; } try { - const id = isIdLike(objectOrId) ? objectOrId : objectOrId.id; await this.db .query() - .match([node('node', 'BaseNode', { id })]) - .match([node('changeset', 'Changeset', { id: changeset })]) + .match(node('node', label, { id })) + .match(node('changeset', 'Changeset', { id: changeset })) .merge([ node('changeset'), relation('out', 'rel', 'changeset'), diff --git a/src/core/database/dto.repository.ts b/src/core/database/dto.repository.ts index 5ca1d1fe8b..17bcb27fb9 100644 --- a/src/core/database/dto.repository.ts +++ b/src/core/database/dto.repository.ts @@ -12,6 +12,7 @@ import { UnsecuredDto, } from '~/common'; import { Privileges } from '../../components/authorization'; +import { ResourceLike } from '../resources'; import { DbChanges, getChanges } from './changes'; import { CommonRepository } from './common.repository'; import { DbTypeOf } from './db-type'; @@ -104,6 +105,16 @@ export const DtoRepository = < .run(); } + async deleteNode( + objectOrId: { id: ID } | ID, + options: { changeset?: ID; resource?: ResourceLike } = {}, + ) { + await super.deleteNode(objectOrId, { + resource: this.resource, + ...options, + }); + } + protected async updateProperties< TObject extends Partial> & { id: ID; diff --git a/src/core/edgedb/common.repository.ts b/src/core/edgedb/common.repository.ts index efeed40baf..c3677429a0 100644 --- a/src/core/edgedb/common.repository.ts +++ b/src/core/edgedb/common.repository.ts @@ -61,9 +61,13 @@ export class CommonRepository implements PublicOf { ); } - async deleteNode(objectOrId: { id: ID } | ID, _: { changeset?: ID } = {}) { + async deleteNode( + objectOrId: { id: ID } | ID, + { resource }: { changeset?: ID; resource?: ResourceLike } = {}, + ) { const id = isIdLike(objectOrId) ? objectOrId : objectOrId.id; - const query = e.delete(e.Object, () => ({ + const type = resource ? this.resources.enhance(resource).db : e.Object; + const query = e.delete(type, () => ({ filter_single: { id }, })); await this.db.run(query); diff --git a/src/core/edgedb/dto.repository.ts b/src/core/edgedb/dto.repository.ts index 2a53f4235e..7620cf0d86 100644 --- a/src/core/edgedb/dto.repository.ts +++ b/src/core/edgedb/dto.repository.ts @@ -232,6 +232,16 @@ export const RepoFor = < async getBaseNodes(ids: readonly ID[], fqn?: ResourceLike) { return await super.getBaseNodes(ids, fqn ?? resource); } + + async deleteNode( + objectOrId: { id: ID } | ID, + options: { changeset?: ID; resource?: ResourceLike } = {}, + ) { + await super.deleteNode(objectOrId, { + resource: this.resource, + ...options, + }); + } } const readManyQuery = e.params({ ids: e.array(e.uuid) }, ({ ids }) => {