Ubuntu Push Notifications

Merge lp:~chipaca/ubuntu-push/client-constructor into lp:ubuntu-push

  1. client-constructor
  2. Merge into trunk
Proposed by John Lenton
Status: Merged
Approved by: John Lenton
Approved revision: 60
Merged at revision: 62
Proposed branch: lp:~chipaca/ubuntu-push/client-constructor
Merge into: lp:ubuntu-push
Diff against target: 472 lines (+77/-65)
2 files modified
client/client.go (+28/-18)
client/client_test.go (+49/-47)
To merge this branch: bzr merge lp:~chipaca/ubuntu-push/client-constructor
Reviewer Review Type Date Requested Status
Samuele Pedroni Approve
Review via email: mp+205469@code.launchpad.net

Commit message

Gave client a constructor, moved setting config file to there.

Description of the change

This gives the push client a constructor, and moves any parameters to
it. This is because I'm going to have to specify the path to the
levels database shortly, and passing it to Start() (and from there
into configure) doesn't make much sense.

To post a comment you must log in.
Revision history for this message
John Lenton (chipaca) wrote :

The other way of doing it would be to leave the client interface as is, and add the ability to config to do substitutions (so that the database path can be specified as "$xdg_data_dir/u-p-c/levels.db" or somesuch), or have a special value such as "default". I don't like these alternatives much, but maybe there's one I'm missing; the con of this is that it'll lead to more mixing private and public attributes in the session config.

lp:~chipaca/ubuntu-push/client-constructor updated
60. By John Lenton

merged unborking of pem checks

Revision history for this message
Samuele Pedroni (pedronis) wrote :

