Merge lp:~chipaca/ubuntu-push/clear-cookie-to-reset-cookie into lp:ubuntu-push/automatic

Proposed by John Lenton
Status: Merged
Approved by: John Lenton
Approved revision: 383
Merged at revision: 378
Proposed branch: lp:~chipaca/ubuntu-push/clear-cookie-to-reset-cookie
Merge into: lp:ubuntu-push/automatic
Prerequisite: lp:~chipaca/ubuntu-push/remove-handleconnstate
Diff against target: 197 lines (+40/-28)
4 files modified
client/client.go (+1/-9)
client/client_test.go (+2/-2)
client/session/session.go (+11/-9)
client/session/session_test.go (+26/-8)
To merge this branch: bzr merge lp:~chipaca/ubuntu-push/clear-cookie-to-reset-cookie
Reviewer Review Type Date Requested Status
Samuele Pedroni Approve
Review via email: mp+253181@code.launchpad.net

Commit message

Move ClearCookie to ResetCookie, which also closes the connection. Call ResetCookie directly instead of through handleAccountsChange; nuke the latter.

To post a comment you must log in.
Revision history for this message
Samuele Pedroni (pedronis) :
review: Approve
Revision history for this message
Samuele Pedroni (pedronis) :
review: Approve
Revision history for this message
Samuele Pedroni (pedronis) :
review: Needs Information
Revision history for this message
Samuele Pedroni (pedronis) :
Revision history for this message
John Lenton (chipaca) wrote :

You're correct once again; fixed!

Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Revision history for this message
Samuele Pedroni (pedronis) wrote :

