Merge lp:~chipaca/ubuntu-push/client-v0-p11 into lp:ubuntu-push
- client-v0-p11
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | John Lenton |
Approved revision: | 60 |
Merged at revision: | 51 |
Proposed branch: | lp:~chipaca/ubuntu-push/client-v0-p11 |
Merge into: | lp:ubuntu-push |
Prerequisite: | lp:~chipaca/ubuntu-push/client-v0-p9 |
Diff against target: |
387 lines (+118/-18) 3 files modified
client/client.go (+31/-6) client/client_test.go (+79/-12) client/session/session.go (+8/-0) |
To merge this branch: | bzr merge lp:~chipaca/ubuntu-push/client-v0-p11 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Samuele Pedroni | Approve | ||
Review via email: mp+204696@code.launchpad.net |
Commit message
Several things:
* features:
* Client.doLoop, the puppet master.
* fixes and cleanups:
* added log_level to client config
* added the mysterious sessionRetryCh, used in doLoop to avoid a rather common starvation scenario.
* found a way not to panic out in initSession (not that it's much better)
* unified logging in the client tests a bit
* added logging to session's start error states.
Description of the change
Several things:
* features:
* Client.doLoop, the puppet master.
* fixes and cleanups:
* added log_level to client config
* added the mysterious sessionRetryCh, used in doLoop to avoid a rather common starvation scenario.
* found a way not to panic out in initSession (not that it's much better)
* unified logging in the client tests a bit
* added logging to session's start error states.
Samuele Pedroni (pedronis) wrote : | # |
Samuele Pedroni (pedronis) wrote : | # |
// ClientConfig holds the client configuration
type ClientConfig struct {
// A reasonably large maximum ping time
that's probably more: A reasonably large timeout for receive/answer pairs
Samuele Pedroni (pedronis) wrote : | # |
ok, but see bug/markers above and as discussed
- 60. By John Lenton
-
added XXX comments, and fixed ExchangeTimeout docstring, as per pedronis suggestions
John Lenton (chipaca) wrote : | # |
Done. Also added auto-highlighting of XXX comments to my .emacs... :-)
Preview Diff
1 | === modified file 'client/client.go' | |||
2 | --- client/client.go 2014-02-04 18:19:28 +0000 | |||
3 | +++ client/client.go 2014-02-04 18:19:28 +0000 | |||
4 | @@ -39,12 +39,14 @@ | |||
5 | 39 | // ClientConfig holds the client configuration | 39 | // ClientConfig holds the client configuration |
6 | 40 | type ClientConfig struct { | 40 | type ClientConfig struct { |
7 | 41 | connectivity.ConnectivityConfig // q.v. | 41 | connectivity.ConnectivityConfig // q.v. |
9 | 42 | // A reasonably large maximum ping time | 42 | // A reasonably larg timeout for receive/answer pairs |
10 | 43 | ExchangeTimeout config.ConfigTimeDuration `json:"exchange_timeout"` | 43 | ExchangeTimeout config.ConfigTimeDuration `json:"exchange_timeout"` |
11 | 44 | // The server to connect to | 44 | // The server to connect to |
12 | 45 | Addr config.ConfigHostPort | 45 | Addr config.ConfigHostPort |
13 | 46 | // The PEM-encoded server certificate | 46 | // The PEM-encoded server certificate |
14 | 47 | CertPEMFile string `json:"cert_pem_file"` | 47 | CertPEMFile string `json:"cert_pem_file"` |
15 | 48 | // The logging level (one of "debug", "info", "error") | ||
16 | 49 | LogLevel string `json:"log_level"` | ||
17 | 48 | } | 50 | } |
18 | 49 | 51 | ||
19 | 50 | // Client is the Ubuntu Push Notifications client-side daemon. | 52 | // Client is the Ubuntu Push Notifications client-side daemon. |
20 | @@ -62,6 +64,7 @@ | |||
21 | 62 | actionsCh <-chan notifications.RawActionReply | 64 | actionsCh <-chan notifications.RawActionReply |
22 | 63 | session *session.ClientSession | 65 | session *session.ClientSession |
23 | 64 | sessionRetrierStopper chan bool | 66 | sessionRetrierStopper chan bool |
24 | 67 | sessionRetryCh chan uint32 | ||
25 | 65 | } | 68 | } |
26 | 66 | 69 | ||
27 | 67 | // Configure loads the configuration specified in configPath, and sets it up. | 70 | // Configure loads the configuration specified in configPath, and sets it up. |
28 | @@ -74,8 +77,8 @@ | |||
29 | 74 | if err != nil { | 77 | if err != nil { |
30 | 75 | return fmt.Errorf("reading config: %v", err) | 78 | return fmt.Errorf("reading config: %v", err) |
31 | 76 | } | 79 | } |
34 | 77 | // later, we'll be specifying logging options in the config file | 80 | // later, we'll be specifying more logging options in the config file |
35 | 78 | client.log = logger.NewSimpleLogger(os.Stderr, "error") | 81 | client.log = logger.NewSimpleLogger(os.Stderr, client.config.LogLevel) |
36 | 79 | 82 | ||
37 | 80 | // overridden for testing | 83 | // overridden for testing |
38 | 81 | client.idder = identifier.New() | 84 | client.idder = identifier.New() |
39 | @@ -84,6 +87,7 @@ | |||
40 | 84 | client.connectivityEndp = bus.SystemBus.Endpoint(networkmanager.BusAddress, client.log) | 87 | client.connectivityEndp = bus.SystemBus.Endpoint(networkmanager.BusAddress, client.log) |
41 | 85 | 88 | ||
42 | 86 | client.connCh = make(chan bool) | 89 | client.connCh = make(chan bool) |
43 | 90 | client.sessionRetryCh = make(chan uint32) | ||
44 | 87 | 91 | ||
45 | 88 | if client.config.CertPEMFile != "" { | 92 | if client.config.CertPEMFile != "" { |
46 | 89 | client.pem, err = ioutil.ReadFile(client.config.CertPEMFile) | 93 | client.pem, err = ioutil.ReadFile(client.config.CertPEMFile) |
47 | @@ -126,17 +130,19 @@ | |||
48 | 126 | } | 130 | } |
49 | 127 | 131 | ||
50 | 128 | // initSession creates the session object | 132 | // initSession creates the session object |
52 | 129 | func (client *Client) initSession() { | 133 | func (client *Client) initSession() error { |
53 | 130 | sess, err := session.NewSession(string(client.config.Addr), client.pem, | 134 | sess, err := session.NewSession(string(client.config.Addr), client.pem, |
54 | 131 | client.config.ExchangeTimeout.Duration, client.deviceId, client.log) | 135 | client.config.ExchangeTimeout.Duration, client.deviceId, client.log) |
55 | 132 | if err != nil { | 136 | if err != nil { |
57 | 133 | panic("Don't know how to handle session creation failure.") | 137 | return err |
58 | 134 | } | 138 | } |
59 | 135 | client.session = sess | 139 | client.session = sess |
60 | 140 | return nil | ||
61 | 136 | } | 141 | } |
62 | 137 | 142 | ||
63 | 138 | // connectSession kicks off the session connection dance | 143 | // connectSession kicks off the session connection dance |
64 | 139 | func (client *Client) connectSession() { | 144 | func (client *Client) connectSession() { |
65 | 145 | // XXX: lp:1276199 | ||
66 | 140 | if client.sessionRetrierStopper != nil { | 146 | if client.sessionRetrierStopper != nil { |
67 | 141 | client.sessionRetrierStopper <- true | 147 | client.sessionRetrierStopper <- true |
68 | 142 | client.sessionRetrierStopper = nil | 148 | client.sessionRetrierStopper = nil |
69 | @@ -146,11 +152,12 @@ | |||
70 | 146 | client.session.Dial, | 152 | client.session.Dial, |
71 | 147 | util.Jitter} | 153 | util.Jitter} |
72 | 148 | client.sessionRetrierStopper = ar.Stop | 154 | client.sessionRetrierStopper = ar.Stop |
74 | 149 | go ar.Retry() | 155 | go func() { client.sessionRetryCh <- ar.Retry() }() |
75 | 150 | } | 156 | } |
76 | 151 | 157 | ||
77 | 152 | // disconnectSession disconnects the session | 158 | // disconnectSession disconnects the session |
78 | 153 | func (client *Client) disconnectSession() { | 159 | func (client *Client) disconnectSession() { |
79 | 160 | // XXX: lp:1276199 | ||
80 | 154 | if client.sessionRetrierStopper != nil { | 161 | if client.sessionRetrierStopper != nil { |
81 | 155 | client.sessionRetrierStopper <- true | 162 | client.sessionRetrierStopper <- true |
82 | 156 | client.sessionRetrierStopper = nil | 163 | client.sessionRetrierStopper = nil |
83 | @@ -212,3 +219,21 @@ | |||
84 | 212 | urld := urldispatcher.New(client.urlDispatcherEndp, client.log) | 219 | urld := urldispatcher.New(client.urlDispatcherEndp, client.log) |
85 | 213 | return urld.DispatchURL("settings:///system/system-update") | 220 | return urld.DispatchURL("settings:///system/system-update") |
86 | 214 | } | 221 | } |
87 | 222 | |||
88 | 223 | // doLoop connects events with their handlers | ||
89 | 224 | func (client *Client) doLoop(connhandler func(bool), clickhandler, notifhandler func() error, errhandler func(error)) { | ||
90 | 225 | for { | ||
91 | 226 | select { | ||
92 | 227 | case state := <-client.connCh: | ||
93 | 228 | connhandler(state) | ||
94 | 229 | case <-client.actionsCh: | ||
95 | 230 | clickhandler() | ||
96 | 231 | case <-client.session.MsgCh: | ||
97 | 232 | notifhandler() | ||
98 | 233 | case err := <-client.session.ErrCh: | ||
99 | 234 | errhandler(err) | ||
100 | 235 | case count := <-client.sessionRetryCh: | ||
101 | 236 | client.log.Debugf("Session connected after %d attempts", count) | ||
102 | 237 | } | ||
103 | 238 | } | ||
104 | 239 | } | ||
105 | 215 | 240 | ||
106 | === modified file 'client/client_test.go' | |||
107 | --- client/client_test.go 2014-02-04 18:19:28 +0000 | |||
108 | +++ client/client_test.go 2014-02-04 18:19:28 +0000 | |||
109 | @@ -18,10 +18,12 @@ | |||
110 | 18 | 18 | ||
111 | 19 | import ( | 19 | import ( |
112 | 20 | "bytes" | 20 | "bytes" |
113 | 21 | "errors" | ||
114 | 21 | "fmt" | 22 | "fmt" |
115 | 22 | "io/ioutil" | 23 | "io/ioutil" |
116 | 23 | . "launchpad.net/gocheck" | 24 | . "launchpad.net/gocheck" |
117 | 24 | "launchpad.net/ubuntu-push/bus/networkmanager" | 25 | "launchpad.net/ubuntu-push/bus/networkmanager" |
118 | 26 | "launchpad.net/ubuntu-push/bus/notifications" | ||
119 | 25 | testibus "launchpad.net/ubuntu-push/bus/testing" | 27 | testibus "launchpad.net/ubuntu-push/bus/testing" |
120 | 26 | "launchpad.net/ubuntu-push/client/session" | 28 | "launchpad.net/ubuntu-push/client/session" |
121 | 27 | "launchpad.net/ubuntu-push/logger" | 29 | "launchpad.net/ubuntu-push/logger" |
122 | @@ -57,6 +59,7 @@ | |||
123 | 57 | 59 | ||
124 | 58 | var nullog = logger.NewSimpleLogger(ioutil.Discard, "error") | 60 | var nullog = logger.NewSimpleLogger(ioutil.Discard, "error") |
125 | 59 | var noisylog = logger.NewSimpleLogger(os.Stderr, "debug") | 61 | var noisylog = logger.NewSimpleLogger(os.Stderr, "debug") |
126 | 62 | var debuglog = noisylog | ||
127 | 60 | var _ = Suite(&clientSuite{}) | 63 | var _ = Suite(&clientSuite{}) |
128 | 61 | 64 | ||
129 | 62 | const ( | 65 | const ( |
130 | @@ -82,6 +85,7 @@ | |||
131 | 82 | } | 85 | } |
132 | 83 | 86 | ||
133 | 84 | func (cs *clientSuite) SetUpTest(c *C) { | 87 | func (cs *clientSuite) SetUpTest(c *C) { |
134 | 88 | debuglog.Debugf("---") | ||
135 | 85 | dir := c.MkDir() | 89 | dir := c.MkDir() |
136 | 86 | cs.configPath = filepath.Join(dir, "config") | 90 | cs.configPath = filepath.Join(dir, "config") |
137 | 87 | cfg := fmt.Sprintf(` | 91 | cfg := fmt.Sprintf(` |
138 | @@ -92,7 +96,8 @@ | |||
139 | 92 | "connectivity_check_md5": "", | 96 | "connectivity_check_md5": "", |
140 | 93 | "addr": ":0", | 97 | "addr": ":0", |
141 | 94 | "cert_pem_file": %#v, | 98 | "cert_pem_file": %#v, |
143 | 95 | "recheck_timeout": "3h" | 99 | "recheck_timeout": "3h", |
144 | 100 | "log_level": "debug" | ||
145 | 96 | }`, helpers.SourceRelative("../server/acceptance/config/testing.cert")) | 101 | }`, helpers.SourceRelative("../server/acceptance/config/testing.cert")) |
146 | 97 | ioutil.WriteFile(cs.configPath, []byte(cfg), 0600) | 102 | ioutil.WriteFile(cs.configPath, []byte(cfg), 0600) |
147 | 98 | } | 103 | } |
148 | @@ -205,6 +210,7 @@ | |||
149 | 205 | 210 | ||
150 | 206 | func (cs *clientSuite) TestGetDeviceIdWorks(c *C) { | 211 | func (cs *clientSuite) TestGetDeviceIdWorks(c *C) { |
151 | 207 | cli := new(Client) | 212 | cli := new(Client) |
152 | 213 | cli.log = debuglog | ||
153 | 208 | cli.idder = identifier.New() | 214 | cli.idder = identifier.New() |
154 | 209 | c.Check(cli.deviceId, Equals, "") | 215 | c.Check(cli.deviceId, Equals, "") |
155 | 210 | c.Check(cli.getDeviceId(), IsNil) | 216 | c.Check(cli.getDeviceId(), IsNil) |
156 | @@ -213,6 +219,7 @@ | |||
157 | 213 | 219 | ||
158 | 214 | func (cs *clientSuite) TestGetDeviceIdCanFail(c *C) { | 220 | func (cs *clientSuite) TestGetDeviceIdCanFail(c *C) { |
159 | 215 | cli := new(Client) | 221 | cli := new(Client) |
160 | 222 | cli.log = debuglog | ||
161 | 216 | cli.idder = idtesting.Failing() | 223 | cli.idder = idtesting.Failing() |
162 | 217 | c.Check(cli.deviceId, Equals, "") | 224 | c.Check(cli.deviceId, Equals, "") |
163 | 218 | c.Check(cli.getDeviceId(), NotNil) | 225 | c.Check(cli.getDeviceId(), NotNil) |
164 | @@ -237,8 +244,10 @@ | |||
165 | 237 | cEndp := testibus.NewTestingEndpoint(cCond, condition.Work(true), | 244 | cEndp := testibus.NewTestingEndpoint(cCond, condition.Work(true), |
166 | 238 | uint32(networkmanager.ConnectedGlobal), | 245 | uint32(networkmanager.ConnectedGlobal), |
167 | 239 | ) | 246 | ) |
168 | 247 | testibus.SetWatchTicker(cEndp, make(chan bool)) | ||
169 | 240 | // ok, create the thing | 248 | // ok, create the thing |
170 | 241 | cli := new(Client) | 249 | cli := new(Client) |
171 | 250 | cli.log = debuglog | ||
172 | 242 | err := cli.Configure(cs.configPath) | 251 | err := cli.Configure(cs.configPath) |
173 | 243 | c.Assert(err, IsNil) | 252 | c.Assert(err, IsNil) |
174 | 244 | // the user actions channel has not been set up | 253 | // the user actions channel has not been set up |
175 | @@ -267,6 +276,7 @@ | |||
176 | 267 | func (cs *clientSuite) TestTakeTheBusCanFail(c *C) { | 276 | func (cs *clientSuite) TestTakeTheBusCanFail(c *C) { |
177 | 268 | cli := new(Client) | 277 | cli := new(Client) |
178 | 269 | err := cli.Configure(cs.configPath) | 278 | err := cli.Configure(cs.configPath) |
179 | 279 | cli.log = debuglog | ||
180 | 270 | c.Assert(err, IsNil) | 280 | c.Assert(err, IsNil) |
181 | 271 | // the user actions channel has not been set up | 281 | // the user actions channel has not been set up |
182 | 272 | c.Check(cli.actionsCh, IsNil) | 282 | c.Check(cli.actionsCh, IsNil) |
183 | @@ -285,15 +295,13 @@ | |||
184 | 285 | ******************************************************************/ | 295 | ******************************************************************/ |
185 | 286 | 296 | ||
186 | 287 | func (cs *clientSuite) TestHandleErr(c *C) { | 297 | func (cs *clientSuite) TestHandleErr(c *C) { |
187 | 298 | buf := &bytes.Buffer{} | ||
188 | 288 | cli := new(Client) | 299 | cli := new(Client) |
190 | 289 | cli.log = noisylog | 300 | cli.log = logger.NewSimpleLogger(buf, "debug") |
191 | 290 | cli.initSession() | 301 | cli.initSession() |
192 | 291 | cli.hasConnectivity = true | 302 | cli.hasConnectivity = true |
198 | 292 | cli.handleErr(nil) | 303 | cli.handleErr(errors.New("bananas")) |
199 | 293 | c.Assert(cli.session, NotNil) | 304 | c.Check(buf.String(), Matches, ".*session exited.*bananas\n") |
195 | 294 | // let the session connection fail | ||
196 | 295 | time.Sleep(100 * time.Millisecond) | ||
197 | 296 | c.Check(cli.session.State(), Equals, session.Error) | ||
200 | 297 | } | 305 | } |
201 | 298 | 306 | ||
202 | 299 | /***************************************************************** | 307 | /***************************************************************** |
203 | @@ -302,19 +310,23 @@ | |||
204 | 302 | 310 | ||
205 | 303 | func (cs *clientSuite) TestHandleConnStateD2C(c *C) { | 311 | func (cs *clientSuite) TestHandleConnStateD2C(c *C) { |
206 | 304 | cli := new(Client) | 312 | cli := new(Client) |
207 | 313 | cli.log = debuglog | ||
208 | 305 | cli.initSession() | 314 | cli.initSession() |
209 | 306 | // let's pretend the client had a previous attempt at connecting still pending | 315 | // let's pretend the client had a previous attempt at connecting still pending |
210 | 307 | // (hard to trigger in real life, but possible) | 316 | // (hard to trigger in real life, but possible) |
212 | 308 | cli.sessionRetrierStopper = make(chan bool, 1) | 317 | ch := make(chan bool, 1) |
213 | 318 | cli.sessionRetrierStopper = ch | ||
214 | 309 | 319 | ||
215 | 310 | c.Assert(cli.hasConnectivity, Equals, false) | 320 | c.Assert(cli.hasConnectivity, Equals, false) |
216 | 311 | cli.handleConnState(true) | 321 | cli.handleConnState(true) |
217 | 322 | c.Check(len(ch), Equals, 1) | ||
218 | 312 | c.Check(cli.hasConnectivity, Equals, true) | 323 | c.Check(cli.hasConnectivity, Equals, true) |
219 | 313 | c.Assert(cli.session, NotNil) | 324 | c.Assert(cli.session, NotNil) |
220 | 314 | } | 325 | } |
221 | 315 | 326 | ||
222 | 316 | func (cs *clientSuite) TestHandleConnStateSame(c *C) { | 327 | func (cs *clientSuite) TestHandleConnStateSame(c *C) { |
223 | 317 | cli := new(Client) | 328 | cli := new(Client) |
224 | 329 | cli.log = debuglog | ||
225 | 318 | // here we want to check that we don't do anything | 330 | // here we want to check that we don't do anything |
226 | 319 | c.Assert(cli.session, IsNil) | 331 | c.Assert(cli.session, IsNil) |
227 | 320 | c.Assert(cli.hasConnectivity, Equals, false) | 332 | c.Assert(cli.hasConnectivity, Equals, false) |
228 | @@ -328,7 +340,8 @@ | |||
229 | 328 | 340 | ||
230 | 329 | func (cs *clientSuite) TestHandleConnStateC2D(c *C) { | 341 | func (cs *clientSuite) TestHandleConnStateC2D(c *C) { |
231 | 330 | cli := new(Client) | 342 | cli := new(Client) |
233 | 331 | cli.session, _ = session.NewSession(string(cli.config.Addr), cli.pem, cli.config.ExchangeTimeout.Duration, cli.deviceId, noisylog) | 343 | cli.log = debuglog |
234 | 344 | cli.session, _ = session.NewSession(string(cli.config.Addr), cli.pem, cli.config.ExchangeTimeout.Duration, cli.deviceId, debuglog) | ||
235 | 332 | cli.session.Dial() | 345 | cli.session.Dial() |
236 | 333 | cli.hasConnectivity = true | 346 | cli.hasConnectivity = true |
237 | 334 | 347 | ||
238 | @@ -340,7 +353,8 @@ | |||
239 | 340 | 353 | ||
240 | 341 | func (cs *clientSuite) TestHandleConnStateC2DPending(c *C) { | 354 | func (cs *clientSuite) TestHandleConnStateC2DPending(c *C) { |
241 | 342 | cli := new(Client) | 355 | cli := new(Client) |
243 | 343 | cli.session, _ = session.NewSession(string(cli.config.Addr), cli.pem, cli.config.ExchangeTimeout.Duration, cli.deviceId, noisylog) | 356 | cli.log = debuglog |
244 | 357 | cli.session, _ = session.NewSession(string(cli.config.Addr), cli.pem, cli.config.ExchangeTimeout.Duration, cli.deviceId, debuglog) | ||
245 | 344 | cli.sessionRetrierStopper = make(chan bool, 1) | 358 | cli.sessionRetrierStopper = make(chan bool, 1) |
246 | 345 | cli.hasConnectivity = true | 359 | cli.hasConnectivity = true |
247 | 346 | 360 | ||
248 | @@ -369,9 +383,9 @@ | |||
249 | 369 | 383 | ||
250 | 370 | func (cs *clientSuite) TestHandleNotificationFail(c *C) { | 384 | func (cs *clientSuite) TestHandleNotificationFail(c *C) { |
251 | 371 | cli := new(Client) | 385 | cli := new(Client) |
252 | 386 | cli.log = debuglog | ||
253 | 372 | endp := testibus.NewTestingEndpoint(nil, condition.Work(false)) | 387 | endp := testibus.NewTestingEndpoint(nil, condition.Work(false)) |
254 | 373 | cli.notificationsEndp = endp | 388 | cli.notificationsEndp = endp |
255 | 374 | cli.log = noisylog | ||
256 | 375 | c.Check(cli.handleNotification(), NotNil) | 389 | c.Check(cli.handleNotification(), NotNil) |
257 | 376 | } | 390 | } |
258 | 377 | 391 | ||
259 | @@ -381,9 +395,9 @@ | |||
260 | 381 | 395 | ||
261 | 382 | func (cs *clientSuite) TestHandleClick(c *C) { | 396 | func (cs *clientSuite) TestHandleClick(c *C) { |
262 | 383 | cli := new(Client) | 397 | cli := new(Client) |
263 | 398 | cli.log = debuglog | ||
264 | 384 | endp := testibus.NewTestingEndpoint(nil, condition.Work(true), nil) | 399 | endp := testibus.NewTestingEndpoint(nil, condition.Work(true), nil) |
265 | 385 | cli.urlDispatcherEndp = endp | 400 | cli.urlDispatcherEndp = endp |
266 | 386 | cli.log = noisylog | ||
267 | 387 | c.Check(cli.handleClick(), IsNil) | 401 | c.Check(cli.handleClick(), IsNil) |
268 | 388 | // check we sent the notification | 402 | // check we sent the notification |
269 | 389 | args := testibus.GetCallArgs(endp) | 403 | args := testibus.GetCallArgs(endp) |
270 | @@ -391,3 +405,56 @@ | |||
271 | 391 | c.Check(args[0].Member, Equals, "DispatchURL") | 405 | c.Check(args[0].Member, Equals, "DispatchURL") |
272 | 392 | c.Check(args[0].Args, DeepEquals, []interface{}{"settings:///system/system-update"}) | 406 | c.Check(args[0].Args, DeepEquals, []interface{}{"settings:///system/system-update"}) |
273 | 393 | } | 407 | } |
274 | 408 | |||
275 | 409 | /***************************************************************** | ||
276 | 410 | doLoop tests | ||
277 | 411 | ******************************************************************/ | ||
278 | 412 | |||
279 | 413 | func (cs *clientSuite) TestDoLoopConn(c *C) { | ||
280 | 414 | cli := new(Client) | ||
281 | 415 | cli.log = debuglog | ||
282 | 416 | cli.connCh = make(chan bool, 1) | ||
283 | 417 | cli.connCh <- true | ||
284 | 418 | cli.initSession() | ||
285 | 419 | |||
286 | 420 | ch := make(chan bool, 1) | ||
287 | 421 | go cli.doLoop(func(bool) { ch <- true }, func() error { return nil }, func() error { return nil }, func(error) {}) | ||
288 | 422 | c.Check(takeNextBool(ch), Equals, true) | ||
289 | 423 | } | ||
290 | 424 | |||
291 | 425 | func (cs *clientSuite) TestDoLoopClick(c *C) { | ||
292 | 426 | cli := new(Client) | ||
293 | 427 | cli.log = debuglog | ||
294 | 428 | cli.initSession() | ||
295 | 429 | aCh := make(chan notifications.RawActionReply, 1) | ||
296 | 430 | aCh <- notifications.RawActionReply{} | ||
297 | 431 | cli.actionsCh = aCh | ||
298 | 432 | |||
299 | 433 | ch := make(chan bool, 1) | ||
300 | 434 | go cli.doLoop(func(bool) {}, func() error { ch <- true; return nil }, func() error { return nil }, func(error) {}) | ||
301 | 435 | c.Check(takeNextBool(ch), Equals, true) | ||
302 | 436 | } | ||
303 | 437 | |||
304 | 438 | func (cs *clientSuite) TestDoLoopNotif(c *C) { | ||
305 | 439 | cli := new(Client) | ||
306 | 440 | cli.log = debuglog | ||
307 | 441 | cli.initSession() | ||
308 | 442 | cli.session.MsgCh = make(chan *session.Notification, 1) | ||
309 | 443 | cli.session.MsgCh <- &session.Notification{} | ||
310 | 444 | |||
311 | 445 | ch := make(chan bool, 1) | ||
312 | 446 | go cli.doLoop(func(bool) {}, func() error { return nil }, func() error { ch <- true; return nil }, func(error) {}) | ||
313 | 447 | c.Check(takeNextBool(ch), Equals, true) | ||
314 | 448 | } | ||
315 | 449 | |||
316 | 450 | func (cs *clientSuite) TestDoLoopErr(c *C) { | ||
317 | 451 | cli := new(Client) | ||
318 | 452 | cli.log = debuglog | ||
319 | 453 | cli.initSession() | ||
320 | 454 | cli.session.ErrCh = make(chan error, 1) | ||
321 | 455 | cli.session.ErrCh <- nil | ||
322 | 456 | |||
323 | 457 | ch := make(chan bool, 1) | ||
324 | 458 | go cli.doLoop(func(bool) {}, func() error { return nil }, func() error { return nil }, func(error) { ch <- true }) | ||
325 | 459 | c.Check(takeNextBool(ch), Equals, true) | ||
326 | 460 | } | ||
327 | 394 | 461 | ||
328 | === modified file 'client/session/session.go' | |||
329 | --- client/session/session.go 2014-02-04 13:02:38 +0000 | |||
330 | +++ client/session/session.go 2014-02-04 18:19:28 +0000 | |||
331 | @@ -147,14 +147,17 @@ | |||
332 | 147 | err := sess.proto.WriteMessage(protocol.AckMsg{"ack"}) | 147 | err := sess.proto.WriteMessage(protocol.AckMsg{"ack"}) |
333 | 148 | if err != nil { | 148 | if err != nil { |
334 | 149 | sess.setState(Error) | 149 | sess.setState(Error) |
335 | 150 | sess.Log.Errorf("unable to ack broadcast: %s", err) | ||
336 | 150 | return err | 151 | return err |
337 | 151 | } | 152 | } |
338 | 152 | sess.Log.Debugf("broadcast chan:%v app:%v topLevel:%d payloads:%s", | 153 | sess.Log.Debugf("broadcast chan:%v app:%v topLevel:%d payloads:%s", |
339 | 153 | bcast.ChanId, bcast.AppId, bcast.TopLevel, bcast.Payloads) | 154 | bcast.ChanId, bcast.AppId, bcast.TopLevel, bcast.Payloads) |
340 | 154 | if bcast.ChanId == protocol.SystemChannelId { | 155 | if bcast.ChanId == protocol.SystemChannelId { |
341 | 155 | // the system channel id, the only one we care about for now | 156 | // the system channel id, the only one we care about for now |
342 | 157 | sess.Log.Debugf("sending it over") | ||
343 | 156 | sess.Levels.Set(bcast.ChanId, bcast.TopLevel) | 158 | sess.Levels.Set(bcast.ChanId, bcast.TopLevel) |
344 | 157 | sess.MsgCh <- &Notification{} | 159 | sess.MsgCh <- &Notification{} |
345 | 160 | sess.Log.Debugf("sent it over") | ||
346 | 158 | } else { | 161 | } else { |
347 | 159 | sess.Log.Debugf("what is this weird channel, %#v?", bcast.ChanId) | 162 | sess.Log.Debugf("what is this weird channel, %#v?", bcast.ChanId) |
348 | 160 | } | 163 | } |
349 | @@ -192,6 +195,7 @@ | |||
350 | 192 | err := conn.SetDeadline(time.Now().Add(sess.ExchangeTimeout)) | 195 | err := conn.SetDeadline(time.Now().Add(sess.ExchangeTimeout)) |
351 | 193 | if err != nil { | 196 | if err != nil { |
352 | 194 | sess.setState(Error) | 197 | sess.setState(Error) |
353 | 198 | sess.Log.Errorf("unable to start: set deadline: %s", err) | ||
354 | 195 | return err | 199 | return err |
355 | 196 | } | 200 | } |
356 | 197 | _, err = conn.Write(wireVersionBytes) | 201 | _, err = conn.Write(wireVersionBytes) |
357 | @@ -199,6 +203,7 @@ | |||
358 | 199 | // n < len(p). So, no need to check number of bytes written, hooray. | 203 | // n < len(p). So, no need to check number of bytes written, hooray. |
359 | 200 | if err != nil { | 204 | if err != nil { |
360 | 201 | sess.setState(Error) | 205 | sess.setState(Error) |
361 | 206 | sess.Log.Errorf("unable to start: write version: %s", err) | ||
362 | 202 | return err | 207 | return err |
363 | 203 | } | 208 | } |
364 | 204 | proto := sess.Protocolator(conn) | 209 | proto := sess.Protocolator(conn) |
365 | @@ -210,12 +215,14 @@ | |||
366 | 210 | }) | 215 | }) |
367 | 211 | if err != nil { | 216 | if err != nil { |
368 | 212 | sess.setState(Error) | 217 | sess.setState(Error) |
369 | 218 | sess.Log.Errorf("unable to start: connect: %s", err) | ||
370 | 213 | return err | 219 | return err |
371 | 214 | } | 220 | } |
372 | 215 | var connAck protocol.ConnAckMsg | 221 | var connAck protocol.ConnAckMsg |
373 | 216 | err = proto.ReadMessage(&connAck) | 222 | err = proto.ReadMessage(&connAck) |
374 | 217 | if err != nil { | 223 | if err != nil { |
375 | 218 | sess.setState(Error) | 224 | sess.setState(Error) |
376 | 225 | sess.Log.Errorf("unable to start: connack: %s", err) | ||
377 | 219 | return err | 226 | return err |
378 | 220 | } | 227 | } |
379 | 221 | if connAck.Type != "connack" { | 228 | if connAck.Type != "connack" { |
380 | @@ -225,6 +232,7 @@ | |||
381 | 225 | pingInterval, err := time.ParseDuration(connAck.Params.PingInterval) | 232 | pingInterval, err := time.ParseDuration(connAck.Params.PingInterval) |
382 | 226 | if err != nil { | 233 | if err != nil { |
383 | 227 | sess.setState(Error) | 234 | sess.setState(Error) |
384 | 235 | sess.Log.Errorf("unable to start: parse ping interval: %s", err) | ||
385 | 228 | return err | 236 | return err |
386 | 229 | } | 237 | } |
387 | 230 | sess.proto = proto | 238 | sess.proto = proto |
ok but land with todo markers
=== modified file 'client/client.go'
--- client/client.go 2014-02-04 13:10:18 +0000
+++ client/client.go 2014-02-04 17:34:01 +0000
@@ -142,6 +142,7 @@
// connectSession kicks off the session connection dance sessionRetrierS topper != nil { sessionRetrierS topper <- true sessionRetrierS topper = nil
func (client *Client) connectSession() {
+ // xxx lp:1276199
if client.
client.
client.
@@ -156,6 +157,7 @@
// disconnectSession disconnects the session sessionRetrierS topper != nil { sessionRetrierS topper <- true
func (client *Client) disconnectSession() {
+ // xxx lp:1276199
if client.
client.