diff --git a/package-lock.json b/package-lock.json index b8d3121..350179e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@athenna/database", - "version": "5.49.0", + "version": "5.50.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@athenna/database", - "version": "5.49.0", + "version": "5.50.0", "license": "MIT", "dependencies": { "@faker-js/faker": "^8.4.1" diff --git a/package.json b/package.json index ed71a23..9aa932c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@athenna/database", - "version": "5.49.0", + "version": "5.50.0", "description": "The Athenna database handler for SQL/NoSQL.", "license": "MIT", "author": "João Lenon ", diff --git a/src/models/builders/ModelQueryBuilder.ts b/src/models/builders/ModelQueryBuilder.ts index bcafe32..19fc344 100644 --- a/src/models/builders/ModelQueryBuilder.ts +++ b/src/models/builders/ModelQueryBuilder.ts @@ -600,16 +600,25 @@ export class ModelQueryBuilder< ) { const options = this.schema.includeWhereHasRelation(relation, closure) + /** + * Snapshot the full options object immediately at call time, before any + * subsequent `with(sameRelation)` call can mutate the shared `options` + * object (e.g. overwriting `closure` or `withClosure`). Because this + * spread happens here — outside the Knex callback — the snapshot is + * frozen regardless of what happens to `options` afterwards. + */ + const snapshot = { ...options } + super.whereExists(query => { - switch (options.type) { + switch (snapshot.type) { case 'hasOne': - return HasOneRelation.whereHas(this.Model, query, options) + return HasOneRelation.whereHas(this.Model, query, snapshot) case 'hasMany': - return HasManyRelation.whereHas(this.Model, query, options) + return HasManyRelation.whereHas(this.Model, query, snapshot) case 'belongsTo': - return BelongsToRelation.whereHas(this.Model, query, options) + return BelongsToRelation.whereHas(this.Model, query, snapshot) case 'belongsToMany': - return BelongsToManyRelation.whereHas(this.Model, query, options) + return BelongsToManyRelation.whereHas(this.Model, query, snapshot) } }) diff --git a/src/models/relations/BelongsTo/BelongsToRelation.ts b/src/models/relations/BelongsTo/BelongsToRelation.ts index 53693fa..e846171 100644 --- a/src/models/relations/BelongsTo/BelongsToRelation.ts +++ b/src/models/relations/BelongsTo/BelongsToRelation.ts @@ -40,7 +40,7 @@ export class BelongsToRelation { .model() .query() .where(relation.primaryKey as never, model[relation.foreignKey]) - .when(relation.closure, relation.closure) + .when(relation.withClosure, relation.withClosure) .find() return model @@ -60,7 +60,7 @@ export class BelongsToRelation { .model() .query() .whereIn(relation.primaryKey as never, foreignValues) - .when(relation.closure, relation.closure) + .when(relation.withClosure, relation.withClosure) .findMany() const map = new Map() diff --git a/src/models/relations/BelongsToMany/BelongsToManyRelation.ts b/src/models/relations/BelongsToMany/BelongsToManyRelation.ts index d54a4d7..ef84aab 100644 --- a/src/models/relations/BelongsToMany/BelongsToManyRelation.ts +++ b/src/models/relations/BelongsToMany/BelongsToManyRelation.ts @@ -49,7 +49,7 @@ export class BelongsToManyRelation { .model() .query() .whereIn(relation.relationPrimaryKey as never, relationIds) - .when(relation.closure, relation.closure) + .when(relation.withClosure, relation.withClosure) .findMany() return model @@ -88,7 +88,7 @@ export class BelongsToManyRelation { .model() .query() .whereIn(relation.relationPrimaryKey as never, relationForeignKey) - .when(relation.closure, relation.closure) + .when(relation.withClosure, relation.withClosure) .findMany() const map = new Map() diff --git a/src/models/relations/HasMany/HasManyRelation.ts b/src/models/relations/HasMany/HasManyRelation.ts index e9da17a..f159d5f 100644 --- a/src/models/relations/HasMany/HasManyRelation.ts +++ b/src/models/relations/HasMany/HasManyRelation.ts @@ -24,7 +24,7 @@ export class HasManyRelation { .model() .query() .where(relation.foreignKey as never, model[relation.primaryKey]) - .when(relation.closure, relation.closure) + .when(relation.withClosure, relation.withClosure) .findMany() return model @@ -42,7 +42,7 @@ export class HasManyRelation { .model() .query() .whereIn(relation.foreignKey as never, primaryValues) - .when(relation.closure, relation.closure) + .when(relation.withClosure, relation.withClosure) .findMany() const map = new Map() diff --git a/src/models/relations/HasOne/HasOneRelation.ts b/src/models/relations/HasOne/HasOneRelation.ts index 2d1315e..bce3c0f 100644 --- a/src/models/relations/HasOne/HasOneRelation.ts +++ b/src/models/relations/HasOne/HasOneRelation.ts @@ -24,7 +24,7 @@ export class HasOneRelation { .model() .query() .where(relation.foreignKey as never, model[relation.primaryKey]) - .when(relation.closure, relation.closure) + .when(relation.withClosure, relation.withClosure) .find() return model @@ -42,7 +42,7 @@ export class HasOneRelation { .model() .query() .whereIn(relation.foreignKey as never, primaryValues) - .when(relation.closure, relation.closure) + .when(relation.withClosure, relation.withClosure) .findMany() const map = new Map() diff --git a/src/models/schemas/ModelSchema.ts b/src/models/schemas/ModelSchema.ts index 54c2192..f1e6de1 100644 --- a/src/models/schemas/ModelSchema.ts +++ b/src/models/schemas/ModelSchema.ts @@ -364,7 +364,7 @@ export class ModelSchema extends Macroable { const i = this.relations.indexOf(options) options.isIncluded = true - options.closure = closure + options.withClosure = closure this.relations[i] = options diff --git a/src/types/relations/BelongsToManyOptions.ts b/src/types/relations/BelongsToManyOptions.ts index ea28673..0b52110 100644 --- a/src/types/relations/BelongsToManyOptions.ts +++ b/src/types/relations/BelongsToManyOptions.ts @@ -28,10 +28,21 @@ export type BelongsToManyOptions< * The closure that should be executed while * querying the relation data from database. * + * Used by `whereHas()` for the WHERE EXISTS subquery. + * * @default undefined */ closure?: (query: ModelQueryBuilder) => any + /** + * The closure provided to `with()` for eager loading. + * Kept separate from {@link closure} so that a `whereHas()` call + * on the same relation never overwrites the eager-load filter. + * + * @default undefined + */ + withClosure?: (query: ModelQueryBuilder) => any + /** * The property name in class of the relation. * diff --git a/src/types/relations/BelongsToOptions.ts b/src/types/relations/BelongsToOptions.ts index 8d897f5..e86db76 100644 --- a/src/types/relations/BelongsToOptions.ts +++ b/src/types/relations/BelongsToOptions.ts @@ -27,10 +27,21 @@ export type BelongsToOptions< * The closure that should be executed while * querying the relation data from database. * + * Used by `whereHas()` for the WHERE EXISTS subquery. + * * @default undefined */ closure?: (query: ModelQueryBuilder) => any + /** + * The closure provided to `with()` for eager loading. + * Kept separate from {@link closure} so that a `whereHas()` call + * on the same relation never overwrites the eager-load filter. + * + * @default undefined + */ + withClosure?: (query: ModelQueryBuilder) => any + /** * The property name in class of the relation. * diff --git a/src/types/relations/HasManyOptions.ts b/src/types/relations/HasManyOptions.ts index e26f311..7028f01 100644 --- a/src/types/relations/HasManyOptions.ts +++ b/src/types/relations/HasManyOptions.ts @@ -27,10 +27,21 @@ export type HasManyOptions< * The closure that should be executed while * querying the relation data from database. * + * Used by `whereHas()` for the WHERE EXISTS subquery. + * * @default undefined */ closure?: (query: ModelQueryBuilder) => any + /** + * The closure provided to `with()` for eager loading. + * Kept separate from {@link closure} so that a `whereHas()` call + * on the same relation never overwrites the eager-load filter. + * + * @default undefined + */ + withClosure?: (query: ModelQueryBuilder) => any + /** * The property name in class of the relation. * diff --git a/src/types/relations/HasOneOptions.ts b/src/types/relations/HasOneOptions.ts index cc78b6f..a853ae6 100644 --- a/src/types/relations/HasOneOptions.ts +++ b/src/types/relations/HasOneOptions.ts @@ -27,10 +27,21 @@ export type HasOneOptions< * The closure that should be executed while * querying the relation data from database. * + * Used by `whereHas()` for the WHERE EXISTS subquery. + * * @default undefined */ closure?: (query: ModelQueryBuilder) => any + /** + * The closure provided to `with()` for eager loading. + * Kept separate from {@link closure} so that a `whereHas()` call + * on the same relation never overwrites the eager-load filter. + * + * @default undefined + */ + withClosure?: (query: ModelQueryBuilder) => any + /** * The property name in class of the relation. * diff --git a/tests/unit/models/builders/ModelQueryBuilderTest.ts b/tests/unit/models/builders/ModelQueryBuilderTest.ts index c7e2bfe..bc6def7 100644 --- a/tests/unit/models/builders/ModelQueryBuilderTest.ts +++ b/tests/unit/models/builders/ModelQueryBuilderTest.ts @@ -2365,4 +2365,104 @@ export default class ModelQueryBuilderTest { assert.equal(findManyCalls, 2) } + + @Test() + public async withBeforeWhereHasShouldStillEagerLoad({ assert }: Context) { + let findManyCalls = 0 + + Mock.stub(Database.driver, 'findMany').callsFake(async () => { + findManyCalls++ + + if (findManyCalls === 1) { + return [{ id: '1', name: 'John Doe' }] + } + + return [{ id: 'p1', userId: '1' }] + }) + + await User.query() + .with('products') + .whereHas('products', qb => qb.where('id', 'p1')) + .findMany() + + assert.equal(findManyCalls, 2) + } + + @Test() + public async whereHasClosureShouldNotLeakIntoEagerLoad({ assert }: Context) { + const whereCalls: string[] = [] + + Mock.stub(Database.driver, 'findMany').callsFake(async () => { + return [{ id: '1', name: 'John Doe' }] + }) + + Mock.stub(Database.driver, 'where').callsFake((...args: any[]) => { + whereCalls.push(args[0]) + return Database.driver + }) + + await User.query() + .with('products') + .whereHas('products', qb => qb.where('whereHasFilter', 'value')) + .findMany() + + assert.isFalse(whereCalls.includes('whereHasFilter')) + } + + @Test() + public async whereHasClosureShouldNotLeakIntoEagerLoadWhenWhereHasRunsFirst({ assert }: Context) { + const whereCalls: string[] = [] + + Mock.stub(Database.driver, 'findMany').callsFake(async () => { + return [{ id: '1', name: 'John Doe' }] + }) + + Mock.stub(Database.driver, 'where').callsFake((...args: any[]) => { + whereCalls.push(args[0]) + return Database.driver + }) + + await User.query() + .whereHas('products', qb => qb.where('whereHasFilter', 'value')) + .with('products') + .findMany() + + assert.isFalse(whereCalls.includes('whereHasFilter')) + } + + @Test() + public async whereExistsIsRegisteredForWhereHasBeforeWith({ assert }: Context) { + let whereExistsCalled = false + + Mock.stub(Database.driver, 'findMany').callsFake(async () => []) + Mock.stub(Database.driver, 'whereExists').callsFake(() => { + whereExistsCalled = true + return Database.driver + }) + + await User.query() + .whereHas('products', qb => qb.where('id', 'p1')) + .with('products') + .findMany() + + assert.isTrue(whereExistsCalled) + } + + @Test() + public async whereExistsIsRegisteredForWithBeforeWhereHas({ assert }: Context) { + let whereExistsCalled = false + + Mock.stub(Database.driver, 'findMany').callsFake(async () => []) + Mock.stub(Database.driver, 'whereExists').callsFake(() => { + whereExistsCalled = true + return Database.driver + }) + + await User.query() + .with('products') + .whereHas('products', qb => qb.where('id', 'p1')) + .findMany() + + assert.isTrue(whereExistsCalled) + } }