diff --git a/chat/command.go b/chat/command.go index ede108e..dea601b 100644 --- a/chat/command.go +++ b/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 - if target.Id() == msg.From().Id() { - return errors.New("cannot ignore self.") - } - - err := msg.From().Ignore(target.Id()) + err := msg.From().Ignore(target) if err != nil { return err } diff --git a/chat/message/user.go b/chat/message/user.go index d66c665..bcda156 100644 --- a/chat/message/user.go +++ b/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 { - if id == "" { +func (u *User) Ignore(identified common.Identified) error { + if identified == nil { return errors.New("user is nil.") } - u.mu.Lock() - defer u.mu.Unlock() - - for _, userId := range u.ignored { - if userId == id { - return errors.New("user already ignored.") - } + if identified.Id() == u.Id() { + return errors.New("cannot ignore self.") } - 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() - defer u.mu.Unlock() - - for i, userId := range u.ignored { - if userId == id { - u.ignored = append(u.ignored[:i], u.ignored[i+1:]...) - return nil - } + identified, err := u.Ignored.Get(id) + if err != nil { + return err } - return errors.New("user not found or not currently ignored.") -} - -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 + return u.Ignored.Remove(identified) } // Container for per-user configurations. diff --git a/chat/room.go b/chat/room.go index 08b27df..8e3ebba 100644 --- a/chat/room.go +++ b/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 - Ops *idSet + Members *common.IdSet + Ops *common.IdSet } // NewRoom creates a new room. @@ -47,8 +48,8 @@ func NewRoom() *Room { history: message.NewHistory(historyLen), commands: *defaultCommands, - Members: newIdSet(), - Ops: newIdSet(), + Members: common.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 } diff --git a/chat/room_test.go b/chat/room_test.go index b6c029b..078b80b 100644 --- a/chat/room_test.go +++ b/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 { - t.Fatalf("should have %d ignored users, has %d", 1, len(names)) + if ignoredList := ignorer.user.Ignored.ListPrefix(""); len(ignoredList) != 1 { + 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 { - t.Fatalf("should have %d ignored users, has %d", 0, len(names)) + if ignoredList := ignorer.user.Ignored.ListPrefix(""); len(ignoredList) != 0 { + 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() { diff --git a/chat/set_test.go b/chat/set_test.go index 6db2e9b..cf8135c 100644 --- a/chat/set_test.go +++ b/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) } diff --git a/chat/set.go b/common/set.go similarity index 67% rename from chat/set.go rename to common/set.go index 438fe97..7d81041 100644 --- a/chat/set.go +++ b/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 { - return &idSet{ - lookup: map[string]identified{}, +func NewIdSet() *IdSet { + return &IdSet{ + 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. -// Used for moving the same identified to a new Id, such as a rename. -func (s *idSet) Replace(oldId string, item identified) error { +// Replace item from old id with new Identified. +// Used for moving the same Identified to a new Id, such as a rename. +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 { - r := []identified{} +func (s *IdSet) ListPrefix(prefix string) []Identified { + r := []Identified{} prefix = strings.ToLower(prefix) s.RLock()