From cfbe5b5ac1a4672f4faf58c9c8d64ef2f775592e Mon Sep 17 00:00:00 2001 From: Rustam Gilyazov <16064414+rusq@users.noreply.github.com> Date: Sun, 10 Nov 2024 10:54:47 +1000 Subject: [PATCH] brush up listing users and channels --- cmd/slackdump/internal/list/channels.go | 152 ++++++++++-------- cmd/slackdump/internal/list/common.go | 98 +++++------ cmd/slackdump/internal/list/users.go | 79 ++++++--- cmd/slackdump/internal/list/users_test.go | 17 +- cmd/slackdump/internal/list/wizard.go | 24 +-- .../internal/ui/cfgui/configuration.go | 1 + internal/osext/move.go | 1 + 7 files changed, 205 insertions(+), 167 deletions(-) diff --git a/cmd/slackdump/internal/list/channels.go b/cmd/slackdump/internal/list/channels.go index 6d9a6363..7bebcc0e 100644 --- a/cmd/slackdump/internal/list/channels.go +++ b/cmd/slackdump/internal/list/channels.go @@ -3,10 +3,13 @@ package list import ( "context" "fmt" + "log" "runtime/trace" "time" + "github.com/rusq/slack" "github.com/rusq/slackdump/v3" + "github.com/rusq/slackdump/v3/cmd/slackdump/internal/bootstrap" "github.com/rusq/slackdump/v3/cmd/slackdump/internal/cfg" "github.com/rusq/slackdump/v3/cmd/slackdump/internal/golang/base" "github.com/rusq/slackdump/v3/internal/cache" @@ -15,7 +18,7 @@ import ( ) var CmdListChannels = &base.Command{ - Run: listChannels, + Run: runListChannels, UsageLine: "slackdump list channels [flags] [filename]", PrintFlags: true, FlagMask: cfg.OmitDownloadFlag, @@ -37,90 +40,109 @@ and -chan-cache-retention flags to control the cache behavior. RequireAuth: true, } -func init() { - CmdListChannels.Wizard = wizChannels -} - -type channelOptions struct { - noResolve bool - cache cacheOpts -} +type ( + channelOptions struct { + resolveUsers bool + cache cacheOpts + } -type cacheOpts struct { - Disabled bool - Retention time.Duration - Filename string -} + cacheOpts struct { + Enabled bool + Retention time.Duration + Filename string + } +) var chanFlags = channelOptions{ - noResolve: false, + resolveUsers: false, cache: cacheOpts{ - Disabled: false, + Enabled: false, Retention: 20 * time.Minute, Filename: "channels.json", }, } func init() { - CmdListChannels.Flag.BoolVar(&chanFlags.cache.Disabled, "no-chan-cache", chanFlags.cache.Disabled, "disable channel cache") + CmdListChannels.Wizard = wizChannels + + CmdListChannels.Flag.BoolVar(&chanFlags.cache.Enabled, "no-chan-cache", chanFlags.cache.Enabled, "disable channel cache") CmdListChannels.Flag.DurationVar(&chanFlags.cache.Retention, "chan-cache-retention", chanFlags.cache.Retention, "channel cache retention time. After this time, the cache is considered stale and will be refreshed.") - CmdListChannels.Flag.BoolVar(&chanFlags.noResolve, "no-resolve", chanFlags.noResolve, "do not resolve user IDs to names") + CmdListChannels.Flag.BoolVar(&chanFlags.resolveUsers, "resolve", chanFlags.resolveUsers, "resolve user IDs to names") } -func listChannels(ctx context.Context, cmd *base.Command, args []string) error { - if err := list(ctx, func(ctx context.Context, sess *slackdump.Session) (any, string, error) { - ctx, task := trace.NewTask(ctx, "listChannels") - defer task.End() - - var filename = makeFilename("channels", sess.Info().TeamID, ".json") - if len(args) > 0 { - filename = args[0] - } - teamID := sess.Info().TeamID - cc, ok := maybeLoadChanCache(cfg.CacheDir(), teamID) - if ok { - // cache hit - trace.Logf(ctx, "cache hit", "teamID=%s", teamID) - return cc, filename, nil - } - // cache miss, load from API - trace.Logf(ctx, "cache miss", "teamID=%s", teamID) - cc, err := sess.GetChannels(ctx) - if err != nil { - return nil, "", fmt.Errorf("error getting channels: %w", err) - } - if err := saveCache(cfg.CacheDir(), teamID, cc); err != nil { - // warn, but don't fail - logger.FromContext(ctx).Printf("failed to save cache: %v", err) - } - return cc, filename, nil - }); err != nil { +func runListChannels(ctx context.Context, cmd *base.Command, args []string) error { + sess, err := bootstrap.SlackdumpSession(ctx) + if err != nil { + base.SetExitStatus(base.SInitializationError) return err } - return nil + var l = &channels{ + opts: chanFlags, + common: commonFlags, + } + + return list(ctx, sess, l, filename) } -func maybeLoadChanCache(cacheDir string, teamID string) (types.Channels, bool) { - if chanFlags.cache.Disabled { - // channel cache disabled - return nil, false - } - m, err := cache.NewManager(cacheDir) - if err != nil { - return nil, false - } - cc, err := m.LoadChannels(teamID, chanFlags.cache.Retention) - if err != nil { - return nil, false - } - return cc, true +type channels struct { + channels types.Channels + users types.Users + + opts channelOptions + common commonOpts } -func saveCache(cacheDir, teamID string, cc types.Channels) error { - m, err := cache.NewManager(cacheDir) +func (l *channels) Type() string { + return "channels" +} + +func (l *channels) Data() types.Channels { + return l.channels +} + +func (l *channels) Users() []slack.User { + return l.users +} + +func (l *channels) Retrieve(ctx context.Context, sess *slackdump.Session, m *cache.Manager) error { + ctx, task := trace.NewTask(ctx, "channels.List") + defer task.End() + lg := cfg.Log + + teamID := sess.Info().TeamID + + usersc := make(chan []slack.User) + go func() { + defer close(usersc) + if l.opts.resolveUsers { + + lg.Println("getting users to resolve DM names") + u, err := fetchUsers(ctx, sess, m, cfg.NoUserCache, teamID) + if err != nil { + log.Printf("error getting users to resolve DM names (ignored): %s", err) + return + } + usersc <- u + } + }() + + if l.opts.cache.Enabled { + var err error + l.channels, err = m.LoadChannels(teamID, l.opts.cache.Retention) + if err == nil { + l.users = <-usersc + return nil + } + } + cc, err := sess.GetChannels(ctx) if err != nil { - return err + return fmt.Errorf("error getting channels: %w", err) } - return m.CacheChannels(teamID, cc) + l.channels = cc + l.users = <-usersc + if err := m.CacheChannels(teamID, cc); err != nil { + logger.FromContext(ctx).Printf("warning: failed to cache channels (ignored): %s", err) + } + return nil } diff --git a/cmd/slackdump/internal/list/common.go b/cmd/slackdump/internal/list/common.go index 9393f50e..f9e259ca 100644 --- a/cmd/slackdump/internal/list/common.go +++ b/cmd/slackdump/internal/list/common.go @@ -2,17 +2,13 @@ package list import ( "context" - "errors" "flag" "fmt" "io" "os" - "path/filepath" - "github.com/rusq/fsadapter" "github.com/rusq/slack" "github.com/rusq/slackdump/v3" - "github.com/rusq/slackdump/v3/cmd/slackdump/internal/bootstrap" "github.com/rusq/slackdump/v3/cmd/slackdump/internal/cfg" "github.com/rusq/slackdump/v3/cmd/slackdump/internal/golang/base" "github.com/rusq/slackdump/v3/internal/cache" @@ -21,10 +17,6 @@ import ( "github.com/rusq/slackdump/v3/types" ) -const ( - userCacheBase = "users.cache" -) - // CmdList is the list command. The logic is in the subcommands. var CmdList = &base.Command{ UsageLine: "slackdump list", @@ -55,14 +47,26 @@ The caching can be turned off by using flags "-no-user-cache" and }, } +type lister[T any] interface { + // Type should return the type of the lister. + Type() string + // Retrieve should retrieve the data from the API or cache. + Retrieve(ctx context.Context, sess *slackdump.Session, m *cache.Manager) error + // Data should return the retrieved data. + Data() T + // Users should return the users for the data, or nil, which indicates + // that there are no associated users or that the users are not resolved. + Users() []slack.User +} + // common flags -type listOptions struct { +type commonOpts struct { listType format.Type quiet bool // quiet mode: don't print anything on the screen, just save the file nosave bool // nosave mode: don't save the data to a file, just print it to the screen } -var commonParams = listOptions{ +var commonFlags = commonOpts{ listType: format.CText, } @@ -74,75 +78,55 @@ func init() { // addCommonFlags adds common flags to the flagset. func addCommonFlags(fs *flag.FlagSet) { - fs.Var(&commonParams.listType, "format", fmt.Sprintf("listing format, should be one of: %v", format.All())) - fs.BoolVar(&commonParams.quiet, "q", false, "quiet mode: don't print anything on the screen, just save the file") - fs.BoolVar(&commonParams.nosave, "no-json", false, "don't save the data to a file, just print it to the screen") + fs.Var(&commonFlags.listType, "format", fmt.Sprintf("listing format, should be one of: %v", format.All())) + fs.BoolVar(&commonFlags.quiet, "q", false, "quiet mode: don't print anything on the screen, just save the file") + fs.BoolVar(&commonFlags.nosave, "no-json", false, "don't save the data to a file, just print it to the screen") } -// listFunc is a function that lists something from the Slack API. It should -// return the object from the api, a filename to save the data to and an -// error. -type listFunc func(ctx context.Context, sess *slackdump.Session) (a any, filename string, err error) - -// list authenticates and creates a slackdump instance, then calls a listFn. -// listFn must return the object from the api, a JSON filename and an error. -func list(ctx context.Context, listFn listFunc) error { - // TODO fix users saving JSON to a text file within archive - if commonParams.listType == format.CUnknown { - return errors.New("unknown listing format, seek help") - } - - // initialize the session. - sess, err := bootstrap.SlackdumpSession(ctx) +func list[T any](ctx context.Context, sess *slackdump.Session, l lister[T], filename string) error { + m, err := cache.NewManager(cfg.CacheDir()) if err != nil { - base.SetExitStatus(base.SInitializationError) return err } - data, filename, err := listFn(ctx, sess) - if err != nil { - return err - } - m, err := cache.NewManager(cfg.CacheDir(), cache.WithUserCacheBase(userCacheBase)) - if err != nil { + if err := l.Retrieve(ctx, sess, m); err != nil { return err } - teamID := sess.Info().TeamID - users, ok := data.(types.Users) // Hax - if !ok && !chanFlags.noResolve { - if cfg.NoUserCache { - users, err = sess.GetUsers(ctx) - } else { - users, err = getCachedUsers(ctx, sess, m, teamID) - } - if err != nil { + if !commonFlags.quiet { + if err := fmtPrint(ctx, os.Stdout, l.Data(), commonFlags.listType, l.Users()); err != nil { return err } } - if !commonParams.nosave { - fsa, err := fsadapter.New(cfg.Output) - if err != nil { - return err + if !commonFlags.nosave { + if filename == "" { + filename = makeFilename(l.Type(), sess.Info().TeamID, extForType(commonFlags.listType)) } - defer fsa.Close() - if err := saveData(ctx, fsa, data, filename, format.CJSON, users); err != nil { + if err := saveData(ctx, l.Data(), filename, commonFlags.listType, l.Users()); err != nil { return err } } + return nil +} - if !commonParams.quiet { - return fmtPrint(ctx, os.Stdout, data, commonParams.listType, users) +func extForType(typ format.Type) string { + switch typ { + case format.CJSON: + return ".json" + case format.CText: + return ".txt" + case format.CCSV: + return ".csv" + default: + return ".json" } - - return nil } // saveData saves the given data to the given filename. -func saveData(ctx context.Context, fs fsadapter.FS, data any, filename string, typ format.Type, users []slack.User) error { +func saveData(ctx context.Context, data any, filename string, typ format.Type, users []slack.User) error { // save to a filesystem. - f, err := fs.Create(filename) + f, err := os.Create(filename) if err != nil { return fmt.Errorf("failed to create file: %w", err) } @@ -150,7 +134,7 @@ func saveData(ctx context.Context, fs fsadapter.FS, data any, filename string, t if err := fmtPrint(ctx, f, data, typ, users); err != nil { return err } - logger.FromContext(ctx).Printf("Data saved to: %q\n", filepath.Join(cfg.Output, filename)) + logger.FromContext(ctx).Printf("Data saved to: %q\n", filename) return nil } diff --git a/cmd/slackdump/internal/list/users.go b/cmd/slackdump/internal/list/users.go index c0b5d633..605b9def 100644 --- a/cmd/slackdump/internal/list/users.go +++ b/cmd/slackdump/internal/list/users.go @@ -5,11 +5,11 @@ import ( "errors" "fmt" "os" - "runtime/trace" "time" "github.com/rusq/slack" "github.com/rusq/slackdump/v3" + "github.com/rusq/slackdump/v3/cmd/slackdump/internal/bootstrap" "github.com/rusq/slackdump/v3/cmd/slackdump/internal/cfg" "github.com/rusq/slackdump/v3/cmd/slackdump/internal/golang/base" "github.com/rusq/slackdump/v3/internal/cache" @@ -19,7 +19,7 @@ import ( ) var CmdListUsers = &base.Command{ - Run: listUsers, + Run: runListUsers, UsageLine: "slackdump list users [flags] [filename]", PrintFlags: true, FlagMask: cfg.OmitDownloadFlag, @@ -40,17 +40,45 @@ func init() { CmdListUsers.Wizard = wizUsers } -func listUsers(ctx context.Context, cmd *base.Command, args []string) error { - if err := list(ctx, func(ctx context.Context, sess *slackdump.Session) (any, string, error) { - var filename = makeFilename("users", sess.Info().TeamID, ".json") - if len(args) > 0 { - filename = args[0] - } - users, err := sess.GetUsers(ctx) - return users, filename, err - }); err != nil { +func runListUsers(ctx context.Context, cmd *base.Command, args []string) error { + sess, err := bootstrap.SlackdumpSession(ctx) + if err != nil { + base.SetExitStatus(base.SInitializationError) return err } + + var l = &users{ + common: commonFlags, + } + + return list(ctx, sess, l, filename) +} + +type users struct { + data types.Users + + common commonOpts +} + +func (u *users) Type() string { + return "users" +} + +func (u *users) Data() types.Users { + return u.data +} + +func (u *users) Users() []slack.User { + return nil +} + +func (u *users) Retrieve(ctx context.Context, sess *slackdump.Session, m *cache.Manager) error { + users, err := fetchUsers(ctx, sess, m, cfg.NoUserCache, sess.Info().TeamID) + if err != nil { + return err + } + m.CacheUsers(sess.Info().TeamID, users) + u.data = users return nil } @@ -65,31 +93,32 @@ type userCacher interface { CacheUsers(teamID string, users []slack.User) error } -func getCachedUsers(ctx context.Context, ug userGetter, m userCacher, teamID string) ([]slack.User, error) { +func fetchUsers(ctx context.Context, ug userGetter, m userCacher, skipCache bool, teamID string) ([]slack.User, error) { lg := logger.FromContext(ctx) - users, err := m.LoadUsers(teamID, cfg.UserCacheRetention) - if err == nil { - return users, nil - } + if !skipCache { + // attempt to load from cache + users, err := m.LoadUsers(teamID, cfg.UserCacheRetention) + if err == nil { + return users, nil + } - // failed to load from cache - if !errors.Is(err, cache.ErrExpired) && !errors.Is(err, cache.ErrEmpty) && !os.IsNotExist(err) && !osext.IsPathError(err) { - // some funky error - return nil, err + // failed to load from cache + if !errors.Is(err, cache.ErrExpired) && !errors.Is(err, cache.ErrEmpty) && !os.IsNotExist(err) && !osext.IsPathError(err) { + // some funky error + return nil, err + } + lg.Println("user cache expired or empty, caching users") } - lg.Println("user cache expired or empty, caching users") - // getting users from API - users, err = ug.GetUsers(ctx) + users, err := ug.GetUsers(ctx) if err != nil { return nil, err } // saving users to cache, will ignore any errors, but notify the user. if err := m.CacheUsers(teamID, users); err != nil { - trace.Logf(ctx, "error", "saving user cache to %q, error: %s", userCacheBase, err) - lg.Printf("warning: failed saving user cache to %q: %s, but nevermind, let's continue", userCacheBase, err) + lg.Printf("warning: failed saving user cache (ignored): %s", err) } return users, nil diff --git a/cmd/slackdump/internal/list/users_test.go b/cmd/slackdump/internal/list/users_test.go index 924276c8..b341c25a 100644 --- a/cmd/slackdump/internal/list/users_test.go +++ b/cmd/slackdump/internal/list/users_test.go @@ -20,8 +20,9 @@ func Test_getCachedUsers(t *testing.T) { } ) type args struct { - ctx context.Context - teamID string + ctx context.Context + skipCache bool + teamID string } tests := []struct { name string @@ -33,7 +34,7 @@ func Test_getCachedUsers(t *testing.T) { /* oh happy days */ { "users loaded from cache", - args{context.Background(), "TEAM1"}, + args{context.Background(), false, "TEAM1"}, func(c *MockuserCacher, g *MockuserGetter) { c.EXPECT().LoadUsers("TEAM1", gomock.Any()).Return(testUsers, nil) }, @@ -42,7 +43,7 @@ func Test_getCachedUsers(t *testing.T) { }, { "getting users from API ok (recoverable cache error)", - args{context.Background(), "TEAM1"}, + args{context.Background(), false, "TEAM1"}, func(c *MockuserCacher, g *MockuserGetter) { c.EXPECT().LoadUsers("TEAM1", gomock.Any()).Return(nil, &fs.PathError{}) g.EXPECT().GetUsers(gomock.Any()).Return(testUsers, nil) @@ -53,7 +54,7 @@ func Test_getCachedUsers(t *testing.T) { }, { "saving cache fails, but we continue", - args{context.Background(), "TEAM1"}, + args{context.Background(), false, "TEAM1"}, func(c *MockuserCacher, g *MockuserGetter) { c.EXPECT().LoadUsers("TEAM1", gomock.Any()).Return(nil, &fs.PathError{}) g.EXPECT().GetUsers(gomock.Any()).Return(testUsers, nil) @@ -65,7 +66,7 @@ func Test_getCachedUsers(t *testing.T) { /* unhappy days */ { "unrecoverable error", - args{context.Background(), "TEAM1"}, + args{context.Background(), false, "TEAM1"}, func(c *MockuserCacher, g *MockuserGetter) { c.EXPECT().LoadUsers("TEAM1", gomock.Any()).Return(nil, errors.New("frobnication error")) }, @@ -74,7 +75,7 @@ func Test_getCachedUsers(t *testing.T) { }, { "getting users from API fails", - args{context.Background(), "TEAM1"}, + args{context.Background(), false, "TEAM1"}, func(c *MockuserCacher, g *MockuserGetter) { c.EXPECT().LoadUsers("TEAM1", gomock.Any()).Return(nil, &fs.PathError{}) g.EXPECT().GetUsers(gomock.Any()).Return(nil, errors.New("blip")) @@ -91,7 +92,7 @@ func Test_getCachedUsers(t *testing.T) { tt.expect(muc, mug) - got, err := getCachedUsers(tt.args.ctx, mug, muc, tt.args.teamID) + got, err := fetchUsers(tt.args.ctx, mug, muc, tt.args.skipCache, tt.args.teamID) if (err != nil) != tt.wantErr { t.Errorf("getCachedUsers() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/cmd/slackdump/internal/list/wizard.go b/cmd/slackdump/internal/list/wizard.go index 44672bd2..7f101d00 100644 --- a/cmd/slackdump/internal/list/wizard.go +++ b/cmd/slackdump/internal/list/wizard.go @@ -63,7 +63,7 @@ func userConfiguration() cfgui.Configuration { }, }, } - return append(c, commonParams.configuration()...) + return append(c, commonFlags.configuration()...) } func filenameParam(placeholder string) cfgui.Parameter { @@ -72,7 +72,7 @@ func filenameParam(placeholder string) cfgui.Parameter { Value: filename, Description: "The filename to save the output to", Inline: true, - Updater: updaters.NewFileNew(&filename, placeholder, true, true), + Updater: updaters.NewFileNew(&filename, placeholder, false, true), } } @@ -83,10 +83,10 @@ func (o *channelOptions) configuration() cfgui.Configuration { Params: []cfgui.Parameter{ filenameParam("channels.json"), { - Name: "Do not Resolve Users", - Value: cfgui.Checkbox(o.noResolve), - Description: "Do not resolve user IDs to names", - Updater: updaters.NewBool(&o.noResolve), + Name: "Resolve Users", + Value: cfgui.Checkbox(o.resolveUsers), + Description: "Resolve user IDs to names. Slow on large Slack workspaces.", + Updater: updaters.NewBool(&o.resolveUsers), }, }, }, @@ -95,31 +95,31 @@ func (o *channelOptions) configuration() cfgui.Configuration { Params: []cfgui.Parameter{ { Name: "Disable Cache", - Value: cfgui.Checkbox(o.cache.Disabled), + Value: cfgui.Checkbox(o.cache.Enabled), Description: "Disable channel cache", - Updater: updaters.NewBool(&o.cache.Disabled), + Updater: updaters.NewBool(&o.cache.Enabled), }, { Name: "Cache Retention", Value: o.cache.Retention.String(), Description: "Channel cache retention time. After this time, the cache is considered stale and will be refreshed.", Inline: true, - Updater: updaters.NewDuration(&o.cache.Retention, true), + Updater: updaters.NewDuration(&o.cache.Retention, false), }, { Name: "Cache Filename", Value: o.cache.Filename, Description: "The filename of the cache", Inline: true, - Updater: updaters.NewString(&o.cache.Filename, "channels.json", true, huh.ValidateNotEmpty()), + Updater: updaters.NewString(&o.cache.Filename, "channels.json", false, huh.ValidateNotEmpty()), }, }, }, } - return append(c, commonParams.configuration()...) + return append(c, commonFlags.configuration()...) } -func (l *listOptions) configuration() cfgui.Configuration { +func (l *commonOpts) configuration() cfgui.Configuration { c := cfgui.Configuration{ cfgui.ParamGroup{ Name: "Common Options", diff --git a/cmd/slackdump/internal/ui/cfgui/configuration.go b/cmd/slackdump/internal/ui/cfgui/configuration.go index 07304e45..2c15cc46 100644 --- a/cmd/slackdump/internal/ui/cfgui/configuration.go +++ b/cmd/slackdump/internal/ui/cfgui/configuration.go @@ -110,6 +110,7 @@ func globalConfig() Configuration { Name: "User Cache Retention", Value: cfg.UserCacheRetention.String(), Description: "For how long user cache is kept, until it is fetched again", + Inline: true, Updater: updaters.NewDuration(&cfg.UserCacheRetention, false), }, { diff --git a/internal/osext/move.go b/internal/osext/move.go index c3e804c9..a7d6d3cc 100644 --- a/internal/osext/move.go +++ b/internal/osext/move.go @@ -45,6 +45,7 @@ func MoveFile(src string, fs fsadapter.FS, dst string) error { // if err := fs.Chmod(dst, si.Mode()); err != nil { // return fmt.Errorf("chmod: %s", err) // } + _ = err // ignore SA9003 in golang-ci-lint } if err := os.Remove(src); err != nil {