Unamrshal (^`1.8.0`) for `time.Time` does not work

jaeheonji commented 7 months ago

Preflight Checklist

[X] I have searched the issue tracker for an issue that matches the one I want to file, without success.
[X] I am not looking for support or already pursued the available support channels without success.
[X] I have checked the troubleshooting guide for my problem, without success.

Viper Version

1.8.1

Go Version

1.21.4

Config Source

Environment variables

Format

No response

Repl.it link

No response

Code reproducing the issue

package main

import (
    "strings"
    "time"

    "github.com/k0kubun/pp/v3"
    "github.com/mitchellh/mapstructure"
    "github.com/spf13/viper"
)

type Example struct {
    String   string
    Int      int
    Duration time.Duration
    Time     time.Time
}

func main() {
    viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
    viper.AutomaticEnv()

    example := &Example{}

    viper.Unmarshal(example, viper.DecodeHook(
        mapstructure.ComposeDecodeHookFunc(
            mapstructure.StringToTimeDurationHookFunc(),
            mapstructure.StringToTimeHookFunc(time.RFC3339),
        ),
    ))

    pp.Println(example)
}

Expected Behavior

Run with Environment Variables

STRING="string" INT=10 DURATION="5ns" TIME="2000-01-01T00:00:00Z" go run main.go

Expected Output (stdout)

&main.Example{
  String:   "string",
  Int:      10,
  Duration: 5,
  Time:     2000-01-01 00:00:00 UTC,
}

Actual Behavior

&main.Example{
  String:   "string",
  Int:      10,
  Duration: 5,
  Time:     1-01-01 00:00:00 UTC,
}

Decode for Time type does not work properly.

Steps To Reproduce

No response

Additional Information

After version 1.8.0 was released, I confirmed new feature about Unmarshal. And in general types, it operates normally as expected above.

However, Time does not work properly. I looked for the reason and I think it related in the flattenAndMergeMap function.

https://github.com/spf13/viper/blob/9154b900c34ad9d88897f7e5288ce43f457f698b/viper.go#L2092-L2105

The reasons why this problem occurs are as follows:

As can be seen in the switch statement, when searching for a key in Struct to find an environment variable, it is recognized as a key only if the type is not map. (default case shadow[strings.ToLower(fullKey)] = true)
In the case of Time Struct, when inferring the type with value.(type), it is considered a map. Therefore, it is not considered the key value of an environment variable.

This problem does not occur if I set the value in advance through SetDefault. However, looking at #1429, it doesn't seem like the intention is to always use SetDefault. So I thought of this problem as a bug.

github-actions[bot] commented 7 months ago

👋 Thanks for reporting!

A maintainer will take a look at your issue shortly. 👀

In the meantime: We are working on Viper v2 and we would love to hear your thoughts about what you like or don't like about Viper, so we can improve or fix those issues.

⏰ If you have a couple minutes, please take some time and share your thoughts: https://forms.gle/R6faU74qPRPAzchZ9

📣 If you've already given us your feedback, you can still help by spreading the news, either by sharing the above link or telling people about this on Twitter:

https://twitter.com/sagikazarmark/status/1306904078967074816

Thank you! ❤️

ramkisiva commented 5 months ago

@jaeheonji
I have raised a PR #1740 with a fix for the issue you reported. Could you validate if the fix addresses the issue.

spf13 / viper