From 98ed0a3409c1eda06e9b6bf36d063ae1f628359a Mon Sep 17 00:00:00 2001 From: Martin Bertschler Date: Sun, 20 Dec 2015 22:09:09 +0100 Subject: [PATCH] return improved errors from MemMapFs methods MemMapFs.Mkdir now returns an error if the file or dir already exists MemMapFs.Remove now returns an error if the file or dir doesn't exists fixes #43 --- README.md | 2 +- memmap.go | 42 +++++++++++++-------------- memmap_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 99 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 61bc984..5c54057 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A FileSystem Abstraction System for Go -[![Build Status](https://travis-ci.org/spf13/afero.svg)](https://travis-ci.org/spf13/afero) [![Build status](https://ci.appveyor.com/api/projects/status/github/spf13/afero?branch=master&svg=true)](https://ci.appveyor.com/project/spf13/afero) [![GoDoc](https://godoc.org/github.com/spf13/afero?status.svg)](https://godoc.org/github.com/spf13/afero)[![Join the chat at https://gitter.im/spf13/afero](https://badges.gitter.im/Dev%20Chat.svg)](https://gitter.im/spf13/afero?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) +[![Build Status](https://travis-ci.org/spf13/afero.svg)](https://travis-ci.org/spf13/afero) [![Build status](https://ci.appveyor.com/api/projects/status/github/spf13/afero?branch=master&svg=true)](https://ci.appveyor.com/project/spf13/afero) [![GoDoc](https://godoc.org/github.com/spf13/afero?status.svg)](https://godoc.org/github.com/spf13/afero) [![Join the chat at https://gitter.im/spf13/afero](https://badges.gitter.im/Dev%20Chat.svg)](https://gitter.im/spf13/afero?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) # Overview diff --git a/memmap.go b/memmap.go index 3cfec11..ff9da99 100644 --- a/memmap.go +++ b/memmap.go @@ -56,13 +56,10 @@ func (m *MemMapFs) Create(name string) (File, error) { return file, nil } -func (m *MemMapFs) unRegisterWithParent(fileName string) { +func (m *MemMapFs) unRegisterWithParent(fileName string) error { f, err := m.lockfreeOpen(fileName) if err != nil { - if os.IsNotExist(err) { - log.Printf("Unable to unregister \"%v\" with Parent, err: %v", fileName, err) - } - return + return err } parent := m.findParent(f) if parent == nil { @@ -71,6 +68,7 @@ func (m *MemMapFs) unRegisterWithParent(fileName string) { pmem := parent.(*mem.File) fmem := f.(*mem.File) mem.RemoveFromMemDir(pmem, fmem) + return nil } func (m *MemMapFs) findParent(f File) File { @@ -136,17 +134,10 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error { name = normalizePath(name) m.mu.RLock() - x, ok := m.getData()[name] + _, ok := m.getData()[name] m.mu.RUnlock() if ok { - // Only return ErrFileExists if it's a file, not a directory. - i, err := x.Stat() - if !i.IsDir() { - return ErrFileExists - } - if err != nil { - return err - } + return &os.PathError{"mkdir", name, ErrFileExists} } else { m.mu.Lock() item := mem.CreateDir(name) @@ -158,7 +149,15 @@ func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error { } func (m *MemMapFs) MkdirAll(path string, perm os.FileMode) error { - return m.Mkdir(path, perm) + err := m.Mkdir(path, perm) + if err != nil { + if err.(*os.PathError).Err == ErrFileExists { + return nil + } else { + return err + } + } + return nil } // Handle some relative paths @@ -190,7 +189,7 @@ func (m *MemMapFs) Open(name string) (File, error) { if ok { return f, nil } else { - return nil, ErrFileNotFound + return nil, &os.PathError{"open", name, ErrFileNotFound} } } @@ -200,8 +199,6 @@ func (m *MemMapFs) lockfreeOpen(name string) (File, error) { ff, ok := f.(*mem.File) if ok { ff.Open() - } - if ok { return f, nil } else { return nil, ErrFileNotFound @@ -240,7 +237,10 @@ func (m *MemMapFs) Remove(name string) error { defer m.mu.Unlock() if _, ok := m.getData()[name]; ok { - m.unRegisterWithParent(name) + err := m.unRegisterWithParent(name) + if err != nil { + return &os.PathError{"remove", name, err} + } delete(m.getData(), name) } else { return &os.PathError{"remove", name, os.ErrNotExist} @@ -292,10 +292,10 @@ func (m *MemMapFs) Rename(oldname, newname string) error { m.mu.Unlock() m.mu.RLock() } else { - return ErrDestinationExists + return &os.PathError{"rename", newname, ErrDestinationExists} } } else { - return ErrFileNotFound + return &os.PathError{"rename", oldname, ErrFileNotFound} } return nil } diff --git a/memmap_test.go b/memmap_test.go index 53ff2af..ce9a4e5 100644 --- a/memmap_test.go +++ b/memmap_test.go @@ -1,6 +1,11 @@ package afero -import "testing" +import ( + "os" + "path/filepath" + "testing" + "time" +) func TestNormalizePath(t *testing.T) { type test struct { @@ -24,3 +29,74 @@ func TestNormalizePath(t *testing.T) { } } } + +func TestPathErrors(t *testing.T) { + path := filepath.Join(".", "some", "path") + path2 := filepath.Join(".", "different", "path") + fs := &MemMapFs{} + perm := os.FileMode(0755) + + // relevant functions: + // func (m *MemMapFs) Chmod(name string, mode os.FileMode) error + // func (m *MemMapFs) Chtimes(name string, atime time.Time, mtime time.Time) error + // func (m *MemMapFs) Create(name string) (File, error) + // func (m *MemMapFs) Mkdir(name string, perm os.FileMode) error + // func (m *MemMapFs) MkdirAll(path string, perm os.FileMode) error + // func (m *MemMapFs) Open(name string) (File, error) + // func (m *MemMapFs) OpenFile(name string, flag int, perm os.FileMode) (File, error) + // func (m *MemMapFs) Remove(name string) error + // func (m *MemMapFs) Rename(oldname, newname string) error + // func (m *MemMapFs) Stat(name string) (os.FileInfo, error) + + err := fs.Chmod(path, perm) + checkPathError(t, err, "Chmod") + + err = fs.Chtimes(path, time.Now(), time.Now()) + checkPathError(t, err, "Chtimes") + + // fs.Create doesn't return an error + + err = fs.Mkdir(path2, perm) + if err != nil { + t.Error(err) + } + err = fs.Mkdir(path2, perm) + checkPathError(t, err, "Mkdir") + + err = fs.MkdirAll(path2, perm) + if err != nil { + t.Error("MkdirAll:", err) + } + + _, err = fs.Open(path) + checkPathError(t, err, "Open") + + _, err = fs.OpenFile(path, os.O_RDWR, perm) + checkPathError(t, err, "OpenFile") + + err = fs.Remove(path) + checkPathError(t, err, "Remove") + + err = fs.RemoveAll(path) + if err != nil { + t.Error("RemoveAll:", err) + } + + err = fs.Rename(path, path2) + checkPathError(t, err, "Rename") + + _, err = fs.Stat(path) + checkPathError(t, err, "Stat") +} + +func checkPathError(t *testing.T, err error, op string) { + pathErr, ok := err.(*os.PathError) + if !ok { + t.Error(op+":", err, "is not a os.PathError") + return + } + _, ok = pathErr.Err.(*os.PathError) + if ok { + t.Error(op+":", err, "contains another os.PathError") + } +}