Skip to content

Commit 71150f4

Browse files
LessUpLessUp
committed
refactor: deepen architecture for testability and maintainability
- Add browserApi abstraction layer for WebSocket, RTCPeerConnection, MediaRecorder, and timer APIs to enable unit testing - Extract shared observable module for state subscription pattern - Refactor state modules to use computed properties instead of getter/setter methods (e.g., room.status instead of getStatus()) - Split handler.go into join.go and forward.go for single responsibility - Add sendErrorAndLog helper to eliminate error handling repetition - Add protocol sync validation script to ensure YAML/Go/JS consistency - Update signaling.yaml with missing error codes and x-comment annotations - Add unit tests for signaling controller with mock WebSocket Files changed: - web/src/browserApi.js (new) - web/src/state/observable.js (new) - web/tests/mocks/mockWebSocket.js (new) - web/tests/mocks/mockPeerConnection.js (new) - web/tests/signaling.test.js (new) - internal/signal/join.go (new) - internal/signal/forward.go (new) - scripts/check-protocol-sync.sh (new)
1 parent 580bc40 commit 71150f4

25 files changed

Lines changed: 1337 additions & 570 deletions

‎CONTEXT.md‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,34 @@ new → connecting → connected → disconnected → closed/failed
9999

100100
---
101101

102+
## 架构概念
103+
104+
### BrowserApi（浏览器 API 抽象层）
105+
106+
浏览器原生 API 的抽象接口，用于解耦控制器与浏览器环境的直接依赖。
107+
108+
- 提供工厂方法：`createWebSocket`、`createPeerConnection`、`getUserMedia`、`getDisplayMedia`
109+
- 生产环境使用真实实现，测试环境注入 mock
110+
- 目的：解锁控制器单元测试能力
111+
112+
### Observable（可观察状态）
113+
114+
状态订阅模式的核心实现，提供 `subscribe` 和 `notify` 方法。
115+
116+
- 被状态模块复用，消除订阅逻辑重复
117+
- 支持重入保护（防止 notify 循环）
118+
119+
### 协议同步验证
120+
121+
自动化脚本检查 YAML、Go、JS 三处协议定义的一致性。
122+
123+
- 检查消息类型 enum 一致性
124+
- 检查错误码 enum 一致性
125+
- 检查字段名一致性
126+
- 集成到 `make check`
127+
128+
---
129+
102130
## 错误码
103131

104132
信令服务器定义的错误码：

‎Makefile‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,12 @@ vet:
3131
fmt:
3232
go fmt ./...
3333

34+
# Protocol sync check
35+
protocol-sync:
36+
@./scripts/check-protocol-sync.sh
37+
3438
# All checks (run before commit)
35-
check: build test lint vet
39+
check: build test lint vet protocol-sync
3640

3741
# Development with hot reload (requires air)
3842
dev:
@@ -42,4 +46,4 @@ dev:
4246
clean:
4347
rm -f coverage.out coverage.html
4448

45-
.PHONY: build run test test-cover lint vet fmt check dev clean
49+
.PHONY: build run test test-cover lint vet fmt check dev clean protocol-sync

‎internal/signal/client.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,15 @@ func (c *Client) sendError(err *ProtocolError) error {
9292
return c.enqueue(Message{Type: MsgTypeError, Room: room, Code: err.Code, Error: err.Message})
9393
}
9494

