Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Commit

Permalink
make image sort stable, and correct
Browse files Browse the repository at this point in the history
The ByCreatedDesc sort was broken in two ways:

1. If there were two images with a zero CreatedAt time, the sort order
would depend on the order in which they appeared in the
orginal (unsorted) list, thus making the order unstable.

2. Images with a zero CreatedAt time were considered more recent than
any images with a non-zero CreatedAt time, which is the exact opposite
of what we want.

These bugs _may_ be the cause of the 'flipping' between different
images we are seeing in #1127.
  • Loading branch information
rade committed Jul 24, 2018
1 parent 63dc183 commit aca71d8
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 14 deletions.
10 changes: 2 additions & 8 deletions image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,14 +282,8 @@ type ByCreatedDesc []Info
func (is ByCreatedDesc) Len() int { return len(is) }
func (is ByCreatedDesc) Swap(i, j int) { is[i], is[j] = is[j], is[i] }
func (is ByCreatedDesc) Less(i, j int) bool {
switch {
case is[i].CreatedAt.IsZero():
return true
case is[j].CreatedAt.IsZero():
return false
case is[i].CreatedAt.Equal(is[j].CreatedAt):
if is[i].CreatedAt.Equal(is[j].CreatedAt) {
return is[i].ID.String() < is[j].ID.String()
default:
return is[i].CreatedAt.After(is[j].CreatedAt)
}
return is[i].CreatedAt.After(is[j].CreatedAt)
}
27 changes: 21 additions & 6 deletions image/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,28 @@ func TestImageInfoCreatedAtZero(t *testing.T) {
func TestImage_OrderByCreationDate(t *testing.T) {
time0 := testTime.Add(time.Second)
time2 := testTime.Add(-time.Second)
imA := mustMakeInfo("my/Image:3", testTime)
imB := mustMakeInfo("my/Image:1", time0)
imC := mustMakeInfo("my/Image:4", time2)
imD := mustMakeInfo("my/Image:0", time.Time{}) // test nil
imE := mustMakeInfo("my/Image:2", testTime) // test equal
imgs := []Info{imA, imB, imC, imD, imE}
imA := mustMakeInfo("my/Image:2", testTime)
imB := mustMakeInfo("my/Image:0", time0)
imC := mustMakeInfo("my/Image:3", time2)
imD := mustMakeInfo("my/Image:4", time.Time{}) // test nil
imE := mustMakeInfo("my/Image:1", testTime) // test equal
imF := mustMakeInfo("my/Image:5", time.Time{}) // test nil equal
imgs := []Info{imA, imB, imC, imD, imE, imF}
sort.Sort(ByCreatedDesc(imgs))
checkSorted(t, imgs)
// now check stability
sort.Sort(ByCreatedDesc(imgs))
checkSorted(t, imgs)
// more stability checks
for i := len(imgs)/2 - 1; i >= 0; i-- {
opp := len(imgs) - 1 - i
imgs[i], imgs[opp] = imgs[opp], imgs[i]
}
sort.Sort(ByCreatedDesc(imgs))
checkSorted(t, imgs)
}

func checkSorted(t *testing.T, imgs []Info) {
for i, im := range imgs {
if strconv.Itoa(i) != im.ID.Tag {
for j, jim := range imgs {
Expand Down

0 comments on commit aca71d8

Please sign in to comment.