Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Cannot find matching entity for condition" when using upsertMany with 5.8.6 #4786

Closed
tmikoss opened this issue Oct 5, 2023 · 8 comments
Closed
Labels
bug Something isn't working

Comments

@tmikoss
Copy link

tmikoss commented Oct 5, 2023

Describe the bug
A "Cannot find matching entity for condition" error gets raised when using upsertMany in 5.8.6, whereas same code works fine in 5.7.14. A what I assume a read-back query gets issued after insert statement, and that query differs between versions.

Stack trace

5.7.14

select "i0".* from "internal_role" as "i0" where "i0"."id" = '4adb342b-ac72-436e-b616-8b38b7a95d19' limit 1 [took 2 ms, 1 result]
insert into "internal_role_permission" ("action", "internal_role_id", "subject") values ('read', '4adb342b-ac72-436e-b616-8b38b7a95d19', 'User'), ('update', '4adb342b-ac72-436e-b616-8b38b7a95d19', 'User') on conflict ("subject", "action", "internal_role_id") do nothing returning "id", "created_at", "updated_at" [took 2 ms]
select "i0"."id", "i0"."created_at", "i0"."updated_at", "i0"."subject", "i0"."action", "i0"."internal_role_id" from "internal_role_permission" as "i0" where (("i0"."subject" = 'User' and "i0"."action" = 'read' and "i0"."internal_role_id" = '4adb342b-ac72-436e-b616-8b38b7a95d19') or ("i0"."subject" = 'User' and "i0"."action" = 'update' and "i0"."internal_role_id" = '4adb342b-ac72-436e-b616-8b38b7a95d19')) [took 1 ms, 2 results]

5.8.6

select "i0".* from "internal_role" as "i0" where "i0"."id" = '68bf0a18-0cb4-4547-b386-6880a51f2327' limit 1 [took 2 ms, 1 result]
insert into "internal_role_permission" ("action", "internal_role_id", "subject") values ('read', '68bf0a18-0cb4-4547-b386-6880a51f2327', 'User'), ('update', '68bf0a18-0cb4-4547-b386-6880a51f2327', 'User') on conflict ("subject", "action", "internal_role_id") do nothing returning "id", "created_at", "updated_at" [took 2 ms]
select "i0"."id", "i0"."created_at", "i0"."updated_at", "i0"."subject", "i0"."action", "i0"."internal_role_id" from "internal_role_permission" as "i0" where ("i0"."internal_role_id" = '68bf0a18-0cb4-4547-b386-6880a51f2327' or "i0"."internal_role_id" = '68bf0a18-0cb4-4547-b386-6880a51f2327') [took 2 ms, 3 results]
ERROR [ExceptionsHandler] Cannot find matching entity for condition {"subject":"User","action":"read","internalRole":{"id":"68bf0a18-0cb4-4547-b386-6880a51f2327","createdAt":"2023-10-05T07:41:23.000Z","updatedAt":"2023-10-05T07:41:23.000Z","name":"dolorum"}}
Error: Cannot find matching entity for condition {"subject":"User","action":"read","internalRole":{"id":"68bf0a18-0cb4-4547-b386-6880a51f2327","createdAt":"2023-10-05T07:41:23.000Z","updatedAt":"2023-10-05T07:41:23.000Z","name":"dolorum"}}
    at SqlEntityManager.upsertMany ({{project_root}}/node_modules/@mikro-orm/core/EntityManager.js:697:27)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at InternalRolesService.update ({{project_root}}/src/internal-api/internal-users/internal-roles/internal-roles.service.ts:46:5)
    at {{project_root}}/node_modules/@nestjs/core/router/router-execution-context.js:46:28
    at {{project_root}}/node_modules/@nestjs/core/router/router-proxy.js:9:17

To Reproduce

// Definitions
@Entity({ abstract: true })
export abstract class ApplicationEntity<T extends { id: string }, Optionals = never> extends BaseEntity<T, 'id'> {
  [OptionalProps]?: Optionals | 'createdAt' | 'updatedAt'

