Skip to content

Commit 29f1845

Browse files
belakbelak
authored
Merge pull request #67 from go-irc/client-cleanup
Move client code out into separate filter functions
2 parents 5bf07c6 + 3ec935b commit 29f1845

File tree

5 files changed

+169
-110
lines changed

5 files changed

+169
-110
lines changed

‎client.go

Lines changed: 0 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -5,100 +5,10 @@ import (
55
"errors"
66
"fmt"
77
"io"
8-
"strings"
98
"sync"
109
"time"
1110
)
1211

13-
// clientFilters are pre-processing which happens for certain message
14-
// types. These were moved from below to keep the complexity of each
15-
// component down.
16-
var clientFilters = map[string]func(*Client, *Message){
17-
"001": func(c *Client, m *Message) {
18-
c.currentNick = m.Params[0]
19-
c.connected = true
20-
},
21-
"433": func(c *Client, m *Message) {
22-
// We only want to try and handle nick collisions during the initial
23-
// handshake.
24-
if c.connected {
25-
return
26-
}
27-
c.currentNick = c.currentNick + "_"
28-
c.Writef("NICK :%s", c.currentNick)
29-
},
30-
"437": func(c *Client, m *Message) {
31-
// We only want to try and handle nick collisions during the initial
32-
// handshake.
33-
if c.connected {
34-
return
35-
}
36-
c.currentNick = c.currentNick + "_"
37-
c.Writef("NICK :%s", c.currentNick)
38-
},
39-
"PING": func(c *Client, m *Message) {
40-
reply := m.Copy()
41-
reply.Command = "PONG"
42-
c.WriteMessage(reply)
43-
},
44-
"PONG": func(c *Client, m *Message) {
45-
if c.incomingPongChan != nil {
46-
select {
47-
case c.incomingPongChan <- m.Trailing():
48-
default:
49-
}
50-
}
51-
},
52-
"NICK": func(c *Client, m *Message) {
53-
if m.Prefix.Name == c.currentNick && len(m.Params) > 0 {
54-
c.currentNick = m.Params[0]
55-
}
56-
},
57-
"CAP": func(c *Client, m *Message) {
58-
if c.remainingCapResponses <= 0 || len(m.Params) <= 2 {
59-
return
60-
}
61-
62-
switch m.Params[1] {
63-
case "LS":
64-
for _, key := range strings.Split(m.Trailing(), " ") {
65-
cap := c.caps[key]
66-
cap.Available = true
67-
c.caps[key] = cap
68-
}
69-
c.remainingCapResponses--
70-
case "ACK":
71-
for _, key := range strings.Split(m.Trailing(), " ") {
72-
cap := c.caps[key]
73-
cap.Enabled = true
74-
c.caps[key] = cap
75-
}
76-
c.remainingCapResponses--
77-
case "NAK":
78-
// If we got a NAK and this REQ was required, we need to bail
79-
// with an error.
80-
for _, key := range strings.Split(m.Trailing(), " ") {
81-
if c.caps[key].Required {
82-
c.sendError(fmt.Errorf("CAP %s requested but was rejected", key))
83-
return
84-
}
85-
}
86-
c.remainingCapResponses--
87-
}
88-
89-
if c.remainingCapResponses <= 0 {
90-
for key, cap := range c.caps {
91-
if cap.Required && !cap.Enabled {
92-
c.sendError(fmt.Errorf("CAP %s requested but not accepted", key))
93-
return
94-
}
95-
}
96-
97-
c.Write("CAP END")
98-
}
99-
},
100-
}
101-
10212
// ClientConfig is a structure used to configure a Client.
10313
type ClientConfig struct {
10414
// General connection information.

‎client_handlers.go

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
package irc
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
)
7+
8+
type clientFilter func(*Client, *Message)
9+
10+
// clientFilters are pre-processing which happens for certain message
11+
// types. These were moved from below to keep the complexity of each
12+
// component down.
13+
var clientFilters = map[string]clientFilter{
14+
"001": handle001,
15+
"433": handle433,
16+
"437": handle437,
17+
"PING": handlePing,
18+
"PONG": handlePong,
19+
"NICK": handleNick,
20+
"CAP": handleCap,
21+
}
22+
23+
// From rfc2812 section 5.1 (Command responses)
24+
//
25+
// 001 RPL_WELCOME
26+
// "Welcome to the Internet Relay Network
27+
// <nick>!<user>@<host>"
28+
func handle001(c *Client, m *Message) {
29+
c.currentNick = m.Params[0]
30+
c.connected = true
31+
}
32+
33+
// From rfc2812 section 5.2 (Error Replies)
34+
//
35+
// 433 ERR_NICKNAMEINUSE
36+
// "<nick> :Nickname is already in use"
37+
//
38+
// - Returned when a NICK message is processed that results
39+
// in an attempt to change to a currently existing
40+
// nickname.
41+
func handle433(c *Client, m *Message) {
42+
// We only want to try and handle nick collisions during the initial
43+
// handshake.
44+
if c.connected {
45+
return
46+
}
47+
c.currentNick = c.currentNick + "_"
48+
c.Writef("NICK :%s", c.currentNick)
49+
}
50+
51+
// From rfc2812 section 5.2 (Error Replies)
52+
//
53+
// 437 ERR_UNAVAILRESOURCE
54+
// "<nick/channel> :Nick/channel is temporarily unavailable"
55+
//
56+
// - Returned by a server to a user trying to join a channel
57+
// currently blocked by the channel delay mechanism.
58+
//
59+
// - Returned by a server to a user trying to change nickname
60+
// when the desired nickname is blocked by the nick delay
61+
// mechanism.
62+
func handle437(c *Client, m *Message) {
63+
// We only want to try and handle nick collisions during the initial
64+
// handshake.
65+
if c.connected {
66+
return
67+
}
68+
c.currentNick = c.currentNick + "_"
69+
c.Writef("NICK :%s", c.currentNick)
70+
}
71+
72+
func handlePing(c *Client, m *Message) {
73+
reply := m.Copy()
74+
reply.Command = "PONG"
75+
c.WriteMessage(reply)
76+
}
77+
78+
func handlePong(c *Client, m *Message) {
79+
if c.incomingPongChan != nil {
80+
select {
81+
case c.incomingPongChan <- m.Trailing():
82+
default:
83+
// Note that this return isn't really needed, but it helps some code
84+
// coverage tools actually see this line.
85+
return
86+
}
87+
}
88+
}
89+
90+
func handleNick(c *Client, m *Message) {
91+
if m.Prefix.Name == c.currentNick && len(m.Params) > 0 {
92+
c.currentNick = m.Params[0]
93+
}
94+
}
95+
96+
var capFilters = map[string]clientFilter{
97+
"LS": handleCapLs,
98+
"ACK": handleCapAck,
99+
"NAK": handleCapNak,
100+
}
101+
102+
func handleCap(c *Client, m *Message) {
103+
if c.remainingCapResponses <= 0 || len(m.Params) <= 2 {
104+
return
105+
}
106+
107+
if filter, ok := capFilters[m.Params[1]]; ok {
108+
filter(c, m)
109+
}
110+
111+
if c.remainingCapResponses <= 0 {
112+
for key, cap := range c.caps {
113+
if cap.Required && !cap.Enabled {
114+
c.sendError(fmt.Errorf("CAP %s requested but not accepted", key))
115+
return
116+
}
117+
}
118+
119+
c.Write("CAP END")
120+
}
121+
}
122+
123+
func handleCapLs(c *Client, m *Message) {
124+
for _, key := range strings.Split(m.Trailing(), " ") {
125+
cap := c.caps[key]
126+
cap.Available = true
127+
c.caps[key] = cap
128+
}
129+
c.remainingCapResponses--
130+
}
131+
132+
func handleCapAck(c *Client, m *Message) {
133+
for _, key := range strings.Split(m.Trailing(), " ") {
134+
cap := c.caps[key]
135+
cap.Enabled = true
136+
c.caps[key] = cap
137+
}
138+
c.remainingCapResponses--
139+
}
140+
141+
func handleCapNak(c *Client, m *Message) {
142+
// If we got a NAK and this REQ was required, we need to bail
143+
// with an error.
144+
for _, key := range strings.Split(m.Trailing(), " ") {
145+
if c.caps[key].Required {
146+
c.sendError(fmt.Errorf("CAP %s requested but was rejected", key))
147+
return
148+
}
149+
}
150+
c.remainingCapResponses--
151+
}

