Repository: syncthing/syncthing
File: lib/model/folder_sendrecv.go (Go)

Code before:
		// Check our list of files to be removed for a match, in which case
		// we can just do a rename instead.
		key := string(fi.BlocksHash)
		for candidate, ok := popCandidate(buckets, key); ok; candidate, ok = popCandidate(buckets, key) {
			// candidate is our current state of the file, where as the
			// desired state with the delete bit set is in the deletion
			// map.
			desired := fileDeletions[candidate.Name]
			if err := f.renameFile(candidate, desired, fi, snap, dbUpdateChan, scanChan); err != nil {
				l.Debugf("rename shortcut for %s failed: %s", fi.Name, err.Error())
				// Failed to rename, try next one.
				continue
			}

			// Remove the pending deletion (as we performed it by renaming)
			delete(fileDeletions, candidate.Name)

			f.queue.Done(fileName)
			continue nextFile
		}

		devices := snap.Availability(fileName)
		for _, dev := range devices {
			if f.model.ConnectedTo(dev) {
				// Check if the remote folder state is valid on connected device (i.e. not paused)
				if state, ok := f.model.remoteFolderStates[dev][f.folderID]; ok {
					if state == remoteFolderValid {
					// Handle the file normally, by copying and pulling, etc.
					f.handleFile(fi, snap, copyChan)
					continue nextFile
					}
				}
			}
		}
		f.newPullError(fileName, errNotAvailable)
		f.queue.Done(fileName)
	}
		
	return changed, fileDeletions, dirDeletions, nil
}

func popCandidate(buckets map[string][]protocol.FileInfo, key string) (protocol.FileInfo, bool) {
	cands := buckets[key]
	if len(cands) == 0 {
		return protocol.FileInfo{}, false
	}

	buckets[key] = cands[1:]
	return cands[0], true
}


Review comment (refactor):
You cannot access `remoteFolderStates` without holding a read lock on the mutex in model. And it feels a bit messy to do that from here. I'd create a utility function in model for this. Respectively it looks like it almost already exists `model.availabilityInSnapshotRLocked` - just that it's for a block instead of the entire file. You could split that into two functions, one for the part with the snapshot, one with the devicedownloads. Then use the snapshot function here. Ok that explanations sounds unintelligible on re-reading, maybe some pseudo-code is clearer:

Split functions in model, and new "wrapper" function (also need to adapt callers to the split functions, leaving that out):

```
func (m *model) availabilityInSnapshot(cfg config.FolderConfiguration, snap *db.Snapshot, file protocol.FileInfo) []Availability {
	m.mut.RLock()
	defer m.mut.RUnlock()
	return m.availabilityInSnapshotRLocked(...)
}

func (m *model) availabilityInSnapshotRLocked(cfg config.FolderConfiguration, snap *db.Snapshot, file protocol.FileInfo) []Availability {
	var availabilities []Availability
	for _, device := range snap.Availability(file.Name) {
...
	return availabilities
}

func (m *model) availabilityFromTemporaryRLocked(cfg config.FolderConfiguration, file protocol.FileInfo, block protocol.BlockInfo) []Availability {
	var availabilities []Availability
	for _, device := range cfg.Devices {
...
	return availabilities
}
```

And then here just:

```
if devices := f.model.availabilityInSnapshot(...); len(devices) > 0 {
f.handleFile(...)
}
```

Does that make any sense?

Code after:
		// Check our list of files to be removed for a match, in which case
		// we can just do a rename instead.
		key := string(fi.BlocksHash)
		for candidate, ok := popCandidate(buckets, key); ok; candidate, ok = popCandidate(buckets, key) {
			// candidate is our current state of the file, where as the
			// desired state with the delete bit set is in the deletion
			// map.
			desired := fileDeletions[candidate.Name]
			if err := f.renameFile(candidate, desired, fi, snap, dbUpdateChan, scanChan); err != nil {
				l.Debugf("rename shortcut for %s failed: %s", fi.Name, err.Error())
				// Failed to rename, try next one.
				continue
			}

			// Remove the pending deletion (as we performed it by renaming)
			delete(fileDeletions, candidate.Name)

			f.queue.Done(fileName)
			continue nextFile
		}

		devices := f.model.fileAvailability(f.FolderConfiguration, snap, fi)
		if len(devices) > 0 {
			f.handleFile(fi, snap, copyChan)
			continue
		}
		f.newPullError(fileName, errNotAvailable)
		f.queue.Done(fileName)
	}

	return changed, fileDeletions, dirDeletions, nil
}

func popCandidate(buckets map[string][]protocol.FileInfo, key string) (protocol.FileInfo, bool) {
	cands := buckets[key]
	if len(cands) == 0 {
		return protocol.FileInfo{}, false
	}

	buckets[key] = cands[1:]
	return cands[0], true
}

func (f *sendReceiveFolder) processDeletions(fileDeletions map[string]protocol.FileInfo, dirDeletions []protocol.FileInfo, snap *db.Snapshot, dbUpdateChan chan<- dbUpdateJob, scanChan chan<- string) {
	for _, file := range fileDeletions {
		select {
		case <-f.ctx.Done():
			return
		default:
		}