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

AfterUpdate subscriber bug #1705

Closed
btwotwo opened this issue Mar 6, 2018 · 20 comments
Closed

AfterUpdate subscriber bug #1705

btwotwo opened this issue Mar 6, 2018 · 20 comments

Comments

@btwotwo
Copy link

btwotwo commented Mar 6, 2018

Issue type:

[ ] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[x] postgres
[ ] sqlite
[ ] sqljs
[ ] websql

TypeORM version:

[ ] latest
[ ] @next
[x] 0.1.9 (or put your version here)

Steps to reproduce or a small repository showing the problem:

  1. Create entity.
  2. Add method with @AfterUpdate decorator to it.
  3. Update entity (with getRepository(Entity).save()).

Expected behaviour:
When method with @AfterUpdate runs, I can get updated entity from database.

Actual behaviour:
Here's what actually happens:

  1. Transaction is created (executing query: START TRANSACTION)
  2. UPDATE query is executed (executing query: UPDATE "entity" SET "something"=$1 WHERE "id"=$2 -- PARAMETERS: [1,2])
  3. Method with @AfterUpdate is called.
  4. Transaction is commited.

So, when @AfterUpdate method is called, entity in the database has not been updated yet. Therefore, I can not get updated entities from the database.

@pleerock
Copy link
Member

pleerock commented Mar 9, 2018

It is expected. If something is failing in AfterUpdate transaction should be reverted, it is by design.

@btwotwo
Copy link
Author

btwotwo commented Mar 12, 2018

@pleerock so, is there any way to notify listeners that something was updated or inserted?

@pleerock
Copy link
Member

Listeners and subscribers already do it.

@btwotwo
Copy link
Author

btwotwo commented Mar 14, 2018

@pleerock they are notified before transaction is commited. I need to notify listeners when the data is already in the database. How to do it proprerly?

@pleerock
Copy link
Member

pleerock commented Mar 14, 2018

Depend of your needs.

You can call your method without listeners simply after you call save method.

But I guess you need a data that inserted but not yet committed, in this case you need to use same entityManager instance (transactional entity manager), for this purpose you need to use subscribers instead of listeners since subscribers pass you event object with entityManager inside you'll need to use to get the data using transactional entity manager and you'll have what you need. Just try.

@btwotwo
Copy link
Author

btwotwo commented Mar 15, 2018

But I guess you need a data that inserted but not yet committed

No, I need a confirmation that data is updated AND it is in the database. AfterUpdate confirms that data is updated, but not that it is in the database.

@pleerock
Copy link
Member

Thats what I was talking about. You need to use same entityManager. Try and let me know if you have problems.

@DavidHooper
Copy link

I think this behaviour should be documented. Coming from a framework like Rails, the callbacks happen after the transaction has been committed.

@didac-wh
Copy link

didac-wh commented Jan 25, 2019

I don't get it.
I'm using a subscriber, and as shown in the image:
https://i.imgur.com/sYmjVCK.png
"Called after entity is updated in the database"

Well, in my ElasticSearch update function I use a parent of this entity, and it updates that parent (and some relations, including my current entity) but the values I get in that process are the old ones. Even if I call entity.reload(). Even if I call "await repository.findOne(entity.id)" to re-assign the entity value.

My thinking is that it's not only undocumented in the docs, but its also missdocumented according to the TypeScript definition.

For a solution, I ended up overriding the save() method for my entity.

async save() : Promise<this> {
    await super.save();
    await this.updateElasticSearch();

    return this;
  }

It seems to work so far.

@cjuega
Copy link

cjuega commented May 7, 2019

I don't understand why this has been closed. I believe this is still a needed feature. As I see it, the two behaviors should be supported:

  • Listeners/Subscribers being called before the transaction is committed (current behavior)
  • Listeners/Subscribers being called after the transaction is committed. I can't think of a better example as what @didac-wh shown. Sync'ing entries on different database engines (like elasticsearch) must be done after transactions are committed. Otherwise, if the transaction fails for any reason, both databases won't be sync'ed anymore.

Is there any plan to support this in the future? I see it was requested at #743 too? are PR implementing this welcome?

@didac-wh, did overwriting save method work when the transaction did fail? I mean, I would expect this.updateElasticSearch() not being called in your example.

@shusson
Copy link

shusson commented May 8, 2019

@cjuega have you tried to use a subscriber as described here: #3563 (comment) ?

@haschu
Copy link
Contributor

haschu commented May 31, 2019

@pleerock Wouldn't it be more convenient to introduce a afterUpdateCommit hook? There might be some rare use-case (I think I stumbled into one of them), where the above solutions wouldn't work.

@daweedm
Copy link

daweedm commented Aug 8, 2019

any updated about this ? would be it possible to have the same InsertEvent or UpdateEvent object passed to callback decorated with @AfterUpdate and so on ?

@jackbilesisdm
Copy link

Can this please be documented in the relevant documentation. It's not super clear on how to monitor this.

For reference when people find them selves here

async afterUpdate(entity: UpdateEvent<Master>) {
        
        var x = await entity.manager.find(Master)
        console.log(x)
    }

@itbsc
Copy link

itbsc commented May 23, 2021

UpdateEvent.entity is undefined in afterUpdate. How does one get the updated entity in the subscriber?

@raphaeltm
Copy link

I'm not sure if this is the best solution, but for anyone else who comes across this issue, this is what fixed it for us. We just added the following lines at the beginning of the afterUpdate function:

    await event.queryRunner.commitTransaction();
    await event.queryRunner.startTransaction();

In our case, the issue was that we were sending IDs to a queue to trigger search indexing, and the queue was being processed before the transaction was committed (though I found that surprising). So data in our search index was always one step behind the actual data.

For most use cases I can imagine, I think the naming of the subscriber is misleading. To me, afterUpdate implies that it will be called after everything to do with the update is complete. Of course at this point, if people rely on this behaviour it's probably not changeable, but I would have expected something like updatePreCommit or something like that for the current behaviour.

@sizarray
Copy link

Still not fixed

@mxvincent
Copy link

It's not fixed and the documentation does not explain the relation between transactions and subscription events.

@HassanAmed
Copy link

This issue still exists. It would be nice to have an event that emit after the transaction is committed to cater to rest of use cases as well

@wdprice
Copy link

wdprice commented Jan 15, 2024

Posting to agree that the name and behavior of this event is unintuitive and not well documented. An event after the transaction commits would be hugely helpful in many circumstances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests