diff --git a/fs/sync.go b/fs/sync.go index 9ddd755c2..bbd6a442b 100644 --- a/fs/sync.go +++ b/fs/sync.go @@ -836,6 +836,77 @@ func (s *syncCopyMove) transfer(dst, src DirEntry, job listDirJob, jobs *[]listD } } +type matchPair struct { + src, dst DirEntry +} + +// Process the two sorted listings, matching up the items in the two +// sorted slices +// +// Into srcOnly go Entries which only exist in the srcList +// Into dstOnly go Entries which only exist in the dstList +// Into matches go matchPair's of src and dst which have the same name +// +// This checks for duplicates and checks the list is sorted. +func matchListings(srcList, dstList DirEntries) (srcOnly DirEntries, dstOnly DirEntries, matches []matchPair) { + for iSrc, iDst := 0, 0; ; iSrc, iDst = iSrc+1, iDst+1 { + var src, dst DirEntry + var srcRemote, dstRemote string + if iSrc < len(srcList) { + src = srcList[iSrc] + srcRemote = src.Remote() + } + if iDst < len(dstList) { + dst = dstList[iDst] + dstRemote = dst.Remote() + } + if src == nil && dst == nil { + break + } + if src != nil && iSrc > 0 { + prev := srcList[iSrc-1].Remote() + if srcRemote == prev { + Logf(src, "Duplicate file found in source - ignoring") + src = nil // ignore the src + } else if srcRemote < prev { + Errorf(src, "Out of order listing in source") + src = nil // ignore the src + } + } + if dst != nil && iDst > 0 { + prev := dstList[iDst-1].Remote() + if dstRemote == prev { + Logf(dst, "Duplicate file found in destination - ignoring") + dst = nil // ignore the dst + } else if dstRemote < prev { + Errorf(dst, "Out of order listing in destination") + dst = nil // ignore the dst + } + } + if src != nil && dst != nil { + if srcRemote < dstRemote { + dst = nil + iDst-- // retry the dst + } else if srcRemote > dstRemote { + src = nil + iSrc-- // retry the src + } + } + // Debugf(nil, "src = %v, dst = %v", src, dst) + switch { + case src == nil && dst == nil: + // do nothing + case src == nil: + dstOnly = append(dstOnly, dst) + case dst == nil: + srcOnly = append(srcOnly, src) + default: + matches = append(matches, matchPair{src: src, dst: dst}) + } + } + return +} + // returns errors using processError func (s *syncCopyMove) _run(job listDirJob) (jobs []listDirJob) { var ( @@ -873,39 +944,25 @@ func (s *syncCopyMove) _run(job listDirJob) (jobs []listDirJob) { return nil } - // Process the two listings, matching up the items in the two sorted slices - for iSrc, iDst := 0, 0; ; iSrc, iDst = iSrc+1, iDst+1 { + // Work out what to do and do it + srcOnly, dstOnly, matches := matchListings(srcList, dstList) + for _, src := range srcOnly { if s.aborting() { return nil } - var src, dst DirEntry - if iSrc < len(srcList) { - src = srcList[iSrc] + s.srcOnly(src, job, &jobs) + } + for _, dst := range dstOnly { + if s.aborting() { + return nil } - if iDst < len(dstList) { - dst = dstList[iDst] - } - if src == nil && dst == nil { - break - } - if src != nil && dst != nil { - if src.Remote() < dst.Remote() { - dst = nil - iDst-- // retry the dst - } else if src.Remote() > dst.Remote() { - src = nil - iSrc-- // retry the src - } - } - // Debugf(nil, "src = %v, dst = %v", src, dst) - switch { - case src == nil: - s.dstOnly(dst, job, &jobs) - case dst == nil: - s.srcOnly(src, job, &jobs) - default: - s.transfer(dst, src, job, &jobs) + s.dstOnly(dst, job, &jobs) + } + for _, match := range matches { + if s.aborting() { + return nil } + s.transfer(match.dst, match.src, job, &jobs) } return jobs } diff --git a/fs/sync_internal_test.go b/fs/sync_internal_test.go new file mode 100644 index 000000000..2f8cf86d2 --- /dev/null +++ b/fs/sync_internal_test.go @@ -0,0 +1,129 @@ +// Internal tests for sync/copy/move + +package fs + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMatchListings(t *testing.T) { + var ( + a = mockObject("a") + b = mockObject("b") + c = mockObject("c") + d = mockObject("d") + ) + + for _, test := range []struct { + what string + input DirEntries // pairs of input src, dst + srcOnly DirEntries + dstOnly DirEntries + matches []matchPair // pairs of output + }{ + { + what: "only src or dst", + input: DirEntries{ + a, nil, + b, nil, + c, nil, + d, nil, + }, + srcOnly: DirEntries{ + a, b, c, d, + }, + }, + { + what: "typical sync #1", + input: DirEntries{ + a, nil, + b, b, + nil, c, + nil, d, + }, + srcOnly: DirEntries{ + a, + }, + dstOnly: DirEntries{ + c, d, + }, + matches: []matchPair{ + {b, b}, + }, + }, + { + what: "typical sync #2", + input: DirEntries{ + a, a, + b, b, + nil, c, + d, d, + }, + dstOnly: DirEntries{ + c, + }, + matches: []matchPair{ + {a, a}, + {b, b}, + {d, d}, + }, + }, + { + what: "One duplicate", + input: DirEntries{ + a, a, + a, nil, + }, + matches: []matchPair{ + {a, a}, + }, + }, + { + what: "Two duplicates", + input: DirEntries{ + a, a, + a, a, + a, nil, + }, + matches: []matchPair{ + {a, a}, + }, + }, + { + what: "Out of order", + input: DirEntries{ + c, nil, + b, b, + a, nil, + }, + srcOnly: DirEntries{ + c, + }, + dstOnly: DirEntries{ + b, + }, + }, + } { + var srcList, dstList DirEntries + for i := 0; i < len(test.input); i += 2 { + src, dst := test.input[i], test.input[i+1] + if src != nil { + srcList = append(srcList, src) + } + if dst != nil { + dstList = append(dstList, dst) + } + } + srcOnly, dstOnly, matches := matchListings(srcList, dstList) + assert.Equal(t, test.srcOnly, srcOnly, test.what) + assert.Equal(t, test.dstOnly, dstOnly, test.what) + assert.Equal(t, test.matches, matches, test.what) + // now swap src and dst + dstOnly, srcOnly, matches = matchListings(dstList, srcList) + assert.Equal(t, test.srcOnly, srcOnly, test.what) + assert.Equal(t, test.dstOnly, dstOnly, test.what) + assert.Equal(t, test.matches, matches, test.what) + } +}