lib/db: Filter repeat files in one update (ref #6650) (#6651)

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)
+			}
+		}
+	}
+}

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 {

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)