From ffabd266534c715eb1e64878f883a7f1a159f9f2 Mon Sep 17 00:00:00 2001
From: Daniel Oaks <daniel@danieloaks.net>
Date: Sun, 30 Jul 2017 22:42:37 +1000
Subject: [PATCH] Make sure ISUPPORT outputs are sorted and tested

---
 irc/isupport.go      | 45 +++++++++++++++++++++++--------------------
 irc/isupport_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 21 deletions(-)
 create mode 100644 irc/isupport_test.go

diff --git a/irc/isupport.go b/irc/isupport.go
index 77dc0b15..17188c32 100644
--- a/irc/isupport.go
+++ b/irc/isupport.go
@@ -4,6 +4,7 @@
 package irc
 
 import "fmt"
+import "sort"
 
 const isupportSupportedString = "are supported by this server"
 
@@ -41,9 +42,7 @@ func getTokenString(name string, value *string) string {
 
 // GetDifference returns the difference between two token lists.
 func (il *ISupportList) GetDifference(newil *ISupportList) [][]string {
-	replies := make([][]string, 0)
-	var length int     // Length of the current cache
-	var cache []string // Token list cache
+	var outTokens sort.StringSlice
 
 	// append removed tokens
 	for name := range il.Tokens {
@@ -54,32 +53,29 @@ func (il *ISupportList) GetDifference(newil *ISupportList) [][]string {
 
 		token := fmt.Sprintf("-%s", name)
 
-		if len(token)+length <= maxLastArgLength {
-			// account for the space separating tokens
-			if len(cache) > 0 {
-				length++
-			}
-			cache = append(cache, token)
-			length += len(token)
-		}
-
-		if len(cache) == 13 || len(token)+length >= maxLastArgLength {
-			cache = append(cache, isupportSupportedString)
-			replies = append(replies, cache)
-			cache = make([]string, 0)
-			length = 0
-		}
+		outTokens = append(outTokens, token)
 	}
 
 	// append added tokens
 	for name, value := range newil.Tokens {
 		newval, exists := il.Tokens[name]
-		if exists && *value == *newval {
+		if exists && ((value == nil && newval == nil) || (value != nil && newval != nil && *value == *newval)) {
 			continue
 		}
 
 		token := getTokenString(name, value)
 
+		outTokens = append(outTokens, token)
+	}
+
+	sort.Sort(outTokens)
+
+	// create output list
+	replies := make([][]string, 0)
+	var length int     // Length of the current cache
+	var cache []string // Token list cache
+
+	for _, token := range outTokens {
 		if len(token)+length <= maxLastArgLength {
 			// account for the space separating tokens
 			if len(cache) > 0 {
@@ -111,8 +107,15 @@ func (il *ISupportList) RegenerateCachedReply() {
 	var length int     // Length of the current cache
 	var cache []string // Token list cache
 
-	for name, value := range il.Tokens {
-		token := getTokenString(name, value)
+	// make sure we get a sorted list of tokens, needed for tests and looks nice
+	var tokens sort.StringSlice
+	for name := range il.Tokens {
+		tokens = append(tokens, name)
+	}
+	sort.Sort(tokens)
+
+	for _, name := range tokens {
+		token := getTokenString(name, il.Tokens[name])
 
 		if len(token)+length <= maxLastArgLength {
 			// account for the space separating tokens
diff --git a/irc/isupport_test.go b/irc/isupport_test.go
new file mode 100644
index 00000000..70c831f6
--- /dev/null
+++ b/irc/isupport_test.go
@@ -0,0 +1,46 @@
+// Copyright (c) 2016 Daniel Oaks <daniel@danieloaks.net>
+// released under the MIT license
+
+package irc
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestISUPPORT(t *testing.T) {
+	// create first list
+	tList1 := NewISupportList()
+	tList1.Add("SASL", "yes")
+	tList1.Add("CASEMAPPING", "rfc1459-strict")
+	tList1.Add("INVEX", "i")
+	tList1.AddNoValue("EXTBAN")
+	tList1.Add("RANDKILL", "whenever")
+	tList1.RegenerateCachedReply()
+
+	expected := [][]string{{"CASEMAPPING=rfc1459-strict", "EXTBAN", "INVEX=i", "RANDKILL=whenever", "SASL=yes", "are supported by this server"}}
+	if !reflect.DeepEqual(tList1.CachedReply, expected) {
+		t.Error("tList1's cached reply does not match expected cached reply")
+	}
+
+	// create second list
+	tList2 := NewISupportList()
+	tList2.Add("SASL", "yes")
+	tList2.Add("CASEMAPPING", "ascii")
+	tList2.AddNoValue("INVEX")
+	tList2.Add("EXTBAN", "TestBah")
+	tList2.AddNoValue("STABLEKILL")
+	tList2.RegenerateCachedReply()
+
+	expected = [][]string{{"CASEMAPPING=ascii", "EXTBAN=TestBah", "INVEX", "SASL=yes", "STABLEKILL", "are supported by this server"}}
+	if !reflect.DeepEqual(tList2.CachedReply, expected) {
+		t.Error("tList2's cached reply does not match expected cached reply")
+	}
+
+	// compare lists
+	actual := tList1.GetDifference(tList2)
+	expected = [][]string{{"-RANDKILL", "CASEMAPPING=ascii", "EXTBAN=TestBah", "INVEX", "STABLEKILL", "are supported by this server"}}
+	if !reflect.DeepEqual(actual, expected) {
+		t.Error("difference reply does not match expected difference reply")
+	}
+}