Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Naming errors in Entity Generator #5400

Open
5 tasks done
jefmenegazzo opened this issue Apr 1, 2024 · 4 comments
Open
5 tasks done

Naming errors in Entity Generator #5400

jefmenegazzo opened this issue Apr 1, 2024 · 4 comments

Comments

@jefmenegazzo
Copy link

Describe the bug

There are two minor bugs about naming in the Entity Generator.

1. Class names that not follows Pascal Case

In entities with more than one underscore between words, pascal case is not applied correctly.

@Entity({ tableName: "wac_tela_principal_wac___ident" })
export class WacTelaPrincipalWac_dent { }

@Entity({ tableName: "sft_tqm___ocorrencia_envolvido" })
export class SftTqm_correnciaEnvolvido { }

@Entity({ tableName: "sft_tela_principal_sft___ident" })
export class SftTelaPrincipalSft_dent { }

As a temporary workaround, I'm using lodash as follows:

onInitialMetadata: (metadata: EntityMetadata<any>[], platform) => {

	for (const entity of metadata) {
		entity.className = lodash.upperFirst(lodash.camelCase(entity.tableName));
	}
},

2. When using scalarPropertiesForRelations: 'always', some relations are not formatted correctly

@ManyToOne({ entity: () => FrUsuario, ref: true, fieldName: "usr_codigo_app" })
public.frUsuario!: Ref<FrUsuario>;

@ManyToOne({ entity: () => SftSubProduto, ref: true, fieldName: "subpro_id_subtituto", nullable: true })
public.sftSubProduto?: Ref<SftSubProduto>;

@OneToOne({ entity: () => SftContatoWhatsOptIn, ref: true, mappedBy: "public.sftContato" })
public.sftContatoInverse?: Ref<SftContatoWhatsOptIn>;

It appears to only happen with ManyToOne and OneToOne relationships. In this case, 'public' is the schema name.

Reproduction

async function getDatabaseMetadata(connection: DatabaseConnection) {

	const orm = await MikroORM.init({
		discovery: {
			warnWhenNoEntities: false,
		},
		extensions: [EntityGenerator],
		driver: PostgreSqlDriver,
		schema: connection.schema,
		dbName: connection.dbName,
		host: connection.host,
		port: connection.port,
		user: connection.user,
		password: connection.password,
		logger: logger.log,
		// debug: true,
	});

	let initialMetadata: EntityMetadata<any>[] = [];
	let processedMetadata: EntityMetadata<any>[] = [];

	const entitiesSource = await orm.entityGenerator.generate({
		bidirectionalRelations: true,
		identifiedReferences: true,
		scalarTypeInDecorator: true,
		scalarPropertiesForRelations: 'always',
		onlyPurePivotTables: false,
		fileName: (className: string) => lodash.kebabCase(className),
		onInitialMetadata: (metadata: EntityMetadata<any>[], platform) => {

			// WORKAROUND
			for (const entity of metadata) {
				entity.className = lodash.upperFirst(lodash.camelCase(entity.tableName));
			}

			initialMetadata = metadata;
		},
		onProcessedMetadata: (metadata, platform) => { processedMetadata = metadata; },
	});

	await orm.close(true);

	return { entitiesSource, initialMetadata, processedMetadata };
}

What driver are you using?

@mikro-orm/postgresql

MikroORM version

6.1.12

Node.js version

v20.11.1, typescript 5.3.3

Operating system

Windows 11

Validations

@boenrobot
Copy link
Collaborator

boenrobot commented Apr 2, 2024

About 1, you can also use a custom naming strategy as a more "legit" workaround.

Extend the underscore one, and override the getEntityName() method. This way, you'll ensure that any references to these tables are also updated properly. You can do that in the hooks too, sure, but it's more work for the hook.

@boenrobot
Copy link
Collaborator

boenrobot commented Apr 2, 2024

About 2... can you share the database's DDL? Not the full... just a reduced subset, including just the tables involved above.

I was recently tackling weird edge cases like this, with the results so far being in #5359, but as I mention even there, there is one edge case that's not covered there, and inherently also in the current version... identifiers with dot in the name. I suspect you have something (table, column, index name, FK constraint name...) that has a dot in its identifier, and that ends up screwing the whole thing.

Or perhaps this is a problem with cross schema FKs... hard to tell without a DDL.

Oh, and btw... you could work around that via a custom naming strategy too... override columnNameToProperty().

@jefmenegazzo
Copy link
Author

@boenrobot About 1, I hadn't seen about Naming Strategy, it really is enough for the problem :)
About 2, I have a simple DDL where the problem also occurs: dump.zip

@boenrobot
Copy link
Collaborator

boenrobot commented Apr 2, 2024

I've added a fix for the first problem in #5359, so that you wouldn't need to override getEntityName().

The same PR also already addressed having invalid characters as identifiers by quoting, meaning that your first example would become

@ManyToOne({ entity: () => FrUsuario, ref: true, fieldName: "usr_codigo_app" })
'public.frUsuario'!: Ref<FrUsuario>;

@ManyToOne({ entity: () => SftSubProduto, ref: true, fieldName: "subpro_id_subtituto", nullable: true })
'public.sftSubProduto'?: Ref<SftSubProduto>;

@OneToOne({ entity: () => SftContatoWhatsOptIn, ref: true, mappedBy: "public.sftContato" })
'public.sftContatoInverse'?: Ref<SftContatoWhatsOptIn>;

which is a valid class definition, where you can access the properties via f.e. (in the case of the first)
entityWithFrUsario['public.frUsuario']

Fixing the "true" problem of the schema name being at all present in the FK name will require more significant refactoring, which I already plan to tackle for the sake of supporting "." in identifiers.

As already mentioned, you can workaround this in the meantime by overring columnNameToProperty() in the naming strategy.

The problem is that when generating a persist: false for a prop that already exists as FK, the FK is prefixed with the name of the table being referenced if possible (i.e. when the FK is the only reference from the current table to the referenced table), or the name of the constraint if not. However, with Postgresql, table names also feature the schema name. I think the problem above, along with supporting "." in table/index/fk names can be "easily" solved by storing the schema in a separate prop, but the hard part is that all related code needs to be updated to handle cross schema references properly after that split.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants