diff --git a/src/common/PrimitiveCriteria.ts b/src/common/PrimitiveCriteria.ts new file mode 100644 index 000000000..e16996e9e --- /dev/null +++ b/src/common/PrimitiveCriteria.ts @@ -0,0 +1,5 @@ +export type SinglePrimitiveCriteria = string | number | Date + +export type PrimitiveCriteria = + | SinglePrimitiveCriteria + | SinglePrimitiveCriteria[] diff --git a/src/entity-manager/EntityManager.ts b/src/entity-manager/EntityManager.ts index b5cb7e784..fa8a4323a 100644 --- a/src/entity-manager/EntityManager.ts +++ b/src/entity-manager/EntityManager.ts @@ -38,6 +38,7 @@ import { UpsertOptions } from "../repository/UpsertOptions" import { InstanceChecker } from "../util/InstanceChecker" import { ObjectLiteral } from "../common/ObjectLiteral" import { PickKeysByType } from "../common/PickKeysByType" +import { OrmUtils } from "../util/OrmUtils" /** * Entity manager supposed to work with any entity, automatically find its repository and call its methods, @@ -761,12 +762,7 @@ export class EntityManager { partialEntity: QueryDeepPartialEntity, ): Promise { // if user passed empty criteria or empty list of criterias, then throw an error - if ( - criteria === undefined || - criteria === null || - criteria === "" || - (Array.isArray(criteria) && criteria.length === 0) - ) { + if (OrmUtils.isCriteriaNullOrEmpty(criteria)) { return Promise.reject( new TypeORMError( `Empty criteria(s) are not allowed for the update method.`, @@ -774,12 +770,7 @@ export class EntityManager { ) } - if ( - typeof criteria === "string" || - typeof criteria === "number" || - criteria instanceof Date || - Array.isArray(criteria) - ) { + if (OrmUtils.isPrimitiveCriteria(criteria)) { return this.createQueryBuilder() .update(target) .set(partialEntity) @@ -815,12 +806,7 @@ export class EntityManager { | any, ): Promise { // if user passed empty criteria or empty list of criterias, then throw an error - if ( - criteria === undefined || - criteria === null || - criteria === "" || - (Array.isArray(criteria) && criteria.length === 0) - ) { + if (OrmUtils.isCriteriaNullOrEmpty(criteria)) { return Promise.reject( new TypeORMError( `Empty criteria(s) are not allowed for the delete method.`, @@ -828,12 +814,7 @@ export class EntityManager { ) } - if ( - typeof criteria === "string" || - typeof criteria === "number" || - criteria instanceof Date || - Array.isArray(criteria) - ) { + if (OrmUtils.isPrimitiveCriteria(criteria)) { return this.createQueryBuilder() .delete() .from(targetOrEntity) @@ -869,12 +850,7 @@ export class EntityManager { | any, ): Promise { // if user passed empty criteria or empty list of criterias, then throw an error - if ( - criteria === undefined || - criteria === null || - criteria === "" || - (Array.isArray(criteria) && criteria.length === 0) - ) { + if (OrmUtils.isCriteriaNullOrEmpty(criteria)) { return Promise.reject( new TypeORMError( `Empty criteria(s) are not allowed for the delete method.`, @@ -882,12 +858,7 @@ export class EntityManager { ) } - if ( - typeof criteria === "string" || - typeof criteria === "number" || - criteria instanceof Date || - Array.isArray(criteria) - ) { + if (OrmUtils.isPrimitiveCriteria(criteria)) { return this.createQueryBuilder() .softDelete() .from(targetOrEntity) @@ -923,12 +894,7 @@ export class EntityManager { | any, ): Promise { // if user passed empty criteria or empty list of criterias, then throw an error - if ( - criteria === undefined || - criteria === null || - criteria === "" || - (Array.isArray(criteria) && criteria.length === 0) - ) { + if (OrmUtils.isCriteriaNullOrEmpty(criteria)) { return Promise.reject( new TypeORMError( `Empty criteria(s) are not allowed for the delete method.`, @@ -936,12 +902,7 @@ export class EntityManager { ) } - if ( - typeof criteria === "string" || - typeof criteria === "number" || - criteria instanceof Date || - Array.isArray(criteria) - ) { + if (OrmUtils.isPrimitiveCriteria(criteria)) { return this.createQueryBuilder() .restore() .from(targetOrEntity) diff --git a/src/repository/Repository.ts b/src/repository/Repository.ts index 8fb68bbc7..c7f2f2a9a 100644 --- a/src/repository/Repository.ts +++ b/src/repository/Repository.ts @@ -355,7 +355,8 @@ export class Repository { | Date[] | ObjectId | ObjectId[] - | FindOptionsWhere, + | FindOptionsWhere + | FindOptionsWhere[], partialEntity: QueryDeepPartialEntity, ): Promise { return this.manager.update( @@ -399,7 +400,8 @@ export class Repository { | Date[] | ObjectId | ObjectId[] - | FindOptionsWhere, + | FindOptionsWhere + | FindOptionsWhere[], ): Promise { return this.manager.delete(this.metadata.target as any, criteria as any) } @@ -420,7 +422,8 @@ export class Repository { | Date[] | ObjectId | ObjectId[] - | FindOptionsWhere, + | FindOptionsWhere + | FindOptionsWhere[], ): Promise { return this.manager.softDelete( this.metadata.target as any, @@ -444,7 +447,8 @@ export class Repository { | Date[] | ObjectId | ObjectId[] - | FindOptionsWhere, + | FindOptionsWhere + | FindOptionsWhere[], ): Promise { return this.manager.restore( this.metadata.target as any, diff --git a/src/util/OrmUtils.ts b/src/util/OrmUtils.ts index d40a63e80..4928443c1 100644 --- a/src/util/OrmUtils.ts +++ b/src/util/OrmUtils.ts @@ -1,4 +1,8 @@ import { ObjectLiteral } from "../common/ObjectLiteral" +import { + PrimitiveCriteria, + SinglePrimitiveCriteria, +} from "../common/PrimitiveCriteria" export class OrmUtils { // ------------------------------------------------------------------------- @@ -8,13 +12,13 @@ export class OrmUtils { /** * Chunks array into pieces. */ - static chunk(array: T[], size: number): T[][] { + public static chunk(array: T[], size: number): T[][] { return Array.from(Array(Math.ceil(array.length / size)), (_, i) => { return array.slice(i * size, i * size + size) }) } - static splitClassesAndStrings( + public static splitClassesAndStrings( classesAndStrings: (string | T)[], ): [T[], string[]] { return [ @@ -27,7 +31,7 @@ export class OrmUtils { ] } - static groupBy( + public static groupBy( array: T[], propertyCallback: (item: T) => R, ): { id: R; items: T[] }[] { @@ -43,9 +47,9 @@ export class OrmUtils { }, [] as Array<{ id: R; items: T[] }>) } - static uniq(array: T[], criteria?: (item: T) => any): T[] - static uniq(array: T[], property: K): T[] - static uniq( + public static uniq(array: T[], criteria?: (item: T) => any): T[] + public static uniq(array: T[], property: K): T[] + public static uniq( array: T[], criteriaOrProperty?: ((item: T) => any) | K, ): T[] { @@ -73,106 +77,10 @@ export class OrmUtils { }, [] as T[]) } - // Checks if it's an object made by Object.create(null), {} or new Object() - private static isPlainObject(item: any) { - if (item === null || item === undefined) { - return false - } - - return !item.constructor || item.constructor === Object - } - - private static mergeArrayKey( - target: any, - key: number, - value: any, - memo: Map, - ) { - // Have we seen this before? Prevent infinite recursion. - if (memo.has(value)) { - target[key] = memo.get(value) - return - } - - if (value instanceof Promise) { - // Skip promises entirely. - // This is a hold-over from the old code & is because we don't want to pull in - // the lazy fields. Ideally we'd remove these promises via another function first - // but for now we have to do it here. - return - } - - if (!this.isPlainObject(value) && !Array.isArray(value)) { - target[key] = value - return - } - - if (!target[key]) { - target[key] = Array.isArray(value) ? [] : {} - } - - memo.set(value, target[key]) - this.merge(target[key], value, memo) - memo.delete(value) - } - - private static mergeObjectKey( - target: any, - key: string, - value: any, - memo: Map, - ) { - // Have we seen this before? Prevent infinite recursion. - if (memo.has(value)) { - Object.assign(target, { [key]: memo.get(value) }) - return - } - - if (value instanceof Promise) { - // Skip promises entirely. - // This is a hold-over from the old code & is because we don't want to pull in - // the lazy fields. Ideally we'd remove these promises via another function first - // but for now we have to do it here. - return - } - - if (!this.isPlainObject(value) && !Array.isArray(value)) { - Object.assign(target, { [key]: value }) - return - } - - if (!target[key]) { - Object.assign(target, { [key]: Array.isArray(value) ? [] : {} }) - } - - memo.set(value, target[key]) - this.merge(target[key], value, memo) - memo.delete(value) - } - - private static merge( - target: any, - source: any, - memo: Map = new Map(), - ): any { - if (this.isPlainObject(target) && this.isPlainObject(source)) { - for (const key of Object.keys(source)) { - if (key === "__proto__") continue - this.mergeObjectKey(target, key, source[key], memo) - } - } - - if (Array.isArray(target) && Array.isArray(source)) { - for (let key = 0; key < source.length; key++) { - this.mergeArrayKey(target, key, source[key], memo) - } - } - } - /** * Deep Object.assign. */ - static mergeDeep(target: any, ...sources: any[]): any { + public static mergeDeep(target: any, ...sources: any[]): any { if (!sources.length) { return target } @@ -189,7 +97,7 @@ export class OrmUtils { * * @see http://stackoverflow.com/a/1144249 */ - static deepCompare(...args: any[]): boolean { + public static deepCompare(...args: any[]): boolean { let i: any, l: any, leftChain: any, rightChain: any if (arguments.length < 1) { @@ -219,7 +127,7 @@ export class OrmUtils { /** * Gets deeper value of object. */ - static deepValue(obj: ObjectLiteral, path: string) { + public static deepValue(obj: ObjectLiteral, path: string) { const segments = path.split(".") for (let i = 0, len = segments.length; i < len; i++) { obj = obj[segments[i]] @@ -227,7 +135,7 @@ export class OrmUtils { return obj } - static replaceEmptyObjectsWithBooleans(obj: any) { + public static replaceEmptyObjectsWithBooleans(obj: any) { for (const key in obj) { if (obj[key] && typeof obj[key] === "object") { if (Object.keys(obj[key]).length === 0) { @@ -239,7 +147,7 @@ export class OrmUtils { } } - static propertyPathsToTruthyObject(paths: string[]) { + public static propertyPathsToTruthyObject(paths: string[]) { const obj: any = {} for (const path of paths) { const props = path.split(".") @@ -270,7 +178,7 @@ export class OrmUtils { /** * Check if two entity-id-maps are the same */ - static compareIds( + public static compareIds( firstId: ObjectLiteral | undefined, secondId: ObjectLiteral | undefined, ): boolean { @@ -300,7 +208,7 @@ export class OrmUtils { /** * Transforms given value into boolean value. */ - static toBoolean(value: any): boolean { + public static toBoolean(value: any): boolean { if (typeof value === "boolean") return value if (typeof value === "string") return value === "true" || value === "1" @@ -313,7 +221,7 @@ export class OrmUtils { /** * Composes an object from the given array of keys and values. */ - static zipObject(keys: any[], values: any[]): ObjectLiteral { + public static zipObject(keys: any[], values: any[]): ObjectLiteral { return keys.reduce((object, column, index) => { object[column] = values[index] return object @@ -323,14 +231,14 @@ export class OrmUtils { /** * Compares two arrays. */ - static isArraysEqual(arr1: any[], arr2: any[]): boolean { + public static isArraysEqual(arr1: any[], arr2: any[]): boolean { if (arr1.length !== arr2.length) return false return arr1.every((element) => { return arr2.indexOf(element) !== -1 }) } - static areMutuallyExclusive(...lists: T[][]): boolean { + public static areMutuallyExclusive(...lists: T[][]): boolean { const haveSharedObjects = lists.some((list) => { const otherLists = lists.filter((otherList) => otherList !== list) return list.some((item) => @@ -345,7 +253,7 @@ export class OrmUtils { * all values allowed by the constraint or undefined if the constraint * is not present. */ - static parseSqlCheckExpression( + public static parseSqlCheckExpression( sql: string, columnName: string, ): string[] | undefined { @@ -415,6 +323,48 @@ export class OrmUtils { return undefined } + /** + * Checks if given criteria is null or empty. + */ + public static isCriteriaNullOrEmpty(criteria: unknown): boolean { + return ( + criteria === undefined || + criteria === null || + criteria === "" || + (Array.isArray(criteria) && criteria.length === 0) || + (this.isPlainObject(criteria) && Object.keys(criteria).length === 0) + ) + } + + /** + * Checks if given criteria is a primitive value. + * Primitive values are strings, numbers and dates. + */ + public static isSinglePrimitiveCriteria( + criteria: unknown, + ): criteria is SinglePrimitiveCriteria { + return ( + typeof criteria === "string" || + typeof criteria === "number" || + criteria instanceof Date + ) + } + + /** + * Checks if given criteria is a primitive value or an array of primitive values. + */ + public static isPrimitiveCriteria( + criteria: unknown, + ): criteria is PrimitiveCriteria { + if (Array.isArray(criteria)) { + return criteria.every((value) => + this.isSinglePrimitiveCriteria(value), + ) + } + + return this.isSinglePrimitiveCriteria(criteria) + } + // ------------------------------------------------------------------------- // Private methods // ------------------------------------------------------------------------- @@ -522,4 +472,100 @@ export class OrmUtils { return true } + + // Checks if it's an object made by Object.create(null), {} or new Object() + private static isPlainObject(item: any) { + if (item === null || item === undefined) { + return false + } + + return !item.constructor || item.constructor === Object + } + + private static mergeArrayKey( + target: any, + key: number, + value: any, + memo: Map, + ) { + // Have we seen this before? Prevent infinite recursion. + if (memo.has(value)) { + target[key] = memo.get(value) + return + } + + if (value instanceof Promise) { + // Skip promises entirely. + // This is a hold-over from the old code & is because we don't want to pull in + // the lazy fields. Ideally we'd remove these promises via another function first + // but for now we have to do it here. + return + } + + if (!this.isPlainObject(value) && !Array.isArray(value)) { + target[key] = value + return + } + + if (!target[key]) { + target[key] = Array.isArray(value) ? [] : {} + } + + memo.set(value, target[key]) + this.merge(target[key], value, memo) + memo.delete(value) + } + + private static mergeObjectKey( + target: any, + key: string, + value: any, + memo: Map, + ) { + // Have we seen this before? Prevent infinite recursion. + if (memo.has(value)) { + Object.assign(target, { [key]: memo.get(value) }) + return + } + + if (value instanceof Promise) { + // Skip promises entirely. + // This is a hold-over from the old code & is because we don't want to pull in + // the lazy fields. Ideally we'd remove these promises via another function first + // but for now we have to do it here. + return + } + + if (!this.isPlainObject(value) && !Array.isArray(value)) { + Object.assign(target, { [key]: value }) + return + } + + if (!target[key]) { + Object.assign(target, { [key]: Array.isArray(value) ? [] : {} }) + } + + memo.set(value, target[key]) + this.merge(target[key], value, memo) + memo.delete(value) + } + + private static merge( + target: any, + source: any, + memo: Map = new Map(), + ): any { + if (this.isPlainObject(target) && this.isPlainObject(source)) { + for (const key of Object.keys(source)) { + if (key === "__proto__") continue + this.mergeObjectKey(target, key, source[key], memo) + } + } + + if (Array.isArray(target) && Array.isArray(source)) { + for (let key = 0; key < source.length; key++) { + this.mergeArrayKey(target, key, source[key], memo) + } + } + } } diff --git a/test/github-issues/10517/entity/Post.ts b/test/github-issues/10517/entity/Post.ts new file mode 100644 index 000000000..d6ddd543f --- /dev/null +++ b/test/github-issues/10517/entity/Post.ts @@ -0,0 +1,18 @@ +import { + Column, + Entity, + PrimaryGeneratedColumn, + DeleteDateColumn, +} from "../../../../src" + +@Entity() +export class Post { + @PrimaryGeneratedColumn() + id: number + + @Column() + title: string + + @DeleteDateColumn() + deletedDate: Date +} diff --git a/test/github-issues/10517/issue-10517.ts b/test/github-issues/10517/issue-10517.ts new file mode 100644 index 000000000..beec5d95d --- /dev/null +++ b/test/github-issues/10517/issue-10517.ts @@ -0,0 +1,248 @@ +import "reflect-metadata" +import { expect } from "chai" +import { DataSource } from "../../../src/data-source/DataSource" +import { + closeTestingConnections, + createTestingConnections, + reloadTestingDatabases, +} from "../../utils/test-utils" +import { Post } from "./entity/Post" + +describe("github issues > #10517 EntityManager update/delete/softDelete don't work with list of where condition objects", function () { + // ------------------------------------------------------------------------- + // Configuration + // ------------------------------------------------------------------------- + + let connections: DataSource[] + before( + async () => + (connections = await createTestingConnections({ + entities: [Post], + })), + ) + beforeEach(() => reloadTestingDatabases(connections)) + after(() => closeTestingConnections(connections)) + + // ------------------------------------------------------------------------- + // Specifications + // ------------------------------------------------------------------------- + + it("update by array of condition objects", () => + Promise.all( + connections.map(async (connection) => { + const postRepository = connection.getRepository(Post) + + // save a new posts + const newPost1 = postRepository.create() + newPost1.title = "Super post #1" + const newPost2 = postRepository.create() + newPost2.title = "Super post #2" + const newPost3 = postRepository.create() + newPost3.title = "Super post #3" + const newPost4 = postRepository.create() + newPost4.title = "Super post #4" + + await postRepository.save(newPost1) + await postRepository.save(newPost2) + await postRepository.save(newPost3) + await postRepository.save(newPost4) + + // update many + await postRepository.update( + [ + { + title: "Super post #1", + }, + { + title: "Super post #2", + }, + ], + { title: "Super post" }, + ) + + // load to check + const loadedPost1 = await postRepository.findOne({ + where: { + id: 1, + }, + }) + const loadedPost2 = await postRepository.findOne({ + where: { + id: 2, + }, + }) + + // assert + expect(loadedPost1).to.be.eql({ + id: 1, + title: "Super post", + deletedDate: null, + }) + + expect(loadedPost2).to.be.eql({ + id: 2, + title: "Super post", + deletedDate: null, + }) + }), + )) + + it("delete by array of condition objects", () => + Promise.all( + connections.map(async (connection) => { + const postRepository = connection.getRepository(Post) + + // save a new posts + const newPost1 = postRepository.create() + newPost1.title = "Super post #1" + const newPost2 = postRepository.create() + newPost2.title = "Super post #2" + const newPost3 = postRepository.create() + newPost3.title = "Super post #3" + const newPost4 = postRepository.create() + newPost4.title = "Super post #4" + + await postRepository.save(newPost1) + await postRepository.save(newPost2) + await postRepository.save(newPost3) + await postRepository.save(newPost4) + + // delete many + await postRepository.delete([ + { + title: "Super post #1", + }, + { + title: "Super post #2", + }, + ]) + + // load to check + const loadedPost1 = await postRepository.findOne({ + where: { + id: 1, + }, + }) + const loadedPost2 = await postRepository.findOne({ + where: { + id: 2, + }, + }) + + // assert + expect(loadedPost1).to.be.eql(null) + + expect(loadedPost2).to.be.eql(null) + }), + )) + + it("soft delete by array of condition objects", () => + Promise.all( + connections.map(async (connection) => { + const postRepository = connection.getRepository(Post) + + // save a new posts + const newPost1 = postRepository.create() + newPost1.title = "Super post #1" + const newPost2 = postRepository.create() + newPost2.title = "Super post #2" + const newPost3 = postRepository.create() + newPost3.title = "Super post #3" + const newPost4 = postRepository.create() + newPost4.title = "Super post #4" + + await postRepository.save(newPost1) + await postRepository.save(newPost2) + await postRepository.save(newPost3) + await postRepository.save(newPost4) + + // delete many + await postRepository.softDelete([ + { + title: "Super post #1", + }, + { + title: "Super post #2", + }, + ]) + + // load to check + const loadedPost1 = await postRepository.findOne({ + where: { + id: 1, + }, + }) + const loadedPost2 = await postRepository.findOne({ + where: { + id: 2, + }, + }) + + // assert + expect(loadedPost1).to.be.eql(null) + + expect(loadedPost2).to.be.eql(null) + }), + )) + + it("restore by array of condition objects", () => + Promise.all( + connections.map(async (connection) => { + const postRepository = connection.getRepository(Post) + + // save a new posts + const newPost1 = postRepository.create() + newPost1.title = "Super post #1" + const newPost2 = postRepository.create() + newPost2.title = "Super post #2" + const newPost3 = postRepository.create() + newPost3.title = "Super post #3" + const newPost4 = postRepository.create() + newPost4.title = "Super post #4" + + await postRepository.save(newPost1) + await postRepository.save(newPost2) + await postRepository.save(newPost3) + await postRepository.save(newPost4) + + const conditions = [ + { + title: "Super post #1", + }, + { + title: "Super post #2", + }, + ] + + // update many + await postRepository.softDelete(conditions) + + await postRepository.restore(conditions) + + // load to check + const loadedPost1 = await postRepository.findOne({ + where: { + id: 1, + }, + }) + const loadedPost2 = await postRepository.findOne({ + where: { + id: 2, + }, + }) + + // assert + expect(loadedPost1).to.be.eql({ + id: 1, + title: "Super post #1", + deletedDate: null, + }) + + expect(loadedPost2).to.be.eql({ + id: 2, + title: "Super post #2", + deletedDate: null, + }) + }), + )) +})