diff --git a/channels.go b/channels.go index 4c91942e..ef047726 100644 --- a/channels.go +++ b/channels.go @@ -79,14 +79,14 @@ func (s *Session) getChannels(ctx context.Context, chanTypes []string, cb func(t } total += len(chans) - s.l().Printf("channels request #%5d, fetched: %4d, total: %8d (speed: %6.2f/sec, avg: %6.2f/sec)\n", + s.log.Printf("channels request #%5d, fetched: %4d, total: %8d (speed: %6.2f/sec, avg: %6.2f/sec)\n", i, len(chans), total, float64(len(chans))/float64(time.Since(reqStart).Seconds()), float64(total)/float64(time.Since(fetchStart).Seconds()), ) if nextcur == "" { - s.l().Printf("channels fetch complete, total: %d channels", total) + s.log.Printf("channels fetch complete, total: %d channels", total) break } diff --git a/channels_test.go b/channels_test.go index 7bbcbe12..2733a1fa 100644 --- a/channels_test.go +++ b/channels_test.go @@ -10,6 +10,7 @@ import ( "github.com/slack-go/slack" "github.com/stretchr/testify/assert" + "github.com/rusq/slackdump/v2/logger" "github.com/rusq/slackdump/v2/types" ) @@ -80,6 +81,7 @@ func TestSession_getChannels(t *testing.T) { client: mc, Users: tt.fields.Users, cfg: tt.fields.options, + log: logger.Silent, } if tt.expectFn != nil { diff --git a/cmd/slackdump/internal/cfg/cfg.go b/cmd/slackdump/internal/cfg/cfg.go index d5e6b1f6..8e8b62a6 100644 --- a/cmd/slackdump/internal/cfg/cfg.go +++ b/cmd/slackdump/internal/cfg/cfg.go @@ -11,6 +11,7 @@ import ( "github.com/rusq/slackdump/v2" "github.com/rusq/slackdump/v2/auth/browser" + "github.com/rusq/slackdump/v2/logger" ) var ( @@ -25,6 +26,8 @@ var ( SlackCookie string Browser browser.Browser SlackConfig = slackdump.DefOptions + + Log logger.Interface ) type FlagMask int diff --git a/cmd/slackdump/internal/dump/dump.go b/cmd/slackdump/internal/dump/dump.go index c8aefb4b..c73cd88e 100644 --- a/cmd/slackdump/internal/dump/dump.go +++ b/cmd/slackdump/internal/dump/dump.go @@ -100,9 +100,7 @@ func RunDump(ctx context.Context, cmd *base.Command, args []string) error { return fmt.Errorf("file template error: %w", err) } - cfg.SlackConfig.Logger = dlog.FromContext(ctx) - - sess, err := slackdump.New(ctx, prov, cfg.SlackConfig) + sess, err := slackdump.New(ctx, prov, cfg.SlackConfig, slackdump.WithLogger(dlog.FromContext(ctx))) if err != nil { base.SetExitStatus(base.SApplicationError) return err diff --git a/cmd/slackdump/internal/v1/main.go b/cmd/slackdump/internal/v1/main.go index 7c256ea2..353d2be0 100644 --- a/cmd/slackdump/internal/v1/main.go +++ b/cmd/slackdump/internal/v1/main.go @@ -170,7 +170,7 @@ func run(ctx context.Context, p params) error { ctx = dlog.NewContext(ctx, lg) // - setting the logger for the application. - p.appCfg.SlackConfig.Logger = lg + p.appCfg.Log = lg // - trace init if traceStopFn, err := initTrace(lg, p.traceFile); err != nil { diff --git a/cmd/slackdump/internal/workspace/list.go b/cmd/slackdump/internal/workspace/list.go index 21a4bd30..acfbc86e 100644 --- a/cmd/slackdump/internal/workspace/list.go +++ b/cmd/slackdump/internal/workspace/list.go @@ -93,7 +93,6 @@ func printAll(m manager, current string, wsps []string) { fmt.Fprintln(tw, "C\tname\tfilename\tmodified\tteam\tuser\terror\n"+ "-\t-------\t------------\t-------------------\t---------\t--------\t-----") - cfg.SlackConfig.Logger = logger.Silent cfg.SlackConfig.UserCache.Disabled = true for _, name := range wsps { curr := "" @@ -119,7 +118,7 @@ func userInfo(ctx context.Context, m manager, name string) (*slack.AuthTestRespo if err != nil { return nil, err } - sess, err := slackdump.New(ctx, prov, cfg.SlackConfig) + sess, err := slackdump.New(ctx, prov, cfg.SlackConfig, slackdump.WithLogger(logger.Silent)) if err != nil { return nil, err } diff --git a/cmd/slackdump/main.go b/cmd/slackdump/main.go index f2ec9e55..95728070 100644 --- a/cmd/slackdump/main.go +++ b/cmd/slackdump/main.go @@ -164,7 +164,7 @@ func invoke(cmd *base.Command, args []string) error { } else { lg.SetPrefix(cmd.Name() + ": ") ctx = dlog.NewContext(ctx, lg) - cfg.SlackConfig.Logger = lg + cfg.Log = lg } if cmd.RequireAuth { diff --git a/config.go b/config.go index 5193926b..2c18b618 100644 --- a/config.go +++ b/config.go @@ -10,8 +10,6 @@ import ( ut "github.com/go-playground/universal-translator" "github.com/go-playground/validator/v10" translations "github.com/go-playground/validator/v10/translations/en" - - "github.com/rusq/slackdump/v2/logger" ) // Config is the option set for the Session. @@ -24,7 +22,7 @@ type Config struct { UserCache CacheConfig BaseLocation string // base location for the dump files - Logger logger.Interface + Logfile string } // CacheConfig represents the options for the cache. @@ -92,9 +90,9 @@ var DefOptions = Config{ }, DumpFiles: false, UserCache: CacheConfig{Filename: "users.cache", MaxAge: 4 * time.Hour}, - CacheDir: "", // default cache dir - Logger: logger.Default, // default logger is the... default logger - BaseLocation: ".", // default location is the current directory + CacheDir: "", // default cache dir + BaseLocation: ".", // default location is the current directory + Logfile: "", // empty, means STDERR } var ( diff --git a/export/expfmt_test.go b/export/expfmt_test.go index a676beb4..28f8cd4d 100644 --- a/export/expfmt_test.go +++ b/export/expfmt_test.go @@ -22,10 +22,10 @@ func Test_newFileExporter(t *testing.T) { args args wantT string }{ - {"unknown is nodownload", args{t: ExportType(255), l: logger.Default, token: "abcd"}, "dl.Nothing"}, - {"no", args{t: TNoDownload, l: logger.Default, token: "abcd"}, "dl.Nothing"}, - {"standard", args{t: TStandard, fs: fsadapter.NewDirectory("."), cl: &slack.Client{}, l: logger.Default, token: "abcd"}, "*dl.Std"}, - {"mattermost", args{t: TMattermost, fs: fsadapter.NewDirectory("."), cl: &slack.Client{}, l: logger.Default, token: "abcd"}, "*dl.Mattermost"}, + {"unknown is nodownload", args{t: ExportType(255), l: logger.Silent, token: "abcd"}, "dl.Nothing"}, + {"no", args{t: TNoDownload, l: logger.Silent, token: "abcd"}, "dl.Nothing"}, + {"standard", args{t: TStandard, fs: fsadapter.NewDirectory("."), cl: &slack.Client{}, l: logger.Silent, token: "abcd"}, "*dl.Std"}, + {"mattermost", args{t: TMattermost, fs: fsadapter.NewDirectory("."), cl: &slack.Client{}, l: logger.Silent, token: "abcd"}, "*dl.Mattermost"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/fsadapter/zipfs_test.go b/fsadapter/zipfs_test.go index ab59dbbd..48013726 100644 --- a/fsadapter/zipfs_test.go +++ b/fsadapter/zipfs_test.go @@ -145,60 +145,64 @@ func TestNewZIP(t *testing.T) { } func TestCreateConcurrency(t *testing.T) { - // test for GH issue#90 - race condition in ZIP.Create - const ( - numRoutines = 16 - testContentsSz = 1 * (1 << 20) - ) - - var buf bytes.Buffer - var wg sync.WaitGroup - - zw := zip.NewWriter(&buf) - defer zw.Close() - - fsa := NewZIP(zw) - defer fsa.Close() - - // prepare workers - readySteadyGo := make(chan struct{}) - panicAttacks := make(chan any, numRoutines) - - for i := 0; i < numRoutines; i++ { - wg.Add(1) - go func(n int) { - defer func() { - if r := recover(); r != nil { - panicAttacks <- fmt.Sprintf("ZIP.Create race condition in gr %d: %v", n, r) + t.Parallel() + t.Run("issue#90", func(t *testing.T) { + t.Parallel() + // test for GH issue#90 - race condition in ZIP.Create + const ( + numRoutines = 16 + testContentsSz = 1 * (1 << 20) + ) + + var buf bytes.Buffer + var wg sync.WaitGroup + + zw := zip.NewWriter(&buf) + defer zw.Close() + + fsa := NewZIP(zw) + defer fsa.Close() + + // prepare workers + readySteadyGo := make(chan struct{}) + panicAttacks := make(chan any, numRoutines) + + for i := 0; i < numRoutines; i++ { + wg.Add(1) + go func(n int) { + defer func() { + if r := recover(); r != nil { + panicAttacks <- fmt.Sprintf("ZIP.Create race condition in gr %d: %v", n, r) + } + }() + + defer wg.Done() + var contents bytes.Buffer + if _, err := io.CopyN(&contents, rand.Reader, testContentsSz); err != nil { + panic(err) } - }() - defer wg.Done() - var contents bytes.Buffer - if _, err := io.CopyN(&contents, rand.Reader, testContentsSz); err != nil { - panic(err) - } - - <-readySteadyGo - fw, err := fsa.Create(fmt.Sprintf("file%d", n)) - if err != nil { - panic(err) - } - defer fw.Close() + <-readySteadyGo + fw, err := fsa.Create(fmt.Sprintf("file%d", n)) + if err != nil { + panic(err) + } + defer fw.Close() - if _, err := io.Copy(fw, &contents); err != nil { - panic(err) + if _, err := io.Copy(fw, &contents); err != nil { + panic(err) + } + }(i) + } + close(readySteadyGo) + wg.Wait() + close(panicAttacks) + for r := range panicAttacks { + if r != nil { + t.Error(r) } - }(i) - } - close(readySteadyGo) - wg.Wait() - close(panicAttacks) - for r := range panicAttacks { - if r != nil { - t.Error(r) } - } + }) } func TestZIP_normalizePath(t *testing.T) { diff --git a/internal/app/config/config.go b/internal/app/config/config.go index a41e9ba3..2e6d9990 100644 --- a/internal/app/config/config.go +++ b/internal/app/config/config.go @@ -43,6 +43,8 @@ type Params struct { Emoji EmojiParams SlackConfig slackdump.Config + + Log logger.Interface } type EmojiParams struct { @@ -160,8 +162,8 @@ func (in Input) Producer(fn func(string) error) error { } func (p *Params) Logger() logger.Interface { - if p.SlackConfig.Logger == nil { + if p.Log == nil { return logger.Default } - return p.SlackConfig.Logger + return p.Log } diff --git a/internal/network/network.go b/internal/network/network.go index 972407cd..c37d5a57 100644 --- a/internal/network/network.go +++ b/internal/network/network.go @@ -83,10 +83,13 @@ func WithRetry(ctx context.Context, l *rate.Limiter, maxAttempts int, fn func() } // waitTime returns the amount of time to wait before retrying depending on -// the current attempt. The wait time is calculated as (x+2)^3 seconds, where -// x is the current attempt number. The maximum wait time is capped at 5 +// the current attempt. This variable exists to reduce the test time. +var waitTime = cubicWait + +// cubicWait is the wait time function. Time is calculated as (x+2)^3 seconds, +// where x is the current attempt number. The maximum wait time is capped at 5 // minutes. -func waitTime(attempt int) time.Duration { +func cubicWait(attempt int) time.Duration { x := attempt + 2 // this is to ensure that we sleep at least 8 seconds. delay := time.Duration(x*x*x) * time.Second if delay > MaxAllowedWaitTime { diff --git a/internal/network/network_test.go b/internal/network/network_test.go index 0ef060fe..6f3f047c 100644 --- a/internal/network/network_test.go +++ b/internal/network/network_test.go @@ -148,14 +148,16 @@ func Test_withRetry(t *testing.T) { } func Test500ErrorHandling(t *testing.T) { - t.Parallel() + waitTime = func(attempt int) time.Duration { return 50 * time.Millisecond } + defer func() { + waitTime = cubicWait + }() var codes = []int{500, 502, 503, 504, 598} for _, code := range codes { var thisCode = code // This test is to ensure that we handle 500 errors correctly. t.Run(fmt.Sprintf("%d error", code), func(t *testing.T) { - t.Parallel() const ( testRetryCount = 1 @@ -227,7 +229,7 @@ func Test500ErrorHandling(t *testing.T) { }) } -func Test_waitTime(t *testing.T) { +func Test_cubicWait(t *testing.T) { type args struct { attempt int } @@ -245,7 +247,7 @@ func Test_waitTime(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := waitTime(tt.args.attempt); !reflect.DeepEqual(got, tt.want) { + if got := cubicWait(tt.args.attempt); !reflect.DeepEqual(got, tt.want) { t.Errorf("waitTime() = %v, want %v", got, tt.want) } }) diff --git a/logger/logger.go b/logger/logger.go index a7ce0b49..9847d22d 100644 --- a/logger/logger.go +++ b/logger/logger.go @@ -1,13 +1,13 @@ package logger import ( - "io" "log" "os" "github.com/rusq/dlog" ) +// Interface is the interface for a logger. type Interface interface { Debug(...any) Debugf(fmt string, a ...any) @@ -16,8 +16,20 @@ type Interface interface { Println(...any) } +// Default is the default logger. It logs to stderr and debug logging can be +// enabled by setting the DEBUG environment variable to 1. For example: +// +// DEBUG=1 slackdump var Default = dlog.New(log.Default().Writer(), "", log.LstdFlags, os.Getenv("DEBUG") == "1") -// note: previously ioutil.Discard which is not deprecated in favord of io.Discard -// so this is valid only from go1.16 -var Silent = dlog.New(io.Discard, "", log.LstdFlags, false) +// Silent is a logger that does not log anything. +var Silent = silent{} + +// Silent is a logger that does not log anything. +type silent struct{} + +func (s silent) Debug(...any) {} +func (s silent) Debugf(fmt string, a ...any) {} +func (s silent) Print(...any) {} +func (s silent) Printf(fmt string, a ...any) {} +func (s silent) Println(...any) {} diff --git a/logger/logger_test.go b/logger/logger_test.go new file mode 100644 index 00000000..fc5f53c8 --- /dev/null +++ b/logger/logger_test.go @@ -0,0 +1,16 @@ +package logger + +import "testing" + +func BenchmarkSlientPrintf(b *testing.B) { + var l = Silent + for i := 0; i < b.N; i++ { + l.Printf("hello world, %s, %d", "foo", i) + } + // This benchmark compares the performance of the Silent logger when + // using io.Discard, and when using a no-op function. + // io.Discard: BenchmarkSlientPrintf-16 93075956 12.92 ns/op 8 B/op 0 allocs/op + // no-op func: BenchmarkSlientPrintf-16 1000000000 0.2364 ns/op 0 B/op 0 allocs/op + // + // Oh, look! We have an WINNER. The no-op function wins, no surprises. +} diff --git a/messages.go b/messages.go index 85f3c309..be36d00f 100644 --- a/messages.go +++ b/messages.go @@ -141,14 +141,14 @@ func (s *Session) dumpChannel(ctx context.Context, channelID string, oldest, lat messages = append(messages, chunk...) - s.l().Printf("messages request #%5d, fetched: %4d (%s), total: %8d (speed: %6.2f/sec, avg: %6.2f/sec)\n", + s.log.Printf("messages request #%5d, fetched: %4d (%s), total: %8d (speed: %6.2f/sec, avg: %6.2f/sec)\n", i, len(resp.Messages), results, len(messages), float64(len(resp.Messages))/float64(time.Since(reqStart).Seconds()), float64(len(messages))/float64(time.Since(fetchStart).Seconds()), ) if !resp.HasMore { - s.l().Printf("messages fetch complete, total: %d", len(messages)) + s.log.Printf("messages fetch complete, total: %d", len(messages)) break } diff --git a/messages_test.go b/messages_test.go index 653744c8..3103255e 100644 --- a/messages_test.go +++ b/messages_test.go @@ -13,6 +13,7 @@ import ( "github.com/rusq/slackdump/v2/internal/fixtures" "github.com/rusq/slackdump/v2/internal/network" + "github.com/rusq/slackdump/v2/logger" "github.com/rusq/slackdump/v2/types" ) @@ -220,6 +221,7 @@ func TestSession_DumpMessages(t *testing.T) { client: mc, Users: tt.fields.Users, cfg: tt.fields.options, + log: logger.Silent, } got, err := sd.DumpAll(tt.args.ctx, tt.args.channelID) if (err != nil) != tt.wantErr { @@ -302,6 +304,7 @@ func TestSession_DumpAll(t *testing.T) { client: mc, Users: tt.fields.Users, cfg: tt.fields.options, + log: logger.Silent, } got, err := sd.DumpAll(tt.args.ctx, tt.args.slackURL) if (err != nil) != tt.wantErr { diff --git a/processors.go b/processors.go index 2e64eab5..f3000bfa 100644 --- a/processors.go +++ b/processors.go @@ -80,7 +80,7 @@ func (s *Session) newFileProcessFn(ctx context.Context, dir string, l *rate.Limi downloader.Limiter(l), downloader.Retries(s.cfg.Limits.DownloadRetries), downloader.Workers(s.cfg.Limits.Workers), - downloader.Logger(s.l()), + downloader.Logger(s.log), ) var filesC = make(chan *slack.File, filesCbufSz) diff --git a/slackdump.go b/slackdump.go index 2f5ea166..e0c468cc 100644 --- a/slackdump.go +++ b/slackdump.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "log" "os" "runtime/trace" @@ -12,6 +13,7 @@ import ( "github.com/slack-go/slack" "golang.org/x/time/rate" + "github.com/rusq/dlog" "github.com/rusq/slackdump/v2/auth" "github.com/rusq/slackdump/v2/fsadapter" "github.com/rusq/slackdump/v2/internal/network" @@ -24,7 +26,8 @@ import ( //go:generate sh -c "mockgen -source slackdump.go -destination clienter_mock_test.go -package slackdump -mock_names clienter=mockClienter,Reporter=mockReporter" //go:generate sed -i ~ -e "s/NewmockClienter/newmockClienter/g" -e "s/NewmockReporter/newmockReporter/g" clienter_mock_test.go -// Session stores basic session parameters. +// Session stores basic session parameters. Zero value is not usable, must be +// initialised with New. type Session struct { client clienter // Slack client @@ -33,8 +36,10 @@ type Session struct { // Users contains the list of users and populated on NewSession Users types.Users `json:"users"` - fs fsadapter.FSCloser // filesystem adapter - ownFS bool // whether the filesystem adapter was created by the session + fs fsadapter.FS // filesystem adapter + log logger.Interface // logger + + atClose []func() error // functions to call on exit cfg Config } @@ -77,6 +82,18 @@ func WithFilesystem(fs fsadapter.FSCloser) Option { } } +// WithLogger sets the logger to use for the session. If this option is not +// given, the default logger is initialised with the filename specified in +// Config.Logfile. If the Config.Logfile is empty, the default logger writes +// to STDERR. +func WithLogger(l logger.Interface) Option { + return func(s *Session) { + if l != nil { + s.log = l + } + } +} + // New creates new Slackdump session with provided options, and populates the // internal cache of users and channels for lookups. If it fails to // authenticate, AuthError is returned. @@ -106,22 +123,23 @@ func New(ctx context.Context, prov auth.Provider, cfg Config, opts ...Option) (* client: cl, cfg: cfg, wspInfo: authTestResp, + log: logger.Default, } for _, opt := range opts { opt(sd) } if sd.fs == nil { - // if no filesystem adapter is provided through Options, initialise - // the default one. - var err error - sd.fs, err = fsadapter.New(cfg.BaseLocation) - if err != nil { + if err := sd.openFS(cfg.BaseLocation); err != nil { return nil, fmt.Errorf("failed to initialise filesystem adapter: %s", err) } - sd.ownFS = true + } + if sd.log == nil { + if err := sd.openLogger(cfg.Logfile); err != nil { + return nil, fmt.Errorf("failed to initialise logger: %s", err) + } } - sd.propagateLogger(sd.l()) + sd.propagateLogger() if err := os.MkdirAll(cfg.CacheDir, 0700); err != nil { return nil, fmt.Errorf("failed to create the cache directory: %s", err) @@ -144,18 +162,51 @@ func (s *Session) Client() *slack.Client { return s.client.(*slack.Client) } +func (s *Session) openFS(loc string) error { + // if no filesystem adapter is provided through Options, initialise + // the default one. + fs, err := fsadapter.New(loc) + if err != nil { + return err + } + s.fs = fs + s.atClose = append(s.atClose, func() error { + return fs.Close() + }) + return nil +} + // Filesystem returns the filesystem adapter used by the session. func (s *Session) Filesystem() fsadapter.FS { return s.fs } +func (s *Session) openLogger(filename string) error { + if filename == "" { + s.log = logger.Default + return nil + } + f, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0600) + if err != nil { + return err + } + s.atClose = append(s.atClose, func() error { + return f.Close() + }) + s.log = dlog.New(f, "", log.LstdFlags, false) + return nil +} + // Close closes the handles if they were created by the session. // It must be called when the session is no longer needed. func (s *Session) Close() error { - if s.ownFS { - return s.fs.Close() + var last error + for _, fn := range s.atClose { + if err := fn(); err != nil { + last = err + } } - return nil + return last } // Me returns the current authenticated user in a rather dirty manner. @@ -190,17 +241,9 @@ func withRetry(ctx context.Context, l *rate.Limiter, maxAttempts int, fn func() return network.WithRetry(ctx, l, maxAttempts, fn) } -// l returns the current logger. -func (s *Session) l() logger.Interface { - if s.cfg.Logger == nil { - return logger.Default - } - return s.cfg.Logger -} - // propagateLogger propagates the slackdump logger to some dumb packages. -func (s *Session) propagateLogger(l logger.Interface) { - network.Logger = l +func (s *Session) propagateLogger() { + network.Logger = s.log } // Info returns a workspace information. Slackdump retrieves workspace diff --git a/slackdump_test.go b/slackdump_test.go index e87d461f..c0940618 100644 --- a/slackdump_test.go +++ b/slackdump_test.go @@ -5,19 +5,18 @@ import ( "log" "math" "os" + "path/filepath" "reflect" "testing" "time" - "github.com/rusq/dlog" - "github.com/slack-go/slack" - "github.com/stretchr/testify/assert" - "github.com/rusq/slackdump/v2/auth" "github.com/rusq/slackdump/v2/internal/fixtures" "github.com/rusq/slackdump/v2/internal/network" "github.com/rusq/slackdump/v2/logger" "github.com/rusq/slackdump/v2/types" + "github.com/slack-go/slack" + "github.com/stretchr/testify/assert" ) func Test_newLimiter(t *testing.T) { @@ -175,45 +174,75 @@ func TestSession_Me(t *testing.T) { } } -func TestSession_l(t *testing.T) { - testLg := dlog.New(os.Stderr, "TEST", log.LstdFlags, false) - type fields struct { - client clienter - wspInfo *slack.AuthTestResponse - Users types.Users - options Config - } - tests := []struct { - name string - fields fields - want logger.Interface - }{ - { - "empty returns the default logger", - fields{ - options: Config{}, - }, - logger.Default, - }, - { - "if logger is set, it returns the custom logger", - fields{ - options: Config{Logger: testLg}, - }, - logger.Interface(testLg), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - sd := &Session{ - client: tt.fields.client, - wspInfo: tt.fields.wspInfo, - Users: tt.fields.Users, - cfg: tt.fields.options, - } - if got := sd.l(); !reflect.DeepEqual(got, tt.want) { - t.Errorf("Session.l() = %v, want %v", got, tt.want) - } - }) - } +func TestSession_openFS(t *testing.T) { + t.Run("ensure that fs os open and close function added", func(t *testing.T) { + var sd = new(Session) + dir := t.TempDir() + sd.cfg = Config{} + testFile := filepath.Join(dir, "test.zip") + + assert.NoError(t, sd.openFS(testFile)) + assert.NotNil(t, sd.fs) + assert.Len(t, sd.atClose, 1) + assert.NoError(t, sd.Close()) + assert.FileExists(t, testFile) + }) + t.Run("ensure works with directory", func(t *testing.T) { + var sd = new(Session) + dir := t.TempDir() + sd.cfg = Config{} + testDir := filepath.Join(dir, "test") + + assert.NoError(t, sd.openFS(testDir)) + assert.NotNil(t, sd.fs) + assert.Len(t, sd.atClose, 1) + + assert.NoError(t, sd.fs.WriteFile("test.txt", []byte("test"), 0644)) + + assert.NoError(t, sd.Close()) + assert.DirExists(t, testDir) + }) +} + +func TestSession_openLogger(t *testing.T) { + t.Run("empty filename should log to stderr", func(t *testing.T) { + var sd = new(Session) + sd.cfg = Config{} + assert.NoError(t, sd.openLogger("")) + assert.NotNil(t, sd.log) + assert.Equal(t, sd.log, logger.Default) + assert.Len(t, sd.atClose, 0) // no close function for stderr + assert.NoError(t, sd.Close()) + }) + t.Run("non-empty file creates a log file", func(t *testing.T) { + testLogFile(t, filepath.Join(t.TempDir(), "test.log"), "hello log") + }) + t.Run("new data is appended to log file if it exists", func(t *testing.T) { + testFile := filepath.Join(t.TempDir(), "test_again.log") + testLogFile(t, testFile, "hello log") + testLogFile(t, testFile, "hello again log") + + data, err := os.ReadFile(testFile) + assert.NoError(t, err) + assert.Contains(t, string(data), "hello log") + assert.Contains(t, string(data), "hello again log") + }) +} + +func testLogFile(t *testing.T, testFile string, msg string) { + var sd = new(Session) + sd.cfg = Config{} + + assert.NoError(t, sd.openLogger(testFile)) + assert.NotNil(t, sd.log) + assert.Len(t, sd.atClose, 1) + + sd.log.Print(msg) + + assert.NoError(t, sd.Close()) + assert.FileExists(t, testFile) + + data, err := os.ReadFile(testFile) + assert.NoError(t, err) + assert.Contains(t, string(data), msg) } diff --git a/thread.go b/thread.go index 15bc0e66..47cf4f89 100644 --- a/thread.go +++ b/thread.go @@ -146,7 +146,7 @@ func (s *Session) dumpThread( return nil, err } - s.l().Printf(" thread request #%5d, fetched: %4d, total: %8d, process results: %s (speed: %6.2f/sec, avg: %6.2f/sec)\n", + s.log.Printf(" thread request #%5d, fetched: %4d, total: %8d, process results: %s (speed: %6.2f/sec, avg: %6.2f/sec)\n", i+1, len(msgs), len(thread), prs, float64(len(msgs))/time.Since(reqStart).Seconds(), @@ -154,7 +154,7 @@ func (s *Session) dumpThread( ) if !hasmore { - s.l().Printf(" thread fetch complete, total: %d", len(thread)) + s.log.Printf(" thread fetch complete, total: %d", len(thread)) break } cursor = nextCursor diff --git a/thread_test.go b/thread_test.go index 93e8c4ba..d9bf4fb8 100644 --- a/thread_test.go +++ b/thread_test.go @@ -14,6 +14,7 @@ import ( "github.com/rusq/slackdump/v2/internal/network" "github.com/rusq/slackdump/v2/internal/structures" + "github.com/rusq/slackdump/v2/logger" "github.com/rusq/slackdump/v2/types" ) @@ -166,6 +167,7 @@ func TestSession_DumpThreadWithFiles(t *testing.T) { client: mc, Users: tt.fields.Users, cfg: tt.fields.options, + log: logger.Silent, } got, err := sd.dumpThreadAsConversation(tt.args.ctx, structures.SlackLink{Channel: tt.args.channelID, ThreadTS: tt.args.threadTS}, tt.args.oldest, tt.args.latest) if (err != nil) != tt.wantErr { @@ -384,6 +386,7 @@ func TestSession_dumpThread(t *testing.T) { client: mc, Users: tt.fields.Users, cfg: tt.fields.options, + log: logger.Silent, } got, err := sd.dumpThread(tt.args.ctx, tt.args.l, tt.args.channelID, tt.args.threadTS, tt.args.oldest, tt.args.latest) if (err != nil) != tt.wantErr { diff --git a/users.go b/users.go index cc3fb65d..09cedcd3 100644 --- a/users.go +++ b/users.go @@ -32,14 +32,14 @@ func (s *Session) GetUsers(ctx context.Context) (types.Users, error) { users, err := m.LoadUsers(s.wspInfo.TeamID, s.cfg.UserCache.MaxAge) if err != nil { - s.l().Println("caching users") + s.log.Println("caching users") users, err = s.fetchUsers(ctx) if err != nil { return nil, err } if err := m.SaveUsers(s.wspInfo.TeamID, users); err != nil { trace.Logf(ctx, "error", "saving user cache to %q, error: %s", s.cfg.UserCache.Filename, err) - s.l().Printf("error saving user cache to %q: %s, but nevermind, let's continue", s.cfg.UserCache.Filename, err) + s.log.Printf("error saving user cache to %q: %s, but nevermind, let's continue", s.cfg.UserCache.Filename, err) } } diff --git a/users_test.go b/users_test.go index 825a6550..d9d13287 100644 --- a/users_test.go +++ b/users_test.go @@ -15,6 +15,7 @@ import ( "github.com/rusq/slackdump/v2/internal/cache" "github.com/rusq/slackdump/v2/internal/fixtures" "github.com/rusq/slackdump/v2/internal/structures" + "github.com/rusq/slackdump/v2/logger" "github.com/rusq/slackdump/v2/types" ) @@ -177,6 +178,7 @@ func TestSession_GetUsers(t *testing.T) { wspInfo: &slack.AuthTestResponse{TeamID: testSuffix}, Users: tt.fields.Users, cfg: tt.fields.options, + log: logger.Silent, } got, err := sd.GetUsers(tt.args.ctx) if (err != nil) != tt.wantErr {