fix: use ModifyIntervals for atomic conflict resolution

sync/conflict.go

@@ -35,7 +35,7 @@ func SolveConflict(userId int64, store storage.Storage) (bool, error) {
 	var removed []data.Interval
 	var added []data.Interval
 	if err != nil {
-		return false, fmt.Errorf("Unable to retrieve User Data for UserId %v from Storage:\n%v", userId, err)
+		return false, fmt.Errorf("getting intervals for user %d: %w", userId, err)
 	}
 
 	// Sort intervals by ascending start time (in place)
@@ -151,25 +151,64 @@ func SolveConflict(userId int64, store storage.Storage) (bool, error) {
 		}
 	}
 
-	// Transfer solved conflict state to storage
-	for _, a := range added {
-		err = store.AddInterval(storage.UserId(userId), a)
-		if err != nil {
-			return conflictDetected, fmt.Errorf("Unable to change User Data for UserId %v in Storage:\n%v",
-				userId, err)
-		}
+	if len(added) == 0 && len(removed) == 0 {
+		return conflictDetected, nil
 	}
-	for _, r := range removed {
-		err = store.RemoveInterval(storage.UserId(userId), r)
-		if err != nil {
-			return conflictDetected, fmt.Errorf("Unable to change User Data for UserId %v in Storage:\n%v",
-				userId, err)
-		}
+
+	netAdd, netDel := computeNetDiff(added, removed)
+
+	if err := store.ModifyIntervals(storage.UserId(userId), netAdd, netDel); err != nil {
+		return conflictDetected, fmt.Errorf("modifying intervals for user %d: %w", userId, err)
 	}
 
 	return conflictDetected, nil
 }
 
+func computeNetDiff(added []data.Interval, removed []data.Interval) ([]data.Interval, []data.Interval) {
+	addedKeys := storage.ConvertToKeys(added)
+	removedKeys := storage.ConvertToKeys(removed)
+
+	addedSet := make(map[storage.IntervalKey]int, len(addedKeys))
+	for _, key := range addedKeys {
+		addedSet[key]++
+	}
+
+	removedSet := make(map[storage.IntervalKey]int, len(removedKeys))
+	for _, key := range removedKeys {
+		removedSet[key]++
+	}
+
+	for key, rc := range removedSet {
+		if ac, ok := addedSet[key]; ok {
+			common := min(ac, rc)
+			addedSet[key] -= common
+			if addedSet[key] == 0 {
+				delete(addedSet, key)
+			}
+			removedSet[key] -= common
+			if removedSet[key] == 0 {
+				delete(removedSet, key)
+			}
+		}
+	}
+
+	netAdd := filterBySet(added, addedKeys, addedSet)
+	netDel := filterBySet(removed, removedKeys, removedSet)
+
+	return netAdd, netDel
+}
+
+func filterBySet(intervals []data.Interval, keys []storage.IntervalKey, set map[storage.IntervalKey]int) []data.Interval {
+	var result []data.Interval
+	for i, key := range keys {
+		if set[key] > 0 {
+			result = append(result, intervals[i])
+			set[key]--
+		}
+	}
+	return result
+}
+
 // UniteTagsAndAnnotation computes the new tags and annotation for overlapping intervals and returns tags, annotation.
 // Case 1: Iff only one interval has an Annotation, we use this annotation. Case 2: Iff no interval has an annotation,
 // we use "" as annotation. Case 3: Iff both intervals have different annotation, we use "" as annotation, and add both

sync/conflict_test.go

@@ -47,6 +47,7 @@ func elementwiseEqual(aSlice []data.Interval, bSlice []data.Interval) bool {
 	}
 	return true
 }
