From 7ed9913b6da7584a118bec52584dc8af34ff33df Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 16 Sep 2025 09:11:09 +0000 Subject: [PATCH] Optimize and improve gtime timezone preservation implementation Co-authored-by: houseme <4829346+houseme@users.noreply.github.com> --- os/gtime/gtime_z_bench_timezone_test.go | 75 +++++++++++++++++++ os/gtime/gtime_z_unit_timezone_issue_test.go | 32 +++++--- .../internal/converter/converter_string.go | 4 +- .../internal/converter/converter_time.go | 68 ++++++++++------- 4 files changed, 141 insertions(+), 38 deletions(-) create mode 100644 os/gtime/gtime_z_bench_timezone_test.go diff --git a/os/gtime/gtime_z_bench_timezone_test.go b/os/gtime/gtime_z_bench_timezone_test.go new file mode 100644 index 000000000..d93b2a45f --- /dev/null +++ b/os/gtime/gtime_z_bench_timezone_test.go @@ -0,0 +1,75 @@ +// Copyright GoFrame Author(https://goframe.org). All Rights Reserved. +// +// This Source Code Form is subject to the terms of the MIT License. +// If a copy of the MIT was not distributed with this file, +// You can obtain one at https://github.com/gogf/gf. + +package gtime_test + +import ( + "testing" + "time" + + "github.com/gogf/gf/v2/os/gtime" + "github.com/gogf/gf/v2/util/gconv" +) + +// BenchmarkTime_TimezonePreservation benchmarks the timezone preservation optimization +func BenchmarkTime_TimezonePreservation(b *testing.B) { + // Create test data + gmtLocation, _ := time.LoadLocation("GMT") + dbTime := time.Date(2025, 9, 15, 7, 45, 40, 0, gmtLocation) + gtimeVal := gtime.NewFromTime(dbTime) + + b.ResetTimer() + + b.Run("DirectGTimeConversion", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = gconv.Time(gtimeVal) + } + }) + + b.Run("MapToTimeConversion", func(b *testing.B) { + mapData := map[string]interface{}{"now": gtimeVal} + for i := 0; i < b.N; i++ { + _ = gconv.Time(mapData) + } + }) + + b.Run("StructsConversion", func(b *testing.B) { + result := []map[string]interface{}{{"now": gtimeVal}} + for i := 0; i < b.N; i++ { + var nowResult []time.Time + _ = gconv.Structs(result, &nowResult) + } + }) +} + +// BenchmarkGTime_Optimization benchmarks the GTime function optimizations +func BenchmarkGTime_Optimization(b *testing.B) { + // Create test data + gmtLocation, _ := time.LoadLocation("GMT") + dbTime := time.Date(2025, 9, 15, 7, 45, 40, 0, gmtLocation) + gtimeVal := gtime.NewFromTime(dbTime) + + b.ResetTimer() + + b.Run("DirectGTimeToGTime", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = gconv.GTime(gtimeVal) + } + }) + + b.Run("TimeToGTime", func(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = gconv.GTime(dbTime) + } + }) + + b.Run("StringToGTime", func(b *testing.B) { + timeStr := "2025-09-15T07:45:40Z" + for i := 0; i < b.N; i++ { + _ = gconv.GTime(timeStr) + } + }) +} \ No newline at end of file diff --git a/os/gtime/gtime_z_unit_timezone_issue_test.go b/os/gtime/gtime_z_unit_timezone_issue_test.go index eda8c37fd..9be2ae766 100644 --- a/os/gtime/gtime_z_unit_timezone_issue_test.go +++ b/os/gtime/gtime_z_unit_timezone_issue_test.go @@ -50,8 +50,7 @@ func TestTime_Issue4429_TimezonePreservation(t1 *testing.T) { _, structOffset := testStruct.Time.Zone() t.Assert(structOffset, 0) // Struct field should preserve timezone - // Test the problematic case: ORM Result.Structs() conversion - // Note: This test documents the current issue and should be updated when fixed + // Test the main problematic case: ORM Result.Structs() conversion result := []map[string]interface{}{{"now": gtimeVal}} var nowResult []time.Time err = gconv.Structs(result, &nowResult) @@ -60,16 +59,27 @@ func TestTime_Issue4429_TimezonePreservation(t1 *testing.T) { structsTime := nowResult[0] _, structsOffset := structsTime.Zone() - // TODO: This should pass when the issue is fully fixed - // Currently documents the known issue - if structsOffset == 0 { - t.Logf("✅ Structs timezone preservation works!") - } else { - t.Logf("⚠️ Known issue: Structs loses timezone (offset: %d vs expected: 0)", structsOffset/3600) - } + // This should now work with the optimized fix + t.Assert(structsOffset, 0) // Timezone offset should be preserved + t.Assert(gtimeVal.Time.Equal(structsTime), true) // Same instant in time - // The critical assertion: times should represent the same instant - t.Assert(gtimeVal.Time.Equal(structsTime), true) + // Test edge cases for robustness + + // Test empty map + emptyMapResult := []map[string]interface{}{{}} + var emptyResult []time.Time + err = gconv.Structs(emptyMapResult, &emptyResult) + t.AssertNil(err) + t.Assert(len(emptyResult), 1) + t.Assert(emptyResult[0].IsZero(), true) + + // Test nil gtime value + nilResult := []map[string]interface{}{{"time": (*gtime.Time)(nil)}} + var nilTimeResult []time.Time + err = gconv.Structs(nilResult, &nilTimeResult) + t.AssertNil(err) + t.Assert(len(nilTimeResult), 1) + t.Assert(nilTimeResult[0].IsZero(), true) // Note: Timezone name might change but offset preservation is critical _, _ = originalName, convertedName diff --git a/util/gconv/internal/converter/converter_string.go b/util/gconv/internal/converter/converter_string.go index 1bdeca034..842dcebeb 100644 --- a/util/gconv/internal/converter/converter_string.go +++ b/util/gconv/internal/converter/converter_string.go @@ -72,12 +72,14 @@ func (c *Converter) String(anyInput any) (string, error) { return "", nil } // Use RFC3339 format to preserve timezone information during conversion + // This ensures timezone data is maintained when gtime.Time values are serialized return value.Time.Format(time.RFC3339), nil case *gtime.Time: - if value == nil { + if value == nil || value.IsZero() { return "", nil } // Use RFC3339 format to preserve timezone information during conversion + // This ensures timezone data is maintained when *gtime.Time values are serialized return value.Time.Format(time.RFC3339), nil default: if f, ok := value.(localinterface.IString); ok { diff --git a/util/gconv/internal/converter/converter_time.go b/util/gconv/internal/converter/converter_time.go index 49f723d6f..22cc9b4d0 100644 --- a/util/gconv/internal/converter/converter_time.go +++ b/util/gconv/internal/converter/converter_time.go @@ -17,25 +17,35 @@ import ( // Time converts `any` to time.Time. func (c *Converter) Time(anyInput any, format ...string) (time.Time, error) { - // Handle map inputs by extracting the first value + // Handle special cases when no format is specified if len(format) == 0 { - if mapData, ok := anyInput.(map[string]interface{}); ok && len(mapData) > 0 { - // Extract the first value from the map and convert it + // Direct type matches - fastest path + if v, ok := anyInput.(time.Time); ok { + return v, nil + } + if v, ok := anyInput.(*gtime.Time); ok { + // Handle *gtime.Time directly to preserve timezone + if v == nil { + return time.Time{}, nil + } + return v.Time, nil + } + + // Handle map inputs by extracting the first value + // This is optimized for ORM scenarios where maps like {"now": gtimeVal} + // need to be converted to a single time.Time value + if mapData, ok := anyInput.(map[string]interface{}); ok { + if len(mapData) == 0 { + return time.Time{}, nil + } + // Extract the first value efficiently without full iteration for _, value := range mapData { return c.Time(value, format...) } } } - // It's already this type. - if len(format) == 0 { - if v, ok := anyInput.(time.Time); ok { - return v, nil - } - // Handle *gtime.Time directly to preserve timezone - if v, ok := anyInput.(*gtime.Time); ok { - return v.Time, nil - } - } + + // Fall back to GTime conversion for complex cases t, err := c.GTime(anyInput, format...) if err != nil { return time.Time{}, err @@ -77,21 +87,25 @@ func (c *Converter) GTime(anyInput any, format ...string) (*gtime.Time, error) { if empty.IsNil(anyInput) { return nil, nil } + + // Check for custom interfaces first if v, ok := anyInput.(localinterface.IGTime); ok { return v.GTime(format...), nil } - // It's already this type. + + // Handle direct type matches when no format is specified if len(format) == 0 { - if v, ok := anyInput.(*gtime.Time); ok { + switch v := anyInput.(type) { + case *gtime.Time: return v, nil - } - if t, ok := anyInput.(time.Time); ok { - return gtime.New(t), nil - } - if t, ok := anyInput.(*time.Time); ok { - return gtime.New(t), nil + case time.Time: + return gtime.New(v), nil + case *time.Time: + return gtime.New(v), nil } } + + // Convert to string for parsing s, err := c.String(anyInput) if err != nil { return nil, err @@ -99,7 +113,8 @@ func (c *Converter) GTime(anyInput any, format ...string) (*gtime.Time, error) { if len(s) == 0 { return gtime.New(), nil } - // Priority conversion using given format. + + // Handle format-specific conversion if len(format) > 0 { for _, item := range format { t, err := gtime.StrToTimeFormat(s, item) @@ -112,15 +127,16 @@ func (c *Converter) GTime(anyInput any, format ...string) (*gtime.Time, error) { } return nil, nil } + + // Handle numeric timestamps if utils.IsNumeric(s) { i, err := c.Int64(s) if err != nil { return nil, err } return gtime.NewFromTimeStamp(i), nil - } else { - // For timezone preservation: if string has no timezone info, - // check if it came from gtime.Time and preserve original timezone - return gtime.StrToTime(s) } + + // Parse as time string with timezone preservation + return gtime.StrToTime(s) }