feat(core): use composite PK in many to many relations (#204)

Previously it was required to have autoincrement primary key for m:n pivot tables. Now this
has changed. By default, only foreign columns are required and composite key over both of them
is used as primary key.

BREAKING CHANGE:
M:N collection items no longer have fixed order by default. To preserve stable order of collections,
you can force previous behaviour by defining the m:n property as `fixedOrder: true`, which will start
ordering by `id` column. You can also override the order column name via `fixedOrderColumn: 'order'`.

You can also specify default ordering via `orderBy: { ... }` attribute.

Closes #121

docs/collections.md

@@ -127,6 +127,20 @@ books = new Collection<Book>(this);
 books = new Collection<Book>(this);
 ```
 
+### Forcing fixed order of collection items
+
+> Since v3 many to many collections does not require having auto increment primary key, that 
+> was used to ensure fixed order of collection items.
+
+To preserve fixed order of collections, you can use `fixedOrder: true` attribute, which will 
+start ordering by `id` column. Schema generator will convert the pivot table to have auto increment
+primary key `id`. You can also change the order column name via `fixedOrderColumn: 'order'`. 
+
+You can also specify default ordering via `orderBy: { ... }` attribute. This will be used when
+you fully populate the collection including its items, as it orders by the referenced entity 
+properties instead of pivot table columns (which `fixedOrderColumn` is). On the other hand, 
+`fixedOrder` is used to maintain the insert order of items instead of ordering by some property. 
+
 ## Propagation of Collection's add() and remove() operations
 
 When you use one of `Collection.add()` method, the item is added to given collection, 

docs/query-builder.md

@@ -234,6 +234,7 @@ console.log(qb.getQuery()); // for MySQL
 
 ```typescript
 qb.select(fields: string | string[], distinct?: boolean): QueryBuilder;
+qb.addSelect(fields: string | string[]): QueryBuilder;
 qb.insert(data: Record<string, any>): QueryBuilder;
 qb.update(data: Record<string, any>): QueryBuilder;
 qb.delete(cond: Record<string, any>): QueryBuilder;

docs/upgrading-v2-to-v3.md

@@ -71,11 +71,17 @@ In postgres driver, it used to be required to pass parameters as indexed dollar
 ($1, $2, ...), while now knex requires the placeholder to be simple question mark (`?`), 
 like in other dialects, so this is now unified with other drivers.
 
-## SchemaGenerator.generate() is now async
+## ManyToMany now uses composite primary key
 
-If you used `SchemaGenerator`, now there is CLI tool you can use instead. Learn more 
-in [SchemaGenerator docs](schema-generator.md). To setup CLI, take a look at 
-[installation section](installation.md).
+Previously it was required to have autoincrement primary key for m:n pivot tables. Now this 
+has changed. By default, only foreign columns are required and composite key over both of them
+is used as primary key.
+
+To preserve stable order of collections, you can force previous behaviour by defining the 
+m:n property as `fixedOrder: true`, which will start ordering by `id` column. You can also 
+override the order column name via `fixedOrderColumn: 'order'`. 
+
+You can also specify default ordering via `orderBy: { ... }` attribute.
 
 ## Strict FilterQuery and smart query conditions
 
@@ -90,14 +96,20 @@ option. `true`/`false` will enable/disable all namespaces.
 
 Available logger namespaces: `'query' | 'query-params' | 'discovery' | 'info'`.
 
+## Removed deprecated fk option from 1:m and m:1 decorators 
+
+Use `mappedBy`/`inversedBy` instead.
+
+## SchemaGenerator.generate() is now async
+
+If you used `SchemaGenerator`, now there is CLI tool you can use instead. Learn more 
+in [SchemaGenerator docs](schema-generator.md). To setup CLI, take a look at 
+[installation section](installation.md).
+
 ## New method on NamingStrategy interface
 
 `getClassName()` is used to find entity class name based on its file name. Now users can 
 override the default implementation to accommodate their specific needs.
 
 If you used custom naming strategy, you will either need to implement this method yourself, 
 or extend `AbstractNamingStrategy`.
-
-## Removed deprecated fk option from 1:m and m:1 decorators 
-
-Use `mappedBy`/`inversedBy` instead.

lib/EntityManager.ts

@@ -79,7 +79,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
 
     await this.entityLoader.populate(entityName, ret, options.populate || [], where, options.orderBy || {});
 
-    return ret;
+    return Utils.unique(ret);
   }
 
   async findAndCount<T extends AnyEntity<T>>(entityName: EntityName<T>, where: FilterQuery<T>, options?: FindOptions): Promise<[T[], number]>;

