Refactor filestore to simplify testing

diff --git a/cmd/mobius-hotline-server/main.go b/cmd/mobius-hotline-server/main.go
index f18ae9d9c841a9edb70062b9ec96c017f8d8e8ba..60327a803e8b2cfd27140012b5995ed4530aa387 100644 (file)
--- a/cmd/mobius-hotline-server/main.go
+++ b/cmd/mobius-hotline-server/main.go
@@ -48,9 +48,7 @@ func main() {
                logger.Fatalw("Configuration directory not found", "path", configDir)
        }
 
-       hotline.FS = &hotline.OSFileStore{}
-
-       srv, err := hotline.NewServer(*configDir, "", *basePort, logger)
+       srv, err := hotline.NewServer(*configDir, "", *basePort, logger, &hotline.OSFileStore{})
        if err != nil {
                logger.Fatal(err)
        }

diff --git a/hotline/file_store.go b/hotline/file_store.go
index ba86c6a3b434f4dc5195033be2eecec62b965632..2ba9d7af6b9e3ea8444f1ff4a74c4d60b1e62292 100644 (file)
--- a/hotline/file_store.go
+++ b/hotline/file_store.go
@@ -6,8 +6,6 @@ import (
        "os"
 )
 
-var FS FileStore
-
 type FileStore interface {
        Mkdir(name string, perm os.FileMode) error
        Stat(name string) (os.FileInfo, error)

diff --git a/hotline/server.go b/hotline/server.go
index b1d8849032cce8db50159a453f8273ac89f14381..33888e08f745a92d1f41802603ba2aadb619c3ce 100644 (file)
--- a/hotline/server.go
+++ b/hotline/server.go
@@ -50,6 +50,7 @@ type Server struct {
        TrackerPassID [4]byte
        Stats         *Stats
 
+       FS FileStore
 
        // newsReader io.Reader
        // newsWriter io.WriteCloser
@@ -187,7 +188,7 @@ const (
 )
 
 // NewServer constructs a new Server from a config dir
-func NewServer(configDir, netInterface string, netPort int, logger *zap.SugaredLogger) (*Server, error) {
+func NewServer(configDir, netInterface string, netPort int, logger *zap.SugaredLogger, FS FileStore) (*Server, error) {
        server := Server{
                Port:          netPort,
                Accounts:      make(map[string]*Account),
@@ -201,6 +202,7 @@ func NewServer(configDir, netInterface string, netPort int, logger *zap.SugaredL
                outbox:        make(chan Transaction),
                Stats:         &Stats{StartTime: time.Now()},
                ThreadedNews:  &ThreadedNews{},
+               FS:            FS,
        }
 
        var err error
@@ -210,7 +212,8 @@ func NewServer(configDir, netInterface string, netPort int, logger *zap.SugaredL
                return nil, err
        }
 
-       if server.Agreement, err = os.ReadFile(configDir + agreementFile); err != nil {
+       server.Agreement, err = os.ReadFile(configDir + agreementFile)
+       if err != nil {
                return nil, err
        }
 
@@ -361,7 +364,7 @@ func (s *Server) NewUser(login, name, password string, access []byte) error {
        }
        s.Accounts[login] = &account
 
-       return FS.WriteFile(s.ConfigDir+"Users/"+login+".yaml", out, 0666)
+       return s.FS.WriteFile(s.ConfigDir+"Users/"+login+".yaml", out, 0666)
 }
 
 func (s *Server) UpdateUser(login, newLogin, name, password string, access []byte) error {
@@ -401,7 +404,7 @@ func (s *Server) DeleteUser(login string) error {
 
        delete(s.Accounts, login)
 
-       return FS.Remove(s.ConfigDir + "Users/" + login + ".yaml")
+       return s.FS.Remove(s.ConfigDir + "Users/" + login + ".yaml")
 }
 
 func (s *Server) connectedUsers() []Field {
@@ -447,7 +450,7 @@ func (s *Server) loadAccounts(userDir string) error {
        }
 
        for _, file := range matches {
-               fh, err := FS.Open(file)
+               fh, err := s.FS.Open(file)
                if err != nil {
                        return err
                }
@@ -464,7 +467,7 @@ func (s *Server) loadAccounts(userDir string) error {
 }
 
 func (s *Server) loadConfig(path string) error {
-       fh, err := FS.Open(path)
+       fh, err := s.FS.Open(path)
        if err != nil {
                return err
        }
@@ -714,7 +717,7 @@ func (s *Server) handleFileTransfer(conn io.ReadWriteCloser) error {
                        }
                }
 
-               file, err := FS.Open(fullFilePath)
+               file, err := s.FS.Open(fullFilePath)
                if err != nil {
                        return err
                }
@@ -906,7 +909,7 @@ func (s *Server) handleFileTransfer(conn io.ReadWriteCloser) error {
                                return err
                        }
 
-                       file, err := FS.Open(path)
+                       file, err := s.FS.Open(path)
                        if err != nil {
                                return err
                        }
@@ -963,8 +966,8 @@ func (s *Server) handleFileTransfer(conn io.ReadWriteCloser) error {
                )
 
                // Check if the target folder exists.  If not, create it.
-               if _, err := FS.Stat(dstPath); os.IsNotExist(err) {
-                       if err := FS.Mkdir(dstPath, 0777); err != nil {
+               if _, err := s.FS.Stat(dstPath); os.IsNotExist(err) {
+                       if err := s.FS.Mkdir(dstPath, 0777); err != nil {
                                return err
                        }
                }
@@ -1077,7 +1080,7 @@ func (s *Server) handleFileTransfer(conn io.ReadWriteCloser) error {
                                        filePath := dstPath + "/" + fu.FormattedPath()
                                        s.Logger.Infow("Starting file transfer", "path", filePath, "fileNum", i+1, "totalFiles", "zz", "fileSize", binary.BigEndian.Uint32(fileSize))
 
-                                       newFile, err := FS.Create(filePath + ".incomplete")
+                                       newFile, err := s.FS.Create(filePath + ".incomplete")
                                        if err != nil {
                                                return err
                                        }

diff --git a/hotline/transaction_handlers.go b/hotline/transaction_handlers.go
index f4c5a2fda080e349683ec78ee8d0fa9e615fcbff..1b6911493862cbf177e42c8a22794abca7a5e20c 100644 (file)
--- a/hotline/transaction_handlers.go
+++ b/hotline/transaction_handlers.go
@@ -391,7 +391,7 @@ func HandleSetFileInfo(cc *ClientConn, t *Transaction) (res []Transaction, err e
        fileNewName := t.GetField(fieldFileNewName).Data
 
        if fileNewName != nil {
-               fi, err := FS.Stat(fullFilePath)
+               fi, err := cc.Server.FS.Stat(fullFilePath)
                if err != nil {
                        return res, err
                }
@@ -524,14 +524,14 @@ func HandleNewFolder(cc *ClientConn, t *Transaction) (res []Transaction, err err
 
        // TODO: check path and folder name lengths
 
-       if _, err := FS.Stat(newFolderPath); !os.IsNotExist(err) {
+       if _, err := cc.Server.FS.Stat(newFolderPath); !os.IsNotExist(err) {
                msg := fmt.Sprintf("Cannot create folder \"%s\" because there is already a file or folder with that name.", folderName)
                return []Transaction{cc.NewErrReply(t, msg)}, nil
        }
 
        // TODO: check for disallowed characters to maintain compatibility for original client
 
-       if err := FS.Mkdir(newFolderPath, 0777); err != nil {
+       if err := cc.Server.FS.Mkdir(newFolderPath, 0777); err != nil {
                msg := fmt.Sprintf("Cannot create folder \"%s\" because an error occurred.", folderName)
                return []Transaction{cc.NewErrReply(t, msg)}, nil
        }
@@ -1523,7 +1523,7 @@ func HandleUploadFile(cc *ClientConn, t *Transaction) (res []Transaction, err er
                        return res, err
                }
 
-               fileInfo, err := FS.Stat(fullFilePath + incompleteFileSuffix)
+               fileInfo, err := cc.Server.FS.Stat(fullFilePath + incompleteFileSuffix)
                if err != nil {
                        return res, err
                }
@@ -1856,7 +1856,7 @@ func HandleMakeAlias(cc *ClientConn, t *Transaction) (res []Transaction, err err
 
        cc.Server.Logger.Debugw("Make alias", "src", fullFilePath, "dst", fullNewFilePath)
 
-       if err := FS.Symlink(fullFilePath, fullNewFilePath); err != nil {
+       if err := cc.Server.FS.Symlink(fullFilePath, fullNewFilePath); err != nil {
                res = append(res, cc.NewErrReply(t, "Error creating alias"))
                return res, nil
        }

diff --git a/hotline/transaction_handlers_test.go b/hotline/transaction_handlers_test.go
index c174822723ad435073dc9f08e96f273a314be518..01ae862fc964e2dcb8276a91189037195cf89b3b 100644 (file)
--- a/hotline/transaction_handlers_test.go
+++ b/hotline/transaction_handlers_test.go
@@ -689,15 +689,13 @@ func TestHandleNewFolder(t *testing.T) {
                t  *Transaction
        }
        tests := []struct {
-               setup   func()
                name    string
                args    args
                wantRes []Transaction
                wantErr bool
        }{
                {
-                       name:  "without required permission",
-                       setup: func() {},
+                       name: "without required permission",
                        args: args{
                                cc: &ClientConn{
                                        Account: &Account{
@@ -744,6 +742,12 @@ func TestHandleNewFolder(t *testing.T) {
                                                Config: &Config{
                                                        FileRoot: "/Files/",
                                                },
+                                               FS: func() *MockFileStore {
+                                                       mfs := &MockFileStore{}
+                                                       mfs.On("Mkdir", "/Files/aaa/testFolder", fs.FileMode(0777)).Return(nil)
+                                                       mfs.On("Stat", "/Files/aaa/testFolder").Return(nil, os.ErrNotExist)
+                                                       return mfs
+                                               }(),
                                        },
                                },
                                t: NewTransaction(
@@ -757,12 +761,6 @@ func TestHandleNewFolder(t *testing.T) {
                                        }),
                                ),
                        },
-                       setup: func() {
-                               mfs := &MockFileStore{}
-                               mfs.On("Mkdir", "/Files/aaa/testFolder", fs.FileMode(0777)).Return(nil)
-                               mfs.On("Stat", "/Files/aaa/testFolder").Return(nil, os.ErrNotExist)
-                               FS = mfs
-                       },
                        wantRes: []Transaction{
                                {
                                        clientID:  &[]byte{0, 1},
@@ -792,6 +790,12 @@ func TestHandleNewFolder(t *testing.T) {
                                                Config: &Config{
                                                        FileRoot: "/Files",
                                                },
+                                               FS: func() *MockFileStore {
+                                                       mfs := &MockFileStore{}
+                                                       mfs.On("Mkdir", "/Files/testFolder", fs.FileMode(0777)).Return(nil)
+                                                       mfs.On("Stat", "/Files/testFolder").Return(nil, os.ErrNotExist)
+                                                       return mfs
+                                               }(),
                                        },
                                },
                                t: NewTransaction(
@@ -799,12 +803,6 @@ func TestHandleNewFolder(t *testing.T) {
                                        NewField(fieldFileName, []byte("testFolder")),
                                ),
                        },
-                       setup: func() {
-                               mfs := &MockFileStore{}
-                               mfs.On("Mkdir", "/Files/testFolder", fs.FileMode(0777)).Return(nil)
-                               mfs.On("Stat", "/Files/testFolder").Return(nil, os.ErrNotExist)
-                               FS = mfs
-                       },
                        wantRes: []Transaction{
                                {
                                        clientID:  &[]byte{0, 1},
@@ -834,6 +832,12 @@ func TestHandleNewFolder(t *testing.T) {
                                                Config: &Config{
                                                        FileRoot: "/Files/",
                                                },
+                                               FS: func() *MockFileStore {
+                                                       mfs := &MockFileStore{}
+                                                       mfs.On("Mkdir", "/Files/aaa/testFolder", fs.FileMode(0777)).Return(nil)
+                                                       mfs.On("Stat", "/Files/aaa/testFolder").Return(nil, os.ErrNotExist)
+                                                       return mfs
+                                               }(),
                                        },
                                },
                                t: NewTransaction(
@@ -844,12 +848,6 @@ func TestHandleNewFolder(t *testing.T) {
                                        }),
                                ),
                        },
-                       setup: func() {
-                               mfs := &MockFileStore{}
-                               mfs.On("Mkdir", "/Files/aaa/testFolder", fs.FileMode(0777)).Return(nil)
-                               mfs.On("Stat", "/Files/aaa/testFolder").Return(nil, os.ErrNotExist)
-                               FS = mfs
-                       },
                        wantRes: []Transaction{},
                        wantErr: true,
                },
@@ -870,6 +868,12 @@ func TestHandleNewFolder(t *testing.T) {
                                                Config: &Config{
                                                        FileRoot: "/Files/",
                                                },
+                                               FS: func() *MockFileStore {
+                                                       mfs := &MockFileStore{}
+                                                       mfs.On("Mkdir", "/Files/testFolder", fs.FileMode(0777)).Return(nil)
+                                                       mfs.On("Stat", "/Files/testFolder").Return(nil, os.ErrNotExist)
+                                                       return mfs
+                                               }(),
                                        },
                                },
                                t: NewTransaction(
@@ -877,12 +881,6 @@ func TestHandleNewFolder(t *testing.T) {
                                        NewField(fieldFileName, []byte("../../testFolder")),
                                ),
                        },
-                       setup: func() {
-                               mfs := &MockFileStore{}
-                               mfs.On("Mkdir", "/Files/testFolder", fs.FileMode(0777)).Return(nil)
-                               mfs.On("Stat", "/Files/testFolder").Return(nil, os.ErrNotExist)
-                               FS = mfs
-                       },
                        wantRes: []Transaction{
                                {
                                        clientID:  &[]byte{0, 1},
@@ -911,6 +909,12 @@ func TestHandleNewFolder(t *testing.T) {
                                                Config: &Config{
                                                        FileRoot: "/Files/",
                                                },
+                                               FS: func() *MockFileStore {
+                                                       mfs := &MockFileStore{}
+                                                       mfs.On("Mkdir", "/Files/foo/testFolder", fs.FileMode(0777)).Return(nil)
+                                                       mfs.On("Stat", "/Files/foo/testFolder").Return(nil, os.ErrNotExist)
+                                                       return mfs
+                                               }(),
                                        },
                                },
                                t: NewTransaction(
@@ -927,12 +931,6 @@ func TestHandleNewFolder(t *testing.T) {
                                        }),
                                ),
                        },
-                       setup: func() {
-                               mfs := &MockFileStore{}
-                               mfs.On("Mkdir", "/Files/foo/testFolder", fs.FileMode(0777)).Return(nil)
-                               mfs.On("Stat", "/Files/foo/testFolder").Return(nil, os.ErrNotExist)
-                               FS = mfs
-                       },
                        wantRes: []Transaction{
                                {
                                        clientID:  &[]byte{0, 1},
@@ -947,7 +945,6 @@ func TestHandleNewFolder(t *testing.T) {
        }
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {
-                       tt.setup()
 
                        gotRes, err := HandleNewFolder(tt.args.cc, tt.args.t)
                        if (err != nil) != tt.wantErr {
@@ -1078,23 +1075,12 @@ func TestHandleMakeAlias(t *testing.T) {
        }
        tests := []struct {
                name    string
-               setup   func()
                args    args
                wantRes []Transaction
                wantErr bool
        }{
                {
                        name: "with valid input and required permissions",
-                       setup: func() {
-                               mfs := &MockFileStore{}
-                               path, _ := os.Getwd()
-                               mfs.On(
-                                       "Symlink",
-                                       path+"/test/config/Files/foo/testFile",
-                                       path+"/test/config/Files/bar/testFile",
-                               ).Return(nil)
-                               FS = mfs
-                       },
                        args: args{
                                cc: &ClientConn{
                                        Account: &Account{
@@ -1113,6 +1099,16 @@ func TestHandleMakeAlias(t *testing.T) {
                                                        }(),
                                                },
                                                Logger: NewTestLogger(),
+                                               FS: func() *MockFileStore {
+                                                       mfs := &MockFileStore{}
+                                                       path, _ := os.Getwd()
+                                                       mfs.On(
+                                                               "Symlink",
+                                                               path+"/test/config/Files/foo/testFile",
+                                                               path+"/test/config/Files/bar/testFile",
+                                                       ).Return(nil)
+                                                       return mfs
+                                               }(),
                                        },
                                },
                                t: NewTransaction(
@@ -1136,16 +1132,6 @@ func TestHandleMakeAlias(t *testing.T) {
                },
                {
                        name: "when symlink returns an error",
-                       setup: func() {
-                               mfs := &MockFileStore{}
-                               path, _ := os.Getwd()
-                               mfs.On(
-                                       "Symlink",
-                                       path+"/test/config/Files/foo/testFile",
-                                       path+"/test/config/Files/bar/testFile",
-                               ).Return(errors.New("ohno"))
-                               FS = mfs
-                       },
                        args: args{
                                cc: &ClientConn{
                                        Account: &Account{
@@ -1164,6 +1150,16 @@ func TestHandleMakeAlias(t *testing.T) {
                                                        }(),
                                                },
                                                Logger: NewTestLogger(),
+                                               FS: func() *MockFileStore {
+                                                       mfs := &MockFileStore{}
+                                                       path, _ := os.Getwd()
+                                                       mfs.On(
+                                                               "Symlink",
+                                                               path+"/test/config/Files/foo/testFile",
+                                                               path+"/test/config/Files/bar/testFile",
+                                                       ).Return(errors.New("ohno"))
+                                                       return mfs
+                                               }(),
                                        },
                                },
                                t: NewTransaction(
@@ -1188,8 +1184,7 @@ func TestHandleMakeAlias(t *testing.T) {
                        wantErr: false,
                },
                {
-                       name:  "when user does not have required permission",
-                       setup: func() {},
+                       name: "when user does not have required permission",
                        args: args{
                                cc: &ClientConn{
                                        Account: &Account{
@@ -1242,8 +1237,6 @@ func TestHandleMakeAlias(t *testing.T) {
        }
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {
-                       tt.setup()
-
                        gotRes, err := HandleMakeAlias(tt.args.cc, tt.args.t)
                        if (err != nil) != tt.wantErr {
                                t.Errorf("HandleMakeAlias(%v, %v)", tt.args.cc, tt.args.t)
@@ -1400,18 +1393,12 @@ func TestHandleDeleteUser(t *testing.T) {
        }
        tests := []struct {
                name    string
-               setup   func()
                args    args
                wantRes []Transaction
                wantErr assert.ErrorAssertionFunc
        }{
                {
                        name: "when user exists",
-                       setup: func() {
-                               mfs := &MockFileStore{}
-                               mfs.On("Remove", "Users/testuser.yaml").Return(nil)
-                               FS = mfs
-                       },
                        args: args{
                                cc: &ClientConn{
                                        Account: &Account{
@@ -1431,6 +1418,11 @@ func TestHandleDeleteUser(t *testing.T) {
                                                                Access:   &[]byte{1},
                                                        },
                                                },
+                                               FS: func() *MockFileStore {
+                                                       mfs := &MockFileStore{}
+                                                       mfs.On("Remove", "Users/testuser.yaml").Return(nil)
+                                                       return mfs
+                                               }(),
                                        },
                                },
                                t: NewTransaction(
@@ -1451,8 +1443,7 @@ func TestHandleDeleteUser(t *testing.T) {
                        wantErr: assert.NoError,
                },
                {
-                       name:  "when user does not have required permission",
-                       setup: func() {},
+                       name: "when user does not have required permission",
                        args: args{
                                cc: &ClientConn{
                                        Account: &Account{
@@ -1488,7 +1479,6 @@ func TestHandleDeleteUser(t *testing.T) {
        }
        for _, tt := range tests {
                t.Run(tt.name, func(t *testing.T) {
-                       tt.setup()
                        gotRes, err := HandleDeleteUser(tt.args.cc, tt.args.t)
                        if !tt.wantErr(t, err, fmt.Sprintf("HandleDeleteUser(%v, %v)", tt.args.cc, tt.args.t)) {
                                return