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

bool custom Transformer for non zero-value false #89

Closed
mberlanda opened this issue Sep 24, 2018 · 9 comments
Closed

bool custom Transformer for non zero-value false #89

mberlanda opened this issue Sep 24, 2018 · 9 comments

Comments

@mberlanda
Copy link

hi @iamdario,

I recently introduced mergo on one of our projects.

I came up with an issue very similar to #24 :

import (
  "testing"
  "github.com/imdario/mergo"
  "github.com/stretchr/testify/assert"
)
func TestBoolean(t *testing.T) {
  type Foo struct {
    Bar bool `json:"bar"`
  }

  f1 := Foo{Bar: true}
  f2 := Foo{Bar: false}

  mergo.Merge(&f2, f1)
  assert.False(t, f2.Bar)
}

Do you think it could be handled with some custom transformers?

@mberlanda mberlanda changed the title boolean Transformer bool custom Transformer for non zero-value false Sep 24, 2018
@hectorj2f
Copy link

Is there any progress on this ? I am facing the same issue

@luisdavim
Copy link

I think this is the same issue reported in Praqma/helmsman#137

@svyotov
Copy link
Contributor

svyotov commented Mar 6, 2019

As far as I can see this is because of https://github.com/imdario/mergo/blob/9316a62528ac99aaecb4e47eadd6dc8aa6533d58/mergo.go#L36-L59
Merging Boolean and Numeric values that are false or zero are considered empty thus replaced.

@github4jiawen
Copy link

This can be accomplished with custom bool and transformer feature

package main

import (
	"encoding/json"
	"log"
	"reflect"

	"github.com/imdario/mergo"
)

type BoolInSetting struct {
	Set   bool
	Value bool
}

func (b *BoolInSetting) UnmarshalJSON(data []byte) error {
	b.Set = true

	err := json.Unmarshal(data, &b.Value)
	return err
}

type BoolInSettingTransformer struct {
}

func (t BoolInSettingTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
	if (typ == reflect.TypeOf(BoolInSetting{})) {
		return func(dst, src reflect.Value) error {
			if dst.CanSet() && src.FieldByName("Set").Bool() {
				dst.Set(src)
			}
			return nil
		}
	}
	return nil
}

type ConfigWithBool struct {
	BooleanField BoolInSetting `json:"BooleanField"`
}

func main() {
	var trusy ConfigWithBool
	var falsy ConfigWithBool
	var err error
	err = json.Unmarshal([]byte(`{"BooleanField": true}`), &trusy)
	if err != nil {
		log.Fatalln(err)
	}
	err = json.Unmarshal([]byte(`{"BooleanField": false}`), &falsy)
	if err != nil {
		log.Fatalln(err)
	}
	err = mergo.Merge(&trusy, falsy, mergo.WithTransformers(BoolInSettingTransformer{}))
	if err != nil {
		log.Fatalln(err)
	}
	log.Printf("%+v", trusy)
}

@vilipwong
Copy link

Hello, is mergo.WithOverride or mergo.WithOverwriteWithEmptyValue suppose to solve this ?

I faced the same issue with string and boolean value, using mergo.WithOverwriteWithEmptyValue e.g:

p1 := map[string]interface{}{
	"A": 3, "B": "note", "C": true,
}
p2 := map[string]interface{}{
	"B": "", "C": false,
}
mergo.Merge(&p1, &p2, mergo.WithOverwriteWithEmptyValue)

Expected result:
{A:3 B:"" C:false}

Actual result:
{A:3 B:"note" C:true}

darccio added a commit that referenced this issue May 17, 2020
@darccio
Copy link
Owner

darccio commented May 17, 2020

Ok, let's see. The OP's example expects f2.Bar to be false, and it is the destination variable, so it's always truthy.

I rewrote the example with more meaningful variable names, as it was misleading me to the opposite direction:

import (
  "testing"
  "github.com/imdario/mergo"
  "github.com/stretchr/testify/assert"
)
func TestBoolean(t *testing.T) {
  type Foo struct {
    Bar bool `json:"bar"`
  }

  f1 := Foo{Bar: true}
  f2 := Foo{Bar: false}

  mergo.Merge(&f2, f1)
  assert.False(t, f2.Bar)
}

Your intention, from what I understood, is to keep the false value as is. Not sure why as I'm missing context here. I'm going to tag along and restate what I said in #24:

This is the expected behavior. As stated in README:

Mergo is intended to assign only zero value fields on destination with source value.

Provided this, your example won't work with the current implementation. If you need to protect false values, you must follow @github4jiawen example. Transformers are a simple way to filter the main behavior.

About your case, @vilipwong, I pushed another test to check it out, and I confirm you that mergo.WithOverwriteWithEmptyValue will help you. Take into account that you have the opposite problem, as you want to override a truthy value with false.

@darccio darccio closed this as completed May 17, 2020
@vilipwong
Copy link

I see. i was using release v0.3.9, it works when i go get @master. Any reason why its not released ?
@imdario

@darccio
Copy link
Owner

darccio commented May 22, 2020

@vilipwong I want to do the release with some more fixes from v0.3.9 related bugs.

darccio added a commit that referenced this issue Jul 17, 2020
commit 64219f269280df8ad7c8abf48c3a33697154e70a
Author: Dario Castañé <d@rio.hn>
Date:   Sat Jul 18 00:11:30 2020 +0200

    Issue #123 reverted

commit 1c0f4c5e64026215964a7365cc2da1750bf44f31
Author: János Pásztor <janoszen@users.noreply.github.com>
Date:   Mon Jun 8 08:42:20 2020 +0200

    Added janoszen/containerssh as  a mergo user

commit 36e7eb818ccb22432f4c0824f820c5c832b70538
Author: Dario Castañé <d@rio.hn>
Date:   Sat Jul 18 00:09:18 2020 +0200

    Fatal* replaced by Error* functions

