lib/model: Scan conflicts after creation (syncthing#5511)

imsodin · calmh · commit 225c0dda805f · 2019-02-12T16:05:20.000+01:00
Also unflakes and improve TestRequestRemoteRenameChanged.
diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go
@@ -196,7 +196,7 @@ func (f *sendReceiveFolder) pull() bool {
 
 		changed := f.pullerIteration(ignores, folderFiles, scanChan)
 
-		l.Debugln(f, "changed", changed, "on try", tries)
+		l.Debugln(f, "changed", changed, "on try", tries+1)
 
 		if changed == 0 {
 			// No files were changed by the puller, so we are in
@@ -795,7 +795,7 @@ func (f *sendReceiveFolder) deleteFile(file protocol.FileInfo, scanChan chan<- s
 		// we have resolved the conflict.
 		file.Version = file.Version.Merge(cur.Version)
 		err = osutil.InWritableDir(func(name string) error {
-			return f.moveForConflict(name, file.ModifiedBy.String())
+			return f.moveForConflict(name, file.ModifiedBy.String(), scanChan)
 		}, f.fs, file.Name)
 	} else if f.versioner != nil && !cur.IsSymlink() {
 		err = osutil.InWritableDir(f.versioner.Archive, f.fs, file.Name)
@@ -1512,7 +1512,7 @@ func (f *sendReceiveFolder) performFinish(ignores *ignore.Matcher, file, curFile
 
 			file.Version = file.Version.Merge(curFile.Version)
 			err = osutil.InWritableDir(func(name string) error {
-				return f.moveForConflict(name, file.ModifiedBy.String())
+				return f.moveForConflict(name, file.ModifiedBy.String(), scanChan)
 			}, f.fs, file.Name)
 			if err != nil {
 				return err
@@ -1736,7 +1736,7 @@ func removeAvailability(availabilities []Availability, availability Availability
 	return availabilities
 }
 
-func (f *sendReceiveFolder) moveForConflict(name string, lastModBy string) error {
+func (f *sendReceiveFolder) moveForConflict(name string, lastModBy string, scanChan chan<- string) error {
 	if strings.Contains(filepath.Base(name), ".sync-conflict-") {
 		l.Infoln("Conflict for", name, "which is already a conflict copy; not copying again.")
 		if err := f.fs.Remove(name); err != nil && !fs.IsNotExist(err) {
@@ -1777,6 +1777,9 @@ func (f *sendReceiveFolder) moveForConflict(name string, lastModBy string) error
 			l.Debugln(f, "globbing for conflicts", gerr)
 		}
 	}
+	if err == nil {
+		scanChan <- newName
+	}
 	return err
 }
 
diff --git a/lib/model/requests_test.go b/lib/model/requests_test.go
@@ -792,17 +792,6 @@ func TestRequestRemoteRenameChanged(t *testing.T) {
 	fc.sendIndexUpdate()
 	select {
 	case <-done:
-		done = make(chan struct{})
-		fc.mut.Lock()
-		fc.indexFn = func(folder string, fs []protocol.FileInfo) {
-			select {
-			case <-done:
-				t.Fatalf("More than one index update sent")
-			default:
-			}
-			close(done)
-		}
-		fc.mut.Unlock()
 	case <-time.After(10 * time.Second):
 		t.Fatal("timed out")
 	}
@@ -813,6 +802,42 @@ func TestRequestRemoteRenameChanged(t *testing.T) {
 		}
 	}
 
+	var gotA, gotB, gotConfl bool
+	done = make(chan struct{})
+	fc.mut.Lock()
+	fc.indexFn = func(folder string, fs []protocol.FileInfo) {
+		select {
+		case <-done:
+			t.Fatalf("Received more index updates than expected")
+		default:
+		}
+		for _, f := range fs {
+			switch {
+			case f.Name == a:
+				if gotA {
+					t.Error("Got more than one index update for", f.Name)
+				}
+				gotA = true
+			case f.Name == b:
+				if gotB {
+					t.Error("Got more than one index update for", f.Name)
+				}
+				gotB = true
+			case strings.HasPrefix(f.Name, "b.sync-conflict-"):
+				if gotConfl {
+					t.Error("Got more than one index update for conflicts of", f.Name)
+				}
+				gotConfl = true
+			default:
+				t.Error("Got unexpected file in index update", f.Name)
+			}
+		}
+		if gotA && gotB && gotConfl {
+			close(done)
+		}
+	}
+	fc.mut.Unlock()
+
 	fd, err := tfs.OpenFile(b, fs.OptReadWrite, 0644)
 	if err != nil {
 		t.Fatal(err)
@@ -826,11 +851,20 @@ func TestRequestRemoteRenameChanged(t *testing.T) {
 	// rename
 	fc.deleteFile(a)
 	fc.updateFile(b, 0644, protocol.FileInfoTypeFile, data[a])
+	// Make sure the remote file for b is newer and thus stays global -> local conflict
+	fc.mut.Lock()
+	for i := range fc.files {
+		if fc.files[i].Name == b {
+			fc.files[i].ModifiedS += 100
+			break
+		}
+	}
+	fc.mut.Unlock()
 	fc.sendIndexUpdate()
 	select {
 	case <-done:
 	case <-time.After(10 * time.Second):
-		t.Fatal("timed out")
+		t.Errorf("timed out without receiving all expected index updates")
 	}
 
 	// Check outcome
@@ -839,9 +873,16 @@ func TestRequestRemoteRenameChanged(t *testing.T) {
 		case path == a:
 			t.Errorf(`File "a" was not removed`)
 		case path == b:
-			if err := equalContents(filepath.Join(tmpDir, b), otherData); err != nil {
-				t.Errorf(`Modified file "b" was overwritten`)
+			if err := equalContents(filepath.Join(tmpDir, b), data[a]); err != nil {
+				t.Error(`File "b" has unexpected content (renamed from a on remote)`)
+			}
+		case strings.HasPrefix(path, b+".sync-conflict-"):
+			if err := equalContents(filepath.Join(tmpDir, path), otherData); err != nil {
+				t.Error(`Sync conflict of "b" has unexptected content`)
 			}
+		case path == "." || strings.HasPrefix(path, ".stfolder"):
+		default:
+			t.Error("Found unexpected file", path)
 		}
 		return nil
 	})
