Move set to common package, use set for ignores

* `chat/set.go` is now `common/set.go`
* use `*IdSet` as type for ignored list for users
* remove `IsIgnoring` and `IgnoredNames` user functions, so to use directly `IdSet` methods
* refactor code accordingly

chat/command.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/shazow/ssh-chat/chat/message"
+	"github.com/shazow/ssh-chat/common"
 )
 
 // The error returned when an invalid command is issued.
@@ -248,13 +249,16 @@ func InitCommands(c *Commands) {
 		Handler: func(room *Room, msg message.CommandMsg) error {
 			id := strings.TrimSpace(strings.TrimLeft(msg.Body(), "/ignore"))
 			if id == "" {
-				ignored := msg.From().IgnoredNames()
+				var names []string
+				msg.From().Ignored.Each(func(i common.Identified) {
+					names = append(names, i.Id())
+				})
 
 				var systemMsg string
-				if len(ignored) == 0 {
+				if len(names) == 0 {
 					systemMsg = "0 users ignored."
 				} else {
-					systemMsg = fmt.Sprintf("%d ignored: %s", len(ignored), strings.Join(ignored, ", "))
+					systemMsg = fmt.Sprintf("%d ignored: %s", len(names), strings.Join(names, ", "))
 				}
 
 				room.Send(message.NewSystemMsg(systemMsg, msg.From()))
@@ -266,12 +270,7 @@ func InitCommands(c *Commands) {
 				return fmt.Errorf("user %s not found.", id)
 			}
 
-			// Don't ignore yourself
+			err := msg.From().Ignore(target)
-			if target.Id() == msg.From().Id() {
-				return errors.New("cannot ignore self.")
-			}
-
-			err := msg.From().Ignore(target.Id())
 			if err != nil {
 				return err
 			}

chat/message/user.go

@@ -8,6 +8,8 @@ import (
 	"regexp"
 	"sync"
 	"time"
+
+	"github.com/shazow/ssh-chat/common"
 )
 
 const messageBuffer = 5
@@ -24,13 +26,11 @@ type User struct {
 	joined   time.Time
 	msg      chan Message
 	done     chan struct{}
-	ignored  []string
+	Ignored  *common.IdSet
 
 	replyTo   *User // Set when user gets a /msg, for replying.
 	screen    io.WriteCloser
 	closeOnce sync.Once
-
-	mu sync.RWMutex
 }
 
 func NewUser(identity Identifier) *User {
@@ -40,6 +40,7 @@ func NewUser(identity Identifier) *User {
 		joined:     time.Now(),
 		msg:        make(chan Message, messageBuffer),
 		done:       make(chan struct{}),
+		Ignored:    common.NewIdSet(),
 	}
 	u.SetColorIdx(rand.Int())
 
@@ -175,21 +176,20 @@ func (u *User) Send(m Message) error {
 	return nil
 }
 
-func (u *User) Ignore(id string) error {
+func (u *User) Ignore(identified common.Identified) error {
-	if id == "" {
+	if identified == nil {
 		return errors.New("user is nil.")
 	}
 
-	u.mu.Lock()
+	if identified.Id() == u.Id() {
-	defer u.mu.Unlock()
+		return errors.New("cannot ignore self.")
-
-	for _, userId := range u.ignored {
-		if userId == id {
-			return errors.New("user already ignored.")
-		}
 	}
 
-	u.ignored = append(u.ignored, id)
+	if u.Ignored.In(identified) {
+		return errors.New("user already ignored.")
+	}
+
+	u.Ignored.Add(identified)
 	return nil
 }
 
@@ -198,40 +198,12 @@ func (u *User) Unignore(id string) error {
 		return errors.New("user is nil.")
 	}
 
-	u.mu.Lock()
+	identified, err := u.Ignored.Get(id)
-	defer u.mu.Unlock()
+	if err != nil {
-
+		return err
-	for i, userId := range u.ignored {
-		if userId == id {
-			u.ignored = append(u.ignored[:i], u.ignored[i+1:]...)
-			return nil
-		}
 	}
 
-	return errors.New("user not found or not currently ignored.")
+	return u.Ignored.Remove(identified)
-}
-
-func (u *User) IgnoredNames() []string {
-	u.mu.RLock()
-	defer u.mu.RUnlock()
-
-	names := make([]string, len(u.ignored))
-	for i := range u.ignored {
-		names[i] = u.ignored[i]
-	}
-	return names
-}
-
-func (u *User) IsIgnoring(id string) bool {
-	u.mu.RLock()
-	defer u.mu.RUnlock()
-
-	for _, userId := range u.ignored {
-		if userId == id {
-			return true
-		}
-	}
-	return false
 }
 
 // Container for per-user configurations.

chat/room.go

@@ -7,6 +7,7 @@ import (
 	"sync"
 
 	"github.com/shazow/ssh-chat/chat/message"
+	"github.com/shazow/ssh-chat/common"
 )
 
 const historyLen = 20
@@ -34,8 +35,8 @@ type Room struct {
 	closed    bool
 	closeOnce sync.Once
 
-	Members *idSet
+	Members *common.IdSet
-	Ops     *idSet
+	Ops     *common.IdSet
 }
 
 // NewRoom creates a new room.
@@ -47,8 +48,8 @@ func NewRoom() *Room {
 		history:   message.NewHistory(historyLen),
 		commands:  *defaultCommands,
 
-		Members: newIdSet(),
+		Members: common.NewIdSet(),
-		Ops:     newIdSet(),
+		Ops:     common.NewIdSet(),
 	}
 }
 
@@ -61,7 +62,7 @@ func (r *Room) SetCommands(commands Commands) {
 func (r *Room) Close() {
 	r.closeOnce.Do(func() {
 		r.closed = true
-		r.Members.Each(func(m identified) {
+		r.Members.Each(func(m common.Identified) {
 			m.(*Member).Close()
 		})
 		r.Members.Clear()
@@ -95,10 +96,10 @@ func (r *Room) HandleMsg(m message.Message) {
 		}
 
 		r.history.Add(m)
-		r.Members.Each(func(u identified) {
+		r.Members.Each(func(u common.Identified) {
 			user := u.(*Member).User
 
-			if fromMsg != nil && user.IsIgnoring(fromMsg.From().Id()) {
+			if fromMsg != nil && user.Ignored.In(fromMsg.From()) {
 				// Skip because ignored
 				return
 			}

chat/room_test.go

@@ -100,8 +100,8 @@ func TestIgnore(t *testing.T) {
 		t.Fatal(err)
 	}
 	expectOutput(t, buffer, "-> Err: user already ignored."+message.Newline)
-	if names := ignorer.user.IgnoredNames(); len(names) != 1 {
+	if ignoredList := ignorer.user.Ignored.ListPrefix(""); len(ignoredList) != 1 {
-		t.Fatalf("should have %d ignored users, has %d", 1, len(names))
+		t.Fatalf("should have %d ignored users, has %d", 1, len(ignoredList))
 	}
 
 	// when a message is sent from the ignored user, it is delivered to non-ignoring users
@@ -132,15 +132,15 @@ func TestIgnore(t *testing.T) {
 	}
 	expectOutput(t, buffer, "-> 0 users ignored."+message.Newline)
 
-	if names := ignorer.user.IgnoredNames(); len(names) != 0 {
+	if ignoredList := ignorer.user.Ignored.ListPrefix(""); len(ignoredList) != 0 {
-		t.Fatalf("should have %d ignored users, has %d", 0, len(names))
+		t.Fatalf("should have %d ignored users, has %d", 0, len(ignoredList))
 	}
 
 	// after unignoring a user, its messages can be received again
 	ch.Send(message.NewPublicMsg("hello again!", ignored.user))
 
 	// give some time for the channel to get the message
-	time.Sleep(50)
+	time.Sleep(100)
 
 	// ensure ignorer has received the message
 	if !ignorer.user.HasMessages() {

chat/set_test.go

@@ -4,11 +4,12 @@ import (
 	"testing"
 
 	"github.com/shazow/ssh-chat/chat/message"
+	"github.com/shazow/ssh-chat/common"
 )
 
 func TestSet(t *testing.T) {
 	var err error
-	s := newIdSet()
+	s := common.NewIdSet()
 	u := message.NewUser(message.SimpleId("foo"))
 
 	if s.In(u) {
@@ -31,7 +32,7 @@ func TestSet(t *testing.T) {
 	}
 
 	err = s.Add(u2)
-	if err != ErrIdTaken {
+	if err != common.ErrIdTaken {
 		t.Error(err)
 	}
 

common/set.go

@@ -1,4 +1,4 @@
-package chat
+package common
 
 import (
 	"errors"
@@ -10,45 +10,45 @@ import (
 var ErrIdTaken = errors.New("id already taken")
 
 // The error returned when a requested item does not exist in the set.
-var ErridentifiedMissing = errors.New("item does not exist")
+var ErrIdentifiedMissing = errors.New("item does not exist")
 
 // Interface for an item storeable in the set
-type identified interface {
+type Identified interface {
 	Id() string
 }
 
 // Set with string lookup.
 // TODO: Add trie for efficient prefix lookup?
-type idSet struct {
+type IdSet struct {
 	sync.RWMutex
-	lookup map[string]identified
+	lookup map[string]Identified
 }
 
 // newIdSet creates a new set.
-func newIdSet() *idSet {
+func NewIdSet() *IdSet {
-	return &idSet{
+	return &IdSet{
-		lookup: map[string]identified{},
+		lookup: map[string]Identified{},
 	}
 }
 
 // Clear removes all items and returns the number removed.
-func (s *idSet) Clear() int {
+func (s *IdSet) Clear() int {
 	s.Lock()
 	n := len(s.lookup)
-	s.lookup = map[string]identified{}
+	s.lookup = map[string]Identified{}
 	s.Unlock()
 	return n
 }
 
 // Len returns the size of the set right now.
-func (s *idSet) Len() int {
+func (s *IdSet) Len() int {
 	s.RLock()
 	defer s.RUnlock()
 	return len(s.lookup)
 }
 
 // In checks if an item exists in this set.
-func (s *idSet) In(item identified) bool {
+func (s *IdSet) In(item Identified) bool {
 	s.RLock()
 	_, ok := s.lookup[item.Id()]
 	s.RUnlock()
@@ -56,20 +56,20 @@ func (s *idSet) In(item identified) bool {
 }
 
 // Get returns an item with the given Id.
-func (s *idSet) Get(id string) (identified, error) {
+func (s *IdSet) Get(id string) (Identified, error) {
 	s.RLock()
 	item, ok := s.lookup[id]
 	s.RUnlock()
 
 	if !ok {
-		return nil, ErridentifiedMissing
+		return nil, ErrIdentifiedMissing
 	}
 
 	return item, nil
 }
 
 // Add item to this set if it does not exist already.
-func (s *idSet) Add(item identified) error {
+func (s *IdSet) Add(item Identified) error {
 	s.Lock()
 	defer s.Unlock()
 
@@ -83,21 +83,21 @@ func (s *idSet) Add(item identified) error {
 }
 
 // Remove item from this set.
-func (s *idSet) Remove(item identified) error {
+func (s *IdSet) Remove(item Identified) error {
 	s.Lock()
 	defer s.Unlock()
 	id := item.Id()
 	_, found := s.lookup[id]
 	if !found {
-		return ErridentifiedMissing
+		return ErrIdentifiedMissing
 	}
 	delete(s.lookup, id)
 	return nil
 }
 
-// Replace item from old id with new identified.
+// Replace item from old id with new Identified.
-// Used for moving the same identified to a new Id, such as a rename.
+// Used for moving the same Identified to a new Id, such as a rename.
-func (s *idSet) Replace(oldId string, item identified) error {
+func (s *IdSet) Replace(oldId string, item Identified) error {
 	s.Lock()
 	defer s.Unlock()
 
@@ -110,11 +110,11 @@ func (s *idSet) Replace(oldId string, item identified) error {
 	// Remove oldId
 	_, found = s.lookup[oldId]
 	if !found {
-		return ErridentifiedMissing
+		return ErrIdentifiedMissing
 	}
 	delete(s.lookup, oldId)
 
-	// Add new identified
+	// Add new Identified
 	s.lookup[item.Id()] = item
 
 	return nil
@@ -122,7 +122,7 @@ func (s *idSet) Replace(oldId string, item identified) error {
 
 // Each loops over every item while holding a read lock and applies fn to each
 // element.
-func (s *idSet) Each(fn func(item identified)) {
+func (s *IdSet) Each(fn func(item Identified)) {
 	s.RLock()
 	for _, item := range s.lookup {
 		fn(item)
@@ -131,8 +131,8 @@ func (s *idSet) Each(fn func(item identified)) {
 }
 
 // ListPrefix returns a list of items with a prefix, case insensitive.
-func (s *idSet) ListPrefix(prefix string) []identified {
+func (s *IdSet) ListPrefix(prefix string) []Identified {
-	r := []identified{}
+	r := []Identified{}
 	prefix = strings.ToLower(prefix)
 
 	s.RLock()