From 3b931a3a7a60f9bbb3caf1817048b8f88f5bccd9 Mon Sep 17 00:00:00 2001 From: Andrei Aaron Date: Mon, 24 Nov 2025 11:02:33 +0200 Subject: [PATCH] fix: handle zero time values in LastUpdated sorting functions (#3580) When GetRepoLastUpdated fails (e.g., redis: nil), it returns time.Time{} which gets converted to &time.Time{} (not nil). The existing nil checks in sorting functions didn't account for zero time values, which should also be treated as "oldest" (sorted last in descending order). This commit: - Adds zero time checks in addition to existing nil checks - Treats both nil and zero time values as "oldest" - Adds comprehensive tests for zero time edge cases Affects ImgSortByUpdateTime, RepoSortByUpdateTime, and GetExpandedRepoInfo sort functions. Signed-off-by: Andrei Aaron --- .../search/pagination/image_pagination.go | 15 +++--- .../search/pagination/pagination_test.go | 46 ++++++++++++++++++- .../search/pagination/repo_pagination.go | 15 +++--- pkg/extensions/search/resolver.go | 15 +++--- 4 files changed, 71 insertions(+), 20 deletions(-) diff --git a/pkg/extensions/search/pagination/image_pagination.go b/pkg/extensions/search/pagination/image_pagination.go index b87615dd..148f8303 100644 --- a/pkg/extensions/search/pagination/image_pagination.go +++ b/pkg/extensions/search/pagination/image_pagination.go @@ -163,17 +163,20 @@ func ImgSortByRelevance(a, b *gql_gen.ImageSummary) int { //nolint:varnamelen // // ImgSortByUpdateTime sorts descending by image update time. func ImgSortByUpdateTime(a, b *gql_gen.ImageSummary) int { //nolint:varnamelen // standard comparison func signature - // Handle nil cases: nil values are treated as oldest (come last in descending sort) - if a.LastUpdated == nil && b.LastUpdated == nil { + // Handle nil and zero time cases: both are treated as oldest (come last in descending sort) + aIsZero := a.LastUpdated == nil || (a.LastUpdated != nil && a.LastUpdated.IsZero()) + bIsZero := b.LastUpdated == nil || (b.LastUpdated != nil && b.LastUpdated.IsZero()) + + if aIsZero && bIsZero { return 0 } - if a.LastUpdated == nil { - return 1 // a is nil, b is not - a comes after b + if aIsZero { + return 1 // a is zero/nil, b is not - a comes after b } - if b.LastUpdated == nil { - return -1 // b is nil, a is not - a comes before b + if bIsZero { + return -1 // b is zero/nil, a is not - a comes before b } if a.LastUpdated.After(*b.LastUpdated) { diff --git a/pkg/extensions/search/pagination/pagination_test.go b/pkg/extensions/search/pagination/pagination_test.go index 78767112..28849da5 100644 --- a/pkg/extensions/search/pagination/pagination_test.go +++ b/pkg/extensions/search/pagination/pagination_test.go @@ -49,25 +49,46 @@ func TestImgSumPagination(t *testing.T) { So(pagination.ImgSortByRelevance(image1, image2), ShouldEqual, -1) }) - Convey("ImgSortByUpdateTime with nil LastUpdated", func() { + Convey("ImgSortByUpdateTime with nil and zero LastUpdated", func() { time1 := time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC) time2 := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC) + zeroTime := time.Time{} // Both nil - should be equal image1 := &gql_generated.ImageSummary{RepoName: ref("repo1"), Tag: ref("tag1"), LastUpdated: nil} image2 := &gql_generated.ImageSummary{RepoName: ref("repo2"), Tag: ref("tag2"), LastUpdated: nil} So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 0) + // Both zero time - should be equal (treated same as nil) + image1.LastUpdated = &zeroTime + image2.LastUpdated = &zeroTime + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 0) + + // a is nil, b is zero - should be equal (both treated as oldest) + image1.LastUpdated = nil + image2.LastUpdated = &zeroTime + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 0) + // a is nil, b is not - a should come after b (return 1) image1.LastUpdated = nil image2.LastUpdated = &time1 So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 1) + // a is zero, b is not - a should come after b (return 1) + image1.LastUpdated = &zeroTime + image2.LastUpdated = &time1 + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, 1) + // b is nil, a is not - a should come before b (return -1) image1.LastUpdated = &time1 image2.LastUpdated = nil So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, -1) + // b is zero, a is not - a should come before b (return -1) + image1.LastUpdated = &time1 + image2.LastUpdated = &zeroTime + So(pagination.ImgSortByUpdateTime(image1, image2), ShouldEqual, -1) + // Both non-nil - normal comparison (a is newer, should come first in descending sort) image1.LastUpdated = &time1 image2.LastUpdated = &time2 @@ -393,25 +414,46 @@ func TestPagination(t *testing.T) { So(pagination.RepoSortByDownloads(repo1, repo1), ShouldEqual, 0) }) - Convey("RepoSortByUpdateTime with nil LastUpdated", func() { + Convey("RepoSortByUpdateTime with nil and zero LastUpdated", func() { time1 := time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC) time2 := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC) + zeroTime := time.Time{} // Both nil - should be equal repo1 := &gql_generated.RepoSummary{Name: ref("repo1"), LastUpdated: nil} repo2 := &gql_generated.RepoSummary{Name: ref("repo2"), LastUpdated: nil} So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 0) + // Both zero time - should be equal (treated same as nil) + repo1.LastUpdated = &zeroTime + repo2.LastUpdated = &zeroTime + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 0) + + // a is nil, b is zero - should be equal (both treated as oldest) + repo1.LastUpdated = nil + repo2.LastUpdated = &zeroTime + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 0) + // a is nil, b is not - a should come after b (return 1) repo1.LastUpdated = nil repo2.LastUpdated = &time1 So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 1) + // a is zero, b is not - a should come after b (return 1) + repo1.LastUpdated = &zeroTime + repo2.LastUpdated = &time1 + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, 1) + // b is nil, a is not - a should come before b (return -1) repo1.LastUpdated = &time1 repo2.LastUpdated = nil So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, -1) + // b is zero, a is not - a should come before b (return -1) + repo1.LastUpdated = &time1 + repo2.LastUpdated = &zeroTime + So(pagination.RepoSortByUpdateTime(repo1, repo2), ShouldEqual, -1) + // Both non-nil - normal comparison (a is newer, should come first in descending sort) repo1.LastUpdated = &time1 repo2.LastUpdated = &time2 diff --git a/pkg/extensions/search/pagination/repo_pagination.go b/pkg/extensions/search/pagination/repo_pagination.go index 5ec2ae5d..898965d2 100644 --- a/pkg/extensions/search/pagination/repo_pagination.go +++ b/pkg/extensions/search/pagination/repo_pagination.go @@ -152,17 +152,20 @@ func RepoSortByRelevance(a, b *gql_gen.RepoSummary) int { // RepoSortByUpdateTime sorts descending by time. func RepoSortByUpdateTime(a, b *gql_gen.RepoSummary) int { //nolint:varnamelen // standard comparison func signature - // Handle nil cases: nil values are treated as oldest (come last in descending sort) - if a.LastUpdated == nil && b.LastUpdated == nil { + // Handle nil and zero time cases: both are treated as oldest (come last in descending sort) + aIsZero := a.LastUpdated == nil || (a.LastUpdated != nil && a.LastUpdated.IsZero()) + bIsZero := b.LastUpdated == nil || (b.LastUpdated != nil && b.LastUpdated.IsZero()) + + if aIsZero && bIsZero { return 0 } - if a.LastUpdated == nil { - return 1 // a is nil, b is not - a comes after b + if aIsZero { + return 1 // a is zero/nil, b is not - a comes after b } - if b.LastUpdated == nil { - return -1 // b is nil, a is not - a comes before b + if bIsZero { + return -1 // b is zero/nil, a is not - a comes before b } if a.LastUpdated.After(*b.LastUpdated) { diff --git a/pkg/extensions/search/resolver.go b/pkg/extensions/search/resolver.go index 4719007a..84cd796e 100644 --- a/pkg/extensions/search/resolver.go +++ b/pkg/extensions/search/resolver.go @@ -1442,17 +1442,20 @@ func expandedRepoInfo(ctx context.Context, repo string, metaDB mTypes.MetaDB, cv //nolint:varnamelen // standard comparison func signature slices.SortFunc(dateSortedImages, func(a, b *gql_generated.ImageSummary) int { - // Handle nil cases: nil values are treated as oldest (come last in descending sort) - if a.LastUpdated == nil && b.LastUpdated == nil { + // Handle nil and zero time cases: both are treated as oldest (come last in descending sort) + aIsZero := a.LastUpdated == nil || (a.LastUpdated != nil && a.LastUpdated.IsZero()) + bIsZero := b.LastUpdated == nil || (b.LastUpdated != nil && b.LastUpdated.IsZero()) + + if aIsZero && bIsZero { return 0 } - if a.LastUpdated == nil { - return 1 // a is nil, b is not - a comes after b + if aIsZero { + return 1 // a is zero/nil, b is not - a comes after b } - if b.LastUpdated == nil { - return -1 // b is nil, a is not - a comes before b + if bIsZero { + return -1 // b is zero/nil, a is not - a comes before b } if a.LastUpdated.After(*b.LastUpdated) {