lib/decorators/ManyToMany.ts

@@ -12,6 +12,11 @@ export function ManyToMany<T extends AnyEntity<T>>(
 ) {
   return function (target: AnyEntity, propertyName: string) {
     options = Utils.isObject<ManyToManyOptions<T>>(entity) ? entity : { ...options, entity, mappedBy };
+    options.fixedOrder = options.fixedOrder || !!options.fixedOrderColumn;
+
+    if (!options.owner && !options.inversedBy && !options.mappedBy) {
+      options.owner = true;
+    }
 
     if (options.owner) {
       Utils.renameKey(options, 'mappedBy', 'inversedBy');
@@ -24,10 +29,6 @@ export function ManyToMany<T extends AnyEntity<T>>(
       throw new Error(`'@ManyToMany({ entity: string | Function })' is required in '${target.constructor.name}.${propertyName}'`);
     }
 
-    if (!options.owner && !options.inversedBy && !options.mappedBy) {
-      options.owner = true;
-    }
-
     const property = { name: propertyName, reference: ReferenceType.MANY_TO_MANY, owner: !!options.inversedBy, cascade: [Cascade.PERSIST, Cascade.MERGE] } as EntityProperty<T>;
     meta.properties[propertyName] = Object.assign(property, options);
   };
@@ -39,6 +40,8 @@ export interface ManyToManyOptions<T extends AnyEntity<T>> extends ReferenceOpti
   inversedBy?: (string & keyof T) | ((e: T) => any);
   mappedBy?: (string & keyof T) | ((e: T) => any);
   orderBy?: { [field: string]: QueryOrder };
+  fixedOrder?: boolean;
+  fixedOrderColumn?: string;
   pivotTable?: string;
   joinColumn?: string;
   inverseJoinColumn?: string;

lib/decorators/ManyToOne.ts

@@ -28,4 +28,5 @@ export interface ManyToOneOptions<T extends AnyEntity<T>> extends ReferenceOptio
   entity?: string | (() => EntityName<T>);
   inversedBy?: (string & keyof T) | ((e: T) => any);
   wrappedReference?: boolean;
+  primary?: boolean;
 }

lib/decorators/OneToOne.ts

@@ -15,4 +15,5 @@ export interface OneToOneOptions<T extends AnyEntity<T>> extends Partial<Omit<On
   owner?: boolean;
   inversedBy?: (string & keyof T) | ((e: T) => any);
   wrappedReference?: boolean;
+  primary?: boolean;
 }

lib/drivers/DatabaseDriver.ts

@@ -1,5 +1,5 @@
 import { IDatabaseDriver } from './IDatabaseDriver';
-import { EntityData, EntityMetadata, EntityProperty, FilterQuery, AnyEntity, Primary } from '../types';
+import { EntityData, EntityMetadata, EntityProperty, FilterQuery, AnyEntity, Primary, Dictionary } from '../types';
 import { MetadataStorage } from '../metadata';
 import { Connection, QueryResult, Transaction } from '../connections';
 import { Configuration, ConnectionOptions, Utils } from '../utils';
@@ -34,7 +34,7 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD
     throw new Error(`Aggregations are not supported by ${this.constructor.name} driver`);
   }
 
