fix: better support of relation-based properties in where clauses (#7805

)

docs/find-options.md

@@ -160,7 +160,10 @@ userRepository.find({
     relations: ["profile", "photos", "videos"],
     where: {
         firstName: "Timber",
-        lastName: "Saw"
+        lastName: "Saw",
+        profile: {
+          userName: "tshaw"
+        }
     },
     order: {
         name: "ASC",

src/find-options/FindOptionsUtils.ts

@@ -99,47 +99,19 @@ export class FindOptionsUtils {
         if (options.select) {
             qb.select([]);
             options.select.forEach(select => {
-                if (!metadata.findColumnWithPropertyPath(String(select)))
+                if (!metadata.hasColumnWithPropertyPath(`${select}`))
                     throw new TypeORMError(`${select} column was not found in the ${metadata.name} entity.`);
 
-                qb.addSelect(qb.alias + "." + select);
-            });
-        }
-
-        if (options.where)
-            qb.where(options.where);
-
-        if ((options as FindManyOptions<T>).skip)
-            qb.skip((options as FindManyOptions<T>).skip!);
+                const columns = metadata.findColumnsWithPropertyPath(`${select}`);
 
-        if ((options as FindManyOptions<T>).take)
-            qb.take((options as FindManyOptions<T>).take!);
-
-        if (options.order)
-            Object.keys(options.order).forEach(key => {
-                const order = ((options as FindOneOptions<T>).order as any)[key as any];
-
-                if (!metadata.findColumnWithPropertyPath(key))
-                    throw new TypeORMError(`${key} column was not found in the ${metadata.name} entity.`);
-
-                switch (order) {
-                    case 1:
-                        qb.addOrderBy(qb.alias + "." + key, "ASC");
-                        break;
-                    case -1:
-                        qb.addOrderBy(qb.alias + "." + key, "DESC");
-                        break;
-                    case "ASC":
-                        qb.addOrderBy(qb.alias + "." + key, "ASC");
-                        break;
-                    case "DESC":
-                        qb.addOrderBy(qb.alias + "." + key, "DESC");
-                        break;
+                for (const column of columns) {
+                    qb.addSelect(qb.alias + "." + column.propertyPath);
                 }
             });
+        }
 
         if (options.relations) {
-            const allRelations = options.relations.map(relation => relation);
+            const allRelations = options.relations;
             this.applyRelationsRecursively(qb, allRelations, qb.expressionMap.mainAlias!.name, qb.expressionMap.mainAlias!.metadata, "");
             // recursive removes found relations from allRelations array
             // if there are relations left in this array it means those relations were not found in the entity structure
@@ -207,6 +179,38 @@ export class FindOptionsUtils {
             qb.loadAllRelationIds(options.loadRelationIds as any);
         }
 
+        if (options.where)
+            qb.where(options.where);
+
+        if ((options as FindManyOptions<T>).skip)
+            qb.skip((options as FindManyOptions<T>).skip!);
+
+        if ((options as FindManyOptions<T>).take)
+            qb.take((options as FindManyOptions<T>).take!);
+
+        if (options.order)
+            Object.keys(options.order).forEach(key => {
+                const order = ((options as FindOneOptions<T>).order as any)[key as any];
+
+                if (!metadata.findColumnWithPropertyPath(key))
+                    throw new Error(`${key} column was not found in the ${metadata.name} entity.`);
+
+                switch (order) {
+                    case 1:
+                        qb.addOrderBy(qb.alias + "." + key, "ASC");
+                        break;
+                    case -1:
+                        qb.addOrderBy(qb.alias + "." + key, "DESC");
+                        break;
+                    case "ASC":
+                        qb.addOrderBy(qb.alias + "." + key, "ASC");
+                        break;
+                    case "DESC":
+                        qb.addOrderBy(qb.alias + "." + key, "DESC");
+                        break;
+                }
+            });
+
         return qb;
     }
 

src/metadata/EntityMetadata.ts

@@ -654,6 +654,14 @@ export class EntityMetadata {
         return this.columns.find(column => column.databaseName === databaseName);
     }
 
+    /**
+     * Checks if there is a column or relationship with a given property path.
+     */
+    hasColumnWithPropertyPath(propertyPath: string): boolean {
+        const hasColumn = this.columns.some(column => column.propertyPath === propertyPath);
+        return hasColumn || this.hasRelationWithPropertyPath(propertyPath);
+    }
+
     /**
      * Finds column with a given property path.
      */
@@ -682,13 +690,20 @@ export class EntityMetadata {
 
         // in the case if column with property path was not found, try to find a relation with such property path
         // if we find relation and it has a single join column then its the column user was seeking
-        const relation = this.relations.find(relation => relation.propertyPath === propertyPath);
+        const relation = this.findRelationWithPropertyPath(propertyPath);
         if (relation && relation.joinColumns)
             return relation.joinColumns;
 
         return [];
     }
 
+    /**
+     * Checks if there is a relation with the given property path.
+     */
+    hasRelationWithPropertyPath(propertyPath: string): boolean {
+        return this.relations.some(relation => relation.propertyPath === propertyPath);
+    }
+
     /**
      * Finds relation with the given property path.
      */
@@ -740,6 +755,8 @@ export class EntityMetadata {
 
     /**
      * Creates a property paths for a given entity.
+     *
+     * @deprecated
      */
     static createPropertyPath(metadata: EntityMetadata, entity: ObjectLiteral, prefix: string = "") {
         const paths: string[] = [];

src/query-builder/QueryBuilder.ts

@@ -853,6 +853,154 @@ export abstract class QueryBuilder<Entity> {
             : whereStrings[0];
     }
 
+    private findColumnsForPropertyPath(propertyPath: string): [ Alias, string[], ColumnMetadata[] ] {
+        // Make a helper to iterate the entity & relations?
+        // Use that to set the correct alias?  Or the other way around?
+
+        // Start with the main alias with our property paths
+        let alias = this.expressionMap.mainAlias;
+        const root: string[] = [];
+        const propertyPathParts = propertyPath.split(".");
+
+        while (propertyPathParts.length > 1) {
+            const part = propertyPathParts[0];
+
+            if (!alias?.hasMetadata) {
+                // If there's no metadata, we're wasting our time
+                // and can't actually look any of this up.
+                break;
+            }
+
+            if (alias.metadata.hasEmbeddedWithPropertyPath(part)) {
+                // If this is an embedded then we should combine the two as part of our lookup.
+                // Instead of just breaking, we keep going with this in case there's an embedded/relation
+                // inside an embedded.
+                propertyPathParts.unshift(
+                    `${propertyPathParts.shift()}.${propertyPathParts.shift()}`
+                );
+                continue;
+            }
+
+            if (alias.metadata.hasRelationWithPropertyPath(part)) {
+                // If this is a relation then we should find the aliases
+                // that match the relation & then continue further down
+                // the property path
+                const joinAttr = this.expressionMap.joinAttributes.find(
+                    (joinAttr) => joinAttr.relationPropertyPath === part
+                );
+
+                if (!joinAttr?.alias) {
+                    const fullRelationPath = root.length > 0 ? `${root.join(".")}.${part}` : part;
+                    throw new Error(`Cannot find alias for relation at ${fullRelationPath}`);
+                }
+
+                alias = joinAttr.alias;
+                root.push(...part.split("."));
+                propertyPathParts.shift();
+                continue;
+            }
+
+            break;
+        }
+
+        if (!alias) {
+            throw new Error(`Cannot find alias for property ${propertyPath}`);
+        }
+
+        // Remaining parts are combined back and used to find the actual property path
+        const aliasPropertyPath = propertyPathParts.join(".");
+
+        const columns = alias.metadata.findColumnsWithPropertyPath(aliasPropertyPath);
+
+        if (!columns.length) {
+            throw new EntityColumnNotFound(propertyPath);
+        }
+
+        return [ alias, root, columns ];
+    }
+
+    /**
+     * Creates a property paths for a given ObjectLiteral.
+     */
+    protected createPropertyPath(metadata: EntityMetadata, entity: ObjectLiteral, prefix: string = "") {
+        const paths: string[] = [];
+
+        for (const key of Object.keys(entity)) {
+            const path = prefix ? `${prefix}.${key}` : key;
+
+            // There's times where we don't actually want to traverse deeper.
+            // If the value is a `FindOperator`, or null, or not an object, then we don't, for example.
+            if (entity[key] === null || typeof entity[key] !== "object" || entity[key] instanceof FindOperator) {
+                paths.push(path);
+                continue;
+            }
+
+            if (metadata.hasEmbeddedWithPropertyPath(path)) {
+                const subPaths = this.createPropertyPath(metadata, entity[key], path);
+                paths.push(...subPaths);
+                continue;
+            }
+
+            if (metadata.hasRelationWithPropertyPath(path)) {
+                const relation = metadata.findRelationWithPropertyPath(path)!;
+
+                // There's also cases where we don't want to return back all of the properties.
+                // These handles the situation where someone passes the model & we don't need to make
+                // a HUGE `where` to uniquely look up the entity.
+
+                // In the case of a *-to-one, there's only ever one possible entity on the other side
+                // so if the join columns are all defined we can return just the relation itself
+                // because it will fetch only the join columns and do the lookup.
+                if (relation.relationType === "one-to-one" || relation.relationType === "many-to-one") {
+                    const joinColumns = relation.joinColumns
+                        .map(j => j.referencedColumn)
+                        .filter((j): j is ColumnMetadata => !!j);
+
+                    const hasAllJoinColumns = joinColumns.length > 0 && joinColumns.every(
+                        column => column.getEntityValue(entity[key], false)
+                    );
+
+                    if (hasAllJoinColumns) {
+                        paths.push(path);
+                        continue;
+                    }
+                }
+
+                if (relation.relationType === "one-to-many" || relation.relationType === "many-to-many") {
+                    throw new Error(`Cannot query across ${relation.relationType} for property ${path}`);
+                }
+
+                // For any other case, if the `entity[key]` contains all of the primary keys we can do a
+                // lookup via these.  We don't need to look up via any other values 'cause these are
+                // the unique primary keys.
+                // This handles the situation where someone passes the model & we don't need to make
+                // a HUGE where.
+                const primaryColumns = relation.inverseEntityMetadata.primaryColumns;
+                const hasAllPrimaryKeys = primaryColumns.length > 0 && primaryColumns.every(
+                    column => column.getEntityValue(entity[key], false)
+                );
+
+                if (hasAllPrimaryKeys) {
+                    const subPaths = primaryColumns.map(
+                        column => `${path}.${column.propertyPath}`
+                    );
+                    paths.push(...subPaths);
+                    continue;
+                }
+
+                // If nothing else, just return every property that's being passed to us.
+                const subPaths = this.createPropertyPath(relation.inverseEntityMetadata, entity[key])
+                    .map(p => `${path}.${p}`);
+                paths.push(...subPaths);
+                continue;
+            }
+
+            paths.push(path);
+        }
+
+        return paths;
+    }
+
     /**
      * Computes given where argument - transforms to a where string all forms it can take.
      */
@@ -884,19 +1032,28 @@ export abstract class QueryBuilder<Entity> {
 
             if (this.expressionMap.mainAlias!.hasMetadata) {
                 andConditions = wheres.map((where, whereIndex) => {
-                    const propertyPaths = EntityMetadata.createPropertyPath(this.expressionMap.mainAlias!.metadata, where);
+                    const propertyPaths = this.createPropertyPath(this.expressionMap.mainAlias!.metadata, where);
 
                     return propertyPaths.map((propertyPath, propertyIndex) => {
-                        const columns = this.expressionMap.mainAlias!.metadata.findColumnsWithPropertyPath(propertyPath);
-
-                        if (!columns.length) {
-                            throw new EntityColumnNotFound(propertyPath);
-                        }
+                        const [ alias, aliasPropertyPath, columns ] = this.findColumnsForPropertyPath(propertyPath);
 
                         return columns.map((column, columnIndex) => {
 
-                            const aliasPath = this.expressionMap.aliasNamePrefixingEnabled ? `${this.alias}.${propertyPath}` : column.propertyPath;
-                            let parameterValue = column.getEntityValue(where, true);
+                            // Use the correct alias & the property path from the column
+                            const aliasPath = this.expressionMap.aliasNamePrefixingEnabled ? `${alias.name}.${column.propertyPath}` : column.propertyPath;
+
+                            let containedWhere = where;
+
+                            for (const part of aliasPropertyPath) {
+                                if (!containedWhere || !(part in containedWhere)) {
+                                    containedWhere = {};
+                                    break;
+                                }
+
+                                containedWhere = containedWhere[part];
+                            }
+
+                            let parameterValue = column.getEntityValue(containedWhere, true);
 
                             if (parameterValue === null) {
                                 return `${aliasPath} IS NULL`;

src/query-builder/UpdateQueryBuilder.ts

@@ -9,7 +9,6 @@ import {SqlServerDriver} from "../driver/sqlserver/SqlServerDriver";
 import {PostgresDriver} from "../driver/postgres/PostgresDriver";
 import {WhereExpression} from "./WhereExpression";
 import {Brackets} from "./Brackets";
-import {EntityMetadata} from "../metadata/EntityMetadata";
 import {UpdateResult} from "./result/UpdateResult";
 import {ReturningStatementNotSupportedError} from "../error/ReturningStatementNotSupportedError";
 import {ReturningResultsEntityUpdator} from "./ReturningResultsEntityUpdator";
@@ -398,7 +397,7 @@ export class UpdateQueryBuilder<Entity> extends QueryBuilder<Entity> implements
         const updateColumnAndValues: string[] = [];
         const updatedColumns: ColumnMetadata[] = [];
         if (metadata) {
-            EntityMetadata.createPropertyPath(metadata, valuesSet).forEach(propertyPath => {
+            this.createPropertyPath(metadata, valuesSet).forEach(propertyPath => {
                 // todo: make this and other query builder to work with properly with tables without metadata
                 const columns = metadata.findColumnsWithPropertyPath(propertyPath);
 

test/functional/query-builder/select/entity/Category.ts

@@ -2,6 +2,8 @@ import {Entity} from "../../../../../src/decorator/entity/Entity";
 import {PrimaryGeneratedColumn} from "../../../../../src/decorator/columns/PrimaryGeneratedColumn";
 import {Column} from "../../../../../src/decorator/columns/Column";
 import {VersionColumn} from "../../../../../src/decorator/columns/VersionColumn";
+import { Post } from "./Post";
+import { OneToMany } from "../../../../../src";
 
 @Entity()
 export class Category {
@@ -18,4 +20,7 @@ export class Category {
     @VersionColumn()
     version: string;
 
-}
+    @OneToMany(() => Post, (post) => post.category)
+    posts: Post[]
+
+}