Skip to content

Commit

Permalink
fix(core): do not lookup in identity map with non-PK conditions
Browse files Browse the repository at this point in the history
This fixes `findOne(Ent, { id: 1, active: false })` like conditions, that were mistakenly checking
the identity map based on the PK present in condition, even if there were additional conditions
like `active: false`.

Closes #625
  • Loading branch information
B4nan committed Aug 9, 2020
1 parent 6a7f627 commit 4fb0e52
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/EntityManager.ts
Expand Up @@ -297,7 +297,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {

entityName = Utils.className(entityName as string);
this.validator.validatePrimaryKey(data as EntityData<T>, this.metadata.get(entityName));
let entity = this.getUnitOfWork().tryGetById<T>(entityName, data as FilterQuery<T>);
let entity = this.getUnitOfWork().tryGetById<T>(entityName, data as FilterQuery<T>, false);

if (entity && wrap(entity).isInitialized() && !refresh) {
return entity;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/unit-of-work/UnitOfWork.ts
Expand Up @@ -61,8 +61,8 @@ export class UnitOfWork {
return this.identityMap[token] as T;
}

tryGetById<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>): T | null {
const pk = Utils.extractPK(where, this.metadata.get(entityName));
tryGetById<T extends AnyEntity<T>>(entityName: string, where: FilterQuery<T>, strict = true): T | null {
const pk = Utils.extractPK(where, this.metadata.get(entityName), strict);

if (!pk) {
return null;
Expand Down
10 changes: 7 additions & 3 deletions packages/core/src/utils/Utils.ts
Expand Up @@ -272,13 +272,17 @@ export class Utils {
/**
* Extracts primary key from `data`. Accepts objects or primary keys directly.
*/
static extractPK<T extends AnyEntity<T>>(data: any, meta?: EntityMetadata): Primary<T> | null {
static extractPK<T extends AnyEntity<T>>(data: any, meta?: EntityMetadata, strict = false): Primary<T> | null {
if (Utils.isPrimaryKey(data)) {
return data as Primary<T>;
}

if (Utils.isEntity(data, true) && !meta) {
meta = wrap(data, true).__meta;
if (Utils.isEntity(data, true)) {
return wrap(data, true).__primaryKey as Primary<T>;
}

if (strict && meta && Object.keys(data).length !== meta.primaryKeys.length) {
return null;
}

if (Utils.isObject(data) && meta) {
Expand Down
8 changes: 8 additions & 0 deletions tests/EntityManager.mysql.test.ts
Expand Up @@ -2232,6 +2232,14 @@ describe('EntityManagerMySql', () => {
// await expect(orm.em.execute('insert into foo_bar2 () values ()')).rejects.toThrow(NotNullConstraintViolationException);
});

test('GH 625', async () => {
const a = new Author2('n', 'e');
a.termsAccepted = false;
await orm.em.persistAndFlush(a);
const res = await orm.em.findOne(Author2, { id: a.id, termsAccepted: true });
expect(res).toBeNull();
});

test('em.execute()', async () => {
const res1 = await orm.em.execute('insert into author2 (name, email) values (?, ?)', ['name', 'email']);
expect(res1).toMatchObject({ affectedRows: 1, insertId: 1 });
Expand Down

0 comments on commit 4fb0e52

Please sign in to comment.