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

Incorrect DELETE_EARLY type #4895

Closed
zoltan-mihalyi opened this issue Nov 4, 2023 · 4 comments
Closed

Incorrect DELETE_EARLY type #4895

zoltan-mihalyi opened this issue Nov 4, 2023 · 4 comments
Labels
bug Something isn't working
Milestone

Comments

@zoltan-mihalyi
Copy link

zoltan-mihalyi commented Nov 4, 2023

Describe the bug
If an entity is created and an other is deleted in a transaction, and they have the same id, the other will have DELETE_EARLY ChangeSetType instead of DELETE.

To Reproduce

import { Entity, MikroORM, PrimaryKey } from '@mikro-orm/sqlite';

@Entity()
class A {

  @PrimaryKey()
  id!: string;

}

@Entity()
class B {

  @PrimaryKey()
  id!: string;

}

describe('GH issue 1346', () => {

  let orm: MikroORM;
  const types: string[][] = [];

  beforeAll(async () => {
    orm = await MikroORM.init({
      entities: [A, B],
      dbName: `:memory:`,
      subscribers: [{
        onFlush: args => {
          const changeSets = args.uow.getChangeSets();
          types.push(changeSets.map(cs => cs.type));
        },
      }],
    });

    await orm.schema.ensureDatabase();
  });

  beforeEach(async () => {
    await orm.schema.dropSchema();
    await orm.schema.createSchema();
  });

  afterAll(() => orm.close(true));

  test('preserve data fields that match pivot field', async () => {
    await orm.em.transactional(async em => {
      em.create(A, { id: '1a' });
      em.remove(em.getReference(B, '1a'));
    });

    await orm.em.transactional(async em => {
      em.create(A, { id: '2a' });
      em.remove(em.getReference(B, '2b'));
    });

    expect(types).toEqual([
      ['create', 'delete'],
      ['create', 'delete'],
    ]);
  });

});

Expected behavior
The following output:

[ 'create', 'delete_early' ]
[ 'create', 'delete' ]

Actual behavior
The following output:

[ 'create', 'delete' ]
[ 'create', 'delete' ]

Versions

Dependency Version
node 16.20.0
typescript 5.2.2
mikro-orm 5.9.2
sqlite 5.9.2
@B4nan
Copy link
Member

B4nan commented Nov 5, 2023

FYI delete_early is a valid change set type, there are cases where you will get that instead of just delete. Maybe I should simplify this in v6, so it's easier to work with it in the subscribers, we could have a flag instead of a separate changeset type, same for update_early.

But in this case it's indeed wrong, and I can see why, the check does not care about entity types. I hope you don't mind if I fix this in v6 only, I want to move forward with that today.

@B4nan B4nan added the bug Something isn't working label Nov 5, 2023
@B4nan B4nan added this to the 6.0 milestone Nov 5, 2023
@B4nan
Copy link
Member

B4nan commented Nov 5, 2023

Fixed in v6 via 9af367c

@B4nan B4nan closed this as completed Nov 5, 2023
@zoltan-mihalyi
Copy link
Author

Thank you!

A simple workaround is to check for delete_early type too.

@B4nan
Copy link
Member

B4nan commented Nov 5, 2023

Backported the fix to 5.x branch:

fef7a1b

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