From aca71d8e2984ce8a5db9a97f71da54691a325dc9 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Tue, 24 Jul 2018 13:00:35 +0100 Subject: [PATCH] make image sort stable, and correct 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. --- image/image.go | 10 ++-------- image/image_test.go | 27 +++++++++++++++++++++------ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/image/image.go b/image/image.go index fbb6e179d..717c8eb0d 100644 --- a/image/image.go +++ b/image/image.go @@ -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) } diff --git a/image/image_test.go b/image/image_test.go index f3a6eb6f2..ed87957f8 100644 --- a/image/image_test.go +++ b/image/image_test.go @@ -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 {