From d828732307a126f4e2886f46d07237094e4adc6b Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Thu, 22 Jan 2015 22:01:00 +0300 Subject: [PATCH] Refactoring: make snapshot sorting non-intrusive to collection contents. #168 --- api/snapshot.go | 52 ++++++++--------- cmd/snapshot_list.go | 9 ++- deb/snapshot.go | 65 +++++++++++++--------- system/t05_snapshot/ListSnapshot7Test_gold | 1 + system/t05_snapshot/list.py | 3 + 5 files changed, 75 insertions(+), 55 deletions(-) diff --git a/api/snapshot.go b/api/snapshot.go index ae39bbab..ad96cff0 100644 --- a/api/snapshot.go +++ b/api/snapshot.go @@ -2,10 +2,10 @@ package api import ( "fmt" - "sort" "github.com/gin-gonic/gin" "github.com/smira/aptly/deb" "github.com/smira/aptly/query" + "sort" ) // GET /api/snapshots @@ -16,12 +16,12 @@ func apiSnapshotsList(c *gin.Context) { collection.RLock() defer collection.RUnlock() - if SortMethodString != "" { - collection.Sort(SortMethodString) + if SortMethodString == "" { + SortMethodString = "name" } result := []*deb.Snapshot{} - collection.ForEach(func(snapshot *deb.Snapshot) error { + collection.ForEachSorted(SortMethodString, func(snapshot *deb.Snapshot) error { result = append(result, snapshot) return nil }) @@ -32,14 +32,14 @@ func apiSnapshotsList(c *gin.Context) { // POST /api/mirrors/:name/snapshots/ func apiSnapshotsCreateFromMirror(c *gin.Context) { var ( - err error + err error repo *deb.RemoteRepo snapshot *deb.Snapshot ) var b struct { - Name string `binding:"required"` - Description string + Name string `binding:"required"` + Description string } if !c.Bind(&b) { @@ -94,15 +94,15 @@ func apiSnapshotsCreateFromMirror(c *gin.Context) { // POST /api/snapshots func apiSnapshotsCreate(c *gin.Context) { var ( - err error + err error snapshot *deb.Snapshot ) var b struct { - Name string `binding:"required"` - Description string - SourceIDs []string - PackageRefs []string + Name string `binding:"required"` + Description string + SourceIDs []string + PackageRefs []string } if !c.Bind(&b) { @@ -110,7 +110,7 @@ func apiSnapshotsCreate(c *gin.Context) { } if b.Description == "" { - if len(b.SourceIDs) + len(b.PackageRefs) == 0 { + if len(b.SourceIDs)+len(b.PackageRefs) == 0 { b.Description = "Created as empty" } } @@ -119,23 +119,23 @@ func apiSnapshotsCreate(c *gin.Context) { snapshotCollection.Lock() defer snapshotCollection.Unlock() - sources := make([]*deb.Snapshot, len(b.SourceIDs)) + sources := make([]*deb.Snapshot, len(b.SourceIDs)) - for i := 0; i < len(b.SourceIDs); i++ { - sources[i], err = snapshotCollection.ByUUID(b.SourceIDs[i]) - if err != nil { + for i := 0; i < len(b.SourceIDs); i++ { + sources[i], err = snapshotCollection.ByUUID(b.SourceIDs[i]) + if err != nil { c.Fail(404, err) return - } + } - err = snapshotCollection.LoadComplete(sources[i]) - if err != nil { + err = snapshotCollection.LoadComplete(sources[i]) + if err != nil { c.Fail(500, err) return - } - } + } + } - packageRefs := make([][]byte, len(b.PackageRefs)) + packageRefs := make([][]byte, len(b.PackageRefs)) for i, ref := range b.PackageRefs { packageRefs[i] = []byte(ref) } @@ -155,14 +155,14 @@ func apiSnapshotsCreate(c *gin.Context) { // POST /api/repos/:name/snapshots/:snapname func apiSnapshotsCreateFromRepository(c *gin.Context) { var ( - err error + err error repo *deb.LocalRepo snapshot *deb.Snapshot ) var b struct { - Name string `binding:"required"` - Description string + Name string `binding:"required"` + Description string } if !c.Bind(&b) { diff --git a/cmd/snapshot_list.go b/cmd/snapshot_list.go index 8c5b6d56..06ba631c 100644 --- a/cmd/snapshot_list.go +++ b/cmd/snapshot_list.go @@ -17,10 +17,9 @@ func aptlySnapshotList(cmd *commander.Command, args []string) error { sortMethodString := cmd.Flag.Lookup("sort").Value.Get().(string) collection := context.CollectionFactory().SnapshotCollection() - collection.Sort(sortMethodString) if raw { - collection.ForEach(func(snapshot *deb.Snapshot) error { + collection.ForEachSorted(sortMethodString, func(snapshot *deb.Snapshot) error { fmt.Printf("%s\n", snapshot.Name) return nil }) @@ -28,11 +27,15 @@ func aptlySnapshotList(cmd *commander.Command, args []string) error { if collection.Len() > 0 { fmt.Printf("List of snapshots:\n") - collection.ForEach(func(snapshot *deb.Snapshot) error { + err = collection.ForEachSorted(sortMethodString, func(snapshot *deb.Snapshot) error { fmt.Printf(" * %s\n", snapshot.String()) return nil }) + if err != nil { + return err + } + fmt.Printf("\nTo get more information about snapshot, run `aptly snapshot show `.\n") } else { fmt.Printf("\nNo snapshots found, create one with `aptly snapshot create...`.\n") diff --git a/deb/snapshot.go b/deb/snapshot.go index e50efb95..eade4ece 100644 --- a/deb/snapshot.go +++ b/deb/snapshot.go @@ -297,6 +297,22 @@ func (collection *SnapshotCollection) ForEach(handler func(*Snapshot) error) err return err } +func (collection *SnapshotCollection) ForEachSorted(sortMethod string, handler func(*Snapshot) error) error { + sorter, err := newSnapshotSorter(sortMethod, collection) + if err != nil { + return err + } + + for _, i := range sorter.list { + err = handler(collection.list[i]) + if err != nil { + return err + } + } + + return nil +} + // Len returns number of snapshots in collection // ForEach runs method for each snapshot func (collection *SnapshotCollection) Len() int { @@ -335,51 +351,48 @@ const ( SortTime ) -type snapshotListToSort struct { - list []*Snapshot +type snapshotSorter struct { + list []int + collection *SnapshotCollection sortMethod int } -func parseSortMethod(sortMethod string) (int, error) { +func newSnapshotSorter(sortMethod string, collection *SnapshotCollection) (*snapshotSorter, error) { + s := &snapshotSorter{collection: collection} + switch sortMethod { case "time", "Time": - return SortTime, nil + s.sortMethod = SortTime case "name", "Name": - return SortName, nil + s.sortMethod = SortName + default: + return nil, fmt.Errorf("sorting method \"%s\" unknown", sortMethod) } - return -1, fmt.Errorf("sorting method \"%s\" unknown", sortMethod) + s.list = make([]int, len(collection.list)) + for i := range s.list { + s.list[i] = i + } + + sort.Sort(s) + + return s, nil } -func (s snapshotListToSort) Swap(i, j int) { +func (s *snapshotSorter) Swap(i, j int) { s.list[i], s.list[j] = s.list[j], s.list[i] } -func (s snapshotListToSort) Less(i, j int) bool { +func (s *snapshotSorter) Less(i, j int) bool { switch s.sortMethod { case SortName: - return s.list[i].Name < s.list[j].Name + return s.collection.list[s.list[i]].Name < s.collection.list[s.list[j]].Name case SortTime: - return s.list[i].CreatedAt.Before(s.list[j].CreatedAt) + return s.collection.list[s.list[i]].CreatedAt.Before(s.collection.list[s.list[j]].CreatedAt) } panic("unknown sort method") } -func (s snapshotListToSort) Len() int { +func (s *snapshotSorter) Len() int { return len(s.list) } - -func (collection *SnapshotCollection) Sort(sortMethodString string) error { - var err error - snapshotsToSort := &snapshotListToSort{} - snapshotsToSort.list = collection.list - snapshotsToSort.sortMethod, err = parseSortMethod(sortMethodString) - if err != nil { - return err - } - - sort.Sort(snapshotsToSort) - collection.list = snapshotsToSort.list - - return err -} diff --git a/system/t05_snapshot/ListSnapshot7Test_gold b/system/t05_snapshot/ListSnapshot7Test_gold index a9dfcb04..c706052d 100644 --- a/system/t05_snapshot/ListSnapshot7Test_gold +++ b/system/t05_snapshot/ListSnapshot7Test_gold @@ -1 +1,2 @@ +List of snapshots: ERROR: sorting method "planet" unknown diff --git a/system/t05_snapshot/list.py b/system/t05_snapshot/list.py index d3e576d0..49bd1f6a 100644 --- a/system/t05_snapshot/list.py +++ b/system/t05_snapshot/list.py @@ -87,5 +87,8 @@ class ListSnapshot7Test(BaseTest): """ list snapshots: wrong parameter sort """ + fixtureCmds = [ + "aptly snapshot create empty empty" + ] runCmd = "aptly -sort=planet snapshot list" expectedCode = 1