95+
// sendErrorAndLog sends an error message to the client and logs if sending fails.
96+
func (c *Client) sendErrorAndLog(err *ProtocolError) error {
97+
sendErr := c.sendError(err)
98+
if sendErr != nil {
99+
log.Printf("signal: failed to send %s error to conn=%d: %v", err.Code, c.connID, sendErr)
100+
}
101+
return sendErr
102+
}
103+
95104
// enqueue queues a message for sending to the client.
96105
func (c *Client) enqueue(msg Message) error {
97106
select {

‎internal/signal/forward.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package signal
2+
3+
import (
4+
"errors"
5+
"log"
6+
)
7+
8+
// forward routes signaling messages between clients.
9+
func (h *Hub) forward(sender *Client, msg Message) error {
10+
id, room := sender.identity()
11+
if id == "" || room == "" {
12+
_ = sender.sendErrorAndLog(ErrNotJoined)
13+
return errors.New("sender not joined")
14+
}
15+
to := normalizeClientID(msg.To, MaxClientIDLength)
16+
if to == "" || to == id {
17+
_ = sender.sendErrorAndLog(ErrInvalidTarget)
18+
return errors.New("invalid target")
19+
}
20+
21+
h.mu.RLock()
22+
m, ok := h.rooms[room]
23+
if !ok {
24+
h.mu.RUnlock()
25+
_ = sender.sendErrorAndLog(ErrRoomMissing)
26+
return errors.New("room missing")
27+
}
28+
current, ok := m[id]
29+
if !ok || current != sender {
30+
h.mu.RUnlock()
31+
_ = sender.sendErrorAndLog(ErrMembershipLost)
32+
return errors.New("sender not registered")
33+
}
34+
dst, ok := m[to]
35+
h.mu.RUnlock()
36+
if !ok || dst == nil {
37+
_ = sender.sendErrorAndLog(ErrTargetNotFound)
38+
return errors.New("target not found")
39+
}
40+
41+
msg.Room = room
42+
msg.From = id
43+
msg.To = to
44+
if err := dst.enqueue(msg); err != nil {
45+
log.Printf("signal: forward failed room=%s from=%s to=%s: %v", room, id, to, err)
46+
h.removeClient(dst)
47+
dst.close()
48+
return err
49+
}
50+
return nil
51+
}

‎internal/signal/handler.go‎

Lines changed: 2 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@ import (
44
"errors"
55
"log"
66
"net/http"
7-
"strings"
87
"time"
9-
"unicode"
108
)
119

1210
// HandleWS handles WebSocket connection upgrades and message processing.
@@ -68,9 +66,7 @@ func (h *Hub) HandleWS(w http.ResponseWriter, r *http.Request) {
6866
func (h *Hub) handleMessage(client *Client, msg Message) error {
6967
// Rate limit check
7068
if !client.checkRateLimit() {
71-
if err := client.sendError(ErrRateLimited); err != nil {
72-
log.Printf("signal: failed to send rate_limited error to conn=%d: %v", client.connID, err)
73-
}
69+
_ = client.sendErrorAndLog(ErrRateLimited)
7470
return errors.New("rate limited")
7571
}
7672

@@ -85,143 +81,7 @@ func (h *Hub) handleMessage(client *Client, msg Message) error {
8581
case MsgTypeOffer, MsgTypeAnswer, MsgTypeCandidate, MsgTypeHangup:
8682
return h.forward(client, msg)
8783
default:
88-
if err := client.sendError(ErrUnknownType); err != nil {
89-
log.Printf("signal: failed to send unknown_type error to conn=%d: %v", client.connID, err)
90-
}
84+
_ = client.sendErrorAndLog(ErrUnknownType)
9185
return errors.New("unknown message type")
9286
}
9387
}
94-
95-
// handleJoin processes a join request.
96-
func (h *Hub) handleJoin(c *Client, msg Message) error {
97-
id := normalizeClientID(msg.From, MaxClientIDLength)
98-
if id == "" {
99-
if err := c.sendError(ErrInvalidID); err != nil {
100-
log.Printf("signal: failed to send invalid_id error to conn=%d: %v", c.connID, err)
101-
}
102-
return errors.New("invalid client id")
103-
}
104-
room := normalizeRoomName(msg.Room, MaxRoomIDLength)
105-
if room == "" {
106-
if err := c.sendError(ErrInvalidRoom); err != nil {
107-
log.Printf("signal: failed to send invalid_room error to conn=%d: %v", c.connID, err)
108-
}
109-
return errors.New("invalid room")
110-
}
111-
112-
boundID, currentRoom := c.identity()
113-
if boundID != "" && boundID != id {
114-
if err := c.sendError(ErrIdentityLocked); err != nil {
115-
log.Printf("signal: failed to send identity_locked error to conn=%d: %v", c.connID, err)
116-
}
117-
return errors.New("identity mismatch")
118-
}
119-
if currentRoom != "" && currentRoom != room {
120-
if err := c.sendError(ErrAlreadyJoined); err != nil {
121-
log.Printf("signal: failed to send already_joined error to conn=%d: %v", c.connID, err)
122-
}
123-
return errors.New("already joined")
124-
}
125-
126-
c.setIdentity(id, room)
127-
if err := h.addClient(c); err != nil {
128-
c.setIdentity("", "") // Clear both id and room on failure
129-
if sendErr := c.sendError(err); sendErr != nil {
130-
log.Printf("signal: failed to send %s error to conn=%d: %v", err.Code, c.connID, sendErr)
131-
}
132-
return err
133-
}
134-
if err := c.enqueue(Message{Type: MsgTypeJoined, Room: room, From: id}); err != nil {
135-
return err
136-
}
137-
h.broadcastMembers(room)
138-
return nil
139-
}
140-
141-
// forward routes signaling messages between clients.
142-
func (h *Hub) forward(sender *Client, msg Message) error {
143-
id, room := sender.identity()
144-
if id == "" || room == "" {
145-
if err := sender.sendError(ErrNotJoined); err != nil {
146-
log.Printf("signal: failed to send not_joined error to conn=%d: %v", sender.connID, err)
147-
}
148-
return errors.New("sender not joined")
149-
}
150-
to := normalizeClientID(msg.To, MaxClientIDLength)
151-
if to == "" || to == id {
152-
if err := sender.sendError(ErrInvalidTarget); err != nil {
153-
log.Printf("signal: failed to send invalid_target error to conn=%d: %v", sender.connID, err)
154-
}
155-
return errors.New("invalid target")
156-
}
157-
158-
h.mu.RLock()
159-
m, ok := h.rooms[room]
160-
if !ok {
161-
h.mu.RUnlock()
162-
if err := sender.sendError(ErrRoomMissing); err != nil {
163-
log.Printf("signal: failed to send room_missing error to conn=%d: %v", sender.connID, err)
164-
}
165-
return errors.New("room missing")
166-
}
167-
current, ok := m[id]
168-
if !ok || current != sender {
169-
h.mu.RUnlock()
170-
if err := sender.sendError(ErrMembershipLost); err != nil {
171-
log.Printf("signal: failed to send membership_lost error to conn=%d: %v", sender.connID, err)
172-
}
173-
return errors.New("sender not registered")
174-
}
175-
dst, ok := m[to]
176-
h.mu.RUnlock()
177-
if !ok || dst == nil {
178-
if err := sender.sendError(ErrTargetNotFound); err != nil {
179-
log.Printf("signal: failed to send target_not_found error to conn=%d: %v", sender.connID, err)
180-
}
181-
return errors.New("target not found")
182-
}
183-
184-
msg.Room = room
185-
msg.From = id
186-
msg.To = to
187-
if err := dst.enqueue(msg); err != nil {
188-
log.Printf("signal: forward failed room=%s from=%s to=%s: %v", room, id, to, err)
189-
h.removeClient(dst)
190-
dst.close()
191-
return err
192-
}
193-
return nil
194-
}
195-
196-
// normalizeClientID validates and normalizes a client ID.
197-
func normalizeClientID(raw string, maxLen int) string {
198-
trimmed := strings.TrimSpace(raw)
199-
if trimmed == "" || len(trimmed) > maxLen {
200-
return ""
201-
}
202-
for _, r := range trimmed {
203-
switch {
204-
case r >= 'a' && r <= 'z':
205-
case r >= 'A' && r <= 'Z':
206-
case r >= '0' && r <= '9':
207-
case r == '-', r == '_':
208-
default:
209-
return ""
210-
}
211-
}
212-
return trimmed
213-
}
214-
215-
// normalizeRoomName validates and normalizes a room name.
216-
func normalizeRoomName(raw string, maxLen int) string {
217-
trimmed := strings.TrimSpace(raw)
218-
if trimmed == "" || len(trimmed) > maxLen {
219-
return ""
220-
}
221-
for _, r := range trimmed {
222-
if unicode.IsControl(r) {
223-
return ""
224-
}
225-
}
226-
return trimmed
227-
}

‎internal/signal/join.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package signal
2+
3+
import (
4+
"errors"
5+
"strings"
6+
"unicode"
7+
)
8+
9+
// handleJoin processes a join request.
10+
func (h *Hub) handleJoin(c *Client, msg Message) error {
11+
id := normalizeClientID(msg.From, MaxClientIDLength)
12+
if id == "" {
13+
_ = c.sendErrorAndLog(ErrInvalidID)
14+
return errors.New("invalid client id")
15+
}
16+
room := normalizeRoomName(msg.Room, MaxRoomIDLength)
17+
if room == "" {
18+
_ = c.sendErrorAndLog(ErrInvalidRoom)
19+
return errors.New("invalid room")
20+
}
21+
22+
boundID, currentRoom := c.identity()
23+
if boundID != "" && boundID != id {
24+
_ = c.sendErrorAndLog(ErrIdentityLocked)
25+
return errors.New("identity mismatch")
26+
}
27+
if currentRoom != "" && currentRoom != room {
28+
_ = c.sendErrorAndLog(ErrAlreadyJoined)
29+
return errors.New("already joined")
30+
}
31+
32+
c.setIdentity(id, room)
33+
if err := h.addClient(c); err != nil {
34+
c.setIdentity("", "") // Clear both id and room on failure
35+
_ = c.sendErrorAndLog(err)
36+
return err
37+
}
38+
if err := c.enqueue(Message{Type: MsgTypeJoined, Room: room, From: id}); err != nil {
39+
return err
40+
}
41+
h.broadcastMembers(room)
42+
return nil
43+
}
44+
45+
// normalizeClientID validates and normalizes a client ID.
46+
// Returns empty string if invalid.
47+
func normalizeClientID(raw string, maxLen int) string {
48+
trimmed := strings.TrimSpace(raw)
49+
if trimmed == "" || len(trimmed) > maxLen {
50+
return ""
51+
}
52+
for _, r := range trimmed {
53+
switch {
54+
case r >= 'a' && r <= 'z':
55+
case r >= 'A' && r <= 'Z':
56+
case r >= '0' && r <= '9':
57+
case r == '-', r == '_':
58+
default:
59+
return ""
60+
}
61+
}
62+
return trimmed
63+
}
64+
65+
// normalizeRoomName validates and normalizes a room name.
66+
// Returns empty string if invalid.
67+
func normalizeRoomName(raw string, maxLen int) string {
68+
trimmed := strings.TrimSpace(raw)
69+
if trimmed == "" || len(trimmed) > maxLen {
70+
return ""
71+
}
72+
for _, r := range trimmed {
73+
if unicode.IsControl(r) {
74+
return ""
75+
}
76+
}
77+
return trimmed
78+
}

0 commit comments

Comments
 (0)