Skip to content

Commit

Permalink
remove implicit fs logic from slackdump
Browse files Browse the repository at this point in the history
  • Loading branch information
rusq committed Mar 23, 2023
1 parent 13072c3 commit f85c981
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 60 deletions.
4 changes: 2 additions & 2 deletions cmd/slackdump/internal/cfg/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ func ucd(ucdFn func() (string, error)) string {
}

func CacheDir() string {
if SlackConfig.CacheDir == "" {
if LocalCacheDir == "" {
return ucd(os.UserCacheDir)
}
return SlackConfig.CacheDir
return LocalCacheDir
}
12 changes: 7 additions & 5 deletions cmd/slackdump/internal/cfg/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ var (
LogFile string
Verbose bool

ConfigFile string
Workspace string
BaseLocation string
ConfigFile string
Workspace string

SlackToken string
SlackCookie string
Expand All @@ -40,6 +41,7 @@ var (
// for the dump to be consistent.
Latest = config.TimeValue(time.Now())

LocalCacheDir string
UserCacheRetention time.Duration
NoUserCache bool

Expand Down Expand Up @@ -90,15 +92,15 @@ func SetBaseFlags(fs *flag.FlagSet, mask FlagMask) {
}
if mask&OmitBaseLocFlag == 0 {
base := fmt.Sprintf("slackdump_%s.zip", time.Now().Format(filenameLayout))
fs.StringVar(&SlackConfig.BaseLocation, "base", osenv.Value("BASE_LOC", base), "a `location` (a directory or a ZIP file) on the local disk to save\ndownloaded files to.")
fs.StringVar(&BaseLocation, "base", osenv.Value("BASE_LOC", base), "a `location` (a directory or a ZIP file) on the local disk to save\ndownloaded files to.")
}
if mask&OmitCacheDir == 0 {
fs.StringVar(&SlackConfig.CacheDir, "cache-dir", osenv.Value("CACHE_DIR", CacheDir()), "cache `directory` location\n")
fs.StringVar(&LocalCacheDir, "cache-dir", osenv.Value("CACHE_DIR", CacheDir()), "cache `directory` location\n")
} else {
// If the OmitCacheDir is specified, then the CacheDir will end up being
// the default value, which is "". Therefore, we need to init the
// cache directory.
SlackConfig.CacheDir = CacheDir()
LocalCacheDir = CacheDir()
}
if mask&OmitWorkspaceFlag == 0 {
fs.StringVar(&Workspace, "workspace", osenv.Value("SLACK_WORKSPACE", ""), "Slack workspace to use") // TODO: load from configuration.
Expand Down
2 changes: 1 addition & 1 deletion cmd/slackdump/internal/emoji/emoji.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func run(ctx context.Context, cmd *base.Command, args []string) error {
return fmt.Errorf("auth error: %s", err)
}

fs, err := fsadapter.New(cfg.SlackConfig.BaseLocation)
fs, err := fsadapter.New(cfg.BaseLocation)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/slackdump/internal/emoji/wizard.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func wizard(ctx context.Context, cmd *base.Command, args []string) error {
}
fmt.Println("invalid filename")
}
cfg.SlackConfig.BaseLocation = baseloc
cfg.BaseLocation = baseloc

var err error
ignoreErrors, err = ui.Confirm("Ignore download errors?", true)
Expand Down
2 changes: 1 addition & 1 deletion cmd/slackdump/internal/export/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func init() {
}

func runExport(ctx context.Context, cmd *base.Command, args []string) error {
if cfg.SlackConfig.BaseLocation == "" {
if cfg.BaseLocation == "" {
return errors.New("use -base to set the base output location")
}
list, err := structures.NewEntityList(args)
Expand Down
2 changes: 1 addition & 1 deletion cmd/slackdump/internal/export/wizard.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func wizExport(ctx context.Context, cmd *base.Command, args []string) error {
if err != nil {
return err
}
cfg.SlackConfig.BaseLocation = baseLoc
cfg.BaseLocation = baseLoc

sess, err := slackdump.New(ctx, prov, cfg.SlackConfig)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/slackdump/internal/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func saveData(ctx context.Context, fs fsadapter.FS, data any, filename string, t
if err := writeData(ctx, fs, filename, data, typ, users); err != nil {
return err
}
dlog.FromContext(ctx).Printf("Data saved to: %q\n", filepath.Join(cfg.SlackConfig.BaseLocation, filename))
dlog.FromContext(ctx).Printf("Data saved to: %q\n", filepath.Join(cfg.BaseLocation, filename))
return nil
}

Expand Down
6 changes: 3 additions & 3 deletions cmd/slackdump/internal/v1/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func runV1(ctx context.Context, cmd *base.Command, args []string) error {
params, cfgErr := parseCmdLine(args[0:])

if params.authReset {
if err := cache.AuthReset(params.appCfg.SlackConfig.CacheDir); err != nil {
if err := cache.AuthReset(cfg.CacheDir()); err != nil {
if !os.IsNotExist(err) {
dlog.Printf("auth reset error: %s", err)
}
Expand Down Expand Up @@ -168,7 +168,7 @@ func run(ctx context.Context, p params) error {
ctx, task := trace.NewTask(ctx, "main.run")
defer task.End()

m, err := cache.NewManager(p.appCfg.SlackConfig.CacheDir)
m, err := cache.NewManager(cfg.CacheDir())
if err != nil {
return err
}
Expand Down Expand Up @@ -355,7 +355,7 @@ func parseCmdLine(args []string) (params, error) {
fs.IntVar(&p.appCfg.SlackConfig.Limits.Request.Replies, "rpr", slackdump.DefOptions.Limits.Request.Replies, "number of `replies` per request.")

// - cache controls
fs.StringVar(&p.appCfg.SlackConfig.CacheDir, "cache-dir", cfg.CacheDir(), "slackdump cache directory")
fs.StringVar(&cfg.LocalCacheDir, "cache-dir", cfg.CacheDir(), "slackdump cache directory")
fs.DurationVar(&cfg.UserCacheRetention, "user-cache-age", cfg.UserCacheRetention, "user cache lifetime `duration`. Set this to 0 to disable cache.")
fs.BoolVar(&cfg.NoUserCache, "no-user-cache", cfg.NoUserCache, "skip fetching users")

Expand Down
4 changes: 0 additions & 4 deletions cmd/slackdump/internal/v1/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/stretchr/testify/assert"

"github.com/rusq/slackdump/v2"
"github.com/rusq/slackdump/v2/cmd/slackdump/internal/cfg"
"github.com/rusq/slackdump/v2/internal/app/config"
"github.com/rusq/slackdump/v2/internal/cache"
"github.com/rusq/slackdump/v2/internal/structures"
Expand Down Expand Up @@ -41,9 +40,6 @@ func Test_output_validFormat(t *testing.T) {
}

func Test_checkParameters(t *testing.T) {
// setup
slackdump.DefOptions.CacheDir = cfg.CacheDir()

// test
type args struct {
args []string
Expand Down
12 changes: 3 additions & 9 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ type Config struct {
Limits Limits

DumpFiles bool // will we save the conversation files?

CacheDir string // cache directory

BaseLocation string // base location for the dump files
Logfile string
Logfile string
}

type Limits struct {
Expand Down Expand Up @@ -79,10 +75,8 @@ var DefOptions = Config{
Replies: 200, // the API-default is 1000 (see conversations.replies), but on large threads it may fail (see #54)
},
},
DumpFiles: false,
CacheDir: "", // default cache dir
BaseLocation: ".", // default location is the current directory
Logfile: "", // empty, means STDERR
DumpFiles: false,
Logfile: "", // empty, means STDERR
}

var (
Expand Down
12 changes: 8 additions & 4 deletions internal/app/dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,13 @@ func Dump(ctx context.Context, cfg config.Params, prov auth.Provider) error {
ctx, task := trace.NewTask(ctx, "runDump")
defer task.End()

cfg.SlackConfig.BaseLocation = cfg.Output.Base
fs, err := fsadapter.New(cfg.Output.Base)
if err != nil {
return err
}
defer fs.Close()

dm, err := newDump(ctx, cfg, prov)
dm, err := newDump(ctx, cfg, prov, fs)
if err != nil {
return err
}
Expand All @@ -51,8 +55,8 @@ func Dump(ctx context.Context, cfg config.Params, prov auth.Provider) error {
return err
}

func newDump(ctx context.Context, cfg config.Params, prov auth.Provider) (*dump, error) {
sess, err := slackdump.New(ctx, prov, cfg.SlackConfig)
func newDump(ctx context.Context, cfg config.Params, prov auth.Provider, fs fsadapter.FS) (*dump, error) {
sess, err := slackdump.New(ctx, prov, cfg.SlackConfig, slackdump.WithFilesystem(fs))
if err != nil {
return nil, err
}
Expand Down
9 changes: 7 additions & 2 deletions internal/app/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"runtime/trace"
"time"

"github.com/rusq/fsadapter"
"github.com/rusq/slackdump/v2"
"github.com/rusq/slackdump/v2/auth"
"github.com/rusq/slackdump/v2/export"
Expand All @@ -26,9 +27,13 @@ func Export(ctx context.Context, cfg config.Params, prov auth.Provider) error {
return errors.New("export directory or filename not specified")
}

cfg.SlackConfig.BaseLocation = cfg.ExportName
fs, err := fsadapter.New(cfg.ExportName)
if err != nil {
return err
}
defer fs.Close()

sess, err := slackdump.New(ctx, prov, cfg.SlackConfig)
sess, err := slackdump.New(ctx, prov, cfg.SlackConfig, slackdump.WithFilesystem(fs))
if err != nil {
return err
}
Expand Down
4 changes: 4 additions & 0 deletions processors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package slackdump

import (
"context"
"errors"
"fmt"
"path"
"runtime/trace"
Expand Down Expand Up @@ -72,6 +73,9 @@ func runProcessFuncs(m []types.Message, channelID string, processFn ...ProcessFu
// file, instead of Slack server URL. It returns ProcessFunction and
// CancelFunc. CancelFunc must be called, i.e. by deferring it's execution.
func (s *Session) newFileProcessFn(ctx context.Context, dir string, l *rate.Limiter) (ProcessFunc, cancelFunc, error) {
if s.fs == nil {
return nil, nil, errors.New("filesystem not set, unable to download files")
}
// set up a file downloader and add it to the post-process functions
// slice
dl := downloader.New(
Expand Down
25 changes: 1 addition & 24 deletions slackdump.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type Option func(*Session)
// WithFilesystem sets the filesystem adapter to use for the session. If this
// option is not given, the default filesystem adapter is initialised with the
// base location specified in the Config.
func WithFilesystem(fs fsadapter.FSCloser) Option {
func WithFilesystem(fs fsadapter.FS) Option {
return func(s *Session) {
if fs != nil {
s.fs = fs
Expand Down Expand Up @@ -148,11 +148,6 @@ func New(ctx context.Context, prov auth.Provider, cfg Config, opts ...Option) (*
for _, opt := range opts {
opt(sd)
}
if sd.fs == nil {
if err := sd.openFS(cfg.BaseLocation); err != nil {
return nil, fmt.Errorf("failed to initialise filesystem adapter: %s", err)
}
}
if sd.log == nil {
if err := sd.openLogger(cfg.Logfile); err != nil {
return nil, fmt.Errorf("failed to initialise logger: %s", err)
Expand All @@ -161,10 +156,6 @@ func New(ctx context.Context, prov auth.Provider, cfg Config, opts ...Option) (*

sd.propagateLogger()

if err := os.MkdirAll(cfg.CacheDir, 0o700); err != nil {
return nil, fmt.Errorf("failed to create the cache directory: %s", err)
}

return sd, nil
}

Expand All @@ -173,20 +164,6 @@ 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
Expand Down
23 changes: 21 additions & 2 deletions slackdump_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"
"time"

"github.com/rusq/fsadapter"
"github.com/rusq/slackdump/v2/auth"
"github.com/rusq/slackdump/v2/internal/network"
"github.com/rusq/slackdump/v2/logger"
Expand Down Expand Up @@ -80,6 +81,9 @@ func ExampleNew_tokenAndCookie() {
log.Print(err)
return
}
fsa := openTempFS()
defer fsa.Close()

sd, err := New(context.Background(), provider, DefOptions)
if err != nil {
log.Print(err)
Expand All @@ -94,6 +98,9 @@ func ExampleNew_cookieFile() {
log.Print(err)
return
}
fsa := openTempFS()
defer fsa.Close()

sd, err := New(context.Background(), provider, DefOptions)
if err != nil {
log.Print(err)
Expand All @@ -108,6 +115,8 @@ func ExampleNew_browserAuth() {
log.Print(err)
return
}
fsa := openTempFS()
defer fsa.Close()
sd, err := New(context.Background(), provider, DefOptions)
if err != nil {
log.Print(err)
Expand All @@ -116,14 +125,25 @@ func ExampleNew_browserAuth() {
_ = sd
}

func openTempFS() fsadapter.FSCloser {
dir, err := os.MkdirTemp("", "slackdump")
if err != nil {
panic(err)
}
fs, err := fsadapter.New(dir)
if err != nil {
panic(err)
}
return fs
}

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())
Expand All @@ -135,7 +155,6 @@ func TestSession_openFS(t *testing.T) {
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)

Expand Down

0 comments on commit f85c981

Please sign in to comment.