diff --git a/packages/graphql-platform-connector-mariadb/src/broker/table/changes.ts b/packages/graphql-platform-connector-mariadb/src/broker/table/changes.ts index 31502197..2d0c8d24 100644 --- a/packages/graphql-platform-connector-mariadb/src/broker/table/changes.ts +++ b/packages/graphql-platform-connector-mariadb/src/broker/table/changes.ts @@ -147,10 +147,9 @@ export class MariaDBBrokerChangesTable extends AbstractTable { OR([ creation || deletion ? `${this.escapeColumnIdentifier('kind', alias)} IN (${[ - creation && utils.MutationType.CREATION, - deletion && utils.MutationType.DELETION, + ...(creation ? [utils.MutationType.CREATION] : []), + ...(deletion ? [utils.MutationType.DELETION] : []), ] - .filter(utils.isNonNil) .map((kind) => this.serializeColumnValue('kind', kind)) .join(',')})` : undefined, diff --git a/packages/graphql-platform-connector-mariadb/src/subscription.test.ts b/packages/graphql-platform-connector-mariadb/src/subscription.test.ts index 69c13c0c..2b70b0df 100644 --- a/packages/graphql-platform-connector-mariadb/src/subscription.test.ts +++ b/packages/graphql-platform-connector-mariadb/src/subscription.test.ts @@ -470,3 +470,58 @@ describe('Subscription', () => { ); }); }); + +// Regression for the companion of PR #20: `EdgeDependency.flattened` surfaces a +// head-node deletion-only dependency (e.g. `Tag` with `{ deletion: true }` and +// no `creation`) whenever the edge has a re-read-synthesizing child. The +// changes-table `filterDependencies` must build a valid `kind IN (...)` clause +// for it instead of crashing while serializing the `false` boolean as a `kind` +// enum value. +describe('Subscription diagnosis with a deletion-only dependency', () => { + const gp = createMyGP('connector_mariadb_subscription_deletion_only'); + + const Article = gp.getNodeByName('Article'); + + let subscription: ChangesSubscriptionStream; + + beforeEach(async () => { + await gp.connector.setup(); + + // Article -> tags -> tag (edge to Tag) -> articles (reverse-edge): the + // `tag` edge gains a re-read-synthesizing child, so its head `Tag` is + // surfaced as deletion-only (no mutable `Tag` field is selected). + subscription = await Article.api.subscribeToChanges(myAdminContext, { + where: { status: ArticleStatus.PUBLISHED }, + selection: { + onUpsert: `{ + id + tags(first: 10) { + tag { + id + articles(first: 10) { + article { id } + } + } + } + }`, + }, + }); + }); + + afterEach(async () => { + await subscription.dispose(); + await gp.connector.teardown(); + }); + + it('surfaces "Tag" as a deletion-only dependency', () => { + assert.deepEqual(subscription.dependencyTree.flattened.toJSON().Tag, { + deletion: true, + }); + }); + + it('diagnoses without crashing on the deletion-only dependency', async () => { + // Before the fix, this rejected with an AssertionError (boolean vs string) + // while building the `kind IN (...)` clause in `filterDependencies`. + await assert.doesNotReject(() => gp.broker.diagnose()); + }); +}); diff --git a/packages/graphql-platform/src/node/dependency.test.ts b/packages/graphql-platform/src/node/dependency.test.ts index 5297b818..6fd85970 100644 --- a/packages/graphql-platform/src/node/dependency.test.ts +++ b/packages/graphql-platform/src/node/dependency.test.ts @@ -2,7 +2,7 @@ import * as utils from '@prismamedia/graphql-platform-utils'; import assert from 'node:assert'; import { before, describe, it } from 'node:test'; import { ArticleStatus, nodes, type MyContext } from '../__tests__/config.js'; -import { GraphQLPlatform } from '../index.js'; +import { GraphQLPlatform, OnEdgeHeadDeletion } from '../index.js'; import { NodeCreation, NodeDeletion, @@ -867,4 +867,139 @@ describe('Dependency', () => { }); }); }); + + // @see https://github.com/prismamedia/graphql-platform - reverse-edge towards a deleted head + describe('Reverse-edge towards a cascade-deleted head', () => { + // A dedicated platform adding a "TagBrandedContent" node, cascade-deleted + // with its "Tag", and reachable from the "Article" selection through + // Article -> tags -> tag -> brandedContents. + const cascadingGP = new GraphQLPlatform({ + nodes: { + ...nodes, + Tag: { + ...nodes.Tag, + reverseEdges: { + ...nodes.Tag.reverseEdges, + brandedContents: { originalEdge: 'TagBrandedContent.tag' }, + }, + }, + TagBrandedContent: { + components: { + tag: { + kind: 'Edge', + head: 'Tag', + onHeadDeletion: OnEdgeHeadDeletion.CASCADE, + nullable: false, + mutable: false, + }, + brandKey: { + kind: 'Leaf', + type: 'NonEmptyTrimmedString', + nullable: false, + mutable: false, + }, + isActive: { kind: 'Leaf', type: 'Boolean' }, + }, + uniques: [['tag', 'brandKey']], + }, + }, + } as any); + + const CascadingArticle = cascadingGP.getNodeByName('Article'); + const Tag = cascadingGP.getNodeByName('Tag'); + const ArticleTag = cascadingGP.getNodeByName('ArticleTag'); + const TagBrandedContent = cascadingGP.getNodeByName('TagBrandedContent'); + + const tagId = '00000000-0000-4000-8000-000000000900'; + + const dependency = new DocumentSetDependency(CascadingArticle, { + selection: CascadingArticle.outputType.select(`{ + _id + tags(first: 100) { + tag { + id + brandedContents(where: { isActive: true }, first: 100) { brandKey } + } + } + }`), + }); + + const tagDeletion = new NodeDeletion(Tag, myRequestContext, { + id: tagId, + title: 'Old Tag', + slug: 'old-tag', + deprecated: false, + createdAt: new Date(), + updatedAt: new Date(), + }); + + const articleTagDeletions = [11, 22, 33].map( + (_id) => + new NodeDeletion(ArticleTag, myRequestContext, { + article: { _id }, + order: 1, + tag: { id: tagId }, + }), + ); + + const brandedContentDeletion = new NodeDeletion( + TagBrandedContent, + myRequestContext, + { tag: { id: tagId }, brandKey: 'CAP', isActive: true }, + ); + + it('surfaces the edge-head deletion to the flattened dependencies (broker delivery)', () => { + // The fold can only collapse the redundant existence if the broker + // actually delivers the Tag deletion. The Article subscription reads only + // immutable Tag fields, so Tag would otherwise be invisible: an edge with + // re-read-synthesizing children (here `tag`, via `brandedContents`) must + // therefore surface its head deletion so the broker wakes on - and + // delivers - it. + assert.deepEqual(dependency.flattened.dependencies.get(Tag)?.toJSON(), { + deletion: true, + }); + }); + + it('does not synthesize a reverse-edge existence towards the deleted tag', () => { + const dependentGraph = dependency.createDependentGraph( + MutationContextChanges.createFromChanges([ + tagDeletion, + ...articleTagDeletions, + brandedContentDeletion, + ]), + ); + + assert(dependentGraph); + + const upsert = dependentGraph.upsertFilter.inputValue; + + // The impacted articles are already captured through the cascade-deleted + // junction rows: the re-read is a plain primary-key lookup. + assert.deepEqual(upsert, { _id_in: [11, 22, 33] }); + + // Explicit guard: no existence towards the deleted tag. + assert( + !JSON.stringify(upsert).includes('tags_some'), + `upsertFilter must not contain an existence towards the deleted tag, got: ${JSON.stringify(upsert)}`, + ); + }); + + it('still synthesizes the existence when the tag is NOT deleted', () => { + // Same branded-content deletion, but the tag survives (no Tag deletion, + // no ArticleTag deletion): the existence towards the affected tag must + // remain, so the affected articles can be re-read. + const dependentGraph = dependency.createDependentGraph( + MutationContextChanges.createFromChanges([brandedContentDeletion]), + ); + + assert(dependentGraph); + + assert( + JSON.stringify(dependentGraph.upsertFilter.inputValue).includes( + 'tags_some', + ), + `upsertFilter must keep the existence towards the surviving tag, got: ${JSON.stringify(dependentGraph.upsertFilter.inputValue)}`, + ); + }); + }); }); diff --git a/packages/graphql-platform/src/node/dependency.test.ts.snapshot b/packages/graphql-platform/src/node/dependency.test.ts.snapshot index d5f8610c..d71d9ecb 100644 --- a/packages/graphql-platform/src/node/dependency.test.ts.snapshot +++ b/packages/graphql-platform/src/node/dependency.test.ts.snapshot @@ -11,6 +11,7 @@ exports[`Dependency > Complex filter & selection > has a consistent dependency-t ] }, "User": { + "deletion": true, "update": [ "lastLoggedInAt" ] diff --git a/packages/graphql-platform/src/node/dependency.ts b/packages/graphql-platform/src/node/dependency.ts index 5d006acb..9d4c5301 100644 --- a/packages/graphql-platform/src/node/dependency.ts +++ b/packages/graphql-platform/src/node/dependency.ts @@ -538,6 +538,45 @@ export class EdgeDependency extends NodeDependencyTree { return [...this.parent.path, this.edge]; } + /** + * An edge surfaces its head-node deletion to the flattened dependencies (and + * therefore to the broker) whenever it has children synthesizing a re-read of + * that head. + * + * When the head is deleted, the existence-filters those children generate + * towards it become unsatisfiable (the referencing rows have been + * cascade-deleted / set-null). Making the head deletion visible lets the + * dependent-graph fold those existences away instead of emitting a + * full-scanning existence towards a row that no longer exists. + * + * Note: this is intentionally added to `flattened` only, not to + * `currentLevel`: the head deletion must be *delivered* so the fold can see + * it, but it must NOT become a hit at this edge level (which would re-emit the + * very existence we want to fold). + */ + @MGetter + public override get flattened(): FlattenedNodeDependencyTree { + const flattened = super.flattened; + + if (!this.children.size || !this.node.isDeletable()) { + return flattened; + } + + const dependencies = new Map(flattened.dependencies); + + const headDeletion = new NodeDependency(this.node, { + [utils.MutationType.DELETION]: true, + }); + + const current = dependencies.get(this.node); + dependencies.set( + this.node, + current ? current.mergeWith(headDeletion) : headDeletion, + ); + + return new FlattenedNodeDependencyTree(dependencies); + } + public attachTo(parent: NodeDependencyTree): this | EdgeDependency { return new EdgeDependency(parent, this.edge, this.config); } diff --git a/packages/graphql-platform/src/node/dependency/dependent-graph.ts b/packages/graphql-platform/src/node/dependency/dependent-graph.ts index 673ab6c8..4fa2fdc9 100644 --- a/packages/graphql-platform/src/node/dependency/dependent-graph.ts +++ b/packages/graphql-platform/src/node/dependency/dependent-graph.ts @@ -1,3 +1,4 @@ +import * as utils from '@prismamedia/graphql-platform-utils'; import assert from 'node:assert'; import type { JsonObject } from 'type-fest'; import { Node } from '../../node.js'; @@ -95,7 +96,9 @@ export class DependentNode { }; } - protected get graphFilter(): NodeFilter { + protected graphFilter( + changes: MutationContextChanges, + ): NodeFilter { const currentLevel = this.path.at(-1)!; if (currentLevel instanceof Node) { @@ -104,7 +107,7 @@ export class DependentNode { OrOperation.create( Array.from( this.children.values(), - (child) => child.graphFilter.filter, + (child) => child.graphFilter(changes).filter, ), ), ); @@ -142,7 +145,7 @@ export class DependentNode { ), ...this.children .values() - .map((child) => child.graphFilter.filter), + .map((child) => child.graphFilter(changes).filter), ]), ), ), @@ -153,7 +156,7 @@ export class DependentNode { OrOperation.create( Array.from( this.children.values(), - (child) => child.graphFilter.filter, + (child) => child.graphFilter(changes).filter, ), ), ); @@ -161,30 +164,45 @@ export class DependentNode { return new NodeFilter( currentLevel.tail, OrOperation.create([ - ...this.hits - .values() - .flatMap((change) => - change instanceof NodeCreation - ? [ + ...this.hits.values().flatMap((change) => + change instanceof NodeCreation + ? [ + currentLevel.originalEdge.referencedUniqueConstraint.createFilterFromValue( + change.newValue[currentLevel.originalEdge.name], + ).filter, + ] + : change instanceof NodeDeletion + ? // A reverse-edge deletion synthesizes a re-read of its + // referenced tail (the parent). If that tail is itself + // deleted within this very change-set, the surrounding + // *ExistsFilter towards it is unsatisfiable: after commit no + // row can still reference the deleted parent (referencing + // rows were cascade-deleted / set-null, and those changes are + // captured elsewhere in the dependent graph). Contributing + // nothing lets the surrounding OrOperation / *ExistsFilter + // fold to FalseValue. + isReferencedTailDeleted( + changes, + currentLevel.tail, currentLevel.originalEdge.referencedUniqueConstraint.createFilterFromValue( - change.newValue[currentLevel.originalEdge.name], - ).filter, - ] - : change instanceof NodeDeletion - ? [ - currentLevel.originalEdge.referencedUniqueConstraint.createFilterFromValue( - change.oldValue[currentLevel.originalEdge.name], - ).filter, - ] + change.oldValue[currentLevel.originalEdge.name], + ), + ) + ? [] : [ - currentLevel.originalEdge.referencedUniqueConstraint.createFilterFromValue( - change.newValue[currentLevel.originalEdge.name], - ).filter, currentLevel.originalEdge.referencedUniqueConstraint.createFilterFromValue( change.oldValue[currentLevel.originalEdge.name], ).filter, - ], - ), + ] + : [ + currentLevel.originalEdge.referencedUniqueConstraint.createFilterFromValue( + change.newValue[currentLevel.originalEdge.name], + ).filter, + currentLevel.originalEdge.referencedUniqueConstraint.createFilterFromValue( + change.oldValue[currentLevel.originalEdge.name], + ).filter, + ], + ), currentLevel instanceof UniqueReverseEdge ? UniqueReverseEdgeExistsFilter.create(currentLevel, headFilter) : MultipleReverseEdgeExistsFilter.create(currentLevel, headFilter), @@ -194,6 +212,27 @@ export class DependentNode { } } +/** + * Returns whether the given tail-filter (a re-read synthesized by a reverse-edge + * change) targets a tail-node that is itself deleted within this change-set. + */ +function isReferencedTailDeleted( + changes: MutationContextChanges, + tail: Node, + tailFilter: NodeFilter, +): boolean { + const deletions = + changes.changesByNode.get(tail)?.[utils.MutationType.DELETION]; + + return deletions + ? deletions + .values() + .some( + (deletion) => tailFilter.execute(deletion.oldValue, true) === true, + ) + : false; +} + export class DependentGraph< TRequestContext extends object = any, > extends DependentNode { @@ -244,8 +283,9 @@ export class DependentGraph< ...deletionOrUpserts .values() .map(({ node, id }) => node.filterInputType.filter(id).filter), - filter.dependencyTree.createDependentGraph(changes)?.graphFilter - .filter ?? FalseValue, + filter.dependencyTree + .createDependentGraph(changes) + ?.graphFilter(changes).filter ?? FalseValue, ]), ]) : FalseValue, @@ -266,7 +306,7 @@ export class DependentGraph< ...upsertIfFilteredIns .values() .map(({ node, id }) => node.filterInputType.filter(id).filter), - this.graphFilter.filter, + this.graphFilter(changes).filter, ]), ]), ]),