diff --git a/lib/db/db_test.go b/lib/db/db_test.go index 8b7b8df83..6263800ff 100644 --- a/lib/db/db_test.go +++ b/lib/db/db_test.go @@ -553,3 +553,44 @@ func TestUpdateTo10(t *testing.T) { t.Error("vl.Versions[1] not deleted for c") } } + +func TestDropDuplicates(t *testing.T) { + names := []string{ + "foo", + "bar", + "dcxvoijnds", + "3d/dsfase/4/ss2", + } + tcs := []struct{ in, out []int }{ + {[]int{0}, []int{0}}, + {[]int{0, 1}, []int{0, 1}}, + {[]int{0, 1, 0, 1}, []int{0, 1}}, + {[]int{0, 1, 1, 1, 1}, []int{0, 1}}, + {[]int{0, 0, 0, 1}, []int{0, 1}}, + {[]int{0, 1, 2, 3}, []int{0, 1, 2, 3}}, + {[]int{3, 2, 1, 0, 0, 1, 2, 3}, []int{0, 1, 2, 3}}, + {[]int{0, 1, 1, 3, 0, 1, 0, 1, 2, 3}, []int{0, 1, 2, 3}}, + } + + for tci, tc := range tcs { + inp := make([]protocol.FileInfo, len(tc.in)) + expSeq := make(map[string]int) + for i, j := range tc.in { + inp[i] = protocol.FileInfo{Name: names[j], Sequence: int64(i)} + expSeq[names[j]] = i + } + outp := normalizeFilenamesAndDropDuplicates(inp) + if len(outp) != len(tc.out) { + t.Errorf("tc %v: Expected %v entries, got %v", tci, len(tc.out), len(outp)) + continue + } + for i, f := range outp { + if exp := names[tc.out[i]]; exp != f.Name { + t.Errorf("tc %v: Got file %v at pos %v, expected %v", tci, f.Name, i, exp) + } + if exp := int64(expSeq[outp[i].Name]); exp != f.Sequence { + t.Errorf("tc %v: Got sequence %v at pos %v, expected %v", tci, f.Sequence, i, exp) + } + } + } +} diff --git a/lib/db/set.go b/lib/db/set.go index 365a9ecf6..e29c64f59 100644 --- a/lib/db/set.go +++ b/lib/db/set.go @@ -124,7 +124,13 @@ func (s *FileSet) Update(device protocol.DeviceID, fs []protocol.FileInfo) { // do not modify fs in place, it is still used in outer scope fs = append([]protocol.FileInfo(nil), fs...) - normalizeFilenames(fs) + // If one file info is present multiple times, only keep the last. + // Updating the same file multiple times is problematic, because the + // previous updates won't yet be represented in the db when we update it + // again. Additionally even if that problem was taken care of, it would + // be pointless because we remove the previously added file info again + // right away. + fs = normalizeFilenamesAndDropDuplicates(fs) s.updateMutex.Lock() defer s.updateMutex.Unlock() @@ -470,10 +476,24 @@ func DropDeltaIndexIDs(db *Lowlevel) { } } -func normalizeFilenames(fs []protocol.FileInfo) { - for i := range fs { - fs[i].Name = osutil.NormalizedFilename(fs[i].Name) +func normalizeFilenamesAndDropDuplicates(fs []protocol.FileInfo) []protocol.FileInfo { + positions := make(map[string]int, len(fs)) + for i, f := range fs { + norm := osutil.NormalizedFilename(f.Name) + if pos, ok := positions[norm]; ok { + fs[pos] = protocol.FileInfo{} + } + positions[norm] = i + fs[i].Name = norm } + for i := 0; i < len(fs); { + if fs[i].Name == "" { + fs = append(fs[:i], fs[i+1:]...) + continue + } + i++ + } + return fs } func nativeFileIterator(fn Iterator) Iterator { diff --git a/lib/db/set_test.go b/lib/db/set_test.go index ae7f22d00..5a475b150 100644 --- a/lib/db/set_test.go +++ b/lib/db/set_test.go @@ -1615,6 +1615,44 @@ func TestIgnoreAfterReceiveOnly(t *testing.T) { } } +// https://github.com/syncthing/syncthing/issues/6650 +func TestUpdateWithOneFileTwice(t *testing.T) { + ldb := db.NewLowlevel(backend.OpenMemory()) + defer ldb.Close() + + file := "foo" + s := db.NewFileSet("test", fs.NewFilesystem(fs.FilesystemTypeFake, ""), ldb) + + fs := fileList{{ + Name: file, + Version: protocol.Vector{Counters: []protocol.Counter{{ID: myID, Value: 1}}}, + Sequence: 1, + }} + + s.Update(protocol.LocalDeviceID, fs) + + fs = append(fs, fs[0]) + for i := range fs { + fs[i].Sequence++ + fs[i].Version = fs[i].Version.Update(myID) + } + fs[1].Sequence++ + fs[1].Version = fs[1].Version.Update(myID) + + s.Update(protocol.LocalDeviceID, fs) + + snap := s.Snapshot() + defer snap.Release() + count := 0 + snap.WithHaveSequence(0, func(f db.FileIntf) bool { + count++ + return true + }) + if count != 1 { + t.Error("Expected to have one file, got", count) + } +} + func replace(fs *db.FileSet, device protocol.DeviceID, files []protocol.FileInfo) { fs.Drop(device) fs.Update(device, files)