go-gorm / gorm

The fantastic ORM library for Golang, aims to be developer friendly
https://gorm.io
MIT License
36.66k stars 3.92k forks source link

AfterUpdate hooks fails for nil uuid pointer #7090

Open neiledgar opened 3 months ago

neiledgar commented 3 months ago

GORM Playground Link

https://github.com/go-gorm/playground/pull/747

Description

We use gorm hooks but we are seeing a problem when a field with type *uuid.UUID is updated with a nil. We expect the value passed to the AfterUpdate() hook to be nil but it is still populated with the original value. Note the database is updated with NULL as expected. It is the go struct in the AfterUpdate() hook that is incorrect.

This was working with gorm version v1.25.1 but is broken in gorm version 1.25.2. Specifically the breakage occurs with the commit 63534145fda9a2ac9ba703650b1a44da6a03e45e

The problem happens using Updates() with a pointer to uuid.UUID

var uuidPtr *uuid.UUID = nil
DB.Model(&p).Updates(map[string]interface{}{"unique_id": uuidPtr}).Error

The problem does not happen in the following scenarios

  1. when a string pointer is used

    var stringPtr *string = nil
    DB.Model(&p).Updates(map[string]interface{}{"name": stringPtr}).Error
  2. when the uuid.UUID pointer is a structure field

    
    type Product6 struct {
    ID       uint `gorm:"primarykey"`
    Name     *string
    UniqueId *uuid.UUID
    }

p = Product6{ID: p.ID, UniqueId: nil} DB.Model(&p).Updates(map[string]interface{}{"unique_id": p.UniqueId}).Error


3.  when nil interface is used

DB.Model(&p).Updates(map[string]interface{}{"unique_id": nil}).Error

omkar-foss commented 2 months ago

Hey @neiledgar so I checked this out, and upon digging deeper I found that the issue here is that after the completion of Updates(), the model object (&p in your case above) doesn't reflect the new nil value of the *uuid.UUID field, which is why you're unable to see the new nil value in AfterUpdates as well.

So the root cause of this issue is the inconsistency of the model object and not just AfterUpdates. The reason why this is happening is because this condition gets satisfied which simply returns without assigning the nil value, thus leaving the model object with the old value.

I've raised this PR to fix this issue, along with few test cases around the Updates() with map and uuid.UUID. Hope this will be helpful. Thanks.

neiledgar commented 2 months ago

Hey @omkar-foss. Thank you for the pr/fix. This has fixed the my issue and the playground test now works. I will integrate and test with our system.

neiledgar commented 2 months ago

Hey @omkar-foss. I can confirm can confirm this fixes my issue having pulled the PR into our system.

Thanks again

omkar-foss commented 2 months ago

@neiledgar Thank you so much for your confirmation that the fix is working well.

The PR is in review now (thanks to @a631807682) and when all concerns are resolved, it may get merged to master soon.