‎client_test.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -388,20 +388,14 @@ func TestPingLoop(t *testing.T) {
388388

389389
// This one is just for coverage, so we know we're hitting the
390390
// branch that drops extra pings.
391-
runClientTest(t, config, io.EOF, nil, []TestAction{
391+
runClientTest(t, config, io.EOF, func(c *Client) {
392+
c.incomingPongChan = make(chan string)
393+
handlePong(c, MustParseMessage("PONG :hello 1"))
394+
}, []TestAction{
392395
ExpectLine("PASS :test_pass\r\n"),
393396
ExpectLine("NICK :test_nick\r\n"),
394397
ExpectLine("USER test_user 0.0.0.0 0.0.0.0 :test_name\r\n"),
395398
SendLine("001 :hello_world\r\n"),
396-
397-
// It's a buffered channel of 5, so we want to send at least 6 of them
398-
SendLine("PONG :hello 1\r\n"),
399-
SendLine("PONG :hello 2\r\n"),
400-
SendLine("PONG :hello 3\r\n"),
401-
SendLine("PONG :hello 4\r\n"),
402-
SendLine("PONG :hello 5\r\n"),
403-
SendLine("PONG :hello 6\r\n"),
404-
SendLine("PONG :hello 7\r\n"),
405399
})
406400

407401
// Successful ping with write error

‎parser_test.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ func TestMessageCopy(t *testing.T) {
113113

114114
type MsgSplitTests struct {
115115
Tests []struct {
116+
Desc string
116117
Input string
117118
Atoms struct {
118119
Source *string
@@ -135,15 +136,15 @@ func TestMsgSplit(t *testing.T) {
135136

136137
for _, test := range splitTests.Tests {
137138
msg, err := ParseMessage(test.Input)
138-
assert.NoError(t, err, "Failed to parse: %s (%s)", test.Input, err)
139+
assert.NoError(t, err, "%s: Failed to parse: %s (%s)", test.Desc, test.Input, err)
139140

140141
assert.Equal(t,
141142
strings.ToUpper(test.Atoms.Verb), msg.Command,
142-
"Wrong command for input: %s", test.Input,
143+
"%s: Wrong command for input: %s", test.Desc, test.Input,
143144
)
144145
assert.Equal(t,
145146
test.Atoms.Params, msg.Params,
146-
"Wrong params for input: %s", test.Input,
147+
"%s: Wrong params for input: %s", test.Desc, test.Input,
147148
)
148149

149150
if test.Atoms.Source != nil {
@@ -152,23 +153,25 @@ func TestMsgSplit(t *testing.T) {
152153

153154
assert.Equal(t,
154155
len(test.Atoms.Tags), len(msg.Tags),
155-
"Wrong number of tags",
156+
"%s: Wrong number of tags",
157+
test.Desc,
156158
)
157159

158160
for k, v := range test.Atoms.Tags {
159161
tag, ok := msg.GetTag(k)
160162
assert.True(t, ok, "Missing tag")
161163
if v == nil {
162-
assert.EqualValues(t, "", tag, "Tag differs")
164+
assert.EqualValues(t, "", tag, "%s: Tag %q differs: %s != \"\"", test.Desc, k, tag)
163165
} else {
164-
assert.EqualValues(t, v, tag, "Tag differs")
166+
assert.EqualValues(t, v, tag, "%s: Tag %q differs: %s != %s", test.Desc, k, v, tag)
165167
}
166168
}
167169
}
168170
}
169171

170172
type MsgJoinTests struct {
171173
Tests []struct {
174+
Desc string
172175
Atoms struct {
173176
Source string
174177
Verb string
@@ -211,6 +214,7 @@ func TestMsgJoin(t *testing.T) {
211214

212215
type UserhostSplitTests struct {
213216
Tests []struct {
217+
Desc string
214218
Source string
215219
Atoms struct {
216220
Nick string
@@ -235,15 +239,15 @@ func TestUserhostSplit(t *testing.T) {
235239

236240
assert.Equal(t,
237241
test.Atoms.Nick, prefix.Name,
238-
"Name did not match for input: %q", test.Source,
242+
"%s: Name did not match for input: %q", test.Desc, test.Source,
239243
)
240244
assert.Equal(t,
241245
test.Atoms.User, prefix.User,
242-
"User did not match for input: %q", test.Source,
246+
"%s: User did not match for input: %q", test.Desc, test.Source,
243247
)
244248
assert.Equal(t,
245249
test.Atoms.Host, prefix.Host,
246-
"Host did not match for input: %q", test.Source,
250+
"%s: Host did not match for input: %q", test.Desc, test.Source,
247251
)
248252
}
249253
}

‎testcases

0 commit comments

Comments
 (0)