Issues with bidirectionalRelations Generation in Entity Models

Ian-Eduware commented 3 months ago

Describe the bug

I am encountering an issue with Mikro-ORM where the entity generator does not automatically create the relationship properties between Users and Licenses. Although I have defined the relationship in the database schema, the entities generated do not reflect this relationship as expected

Reproduction

import { MsSqlDriver, Options } from '@mikro-orm/mssql';
import { TsMorphMetadataProvider } from '@mikro-orm/reflection';
import dotenv from 'dotenv';
dotenv.config();

const config: Options = {
    driver: MsSqlDriver,
    host: process.env.DB_HOST,
    port: 1433,
    user: process.env.DB_USER,
    password: process.env.DB_PASS,
    dbName: process.env.DB_NAME,
    entities: ['dist/entities/**/*.js'],
    entitiesTs: ['src/entities/**/*.ts'],
    discovery: {
        warnWhenNoEntities: false,
    },
    metadataProvider: TsMorphMetadataProvider,
    baseDir: process.cwd(),
    debug: true,
};
export default config;

// scripts/generate-entities.ts
import { MikroORM } from '@mikro-orm/core';
import { EntityGenerator } from '@mikro-orm/entity-generator';
import config from '../src/config/mikro-orm.config';

const generateEntities = async () => {
    const orm = await MikroORM.init({
        ...config,
        discovery: { warnWhenNoEntities: false },
        extensions: [EntityGenerator],
    });
    const dump = await orm.entityGenerator.generate({
        save: true,
        path: process.cwd() + '/src/entities',
        bidirectionalRelations: true,
    });
    await orm.close(true);
};

generateEntities().catch((err) => {
    console.error(err);
    process.exit(1);
});

@Entity({ tableName: 'Users' })
export class Users {
  @PrimaryKey({ fieldName: 'UserID' })
  UserID!: number;

  @Property({ fieldName: 'Username', length: 255 })
  Username!: string;

}

@Entity({ tableName: 'Licenses' })
export class Licenses {
  @PrimaryKey({ fieldName: 'LicenseID' })
  LicenseID!: number;

 @ManyToOne({ entity: () => Users, fieldName: 'UserID', index: '<Index_License_UserID>' })
  UserID!: Users;

}

What driver are you using?

@mikro-orm/mssql

MikroORM version

^6.2.9

Node.js version

v22.3.0

Operating system

Windows 11

Validations