-  async loadFromPivotTable<T extends AnyEntity<T>>(prop: EntityProperty, owners: Primary<T>[], where?: FilterQuery<T>, orderBy?: QueryOrderMap, ctx?: Transaction): Promise<Record<string, T[]>> {
+  async loadFromPivotTable<T extends AnyEntity<T>>(prop: EntityProperty, owners: Primary<T>[], where?: FilterQuery<T>, orderBy?: QueryOrderMap, ctx?: Transaction): Promise<Dictionary<T[]>> {
     if (!this.platform.usesPivotTable()) {
       throw new Error(`${this.constructor.name} does not use pivot tables`);
     }
@@ -43,10 +43,10 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD
     const fk2 = prop.inverseJoinColumn;
     const pivotProp2 = this.getPivotInverseProperty(prop);
     where = { ...where, [`${prop.pivotTable}.${pivotProp2.name}`]: { $in: owners } };
-    orderBy = orderBy || prop.orderBy || { [`${(prop.pivotTable)}.${this.metadata.get(prop.pivotTable).primaryKey}`]: QueryOrder.ASC };
+    orderBy = this.getPivotOrderBy(prop, orderBy);
     const items = owners.length ? await this.find(prop.type, where, [prop.pivotTable], orderBy, undefined, undefined, undefined, ctx) : [];
 
-    const map: Record<string, T[]> = {};
+    const map: Dictionary<T[]> = {};
     owners.forEach(owner => map['' + owner] = []);
     items.forEach((item: any) => {
       map[item[fk1]].push(item);
@@ -112,6 +112,22 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD
     return this.dependencies;
   }
 
+  protected getPivotOrderBy(prop: EntityProperty, orderBy?: QueryOrderMap): QueryOrderMap {
+    if (orderBy) {
+      return orderBy;
+    }
+
+    if (prop.orderBy) {
+      return prop.orderBy;
+    }
+
+    if (prop.fixedOrder) {
+      return { [`${prop.pivotTable}.${prop.fixedOrderColumn}`]: QueryOrder.ASC };
+    }
+
+    return {};
+  }
+
   protected getPrimaryKeyField(entityName: string): string {
     const meta = this.metadata.get(entityName, false, false);
     return meta ? meta.primaryKey : this.config.getNamingStrategy().referenceColumnName();

lib/entity/EntityLoader.ts

@@ -130,7 +130,6 @@ export class EntityLoader {
   }
 
   private async findChildrenFromPivotTable<T extends AnyEntity<T>>(filtered: T[], prop: EntityProperty, field: keyof T, where?: FilterQuery<T>, orderBy?: QueryOrderMap): Promise<AnyEntity[]> {
-    orderBy = orderBy || prop.orderBy;
     const map = await this.driver.loadFromPivotTable(prop, filtered.map(e => wrap(e).__primaryKey) as Primary<T>[], where, orderBy, this.em.getTransactionContext());
     const children: AnyEntity[] = [];
 

lib/metadata/MetadataDiscovery.ts

@@ -175,14 +175,21 @@ export class MetadataDiscovery {
 
   private initManyToManyFields(meta: EntityMetadata, prop: EntityProperty): void {
     const meta2 = this.metadata.get(prop.type);
+    Utils.defaultValue(prop, 'fixedOrder', !!prop.fixedOrderColumn);
+
+    if (!prop.owner && !prop.inversedBy && !prop.mappedBy) {
+      prop.owner = true;
+    }
 
     if (!prop.pivotTable && prop.owner) {
       prop.pivotTable = this.namingStrategy.joinTableName(meta.collection, meta2.collection, prop.name);
     }
 
-    if (prop.owner && prop.inversedBy) {
-      const prop2 = meta2.properties[prop.inversedBy];
-      prop2.pivotTable = prop.pivotTable;
+    if (prop.mappedBy) {
+      const prop2 = meta2.properties[prop.mappedBy];
+      prop.pivotTable = prop2.pivotTable;
+      prop.fixedOrder = prop2.fixedOrder;
+      prop.fixedOrderColumn = prop2.fixedOrderColumn || this.namingStrategy.referenceColumnName();
     }
 
     if (!prop.referenceColumnName) {
@@ -214,6 +221,10 @@ export class MetadataDiscovery {
   }
 
   private processEntity(meta: EntityMetadata): EntityMetadata[] {
+    const pks = Object.values(meta.properties).filter(prop => prop.primary);
+    meta.primaryKeys = pks.map(prop => prop.name);
+    meta.compositePK = pks.length > 1;
+
     this.validator.validateEntityDefinition(this.metadata, meta.name);
 
     Object.values(meta.properties).forEach(prop => {
@@ -256,17 +267,31 @@ export class MetadataDiscovery {
       this.initColumnType(primaryProp);
       this.initUnsigned(primaryProp);
 
-      return this.metadata.set(prop.pivotTable, {
+      const data = {
         name: prop.pivotTable,
         collection: prop.pivotTable,
         pivotTable: true,
-        primaryKey: pk,
-        properties: {
-          [pk]: primaryProp,
-          [meta.name]: this.definePivotProperty(prop, meta.name, prop.type),
-          [prop.type]: this.definePivotProperty(prop, prop.type, meta.name),
-        },
-      } as EntityMetadata);
+        properties: {} as Record<string, EntityProperty>,
+      } as EntityMetadata;
+
+      if (prop.fixedOrder) {
+        const pk = prop.fixedOrderColumn || this.namingStrategy.referenceColumnName();
+        const primaryProp = { name: pk, type: 'number', reference: ReferenceType.SCALAR, primary: true, unsigned: true } as EntityProperty;
+        this.initFieldName(primaryProp);
+        this.initColumnType(primaryProp);
+        this.initUnsigned(primaryProp);
+        data.properties[pk] = primaryProp;
+        prop.fixedOrderColumn = pk;
+        data.primaryKey = pk;
+      } else {
+        data.primaryKeys = [meta.name, prop.type];
+        data.compositePK = true;
+      }
+
+      data.properties[meta.name] = this.definePivotProperty(prop, meta.name, prop.type);
+      data.properties[prop.type] = this.definePivotProperty(prop, prop.type, meta.name);
+
+      return this.metadata.set(prop.pivotTable, data);
     }
   }
 

lib/metadata/MetadataValidator.ts

@@ -60,11 +60,6 @@ export class MetadataValidator {
   }
 
   private validateBidirectional(meta: EntityMetadata, prop: EntityProperty, metadata: MetadataStorage): void {
-    // 1:1 reference either is owner or has `mappedBy`
-    if (!prop.owner && !prop.mappedBy && !prop.inversedBy) {
-      throw ValidationError.fromMissingOwnership(meta, prop);
-    }
-
     if (prop.inversedBy) {
       const inverse = metadata.get(prop.type).properties[prop.inversedBy];
       this.validateOwningSide(meta, prop, inverse);

lib/query/CriteriaNode.ts

@@ -116,7 +116,7 @@ export class ScalarCriteriaNode extends CriteriaNode {
       }
 
       if (this.prop!.reference === ReferenceType.ONE_TO_ONE) {
-        qb._fields!.push(field);
+        qb.addSelect(field);
       }
     }
 

lib/query/QueryBuilder.ts

@@ -56,6 +56,10 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
     return this.init(QueryType.SELECT);
   }
 
+  addSelect(fields: string | string[]): this {
+    return this.select([...Utils.asArray(this._fields), ...Utils.asArray(fields)]);
+  }
+
   insert(data: any): this {
     return this.init(QueryType.INSERT, data);
   }

lib/query/QueryBuilderHelper.ts

@@ -350,10 +350,10 @@ export class QueryBuilderHelper {
   }
 
   finalize(type: QueryType, qb: KnexQueryBuilder, meta?: EntityMetadata): void {
-    const useReturningStatement = type === QueryType.INSERT && this.platform.usesReturningStatement();
+    const useReturningStatement = type === QueryType.INSERT && this.platform.usesReturningStatement() && meta && !meta.compositePK;
 
-    if (useReturningStatement && meta) {
-      const returningProps = Object.values(meta.properties).filter(prop => prop.primary || prop.default);
+    if (useReturningStatement) {
+      const returningProps = Object.values(meta!.properties).filter(prop => prop.primary || prop.default);
       qb.returning(returningProps.map(prop => prop.fieldName));
     }
   }

lib/schema/DatabaseTable.ts

@@ -21,7 +21,7 @@ export class DatabaseTable {
     this.columns = cols.reduce((o, v) => {
       const index = indexes[v.name] || [];
       v.primary = pks.includes(v.name);
-      v.unique = index.some(i => i.unique);
+      v.unique = index.some(i => i.unique && !i.primary);
       v.fk = fks[v.name];
       v.indexes = index.filter(i => !i.primary);
       o[v.name] = v;

lib/schema/EntityGenerator.ts

@@ -157,13 +157,13 @@ export class EntityGenerator {
     if (!(cascade.length === 2 && cascade.includes('Cascade.PERSIST') && cascade.includes('Cascade.MERGE'))) {
       options.cascade = `[${cascade.sort().join(', ')}]`;
     }
-  }
 
-  private getDecoratorType(column: Column): string {
     if (column.primary) {
-      return '@PrimaryKey';
+      options.primary = true;
     }
+  }
 
+  private getDecoratorType(column: Column): string {
     if (column.fk && column.unique) {
       return '@OneToOne';
     }
@@ -172,6 +172,10 @@ export class EntityGenerator {
       return '@ManyToOne';
     }
 
+    if (column.primary) {
+      return '@PrimaryKey';
+    }
+
     return '@Property';
   }