+1 wip

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 2015-03-17 16:54:36 +0000
3+++ client/client.go 2015-03-17 16:54:36 +0000
4@@ -452,14 +452,6 @@
5 return nil
6 }
7
8-// handleAccountsChange deals with the user adding or removing (or
9-// changing) the u1 account used to auth
10-func (client *PushClient) handleAccountsChange() {
11- client.log.Infof("U1 account changed; restarting session")
12- client.session.ClearCookie()
13- client.session.Close()
14-}
15-
16 // doLoop connects events with their handlers
17 func (client *PushClient) doLoop(connhandler func(bool), bcasthandler func(*session.BroadcastNotification) error, ucasthandler func(session.AddressedNotification) error, unregisterhandler func(*click.AppId), accountshandler func()) {
18 for {
19@@ -497,7 +489,7 @@
20 client.handleBroadcastNotification,
21 client.handleUnicastNotification,
22 client.handleUnregister,
23- client.handleAccountsChange,
24+ client.session.ResetCookie,
25 )
26 }
27
28
29=== modified file 'client/client_test.go'
30--- client/client_test.go 2015-03-17 16:54:36 +0000
31+++ client/client_test.go 2015-03-17 16:54:36 +0000
32@@ -530,7 +530,7 @@
33 type derivePollerSession struct{}
34
35 func (s *derivePollerSession) Close() {}
36-func (s *derivePollerSession) ClearCookie() {}
37+func (s *derivePollerSession) ResetCookie() {}
38 func (s *derivePollerSession) State() session.ClientSessionState { return session.Unknown }
39 func (s *derivePollerSession) HasConnectivity(bool) {}
40 func (s *derivePollerSession) KeepConnection() error { return nil }
41@@ -1060,7 +1060,7 @@
42 type loopSession struct{ hasConn bool }
43
44 func (s *loopSession) Close() {}
45-func (s *loopSession) ClearCookie() {}
46+func (s *loopSession) ResetCookie() {}
47 func (s *loopSession) State() session.ClientSessionState {
48 if s.hasConn {
49 return session.Connected
50
51=== modified file 'client/session/session.go'
52--- client/session/session.go 2015-03-17 16:54:36 +0000
53+++ client/session/session.go 2015-03-17 16:54:36 +0000
54@@ -129,7 +129,7 @@
55 // ClientSession holds a client<->server session and its configuration.
56 type ClientSession interface {
57 Close()
58- ClearCookie()
59+ ResetCookie()
60 State() ClientSessionState
61 HasConnectivity(bool)
62 KeepConnection() error
63@@ -302,10 +302,9 @@
64 return sess.cookie
65 }
66
67-func (sess *clientSession) ClearCookie() {
68- sess.connLock.Lock()
69- defer sess.connLock.Unlock()
70- sess.cookie = ""
71+func (sess *clientSession) ResetCookie() {
72+ sess.stopRedial()
73+ sess.doClose(true)
74 }
75
76 // getHosts sets deliveryHosts possibly querying a remote endpoint
77@@ -439,12 +438,15 @@
78
79 func (sess *clientSession) Close() {
80 sess.stopRedial()
81- sess.doClose()
82+ sess.doClose(false)
83 }
84
85-func (sess *clientSession) doClose() {
86+func (sess *clientSession) doClose(resetCookie bool) {
87 sess.connLock.Lock()
88 defer sess.connLock.Unlock()
89+ if resetCookie {
90+ sess.cookie = ""
91+ }
92 if sess.Connection != nil {
93 sess.Connection.Close()
94 // we ignore Close errors, on purpose (the thinking being that
95@@ -673,8 +675,8 @@
96
97 // run calls connect, and if it works it calls start, and if it works
98 // it runs loop in a goroutine, and ships its return value over ErrCh.
99-func (sess *clientSession) run(closer func(), authChecker, hostGetter, connecter, starter, looper func() error) error {
100- closer()
101+func (sess *clientSession) run(closer func(bool), authChecker, hostGetter, connecter, starter, looper func() error) error {
102+ closer(false)
103 if err := authChecker(); err != nil {
104 return err
105 }
106
107=== modified file 'client/session/session_test.go'
108--- client/session/session_test.go 2015-03-12 12:01:13 +0000
109+++ client/session/session_test.go 2015-03-17 16:54:36 +0000
110@@ -1371,13 +1371,31 @@
111 run() tests
112 ****************************************************************/
113
114+func (cs *clientSessionSuite) TestRunCallsCloserWithFalse(c *C) {
115+ sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
116+ c.Assert(err, IsNil)
117+ failure := errors.New("bail")
118+ has_closed := false
119+ with_false := false
120+ err = sess.run(
121+ func(b bool) { has_closed = true; with_false = !b },
122+ func() error { return failure },
123+ nil,
124+ nil,
125+ nil,
126+ nil)
127+ c.Check(err, Equals, failure)
128+ c.Check(has_closed, Equals, true)
129+ c.Check(with_false, Equals, true)
130+}
131+
132 func (cs *clientSessionSuite) TestRunBailsIfAuthCheckFails(c *C) {
133 sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
134 c.Assert(err, IsNil)
135 failure := errors.New("TestRunBailsIfAuthCheckFails")
136 has_closed := false
137 err = sess.run(
138- func() { has_closed = true },
139+ func(bool) { has_closed = true },
140 func() error { return failure },
141 nil,
142 nil,
143@@ -1393,7 +1411,7 @@
144 failure := errors.New("TestRunBailsIfHostGetterFails")
145 has_closed := false
146 err = sess.run(
147- func() { has_closed = true },
148+ func(bool) { has_closed = true },
149 func() error { return nil },
150 func() error { return failure },
151 nil,
152@@ -1408,7 +1426,7 @@
153 c.Assert(err, IsNil)
154 failure := errors.New("TestRunBailsIfConnectFails")
155 err = sess.run(
156- func() {},
157+ func(bool) {},
158 func() error { return nil },
159 func() error { return nil },
160 func() error { return failure },
161@@ -1422,7 +1440,7 @@
162 c.Assert(err, IsNil)
163 failure := errors.New("TestRunBailsIfStartFails")
164 err = sess.run(
165- func() {},
166+ func(bool) {},
167 func() error { return nil },
168 func() error { return nil },
169 func() error { return nil },
170@@ -1437,7 +1455,7 @@
171 failureCh := make(chan error) // must be unbuffered
172 notf := &BroadcastNotification{}
173 err = sess.run(
174- func() {},
175+ func(bool) {},
176 func() error { return nil },
177 func() error { return nil },
178 func() error { return nil },
179@@ -1710,16 +1728,16 @@
180 }
181
182 /****************************************************************
183- ClearCookie() tests
184+ ResetCookie() tests
185 ****************************************************************/
186
187-func (cs *clientSessionSuite) TestClearCookie(c *C) {
188+func (cs *clientSessionSuite) TestResetCookie(c *C) {
189 sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
190 c.Assert(err, IsNil)
191 c.Check(sess.getCookie(), Equals, "")
192 sess.setCookie("COOKIE")
193 c.Check(sess.getCookie(), Equals, "COOKIE")
194- sess.ClearCookie()
195+ sess.ResetCookie()
196 c.Check(sess.getCookie(), Equals, "")
197 }
198

Subscribers

People subscribed via source and target branches