From cf98ac506bc06b46ae160db6f6f2788ecf97d72b Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 3 Apr 2023 11:06:26 +0200 Subject: [PATCH 1/7] add option to handle errors --- README.md | 3 +++ all_test.go | 33 +++++++++++++++++++++++++++++++++ copy.go | 21 +++++++++++++++++---- options.go | 4 ++++ 4 files changed, 57 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 32edfd8..d95f9f5 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,9 @@ type Options struct { // OnDirExists can specify what to do when there is a directory already existing in destination. OnDirExists func(src, dest string) DirExistsAction + // OnErr lets called decide whether or not to continue on particular copy error. + OnErr func(err error) error + // Skip can specify which files should be skipped Skip func(srcinfo os.FileInfo, src, dest string) (bool, error) diff --git a/all_test.go b/all_test.go index 910570a..f5348ef 100644 --- a/all_test.go +++ b/all_test.go @@ -362,6 +362,39 @@ func TestOptions_CopyRateLimit(t *testing.T) { Expect(t, elapsed > 5*time.Second).ToBe(true) } +func TestOptions_OnFileError(t *testing.T) { + opt := Options{ + OnErr: nil, + } + + // existing, process nromally + err := Copy("test/data/case17", "test/data.copy/case17", opt) + Expect(t, err).ToBe(nil) + + // not existing, process err + err = Copy("test/data/case17/non-existing", "test/data.copy/case17/non-existing", opt) + Expect(t, os.IsNotExist(err)).ToBe(true) + + _, err = os.Stat("test/data.copy/case17/non-existing") + Expect(t, os.IsNotExist(err)).ToBe(true) + + // not existing, process err + opt.OnErr = func(err error) error { return err } + err = Copy("test/data/case17/non-existing", "test/data.copy/case17/non-existing", opt) + Expect(t, os.IsNotExist(err)).ToBe(true) + + _, err = os.Stat("test/data.copy/case17/non-existing") + Expect(t, os.IsNotExist(err)).ToBe(true) + + // not existing, ignore err + opt.OnErr = func(err error) error { return nil } + err = Copy("test/data/case17/non-existing", "test/data.copy/case17/non-existing", opt) + Expect(t, err).ToBe(nil) + + _, err = os.Stat("test/data.copy/case17/non-existing") + Expect(t, os.IsNotExist(err)).ToBe(true) +} + type SleepyReader struct { src *os.File sec time.Duration diff --git a/copy.go b/copy.go index 60643dd..6458918 100644 --- a/copy.go +++ b/copy.go @@ -15,17 +15,21 @@ type timespec struct { } // Copy copies src to dest, doesn't matter if src is a directory or a file. -func Copy(src, dest string, opt ...Options) error { +func Copy(src, dest string, opts ...Options) error { + opt := assureOptions(src, dest, opts...) info, err := os.Lstat(src) if err != nil { - return err + return onError(err, opt) // called both, we don't know if it's a file or directory } - return switchboard(src, dest, info, assureOptions(src, dest, opt...)) + return switchboard(src, dest, info, opt) } // switchboard switches proper copy functions regarding file type, etc... // If there would be anything else here, add a case to this switchboard. func switchboard(src, dest string, info os.FileInfo, opt Options) (err error) { + defer func() { + err = onError(err, opt) + }() if info.Mode()&os.ModeDevice != 0 && !opt.Specials { return err @@ -130,7 +134,6 @@ func fcopy(src, dest string, info os.FileInfo, opt Options) (err error) { // with scanning contents inside the directory // and pass everything to "copy" recursively. func dcopy(srcdir, destdir string, info os.FileInfo, opt Options) (err error) { - if skip, err := onDirExists(opt, srcdir, destdir); err != nil { return err } else if skip { @@ -235,3 +238,13 @@ func fclose(f *os.File, reported *error) { *reported = err } } + +// onError lets caller to handle errors +// occured when copying a file. +func onError(err error, opt Options) error { + if opt.OnErr == nil { + return err + } + + return opt.OnErr(err) +} diff --git a/options.go b/options.go index bd18d56..bc898fa 100644 --- a/options.go +++ b/options.go @@ -14,6 +14,9 @@ type Options struct { // OnDirExists can specify what to do when there is a directory already existing in destination. OnDirExists func(src, dest string) DirExistsAction + // OnErr lets called decide whether or not to continue on particular copy error. + OnErr func(err error) error + // Skip can specify which files should be skipped Skip func(srcinfo os.FileInfo, src, dest string) (bool, error) @@ -95,6 +98,7 @@ func getDefaultOptions(src, dest string) Options { return Shallow // Do shallow copy }, OnDirExists: nil, // Default behavior is "Merge". + OnErr: nil, // Default is "accept error" Skip: nil, // Do not skip anything AddPermission: 0, // Add nothing PermissionControl: PerservePermission, // Just preserve permission From a7319430081205fc2f5a8c6471a62b54ec52ba95 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 3 Apr 2023 11:06:57 +0200 Subject: [PATCH 2/7] add option to handle errors --- copy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/copy.go b/copy.go index 6458918..b72f35a 100644 --- a/copy.go +++ b/copy.go @@ -19,7 +19,7 @@ func Copy(src, dest string, opts ...Options) error { opt := assureOptions(src, dest, opts...) info, err := os.Lstat(src) if err != nil { - return onError(err, opt) // called both, we don't know if it's a file or directory + return onError(err, opt) } return switchboard(src, dest, info, opt) } From 816a7059e2ca8782f4e257b7c201351fdc4bab26 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 3 Apr 2023 11:43:38 +0200 Subject: [PATCH 3/7] nil err check --- all_test.go | 10 ++++++++++ copy.go | 3 +++ 2 files changed, 13 insertions(+) diff --git a/all_test.go b/all_test.go index f5348ef..8a36ceb 100644 --- a/all_test.go +++ b/all_test.go @@ -378,6 +378,16 @@ func TestOptions_OnFileError(t *testing.T) { _, err = os.Stat("test/data.copy/case17/non-existing") Expect(t, os.IsNotExist(err)).ToBe(true) + // existing, err not passed + var called bool + opt.OnErr = func(err error) error { + called = true + return err + } + err = Copy("test/data/case17", "test/data.copy/case17", opt) + Expect(t, err).ToBe(nil) + Expect(t, called).ToBe(false) + // not existing, process err opt.OnErr = func(err error) error { return err } err = Copy("test/data/case17/non-existing", "test/data.copy/case17/non-existing", opt) diff --git a/copy.go b/copy.go index b72f35a..84bdfba 100644 --- a/copy.go +++ b/copy.go @@ -242,6 +242,9 @@ func fclose(f *os.File, reported *error) { // onError lets caller to handle errors // occured when copying a file. func onError(err error, opt Options) error { + if err == nil { + return nil + } if opt.OnErr == nil { return err } From b348b50c44542ae4880ef74b2918e03dfc888e1b Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 3 Apr 2023 13:32:09 +0200 Subject: [PATCH 4/7] missing files --- test/data/case17/README.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 test/data/case17/README.md diff --git a/test/data/case17/README.md b/test/data/case17/README.md new file mode 100644 index 0000000..880f3b9 --- /dev/null +++ b/test/data/case17/README.md @@ -0,0 +1,5 @@ +So if you wanted to ignore error you should add something like this: +```go +opt.OnFileErr = func(_ error) error { return nil } +``` +The default value is nil and accepts raised error. \ No newline at end of file From 80eee8d7eb9f75142db2d16b5ba9c40818d2f6d9 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 17 Apr 2023 08:48:54 +0200 Subject: [PATCH 5/7] updated according to code review --- all_test.go | 13 +++++-------- copy.go | 19 ++++++------------- options.go | 4 ++-- test/data/case17/README.md | 2 +- 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/all_test.go b/all_test.go index 8a36ceb..a370e44 100644 --- a/all_test.go +++ b/all_test.go @@ -364,7 +364,7 @@ func TestOptions_CopyRateLimit(t *testing.T) { func TestOptions_OnFileError(t *testing.T) { opt := Options{ - OnErr: nil, + OnError: nil, } // existing, process nromally @@ -378,18 +378,15 @@ func TestOptions_OnFileError(t *testing.T) { _, err = os.Stat("test/data.copy/case17/non-existing") Expect(t, os.IsNotExist(err)).ToBe(true) - // existing, err not passed - var called bool - opt.OnErr = func(err error) error { - called = true + // existing, nil err not passed + opt.OnError = func(_, _ string, err error) error { return err } err = Copy("test/data/case17", "test/data.copy/case17", opt) Expect(t, err).ToBe(nil) - Expect(t, called).ToBe(false) // not existing, process err - opt.OnErr = func(err error) error { return err } + opt.OnError = func(_, _ string, err error) error { return err } err = Copy("test/data/case17/non-existing", "test/data.copy/case17/non-existing", opt) Expect(t, os.IsNotExist(err)).ToBe(true) @@ -397,7 +394,7 @@ func TestOptions_OnFileError(t *testing.T) { Expect(t, os.IsNotExist(err)).ToBe(true) // not existing, ignore err - opt.OnErr = func(err error) error { return nil } + opt.OnError = func(_, _ string, err error) error { return nil } err = Copy("test/data/case17/non-existing", "test/data.copy/case17/non-existing", opt) Expect(t, err).ToBe(nil) diff --git a/copy.go b/copy.go index 84bdfba..71ff844 100644 --- a/copy.go +++ b/copy.go @@ -19,7 +19,7 @@ func Copy(src, dest string, opts ...Options) error { opt := assureOptions(src, dest, opts...) info, err := os.Lstat(src) if err != nil { - return onError(err, opt) + return onError(src, dest, err, opt) } return switchboard(src, dest, info, opt) } @@ -27,12 +27,8 @@ func Copy(src, dest string, opts ...Options) error { // switchboard switches proper copy functions regarding file type, etc... // If there would be anything else here, add a case to this switchboard. func switchboard(src, dest string, info os.FileInfo, opt Options) (err error) { - defer func() { - err = onError(err, opt) - }() - if info.Mode()&os.ModeDevice != 0 && !opt.Specials { - return err + return onError(src, dest, err, opt) } switch { @@ -46,7 +42,7 @@ func switchboard(src, dest string, info os.FileInfo, opt Options) (err error) { err = fcopy(src, dest, info, opt) } - return err + return onError(src, dest, err, opt) } // copyNextOrSkip decide if this src should be copied or not. @@ -241,13 +237,10 @@ func fclose(f *os.File, reported *error) { // onError lets caller to handle errors // occured when copying a file. -func onError(err error, opt Options) error { - if err == nil { - return nil - } - if opt.OnErr == nil { +func onError(src, dest string, err error, opt Options) error { + if opt.OnError == nil { return err } - return opt.OnErr(err) + return opt.OnError(src, dest, err) } diff --git a/options.go b/options.go index bc898fa..52d636c 100644 --- a/options.go +++ b/options.go @@ -15,7 +15,7 @@ type Options struct { OnDirExists func(src, dest string) DirExistsAction // OnErr lets called decide whether or not to continue on particular copy error. - OnErr func(err error) error + OnError func(src, dest string, err error) error // Skip can specify which files should be skipped Skip func(srcinfo os.FileInfo, src, dest string) (bool, error) @@ -98,7 +98,7 @@ func getDefaultOptions(src, dest string) Options { return Shallow // Do shallow copy }, OnDirExists: nil, // Default behavior is "Merge". - OnErr: nil, // Default is "accept error" + OnError: nil, // Default is "accept error" Skip: nil, // Do not skip anything AddPermission: 0, // Add nothing PermissionControl: PerservePermission, // Just preserve permission diff --git a/test/data/case17/README.md b/test/data/case17/README.md index 880f3b9..bfce975 100644 --- a/test/data/case17/README.md +++ b/test/data/case17/README.md @@ -1,5 +1,5 @@ So if you wanted to ignore error you should add something like this: ```go -opt.OnFileErr = func(_ error) error { return nil } +opt.OnError = func(src, dst string, _ error) error { return nil } ``` The default value is nil and accepts raised error. \ No newline at end of file From 209f18c9d5b879068eaaee33023011af78b177ff Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 18 Apr 2023 09:00:17 +0200 Subject: [PATCH 6/7] Update README.md Co-authored-by: Hiromu OCHIAI --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d95f9f5..699ca3f 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ type Options struct { OnDirExists func(src, dest string) DirExistsAction // OnErr lets called decide whether or not to continue on particular copy error. - OnErr func(err error) error + OnError func(src, dest, string, err error) error // Skip can specify which files should be skipped Skip func(srcinfo os.FileInfo, src, dest string) (bool, error) From 2abaf6c11b5fa2184bf7944831a1cb1161f21859 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 18 Apr 2023 09:00:25 +0200 Subject: [PATCH 7/7] Update README.md Co-authored-by: Hiromu OCHIAI --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 699ca3f..dc66c4e 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ type Options struct { // OnDirExists can specify what to do when there is a directory already existing in destination. OnDirExists func(src, dest string) DirExistsAction - // OnErr lets called decide whether or not to continue on particular copy error. + // OnError can let users decide how to handle errors (e.g., you can suppress specific error). OnError func(src, dest, string, err error) error // Skip can specify which files should be skipped