Skip to content

Commit

Permalink
feat(entity-generator): detect more ManyToMany relations (#4974)
Browse files Browse the repository at this point in the history
Auto increment columns in pivot entities are now detected.

In addition, pivot tables are allowed to contain additional props,
including relations, but only if those props would not hinder the ORM's
ability to freely insert and remove records from the collection.

In other words, those additional props need to be optional, and have
defaults that are either null or non-unique.
  • Loading branch information
boenrobot committed Jan 4, 2024
1 parent b43ef63 commit d0e3ac9
Show file tree
Hide file tree
Showing 27 changed files with 4,495 additions and 302 deletions.
2 changes: 2 additions & 0 deletions docs/docs/entity-generator.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ Available options:
| `esmImport: boolean` | By default, import statements for entities without extensions are used. If set to `true`, uses ESM style import for imported entities, i.e. adds a `.js` suffix as extension. |
| `scalarTypeInDecorator: boolean` | If `true`, include the `type` option in scalar property decorators. This information is discovered at runtime, but the process of discovery can be skipped by including this option in the decorator. If using `EntitySchema`, this type information is always included. |
| `scalarPropertiesForRelations: 'never' \| 'always' \| 'smart'` | <ul><li> `'never'` (default) - Do not generate any scalar properties for columns covered by foreign key relations. This effectively forces the application to always provide the entire relation, or (if all columns in the relation are nullable) omit the entire relation.</li><li> `'always'` - Generate all scalar properties for all columns covered by foreign key relations. This enables the application to deal with code that disables foreign key checks.</li><li> `'smart'` - Only generate scalar properties for foreign key relations, where doing so is necessary to enable the management of rows where a composite foreign key is not enforced due to some columns being set to NULL. This enables the application to deal with all rows that could possibly be part of a table, even when foreign key checks are always enabled.</li></ul> |
| `onlyPurePivotTables: boolean` | By default, M:N relations are allowed to use pivot tables containing additional columns. If set to `true`, M:N relations will not be generated for such pivot tables. |
| `readOnlyPivotTables: boolean` | By default, M:N relations are only generated if the collection would be writable, i.e. any additional columns need to be optional and have non-unique default values. If set to `true`, also generate M:N relations even if the collection would be read only (meaning the only way to write to it is by using the pivot entity directly). Such collections will include the `persist: false` option. This setting is effectively meaningless if `onlyPurePivotTables` is set to `true`. |

Example configuration:

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/metadata/EntitySchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ export class EntitySchema<Entity = any, Base = never> {
this._meta.simplePK = !this._meta.compositePK && pks[0].kind === ReferenceKind.SCALAR && !pks[0].customType;
}

if (pks.length === 1 && pks[0].type === 'number') {
if (pks.length === 1 && ['number', 'bigint'].includes(pks[0].type)) {
pks[0].autoincrement ??= true;
}

Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,8 @@ export interface GenerateOptions {
scalarTypeInDecorator?: boolean;
scalarPropertiesForRelations?: 'always' | 'never' | 'smart';
fileName?: (className: string) => string;
onlyPurePivotTables?: boolean;
readOnlyPivotTables?: boolean;
}

export interface IEntityGenerator {
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/utils/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ export class Configuration<D extends IDatabaseDriver = IDatabaseDriver, EM exten
scalarTypeInDecorator: false,
scalarPropertiesForRelations: 'never',
fileName: (className: string) => className,
onlyPurePivotTables: false,
readOnlyPivotTables: false,
},
metadataCache: {
pretty: false,
Expand Down Expand Up @@ -594,6 +596,8 @@ export interface MikroORMOptions<D extends IDatabaseDriver = IDatabaseDriver, EM
scalarTypeInDecorator?: boolean;
scalarPropertiesForRelations?: 'always' | 'never' | 'smart';
fileName?: (className: string) => string;
onlyPurePivotTables?: boolean;
readOnlyPivotTables?: boolean;
};
metadataCache: {
enabled?: boolean;
Expand Down
113 changes: 87 additions & 26 deletions packages/entity-generator/src/EntityGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class EntityGenerator {

metadata = metadata.filter(table => !options.skipTables || !options.skipTables.includes(table.tableName));

this.detectManyToManyRelations(metadata);
this.detectManyToManyRelations(metadata, options.onlyPurePivotTables!, options.readOnlyPivotTables!);

if (options.bidirectionalRelations) {
this.generateBidirectionalRelations(metadata);
Expand All @@ -110,7 +110,7 @@ export class EntityGenerator {
return metadata;
}

private detectManyToManyRelations(metadata: EntityMetadata[]): void {
private detectManyToManyRelations(metadata: EntityMetadata[], onlyPurePivotTables: boolean, readOnlyPivotTables: boolean): void {
for (const meta of metadata) {
const isReferenced = metadata.some(m => {
return m.tableName !== meta.tableName && m.relations.some(r => {
Expand All @@ -122,39 +122,99 @@ export class EntityGenerator {
this.referencedEntities.add(meta);
}

// Entities with non-composite PKs are never pivot tables. Skip.
if (!meta.compositePK) {
continue;
}

// Entities where there are not exactly 2 PK relations that are both ManyToOne are never pivot tables. Skip.
const pkRelations = meta.relations.filter(rel => rel.primary);
if (
meta.compositePK && // needs to have composite PK
meta.relations.length === 2 && // there are exactly two relation properties
!meta.relations.some(rel => !rel.primary || rel.kind !== ReferenceKind.MANY_TO_ONE) && // all relations are m:1 and PKs
(
// all properties are relations...
meta.relations.length === meta.props.length
// ... or at least all fields involved are only the fields of the relations
|| (new Set<string>(meta.props.flatMap(prop => prop.fieldNames)).size === (new Set<string>(meta.relations.flatMap(rel => rel.fieldNames)).size))
)
pkRelations.length !== 2 ||
pkRelations.some(rel => rel.kind !== ReferenceKind.MANY_TO_ONE)
) {
meta.pivotTable = true;
const owner = metadata.find(m => m.className === meta.relations[0].type);
continue;
}

if (!owner) {
const pkRelationFields = new Set<string>(pkRelations.flatMap(rel => rel.fieldNames));
const nonPkFields = Array.from(new Set<string>(meta.props.flatMap(prop => prop.fieldNames))).filter(fieldName => !pkRelationFields.has(fieldName));

let fixedOrderColumn: string | undefined;
let isReadOnly = false;

// If there are any fields other than the ones in the two PK relations, table may or may not be a pivot one.
// Check further and skip on disqualification.
if (nonPkFields.length > 0) {
// Additional columns have been disabled with the setting.
// Skip table even it otherwise would have qualified as a pivot table.
if (onlyPurePivotTables) {
continue;
}

const name = this.namingStrategy.columnNameToProperty(meta.tableName.replace(new RegExp('^' + owner.tableName + '_'), ''));
const ownerProp = {
name,
kind: ReferenceKind.MANY_TO_MANY,
pivotTable: meta.tableName,
type: meta.relations[1].type,
joinColumns: meta.relations[0].fieldNames,
inverseJoinColumns: meta.relations[1].fieldNames,
} as EntityProperty;
const pkRelationNames = pkRelations.map(rel => rel.name);
let otherProps = meta.props
.filter(prop => !pkRelationNames.includes(prop.name) &&
prop.persist !== false && // Skip checking non-persist props
prop.fieldNames.some(fieldName => nonPkFields.includes(fieldName)),
);

if (this.referencedEntities.has(meta)) {
ownerProp.pivotEntity = meta.className;
// Deal with the auto increment column first. That is the column used for fixed ordering, if present.
const autoIncrementProp = meta.props.find(prop => prop.autoincrement && prop.fieldNames.length === 1);
if (autoIncrementProp) {
otherProps = otherProps.filter(prop => prop !== autoIncrementProp);
fixedOrderColumn = autoIncrementProp.fieldNames[0];
}
owner.addProperty(ownerProp);

isReadOnly = otherProps.some(prop => {
// If the prop is non-nullable and unique, it will trivially end up causing issues.
// Mark as read only.
if (!prop.nullable && prop.unique) {
return true;
}

// Any other props need to also be optional.
// Whether they have a default or are generated,
// we've already checked that not explicitly setting the property means the default is either NULL,
// or a non-unique non-null value, making it safe to write to pivot entity.
return !prop.optional;
});

if (isReadOnly && !readOnlyPivotTables) {
continue;
}

// If this now proven pivot entity has persistent props other than the fixed order column,
// output it, by considering it as a referenced one.
if (otherProps.length > 0) {
this.referencedEntities.add(meta);
}
}

meta.pivotTable = true;
const owner = metadata.find(m => m.className === meta.relations[0].type)!;

const name = this.namingStrategy.columnNameToProperty(meta.tableName.replace(new RegExp('^' + owner.tableName + '_'), ''));
const ownerProp = {
name,
kind: ReferenceKind.MANY_TO_MANY,
pivotTable: meta.tableName,
type: meta.relations[1].type,
joinColumns: meta.relations[0].fieldNames,
inverseJoinColumns: meta.relations[1].fieldNames,
} as EntityProperty;

if (this.referencedEntities.has(meta)) {
ownerProp.pivotEntity = meta.className;
}
if (fixedOrderColumn) {
ownerProp.fixedOrder = true;
ownerProp.fixedOrderColumn = fixedOrderColumn;
}
if (isReadOnly) {
ownerProp.persist = false;
}

owner.addProperty(ownerProp);
}
}

Expand All @@ -169,6 +229,7 @@ export class EntityGenerator {
referencedTableName: meta.tableName,
referencedColumnNames: Utils.flatten(targetMeta.getPrimaryProps().map(pk => pk.fieldNames)),
mappedBy: prop.name,
persist: prop.persist,
} as EntityProperty;

if (prop.kind === ReferenceKind.MANY_TO_ONE) {
Expand Down
16 changes: 16 additions & 0 deletions packages/entity-generator/src/SourceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,15 @@ export class SourceFile {
assign('precision');
assign('scale');
}
if (prop.autoincrement) {
if (!prop.primary || !['number', 'bigint'].includes(t) || this.meta.getPrimaryProps().length !== 1) {
options.autoincrement = true;
}
} else {
if (prop.primary && ['number', 'bigint'].includes(t) && this.meta.getPrimaryProps().length === 1) {
options.autoincrement = false;
}
}
}

protected getManyToManyDecoratorOptions(options: Dictionary, prop: EntityProperty) {
Expand Down Expand Up @@ -365,6 +374,13 @@ export class SourceFile {
} else {
options.inverseJoinColumns = `[${prop.inverseJoinColumns.map(this.quote).join(', ')}]`;
}

if (prop.fixedOrder) {
options.fixedOrder = true;
if (prop.fixedOrderColumn && prop.fixedOrderColumn !== this.namingStrategy.referenceColumnName()) {
options.fixedOrderColumn = this.quote(prop.fixedOrderColumn);
}
}
}

protected getOneToManyDecoratorOptions(options: Dictionary, prop: EntityProperty) {
Expand Down
11 changes: 6 additions & 5 deletions packages/knex/src/schema/DatabaseTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ export class DatabaseTable {

// Index is for FK. Map to the FK prop and move on.
const fkForIndex = fkIndexes.get(index);
if (fkForIndex) {
if (fkForIndex && !fkForIndex.fk.columnNames.some(col => !index.columnNames.includes(col))) {
ret.properties = [this.getPropertyName(namingStrategy, fkForIndex.baseName, fkForIndex.fk)];
const map = index.unique ? compositeFkUniques : compositeFkIndexes;
map[ret.properties[0]] = { keyName: index.keyName };
Expand Down Expand Up @@ -438,6 +438,10 @@ export class DatabaseTable {
return index.columnNames.length >= fkColumnsLength && !currentFk.columnNames.some((columnName, i) => index.columnNames[i] !== columnName);
});
possibleIndexes.sort((a, b) => {
if (a.columnNames.length !== b.columnNames.length) {
return a.columnNames.length < b.columnNames.length ? -1 : 1;
}

if (a.primary !== b.primary) {
return a.primary ? -1 : 1;
}
Expand All @@ -446,10 +450,6 @@ export class DatabaseTable {
return a.unique ? -1 : 1;
}

if (a.columnNames.length !== b.columnNames.length) {
return a.columnNames.length < b.columnNames.length ? -1 : 1;
}

return a.keyName.localeCompare(b.keyName);
});

Expand Down Expand Up @@ -661,6 +661,7 @@ export class DatabaseTable {
defaultRaw: this.getPropertyDefaultValue(schemaHelper, column, type, true),
nullable: column.nullable,
primary: column.primary && persist,
autoincrement: column.autoincrement,
fieldName: column.name,
length: column.length,
precision: column.precision,
Expand Down
19 changes: 5 additions & 14 deletions tests/features/entity-generator/AmbiguousFks.mysql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,11 @@ beforeAll(async () => {
extensions: [EntityGenerator],
multipleStatements: true,
});
await orm.schema.ensureDatabase();
await orm.schema.execute(schema);
const driver = orm.config.getDriver();
if (!await driver.getPlatform().getSchemaHelper()?.databaseExists(driver.getConnection(), schemaName)) {
await orm.schema.createSchema({ schema: schemaName });
await orm.schema.execute(schema);
}
await orm.close(true);
});

Expand All @@ -188,18 +191,6 @@ afterEach(async () => {
await orm.close(true);
});

afterAll(async () => {
orm = await MikroORM.init({
dbName: schemaName,
port: 3308,
discovery: { warnWhenNoEntities: false },
extensions: [EntityGenerator],
multipleStatements: true,
});
await orm.schema.dropDatabase();
await orm.close(true);
});

describe(schemaName, () => {
describe.each(['never', 'always', 'smart'])('scalarPropertiesForRelations=%s', i => {
const scalarPropertiesForRelations = i as NonNullable<MikroORMOptions['entityGenerator']['scalarPropertiesForRelations']>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ describe('EntityGenerator', () => {
const orm = await initORMMySql('mysql', {}, true);
const dump = await orm.entityGenerator.generate({ save: true, path: './temp/entities' });
expect(dump).toMatchSnapshot('mysql-entity-dump');

await expect(pathExists('./temp/entities/Author2.ts')).resolves.toBe(true);
await remove('./temp/entities');

Expand Down
19 changes: 5 additions & 14 deletions tests/features/entity-generator/FkIndexSelection.mysql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,11 @@ beforeAll(async () => {
extensions: [EntityGenerator],
multipleStatements: true,
});
await orm.schema.ensureDatabase();
await orm.schema.execute(schema);
const driver = orm.config.getDriver();
if (!await driver.getPlatform().getSchemaHelper()?.databaseExists(driver.getConnection(), schemaName)) {
await orm.schema.createSchema({ schema: schemaName });
await orm.schema.execute(schema);
}
await orm.close(true);
});

Expand All @@ -68,18 +71,6 @@ afterEach(async () => {
await orm.close(true);
});

afterAll(async () => {
orm = await MikroORM.init({
dbName: schemaName,
port: 3308,
discovery: { warnWhenNoEntities: false },
extensions: [EntityGenerator],
multipleStatements: true,
});
await orm.schema.dropDatabase();
await orm.close(true);
});

describe(schemaName, () => {
describe.each(['never', 'always', 'smart'])('scalarPropertiesForRelations=%s', i => {
const scalarPropertiesForRelations = i as NonNullable<MikroORMOptions['entityGenerator']['scalarPropertiesForRelations']>;
Expand Down
19 changes: 5 additions & 14 deletions tests/features/entity-generator/FkSharedWithColumn.mysql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ beforeAll(async () => {
extensions: [EntityGenerator],
multipleStatements: true,
});
await orm.schema.ensureDatabase();
await orm.schema.execute(schema);
const driver = orm.config.getDriver();
if (!await driver.getPlatform().getSchemaHelper()?.databaseExists(driver.getConnection(), schemaName)) {
await orm.schema.createSchema({ schema: schemaName });
await orm.schema.execute(schema);
}
await orm.close(true);
});

Expand All @@ -72,18 +75,6 @@ afterEach(async () => {
await orm.close(true);
});

afterAll(async () => {
orm = await MikroORM.init({
dbName: schemaName,
port: 3308,
discovery: { warnWhenNoEntities: false },
extensions: [EntityGenerator],
multipleStatements: true,
});
await orm.schema.dropDatabase();
await orm.close(true);
});

describe(schemaName, () => {
describe.each(['never', 'always', 'smart'])('scalarPropertiesForRelations=%s', i => {
const scalarPropertiesForRelations = i as NonNullable<MikroORMOptions['entityGenerator']['scalarPropertiesForRelations']>;
Expand Down
Loading

0 comments on commit d0e3ac9

Please sign in to comment.