fix: update/delete/softDelete by criteria of condition objects (#10910)

Co-authored-by: maxbronnikov10 <maxbronnikov2004@gmail.com>
This commit is contained in:
Maxim Bronnikov 2025-05-07 00:30:05 +03:00 committed by GitHub
parent 3ffeea590f
commit b94dfb3e31
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 447 additions and 165 deletions

View File

@ -0,0 +1,5 @@
export type SinglePrimitiveCriteria = string | number | Date
export type PrimitiveCriteria =
| SinglePrimitiveCriteria
| SinglePrimitiveCriteria[]

View File

@ -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<Entity>,
): Promise<UpdateResult> {
// 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<DeleteResult> {
// 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<UpdateResult> {
// 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<UpdateResult> {
// 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)

View File

@ -355,7 +355,8 @@ export class Repository<Entity extends ObjectLiteral> {
| Date[]
| ObjectId
| ObjectId[]
| FindOptionsWhere<Entity>,
| FindOptionsWhere<Entity>
| FindOptionsWhere<Entity>[],
partialEntity: QueryDeepPartialEntity<Entity>,
): Promise<UpdateResult> {
return this.manager.update(
@ -399,7 +400,8 @@ export class Repository<Entity extends ObjectLiteral> {
| Date[]
| ObjectId
| ObjectId[]
| FindOptionsWhere<Entity>,
| FindOptionsWhere<Entity>
| FindOptionsWhere<Entity>[],
): Promise<DeleteResult> {
return this.manager.delete(this.metadata.target as any, criteria as any)
}
@ -420,7 +422,8 @@ export class Repository<Entity extends ObjectLiteral> {
| Date[]
| ObjectId
| ObjectId[]
| FindOptionsWhere<Entity>,
| FindOptionsWhere<Entity>
| FindOptionsWhere<Entity>[],
): Promise<UpdateResult> {
return this.manager.softDelete(
this.metadata.target as any,
@ -444,7 +447,8 @@ export class Repository<Entity extends ObjectLiteral> {
| Date[]
| ObjectId
| ObjectId[]
| FindOptionsWhere<Entity>,
| FindOptionsWhere<Entity>
| FindOptionsWhere<Entity>[],
): Promise<UpdateResult> {
return this.manager.restore(
this.metadata.target as any,

View File

@ -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<T>(array: T[], size: number): T[][] {
public static chunk<T>(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<T>(
public static splitClassesAndStrings<T>(
classesAndStrings: (string | T)[],
): [T[], string[]] {
return [
@ -27,7 +31,7 @@ export class OrmUtils {
]
}
static groupBy<T, R>(
public static groupBy<T, R>(
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<T>(array: T[], criteria?: (item: T) => any): T[]
static uniq<T, K extends keyof T>(array: T[], property: K): T[]
static uniq<T, K extends keyof T>(
public static uniq<T>(array: T[], criteria?: (item: T) => any): T[]
public static uniq<T, K extends keyof T>(array: T[], property: K): T[]
public static uniq<T, K extends keyof T>(
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<any, any>,
) {
// 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<any, any>,
) {
// 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<any, any> = 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<T>(...lists: T[][]): boolean {
public static areMutuallyExclusive<T>(...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<any, any>,
) {
// 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<any, any>,
) {
// 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<any, any> = 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)
}
}
}
}

View File

@ -0,0 +1,18 @@
import {
Column,
Entity,
PrimaryGeneratedColumn,
DeleteDateColumn,
} from "../../../../src"
@Entity()
export class Post {
@PrimaryGeneratedColumn()
id: number
@Column()
title: string
@DeleteDateColumn()
deletedDate: Date
}

View File

@ -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,
})
}),
))
})