+
 func sliceString(s []data.Interval) {
 	print("[\n")
 	for _, i := range s {
@@ -566,6 +567,106 @@ func TestSolveConflict_NoIntervals(t *testing.T) {
 	}
 }
 
+type recordingStorage struct {
+	storage.Ephemeral
+	addCalls    int
+	removeCalls int
+	modifyCalls int
+}
+
+func (r *recordingStorage) AddInterval(userId storage.UserId, interval data.Interval) error {
+	r.addCalls++
+	return r.Ephemeral.AddInterval(userId, interval)
+}
+
+func (r *recordingStorage) RemoveInterval(userId storage.UserId, interval data.Interval) error {
+	r.removeCalls++
+	return r.Ephemeral.RemoveInterval(userId, interval)
+}
+
+func (r *recordingStorage) ModifyIntervals(userId storage.UserId, add []data.Interval, del []data.Interval) error {
+	r.modifyCalls++
+	return r.Ephemeral.ModifyIntervals(userId, add, del)
+}
+
+func TestSolveConflict_UsesModifyIntervals(t *testing.T) {
+	store := &recordingStorage{}
+	if err := store.Initialize(); err != nil {
+		t.Fatal(err)
+	}
+
+	intervals := []data.Interval{
+		{
+			Start:      time.Date(2000, 5, 11, 12, 30, 40, 0, time.UTC),
+			End:        time.Date(2000, 5, 11, 18, 30, 40, 0, time.UTC),
+			Tags:       []string{"a"},
+			Annotation: "x",
+		},
+		{
+			Start:      time.Date(2000, 5, 11, 14, 30, 40, 0, time.UTC),
+			End:        time.Date(2000, 5, 11, 21, 30, 40, 0, time.UTC),
+			Tags:       []string{"b"},
+			Annotation: "y",
+		},
+	}
+	if err := store.SetIntervals(storage.UserId(0), intervals); err != nil {
+		t.Fatal(err)
+	}
+
+	conflict, err := SolveConflict(0, store)
+	if err != nil {
+		t.Fatalf("SolveConflict returned error: %v", err)
+	}
+	if !conflict {
+		t.Fatal("expected conflict to be detected")
+	}
+	if store.modifyCalls != 1 {
+		t.Errorf("expected exactly 1 ModifyIntervals call, got %d", store.modifyCalls)
+	}
+	if store.addCalls != 0 {
+		t.Errorf("expected 0 AddInterval calls, got %d", store.addCalls)
+	}
+	if store.removeCalls != 0 {
+		t.Errorf("expected 0 RemoveInterval calls, got %d", store.removeCalls)
+	}
+}
+
+func TestSolveConflict_NoConflict_NoModifyIntervals(t *testing.T) {
+	store := &recordingStorage{}
+	if err := store.Initialize(); err != nil {
+		t.Fatal(err)
+	}
+
+	intervals := []data.Interval{
+		{
+			Start:      time.Date(2000, 5, 10, 12, 0, 0, 0, time.UTC),
+			End:        time.Date(2000, 5, 10, 13, 0, 0, 0, time.UTC),
+			Tags:       []string{"a"},
+			Annotation: "",
+		},
+		{
+			Start:      time.Date(2000, 5, 10, 14, 0, 0, 0, time.UTC),
+			End:        time.Date(2000, 5, 10, 15, 0, 0, 0, time.UTC),
+			Tags:       []string{"b"},
+			Annotation: "",
+		},
+	}
+	if err := store.SetIntervals(storage.UserId(0), intervals); err != nil {
+		t.Fatal(err)
+	}
+
+	conflict, err := SolveConflict(0, store)
+	if err != nil {
+		t.Fatalf("SolveConflict returned error: %v", err)
+	}
+	if conflict {
+		t.Fatal("expected no conflict")
+	}
+	if store.modifyCalls != 0 {
+		t.Errorf("expected 0 ModifyIntervals calls when no conflict, got %d", store.modifyCalls)
+	}
+}
+
 func TestUniteTagsAndAnnotation_AllEmpty(t *testing.T) {
 	a := data.Interval{
 		Start:      time.Time{},