20 +type pushClient struct {

so go let's you keep it pacakge private but use the result of NewClient:

https://groups.google.com/forum/#!topic/golang-nuts/UpxU4rwM0OQ

I don't think it's a case where that makes a lot of sense though, I would make it PushClient with unexported fields

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'client/client.go'
2--- client/client.go 2014-02-05 16:56:09 +0000
3+++ client/client.go 2014-02-08 18:19:14 +0000
4@@ -39,7 +39,7 @@
5 // ClientConfig holds the client configuration
6 type ClientConfig struct {
7 connectivity.ConnectivityConfig // q.v.
8- // A reasonably larg timeout for receive/answer pairs
9+ // A reasonably large timeout for receive/answer pairs
10 ExchangeTimeout config.ConfigTimeDuration `json:"exchange_timeout"`
11 // The server to connect to
12 Addr config.ConfigHostPort
13@@ -49,8 +49,9 @@
14 LogLevel string `json:"log_level"`
15 }
16
17-// Client is the Ubuntu Push Notifications client-side daemon.
18-type Client struct {
19+// pushClient is the Ubuntu Push Notifications client-side daemon.
20+type pushClient struct {
21+ configPath string
22 config ClientConfig
23 log logger.Logger
24 pem []byte
25@@ -66,9 +67,18 @@
26 sessionConnectedCh chan uint32
27 }
28
29-// configure loads the configuration specified in configPath, and sets it up.
30-func (client *Client) configure(configPath string) error {
31- f, err := os.Open(configPath)
32+// Creates a new Ubuntu Push Notifications client-side daemon that will use
33+// the given configuration file.
34+func NewPushClient(configPath string) *pushClient {
35+ client := new(pushClient)
36+ client.configPath = configPath
37+
38+ return client
39+}
40+
41+// configure loads its configuration, and sets it up.
42+func (client *pushClient) configure() error {
43+ f, err := os.Open(client.configPath)
44 if err != nil {
45 return fmt.Errorf("opening config: %v", err)
46 }
47@@ -104,7 +114,7 @@
48 }
49
50 // getDeviceId gets the whoopsie identifier for the device
51-func (client *Client) getDeviceId() error {
52+func (client *pushClient) getDeviceId() error {
53 err := client.idder.Generate()
54 if err != nil {
55 return err
56@@ -114,7 +124,7 @@
57 }
58
59 // takeTheBus starts the connection(s) to D-Bus and sets up associated event channels
60-func (client *Client) takeTheBus() error {
61+func (client *pushClient) takeTheBus() error {
62 go connectivity.ConnectedState(client.connectivityEndp,
63 client.config.ConnectivityConfig, client.log, client.connCh)
64 iniCh := make(chan uint32)
65@@ -129,7 +139,7 @@
66 }
67
68 // initSession creates the session object
69-func (client *Client) initSession() error {
70+func (client *pushClient) initSession() error {
71 sess, err := session.NewSession(string(client.config.Addr), client.pem,
72 client.config.ExchangeTimeout.Duration, client.deviceId, client.log)
73 if err != nil {
74@@ -140,7 +150,7 @@
75 }
76
77 // handleConnState deals with connectivity events
78-func (client *Client) handleConnState(hasConnectivity bool) {
79+func (client *pushClient) handleConnState(hasConnectivity bool) {
80 if client.hasConnectivity == hasConnectivity {
81 // nothing to do!
82 return
83@@ -154,7 +164,7 @@
84 }
85
86 // handleErr deals with the session erroring out of its loop
87-func (client *Client) handleErr(err error) {
88+func (client *pushClient) handleErr(err error) {
89 // if we're not connected, we don't really care
90 client.log.Errorf("session exited: %s", err)
91 if client.hasConnectivity {
92@@ -163,7 +173,7 @@
93 }
94
95 // handleNotification deals with receiving a notification
96-func (client *Client) handleNotification() error {
97+func (client *pushClient) handleNotification() error {
98 action_id := "dummy_id"
99 a := []string{action_id, "Go get it!"} // action value not visible on the phone
100 h := map[string]*dbus.Variant{"x-canonical-switch-to-application": &dbus.Variant{true}}
101@@ -187,14 +197,14 @@
102 }
103
104 // handleClick deals with the user clicking a notification
105-func (client *Client) handleClick() error {
106+func (client *pushClient) handleClick() error {
107 // it doesn't get much simpler...
108 urld := urldispatcher.New(client.urlDispatcherEndp, client.log)
109 return urld.DispatchURL("settings:///system/system-update")
110 }
111
112 // doLoop connects events with their handlers
113-func (client *Client) doLoop(connhandler func(bool), clickhandler, notifhandler func() error, errhandler func(error)) {
114+func (client *pushClient) doLoop(connhandler func(bool), clickhandler, notifhandler func() error, errhandler func(error)) {
115 for {
116 select {
117 case state := <-client.connCh:
118@@ -213,7 +223,7 @@
119
120 // doStart calls each of its arguments in order, returning the first non-nil
121 // error (or nil at the end)
122-func (client *Client) doStart(fs ...func() error) error {
123+func (client *pushClient) doStart(fs ...func() error) error {
124 for _, f := range fs {
125 if err := f(); err != nil {
126 return err
127@@ -223,15 +233,15 @@
128 }
129
130 // Loop calls doLoop with the "real" handlers
131-func (client *Client) Loop() {
132+func (client *pushClient) Loop() {
133 client.doLoop(client.handleConnState, client.handleClick,
134 client.handleNotification, client.handleErr)
135 }
136
137 // Start calls doStart with the "real" starters
138-func (client *Client) Start(configPath string) error {
139+func (client *pushClient) Start() error {
140 return client.doStart(
141- func() error { return client.configure(configPath) },
142+ client.configure,
143 client.getDeviceId,
144 client.initSession,
145 client.takeTheBus,
146
147=== modified file 'client/client_test.go'
148--- client/client_test.go 2014-02-06 13:11:32 +0000
149+++ client/client_test.go 2014-02-08 18:19:14 +0000
150@@ -103,43 +103,43 @@
151 ******************************************************************/
152
153 func (cs *clientSuite) TestConfigureWorks(c *C) {
154- cli := new(Client)
155- err := cli.configure(cs.configPath)
156+ cli := NewPushClient(cs.configPath)
157+ err := cli.configure()
158 c.Assert(err, IsNil)
159 c.Assert(cli.config, NotNil)
160 c.Check(cli.config.ExchangeTimeout.Duration, Equals, time.Duration(10*time.Millisecond))
161 }
162
163 func (cs *clientSuite) TestConfigureSetsUpLog(c *C) {
164- cli := new(Client)
165+ cli := NewPushClient(cs.configPath)
166 c.Check(cli.log, IsNil)
167- err := cli.configure(cs.configPath)
168+ err := cli.configure()
169 c.Assert(err, IsNil)
170 c.Assert(cli.log, NotNil)
171 }
172
173 func (cs *clientSuite) TestConfigureSetsUpPEM(c *C) {
174- cli := new(Client)
175+ cli := NewPushClient(cs.configPath)
176 c.Check(cli.pem, IsNil)
177- err := cli.configure(cs.configPath)
178+ err := cli.configure()
179 c.Assert(err, IsNil)
180 c.Assert(cli.pem, NotNil)
181 }
182
183 func (cs *clientSuite) TestConfigureSetsUpIdder(c *C) {
184- cli := new(Client)
185+ cli := NewPushClient(cs.configPath)
186 c.Check(cli.idder, IsNil)
187- err := cli.configure(cs.configPath)
188+ err := cli.configure()
189 c.Assert(err, IsNil)
190 c.Assert(cli.idder, DeepEquals, identifier.New())
191 }
192
193 func (cs *clientSuite) TestConfigureSetsUpEndpoints(c *C) {
194- cli := new(Client)
195+ cli := NewPushClient(cs.configPath)
196 c.Check(cli.notificationsEndp, IsNil)
197 c.Check(cli.urlDispatcherEndp, IsNil)
198 c.Check(cli.connectivityEndp, IsNil)
199- err := cli.configure(cs.configPath)
200+ err := cli.configure()
201 c.Assert(err, IsNil)
202 c.Assert(cli.notificationsEndp, NotNil)
203 c.Assert(cli.urlDispatcherEndp, NotNil)
204@@ -147,22 +147,22 @@
205 }
206
207 func (cs *clientSuite) TestConfigureSetsUpConnCh(c *C) {
208- cli := new(Client)
209+ cli := NewPushClient(cs.configPath)
210 c.Check(cli.connCh, IsNil)
211- err := cli.configure(cs.configPath)
212+ err := cli.configure()
213 c.Assert(err, IsNil)
214 c.Assert(cli.connCh, NotNil)
215 }
216
217 func (cs *clientSuite) TestConfigureBailsOnBadFilename(c *C) {
218- cli := new(Client)
219- err := cli.configure("/does/not/exist")
220+ cli := NewPushClient("/does/not/exist")
221+ err := cli.configure()
222 c.Assert(err, NotNil)
223 }
224
225 func (cs *clientSuite) TestConfigureBailsOnBadConfig(c *C) {
226- cli := new(Client)
227- err := cli.configure("/etc/passwd")
228+ cli := NewPushClient("/etc/passwd")
229+ err := cli.configure()
230 c.Assert(err, NotNil)
231 }
232
233@@ -175,12 +175,13 @@
234 "connectivity_check_md5": "",
235 "addr": ":0",
236 "cert_pem_file": "/a/b/c",
237+ "log_level": "debug",
238 "recheck_timeout": "3h"
239 }`), 0600)
240
241- cli := new(Client)
242- err := cli.configure(cs.configPath)
243- c.Assert(err, NotNil)
244+ cli := NewPushClient(cs.configPath)
245+ err := cli.configure()
246+ c.Assert(err, ErrorMatches, "reading PEM file: .*")
247 }
248
249 func (cs *clientSuite) TestConfigureBailsOnBadPEM(c *C) {
250@@ -192,12 +193,13 @@
251 "connectivity_check_md5": "",
252 "addr": ":0",
253 "cert_pem_file": "/etc/passwd",
254+ "log_level": "debug",
255 "recheck_timeout": "3h"
256 }`), 0600)
257
258- cli := new(Client)
259- err := cli.configure(cs.configPath)
260- c.Assert(err, NotNil)
261+ cli := NewPushClient(cs.configPath)
262+ err := cli.configure()
263+ c.Assert(err, ErrorMatches, "no PEM found.*")
264 }
265
266 /*****************************************************************
267@@ -205,7 +207,7 @@
268 ******************************************************************/
269
270 func (cs *clientSuite) TestGetDeviceIdWorks(c *C) {
271- cli := new(Client)
272+ cli := NewPushClient(cs.configPath)
273 cli.log = cs.log
274 cli.idder = identifier.New()
275 c.Check(cli.deviceId, Equals, "")
276@@ -214,7 +216,7 @@
277 }
278
279 func (cs *clientSuite) TestGetDeviceIdCanFail(c *C) {
280- cli := new(Client)
281+ cli := NewPushClient(cs.configPath)
282 cli.log = cs.log
283 cli.idder = idtesting.Failing()
284 c.Check(cli.deviceId, Equals, "")
285@@ -242,9 +244,9 @@
286 )
287 testibus.SetWatchTicker(cEndp, make(chan bool))
288 // ok, create the thing
289- cli := new(Client)
290+ cli := NewPushClient(cs.configPath)
291 cli.log = cs.log
292- err := cli.configure(cs.configPath)
293+ err := cli.configure()
294 c.Assert(err, IsNil)
295 // the user actions channel has not been set up
296 c.Check(cli.actionsCh, IsNil)
297@@ -270,8 +272,8 @@
298
299 // takeTheBus can, in fact, fail
300 func (cs *clientSuite) TestTakeTheBusCanFail(c *C) {
301- cli := new(Client)
302- err := cli.configure(cs.configPath)
303+ cli := NewPushClient(cs.configPath)
304+ err := cli.configure()
305 cli.log = cs.log
306 c.Assert(err, IsNil)
307 // the user actions channel has not been set up
308@@ -291,7 +293,7 @@
309 ******************************************************************/
310
311 func (cs *clientSuite) TestHandleErr(c *C) {
312- cli := new(Client)
313+ cli := NewPushClient(cs.configPath)
314 cli.log = cs.log
315 c.Assert(cli.initSession(), IsNil)
316 cli.hasConnectivity = true
317@@ -304,7 +306,7 @@
318 ******************************************************************/
319
320 func (cs *clientSuite) TestHandleConnStateD2C(c *C) {
321- cli := new(Client)
322+ cli := NewPushClient(cs.configPath)
323 cli.log = cs.log
324 c.Assert(cli.initSession(), IsNil)
325
326@@ -315,7 +317,7 @@
327 }
328
329 func (cs *clientSuite) TestHandleConnStateSame(c *C) {
330- cli := new(Client)
331+ cli := NewPushClient(cs.configPath)
332 cli.log = cs.log
333 // here we want to check that we don't do anything
334 c.Assert(cli.session, IsNil)
335@@ -329,7 +331,7 @@
336 }
337
338 func (cs *clientSuite) TestHandleConnStateC2D(c *C) {
339- cli := new(Client)
340+ cli := NewPushClient(cs.configPath)
341 cli.log = cs.log
342 cli.session, _ = session.NewSession(string(cli.config.Addr), cli.pem, cli.config.ExchangeTimeout.Duration, cli.deviceId, cs.log)
343 cli.session.Dial()
344@@ -342,7 +344,7 @@
345 }
346
347 func (cs *clientSuite) TestHandleConnStateC2DPending(c *C) {
348- cli := new(Client)
349+ cli := NewPushClient(cs.configPath)
350 cli.log = cs.log
351 cli.session, _ = session.NewSession(string(cli.config.Addr), cli.pem, cli.config.ExchangeTimeout.Duration, cli.deviceId, cs.log)
352 cli.hasConnectivity = true
353@@ -356,7 +358,7 @@
354 ******************************************************************/
355
356 func (cs *clientSuite) TestHandleNotification(c *C) {
357- cli := new(Client)
358+ cli := NewPushClient(cs.configPath)
359 endp := testibus.NewTestingEndpoint(nil, condition.Work(true), uint32(1))
360 cli.notificationsEndp = endp
361 cli.log = cs.log
362@@ -369,7 +371,7 @@
363 }
364
365 func (cs *clientSuite) TestHandleNotificationFail(c *C) {
366- cli := new(Client)
367+ cli := NewPushClient(cs.configPath)
368 cli.log = cs.log
369 endp := testibus.NewTestingEndpoint(nil, condition.Work(false))
370 cli.notificationsEndp = endp
371@@ -381,7 +383,7 @@
372 ******************************************************************/
373
374 func (cs *clientSuite) TestHandleClick(c *C) {
375- cli := new(Client)
376+ cli := NewPushClient(cs.configPath)
377 cli.log = cs.log
378 endp := testibus.NewTestingEndpoint(nil, condition.Work(true), nil)
379 cli.urlDispatcherEndp = endp
380@@ -398,7 +400,7 @@
381 ******************************************************************/
382
383 func (cs *clientSuite) TestDoLoopConn(c *C) {
384- cli := new(Client)
385+ cli := NewPushClient(cs.configPath)
386 cli.log = cs.log
387 cli.connCh = make(chan bool, 1)
388 cli.connCh <- true
389@@ -410,7 +412,7 @@
390 }
391
392 func (cs *clientSuite) TestDoLoopClick(c *C) {
393- cli := new(Client)
394+ cli := NewPushClient(cs.configPath)
395 cli.log = cs.log
396 c.Assert(cli.initSession(), IsNil)
397 aCh := make(chan notifications.RawActionReply, 1)
398@@ -423,7 +425,7 @@
399 }
400
401 func (cs *clientSuite) TestDoLoopNotif(c *C) {
402- cli := new(Client)
403+ cli := NewPushClient(cs.configPath)
404 cli.log = cs.log
405 c.Assert(cli.initSession(), IsNil)
406 cli.session.MsgCh = make(chan *session.Notification, 1)
407@@ -435,7 +437,7 @@
408 }
409
410 func (cs *clientSuite) TestDoLoopErr(c *C) {
411- cli := new(Client)
412+ cli := NewPushClient(cs.configPath)
413 cli.log = cs.log
414 c.Assert(cli.initSession(), IsNil)
415 cli.session.ErrCh = make(chan error, 1)
416@@ -451,7 +453,7 @@
417 ******************************************************************/
418
419 func (cs *clientSuite) TestDoStartWorks(c *C) {
420- cli := new(Client)
421+ cli := NewPushClient(cs.configPath)
422 one_called := false
423 two_called := false
424 one := func() error { one_called = true; return nil }
425@@ -462,7 +464,7 @@
426 }
427
428 func (cs *clientSuite) TestDoStartFailsAsExpected(c *C) {
429- cli := new(Client)
430+ cli := NewPushClient(cs.configPath)
431 one_called := false
432 two_called := false
433 failure := errors.New("Failure")
434@@ -478,7 +480,7 @@
435 ******************************************************************/
436
437 func (cs *clientSuite) TestLoop(c *C) {
438- cli := new(Client)
439+ cli := NewPushClient(cs.configPath)
440 cli.connCh = make(chan bool)
441 cli.sessionConnectedCh = make(chan uint32)
442 aCh := make(chan notifications.RawActionReply, 1)
443@@ -556,7 +558,7 @@
444 c.Skip("no dbus")
445 }
446
447- cli := new(Client)
448+ cli := NewPushClient(cs.configPath)
449 // before start, everything sucks:
450 // no config,
451 c.Check(string(cli.config.Addr), Equals, "")
452@@ -569,7 +571,7 @@
453 // no nuthin'.
454
455 // so we start,
456- err := cli.Start(cs.configPath)
457+ err := cli.Start()
458 // and it works
459 c.Check(err, IsNil)
460
461@@ -585,9 +587,9 @@
462 }
463
464 func (cs *clientSuite) TestStartCanFail(c *C) {
465- cli := new(Client)
466+ cli := NewPushClient("/does/not/exist")
467 // easiest way for it to fail is to feed it a bad config
468- err := cli.Start("/does/not/exist")
469+ err := cli.Start()
470 // and it works. Err. Doesn't.
471 c.Check(err, NotNil)
472 }

Subscribers

People subscribed via source and target branches