Pointers not being mapped correctly with only ENV variables

[afdecastro879]

Hi,

First thanks for the contribution.
I'm checking this package becase I've experienced the issue with viper, however seems like the example you provide in the documentation does not work as expected when there's no configuration file that fills values for pointers and maps.

If I execute the example without any config file (just adding environment values) the value for Barries map[string]barry and Quxxy *quxxy is allways nil. I have pushed some code in a personal repo to illustrate this issue: https://github.com/afdecastro879/enviper-test If you run the main program you will find that I'm setting the environment variables like this:

	os.Setenv("FOO", "foo")
	os.Setenv("BARRY_BAR", "42")
	os.Setenv("BAZ", "true")
	os.Setenv("BARRIES_KEY1_BAR", "42")
	os.Setenv("QUXXY_QUX", "ipsum")

But when printing the struct the result for the values explained above are nil:

main.config{Foo:"foo", Barry:main.barry{Bar:42}, Barries:map[string]main.barry(nil), Bazzy:main.bazzy{Baz:true}, Quxxy:(*main.quxxy)(nil)}

[iamolegga]

Hi, thanks for reporting. I'll try to find some time to check this

[jguttman94]

any news on this @iamolegga?

[jguttman94]

@iamolegga, took a stab at a fix on this. Let me know what your thoughts are!

[iamolegga]

@jguttman94 as I've said before in your PR, everything looks good, but there is only one bug with map. Could you please check this branch and maybe help to fix that?

[jguttman94]

@iamolegga, I took a look, and I'll be honest, I'm struggling with this one...

I believe this to be a separate issue that is more challenging to address. The issue with the unmarshalling of the pointers was easier to solve as the reflection used in the recursive loop over the raw configuration value just needed knowledge of a new pointer element. With that provided, and without mutating the original raw config value, the rest of the recursion loop was able to successfully destructure the environment variable into the pointer.

For the maps, it's a little different. It's easy enough to create a new map if the raw config value is nil, but the rest of the logic is dependent on iterating over each provided map value and recursively invoking bindEnvs. However, because we need to create a new instance of the map, there are no values to iterate over; that isn't known until bindEnvs returns and e.Viper.Unmarshal is invoked.

I may need some extra support on this one, as I'm quite new to golang reflection.

Here is some example code I was playing around with to create a new map in bindEnvs:

case reflect.Map:
  if fv.IsNil() {
    var mapType = reflect.MapOf(fv.Type().Key(), fv.Type().Elem())
    fv = reflect.MakeMap(mapType)
  }
  iter := fv.MapRange()
  for iter.Next() { // failing to destructure here, as the new map is empty, and we do not know the potential contents at this time
    if key, ok := iter.Key().Interface().(string); ok {
      e.bindEnvs(iter.Value().Interface(), append(prev, tv, key)...)
    }
  }

[iamolegga]

Thanks for the investigation @jguttman94

Let's move forward then and I'll change the test case for only env variables without config file to pass tests. Anyway, I believe this will work for 90% of usage and as you've said we can simply open a new issue for that.

[iamolegga]

Just made a release v1.3.0 with basic support of unmarshaling only from env vars, and opened an issue for map #9

[jguttman94]

Thanks for the release, @iamolegga. Just verified it works within my cobra CLI!