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

Fix StringMap and StringsMap #139

Merged
merged 1 commit into from Mar 22, 2022
Merged

Fix StringMap and StringsMap #139

merged 1 commit into from Mar 22, 2022

Conversation

hoshsadiq
Copy link
Contributor

When calling StringMap, or StringsMap, and the ko.Get() returns
the value as expected, e.g. map[string]string for StringMap, the
type assertion for map[string]interface{} does not match. This
causes it to return an empty map even when there are actual values.

This commit addresses this by simply returning a copy if the type
assertion matches what we're supposed to return.

@hoshsadiq
Copy link
Contributor Author

@knadh I'm not sure where the place is to add a test case for it, as all the current test cases always retrieve map[string]interface{}. I figured adding a new test case just for this didn't make sense and thought it's worth checking first. Let me know where a good place is to add tests and I'd be happy to add them.

@knadh
Copy link
Owner

knadh commented Mar 14, 2022

Thanks for the PR @hoshsadiq.

When calling StringMap, or StringsMap, and the ko.Get() returns the value as expected, e.g. map[string]string for StringMap, the type assertion for map[string]interface{} does not match. This causes it to return an empty map even when there are actual values.

I'm a little confused by this. Could you share some example code that demonstrates this?

@hoshsadiq
Copy link
Contributor Author

Example:

package main

import (
	"fmt"
	"log"

	"github.com/knadh/koanf"
	"github.com/knadh/koanf/providers/structs"
)

type parentStruct struct {
	MapStrStr   map[string]string      `koanf:"mpss"`
	MapStrIface map[string]interface{} `koanf:"mpsi"`
}

func main() {
	in := parentStruct{
		MapStrStr: map[string]string{
			"hello": "test",
		},
		MapStrIface: map[string]interface{}{
			"hello": "test",
		},
	}

	var k = koanf.New(".")
	if err := k.Load(structs.Provider(in, "koanf"), nil); err != nil {
		log.Fatalf("error loading config: %v", err)
	}

	fmt.Println("mpss = ", k.StringMap("mpss"))
	fmt.Println("mpsi = ", k.StringMap("mpsi"))
}

Output:

mpss =  map[]
mpsi =  map[hello:test]

With this fix output is:

mpss =  map[hello:test]
mpsi =  map[hello:test]

When calling `StringMap`, or `StringsMap`, and the `ko.Get()` returns
the value as expected, e.g. `map[string]string` for `StringMap`, the
type assertion for `map[string]interface{}` does not match. This
causes it to return an empty map even when there are actual values.

This commit addresses this by simply returning a copy if the type
assertion matches what we're supposed to return.
@knadh
Copy link
Owner

knadh commented Mar 15, 2022

Looks good. @rhnvrm could you please review as well?

@rhnvrm
Copy link
Collaborator

rhnvrm commented Mar 21, 2022

LGTM

@knadh knadh merged commit f7f848b into knadh:master Mar 22, 2022
@hoshsadiq
Copy link
Contributor Author

Thanks both!

@hoshsadiq hoshsadiq deleted the fix-stringmap branch March 22, 2022 12:09
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

Successfully merging this pull request may close these issues.

None yet

3 participants