r3labs / diff

A library for diffing golang structures
Mozilla Public License 2.0
895 stars 84 forks source link

unsupported type: MongoDB primitive.ObjectID #52

Open hwebb opened 3 years ago

hwebb commented 3 years ago

Will there be a support for the type primitive.ObjectID for MongoDB? Or is there a way to add types ourselves? https://pkg.go.dev/go.mongodb.org/mongo-driver

Right now I have to deactivate all of them with diff:"-", that wouldn't cause an issue if I was only using primitive.ObjectID as main ID, but I'm also using that to link collections, so I need it in other area on the struct.

type ObjectID [12]byte

fmt.Print(reflect.ValueOf(obj.ID).Kind()) // array unsupported type: array

It seems that you support Slice only and not Array

Besides that, it works well. Thanks for this library!

hjr265 commented 3 years ago

For one of my projects I have defined a custom value differ for ObjectIDs:

type objectIDDiffer struct{}

var (
    objectIDType = reflect.TypeOf(primitive.NilObjectID)
)

func (d objectIDDiffer) Match(a, b reflect.Value) bool {
    aok := a.Kind() == objectIDType.Kind() && a.Type() == objectIDType
    bok := b.Kind() == objectIDType.Kind() && b.Type() == objectIDType
    return aok && bok || a.Kind() == reflect.Invalid && bok || b.Kind() == reflect.Invalid && aok
}

func (d objectIDDiffer) Diff(cl *diff.Changelog, path []string, a, b reflect.Value) error {
    if a.Kind() == reflect.Invalid {
        cl.Add(diff.CREATE, path, nil, b.Interface())
        return nil
    }
    if b.Kind() == reflect.Invalid {
        cl.Add(diff.DELETE, path, a.Interface(), nil)
        return nil
    }
    av := a.Interface().(primitive.ObjectID)
    bv := b.Interface().(primitive.ObjectID)
    if av != bv {
        cl.Add(diff.UPDATE, path, av, bv)
    }
    return nil
}

func (d objectIDDiffer) InsertParentDiffer(dfunc func(path []string, a, b reflect.Value, p interface{}) error) {
}
differ, err = diff.NewDiffer(
    diff.CustomValueDiffers(objectIDDiffer{}),
)
hwebb commented 3 years ago

Thank you @hjr265 , so you think that type will never be included in this library? Because MongoDB is increasingly popular.

hjr265 commented 3 years ago

@hwebb That is up to the author of the package.

If it was me, I wouldn't add support for primitive.ObjectID. This package should be kept generic. Moreover, it already allows you to add custom differs for types that you need to support.

purehyperbole commented 3 years ago

Hi @hwebb, thanks for raising the issue.

I pretty much agree with what @hjr265 said. Directly supporting types from external libraries is outside the scope of this project as it would result in imposing a larger set of dependencies on our users.

We may be able to resolve your issue by adding support for array, but thats not a guaranteed fix as there may be other types in primitive.ObjectID that are unsupported.

As @hjr265 suggested, you are probably best of making use of the CustomValueDiffers for now.

I will try and take a look at adding support for arrays, as that may be helpfulfor other usecases.

hwebb commented 3 years ago

Thanks @purehyperbole thanks for the explanation, obviously adding dependencies is not a good option. I'll stick to the code from @hjr265 for now. Thanks

peterfajdiga commented 3 years ago

I had the same issue with my own struct that contains an array. As I was only using this library to debug something, I solved this by modifying my local vendored code of this library (I don't recommend doing this). I added

    case are(a, b, reflect.Array, reflect.Invalid):
        return d.diffSlice(path, a, b)

to this switch: https://github.com/r3labs/diff/blob/master/diff.go#L164

I'm posting this, because it seems to work correctly, and this may be all that's needed to support arrays. But I admit, I didn't delve much into the library's code, and the fix seems a bit too simple, so I'm probably missing something. That's why I didn't create a pull request.

Alberto-Cortes commented 4 weeks ago

Hello everyone, in case someone stumbles upon this issue, the above custom differ is outdated, leaving a new one:

type objectIDDiffer struct{}

var (
    objectIDType = reflect.TypeOf(primitive.NilObjectID)
)

func (d objectIDDiffer) Match(a, b reflect.Value) bool {
    aok := a.Kind() == objectIDType.Kind() && a.Type() == objectIDType
    bok := b.Kind() == objectIDType.Kind() && b.Type() == objectIDType
    return aok && bok || a.Kind() == reflect.Invalid && bok || b.Kind() == reflect.Invalid && aok
}

func (d objectIDDiffer) Diff(dt diff.DiffType, df diff.DiffFunc, cl *diff.Changelog, path []string, a, b reflect.Value, parent interface{}) error {
    if a.Kind() == reflect.Invalid {
        cl.Add(diff.CREATE, path, nil, b.Interface())
        return nil
    }
    if b.Kind() == reflect.Invalid {
        cl.Add(diff.DELETE, path, a.Interface(), nil)
        return nil
    }
    av := a.Interface().(primitive.ObjectID)
    bv := b.Interface().(primitive.ObjectID)
    if av != bv {
        cl.Add(diff.UPDATE, path, av, bv)
    }
    return nil
}

func (d objectIDDiffer) InsertParentDiffer(dfunc func(path []string, a, b reflect.Value, p interface{}) error) {
}

You can consume it like this:

    differ, err := diff.NewDiffer(
        diff.CustomValueDiffers(objectIDDiffer{}),
    )