  @PrimaryKey({ type: types.uuid, defaultRaw: 'gen_random_uuid()' })
  id: string

  @Property({ name: 'created_at', defaultRaw: 'now()' })
  createdAt: Date

  @Property({ name: 'updated_at', defaultRaw: 'now()', onUpdate: () => new Date() })
  updatedAt: Date
}

@Entity()
export class InternalRole extends ApplicationEntity<InternalRole> {
  @Property()
  name: string

  @OneToMany(() => InternalRolePermission, (permission) => permission.internalRole, { orphanRemoval: true })
  permissions = new Collection<InternalRolePermission>(this)
}

@Entity()
@Unique({ properties: ['subject', 'action', 'internalRole'] })
export class InternalRolePermission extends ApplicationEntity<InternalRolePermission> {
  @Property()
  subject: string

  @Property()
  action: string

  @ManyToOne(() => InternalRole, { onDelete: 'CASCADE' })
  internalRole: InternalRole
}


// For context, InternalRolesService definition
  constructor(
    @InjectRepository(InternalRole) private internalRoles: EntityRepository<InternalRole>,
    private em: EntityManager
  ) {}

 // Trimmed down minimum failing code
    const role = await this.internalRoles.findOneOrFail({ id })

    await this.em.upsertMany(InternalRolePermission, [
      { subject: 'User', action: 'read', internalRole: role },
      { action: 'update', subject: 'User', internalRole: role }
    ])

Expected behavior
Records get inserted without raising errors.

Additional context
NestJS 10.2 + Postgres 15.2

Versions

Dependency Version
node 18.16.0
typescript 5.1.6
mikro-orm 5.8.6
@mikro-orm/nestjs 5.2.0
pg 8.11.0
@B4nan
Copy link
Member

B4nan commented Oct 5, 2023

Failing repro:

import {
  Collection,
  Entity,
  ManyToOne,
  OneToMany,
  OptionalProps,
  PrimaryKey,
  Property,
  Unique,
} from '@mikro-orm/core';
import { MikroORM } from '@mikro-orm/sqlite';

@Entity({ abstract: true })
abstract class ApplicationEntity<Optionals = never> {

  [OptionalProps]?: Optionals | 'createdAt' | 'updatedAt';

  @PrimaryKey()
  id!: number;

  @Property({ name: 'created_at', defaultRaw: 'current_timestamp' })
  createdAt!: Date;

  @Property({ name: 'updated_at', defaultRaw: 'current_timestamp', onUpdate: () => new Date() })
  updatedAt!: Date;

}

@Entity()
class InternalRole extends ApplicationEntity {

  @Property()
  name!: string;

  @OneToMany(() => InternalRolePermission, permission => permission.internalRole, { orphanRemoval: true })
  permissions = new Collection<InternalRolePermission>(this);

}

@Entity()
@Unique({ properties: ['subject', 'action', 'internalRole'] })
class InternalRolePermission extends ApplicationEntity {

  @Property()
  subject!: string;

  @Property()
  action!: string;

  @ManyToOne(() => InternalRole, { onDelete: 'cascade' })
  internalRole!: InternalRole;

}

let orm: MikroORM;

beforeAll(async () => {
  orm = await MikroORM.init({
    dbName: ':memory:',
    entities: [InternalRole],
    debug: true,
  });
  await orm.schema.refreshDatabase();
});

afterAll(async () => {
  await orm.close(true);
});

test('4786', async () => {
  const a = orm.em.create(InternalRole, { name: 'role' });
  await orm.em.flush();
  orm.em.clear();

  let role = await orm.em.findOneOrFail(InternalRole, a);
  await orm.em.upsertMany(InternalRolePermission, [
    { subject: 'User', action: 'read', internalRole: role },
    { action: 'update', subject: 'User', internalRole: role },
  ]);

  orm.em.clear();
  role = await orm.em.findOneOrFail(InternalRole, a);
  await orm.em.upsertMany(InternalRolePermission, [
    { subject: 'User', action: 'read', internalRole: role },
    { action: 'update', subject: 'User', internalRole: role },
  ]);
});

