feat(core): rework serialization rules to always respect populate hint

Closes #4138

packages/core/src/EntityManager.ts

@@ -296,14 +296,14 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
     Fields extends string = '*',
   >(entityName: string, where: FilterQuery<Entity>, options: FindOptions<Entity, Hint, Fields> | FindOneOptions<Entity, Hint, Fields>, type: 'read' | 'update' | 'delete'): Promise<FilterQuery<Entity>> {
     where = QueryHelper.processWhere({
-      where: where as FilterQuery<Entity>,
+      where,
       entityName,
       metadata: this.metadata,
       platform: this.driver.getPlatform(),
       convertCustomTypes: options.convertCustomTypes,
       aliased: type === 'read',
     });
-    where = await this.applyFilters(entityName, where, options.filters ?? {}, type);
+    where = (await this.applyFilters(entityName, where, options.filters ?? {}, type))!;
     where = await this.applyDiscriminatorCondition(entityName, where);
 
     return where;
@@ -334,7 +334,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
   /**
    * @internal
    */
-  async applyFilters<Entity extends object>(entityName: string, where: FilterQuery<Entity>, options: Dictionary<boolean | Dictionary> | string[] | boolean, type: 'read' | 'update' | 'delete'): Promise<FilterQuery<Entity>> {
+  async applyFilters<Entity extends object>(entityName: string, where: FilterQuery<Entity> | undefined, options: Dictionary<boolean | Dictionary> | string[] | boolean, type: 'read' | 'update' | 'delete'): Promise<FilterQuery<Entity> | undefined> {
     const meta = this.metadata.find<Entity>(entityName);
     const filters: FilterDef[] = [];
     const ret: Dictionary[] = [];

packages/core/src/drivers/DatabaseDriver.ts

@@ -91,7 +91,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 object, O extends object>(prop: EntityProperty, owners: Primary<O>[][], where?: FilterQuery<any>, orderBy?: QueryOrderMap<T>[], ctx?: Transaction, options?: FindOptions<T, any>): Promise<Dictionary<T[]>> {
+  async loadFromPivotTable<T extends object, O extends object>(prop: EntityProperty, owners: Primary<O>[][], where?: FilterQuery<any>, orderBy?: OrderDefinition<T>, ctx?: Transaction, options?: FindOptions<T, any>): Promise<Dictionary<T[]>> {
     throw new Error(`${this.constructor.name} does not use pivot tables`);
   }
 
@@ -313,9 +313,9 @@ export abstract class DatabaseDriver<C extends Connection> implements IDatabaseD
     });
   }
 
-  protected getPivotOrderBy<T>(prop: EntityProperty<T>, orderBy?: QueryOrderMap<T>[]): QueryOrderMap<T>[] {
+  protected getPivotOrderBy<T>(prop: EntityProperty<T>, orderBy?: OrderDefinition<T>): QueryOrderMap<T>[] {
     if (!Utils.isEmpty(orderBy)) {
-      return orderBy!;
+      return orderBy as QueryOrderMap<T>[];
     }
 
     if (!Utils.isEmpty(prop.orderBy)) {

packages/core/src/drivers/IDatabaseDriver.ts

@@ -63,7 +63,7 @@ export interface IDatabaseDriver<C extends Connection = Connection> {
   /**
    * When driver uses pivot tables for M:N, this method will load identifiers for given collections from them
    */
-  loadFromPivotTable<T extends object, O extends object>(prop: EntityProperty, owners: Primary<O>[][], where?: FilterQuery<T>, orderBy?: QueryOrderMap<T>[], ctx?: Transaction, options?: FindOptions<T, any>): Promise<Dictionary<T[]>>;
+  loadFromPivotTable<T extends object, O extends object>(prop: EntityProperty, owners: Primary<O>[][], where?: FilterQuery<T>, orderBy?: OrderDefinition<T>, ctx?: Transaction, options?: FindOptions<T, any>): Promise<Dictionary<T[]>>;
 
   getPlatform(): Platform;
 

packages/core/src/entity/Collection.ts

@@ -19,6 +19,7 @@ import { Reference } from './Reference';
 import type { Transaction } from '../connections/Connection';
 import type { FindOptions } from '../drivers/IDatabaseDriver';
 import { helper } from './wrap';
+import type { EntityManager } from '../EntityManager';
 
 export interface MatchingOptions<T extends object, P extends string = never> extends FindOptions<T, P> {
   where?: FilterQuery<T>;
@@ -30,8 +31,7 @@ export class Collection<T extends object, O extends object = object> extends Arr
 
   private snapshot: T[] | undefined = []; // used to create a diff of the collection at commit time, undefined marks overridden values so we need to wipe when flushing
   private readonly?: boolean;
-  private _populated = false;
-  private _lazyInitialized = false;
+  private _populated?: boolean;
   private _em?: unknown;
 
   constructor(owner: O, items?: T[], initialized = true) {
@@ -77,17 +77,25 @@ export class Collection<T extends object, O extends object = object> extends Arr
 
     const em = this.getEntityManager();
     const pivotMeta = em.getMetadata().find(this.property.pivotEntity)!;
+    const where = this.createLoadCountCondition(options.where ?? {} as FilterQuery<T>, pivotMeta);
 
     if (!em.getPlatform().usesPivotTable() && this.property.kind === ReferenceKind.MANY_TO_MANY) {
       return this._count = this.length;
-    } else if (this.property.pivotTable && !(this.property.inversedBy || this.property.mappedBy)) {
-      const count = await em.count(this.property.type, this.createLoadCountCondition(options.where ?? {} as FilterQuery<T>, pivotMeta), { populate: [{ field: this.property.pivotEntity }] });
+    }
+
+    if (this.property.pivotTable && !(this.property.inversedBy || this.property.mappedBy)) {
+      const count = await em.count(this.property.type, where, {
+        populate: [{ field: this.property.pivotEntity } as any],
+      });
+
       if (!options.where) {
         this._count = count;
       }
+
       return count;
     }
-    const count = await em.count(this.property.type, this.createLoadCountCondition(options.where ?? {} as FilterQuery<T>, pivotMeta));
+    const count = await em.count(this.property.type, where);
+
     if (!options.where) {
       this._count = count;
     }
@@ -103,9 +111,9 @@ export class Collection<T extends object, O extends object = object> extends Arr
     if (this.property.kind === ReferenceKind.MANY_TO_MANY && em.getPlatform().usesPivotTable()) {
       const cond = await em.applyFilters(this.property.type, where, options.filters ?? {}, 'read');
       const map = await em.getDriver().loadFromPivotTable(this.property, [helper(this.owner).__primaryKeys], cond, opts.orderBy, ctx, options);
-      items = map[helper(this.owner).getSerializedPrimaryKey()].map((item: EntityData<T>) => em.merge(this.property.type, item, { convertCustomTypes: true }));
+      items = map[helper(this.owner).getSerializedPrimaryKey()].map((item: EntityData<TT>) => em.merge(this.property.type, item, { convertCustomTypes: true })) as any;
     } else {
-      items = await em.find(this.property.type, this.createCondition(where), opts);
+      items = await em.find(this.property.type, this.createCondition(where), opts) as any;
     }
 
     if (options.store) {
@@ -209,13 +217,20 @@ export class Collection<T extends object, O extends object = object> extends Arr
     return super.count();
   }
 
-  shouldPopulate(): boolean {
-    return this._populated && !this._lazyInitialized;
+  shouldPopulate(populated?: boolean): boolean {
+    if (!this.isInitialized(true)) {
+      return false;
+    }
+
+    if (this._populated != null) {
+      return this._populated;
+    }
+
+    return !!populated;
   }
 
-  populated(populated = true): void {
+  populated(populated: boolean | undefined = true): void {
     this._populated = populated;
-    this._lazyInitialized = false;
   }
 
   async init<TT extends T, P extends string = never>(options: InitOptions<TT, P> = {}): Promise<LoadedCollection<Loaded<TT, P>>> {
@@ -234,7 +249,6 @@ export class Collection<T extends object, O extends object = object> extends Arr
       const cond = await em.applyFilters(this.property.type, options.where, {}, 'read');
       const map = await em.getDriver().loadFromPivotTable(this.property, [helper(this.owner).__primaryKeys], cond, options.orderBy, undefined, options);
       this.hydrate(map[helper(this.owner).getSerializedPrimaryKey()].map((item: EntityData<T>) => em.merge(this.property.type, item, { convertCustomTypes: true })), true);
-      this._lazyInitialized = true;
 
       return this as unknown as LoadedCollection<Loaded<TT, P>>;
     }
@@ -243,18 +257,17 @@ export class Collection<T extends object, O extends object = object> extends Arr
     if (this.property.kind === ReferenceKind.MANY_TO_MANY && (this.property.owner || em.getPlatform().usesPivotTable()) && this.length === 0) {
       this.initialized = true;
       this.dirty = false;
-      this._lazyInitialized = true;
 
       return this as unknown as LoadedCollection<Loaded<TT, P>>;
     }
 
-    const where = this.createCondition(options.where as FilterQuery<T>);
+    const where = this.createCondition(options.where);
     const order = [...this.items]; // copy order of references
     const customOrder = !!options.orderBy;
-    const items: T[] = await em.find(this.property.type, where, {
+    const items: TT[] = await em.find(this.property.type, where, {
       populate: options.populate,
       lockMode: options.lockMode,
-      orderBy: this.createOrderBy(options.orderBy as QueryOrderMap<T>),
+      orderBy: this.createOrderBy(options.orderBy as QueryOrderMap<TT>),
       connectionType: options.connectionType,
       schema: this.property.targetMeta!.schema === '*'
         ? helper(this.owner).__schema
@@ -274,7 +287,6 @@ export class Collection<T extends object, O extends object = object> extends Arr
 
     this.initialized = true;
     this.dirty = false;
-    this._lazyInitialized = true;
 
     return this as unknown as LoadedCollection<Loaded<TT, P>>;
   }
@@ -320,31 +332,31 @@ export class Collection<T extends object, O extends object = object> extends Arr
       throw ValidationError.entityNotManaged(this.owner);
     }
 
-    return em;
+    return em as EntityManager;
   }
 
-  private createCondition(cond: FilterQuery<T> = {}): FilterQuery<T> {
+  private createCondition<TT extends T>(cond: FilterQuery<TT> = {}): FilterQuery<TT> {
     if (this.property.kind === ReferenceKind.ONE_TO_MANY) {
-      cond[this.property.mappedBy as FilterKey<T>] = helper(this.owner).getPrimaryKey() as any;
+      cond[this.property.mappedBy as FilterKey<TT>] = helper(this.owner).getPrimaryKey() as any;
     } else { // MANY_TO_MANY
       this.createManyToManyCondition(cond);
     }
 
     return cond;
   }
 
-  private createOrderBy(orderBy: QueryOrderMap<T> | QueryOrderMap<T>[] = []): QueryOrderMap<T>[] {
+  private createOrderBy<TT extends T>(orderBy: QueryOrderMap<TT> | QueryOrderMap<TT>[] = []): QueryOrderMap<TT>[] {
     if (Utils.isEmpty(orderBy) && this.property.kind === ReferenceKind.ONE_TO_MANY) {
       const defaultOrder = this.property.referencedColumnNames.map(name => {
         return { [name]: QueryOrder.ASC };
       });
-      orderBy = this.property.orderBy as QueryOrderMap<T> || defaultOrder;
+      orderBy = this.property.orderBy as QueryOrderMap<TT> || defaultOrder;
     }
 
     return Utils.asArray(orderBy);
   }
 
-  private createManyToManyCondition(cond: FilterQuery<T>) {
+  private createManyToManyCondition<TT extends T>(cond: FilterQuery<TT>) {
     const dict = cond as Dictionary;
 
     if (this.property.owner || this.property.pivotTable) {

packages/core/src/entity/EntityLoader.ts

@@ -193,17 +193,6 @@ export class EntityLoader {
       return [];
     }
 
-    // set populate flag
-    entities.forEach(entity => {
-      const value = entity[field];
-
-      if (Utils.isEntity(value, true)) {
-        (value as AnyEntity).__helper!.populated();
-      } else if (Utils.isCollection(value)) {
-        (value as Collection<any>).populated();
-      }
-    });
-
     const filtered = this.filterCollections<T>(entities, field, options.refresh);
     const innerOrderBy = Utils.asArray(options.orderBy)
       .filter(orderBy => Utils.isObject(orderBy[prop.name]))

packages/core/src/entity/WrappedEntity.ts

@@ -20,7 +20,6 @@ export class WrappedEntity<Entity extends object> {
   __initialized = true;
   __touched = false;
   __populated?: boolean;
-  __lazyInitialized?: boolean;
   __managed?: boolean;
   __onLoadFired?: boolean;
   __schema?: string;
@@ -56,9 +55,8 @@ export class WrappedEntity<Entity extends object> {
     return this.__touched;
   }
 
-  populated(populated = true): void {
+  populated(populated: boolean | undefined = true): void {
     this.__populated = populated;
-    this.__lazyInitialized = false;
   }
 
   toReference(): Ref<Entity> & LoadedReference<Loaded<Entity, AddEager<Entity>>> {
@@ -93,8 +91,6 @@ export class WrappedEntity<Entity extends object> {
     }
 
     await this.__em.findOne(this.entity.constructor.name, this.entity, { refresh: true, lockMode, populate, connectionType, schema: this.__schema });
-    this.populated(populated);
-    this.__lazyInitialized = true;
 
     return this.entity;
   }

packages/core/src/serialization/EntityTransformer.ts

@@ -55,13 +55,14 @@ export class EntityTransformer {
     [...keys]
       .filter(prop => raw ? meta.properties[prop] : isVisible<Entity>(meta, prop, ignoreFields))
       .map(prop => {
+        const populated = root.isMarkedAsPopulated(meta.className, prop);
         const cycle = root.visit(meta.className, prop);
 
         if (cycle && visited) {
           return [prop, undefined];
         }
 
-        const val = EntityTransformer.processProperty<Entity>(prop, entity, raw);
+        const val = EntityTransformer.processProperty<Entity>(prop, entity, raw, populated);
 
         if (!cycle) {
           root.leave(meta.className, prop);
@@ -109,7 +110,7 @@ export class EntityTransformer {
     return prop;
   }
 
-  private static processProperty<Entity extends object>(prop: EntityKey<Entity>, entity: Entity, raw: boolean): EntityValue<Entity> | undefined {
+  private static processProperty<Entity extends object>(prop: EntityKey<Entity>, entity: Entity, raw: boolean, populated: boolean): EntityValue<Entity> | undefined {
     const wrapped = helper(entity);
     const property = wrapped.__meta.properties[prop];
     const serializer = property?.serializer;
@@ -119,11 +120,11 @@ export class EntityTransformer {
     }
 
     if (Utils.isCollection(entity[prop])) {
-      return EntityTransformer.processCollection(prop, entity, raw);
+      return EntityTransformer.processCollection(prop, entity, raw, populated);
     }
 
     if (Utils.isEntity(entity[prop], true)) {
-      return EntityTransformer.processEntity(prop, entity, wrapped.__platform, raw);
+      return EntityTransformer.processEntity(prop, entity, wrapped.__platform, raw, populated);
     }
 
     if (property.kind === ReferenceKind.EMBEDDED) {
@@ -147,30 +148,42 @@ export class EntityTransformer {
     return wrapped.__platform.normalizePrimaryKey(entity[prop] as unknown as IPrimaryKey) as unknown as EntityValue<Entity>;
   }
 
-  private static processEntity<Entity extends object>(prop: keyof Entity, entity: Entity, platform: Platform, raw: boolean): EntityValue<Entity> | undefined {
+  private static processEntity<Entity extends object>(prop: keyof Entity, entity: Entity, platform: Platform, raw: boolean, populated: boolean): EntityValue<Entity> | undefined {
     const child = entity[prop] as unknown as Entity | Reference<Entity>;
     const wrapped = helper(child);
 
     if (raw && wrapped.isInitialized() && child !== entity) {
       return wrapped.toPOJO() as unknown as EntityValue<Entity>;
     }
 
-    if (wrapped.isInitialized() && (wrapped.__populated || !wrapped.__managed) && child !== entity && !wrapped.__lazyInitialized) {
+    function isPopulated() {
+      if (wrapped.__populated != null) {
+        return wrapped.__populated;
+      }
+
+      if (populated) {
+        return true;
+      }
+
+      return !wrapped.__managed;
+    }
+
+    if (wrapped.isInitialized() && isPopulated() && child !== entity) {
       const args = [...wrapped.__meta.toJsonParams.map(() => undefined)];
       return wrap(child).toJSON(...args) as EntityValue<Entity>;
     }
 
     return platform.normalizePrimaryKey(wrapped.getPrimaryKey() as IPrimaryKey) as unknown as EntityValue<Entity>;
   }
 
-  private static processCollection<Entity>(prop: keyof Entity, entity: Entity, raw: boolean): EntityValue<Entity> | undefined {
+  private static processCollection<Entity>(prop: keyof Entity, entity: Entity, raw: boolean, populated: boolean): EntityValue<Entity> | undefined {
     const col = entity[prop] as Collection<AnyEntity>;
 
     if (raw && col.isInitialized(true)) {
       return col.getItems().map(item => wrap(item).toPOJO()) as EntityValue<Entity>;
     }
 
-    if (col.isInitialized(true) && col.shouldPopulate()) {
+    if (col.shouldPopulate(populated)) {
       return col.toArray() as EntityValue<Entity>;
     }
 

packages/core/src/serialization/SerializationContext.ts

@@ -16,14 +16,17 @@ export class SerializationContext<T> {
 
   constructor(private readonly populate: PopulateOptions<T>[] = []) {}
 
+  /**
+   * Returns true when there is a cycle detected.
+   */
   visit(entityName: string, prop: string): boolean {
     if (!this.path.find(([cls, item]) => entityName === cls && prop === item)) {
       this.path.push([entityName, prop]);
       return false;
     }
 
     // check if the path is explicitly populated
-    if (!this.isMarkedAsPopulated(prop)) {
+    if (!this.isMarkedAsPopulated(entityName, prop)) {
       return true;
     }
 
@@ -69,7 +72,7 @@ export class SerializationContext<T> {
       .forEach(item => this.propagate(root, item, isVisible));
   }
 
-  private isMarkedAsPopulated(prop: string): boolean {
+  isMarkedAsPopulated(entityName: string, prop: string): boolean {
     let populate: PopulateOptions<T>[] | undefined = this.populate;
 
     for (const segment of this.path) {
@@ -80,6 +83,11 @@ export class SerializationContext<T> {
       const exists = populate.find(p => p.field === segment[1]) as PopulateOptions<T>;
 
       if (exists) {
+        // we need to check for cycles here too, as we could fall into endless loops for bidirectional relations
+        if (exists.all) {
+          return !this.path.find(([cls, item]) => entityName === cls && prop === item);
+        }
+
         populate = exists.children as PopulateOptions<T>[];
       }
     }

packages/core/src/typings.ts

@@ -156,7 +156,6 @@ export interface IWrappedEntityInternal<
   __populated: boolean;
   __onLoadFired: boolean;
   __reference?: Ref<T>;
-  __lazyInitialized: boolean;
   __pk?: Primary<T>;
   __primaryKeys: Primary<T>[];
   __serializationContext: { root?: SerializationContext<T>; populate?: PopulateOptions<T>[] };