From f5ac97e534cf5eb19aec52f582ad0a79fb9481fe Mon Sep 17 00:00:00 2001 From: Sam Hofius Date: Fri, 11 Mar 2022 09:39:53 -0700 Subject: [PATCH] More cleanup and refactoring --- chat.go | 47 +++++++++++++++++++++++++------------- keybase.go | 55 ++++++++++++++++++++++++--------------------- keybase_test.go | 60 ++++++++++++++++++++++++------------------------- 3 files changed, 89 insertions(+), 73 deletions(-) diff --git a/chat.go b/chat.go index a33f09f..eb04586 100644 --- a/chat.go +++ b/chat.go @@ -15,28 +15,47 @@ type ChatAPI struct { fromStdout chan []byte fromListen chan []byte errors chan error + kbLoc string } -func NewChatAPI(ctx context.Context, opts *Options) (api *ChatAPI, err error) { +func NewChatAPI(ctx context.Context, opts Options) (api *ChatAPI, err error) { + if opts.ChannelBufferSize <= 0 { + opts.ChannelBufferSize = 10 + } + // set up the channels - api.toStdin = make(chan []byte, opts.ChannelBufferSize) - api.fromStdout = make(chan []byte, opts.ChannelBufferSize) - api.fromListen = make(chan []byte, opts.ChannelBufferSize) - api.errors = make(chan error, opts.ChannelBufferSize) + api = &ChatAPI{ + toStdin: make(chan []byte, opts.ChannelBufferSize), + fromStdout: make(chan []byte, opts.ChannelBufferSize), + fromListen: make(chan []byte, opts.ChannelBufferSize), + errors: make(chan error, opts.ChannelBufferSize), + } // get the basics - kbCmd := opts.locateKeybase() - chatApiArgs := opts.buildArgs("chat", "api") - chatApiListenArgs := opts.buildArgs("chat", "api-listen") + if opts.KeybaseLocation == "" { + opts.KeybaseLocation, err = locateKeybase() + if err != nil { + return nil, fmt.Errorf("failed to locate keybase: %v", err) + } + } + api.kbLoc = opts.KeybaseLocation + chatApiArgs, err := buildArgs(opts, "chat", "api") + if err != nil { + return nil, fmt.Errorf("failed to build api args: %v", err) + } + chatApiListenArgs, err := buildArgs(opts, "chat", "api-listen") + if err != nil { + return nil, fmt.Errorf("failed to build api-listen args: %v", err) + } // build the commands - chatApi, err := newApiCmd(ctx, kbCmd, chatApiArgs...) + chatApi, err := newApiCmd(ctx, api.kbLoc, chatApiArgs...) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to build api command") } - chatListen, err := newApiCmd(ctx, kbCmd, chatApiListenArgs...) + chatListen, err := newApiCmd(ctx, api.kbLoc, chatApiListenArgs...) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to build api-listen command") } // create the goroutines @@ -95,10 +114,6 @@ func NewChatAPI(ctx context.Context, opts *Options) (api *ChatAPI, err error) { return } -func (c *ChatAPI) Listen() chan []byte { - return c.fromListen -} - func (c *ChatAPI) SendRaw(msg []byte) ([]byte, error) { c.Lock() defer c.Unlock() diff --git a/keybase.go b/keybase.go index df484b3..345cdee 100644 --- a/keybase.go +++ b/keybase.go @@ -1,52 +1,55 @@ package keybase import ( - "log" + "fmt" "os/exec" ) // Options holds... run... options... type Options struct { - KeybaseLoction string // Optional, but required if keybase is not in your path - HomeDir string // Only use this if you know what you're doing - EnableTyping bool // Show others a typing notification while the bot is working on a command - BotLiteMode bool // Defaults to true - only disable if you need to. - ChannelBufferSize int // The size of the channel buffers, may vary on rate of message ingestion -} + // Optional, but required if keybase is not in your path + KeybaseLocation string -// NewOptions returns a new instance of *Options with sensible defaults -func NewOptions() *Options { - return &Options{ - BotLiteMode: true, - ChannelBufferSize: 10, - } + // Only use this if you know what you're doing + HomeDir string + + // Show others a typing notification while the bot is working on a command + EnableTyping bool + + // Turn off Keybase's 'bot lite' mode. Only disable if you need to. + DisableLiteMode bool + + // The size of the channel buffers, may vary on rate of message ingestion + ChannelBufferSize int } // locateKeybase attempts to find the location of the keybase binary in the following order: // 1. What the user has specified as the location [user specified] // 2. Looks up the binary location using exec.LookPath [default] // 3. Returns "keybase" and hopes its pathed on the users system [fallback] -func (opt *Options) locateKeybase() string { - if opt.KeybaseLoction != "" { - return opt.KeybaseLoction - } +func locateKeybase() (string, error) { path, err := exec.LookPath("keybase") if err != nil { - log.Println("INFO: Could not detect keybase in path") - return "keybase" + return "", fmt.Errorf("could not determine path for keybase binary: %v", err) } - return path + return path, nil } // buildBaseCommand adds the homedirectory before the args, when required -func (opt *Options) buildArgs(args ...string) []string { - var cmd []string - if opt.HomeDir != "" { - cmd = append(cmd, "--home", opt.HomeDir) +func buildArgs(opts Options, args ...string) ([]string, error) { + if len(args) == 0 { + return nil, fmt.Errorf("no arguments") + } + + // initialize slice so we never return a nil object + cmd := make([]string, 0) + if opts.HomeDir != "" { + cmd = append(cmd, "--home", opts.HomeDir) } - if opt.BotLiteMode { + if !opts.DisableLiteMode { cmd = append(cmd, "--enable-bot-lite-mode") } cmd = append(cmd, args...) - return cmd + + return cmd, nil } diff --git a/keybase_test.go b/keybase_test.go index 17b8229..8f1277d 100644 --- a/keybase_test.go +++ b/keybase_test.go @@ -2,41 +2,39 @@ package keybase import ( "reflect" - "strings" "testing" ) -func TestNewOptions(t *testing.T) { - want := &Options{ - KeybaseLoction: "", - HomeDir: "", - EnableTyping: false, - BotLiteMode: true, +func TestBuildArgs(t *testing.T) { + cases := []struct { + Opts Options + Args []string + Expected []string + }{ + { + Opts: Options{HomeDir: "/home/foo"}, + Args: []string{"arg1"}, + Expected: []string{"--home", "/home/foo", "--enable-bot-lite-mode", "arg1"}, + }, + { + Opts: Options{HomeDir: "/home/foo"}, + Args: []string{"arg1", "arg2"}, + Expected: []string{"--home", "/home/foo", "--enable-bot-lite-mode", "arg1", "arg2"}, + }, + { + Opts: Options{HomeDir: "/home/foo", DisableLiteMode: true}, + Args: []string{"arg1"}, + Expected: []string{"--home", "/home/foo", "arg1"}, + }, } - got := NewOptions() - if !reflect.DeepEqual(want, got) { - t.Errorf("NewOptions returned non-equal structs") - } -} - -func TestLocateKeybase(t *testing.T) { - opts := NewOptions() - opts.KeybaseLoction = "/usr/bin/keybase" - if opts.locateKeybase() != "/usr/bin/keybase" { - t.Errorf("locateKeybase returned non-specified keybase") - } - opts2 := NewOptions() - if !strings.Contains(opts2.locateKeybase(), "keybase") { - t.Errorf("locateKeybase did not return a keybase path") - } -} -func TestBaseCommand(t *testing.T) { - opts := NewOptions() - opts.HomeDir = "/home/foo" - want := []string{"--home", "/home/foo", "--enable-bot-lite-mode", "arg"} - got := opts.buildArgs("arg") - if !reflect.DeepEqual(want, got) { - t.Errorf("buildBaseCommand returned invalid command set") + for i, c := range cases { + actual, err := buildArgs(c.Opts, c.Args...) + if err != nil { + t.Errorf("[case %d] returned error: %v", i, err) + } + if !reflect.DeepEqual(actual, c.Expected) { + t.Errorf("[case %d] expected: %#v, got: %#v", i, c.Expected, actual) + } } }