@B4nan B4nan added the bug Something isn't working label Oct 5, 2023
@B4nan
Copy link
Member

B4nan commented Oct 5, 2023

Workaround is to provide the FK in your query:

await orm.em.upsertMany(InternalRolePermission, [
  { subject: 'User', action: 'read', internalRole: role.id }, // using `role.id` instead of just `role`
  { action: 'update', subject: 'User', internalRole: role.id },
]);

@tmikoss
Copy link
Author

tmikoss commented Oct 5, 2023

The returning "id", "created_at", "updated_at" caught my eye - those are the only columns defined on ApplicationEntity.

Looks like when metadata is fetched at

const meta = this.metadata.get(entityName);
, the comparableProps contains only columns from parent class. So getOnConflictReturningFields will not see the subject and name fields, and be unable to match data up with entities.

@B4nan
Copy link
Member

B4nan commented Oct 5, 2023

That's a wrong observation, as I said, the only issue to fix is the lookup of entities where you use a FK as part of a unique constraint. The returning statement won't contain all the properties, only those where we need the value. You provided values for both subject and name, there is no point in reloading those, as the point of upsert call is to make sure those are set to the values you provided. We only reload props without explicit value.

@tmikoss
Copy link
Author

tmikoss commented Oct 5, 2023

OK, I have very little understanding of the library internals, just trying to step through the code and highlight what "feels wrong", given that observable difference is the read-back query that used to include upserted data fields, but no longer does.

As for the workaround - yes, that works, thank you.

@B4nan
Copy link
Member

B4nan commented Oct 5, 2023

that used to include upserted data fields, but no longer does.

I kinda doubt that as well, it never included the values you provide, as we already know the value. You can see how the queries changed in this snapshot:

ab0ddee?w=1#diff-7b4e3cec759b75f447649d325800ca7180b41a5e3046c91c71c524b207dc1f96

@tmikoss
Copy link
Author

tmikoss commented Oct 5, 2023

I was referring to the read-back query - with 5.7.14 I was getting:

select "i0"."id", "i0"."created_at", "i0"."updated_at", "i0"."subject", "i0"."action", "i0"."internal_role_id" from "internal_role_permission" as "i0" where (("i0"."subject" = 'User' and "i0"."action" = 'read' and "i0"."internal_role_id" = '4adb342b-ac72-436e-b616-8b38b7a95d19') or ("i0"."subject" = 'User' and "i0"."action" = 'update' and "i0"."internal_role_id" = '4adb342b-ac72-436e-b616-8b38b7a95d19'))

whereas 5.8.6 executes this for the same code:

select "i0"."id", "i0"."created_at", "i0"."updated_at", "i0"."subject", "i0"."action", "i0"."internal_role_id" from "internal_role_permission" as "i0" where ("i0"."internal_role_id" = '68bf0a18-0cb4-4547-b386-6880a51f2327' or "i0"."internal_role_id" = '68bf0a18-0cb4-4547-b386-6880a51f2327')

It's possibly unrelated, maybe caused by some other change, but that's what originally stuck out to me.

@B4nan
Copy link
Member

B4nan commented Oct 5, 2023

Hmm, that's indeed weird, but for a different reason - there should be no such query, everything is already returned via the returning statements of the upsert query (unless you use onConflictAction: 'ignore', which effectively disables the returning statements).

edit: nah I was wrong with that take, this particular example results in do nothing as I just used the exact same values for both calls, and that also means no returning data provided, so that's why the select is there. but as you mentioned, the condition is wrong, it needs to contain the full composite key

@B4nan B4nan closed this as completed in 2f58556 Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants