lib/model: Properly handle deleting multiple files when doing scans with subs (fixes syncthing#2851)

calmh · AudriusButkevicius · commit aba2cc4db2e8 · 2016-03-18T12:16:33.000Z
diff --git a/lib/db/leveldb_dbinstance.go b/lib/db/leveldb_dbinstance.go
@@ -259,11 +259,11 @@ func (db *Instance) updateFiles(folder, device []byte, fs []protocol.FileInfo, l
 	return maxLocalVer
 }
 
-func (db *Instance) withHave(folder, device []byte, truncate bool, fn Iterator) {
+func (db *Instance) withHave(folder, device, prefix []byte, truncate bool, fn Iterator) {
 	t := db.newReadOnlyTransaction()
 	defer t.close()
 
-	dbi := t.NewIterator(util.BytesPrefix(db.deviceKey(folder, device, nil)[:keyPrefixLen+keyFolderLen+keyDeviceLen]), nil)
+	dbi := t.NewIterator(util.BytesPrefix(db.deviceKey(folder, device, prefix)[:keyPrefixLen+keyFolderLen+keyDeviceLen+len(prefix)]), nil)
 	defer dbi.Release()
 
 	for dbi.Next() {
diff --git a/lib/db/set.go b/lib/db/set.go
@@ -171,14 +171,18 @@ func (s *FileSet) WithNeedTruncated(device protocol.DeviceID, fn Iterator) {
 
 func (s *FileSet) WithHave(device protocol.DeviceID, fn Iterator) {
 	l.Debugf("%s WithHave(%v)", s.folder, device)
-	s.db.withHave([]byte(s.folder), device[:], false, nativeFileIterator(fn))
+	s.db.withHave([]byte(s.folder), device[:], nil, false, nativeFileIterator(fn))
 }
 
 func (s *FileSet) WithHaveTruncated(device protocol.DeviceID, fn Iterator) {
 	l.Debugf("%s WithHaveTruncated(%v)", s.folder, device)
-	s.db.withHave([]byte(s.folder), device[:], true, nativeFileIterator(fn))
+	s.db.withHave([]byte(s.folder), device[:], nil, true, nativeFileIterator(fn))
 }
 
+func (s *FileSet) WithPrefixedHaveTruncated(device protocol.DeviceID, prefix string, fn Iterator) {
+	l.Debugf("%s WithPrefixedHaveTruncated(%v)", s.folder, device)
+	s.db.withHave([]byte(s.folder), device[:], []byte(prefix), true, nativeFileIterator(fn))
+}
 func (s *FileSet) WithGlobal(fn Iterator) {
 	l.Debugf("%s WithGlobal()", s.folder)
 	s.db.withGlobal([]byte(s.folder), nil, false, nativeFileIterator(fn))
diff --git a/lib/model/model.go b/lib/model/model.go
@@ -1392,76 +1392,69 @@ func (m *Model) internalScanFolderSubs(folder string, subs []string) error {
 		m.updateLocals(folder, batch)
 	}
 
-	batch = batch[:0]
-	// TODO: We should limit the Have scanning to start at sub
-	seenPrefix := false
-	var iterError error
-	fs.WithHaveTruncated(protocol.LocalDeviceID, func(fi db.FileIntf) bool {
-		f := fi.(db.FileInfoTruncated)
-		hasPrefix := len(subs) == 0
-		for _, sub := range subs {
-			if strings.HasPrefix(f.Name, sub) {
-				hasPrefix = true
-				break
-			}
-		}
-		// Return true so that we keep iterating, until we get to the part
-		// of the tree we are interested in. Then return false so we stop
-		// iterating when we've passed the end of the subtree.
-		if !hasPrefix {
-			return !seenPrefix
-		}
+	if len(subs) == 0 {
+		// If we have no specific subdirectories to traverse, set it to one
+		// empty prefix so we traverse the entire folder contents once.
+		subs = []string{""}
+	}
 
-		seenPrefix = true
-		if !f.IsDeleted() {
-			if f.IsInvalid() {
-				return true
-			}
+	// Do a scan of the database for each prefix, to check for deleted files.
+	batch = batch[:0]
+	for _, sub := range subs {
+		var iterError error
 
-			if len(batch) == batchSizeFiles {
-				if err := m.CheckFolderHealth(folder); err != nil {
-					iterError = err
-					return false
+		fs.WithPrefixedHaveTruncated(protocol.LocalDeviceID, sub, func(fi db.FileIntf) bool {
+			f := fi.(db.FileInfoTruncated)
+			if !f.IsDeleted() {
+				if f.IsInvalid() {
+					return true
 				}
-				m.updateLocals(folder, batch)
-				batch = batch[:0]
-			}
 
-			if ignores.Match(f.Name) || symlinkInvalid(folder, f) {
-				// File has been ignored or an unsupported symlink. Set invalid bit.
-				l.Debugln("setting invalid bit on ignored", f)
-				nf := protocol.FileInfo{
-					Name:     f.Name,
-					Flags:    f.Flags | protocol.FlagInvalid,
-					Modified: f.Modified,
-					Version:  f.Version, // The file is still the same, so don't bump version
+				if len(batch) == batchSizeFiles {
+					if err := m.CheckFolderHealth(folder); err != nil {
+						iterError = err
+						return false
+					}
+					m.updateLocals(folder, batch)
+					batch = batch[:0]
 				}
-				batch = append(batch, nf)
-			} else if _, err := osutil.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil {
-				// File has been deleted.
-
-				// We don't specifically verify that the error is
-				// os.IsNotExist because there is a corner case when a
-				// directory is suddenly transformed into a file. When that
-				// happens, files that were in the directory (that is now a
-				// file) are deleted but will return a confusing error ("not a
-				// directory") when we try to Lstat() them.
-
-				nf := protocol.FileInfo{
-					Name:     f.Name,
-					Flags:    f.Flags | protocol.FlagDeleted,
-					Modified: f.Modified,
-					Version:  f.Version.Update(m.shortID),
+
+				if ignores.Match(f.Name) || symlinkInvalid(folder, f) {
+					// File has been ignored or an unsupported symlink. Set invalid bit.
+					l.Debugln("setting invalid bit on ignored", f)
+					nf := protocol.FileInfo{
+						Name:     f.Name,
+						Flags:    f.Flags | protocol.FlagInvalid,
+						Modified: f.Modified,
+						Version:  f.Version, // The file is still the same, so don't bump version
+					}
+					batch = append(batch, nf)
+				} else if _, err := osutil.Lstat(filepath.Join(folderCfg.Path(), f.Name)); err != nil {
+					// File has been deleted.
+
+					// We don't specifically verify that the error is
+					// os.IsNotExist because there is a corner case when a
+					// directory is suddenly transformed into a file. When that
+					// happens, files that were in the directory (that is now a
+					// file) are deleted but will return a confusing error ("not a
+					// directory") when we try to Lstat() them.
+
+					nf := protocol.FileInfo{
+						Name:     f.Name,
+						Flags:    f.Flags | protocol.FlagDeleted,
+						Modified: f.Modified,
+						Version:  f.Version.Update(m.shortID),
+					}
+					batch = append(batch, nf)
 				}
-				batch = append(batch, nf)
 			}
-		}
-		return true
-	})
+			return true
+		})
 
-	if iterError != nil {
-		l.Infof("Stopping folder %s mid-scan due to folder error: %s", folder, iterError)
-		return iterError
+		if iterError != nil {
+			l.Infof("Stopping folder %s mid-scan due to folder error: %s", folder, iterError)
+			return iterError
+		}
 	}
 
 	if err := m.CheckFolderHealth(folder); err != nil {