[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Slack.
[X] The provided reproduction is a minimal reproducible example of the bug.

boenrobot commented 3 months ago

Could you add a schema (DDL) dump please? That would allow a reproduction.

B4nan commented 3 months ago

Yes please, provide a schema, what you gave us is not a reproduction without it.

B4nan commented 3 months ago

Closing as not reproducible, let's reopen with a reproduction.

Ian-Eduware commented 3 months ago

My apologies for the wait. I kept getting a TypeError: Unknown file extension ".ts" error when trying to run npx mikro-orm schema:create -d so instead I used the SchemaGenerator programmatically. with const createDump = await orm.schema.getCreateSchemaSQL();. Please see a stripped version of schema for users and licenses.

Schema create output below

exec sp_MSforeachtable 'alter table ? nocheck constraint all';

CREATE TABLE [Users] ([UserID] int identity(1,1) not null primary key, [Username] nvarchar(255) not null);
CREATE UNIQUE INDEX [IX_Users_Username] ON [Users] ([Username]) WHERE [Username] IS NOT NULL;

CREATE TABLE [Licenses] ([LicenseID] int identity(1,1) not null primary key, [UserID] int not null);
CREATE INDEX [<Index_License_UserID>] ON [Licenses] ([UserID]);

alter table [Licenses] add constraint [Licenses_UserID_foreign] foreign key ([UserID]) references [Users] ([UserID]) on update cascade;

exec sp_MSforeachtable 'alter table ? check constraint all';

If I try to test the User entity with the findOne() function, ignoring the fact that the license relationship isnt created I get a query error of

[query] select top (1) [u0].*, [u1].[id] as [u1__id] from [users] as [u0] left join [user_twitter_relation] as [u1] on [u0].[UserID] = [u1].[user_id] where [u0].[Username] = N'test' [took 27 ms]
Authentication error: TableNotFoundException: select top (1) [u0].*, [u1].[id] as [u1__id] from [users] as [u0] left join [user_twitter_relation] as [u1] on [u0].[UserID] = [u1].[user_id] where [u0].[Username] = N'test' - Invalid object name 'user_twitter_relation'.

I have tried to re-create the schema below with UserTwitterRelation added but I keep getting this schema gen error

ReferenceError: Cannot access 'Users' before initialization
    at file:///C:/Node/Mikro-orm/src/entities/UserTwitterRelation.ts:22:31

B4nan commented 3 months ago

I am not following, you reported an issue with the entity generator, so you must have had a schema before - how is using the schema generator now relevant? We wanted to see your schema, not output from the schema generator. What kind of setup are you trying to build?

I kept getting a TypeError: Unknown file extension ".ts"
ReferenceError: Cannot access 'Users' before initialization

Sounds like you have an ESM project? The Getting Started guide has some information on this topic, it's a bit tricky, especially with decorators.

https://mikro-orm.io/docs/guide/first-entity#ecmascript-modules

B4nan commented 3 months ago

The schema you provided here works just fine, it produces the following entities, including the 1:m relations:

import { Entity, ManyToOne, PrimaryKey, PrimaryKeyProp } from '@mikro-orm/core';
import { Users } from './Users';

@Entity({ tableName: 'Licenses' })
export class Licenses {

  [PrimaryKeyProp]?: 'LicenseID';

  @PrimaryKey({ fieldName: 'LicenseID' })
  LicenseID!: number;

  @ManyToOne({ entity: () => Users, fieldName: 'UserID', updateRule: 'cascade', index: '<Index_License_UserID>' })
  UserID!: Users;

}

import { Collection, Entity, OneToMany, PrimaryKey, PrimaryKeyProp, Property, Unique } from '@mikro-orm/core';
import { Licenses } from './Licenses';

@Entity({ tableName: 'Users' })
@Unique({ name: 'IX_Users_Username', expression: 'create unique index [IX_Users_Username] on [Users] (where ([Username] IS NOT NULL))' })
export class Users {

  [PrimaryKeyProp]?: 'UserID';

  @PrimaryKey({ fieldName: 'UserID' })
  UserID!: number;

  @Unique({ name: 'IX_Users_Username' })
  @Property({ fieldName: 'Username' })
  Username!: string;

  @OneToMany({ entity: () => Licenses, mappedBy: 'UserID' })
  UserIDInverse = new Collection<Licenses>(this);

}

Note that I am trying this out on master, could be something already addressed in the last month, so maybe try the latest dev version instead of 6.2.9.

I am also not following why are you using ts-morph metadata provider if you are bootstrapping entities from an existing schema, which brings me again to my previous question - what kind of setup are you trying to build?

Ian-Eduware commented 3 months ago

I am not following, you reported an issue with the entity generator, so you must have had a schema before - how is using the schema generator now relevant? We wanted to see your schema, not output from the schema generator. What kind of setup are you trying to build?

I cant return the whole schema for data privacy reasons hence why I just returned the schema that was generated from the current entities shown thinking that would help let you repro it in some way.

I am also not following why are you using ts-morph metadata provider if you are bootstrapping entities from an existing schema, which brings me again to my previous question - what kind of setup are you trying to build?

Yes I am generating my types from an existing schema. Is a metadata provider not recommended in that case? I assume regardless if they are generated or not I should still run an entity discovery process to ensure the entities are correct.

Note that I am trying this out on master, could be something already addressed in the last month, so maybe try the latest dev version instead of 6.2.9.

I have tried using the dev version but still no change. Something I noticed is that only one relationship that maps to UserID is created. In that case, I see the mapped field name is UserIDInverse. Now, what if I have multiple items mapped to UserID? How does Mikro-ORM handle that naming convention? Is it with UserIDInverse1, UserIDInverse2, etc.?

The reason I ask is that I have noticed many of my tables with a bidirectional connection mapped by UserID aren't showing, and only one is returned when generating. For example, here are some more of my entities stripped down. You can see only one UserIDInverse is created. Is that a possible bug? I am trying to generate the schema for you, as I can't return my production one here, but for now, hopefully, the entity example is helpful until I get past the TypeError: Unknown file extension ".ts" and ReferenceError: Cannot access 'Users' before initialization.

what kind of setup are you trying to build?

TS backend for a legacy mssql DB. Hopefully with auto generating entities (with bidirectional relationships). Nothing to complex

import { Collection, Entity, Index, ManyToMany, ManyToOne, OneToMany, OneToOne, type Opt, PrimaryKey, PrimaryKeyProp, Property, Unique } from '@mikro-orm/core';
import { WizardOrigin } from './WizardOrigin';

@Entity()
@Index({ name: 'Deleted', properties: ['deleted', 'userId'] })
export class Users {
    [PrimaryKeyProp]?: 'userId';

    @PrimaryKey({ fieldName: 'UserID' })
    userId!: number;

    @Unique({ name: 'IX_Users_Username' })
    @Property({ fieldName: 'Username', type: 'string', columnType: 'varchar(255)' })
    username!: string;

    @OneToMany({ entity: () => WizardOrigin, mappedBy: 'userId' })
    userIdInverse = new Collection<WizardOrigin>(this);
}

import { Entity, ManyToOne, PrimaryKey, PrimaryKeyProp, Property } from '@mikro-orm/core';
import { Users } from './Users';

@Entity()
export class WizardOrigin {
    [PrimaryKeyProp]?: 'wizardOriginId';

    @PrimaryKey({ fieldName: 'WizardOriginID' })
    wizardOriginId!: number;

    @ManyToOne({ entity: () => Users, fieldName: 'UserID' })
    userId!: Users;
}

import { Entity, OneToOne, PrimaryKey, PrimaryKeyProp } from '@mikro-orm/core';
import { Users } from './Users';

@Entity()
export class UserInfo {
    [PrimaryKeyProp]?: 'userInfoId';

    @PrimaryKey({ fieldName: 'UserInfoID' })
    userInfoId!: number;

    @OneToOne({ entity: () => Users, fieldName: 'UserID', nullable: true, unique: 'IX_UserID' })
    userId?: Users;
}

import { Entity, OneToOne, PrimaryKeyProp } from '@mikro-orm/core';
import { Users } from '../entities/Users';

@Entity()
export class TrialInfo {
    [PrimaryKeyProp]?: 'userId';

    @OneToOne({ entity: () => Users, fieldName: 'UserID', primary: true })
    userId!: Users;
}

import { Entity, ManyToOne, PrimaryKey, PrimaryKeyProp } from '@mikro-orm/core';
import { Users } from './Users';

@Entity()
export class Licenses {
    [PrimaryKeyProp]?: 'licenseId';

    @PrimaryKey({ fieldName: 'LicenseID' })
    licenseId!: number;

    @ManyToOne({ entity: () => Users, fieldName: 'UserID', index: '<Index_License_UserID>' })
    userId!: Users;
}

B4nan commented 3 months ago

Ha, the duplicities might be actually it, looking at the code, we are not handling that apparently:

https://github.com/mikro-orm/mikro-orm/blob/master/packages/entity-generator/src/EntityGenerator.ts#L265

Btw no one is asking for your production schema, you should provide minimal reproduction, so make a copy of it, start dropping tables till it works correctly, then drop all columns till it starts working correctly, the result is what we need, and you can obfuscate the names as much as you wish. But I guess we don't need it now, since you identified the cause already, it should be easy to reproduce.

You don't need the ts-morph metadata provider if you generate entities via the generator, you are making your setup unnecessarily more complex with it, just use the defaults. Also, you don't need metadata provider to validate your schema, that is a component to load metadata from entity definitions on the ORM init, so it can work with them (e.g. when hydrating or creating queries or altering schema). The entity generator is not using this component at all, and it generates the definitions that are valid for the default reflect metadata provider. You can also opt in to generate the entities including the types, so even reflect-metadata wont be necessary, or use EntitySchema so you don't have decorators (in case you would like to compile with something like esbuild which doesn't support the legacy decorators).

B4nan commented 3 months ago

Hmm fixing this actually feels a tiny bit breaking, since now we only define the last relation from the list of duplicities, if I add the suffix, they will change order (the one without suffix will be a different one than before potentially - it happens in the existing tests multiple times). I guess I will change the naming for the inverse sides completely and leave the Inverse suffix only for M:N relations where it makes some sense, and use the target entity name for the others. Now it's really meant for users to change it anyway, having an inverse called userIdInverse1/2/3 feels weird when it could be called wizardOrigins/userInfo/trialInfo. Will also make it configurable for anyone who could be already depending on those dummy names.

mikro-orm / mikro-orm