fix: prevent BelongsTo's inverse association from itself creating a B…

…elongsTo (#15756)

packages/core/src/associations/belongs-to.ts

@@ -221,11 +221,11 @@ export class BelongsTo<
 
       switch (options.inverse.type) {
         case 'hasMany':
-          HasMany.associate(secret, target, source, passDown, this);
+          HasMany.associate(secret, target, source, passDown, this, this);
           break;
 
         case 'hasOne':
-          HasOne.associate(secret, target, source, passDown, this);
+          HasOne.associate(secret, target, source, passDown, this, this);
           break;
 
         default:

packages/core/src/associations/has-many.ts

@@ -87,6 +87,7 @@ export class HasMany<
     target: ModelStatic<T>,
     options: NormalizedHasManyOptions<SourceKey, TargetKey>,
     parent?: Association,
+    inverse?: BelongsTo<T, S, TargetKey, SourceKey>,
   ) {
     if (
       options.sourceKey
@@ -105,7 +106,7 @@ export class HasMany<
 
     super(secret, source, target, options, parent);
 
-    this.inverse = BelongsTo.associate(secret, target, source, removeUndefined({
+    this.inverse = inverse ?? BelongsTo.associate(secret, target, source, removeUndefined({
       as: options.inverse?.as,
       scope: options.inverse?.scope,
       foreignKey: options.foreignKey,
@@ -140,12 +141,13 @@ export class HasMany<
     T extends Model,
     SourceKey extends AttributeNames<S>,
     TargetKey extends AttributeNames<T>,
-    >(
+  >(
     secret: symbol,
     source: ModelStatic<S>,
     target: ModelStatic<T>,
     options: HasManyOptions<SourceKey, TargetKey> = {},
     parent?: Association<any>,
+    inverse?: BelongsTo<T, S, TargetKey, SourceKey>,
   ): HasMany<S, T, SourceKey, TargetKey> {
 
     return defineAssociation<
@@ -160,7 +162,7 @@ export class HasMany<
         throw new AssociationError('Both options "as" and "inverse.as" must be defined for hasMany self-associations, and their value must be different.');
       }
 
-      return new HasMany(secret, source, target, normalizedOptions, parent);
+      return new HasMany(secret, source, target, normalizedOptions, parent, inverse);
     });
   }
 
@@ -696,13 +698,12 @@ export interface HasManyCreateAssociationMixinOptions<T extends Model>
  * @see Model.hasMany
  */
 export type HasManyCreateAssociationMixin<
-  TModel extends Model,
-  TForeignKey extends keyof CreationAttributes<TModel> = never,
-  TScope extends keyof CreationAttributes<TModel> = never,
+  Target extends Model,
+  ExcludedAttributes extends keyof CreationAttributes<Target> = never,
   > = (
-  values?: Omit<CreationAttributes<TModel>, TForeignKey | TScope>,
-  options?: HasManyCreateAssociationMixinOptions<TModel>
-) => Promise<TModel>;
+  values?: Omit<CreationAttributes<Target>, ExcludedAttributes>,
+  options?: HasManyCreateAssociationMixinOptions<Target>
+) => Promise<Target>;
 
 /**
  * The options for the removeAssociation mixin of the hasMany association.
@@ -807,6 +808,9 @@ export interface HasManyHasAssociationsMixinOptions<T extends Model>
  *
  * @see Model.hasMany
  */
+// TODO: this should be renamed to "HasManyHasAllAssociationsMixin",
+//       we should also add a "HasManyHasAnyAssociationsMixin"
+//       and "HasManyHasAssociationsMixin" should instead return a Map of id -> boolean or WeakMap of instance -> boolean
 export type HasManyHasAssociationsMixin<TModel extends Model, TModelPrimaryKey> = (
   targets: Array<TModel | TModelPrimaryKey>,
   options?: HasManyHasAssociationsMixinOptions<TModel>

packages/core/src/associations/has-one.ts

@@ -88,6 +88,7 @@ export class HasOne<
     target: ModelStatic<T>,
     options: NormalizedHasOneOptions<SourceKey, TargetKey>,
     parent?: Association,
+    inverse?: BelongsTo<T, S, TargetKey, SourceKey>,
   ) {
     if (
       options?.sourceKey
@@ -97,14 +98,12 @@ export class HasOne<
     }
 
     if ('keyType' in options) {
-      throw new TypeError('Option "keyType" has been removed from the BelongsTo\'s options. Set "foreignKey.type" instead.');
+      throw new TypeError(`Option "keyType" has been removed from the BelongsTo's options. Set "foreignKey.type" instead.`);
     }
 
-    // TODO: throw is source model has a composite primary key.
-
     super(secret, source, target, options, parent);
 
-    this.inverse = BelongsTo.associate(secret, target, source, removeUndefined({
+    this.inverse = inverse ?? BelongsTo.associate(secret, target, source, removeUndefined({
       as: options.inverse?.as,
       scope: options.inverse?.scope,
       foreignKey: options.foreignKey,
@@ -134,12 +133,13 @@ export class HasOne<
     T extends Model,
     SourceKey extends AttributeNames<S>,
     TargetKey extends AttributeNames<T>,
-    >(
+  >(
     secret: symbol,
     source: ModelStatic<S>,
     target: ModelStatic<T>,
     options: HasOneOptions<SourceKey, TargetKey> = {},
     parent?: Association<any>,
+    inverse?: BelongsTo<T, S, TargetKey, SourceKey>,
   ): HasOne<S, T, SourceKey, TargetKey> {
     return defineAssociation<
       HasOne<S, T, SourceKey, TargetKey>,
@@ -156,7 +156,7 @@ This is because hasOne associations automatically create the corresponding belon
 If having two associations does not make sense (for instance a "spouse" association from user to user), consider using belongsTo instead of hasOne.`);
       }
 
-      return new HasOne(secret, source, target, normalizedOptions, parent);
+      return new HasOne(secret, source, target, normalizedOptions, parent, inverse);
     });
   }
 
@@ -457,8 +457,10 @@ export interface HasOneCreateAssociationMixinOptions<T extends Model>
  *
  * @see Model.hasOne
  */
-export type HasOneCreateAssociationMixin<T extends Model> = (
-  // TODO: omit the foreign key from CreationAttributes once we have a way to determine which key is the foreign key in typings
-  values?: CreationAttributes<T>,
-  options?: HasOneCreateAssociationMixinOptions<T>
-) => Promise<T>;
+export type HasOneCreateAssociationMixin<
+  Target extends Model,
+  ExcludedAttributes extends keyof CreationAttributes<Target> = never,
+> = (
+  values?: Omit<CreationAttributes<Target>, ExcludedAttributes>,
+  options?: HasOneCreateAssociationMixinOptions<Target>
+) => Promise<Target>;

packages/core/test/unit/associations/belongs-to.test.ts

@@ -1,8 +1,9 @@
 import { expect } from 'chai';
 import each from 'lodash/each';
 import sinon from 'sinon';
-import type { ModelStatic } from '@sequelize/core';
-import { DataTypes, Deferrable } from '@sequelize/core';
+import type { CreationOptional, ModelStatic, NonAttribute } from '@sequelize/core';
+import { DataTypes, Deferrable, Model } from '@sequelize/core';
+import { BelongsTo } from '@sequelize/core/decorators-legacy';
 import { sequelize, getTestDialectTeaser } from '../../support';
 
 describe(getTestDialectTeaser('belongsTo'), () => {
@@ -103,6 +104,39 @@ describe(getTestDialectTeaser('belongsTo'), () => {
     expect(A.getAttributes().BId.references?.deferrable).to.equal(Deferrable.INITIALLY_IMMEDIATE);
   });
 
+  // See https://github.com/sequelize/sequelize/issues/15625 for more details
+  it('should be possible to define two belongsTo associations with the same target #15625', () => {
+    class Post extends Model {
+      declare id: CreationOptional<number>;
+
+      @BelongsTo(() => Author, {
+        foreignKey: 'authorId',
+        targetKey: 'id',
+        inverse: { as: 'myBooks', type: 'hasMany' },
+      })
+      declare author: NonAttribute<Author>;
+
+      declare authorId: number;
+
+      @BelongsTo(() => Author, {
+        foreignKey: 'coAuthorId',
+        targetKey: 'id',
+        inverse: { as: 'notMyBooks', type: 'hasMany' },
+      })
+      declare coAuthor: NonAttribute<Author>;
+
+      declare coAuthorId: number;
+    }
+
+    class Author extends Model {
+      declare id: number;
+    }
+
+    // This would previously fail because the BelongsTo association would create an hasMany association which would
+    // then try to create a redundant belongsTo association
+    sequelize.addModels([Post, Author]);
+  });
+
   describe('association hooks', () => {
     let Projects: ModelStatic<any>;
     let Tasks: ModelStatic<any>;
@@ -112,7 +146,7 @@ describe(getTestDialectTeaser('belongsTo'), () => {
       Tasks = sequelize.define('Task', { title: DataTypes.STRING });
     });
 
-    describe('beforeBelongsToAssociate', () => {
+    describe('beforeAssociate', () => {
       it('should trigger', () => {
         const beforeAssociate = sinon.spy();
         Projects.beforeAssociate(beforeAssociate);
@@ -138,7 +172,8 @@ describe(getTestDialectTeaser('belongsTo'), () => {
         expect(beforeAssociate).to.not.have.been.called;
       });
     });
-    describe('afterBelongsToAssociate', () => {
+
+    describe('afterAssociate', () => {
       it('should trigger', () => {
         const afterAssociate = sinon.spy();
         Projects.afterAssociate(afterAssociate);
@@ -159,6 +194,7 @@ describe(getTestDialectTeaser('belongsTo'), () => {
 
         expect(afterAssociateArgs[1].sequelize.constructor.name).to.equal('Sequelize');
       });
+
       it('should not trigger association hooks', () => {
         const afterAssociate = sinon.spy();
         Projects.afterAssociate(afterAssociate);