From 63bed346088036b1ef7eb67dbf799a22850350ff Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 3 Dec 2022 11:55:33 +0100 Subject: [PATCH] restic: Clean up restic.IDs type IDs.Less can be rewritten as string(list[i][:]) < string(list[j][:]) Note that this does not copy the ID's. The Uniq method was no longer used. The String method has been reimplemented without first copying into a separate slice of a custom type. --- internal/restic/ids.go | 65 ++++++++++------------------------- internal/restic/ids_test.go | 56 +++++------------------------- internal/restic/idset.go | 6 +--- internal/restic/idset_test.go | 6 ++++ 4 files changed, 34 insertions(+), 99 deletions(-) diff --git a/internal/restic/ids.go b/internal/restic/ids.go index b3e6b6f22..de91dbb4c 100644 --- a/internal/restic/ids.go +++ b/internal/restic/ids.go @@ -2,10 +2,10 @@ package restic import ( "encoding/hex" - "fmt" + "strings" ) -// IDs is an ordered list of IDs that implements sort.Interface. +// IDs is a slice of IDs that implements sort.Interface and fmt.Stringer. type IDs []ID func (ids IDs) Len() int { @@ -13,57 +13,28 @@ func (ids IDs) Len() int { } func (ids IDs) Less(i, j int) bool { - if len(ids[i]) < len(ids[j]) { - return true - } - - for k, b := range ids[i] { - if b == ids[j][k] { - continue - } - - if b < ids[j][k] { - return true - } - - return false - } - - return false + return string(ids[i][:]) < string(ids[j][:]) } func (ids IDs) Swap(i, j int) { ids[i], ids[j] = ids[j], ids[i] } -// Uniq returns list without duplicate IDs. The returned list retains the order -// of the original list so that the order of the first occurrence of each ID -// stays the same. -func (ids IDs) Uniq() (list IDs) { - seen := NewIDSet() - - for _, id := range ids { - if seen.Has(id) { - continue - } - - list = append(list, id) - seen.Insert(id) - } - - return list -} - -type shortID ID - -func (id shortID) String() string { - return hex.EncodeToString(id[:shortStr]) -} - func (ids IDs) String() string { - elements := make([]shortID, 0, len(ids)) - for _, id := range ids { - elements = append(elements, shortID(id)) + var sb strings.Builder + sb.Grow(1 + (1+2*shortStr)*len(ids)) + + buf := make([]byte, 2*shortStr) + + sb.WriteByte('[') + for i, id := range ids { + if i > 0 { + sb.WriteByte(' ') + } + hex.Encode(buf, id[:shortStr]) + sb.Write(buf) } - return fmt.Sprint(elements) + sb.WriteByte(']') + + return sb.String() } diff --git a/internal/restic/ids_test.go b/internal/restic/ids_test.go index 9ce02607b..8f1e9d5a9 100644 --- a/internal/restic/ids_test.go +++ b/internal/restic/ids_test.go @@ -1,55 +1,17 @@ package restic import ( - "reflect" "testing" + + rtest "github.com/restic/restic/internal/test" ) -var uniqTests = []struct { - before, after IDs -}{ - { - IDs{ - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - IDs{ - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - }, - }, - { - IDs{ - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - IDs{ - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - }, - { - IDs{ - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("f658198b405d7e80db5ace1980d125c8da62f636b586c46bf81dfb856a49d0c8"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - IDs{ - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("f658198b405d7e80db5ace1980d125c8da62f636b586c46bf81dfb856a49d0c8"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - }, -} - -func TestUniqIDs(t *testing.T) { - for i, test := range uniqTests { - uniq := test.before.Uniq() - if !reflect.DeepEqual(uniq, test.after) { - t.Errorf("uniqIDs() test %v failed\n wanted: %v\n got: %v", i, test.after, uniq) - } +func TestIDsString(t *testing.T) { + ids := IDs{ + TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), + TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), + TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), } + + rtest.Equals(t, "[7bb086db 1285b303 7bb086db]", ids.String()) } diff --git a/internal/restic/idset.go b/internal/restic/idset.go index c31ca7747..1b12a6398 100644 --- a/internal/restic/idset.go +++ b/internal/restic/idset.go @@ -31,7 +31,7 @@ func (s IDSet) Delete(id ID) { delete(s, id) } -// List returns a slice of all IDs in the set. +// List returns a sorted slice of all IDs in the set. func (s IDSet) List() IDs { list := make(IDs, 0, len(s)) for id := range s { @@ -103,9 +103,5 @@ func (s IDSet) Sub(other IDSet) (result IDSet) { func (s IDSet) String() string { str := s.List().String() - if len(str) < 2 { - return "{}" - } - return "{" + str[1:len(str)-1] + "}" } diff --git a/internal/restic/idset_test.go b/internal/restic/idset_test.go index 5525eab79..734b31237 100644 --- a/internal/restic/idset_test.go +++ b/internal/restic/idset_test.go @@ -2,6 +2,8 @@ package restic import ( "testing" + + rtest "github.com/restic/restic/internal/test" ) var idsetTests = []struct { @@ -22,6 +24,8 @@ var idsetTests = []struct { func TestIDSet(t *testing.T) { set := NewIDSet() + rtest.Equals(t, "{}", set.String()) + for i, test := range idsetTests { seen := set.Has(test.id) if seen != test.seen { @@ -29,4 +33,6 @@ func TestIDSet(t *testing.T) { } set.Insert(test.id) } + + rtest.Equals(t, "{1285b303 7bb086db f658198b}", set.String()) }