mirror of
https://github.com/project-zot/zot.git
synced 2026-06-17 21:17:58 +08:00
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 <andreifdaaron@gmail.com>
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user