refactor(downloader): improve structure and readability

internal/downloader/client.go

@@ -11,7 +11,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/briandowns/spinner"
 	"github.com/mathismqn/godeez/internal/config"
 	"github.com/mathismqn/godeez/internal/crypto"
 	"github.com/mathismqn/godeez/internal/deezer"
@@ -45,6 +44,19 @@ func New(appConfig *config.Config, resourceType string) *Client {
 }
 
 func (c *Client) Run(ctx context.Context, opts Options, id string) error {
+	if err := c.initDeezerClient(ctx, opts); err != nil {
+		return err
+	}
+
+	resource, outputDir, err := c.prepareResource(ctx, id, opts)
+	if err != nil {
+		return err
+	}
+
+	return c.downloadAllSongs(ctx, resource, id, opts, outputDir)
+}
+
+func (c *Client) initDeezerClient(ctx context.Context, opts Options) error {
 	var err error
 	c.deezerClient, err = deezer.NewClient(ctx, c.appConfig)
 	if err != nil {
@@ -55,180 +67,112 @@ func (c *Client) Run(ctx context.Context, opts Options, id string) error {
 		return fmt.Errorf("premium account required for '%s' quality", opts.Quality)
 	}
 
-	var resource deezer.Resource
+	return nil
-	switch c.resourceType {
+}
-	case "album":
+
-		resource = &deezer.Album{}
+func (c *Client) prepareResource(ctx context.Context, id string, opts Options) (deezer.Resource, string, error) {
-	case "playlist":
+	resource, err := c.createResource()
-		resource = &deezer.Playlist{}
+	if err != nil {
-	case "artist":
+		return nil, "", err
-		resource = &deezer.Artist{}
-	case "track":
-		resource = &deezer.Track{}
-	default:
-		return fmt.Errorf("unsupported resource type: %s", c.resourceType)
 	}
 
 	if err := c.deezerClient.FetchResource(ctx, resource, id); err != nil {
-		return fmt.Errorf("failed to fetch resource: %w", err)
+		return nil, "", fmt.Errorf("failed to fetch resource: %w", err)
 	}
 
 	songs := resource.GetSongs()
 	if len(songs) == 0 {
 		if c.resourceType == "track" {
-			return fmt.Errorf("track with ID %s not found", id)
+			return nil, "", fmt.Errorf("track with ID %s not found", id)
 		}
-		return fmt.Errorf("%s has no songs", c.resourceType)
+		return nil, "", fmt.Errorf("%s has no songs", c.resourceType)
 	}
+
 	if c.resourceType == "artist" && len(songs) > opts.Limit {
 		songs = songs[:opts.Limit]
 		resource.SetSongs(songs)
 	}
 
-	rootOutputDir := c.appConfig.OutputDir
+	resourceOutputDir := resource.GetOutputDir(c.appConfig.OutputDir)
-	resourceOutputDir := resource.GetOutputDir(rootOutputDir)
 	if err := fileutil.EnsureDir(resourceOutputDir); err != nil {
-		return fmt.Errorf("failed to create output directory: %w", err)
+		return nil, "", fmt.Errorf("failed to create output directory: %w", err)
 	}
 
+	return resource, resourceOutputDir, nil
+}
+
+func (c *Client) createResource() (deezer.Resource, error) {
+	switch c.resourceType {
+	case "album":
+		return &deezer.Album{}, nil
+	case "playlist":
+		return &deezer.Playlist{}, nil
+	case "artist":
+		return &deezer.Artist{}, nil
+	case "track":
+		return &deezer.Track{}, nil
+	default:
+		return nil, fmt.Errorf("unsupported resource type: %s", c.resourceType)
+	}
+}
+
+func (c *Client) downloadAllSongs(ctx context.Context, resource deezer.Resource, resourceID string, opts Options, outputDir string) error {
+	songs := resource.GetSongs()
 	startTime := time.Now()
+
 	if c.resourceType != "track" {
 		fmt.Printf("%s\n\nStarting download...\n\n", resource)
 	}
 
-	downloaded := 0
+	progress := newProgressTracker(c.Logger, len(songs), c.resourceType)
-	skipped := 0
-	failed := 0
 
 	for i, song := range songs {
 		if ctx.Err() != nil {
 			return ctx.Err()
 		}
 
-		songTitle := song.GetTitle()
+		sp := progress.startDownload(i, song)
-		trackProgress := fmt.Sprintf("[%d/%d]", i+1, len(songs))
+		result := c.downloadSong(ctx, resource, song, opts, outputDir)
-
-		sp := spinner.New(spinner.CharSets[14], 100*time.Millisecond)
-		sp.Writer = os.Stdout
-		sp.Prefix = trackProgress + " "
-		sp.Suffix = fmt.Sprintf(" Downloading: %s - %s", song.Artist, songTitle)
-		sp.Start()
-
-		warnings, err := c.downloadSong(ctx, resource, song, opts, resourceOutputDir)
 		sp.Stop()
 
-		if err != nil {
+		if result.err != nil {
-			if errors.Is(err, context.Canceled) {
+			if errors.Is(result.err, context.Canceled) {
-				return err
+				return result.err
 			}
-
-			if path, ok := IsSkipError(err); ok {
-				skipped++
-				fmt.Printf("%s ↷ Skipped: %s - %s\n    Already exists at: %s\n", trackProgress, song.Artist, songTitle, path)
-				continue
-			}
-
-			failed++
-			c.Logger.Errorf("Failed to download %s - %s: %v\n", song.Artist, songTitle, err)
-			fmt.Printf("%s ✖ Failed: %s - %s:\n    Error: %v\n", trackProgress, song.Artist, songTitle, err)
-
-			continue
 		}
 
-		symbol := "✔"
+		progress.handleResult(i, song, result)
-		if len(warnings) > 0 {
-			symbol = "⚠"
-		}
-
-		downloaded++
-		c.Logger.Infof("Downloaded %s - %s\n", song.Artist, songTitle)
-		fmt.Printf("%s %s Downloaded: %s - %s\n", trackProgress, symbol, song.Artist, songTitle)
-
-		for _, w := range warnings {
-			c.Logger.Warnf("Warning: %s\n", w)
-			fmt.Printf("    Warning: %s\n", w)
-		}
 	}
 
-	if downloaded > 0 || failed > 0 {
+	progress.printSummary(resource.GetTitle(), resourceID, outputDir, time.Since(startTime))
-		c.Logger.Infof("Playlist %s (%s): %d downloaded, %d skipped, %d failed\n", resource.GetTitle(), id, downloaded, skipped, failed)
-	}
-
-	if c.resourceType != "track" {
-		fmt.Printf(`
-================== [ Summary ] ==================
-Downloaded:     %d
-Skipped:        %d
-Failed:         %d
-Elapsed time:   %s
-Files saved to: %s
-=================================================
-`,
-			downloaded,
-			skipped,
-			failed,
-			time.Since(startTime).Round(time.Second),
-			resourceOutputDir,
-		)
-	}
 
 	return nil
 }
 
-func (c *Client) downloadSong(ctx context.Context, resource deezer.Resource, song *deezer.Song, opts Options, outputDir string) ([]string, error) {
+func (c *Client) downloadSong(ctx context.Context, resource deezer.Resource, song *deezer.Song, opts Options, outputDir string) downloadResult {
 	var warnings []string
 
 	media, err := c.deezerClient.FetchMedia(ctx, song, opts.Quality)
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch media: %w", err)
+		return handleError(fmt.Errorf("failed to fetch media: %w", err))
 	}
 
 	mediaFormat := media.GetFormat()
 	if opts.Strict && strings.ToLower(mediaFormat) != opts.Quality {
-		return nil, fmt.Errorf("requested quality '%s' not available", opts.Quality)
+		return handleError(fmt.Errorf("requested quality '%s' not available", opts.Quality))
 	}
 
 	if path, skip := c.shouldSkipDownload(ctx, song.ID, mediaFormat); skip {
-		return nil, SkipError{Path: path}
+		return handleError(SkipError{Path: path})
 	}
 
-	var bpmChan chan provider.BPMKey
+	metadataFetcher := newMetadataFetcher(c.deezerClient.Session.HttpClient)
-	var bpmErrChan chan error
+	metadataResult := metadataFetcher.fetch(ctx, song, opts)
-	if opts.BPM {
+	warnings = append(warnings, metadataResult.warnings...)
-		bpmChan = make(chan provider.BPMKey, 1)
-		bpmErrChan = make(chan error, 1)
-		go func() {
-			p := provider.BPMProvider{}
-			bpmKey, err := p.Fetch(ctx, c.deezerClient.Session.HttpClient, song.Artist, song.Title, song.Duration)
-			if err != nil {
-				bpmErrChan <- err
-				return
-			}
-
-			bpmChan <- bpmKey
-		}()
-	}
-
-	var genreChan chan string
-	var genreErrChan chan error
-	if opts.Genre {
-		genreChan = make(chan string, 1)
-		genreErrChan = make(chan error, 1)
-		go func() {
-			p := provider.GenreProvider{}
-			genre, err := p.Fetch(ctx, c.deezerClient.Session.HttpClient, song.Artist, song.GetTitle())
-			if err != nil {
-				genreErrChan <- err
-				return
-			}
-
-			genreChan <- genre
-		}()
-	}
 
 	stream, err := c.deezerClient.GetMediaStream(ctx, media, song.ID)
 	if err != nil {
-		return nil, fmt.Errorf("failed to get media stream: %w", err)
+		return handleError(fmt.Errorf("failed to get media stream: %w", err))
 	}
 
 	dlCtx, cancel := context.WithTimeout(ctx, opts.Timeout)
@@ -240,30 +184,7 @@ func (c *Client) downloadSong(ctx context.Context, resource deezer.Resource, son
 	key := crypto.GetKey(c.appConfig.SecretKey, song.ID)
 	if err := c.streamToFile(dlCtx, stream, outputPath, key); err != nil {
 		fileutil.DeleteFile(outputPath)
-
+		return handleError(fmt.Errorf("failed to stream to file: %w", err))
-		return nil, fmt.Errorf("failed to stream to file: %w", err)
-	}
-
-	bpmKey := provider.BPMKey{}
-	if opts.BPM {
-		select {
-		case bpmKey = <-bpmChan:
-		case err := <-bpmErrChan:
-			if !errors.Is(err, context.Canceled) {
-				warnings = append(warnings, fmt.Sprintf("failed to fetch BPM and key: %v", err))
-			}
-		}
-	}
-
-	var genre string
-	if opts.Genre {
-		select {
-		case genre = <-genreChan:
-		case err := <-genreErrChan:
-			if !errors.Is(err, context.Canceled) {
-				warnings = append(warnings, fmt.Sprintf("failed to fetch genre: %v", err))
-			}
-		}
 	}
 
 	if opts.Quality != strings.ToLower(mediaFormat) {
@@ -275,9 +196,13 @@ func (c *Client) downloadSong(ctx context.Context, resource deezer.Resource, son
 		warnings = append(warnings, fmt.Sprintf("failed to fetch cover image: %v", err))
 	}
 
-	warnings = append(warnings, c.finalizeDownload(resource, song, outputPath, mediaFormat, genre, cover, bpmKey)...)
+	finalizeWarnings := c.finalizeDownload(resource, song, outputPath, mediaFormat, metadataResult.genre, cover, metadataResult.bpmKey)
+	warnings = append(warnings, finalizeWarnings...)
 
-	return warnings, nil
+	return downloadResult{
+		success:  true,
+		warnings: warnings,
+	}
 }
 
 func (c *Client) streamToFile(ctx context.Context, stream io.ReadCloser, outputPath string, key []byte) error {

internal/downloader/errors.go

@@ -0,0 +1,15 @@
+package downloader
+
+func handleError(err error) downloadResult {
+	if path, ok := IsSkipError(err); ok {
+		return downloadResult{
+			skipped: true,
+			path:    path,
+		}
+	}
+
+	return downloadResult{
+		success: false,
+		err:     err,
+	}
+}

internal/downloader/metadata.go

@@ -0,0 +1,92 @@
+package downloader
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net/http"
+
+	"github.com/mathismqn/godeez/internal/deezer"
+	"github.com/mathismqn/godeez/internal/provider"
+)
+
+type metadataResult struct {
+	bpmKey   provider.BPMKey
+	genre    string
+	warnings []string
+}
+
+type metadataFetcher struct {
+	httpClient *http.Client
+}
+
+func newMetadataFetcher(httpClient *http.Client) *metadataFetcher {
+	return &metadataFetcher{
+		httpClient: httpClient,
+	}
+}
+
+func (mf *metadataFetcher) fetch(ctx context.Context, song *deezer.Song, opts Options) metadataResult {
+	result := metadataResult{
+		bpmKey:   provider.BPMKey{},
+		genre:    "",
+		warnings: []string{},
+	}
+
+	if !opts.BPM && !opts.Genre {
+		return result
+	}
+
+	bmpChan := make(chan provider.BPMKey, 1)
+	bmpErrChan := make(chan error, 1)
+	genreChan := make(chan string, 1)
+	genreErrChan := make(chan error, 1)
+
+	if opts.BPM {
+		go func() {
+			p := provider.BPMProvider{}
+			bmpKey, err := p.Fetch(ctx, mf.httpClient, song.Artist, song.Title, song.Duration)
+			if err != nil {
+				bmpErrChan <- err
+			} else {
+				bmpChan <- bmpKey
+			}
+		}()
+	}
+
+	if opts.Genre {
+		go func() {
+			p := provider.GenreProvider{}
+			genre, err := p.Fetch(ctx, mf.httpClient, song.Artist, song.GetTitle())
+			if err != nil {
+				genreErrChan <- err
+			} else {
+				genreChan <- genre
+			}
+		}()
+	}
+
+	if opts.BPM {
+		select {
+		case bmpKey := <-bmpChan:
+			result.bpmKey = bmpKey
+		case err := <-bmpErrChan:
+			if !errors.Is(err, context.Canceled) {
+				result.warnings = append(result.warnings, fmt.Sprintf("failed to fetch BPM and key: %v", err))
+			}
+		}
+	}
+
+	if opts.Genre {
+		select {
+		case genre := <-genreChan:
+			result.genre = genre
+		case err := <-genreErrChan:
+			if !errors.Is(err, context.Canceled) {
+				result.warnings = append(result.warnings, fmt.Sprintf("failed to fetch genre: %v", err))
+			}
+		}
+	}
+
+	return result
+}

internal/downloader/progress.go

@@ -0,0 +1,113 @@
+package downloader
+
+import (
+	"fmt"
+	"os"
+	"time"
+
+	"github.com/briandowns/spinner"
+	"github.com/mathismqn/godeez/internal/deezer"
+	"github.com/mathismqn/godeez/internal/logger"
+)
+
+type downloadStats struct {
+	downloaded int
+	skipped    int
+	failed     int
+}
+
+type downloadResult struct {
+	success  bool
+	skipped  bool
+	path     string
+	warnings []string
+	err      error
+}
+
+type progressTracker struct {
+	logger       *logger.Logger
+	stats        *downloadStats
+	totalSongs   int
+	resourceType string
+}
+
+func newProgressTracker(logger *logger.Logger, totalSongs int, resourceType string) *progressTracker {
+	return &progressTracker{
+		logger:       logger,
+		stats:        &downloadStats{},
+		totalSongs:   totalSongs,
+		resourceType: resourceType,
+	}
+}
+
+func (pt *progressTracker) startDownload(index int, song *deezer.Song) *spinner.Spinner {
+	songTitle := song.GetTitle()
+	trackProgress := fmt.Sprintf("[%d/%d]", index+1, pt.totalSongs)
+
+	sp := spinner.New(spinner.CharSets[14], 100*time.Millisecond)
+	sp.Writer = os.Stdout
+	sp.Prefix = trackProgress + " "
+	sp.Suffix = fmt.Sprintf(" Downloading: %s - %s", song.Artist, songTitle)
+	sp.Start()
+
+	return sp
+}
+
+func (pt *progressTracker) handleResult(index int, song *deezer.Song, result downloadResult) {
+	trackProgress := fmt.Sprintf("[%d/%d]", index+1, pt.totalSongs)
+	songTitle := song.GetTitle()
+
+	if result.skipped {
+		pt.stats.skipped++
+		fmt.Printf("%s ↷ Skipped: %s - %s\n    Already exists at: %s\n",
+			trackProgress, song.Artist, songTitle, result.path)
+		return
+	}
+
+	if result.err != nil {
+		pt.stats.failed++
+		pt.logger.Errorf("Failed to download %s - %s: %v\n", song.Artist, songTitle, result.err)
+		fmt.Printf("%s ✖ Failed: %s - %s:\n    Error: %v\n",
+			trackProgress, song.Artist, songTitle, result.err)
+		return
+	}
+
+	symbol := "✔"
+	if len(result.warnings) > 0 {
+		symbol = "⚠"
+	}
+
+	pt.stats.downloaded++
+	pt.logger.Infof("Downloaded %s - %s\n", song.Artist, songTitle)
+	fmt.Printf("%s %s Downloaded: %s - %s\n", trackProgress, symbol, song.Artist, songTitle)
+
+	for _, w := range result.warnings {
+		pt.logger.Warnf("Warning: %s\n", w)
+		fmt.Printf("    Warning: %s\n", w)
+	}
+}
+
+func (pt *progressTracker) printSummary(resourceTitle, resourceID, outputDir string, elapsed time.Duration) {
+	if pt.stats.downloaded > 0 || pt.stats.failed > 0 {
+		pt.logger.Infof("Resource %s (%s): %d downloaded, %d skipped, %d failed\n",
+			resourceTitle, resourceID, pt.stats.downloaded, pt.stats.skipped, pt.stats.failed)
+	}
+
+	if pt.resourceType != "track" {
+		fmt.Printf(`
+================== [ Summary ] ==================
+Downloaded:     %d
+Skipped:        %d
+Failed:         %d
+Elapsed time:   %s
+Files saved to: %s
+=================================================
+`,
+			pt.stats.downloaded,
+			pt.stats.skipped,
+			pt.stats.failed,
+			elapsed.Round(time.Second),
+			outputDir,
+		)
+	}
+}