Refactor Downloader: it accepts channel, not generates it.

utils/download.go

@@ -14,7 +14,7 @@ import (
 
 // Downloader is parallel HTTP fetcher
 type Downloader interface {
-	Download(url string, destination string) <-chan error
+	Download(url string, destination string, result chan<- error)
 	Shutdown()
 }
 
@@ -68,12 +68,8 @@ func (downloader *downloaderImpl) Shutdown() {
 }
 
 // Download starts new download task
-func (downloader *downloaderImpl) Download(url string, destination string) <-chan error {
+func (downloader *downloaderImpl) Download(url string, destination string, result chan<- error) {
-	ch := make(chan error, 1)
+	downloader.queue <- &downloadTask{url: url, destination: destination, result: result}
-
-	downloader.queue <- &downloadTask{url: url, destination: destination, result: ch}
-
-	return ch
 }
 
 // handleTask processes single download task
@@ -149,7 +145,8 @@ func DownloadTemp(downloader Downloader, url string) (*os.File, error) {
 
 	tempfile := filepath.Join(tempdir, "buffer")
 
-	ch := downloader.Download(url, tempfile)
+	ch := make(chan error, 1)
+	downloader.Download(url, tempfile, ch)
 
 	err = <-ch
 	if err != nil {

utils/download_test.go

@@ -48,32 +48,40 @@ func (s *DownloaderSuite) TestStartupShutdown(c *C) {
 func (s *DownloaderSuite) TestDownloadOK(c *C) {
 	d := NewDownloader(2)
 	defer d.Shutdown()
+	ch := make(chan error)
 
-	res := <-d.Download("http://smira.ru/", s.tempfile.Name())
+	d.Download("http://smira.ru/", s.tempfile.Name(), ch)
+	res := <-ch
 	c.Assert(res, IsNil)
 }
 
 func (s *DownloaderSuite) TestDownload404(c *C) {
 	d := NewDownloader(2)
 	defer d.Shutdown()
+	ch := make(chan error)
 
-	res := <-d.Download("http://smira.ru/doesntexist", s.tempfile.Name())
+	d.Download("http://smira.ru/doesntexist", s.tempfile.Name(), ch)
+	res := <-ch
 	c.Assert(res, ErrorMatches, "HTTP code 404.*")
 }
 
 func (s *DownloaderSuite) TestDownloadConnectError(c *C) {
 	d := NewDownloader(2)
 	defer d.Shutdown()
+	ch := make(chan error)
 
-	res := <-d.Download("http://nosuch.smira.ru/", s.tempfile.Name())
+	d.Download("http://nosuch.smira.ru/", s.tempfile.Name(), ch)
+	res := <-ch
 	c.Assert(res, ErrorMatches, ".*no such host")
 }
 
 func (s *DownloaderSuite) TestDownloadFileError(c *C) {
 	d := NewDownloader(2)
 	defer d.Shutdown()
+	ch := make(chan error)
 
-	res := <-d.Download("http://smira.ru/", "/")
+	d.Download("http://smira.ru/", "/", ch)
+	res := <-ch
 	c.Assert(res, ErrorMatches, ".*permission denied")
 }
 

utils/fake.go

@@ -47,12 +47,10 @@ func (f *FakeDownloader) Empty() bool {
 }
 
 // Download performs fake download by matching against first expectation in the queue
-func (f *FakeDownloader) Download(url string, filename string) <-chan error {
+func (f *FakeDownloader) Download(url string, filename string, result chan<- error) {
-	result := make(chan error, 1)
-
 	if len(f.expected) == 0 || f.expected[0].URL != url {
 		result <- fmt.Errorf("unexpected request for %s", url)
-		return result
+		return
 	}
 
 	expected := f.expected[0]
@@ -60,24 +58,24 @@ func (f *FakeDownloader) Download(url string, filename string) <-chan error {
 
 	if expected.Err != nil {
 		result <- expected.Err
-		return result
+		return
 	}
 
 	outfile, err := os.Create(filename)
 	if err != nil {
 		result <- err
-		return result
+		return
 	}
 	defer outfile.Close()
 
 	_, err = outfile.Write([]byte(expected.Response))
 	if err != nil {
 		result <- err
-		return result
+		return
 	}
 
 	result <- nil
-	return result
+	return
 }
 
 // Shutdown does nothing