Merge pull request #276 from rusq/i270

i270

downloader/downloader.go

@@ -239,7 +239,7 @@ func (c *Client) saveFile(ctx context.Context, dir string, sf *slack.File) (int6
 	if c.fs == nil {
 		return 0, ErrNoFS
 	}
-	if mode := sf.Mode; mode == "hidden_by_limit" || mode == "external" || sf.IsExternal {
+	if !IsValid(sf) {
 		trace.Logf(ctx, "info", "file %q is not downloadable", sf.Name)
 		return 0, nil
 	}
@@ -337,3 +337,20 @@ func (c *Client) l() logger.Interface {
 	}
 	return c.dlog
 }
+
+var invalidModes = map[string]struct{}{
+	"hidden_by_limit": {},
+	"external":        {},
+	"tombstone":       {},
+}
+
+// IsValid returns true if the file can be downloaded and is valid.
+func IsValid(f *slack.File) bool {
+	if f == nil {
+		return false
+	}
+	if _, ok := invalidModes[f.Mode]; ok {
+		return false
+	}
+	return !f.IsExternal && f.Name != ""
+}

downloader/downloader_test.go

@@ -2,23 +2,21 @@ package downloader
 
 import (
 	"context"
+	"errors"
 	"os"
 	"path"
 	"path/filepath"
 	"testing"
 	"time"
 
-	"errors"
-
+	"github.com/rusq/slackdump/v2/fsadapter"
+	"github.com/rusq/slackdump/v2/internal/fixtures"
+	"github.com/rusq/slackdump/v2/internal/mocks/mock_downloader"
 	"github.com/slack-go/slack"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
 	"golang.org/x/time/rate"
-
-	"github.com/rusq/slackdump/v2/fsadapter"
-	"github.com/rusq/slackdump/v2/internal/fixtures"
-	"github.com/rusq/slackdump/v2/internal/mocks/mock_downloader"
 )
 
 var (
@@ -466,3 +464,62 @@ func TestClient_DownloadFile(t *testing.T) {
 		c.Stop()
 	})
 }
+
+func TestIsValid(t *testing.T) {
+	type args struct {
+		f *slack.File
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			"valid file",
+			args{&file1},
+			true,
+		},
+		{
+			"tombstone",
+			args{&slack.File{Mode: "tombstone", Name: "foo"}},
+			false,
+		},
+		{
+			"external file",
+			args{&slack.File{Mode: "external", Name: "foo", IsExternal: true}},
+			false,
+		},
+		{
+			"hidden by limit",
+			args{&slack.File{Mode: "hidden_by_limit", Name: "foo"}},
+			false,
+		},
+		{
+			"tombstone",
+			args{&slack.File{Mode: "tombstone", Name: "foo"}},
+			false,
+		},
+		{
+			"external file",
+			args{&slack.File{Mode: "", Name: "foo", IsExternal: true}},
+			false,
+		},
+		{
+			"empty name",
+			args{&slack.File{Mode: "", Name: "", IsExternal: false}},
+			false,
+		},
+		{
+			"nil file",
+			args{nil},
+			false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsValid(tt.args.f); got != tt.want {
+				t.Errorf("IsValid() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}