commit a3badb1749efbc887cf881cf78eba881c0864773
Author: Dario Castañé <i@dario.im>
Date:   Sun May 17 16:59:56 2020 +0200

    Improved semantics of tests for issue #129

commit e7166a5d1ccf985ad9077a4ce23edc0fae0d7378
Author: Dario Castañé <i@dario.im>
Date:   Sun May 17 16:52:27 2020 +0200

    Issue #129 closed

commit 4c6b27f7c7eacc5575f1eb454419258e8c92c2d1
Author: Dario Castañé <i@dario.im>
Date:   Sun May 17 16:40:14 2020 +0200

    Issue #89 closed

commit 293f485af2dce978b4fd27b65789b5e4a92df4fd
Author: Dario Castañé <i@dario.im>
Date:   Sun May 17 16:36:55 2020 +0200

    Improved name for the final condition to set

commit a4db553223c3a1ba8de4035e1110133ec3c5ae48
Author: Dario Castañé <i@dario.im>
Date:   Sun May 17 16:06:36 2020 +0200

    Issue #138 fixed

commit 8583b70567e0f59382ca76f0bbe999470e8982df
Author: Dario Castañé <i@dario.im>
Date:   Sun May 17 14:58:37 2020 +0200

    Issue #136 resolved

commit f34173f37dbe977477feb95cf3cba1cfb3919630
Author: Dario Castañé <d@rio.hn>
Date:   Fri Jul 17 23:50:34 2020 +0200

    Issue #131 fixed again

commit 1fdd4e8dbae7c1b8c88d550aab142f32bc3d0c86
Author: Dario Castañé <i@dario.im>
Date:   Sun May 17 14:47:55 2020 +0200

    Issue 131 fixed

commit 2fbb87d2f9fbdb47fdf6d2d4274ec8a05ca698c2
Author: Tariq Ibrahim <tariq181290@gmail.com>
Date:   Thu Apr 23 12:49:33 2020 -0700

    add release badge to track latest release of mergo

commit 815b47be4da2a8dcb87ab36544466b89c1c0e40d
Author: Eyal <eyalsoha@google.com>
Date:   Tue Apr 14 08:57:57 2020 -0400

    Fix typo transfomer -> transformer

commit 6ed75f818a997495ab9eb9d474f4bc13bad3e5bc
Author: Dario Castañé <dcastane@loyal.guru>
Date:   Fri Mar 27 08:34:25 2020 +0100

    README updated to last release v0.3..9

commit 9b0f1c7e5c418675c7f9d1ab588b8bf6b29ac1ff
Author: Dario Castañé <d@rio.hn>
Date:   Fri Jul 17 23:28:50 2020 +0200

    Improved semantics for mergeable fields

commit cf80c4cbd6102982dd91eb0ded30558692a3f43d
Author: Dario Castañé <d@rio.hn>
Date:   Fri Jul 17 23:18:59 2020 +0200

    Faulty test removed

commit 5a49eb5583bd3ebdae0afe011a66ae5365196918
Author: Dario Castañé <d@rio.hn>
Date:   Fri Jul 17 23:10:01 2020 +0200

    Working on CI fixes and v0.3.9 bugs

commit b75bbeef3b546d3fb916271538adc25bbde20f8d
Author: Dario Castañé <dcastane@loyal.guru>
Date:   Thu Mar 26 22:43:59 2020 +0100

    Dead code removed

commit c08f60a65adf317d234f1f5eb51572c30c9b87d0
Author: Dario Castañé <dcastane@loyal.guru>
Date:   Thu Mar 26 22:43:50 2020 +0100

    Destination pointer type check added to map

commit e5e7507289bba3e635e6a9df7671d58915ac0487
Author: Stefan Bourlon <s.bourlon@gmail.com>
Date:   Mon Feb 3 11:51:09 2020 -0800

    merge: test dst is a pointer

    Return an error instead of panicking if merge receives dst != pointer

commit b8a2f9bd2feb2110ee3cb550372cc4105ebcaa99
Author: Umair Idris <umair.m.idris@gmail.com>
Date:   Mon Jan 20 05:21:09 2020 -0500

    Fix lint (#135)

commit 3cebbeca61cb9226249551e9c8afc24bf64fe559
Author: komalsinha-g <59949151+komalsinha-g@users.noreply.github.com>
Date:   Fri Jan 17 14:36:55 2020 +0530

    Make "OverwriteWithEmptyValue" in config struct to be set by any method. (#133)

    Make "OverwriteWithEmptyValue" in config struct to be set by any method.

    this make the merge more customizable. In cases, where request toggles a boolean value and the the request message can change a "true" value to false we can pass the flag to merge accordingly, which is not possible otherwise.

commit f444b9dcd2f5e66f8572b474b465564ba4f54240
Author: Dario Castañé <i@dario.im>
Date:   Wed Jan 1 22:27:24 2020 +0100

    DeepSource support

commit a6318969156f71a80c41afd42104c53f239ff23a
Author: Dario Castañé <i@dario.im>
Date:   Wed Jan 1 22:25:43 2020 +0100

    Assert removed

commit 6c2f66f933a1741f448e437dea3cb491a7c979d8
Author: Peer Xu <pppeerxu@gmail.com>
Date:   Sat Oct 12 10:35:47 2019 +0800

    Fixed typo

commit 8993363a0c7c25aec36e0da04e82d4988d6cc96b
Author: Peer Xu <pppeerxu@gmail.com>
Date:   Sat Oct 12 10:26:25 2019 +0800

    added WithSliceDeepCopy config flag
@TomOperator
Copy link

For those of you who need it, @Pothulapati built a fork that solves this here.

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

8 participants