From dd444d7eddc77dcefa3fbe0616198c661afa01ca Mon Sep 17 00:00:00 2001 From: Simon Dassow Date: Fri, 18 Feb 2022 16:14:11 +0100 Subject: [PATCH 1/6] Add support for changing the default file mode using options pattern --- README.md | 14 +++++++------- atomic.go | 30 ++++++++++++++++++++++++++++-- atomic_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 atomic_test.go diff --git a/README.md b/README.md index 37cd673..26c7624 100644 --- a/README.md +++ b/README.md @@ -25,11 +25,11 @@ change either file. ## func WriteFile ``` go -func WriteFile(filename string, r io.Reader) (err error) +func WriteFile(filename string, r io.Reader, opts ...Option) (err error) ``` -WriteFile atomically writes the contents of r to the specified filepath. If -an error occurs, the target file is guaranteed to be either fully written, or -not written at all. WriteFile overwrites any file that exists at the -location (but only if the write fully succeeds, otherwise the existing file -is unmodified). - +WriteFile atomically writes the contents of r to the specified filepath. If an +error occurs, the target file is guaranteed to be either fully written, or not +written at all. WriteFile overwrites any file that exists at the location (but +only if the write fully succeeds, otherwise the existing file is unmodified). +Permissions are copied from an existing file or the FileMode option can be given +to be used instead of the default `0600` from ioutil.TempFile(). diff --git a/atomic.go b/atomic.go index f7e2706..bf3dce3 100644 --- a/atomic.go +++ b/atomic.go @@ -5,16 +5,36 @@ import ( "fmt" "io" "io/ioutil" + "io/fs" "os" "path/filepath" ) +type FileOptions struct { + mode fs.FileMode +} + +type Option func(*FileOptions) + +func FileMode(mode fs.FileMode) Option { + return func(opts *FileOptions) { + opts.mode = mode + } +} + // WriteFile atomically writes the contents of r to the specified filepath. If // an error occurs, the target file is guaranteed to be either fully written, or // not written at all. WriteFile overwrites any file that exists at the // location (but only if the write fully succeeds, otherwise the existing file -// is unmodified). -func WriteFile(filename string, r io.Reader) (err error) { +// is unmodified). Permissions are copied from an existing file or the FileMode +// option can be given to be used instead of the default `0600` from +// ioutil.TempFile(). +func WriteFile(filename string, r io.Reader, opts ...Option) (err error) { + fopts := &FileOptions{} + for _, opt := range opts { + opt(fopts) + } + // write to a temp file first, then we'll atomically replace the target file // with the temp file. dir, file := filepath.Split(filename) @@ -43,6 +63,12 @@ func WriteFile(filename string, r io.Reader) (err error) { if err := f.Sync(); err != nil { return fmt.Errorf("can't flush tempfile %q: %v", name, err) } + // when optional mode was given change default mode of temp file. + if fopts.mode != 0 { + if err := f.Chmod(fopts.mode); err != nil { + return fmt.Errorf("cannot change file mode %q: %v", name, err); + } + } if err := f.Close(); err != nil { return fmt.Errorf("can't close tempfile %q: %v", name, err) } diff --git a/atomic_test.go b/atomic_test.go new file mode 100644 index 0000000..7f59d9d --- /dev/null +++ b/atomic_test.go @@ -0,0 +1,39 @@ +package atomic + +import ( + "bytes" + "os" + "testing" +) + +func TestWriteFile(t *testing.T) { + file := "foo.txt" + content := bytes.NewBufferString("foo") + defer func() { _ = os.Remove(file) }() + if err := WriteFile(file, content); err != nil { + t.Errorf("Failed to write file: %q: %v", file, err) + } + fi, err := os.Stat(file) + if err != nil { + t.Errorf("Failed to stat file: %q: %v", file, err) + } + if fi.Mode() != 0600 { + t.Errorf("File mode not correct") + } +} + +func TestWriteFileMode(t *testing.T) { + file := "bar.txt" + content := bytes.NewBufferString("bar") + defer func() { _ = os.Remove(file) }() + if err := WriteFile(file, content, FileMode(0644)); err != nil { + t.Errorf("Failed to write file: %q: %v", file, err) + } + fi, err := os.Stat(file) + if err != nil { + t.Errorf("Failed to stat file: %q: %v", file, err) + } + if fi.Mode() != 0644 { + t.Errorf("File mode not correct") + } +} From f3e6fa59cd8b4122ae32090aab4dab64cff4e1fd Mon Sep 17 00:00:00 2001 From: Simon Dassow Date: Fri, 18 Feb 2022 16:22:24 +0100 Subject: [PATCH 2/6] Document FileMode option func --- atomic.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/atomic.go b/atomic.go index bf3dce3..394974a 100644 --- a/atomic.go +++ b/atomic.go @@ -16,6 +16,8 @@ type FileOptions struct { type Option func(*FileOptions) +// FileMode can be given as an argument to `WriteFile()` to change the default +// file mode from the default value of ioutil.TempFile() (`0600`). func FileMode(mode fs.FileMode) Option { return func(opts *FileOptions) { opts.mode = mode From 0fc121ab60becfe50453398fad49d046907b2401 Mon Sep 17 00:00:00 2001 From: Simon Dassow Date: Fri, 18 Feb 2022 17:46:43 +0100 Subject: [PATCH 3/6] Change namespace for compatibility by @eau-u4f --- atomic.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/atomic.go b/atomic.go index 394974a..08690c5 100644 --- a/atomic.go +++ b/atomic.go @@ -5,20 +5,19 @@ import ( "fmt" "io" "io/ioutil" - "io/fs" "os" "path/filepath" ) type FileOptions struct { - mode fs.FileMode + mode os.FileMode } type Option func(*FileOptions) // FileMode can be given as an argument to `WriteFile()` to change the default // file mode from the default value of ioutil.TempFile() (`0600`). -func FileMode(mode fs.FileMode) Option { +func FileMode(mode os.FileMode) Option { return func(opts *FileOptions) { opts.mode = mode } From 4ccece75fa0d8ec03632a77dd492e71cc38d692b Mon Sep 17 00:00:00 2001 From: Simon Dassow Date: Sat, 19 Feb 2022 11:13:08 +0100 Subject: [PATCH 4/6] Rename option to DefaultFileMode to express more precisely what it does --- atomic.go | 12 ++++++------ atomic_test.go | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/atomic.go b/atomic.go index 08690c5..1696e24 100644 --- a/atomic.go +++ b/atomic.go @@ -10,16 +10,16 @@ import ( ) type FileOptions struct { - mode os.FileMode + defaultMode os.FileMode } type Option func(*FileOptions) // FileMode can be given as an argument to `WriteFile()` to change the default // file mode from the default value of ioutil.TempFile() (`0600`). -func FileMode(mode os.FileMode) Option { +func DefaultFileMode(mode os.FileMode) Option { return func(opts *FileOptions) { - opts.mode = mode + opts.defaultMode = mode } } @@ -65,9 +65,9 @@ func WriteFile(filename string, r io.Reader, opts ...Option) (err error) { return fmt.Errorf("can't flush tempfile %q: %v", name, err) } // when optional mode was given change default mode of temp file. - if fopts.mode != 0 { - if err := f.Chmod(fopts.mode); err != nil { - return fmt.Errorf("cannot change file mode %q: %v", name, err); + if fopts.defaultMode != 0 { + if err := f.Chmod(fopts.defaultMode); err != nil { + return fmt.Errorf("cannot change file mode %q: %v", name, err) } } if err := f.Close(); err != nil { diff --git a/atomic_test.go b/atomic_test.go index 7f59d9d..5276708 100644 --- a/atomic_test.go +++ b/atomic_test.go @@ -22,11 +22,11 @@ func TestWriteFile(t *testing.T) { } } -func TestWriteFileMode(t *testing.T) { +func TestWriteDefaultFileMode(t *testing.T) { file := "bar.txt" content := bytes.NewBufferString("bar") defer func() { _ = os.Remove(file) }() - if err := WriteFile(file, content, FileMode(0644)); err != nil { + if err := WriteFile(file, content, DefaultFileMode(0644)); err != nil { t.Errorf("Failed to write file: %q: %v", file, err) } fi, err := os.Stat(file) From 0c6ee36932407820d68da0a6f8d8a7cbcd919fa1 Mon Sep 17 00:00:00 2001 From: Simon Dassow Date: Sat, 19 Feb 2022 11:21:12 +0100 Subject: [PATCH 5/6] Adjust documentation after option renaming --- README.md | 4 ++-- atomic.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 26c7624..ad26415 100644 --- a/README.md +++ b/README.md @@ -31,5 +31,5 @@ WriteFile atomically writes the contents of r to the specified filepath. If an error occurs, the target file is guaranteed to be either fully written, or not written at all. WriteFile overwrites any file that exists at the location (but only if the write fully succeeds, otherwise the existing file is unmodified). -Permissions are copied from an existing file or the FileMode option can be given -to be used instead of the default `0600` from ioutil.TempFile(). +Permissions are copied from an existing file or the DefaultFileMode option can +be given to be used instead of the default `0600` from ioutil.TempFile(). diff --git a/atomic.go b/atomic.go index 1696e24..47a1c19 100644 --- a/atomic.go +++ b/atomic.go @@ -27,9 +27,9 @@ func DefaultFileMode(mode os.FileMode) Option { // an error occurs, the target file is guaranteed to be either fully written, or // not written at all. WriteFile overwrites any file that exists at the // location (but only if the write fully succeeds, otherwise the existing file -// is unmodified). Permissions are copied from an existing file or the FileMode -// option can be given to be used instead of the default `0600` from -// ioutil.TempFile(). +// is unmodified). Permissions are copied from an existing file or the +// DefaultFileMode option can be given to be used instead of the default `0600` +// from ioutil.TempFile(). func WriteFile(filename string, r io.Reader, opts ...Option) (err error) { fopts := &FileOptions{} for _, opt := range opts { From 174b5d2a3ea6c4505fdb66476428951b1d1aa4bf Mon Sep 17 00:00:00 2001 From: Simon Dassow Date: Sat, 19 Feb 2022 11:25:31 +0100 Subject: [PATCH 6/6] Test for file mode preservation as well --- atomic_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/atomic_test.go b/atomic_test.go index 5276708..b6a1844 100644 --- a/atomic_test.go +++ b/atomic_test.go @@ -36,4 +36,18 @@ func TestWriteDefaultFileMode(t *testing.T) { if fi.Mode() != 0644 { t.Errorf("File mode not correct") } + // check if file mode is preserved + if err := os.Chmod(file, 0600); err != nil { + t.Errorf("Failed to change file mode: %q: %v", file, err) + } + if err := WriteFile(file, content, DefaultFileMode(0644)); err != nil { + t.Errorf("Failed to write file: %q: %v", file, err) + } + fi, err = os.Stat(file) + if err != nil { + t.Errorf("Failed to stat file: %q: %v", file, err) + } + if fi.Mode() != 0600 { + t.Errorf("File mode not correct") + } }