Skip to content

Commit

Permalink
fix: BaseEntity.reload method regression and made findOne to throw …
Browse files Browse the repository at this point in the history 
…error on missing conditions in runtime (typeorm#8801)

* fixed reload method and how `findOne` works in runtime

* fixing broken tests

* fixing broken tests

* fixing broken tests

* fixing broken tests

* fixing broken tests
  • Loading branch information
@pleerock @M-TGH
pleerock authored and M-TGH committed Mar 29, 2022
1 parent 2c13e12 commit 512914e
Show file tree
Hide file tree
Showing 13 changed files with 108 additions and 37 deletions.
6 changes: 6 additions & 0 deletions src/entity-manager/EntityManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,12 @@ export class EntityManager {
alias = options.join.alias
}

if (!options.where) {
throw new Error(
`You must provide selection conditions in order to find a single row.`,
)
}

// create query builder and apply find options
return this.createQueryBuilder<Entity>(entityClass, alias)
.setFindOptions({
Expand Down
12 changes: 9 additions & 3 deletions src/repository/BaseEntity.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,17 @@ export class BaseEntity {
*/
async reload(): Promise<void> {
const baseEntity = this.constructor as typeof BaseEntity
const newestEntity: BaseEntity = await baseEntity
const id = baseEntity.getRepository().metadata.getEntityIdMap(this)
if (!id) {
throw new Error(
`Entity doesn't have id-s set, cannot reload entity`,
)
}
const reloadedEntity: BaseEntity = await baseEntity
.getRepository()
.findOneOrFail(baseEntity.getId(this))
.findOneByOrFail(id)

ObjectUtils.assign(this, newestEntity)
ObjectUtils.assign(this, reloadedEntity)
}

// -------------------------------------------------------------------------
Expand Down
53 changes: 51 additions & 2 deletions test/functional/entity-model/entity-model.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import "reflect-metadata"
import "../../utils/test-setup"
import { Post } from "./entity/Post"
import { Category } from "./entity/Category"
import {
closeTestingConnections,
createTestingConnections,
reloadTestingDatabases,
} from "../../utils/test-utils"
import { DataSource } from "../../../src/data-source/DataSource"
import { DataSource } from "../../../src"

describe("entity-model", () => {
let connections: DataSource[]
Expand Down Expand Up @@ -117,4 +117,53 @@ describe("entity-model", () => {
})
}
})

it("should reload exactly the same entity", async () => {
// These must run sequentially as we have the global context of the `Post` ActiveRecord class
for (const connection of connections) {
await connection.synchronize(true)
Post.useDataSource(connection)
Category.useDataSource(connection)

const post1 = Post.create()
post1.title = "About ActiveRecord 1"
post1.externalId = "some external id 1"
await post1.save()

post1.should.be.eql({
id: 1,
title: "About ActiveRecord 1",
text: "This is default text.",
externalId: "some external id 1",
})
await post1.reload()

post1.should.be.eql({
id: 1,
title: "About ActiveRecord 1",
text: "This is default text.",
externalId: "some external id 1",
})

const post2 = Post.create()
post2.title = "About ActiveRecord 2"
post2.externalId = "some external id 2"
await post2.save()

post2.should.be.eql({
id: 2,
title: "About ActiveRecord 2",
text: "This is default text.",
externalId: "some external id 2",
})
await post2.reload()

post2.should.be.eql({
id: 2,
title: "About ActiveRecord 2",
text: "This is default text.",
externalId: "some external id 2",
})
}
})
})
5 changes: 1 addition & 4 deletions test/functional/entity-model/entity/Category.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import { PrimaryColumn } from "../../../../src/decorator/columns/PrimaryColumn"
import { Entity } from "../../../../src/decorator/entity/Entity"
import { BaseEntity } from "../../../../src/repository/BaseEntity"
import { Column } from "../../../../src/decorator/columns/Column"
import { BaseEntity, Column, Entity, PrimaryColumn } from "../../../../src"

@Entity()
export class Category extends BaseEntity {
Expand Down
13 changes: 8 additions & 5 deletions test/functional/entity-model/entity/Post.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { Entity } from "../../../../src/decorator/entity/Entity"
import { BaseEntity } from "../../../../src/repository/BaseEntity"
import { PrimaryGeneratedColumn } from "../../../../src/decorator/columns/PrimaryGeneratedColumn"
import { Column } from "../../../../src/decorator/columns/Column"
import { JoinTable, ManyToMany } from "../../../../src"
import {
BaseEntity,
Column,
Entity,
JoinTable,
ManyToMany,
PrimaryGeneratedColumn,
} from "../../../../src"
import { Category } from "./Category"

@Entity()
Expand Down
8 changes: 4 additions & 4 deletions ...tiple-primary-keys/multiple-primary-keys-one-to-many/multiple-primary-keys-one-to-many.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe("relations > multiple-primary-keys > one-to-many", () => {
connections.map(async (connection) => {
await insertSimpleTestData(connection)

const user = await connection.getRepository(User).findOne({
const [user] = await connection.getRepository(User).find({
relations: ["settings"],
// relationLoadStrategy: "join"
})
Expand All @@ -84,9 +84,9 @@ describe("relations > multiple-primary-keys > one-to-many", () => {
},
])

const user = await connection
const [user] = await connection
.getRepository(User)
.findOne({ relations: ["settings"] })
.find({ relations: ["settings"] })

// check the saved items have correctly updated value
expect(user!).not.to.be.undefined
Expand Down Expand Up @@ -116,7 +116,7 @@ describe("relations > multiple-primary-keys > one-to-many", () => {
settings: [],
})

const user = await connection.getRepository(User).findOne({
const [user] = await connection.getRepository(User).find({
relations: ["settings"],
})

Expand Down
15 changes: 8 additions & 7 deletions test/functional/repository/find-methods/repostiory-find-methods.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ describe("repository > find methods", () => {
})

describe("findOne", function () {
it("should return first when no criteria given", () =>
it("should throw an error when no criteria given", () =>
Promise.all(
connections.map(async (connection) => {
const userRepository =
Expand All @@ -390,12 +390,13 @@ describe("repository > find methods", () => {
await userRepository.save(user)
}

const loadedUser = (await userRepository.findOne({
order: { id: "ASC" },
}))!
loadedUser.id.should.be.equal(0)
loadedUser.firstName.should.be.equal("name #0")
loadedUser.secondName.should.be.equal("Doe")
return userRepository
.findOne({
order: { id: "ASC" },
})
.should.be.rejectedWith(
`You must provide selection conditions in order to find a single row.`,
)
}),
))

Expand Down
11 changes: 11 additions & 0 deletions test/functional/repository/find-options-locking/find-options-locking.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ describe("repository > find options > locking", () => {
entityManager
.getRepository(Post)
.findOne({
where: { id: 1 },
lock: { mode: "pessimistic_write", tables: [] },
})
.should.be.rejectedWith(
Expand All @@ -491,6 +492,7 @@ describe("repository > find options > locking", () => {
entityManager
.getRepository(Post)
.findOne({
where: { id: 1 },
relations: ["author"],
lock: {
mode: "pessimistic_write",
Expand All @@ -512,6 +514,7 @@ describe("repository > find options > locking", () => {
return connection.manager.transaction((entityManager) => {
return Promise.all([
entityManager.getRepository(Post).findOne({
where: { id: 1 },
relations: ["author"],
lock: {
mode: "pessimistic_write",
Expand All @@ -530,6 +533,7 @@ describe("repository > find options > locking", () => {
return connection.manager.transaction((entityManager) => {
return Promise.all([
entityManager.getRepository(Post).findOne({
where: { id: 1 },
relations: ["author"],
lock: {
mode: "pessimistic_write",
Expand All @@ -539,6 +543,7 @@ describe("repository > find options > locking", () => {
entityManager
.getRepository(Post)
.findOne({
where: { id: 1 },
relations: ["author"],
lock: { mode: "pessimistic_write" },
})
Expand All @@ -561,34 +566,39 @@ describe("repository > find options > locking", () => {
return connection.manager.transaction((entityManager) => {
return Promise.all([
entityManager.getRepository(Post).findOne({
where: { id: 1 },
relations: ["author"],
lock: {
mode: "pessimistic_read",
tables: ["post"],
},
}),
entityManager.getRepository(Post).findOne({
where: { id: 1 },
relations: ["author"],
lock: {
mode: "pessimistic_write",
tables: ["post"],
},
}),
entityManager.getRepository(Post).findOne({
where: { id: 1 },
relations: ["author"],
lock: {
mode: "pessimistic_partial_write",
tables: ["post"],
},
}),
entityManager.getRepository(Post).findOne({
where: { id: 1 },
relations: ["author"],
lock: {
mode: "pessimistic_write_or_fail",
tables: ["post"],
},
}),
entityManager.getRepository(Post).findOne({
where: { id: 1 },
relations: ["author"],
lock: {
mode: "for_no_key_update",
Expand All @@ -614,6 +624,7 @@ describe("repository > find options > locking", () => {
return connection.manager.transaction((entityManager) => {
return Promise.all([
entityManager.getRepository(Post).findOne({
where: { id: 1 },
join: {
alias: "post",
innerJoinAndSelect: {
Expand Down
8 changes: 3 additions & 5 deletions test/functional/repository/find-options/repository-find-options.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,9 @@ describe("repository > find options", () => {
post.categories = [category]
await connection.manager.save(post)

const loadedPost = await connection
.getRepository(Post)
.findOne({
relations: ["author", "categories"],
})
const [loadedPost] = await connection.getRepository(Post).find({
relations: ["author", "categories"],
})
expect(loadedPost).to.be.eql({
id: 1,
title: "About Alex Messer",
Expand Down
8 changes: 4 additions & 4 deletions test/functional/table-inheritance/single-table/no-type-column/no-type-column.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,17 @@ describe("table-inheritance > single-table > no-type-column", () => {
// Select
// -------------------------------------------------------------------------

const postIt = (await postItRepo.findOne({
const [postIt] = await postItRepo.find({
relations: ["owner"],
})) as PostItNote
})

postIt.owner.should.be.an.instanceOf(Employee)
postIt.owner.name.should.be.equal("alicefoo")
postIt.owner.employeeName.should.be.equal("Alice Foo")

const sticky = (await stickyRepo.findOne({
const [sticky] = await stickyRepo.find({
relations: ["owner"],
})) as StickyNote
})

sticky.owner.should.be.an.instanceOf(Author)
sticky.owner.name.should.be.equal("bobbar")
Expand Down
2 changes: 1 addition & 1 deletion test/github-issues/1720/issue-1720.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe("github issues > #1720 Listener not invoked when relation loaded throug
post1.categories = [category1, category2]
await connection.manager.save(post1)

const loadedPost = await connection.manager.findOne(Post, {
const [loadedPost] = await connection.manager.find(Post, {
relations: ["categories"],
})
loadedPost!.categories[0].loaded.should.be.equal(true)
Expand Down
2 changes: 1 addition & 1 deletion test/github-issues/3118/issue-3118.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe("github issues > #3118 shorten alias names (for RDBMS with a limit) whe
.save(category)
}

const loadedCategory = await connection.manager.findOne(
const [loadedCategory] = await connection.manager.find(
CategoryWithVeryLongName,
{
relations: [
Expand Down
2 changes: 1 addition & 1 deletion test/github-issues/7932/issue-7932.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe("github issues > #7932 non-ascii characters assigned to var/char colum
entity.fixedLengthContent = "\u2022"

await repo.save(entity)
const savedEntity = await repo.findOne({
const [savedEntity] = await repo.find({
order: { created: "DESC" },
})

Expand Down

0 comments on commit 512914e

Please sign in to comment.