add test coverage to ChatRegistry and ChatRoom types

handler/auth.go

@@ -69,7 +69,7 @@ func (s AuthService) SignoutChat(ctx context.Context, sess *state.Session, chatI
 	alertUserLeft(ctx, sess, chatSessMgr.(ChatMessageRelayer))
 	chatSessMgr.(SessionManager).Remove(sess)
 	if chatSessMgr.(SessionManager).Empty() {
-		s.chatRegistry.RemoveRoom(chatRoom.Cookie)
+		s.chatRegistry.Remove(chatRoom.Cookie)
 	}
 	return nil
 }

handler/auth_test.go

@@ -737,7 +737,7 @@ func TestAuthService_SignoutChat(t *testing.T) {
 			for _, params := range tt.mockParams.emptyParams {
 				sessionManager.EXPECT().Empty().Return(params.result)
 				if params.result {
-					chatRegistry.EXPECT().RemoveRoom(tt.chatRoom.Cookie)
+					chatRegistry.EXPECT().Remove(tt.chatRoom.Cookie)
 				}
 			}
 			chatSessionManager := struct {

handler/chat_nav.go

@@ -47,7 +47,7 @@ func (s ChatNavService) RequestChatRightsHandler(_ context.Context, inFrame osca
 							TLVList: oscar.TLVList{
 								oscar.NewTLV(oscar.ChatNavTLVClassPerms, uint16(0x0010)),
 								oscar.NewTLV(oscar.ChatNavTLVFlags, uint16(15)),
-								oscar.NewTLV(oscar.ChatNavTLVExchangeDesc, "default exchange"),
+								oscar.NewTLV(oscar.ChatNavTLVRoomName, "default exchange"),
 								oscar.NewTLV(oscar.ChatNavTLVCreatePerms, uint8(2)),
 								oscar.NewTLV(oscar.ChatNavTLVCharSet1, "us-ascii"),
 								oscar.NewTLV(oscar.ChatNavTLVLang1, "en"),

handler/chat_nav_test.go

@@ -174,7 +174,7 @@ func TestChatNavService_RequestChatRightsHandler(t *testing.T) {
 							TLVList: oscar.TLVList{
 								oscar.NewTLV(oscar.ChatNavTLVClassPerms, uint16(0x0010)),
 								oscar.NewTLV(oscar.ChatNavTLVFlags, uint16(15)),
-								oscar.NewTLV(oscar.ChatNavTLVExchangeDesc, "default exchange"),
+								oscar.NewTLV(oscar.ChatNavTLVRoomName, "default exchange"),
 								oscar.NewTLV(oscar.ChatNavTLVCreatePerms, uint8(2)),
 								oscar.NewTLV(oscar.ChatNavTLVCharSet1, "us-ascii"),
 								oscar.NewTLV(oscar.ChatNavTLVLang1, "en"),

handler/chat_registry_test.go

@@ -54,35 +54,35 @@ func (_c *mockChatRegistry_Register_Call) RunAndReturn(run func(state.ChatRoom,
 	return _c
 }
 
-// RemoveRoom provides a mock function with given fields: chatID
-func (_m *mockChatRegistry) RemoveRoom(chatID string) {
+// Remove provides a mock function with given fields: chatID
+func (_m *mockChatRegistry) Remove(chatID string) {
 	_m.Called(chatID)
 }
 
-// mockChatRegistry_RemoveRoom_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'RemoveRoom'
-type mockChatRegistry_RemoveRoom_Call struct {
+// mockChatRegistry_Remove_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Remove'
+type mockChatRegistry_Remove_Call struct {
 	*mock.Call
 }
 
-// RemoveRoom is a helper method to define mock.On call
+// Remove is a helper method to define mock.On call
 //   - chatID string
-func (_e *mockChatRegistry_Expecter) RemoveRoom(chatID interface{}) *mockChatRegistry_RemoveRoom_Call {
-	return &mockChatRegistry_RemoveRoom_Call{Call: _e.mock.On("RemoveRoom", chatID)}
+func (_e *mockChatRegistry_Expecter) Remove(chatID interface{}) *mockChatRegistry_Remove_Call {
+	return &mockChatRegistry_Remove_Call{Call: _e.mock.On("Remove", chatID)}
 }
 
-func (_c *mockChatRegistry_RemoveRoom_Call) Run(run func(chatID string)) *mockChatRegistry_RemoveRoom_Call {
+func (_c *mockChatRegistry_Remove_Call) Run(run func(chatID string)) *mockChatRegistry_Remove_Call {
 	_c.Call.Run(func(args mock.Arguments) {
 		run(args[0].(string))
 	})
 	return _c
 }
 
-func (_c *mockChatRegistry_RemoveRoom_Call) Return() *mockChatRegistry_RemoveRoom_Call {
+func (_c *mockChatRegistry_Remove_Call) Return() *mockChatRegistry_Remove_Call {
 	_c.Call.Return()
 	return _c
 }
 
-func (_c *mockChatRegistry_RemoveRoom_Call) RunAndReturn(run func(string)) *mockChatRegistry_RemoveRoom_Call {
+func (_c *mockChatRegistry_Remove_Call) RunAndReturn(run func(string)) *mockChatRegistry_Remove_Call {
 	_c.Call.Return(run)
 	return _c
 }

handler/types.go

@@ -50,5 +50,5 @@ type ChatMessageRelayer interface {
 type ChatRegistry interface {
 	Register(room state.ChatRoom, sessionManager any)
 	Retrieve(chatID string) (state.ChatRoom, any, error)
-	RemoveRoom(chatID string)
+	Remove(chatID string)
 }

oscar/snacs.go

@@ -489,8 +489,12 @@ const (
 	ChatNavTLVMaxConcurrentRooms uint16 = 0x02
 	ChatNavTLVClassPerms         uint16 = 0x02
 	ChatNavTLVExchangeInfo       uint16 = 0x03
+	ChatNavTLVFullyQualifiedName uint16 = 0x6A
+	ChatNavCreateTime            uint16 = 0x00CA
 	ChatNavTLVFlags              uint16 = 0xC9
-	ChatNavTLVExchangeDesc       uint16 = 0xD3
+	ChatNavTLVMaxMsgLen          uint16 = 0xD1
+	ChatNavTLVMaxOccupancy       uint16 = 0xD2
+	ChatNavTLVRoomName           uint16 = 0xD3
 	ChatNavTLVCreatePerms        uint16 = 0xD5
 	ChatNavTLVCharSet1           uint16 = 0xD6
 	ChatNavTLVLang1              uint16 = 0xD7

state/chat_registry.go

@@ -10,67 +10,97 @@ import (
 	"github.com/mkaminski/goaim/oscar"
 )
 
+// ErrChatRoomNotFound indicates that a chat room lookup failed.
+var ErrChatRoomNotFound = errors.New("chat room not found")
+
+// ChatRegistry keeps track of chat rooms. A ChatRegistry is safe for
+// concurrent use by multiple goroutines.
 type ChatRegistry struct {
-	chatRoomStore map[string]ChatRoom
-	smStore       map[string]any
-	mapMutex      sync.RWMutex
+	roomStore  map[string]ChatRoom // association of cookie identifier->chat room
+	valueStore map[string]any      // association of cookie identifier->value
+	mutex      sync.RWMutex        // ensures thread-safe read-write access to stores
 }
 
+// NewChatRegistry creates a new instance of ChatRegistry
 func NewChatRegistry() *ChatRegistry {
 	return &ChatRegistry{
-		chatRoomStore: make(map[string]ChatRoom),
-		smStore:       make(map[string]any),
+		roomStore:  make(map[string]ChatRoom),
+		valueStore: make(map[string]any),
 	}
 }
 
-func (c *ChatRegistry) Register(room ChatRoom, sessionManager any) {
-	c.mapMutex.Lock()
-	defer c.mapMutex.Unlock()
-	c.chatRoomStore[room.Cookie] = room
-	c.smStore[room.Cookie] = sessionManager
+// Register adds a chat room to the registry and associates an arbitrary value
+// with the room.
+func (c *ChatRegistry) Register(chatRoom ChatRoom, value any) {
+	c.mutex.Lock()
+	defer c.mutex.Unlock()
+	c.roomStore[chatRoom.Cookie] = chatRoom
+	c.valueStore[chatRoom.Cookie] = value
 }
 
-var ErrChatRoomNotFound = errors.New("chat room not found")
-
+// Retrieve retrieves a chat room and the arbitrary value associated with it.
+// Returns ErrChatRoomNotFound if the room is not registered.
 func (c *ChatRegistry) Retrieve(cookie string) (ChatRoom, any, error) {
-	c.mapMutex.RLock()
-	defer c.mapMutex.RUnlock()
-	chatRoom, found := c.chatRoomStore[cookie]
+	c.mutex.RLock()
+	defer c.mutex.RUnlock()
+	chatRoom, found := c.roomStore[cookie]
 	if !found {
 		return ChatRoom{}, nil, fmt.Errorf("%w cookie: %s", ErrChatRoomNotFound, cookie)
 	}
-	sessionManager, found := c.smStore[cookie]
+	value, found := c.valueStore[cookie]
 	if !found {
-		panic("unable to find session manager for chat")
+		panic("unable to find value for chat room")
 	}
-	return chatRoom, sessionManager, nil
+	return chatRoom, value, nil
 }
 
-func (c *ChatRegistry) RemoveRoom(cookie string) {
-	c.mapMutex.Lock()
-	defer c.mapMutex.Unlock()
-	delete(c.chatRoomStore, cookie)
-	delete(c.smStore, cookie)
+// Remove removes a chat room and the arbitrary value associated with it.
+func (c *ChatRegistry) Remove(cookie string) {
+	c.mutex.Lock()
+	defer c.mutex.Unlock()
+	delete(c.roomStore, cookie)
+	delete(c.valueStore, cookie)
 }
 
+// ChatRoom is the representation of a chat room's metadata.
 type ChatRoom struct {
-	CreateTime     time.Time
-	DetailLevel    uint8
-	Exchange       uint16
-	Cookie         string
+	// Cookie is the unique chat room identifier.
+	Cookie string
+	// CreateTime indicates when the chat room was created.
+	CreateTime time.Time
+	// DetailLevel is the detail level of the chat room.  Unclear what this value means.
+	DetailLevel uint8
+	// Exchange indicates which exchange the chatroom belongs to. Typically, a canned value.
+	Exchange uint16
+	// InstanceNumber indicates which instance chatroom exists in. Typically, a canned value.
 	InstanceNumber uint16
-	Name           string
+	// Name is the name of the chat room.
+	Name string
 }
 
+// TLVList returns a TLV list of chat room metadata.
 func (c ChatRoom) TLVList() []oscar.TLV {
 	return []oscar.TLV{
-		oscar.NewTLV(0x00c9, uint16(15)),
-		oscar.NewTLV(0x00ca, uint32(c.CreateTime.Unix())),
-		oscar.NewTLV(0x00d1, uint16(1024)),
-		oscar.NewTLV(0x00d2, uint16(100)),
-		oscar.NewTLV(0x00d5, uint8(2)),
-		oscar.NewTLV(0x006a, c.Name),
-		oscar.NewTLV(0x00d3, c.Name),
+		// From protocols/oscar/family_chatnav.c in lib purple, these are the
+		// room creation flags:
+		// - 1 Evilable
+		// - 2 Nav Only
+		// - 4 Instancing Allowed
+		// - 8 Occupant Peek Allowed
+		// It's unclear what effect they actually have.
+		oscar.NewTLV(oscar.ChatNavTLVFlags, uint16(15)),
+		oscar.NewTLV(oscar.ChatNavCreateTime, uint32(c.CreateTime.Unix())),
+		oscar.NewTLV(oscar.ChatNavTLVMaxMsgLen, uint16(1024)),
+		oscar.NewTLV(oscar.ChatNavTLVMaxOccupancy, uint16(100)),
+		// From protocols/oscar/family_chatnav.c in lib purple, these are the
+		// room creation permission values:
+		// - 0  creation not allowed
+		// - 1  room creation allowed
+		// - 2  exchange creation allowed
+		// It's unclear what effect they actually have.
+		oscar.NewTLV(oscar.ChatNavTLVCreatePerms, uint8(2)),
+		oscar.NewTLV(oscar.ChatNavTLVFullyQualifiedName, c.Name),
+		oscar.NewTLV(oscar.ChatNavTLVRoomName, c.Name),
 	}
 }
 

state/chat_registry_test.go

@@ -0,0 +1,164 @@
+package state
+
+import (
+	"testing"
+
+	"github.com/mkaminski/goaim/oscar"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestChatRegistry_RegisterAndReceive(t *testing.T) {
+	type registration struct {
+		room  ChatRoom
+		value any
+	}
+	tests := []struct {
+		name            string
+		givenRegistered []registration
+		lookupCookie    string
+		wantRegistered  registration
+		wantErr         error
+	}{
+		{
+			name: "chat room and value found",
+			givenRegistered: []registration{
+				{
+					room:  ChatRoom{Cookie: "cookie1"},
+					value: "value1",
+				},
+				{
+					room:  ChatRoom{Cookie: "cookie2"},
+					value: "value2",
+				},
+			},
+			lookupCookie: "cookie2",
+			wantRegistered: registration{
+				room:  ChatRoom{Cookie: "cookie2"},
+				value: "value2",
+			},
+		},
+		{
+			name: "chat room and value not found",
+			givenRegistered: []registration{
+				{
+					room:  ChatRoom{Cookie: "cookie1"},
+					value: "value1",
+				},
+				{
+					room:  ChatRoom{Cookie: "cookie2"},
+					value: "value2",
+				},
+			},
+			lookupCookie: "cookie3",
+			wantErr:      ErrChatRoomNotFound,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			chatRegistry := NewChatRegistry()
+			for _, r := range tt.givenRegistered {
+				chatRegistry.Register(r.room, r.value)
+			}
+
+			room, value, err := chatRegistry.Retrieve(tt.lookupCookie)
+
+			assert.Equal(t, tt.wantRegistered.room, room)
+			assert.Equal(t, tt.wantRegistered.value, value)
+			assert.ErrorIs(t, err, tt.wantErr)
+		})
+	}
+}
+
+func TestChatRegistry_RegisterAndRemove(t *testing.T) {
+	type registration struct {
+		room  ChatRoom
+		value any
+	}
+	tests := []struct {
+		name            string
+		givenRegistered []registration
+		removeCookie    string
+		wantRegistered  []registration
+		wantErr         error
+	}{
+		{
+			name: "chat room and value removed",
+			givenRegistered: []registration{
+				{
+					room:  ChatRoom{Cookie: "cookie1"},
+					value: "value1",
+				},
+				{
+					room:  ChatRoom{Cookie: "cookie2"},
+					value: "value2",
+				},
+			},
+			removeCookie: "cookie2",
+			wantRegistered: []registration{
+				{
+					room:  ChatRoom{Cookie: "cookie1"},
+					value: "value1",
+				},
+			},
+		},
+		{
+			name: "no chat room and value removed",
+			givenRegistered: []registration{
+				{
+					room:  ChatRoom{Cookie: "cookie1"},
+					value: "value1",
+				},
+				{
+					room:  ChatRoom{Cookie: "cookie2"},
+					value: "value2",
+				},
+			},
+			removeCookie: "cookie3",
+			wantRegistered: []registration{
+				{
+					room:  ChatRoom{Cookie: "cookie1"},
+					value: "value1",
+				},
+				{
+					room:  ChatRoom{Cookie: "cookie2"},
+					value: "value2",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			chatRegistry := NewChatRegistry()
+			for _, r := range tt.givenRegistered {
+				chatRegistry.Register(r.room, r.value)
+			}
+
+			chatRegistry.Remove(tt.removeCookie)
+
+			for _, r := range tt.wantRegistered {
+				room, value, err := chatRegistry.Retrieve(r.room.Cookie)
+				assert.Equal(t, r.room, room)
+				assert.Equal(t, r.value, value)
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestChatRoom_TLVList(t *testing.T) {
+	room := NewChatRoom()
+	room.Name = "chat-room-name"
+
+	have := room.TLVList()
+	want := []oscar.TLV{
+		oscar.NewTLV(oscar.ChatNavTLVFlags, uint16(15)),
+		oscar.NewTLV(oscar.ChatNavCreateTime, uint32(room.CreateTime.Unix())),
+		oscar.NewTLV(oscar.ChatNavTLVMaxMsgLen, uint16(1024)),
+		oscar.NewTLV(oscar.ChatNavTLVMaxOccupancy, uint16(100)),
+		oscar.NewTLV(oscar.ChatNavTLVCreatePerms, uint8(2)),
+		oscar.NewTLV(oscar.ChatNavTLVFullyQualifiedName, room.Name),
+		oscar.NewTLV(oscar.ChatNavTLVRoomName, room.Name),
+	}
+
+	assert.Equal(t, want, have)
+}