From e6e290ca7fa893ee02903a41aec0c6fefad6fa00 Mon Sep 17 00:00:00 2001 From: John Episcopo Date: Wed, 1 May 2019 17:00:01 +0100 Subject: [PATCH 1/5] Added initial logic and tests for the Walker --- ftp.go | 14 +++++ walker.go | 84 ++++++++++++++++++++++++++++ walker_test.go | 147 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 walker.go create mode 100644 walker_test.go diff --git a/ftp.go b/ftp.go index 9746e05..7184c89 100644 --- a/ftp.go +++ b/ftp.go @@ -680,6 +680,20 @@ func (c *ServerConn) RemoveDir(path string) error { return err } +//Walk prepares the internal walk function so that the caller can begin traversing the directory +func (c *ServerConn) Walk(root string) *Walker { + w := new(Walker) + w.serverConn = c + + if !strings.HasSuffix(root, "/") { + root += "/" + } + + w.root = root + + return w +} + // NoOp issues a NOOP FTP command. // NOOP has no effects and is usually used to prevent the remote FTP server to // close the otherwise idle connection. diff --git a/walker.go b/walker.go new file mode 100644 index 0000000..d87a1cf --- /dev/null +++ b/walker.go @@ -0,0 +1,84 @@ +package ftp + +import ( + "fmt" + "strings" +) + +//Walker traverses the directory tree of a remote FTP server +type Walker struct { + serverConn *ServerConn + root string + cur item + stack []item + descend bool +} + +type item struct { + path string + entry Entry + err error +} + +// Step advances the Walker to the next file or directory, +// which will then be available through the Path, Stat, and Err methods. +// It returns false when the walk stops at the end of the tree. +func (w *Walker) Step() bool { + if w.descend && w.cur.err == nil && w.cur.entry.Type == EntryTypeFolder { + list, err := w.serverConn.List(w.cur.path) + if err != nil { + w.cur.err = nil + w.stack = append(w.stack, w.cur) + } else { + for i := len(list) - 1; i >= 0; i-- { + if !strings.HasSuffix(w.cur.path, "/") { + w.cur.path += "/" + } + + var path string + if list[i].Type == EntryTypeFolder { + path = fmt.Sprintf("%s%s", w.cur.path, list[i].Name) + } else { + path = w.cur.path + } + + w.stack = append(w.stack, item{path, *list[i], nil}) + } + } + } + + if len(w.stack) == 0 { + return false + } + i := len(w.stack) - 1 + w.cur = w.stack[i] + w.stack = w.stack[:i] + w.descend = true + return true +} + +//SkipDir tells the step function to skip the currently processed directory +func (w *Walker) SkipDir() { + w.descend = false +} + +//Err returns the error, if any, for the most recent attempt by Step to +//visit a file or a directory. If a directory has an error, the walker +//will not descend in that directory +func (w *Walker) Err() error { + return w.cur.err +} + +// Stat returns info for the most recent file or directory +// visited by a call to Step. +func (w *Walker) Stat() Entry { + return w.cur.entry +} + +// Path returns the path to the most recent file or directory +// visited by a call to Step. It contains the argument to Walk +// as a prefix; that is, if Walk is called with "dir", which is +// a directory containing the file "a", Path will return "dir/a". +func (w *Walker) Path() string { + return w.cur.path +} diff --git a/walker_test.go b/walker_test.go new file mode 100644 index 0000000..1b6fea1 --- /dev/null +++ b/walker_test.go @@ -0,0 +1,147 @@ +package ftp + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestFieldsReturnCorrectData(t *testing.T) { + w := Walker{ + cur: item{ + path: "/root/", + err: fmt.Errorf("This is an error"), + entry: Entry{ + Name: "root", + Size: 123, + Time: time.Now(), + Type: EntryTypeFolder, + }, + }, + } + + assert.Equal(t, "This is an error", w.Err().Error()) + assert.Equal(t, "/root/", w.Path()) + assert.Equal(t, EntryTypeFolder, w.Stat().Type) +} + +func TestSkipDirIsCorrectlySet(t *testing.T) { + w := Walker{} + + w.SkipDir() + + assert.Equal(t, false, w.descend) +} + +func TestNoDescendDoesNotAddToStack(t *testing.T) { + w := new(Walker) + w.cur = item{ + path: "/root/", + err: nil, + entry: Entry{ + Name: "root", + Size: 123, + Time: time.Now(), + Type: EntryTypeFolder, + }, + } + + w.stack = []item{ + item{ + path: "file", + err: nil, + entry: Entry{ + Name: "file", + Size: 123, + Time: time.Now(), + Type: EntryTypeFile, + }, + }, + } + + w.SkipDir() + + result := w.Step() + + assert.Equal(t, true, result, "Result should return true") + assert.Equal(t, 1, len(w.stack)) + assert.Equal(t, true, w.descend) +} + +func TestEmptyStackReturnsFalse(t *testing.T) { + w := new(Walker) + w.cur = item{ + path: "/root/", + err: nil, + entry: Entry{ + Name: "root", + Size: 123, + Time: time.Now(), + Type: EntryTypeFolder, + }, + } + + w.stack = []item{} + + w.SkipDir() + + result := w.Step() + + assert.Equal(t, false, result, "Result should return false") +} + +func TestCurAndStackSetCorrectly(t *testing.T) { + w := new(Walker) + w.cur = item{ + path: "root/file1", + err: nil, + entry: Entry{ + Name: "file1", + Size: 123, + Time: time.Now(), + Type: EntryTypeFile, + }, + } + + w.stack = []item{ + item{ + path: "file", + err: nil, + entry: Entry{ + Name: "file", + Size: 123, + Time: time.Now(), + Type: EntryTypeFile, + }, + }, + item{ + path: "root/file1", + err: nil, + entry: Entry{ + Name: "file1", + Size: 123, + Time: time.Now(), + Type: EntryTypeFile, + }, + }, + } + + result := w.Step() + result = w.Step() + + assert.Equal(t, true, result, "Result should return true") + assert.Equal(t, 0, len(w.stack)) + assert.Equal(t, "file", w.cur.entry.Name) +} + +func TestErrorsFromListAreHandledCorrectly(t *testing.T) { + //Get error + //Check w.cur.err + //Check stack +} + +func TestStackIsPopulatedCorrectly(t *testing.T) { + //Check things are added to the stack correcty +} From a6055c7bc8ebe5df2fc8114c78f676eeb2cb9395 Mon Sep 17 00:00:00 2001 From: John Episcopo Date: Thu, 2 May 2019 09:29:41 +0100 Subject: [PATCH 2/5] Added further tests --- walker_test.go | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/walker_test.go b/walker_test.go index 1b6fea1..a2f7772 100644 --- a/walker_test.go +++ b/walker_test.go @@ -2,6 +2,7 @@ package ftp import ( "fmt" + "strings" "testing" "time" @@ -66,7 +67,7 @@ func TestNoDescendDoesNotAddToStack(t *testing.T) { result := w.Step() assert.Equal(t, true, result, "Result should return true") - assert.Equal(t, 1, len(w.stack)) + assert.Equal(t, 0, len(w.stack)) assert.Equal(t, true, w.descend) } @@ -136,12 +137,37 @@ func TestCurAndStackSetCorrectly(t *testing.T) { assert.Equal(t, "file", w.cur.entry.Name) } -func TestErrorsFromListAreHandledCorrectly(t *testing.T) { - //Get error - //Check w.cur.err - //Check stack -} - func TestStackIsPopulatedCorrectly(t *testing.T) { - //Check things are added to the stack correcty + + mock, err := newFtpMock(t, "127.0.0.1") + if err != nil { + t.Fatal(err) + } + defer mock.Close() + + c, cErr := Connect(mock.Addr()) + if cErr != nil { + t.Fatal(err) + } + + w := Walker{ + cur: item{ + path: "/root", + entry: Entry{ + Name: "root", + Size: 123, + Time: time.Now(), + Type: EntryTypeFolder, + }, + }, + serverConn: c, + } + + w.descend = true + + w.Step() + + assert.Equal(t, 0, len(w.stack)) + assert.Equal(t, "lo", w.cur.entry.Name) + assert.Equal(t, true, strings.HasSuffix(w.cur.path, "/")) } From 434fa14f3d25febd29ae3783bbe7bca6f6d4856f Mon Sep 17 00:00:00 2001 From: John Episcopo Date: Thu, 2 May 2019 09:47:29 +0100 Subject: [PATCH 3/5] Added test to check the creation of the walker is --- walker_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/walker_test.go b/walker_test.go index a2f7772..81877af 100644 --- a/walker_test.go +++ b/walker_test.go @@ -9,6 +9,24 @@ import ( "github.com/stretchr/testify/assert" ) +func TestWalkReturnsCorrectlyPopulatedWalker(t *testing.T) { + mock, err := newFtpMock(t, "127.0.0.1") + if err != nil { + t.Fatal(err) + } + defer mock.Close() + + c, cErr := Connect(mock.Addr()) + if cErr != nil { + t.Fatal(err) + } + + w := c.Walk("root") + + assert.Equal(t, "root/", w.root) + assert.Equal(t, &c, &w.serverConn) +} + func TestFieldsReturnCorrectData(t *testing.T) { w := Walker{ cur: item{ From 74a8c4bcbc715b2eed925a97673fb36b63d13d60 Mon Sep 17 00:00:00 2001 From: John Episcopo Date: Mon, 20 May 2019 08:56:38 +0100 Subject: [PATCH 4/5] Renamed Step func to Next() as per PR comments --- walker.go | 10 +++++----- walker_test.go | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/walker.go b/walker.go index d87a1cf..e5f48f4 100644 --- a/walker.go +++ b/walker.go @@ -20,10 +20,10 @@ type item struct { err error } -// Step advances the Walker to the next file or directory, +// Next advances the Walker to the next file or directory, // which will then be available through the Path, Stat, and Err methods. // It returns false when the walk stops at the end of the tree. -func (w *Walker) Step() bool { +func (w *Walker) Next() bool { if w.descend && w.cur.err == nil && w.cur.entry.Type == EntryTypeFolder { list, err := w.serverConn.List(w.cur.path) if err != nil { @@ -57,12 +57,12 @@ func (w *Walker) Step() bool { return true } -//SkipDir tells the step function to skip the currently processed directory +//SkipDir tells the Next function to skip the currently processed directory func (w *Walker) SkipDir() { w.descend = false } -//Err returns the error, if any, for the most recent attempt by Step to +//Err returns the error, if any, for the most recent attempt by Next to //visit a file or a directory. If a directory has an error, the walker //will not descend in that directory func (w *Walker) Err() error { @@ -76,7 +76,7 @@ func (w *Walker) Stat() Entry { } // Path returns the path to the most recent file or directory -// visited by a call to Step. It contains the argument to Walk +// visited by a call to Next. It contains the argument to Walk // as a prefix; that is, if Walk is called with "dir", which is // a directory containing the file "a", Path will return "dir/a". func (w *Walker) Path() string { diff --git a/walker_test.go b/walker_test.go index 81877af..f4a4aeb 100644 --- a/walker_test.go +++ b/walker_test.go @@ -82,7 +82,7 @@ func TestNoDescendDoesNotAddToStack(t *testing.T) { w.SkipDir() - result := w.Step() + result := w.Next() assert.Equal(t, true, result, "Result should return true") assert.Equal(t, 0, len(w.stack)) @@ -106,7 +106,7 @@ func TestEmptyStackReturnsFalse(t *testing.T) { w.SkipDir() - result := w.Step() + result := w.Next() assert.Equal(t, false, result, "Result should return false") } @@ -147,8 +147,8 @@ func TestCurAndStackSetCorrectly(t *testing.T) { }, } - result := w.Step() - result = w.Step() + result := w.Next() + result = w.Next() assert.Equal(t, true, result, "Result should return true") assert.Equal(t, 0, len(w.stack)) @@ -183,7 +183,7 @@ func TestStackIsPopulatedCorrectly(t *testing.T) { w.descend = true - w.Step() + w.Next() assert.Equal(t, 0, len(w.stack)) assert.Equal(t, "lo", w.cur.entry.Name) From 37a04759dd46afeea20c5289af4b5a3f0de14542 Mon Sep 17 00:00:00 2001 From: johnepiscopo Date: Tue, 10 Mar 2020 14:26:54 +0000 Subject: [PATCH 5/5] Fixed a typo so that we are no longer ignoring an error and am now using path.Join instead of fmt.Sprintf() --- walker.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/walker.go b/walker.go index e5f48f4..dacd0d8 100644 --- a/walker.go +++ b/walker.go @@ -1,7 +1,7 @@ package ftp import ( - "fmt" + pa "path" "strings" ) @@ -27,7 +27,7 @@ func (w *Walker) Next() bool { if w.descend && w.cur.err == nil && w.cur.entry.Type == EntryTypeFolder { list, err := w.serverConn.List(w.cur.path) if err != nil { - w.cur.err = nil + w.cur.err = err w.stack = append(w.stack, w.cur) } else { for i := len(list) - 1; i >= 0; i-- { @@ -37,7 +37,7 @@ func (w *Walker) Next() bool { var path string if list[i].Type == EntryTypeFolder { - path = fmt.Sprintf("%s%s", w.cur.path, list[i].Name) + path = pa.Join(w.cur.path, list[i].Name) } else { path = w.cur.path }