From 218057ea48065f225ee09173461d7f72a5fa3ee9 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Thu, 26 Dec 2013 16:58:04 +0400 Subject: [PATCH] Add error handling for ForEach's. --- cmd_mirror.go | 3 ++- cmd_snapshot.go | 16 ++++++++++---- debian/list.go | 22 ++++++++++++------- debian/list_test.go | 26 ++++++++++++++++++++-- debian/publish.go | 40 ++++++++++++++++++++++++++-------- debian/remote.go | 48 ++++++++++++++++++++++++++++++----------- debian/remote_test.go | 14 +++++++++++- debian/snapshot.go | 9 ++++++-- debian/snapshot_test.go | 13 ++++++++++- 9 files changed, 151 insertions(+), 40 deletions(-) diff --git a/cmd_mirror.go b/cmd_mirror.go index 9a320b93..9e708320 100644 --- a/cmd_mirror.go +++ b/cmd_mirror.go @@ -18,8 +18,9 @@ func aptlyMirrorList(cmd *commander.Command, args []string) error { fmt.Printf("List of mirrors:\n") repoCollection := debian.NewRemoteRepoCollection(context.database) - repoCollection.ForEach(func(repo *debian.RemoteRepo) { + repoCollection.ForEach(func(repo *debian.RemoteRepo) error { fmt.Printf(" * %s\n", repo) + return nil }) fmt.Printf("\nTo get more information about repository, run `aptly mirror show `.\n") diff --git a/cmd_snapshot.go b/cmd_snapshot.go index 3e02bbf1..57a1bf24 100644 --- a/cmd_snapshot.go +++ b/cmd_snapshot.go @@ -40,7 +40,7 @@ func aptlySnapshotCreate(cmd *commander.Command, args []string) error { return fmt.Errorf("unable to add snapshot: %s", err) } - fmt.Printf("\nSnapshot %s successfully created.\nYou can run 'aptly snapshot publish %s' to publish snapshot as Debian repository.\n", snapshot.Name, snapshot.Name) + fmt.Printf("\nSnapshot %s successfully created.\nYou can run 'aptly publish snapshot %s' to publish snapshot as Debian repository.\n", snapshot.Name, snapshot.Name) return err } @@ -55,8 +55,9 @@ func aptlySnapshotList(cmd *commander.Command, args []string) error { fmt.Printf("List of snapshots:\n") snapshotCollection := debian.NewSnapshotCollection(context.database) - snapshotCollection.ForEach(func(snapshot *debian.Snapshot) { + snapshotCollection.ForEach(func(snapshot *debian.Snapshot) error { fmt.Printf(" * %s\n", snapshot) + return nil }) fmt.Printf("\nTo get more information about snapshot, run `aptly snapshot show `.\n") @@ -91,10 +92,17 @@ func aptlySnapshotShow(cmd *commander.Command, args []string) error { packageCollection := debian.NewPackageCollection(context.database) - snapshot.RefList().ForEach(func(key []byte) { - p, _ := packageCollection.ByKey(key) + err = snapshot.RefList().ForEach(func(key []byte) error { + p, err := packageCollection.ByKey(key) + if err != nil { + return err + } fmt.Printf(" %s\n", p) + return nil }) + if err != nil { + return fmt.Errorf("unable to load packages: %s", err) + } return err } diff --git a/debian/list.go b/debian/list.go index 36fa9e1a..dcb3a20b 100644 --- a/debian/list.go +++ b/debian/list.go @@ -35,12 +35,15 @@ func (l *PackageList) Add(p *Package) error { } // ForEach calls handler for each package in list -// -// TODO: Error handling -func (l *PackageList) ForEach(handler func(*Package)) { +func (l *PackageList) ForEach(handler func(*Package) error) error { + var err error for _, p := range l.packages { - handler(p) + err = handler(p) + if err != nil { + return err + } } + return err } // Len returns number of packages in the list @@ -105,10 +108,13 @@ func (l *PackageRefList) Decode(input []byte) error { } // ForEach calls handler for each package ref in list -// -// TODO: Error handling -func (l *PackageRefList) ForEach(handler func([]byte)) { +func (l *PackageRefList) ForEach(handler func([]byte) error) error { + var err error for _, p := range l.Refs { - handler(p) + err = handler(p) + if err != nil { + return err + } } + return err } diff --git a/debian/list_test.go b/debian/list_test.go index b77ccc98..5eb15904 100644 --- a/debian/list_test.go +++ b/debian/list_test.go @@ -1,6 +1,7 @@ package debian import ( + "errors" . "launchpad.net/gocheck" ) @@ -46,11 +47,22 @@ func (s *PackageListSuite) TestForeach(c *C) { s.list.Add(s.p3) Len := 0 - s.list.ForEach(func(*Package) { + err := s.list.ForEach(func(*Package) error { Len++ + return nil }) c.Check(Len, Equals, 2) + c.Check(err, IsNil) + + e := errors.New("a") + + err = s.list.ForEach(func(*Package) error { + return e + }) + + c.Check(err, Equals, e) + } func (s *PackageListSuite) TestNewPackageRefList(c *C) { @@ -91,9 +103,19 @@ func (s *PackageListSuite) TestPackageRefListForeach(c *C) { reflist := NewPackageRefListFromPackageList(s.list) Len := 0 - reflist.ForEach(func([]byte) { + err := reflist.ForEach(func([]byte) error { Len++ + return nil }) c.Check(Len, Equals, 4) + c.Check(err, IsNil) + + e := errors.New("b") + + err = reflist.ForEach(func([]byte) error { + return e + }) + + c.Check(err, Equals, e) } diff --git a/debian/publish.go b/debian/publish.go index 4e046f6b..ba5bc30f 100644 --- a/debian/publish.go +++ b/debian/publish.go @@ -50,10 +50,16 @@ func (p *PublishedRepo) Publish(repo *Repository, packageCollection *PackageColl // Load all packages list := NewPackageList() - p.snapshot.RefList().ForEach(func(key []byte) { - pkg, _ := packageCollection.ByKey(key) - list.Add(pkg) + err = p.snapshot.RefList().ForEach(func(key []byte) error { + pkg, err := packageCollection.ByKey(key) + if err != nil { + return err + } + return list.Add(pkg) }) + if err != nil { + return fmt.Errorf("unable to load packages: %s", err) + } if list.Len() == 0 { return fmt.Errorf("repository is empty, can't publish") @@ -61,10 +67,11 @@ func (p *PublishedRepo) Publish(repo *Repository, packageCollection *PackageColl if p.Architectures == nil { p.Architectures = make([]string, 0, 10) - list.ForEach(func(pkg *Package) { + list.ForEach(func(pkg *Package) error { if pkg.Architecture != "all" && !utils.StrSliceHasItem(p.Architectures, pkg.Architecture) { p.Architectures = append(p.Architectures, pkg.Architecture) } + return nil }) } @@ -89,18 +96,33 @@ func (p *PublishedRepo) Publish(repo *Repository, packageCollection *PackageColl bufWriter := bufio.NewWriter(packagesFile) - list.ForEach(func(pkg *Package) { + err = list.ForEach(func(pkg *Package) error { if pkg.Architecture == arch || pkg.Architecture == "all" { - path, _ := repo.LinkFromPool(p.Prefix, p.Component, pkg.Filename, pkg.HashMD5, pkg.Source) + path, err := repo.LinkFromPool(p.Prefix, p.Component, pkg.Filename, pkg.HashMD5, pkg.Source) + if err != nil { + return err + } - // TODO: error handling pkg.Filename = path - pkg.Stanza().WriteTo(bufWriter) - bufWriter.WriteByte('\n') + + err = pkg.Stanza().WriteTo(bufWriter) + if err != nil { + return err + } + err = bufWriter.WriteByte('\n') + if err != nil { + return err + } } + + return nil }) + if err != nil { + return fmt.Errorf("unable to creates process packages: %s", err) + } + err = bufWriter.Flush() if err != nil { return fmt.Errorf("unable to write Packages file: %s", err) diff --git a/debian/remote.go b/debian/remote.go index 18ea5aaa..3f1999a4 100644 --- a/debian/remote.go +++ b/debian/remote.go @@ -176,31 +176,50 @@ func (repo *RemoteRepo) Download(d utils.Downloader, packageCollection *PackageC } // Save package meta information to DB - list.ForEach(func(p *Package) { - packageCollection.Update(p) + err := list.ForEach(func(p *Package) error { + return packageCollection.Update(p) }) + if err != nil { + return fmt.Errorf("unable to save packages to db: %s", err) + } + // Download all package files ch := make(chan error, list.Len()) count := 0 - list.ForEach(func(p *Package) { + err = list.ForEach(func(p *Package) error { poolPath, err := packageRepo.PoolPath(p.Filename, p.HashMD5) - if err == nil { - if !p.VerifyFile(poolPath) { - d.Download(repo.PackageURL(p.Filename).String(), poolPath, ch) - count++ - } + if err != nil { + return err } + + if !p.VerifyFile(poolPath) { + d.Download(repo.PackageURL(p.Filename).String(), poolPath, ch) + count++ + } + return nil }) + if err != nil { + return fmt.Errorf("unable to download packages: %s", err) + } + + errors := make([]string, 0) + // Wait for all downloads to finish - // TODO: report errors for count > 0 { - _ = <-ch + err = <-ch + if err != nil { + errors = append(errors, err.Error()) + } count-- } + if len(errors) > 0 { + return fmt.Errorf("download errors: %s", strings.Join(errors, ", ")) + } + repo.LastDownloadDate = time.Now() repo.packageRefs = NewPackageRefListFromPackageList(list) @@ -331,8 +350,13 @@ func (collection *RemoteRepoCollection) ByUUID(uuid string) (*RemoteRepo, error) } // ForEach runs method for each repository -func (collection *RemoteRepoCollection) ForEach(handler func(*RemoteRepo)) { +func (collection *RemoteRepoCollection) ForEach(handler func(*RemoteRepo) error) error { + var err error for _, r := range collection.list { - handler(r) + err = handler(r) + if err != nil { + return err + } } + return err } diff --git a/debian/remote_test.go b/debian/remote_test.go index 35389146..fbd8a4f2 100644 --- a/debian/remote_test.go +++ b/debian/remote_test.go @@ -1,6 +1,7 @@ package debian import ( + "errors" "github.com/smira/aptly/database" "github.com/smira/aptly/utils" . "launchpad.net/gocheck" @@ -187,8 +188,19 @@ func (s *RemoteRepoCollectionSuite) TestForEach(c *C) { s.collection.Add(repo) count := 0 - s.collection.ForEach(func(*RemoteRepo) { count++ }) + err := s.collection.ForEach(func(*RemoteRepo) error { + count++ + return nil + }) c.Assert(count, Equals, 1) + c.Assert(err, IsNil) + + e := errors.New("c") + + err = s.collection.ForEach(func(*RemoteRepo) error { + return e + }) + c.Assert(err, Equals, e) } const exampleReleaseFile = `Origin: LP-PPA-agenda-developers-daily diff --git a/debian/snapshot.go b/debian/snapshot.go index 274f05cf..c43ca1c7 100644 --- a/debian/snapshot.go +++ b/debian/snapshot.go @@ -162,8 +162,13 @@ func (collection *SnapshotCollection) ByName(name string) (*Snapshot, error) { } // ForEach runs method for each snapshot -func (collection *SnapshotCollection) ForEach(handler func(*Snapshot)) { +func (collection *SnapshotCollection) ForEach(handler func(*Snapshot) error) error { + var err error for _, s := range collection.list { - handler(s) + err = handler(s) + if err != nil { + return err + } } + return err } diff --git a/debian/snapshot_test.go b/debian/snapshot_test.go index df3af289..7d91f719 100644 --- a/debian/snapshot_test.go +++ b/debian/snapshot_test.go @@ -1,6 +1,7 @@ package debian import ( + "errors" "github.com/smira/aptly/database" . "launchpad.net/gocheck" ) @@ -116,6 +117,16 @@ func (s *SnapshotCollectionSuite) TestForEach(c *C) { s.collection.Add(s.snapshot2) count := 0 - s.collection.ForEach(func(*Snapshot) { count++ }) + err := s.collection.ForEach(func(*Snapshot) error { + count++ + return nil + }) c.Assert(count, Equals, 2) + c.Assert(err, IsNil) + + e := errors.New("d") + err = s.collection.ForEach(func(*Snapshot) error { + return e + }) + c.Assert(err, Equals, e) }