Add tests for combineDeltas

src/github.com/matrix-org/dendrite/federationsender/consumers/roomserver.go

@@ -91,9 +91,7 @@ func (s *OutputRoomEvent) onMessage(msg *sarama.ConsumerMessage) error {
 		"send_as_server": output.SendAsServer,
 	}).Info("received event from roomserver")
 
-	err = s.processMessage(output, ev)
+	if err = s.processMessage(output, ev); err != nil {
-
-	if err != nil {
 		// panic rather than continue with an inconsistent database
 		log.WithFields(log.Fields{
 			"event":      string(ev.JSON()),
@@ -107,6 +105,8 @@ func (s *OutputRoomEvent) onMessage(msg *sarama.ConsumerMessage) error {
 	return nil
 }
 
+// processMessage updates the list of currently joined hosts in the room
+// and then sends the event to the hosts that were joined before the event.
 func (s *OutputRoomEvent) processMessage(ore api.OutputRoomEvent, ev gomatrixserverlib.Event) error {
 	addsStateEvents, err := s.lookupStateEvents(ore.AddsStateEventIDs, ev)
 	if err != nil {
@@ -240,39 +240,40 @@ func joinedHostsFromEvents(evs []gomatrixserverlib.Event) ([]types.JoinedHost, e
 }
 
 // combineDeltas combines two deltas into a single delta.
+// Assumes that the order of operations is add(1), remove(1), add(2), remove(2).
+// Removes duplicate entries and redundant operations from each delta.
 func combineDeltas(adds1, removes1, adds2, removes2 []string) (adds, removes []string) {
 	addSet := map[string]bool{}
 	removeSet := map[string]bool{}
-	var ok bool
+
-	for _, value := range adds1 {
+	// combine processes each unique value in a list.
-		addSet[value] = true
+	// If the value is in the removeFrom set then it is removed from that set.
-	}
+	// Otherwise it is added to the addTo set.
-	for _, value := range removes1 {
+	combine := func(values []string, removeFrom, addTo map[string]bool) {
-		removeSet[value] = true
+		processed := map[string]bool{}
-	}
+		for _, value := range values {
-	for _, value := range adds2 {
+			if processed[value] {
-		if _, ok = removeSet[value]; ok {
+				continue
-			removeSet[value] = false
+			}
-		} else {
+			processed[value] = true
-			addSet[value] = true
+			if removeFrom[value] {
+				delete(removeFrom, value)
+			} else {
+				addTo[value] = true
+			}
 		}
 	}
-	for _, value := range removes2 {
+
-		if _, ok = addSet[value]; ok {
+	combine(adds1, nil, addSet)
-			addSet[value] = false
+	combine(removes1, addSet, removeSet)
-		} else {
+	combine(adds2, removeSet, addSet)
-			removeSet[value] = true
+	combine(removes2, addSet, removeSet)
-		}
+
+	for value := range addSet {
+		adds = append(adds, value)
 	}
-	for value, include := range addSet {
+	for value := range removeSet {
-		if include {
+		removes = append(removes, value)
-			adds = append(adds, value)
-		}
-	}
-	for value, include := range removeSet {
-		if include {
-			removes = append(removes, value)
-		}
 	}
 	return
 }

src/github.com/matrix-org/dendrite/federationsender/consumers/roomserver_test.go

@@ -0,0 +1,54 @@
+// Copyright 2017 Vector Creations Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package consumers
+
+import (
+	"testing"
+)
+
+func TestCombineNoOp(t *testing.T) {
+	inputAdd1 := []string{"a", "b", "c"}
+	inputDel1 := []string{"a", "b", "d"}
+	inputAdd2 := []string{"a", "d", "e"}
+	inputDel2 := []string{"a", "c", "e", "e"}
+
+	gotAdd, gotDel := combineDeltas(inputAdd1, inputDel1, inputAdd2, inputDel2)
+
+	if len(gotAdd) != 0 {
+		t.Errorf("wanted combined adds to be an empty list, got %#v", gotAdd)
+	}
+
+	if len(gotDel) != 0 {
+		t.Errorf("wanted combined removes to be an empty list, got %#v", gotDel)
+	}
+}
+
+func TestCombineDedup(t *testing.T) {
+	inputAdd1 := []string{"a", "a"}
+	inputDel1 := []string{"b", "b"}
+	inputAdd2 := []string{"a", "a"}
+	inputDel2 := []string{"b", "b"}
+
+	gotAdd, gotDel := combineDeltas(inputAdd1, inputDel1, inputAdd2, inputDel2)
+
+	if len(gotAdd) != 1 || gotAdd[0] != "a" {
+		t.Errorf("wanted combined adds to be %#v, got %#v", []string{"a"}, gotAdd)
+	}
+
+	if len(gotDel) != 1 || gotDel[0] != "b" {
+		t.Errorf("wanted combined removes to be %#v, got %#v", []string{"b"}, gotDel)
+	}
+
+}

src/github.com/matrix-org/dendrite/federationsender/types/types.go

@@ -16,14 +16,15 @@ package types
 
 import (
 	"fmt"
+
 	"github.com/matrix-org/gomatrixserverlib"
 )
 
 // A JoinedHost is a server that is joined to a matrix room.
 type JoinedHost struct {
-	// THe EventID of a m.room.member event that joins a server to a room.
+	// The EventID of a m.room.member join event.
 	EventID string
-	// The
+	// The domain part of the state key of the m.room.member join event
 	ServerName gomatrixserverlib.ServerName
 }