Merge lp:~chipaca/ubuntu-push/session-refactor-002 into lp:ubuntu-push/automatic

Proposed by John Lenton on 2015-02-12
Status: Work in progress
Proposed branch: lp:~chipaca/ubuntu-push/session-refactor-002
Merge into: lp:ubuntu-push/automatic
Prerequisite: lp:~chipaca/ubuntu-push/session-refactor-001
Diff against target: 209 lines (+130/-7)
3 files modified
client/client.go (+7/-3)
client/client_test.go (+5/-3)
client/session/session.go (+118/-1)
To merge this branch: bzr merge lp:~chipaca/ubuntu-push/session-refactor-002
Reviewer Review Type Date Requested Status
Ubuntu Push Hackers 2015-02-12 Pending
Review via email: mp+249500@code.launchpad.net

This proposal supersedes a proposal from 2015-02-12.

To post a comment you must log in.

Unmerged revisions

367. By John Lenton on 2015-02-12

tmp checking; does not work

366. By John Lenton on 2015-02-06

protect the chs with a lock. any lock will do.

365. By John Lenton on 2015-02-06

added SetChForTesting, for testing

364. By John Lenton on 2015-02-06

first step

363. By John Lenton on 2015-02-05

Merged explicit-check-for-oversize-webcheck-response-bodies into session-refactor.

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-02-12 14:49:15 +0000
3+++ client/client.go 2015-02-12 14:49:16 +0000
4@@ -478,16 +478,20 @@
5 // doLoop connects events with their handlers
6 func (client *PushClient) doLoop(connhandler func(bool), bcasthandler func(*session.BroadcastNotification) error, ucasthandler func(session.AddressedNotification) error, errhandler func(error), unregisterhandler func(*click.AppId), accountshandler func()) {
7 for {
8+ bch := client.session.GetBroadcastCh()
9+ uch := client.session.GetNotificationsCh()
10+ ech := client.session.GetErrCh()
11+ client.log.Debugf("XXX S: %#v -- B:%v U:%v E:%v", client.session, bch, uch, ech)
12 select {
13 case <-client.accountsCh:
14 accountshandler()
15 case state := <-client.connCh:
16 connhandler(state)
17- case bcast := <-client.session.GetBroadcastCh():
18+ case bcast := <-bch:
19 bcasthandler(bcast)
20- case aucast := <-client.session.GetNotificationsCh():
21+ case aucast := <-uch:
22 ucasthandler(aucast)
23- case err := <-client.session.GetErrCh():
24+ case err := <-ech:
25 errhandler(err)
26 case count := <-client.sessionConnectedCh:
27 client.log.Debugf("session connected after %d attempts", count)
28
29=== modified file 'client/client_test.go'
30--- client/client_test.go 2015-02-12 14:49:15 +0000
31+++ client/client_test.go 2015-02-12 14:49:16 +0000
32@@ -714,7 +714,7 @@
33 cli.hasConnectivity = true
34 defer cli.session.Close()
35 cli.handleErr(errors.New("bananas"))
36- c.Check(cs.log.Captured(), Matches, ".*session exited.*bananas\n")
37+ c.Check(cs.log.Captured(), Matches, "(?ism).*session exited.*bananas")
38 }
39
40 /*****************************************************************
41@@ -775,8 +775,7 @@
42 cli.session.Dial()
43 cli.hasConnectivity = true
44
45- // cli.session.State() will be "Error" here, for now at least
46- c.Check(cli.session.State(), Not(Equals), session.Disconnected)
47+ c.Check(cli.session.State(), Equals, session.Disconnected) // internally Error, but invisible now
48 cli.handleConnState(false)
49 c.Check(cli.session.State(), Equals, session.Disconnected)
50 }
51@@ -1162,11 +1161,14 @@
52 c.Check(cli.hasConnectivity, Equals, false)
53 cli.connCh <- true
54 tick()
55+ cs.log.Debugf("================================================================")
56 c.Check(cli.hasConnectivity, Equals, true)
57 cli.connCh <- false
58 tick()
59 c.Check(cli.hasConnectivity, Equals, false)
60
61+ cs.log.Debugf("here")
62+
63 // * session.BroadcastCh to the notifications handler
64 c.Check(d.bcastCount, Equals, 0)
65 bcastCh <- positiveBroadcastNotification
66
67=== modified file 'client/session/session.go'
68--- client/session/session.go 2015-02-12 14:49:15 +0000
69+++ client/session/session.go 2015-02-12 14:49:16 +0000
70@@ -37,6 +37,7 @@
71 "launchpad.net/ubuntu-push/logger"
72 "launchpad.net/ubuntu-push/protocol"
73 "launchpad.net/ubuntu-push/util"
74+ "runtime"
75 )
76
77 var (
78@@ -132,6 +133,122 @@
79 SetChForTesting(chan *BroadcastNotification, chan AddressedNotification, chan error)
80 }
81
82+// SessionManager presents a ClientSession interface, but hides the error states.
83+type SessionManager struct {
84+ session ClientSession
85+ serverAddrSpec string
86+ conf ClientSessionConfig
87+ deviceId string
88+ seenStateFactory func() (seenstate.SeenState, error)
89+ log logger.Logger
90+ lck sync.Mutex
91+}
92+
93+func funcName() string {
94+ pc, _, _, _ := runtime.Caller(2)
95+ return runtime.FuncForPC(pc).Name()
96+}
97+
98+func (sm *SessionManager) lock() {
99+ sm.log.Debugf("[%s] locking...", funcName())
100+ sm.lck.Lock()
101+ sm.log.Debugf("[%s] locked.", funcName())
102+}
103+
104+func (sm *SessionManager) unlock() {
105+ sm.lck.Unlock()
106+ sm.log.Debugf("[%s] unlocked.", funcName())
107+}
108+
109+func newSessionManager(serverAddrSpec string, conf ClientSessionConfig,
110+ deviceId string, seenStateFactory func() (seenstate.SeenState, error),
111+ log logger.Logger) (*SessionManager, error) {
112+ sm := &SessionManager{
113+ session: nil,
114+ serverAddrSpec: serverAddrSpec,
115+ conf: conf,
116+ deviceId: deviceId,
117+ seenStateFactory: seenStateFactory,
118+ log: log,
119+ }
120+ return sm, sm.newSession()
121+}
122+
123+func (sm *SessionManager) newSession() error {
124+ if sm.state() != Disconnected {
125+ return errors.New("trying to create a new session with non-stopped non-errored one existing.")
126+ }
127+ sess, err := newSession(sm.serverAddrSpec, sm.conf, sm.deviceId, sm.seenStateFactory, sm.log)
128+ if err != nil {
129+ return err
130+ }
131+ sm.session = sess
132+ return nil
133+}
134+
135+func (sm *SessionManager) State() ClientSessionState {
136+ sm.lock()
137+ defer sm.unlock()
138+ return sm.state()
139+}
140+func (sm *SessionManager) state() ClientSessionState {
141+ if sm.session == nil {
142+ return Disconnected
143+ }
144+ state := sm.session.State()
145+ if state == Error {
146+ return Disconnected
147+ }
148+ return state
149+}
150+
151+func (sm *SessionManager) AutoRedial(done chan uint32) {
152+ sm.lock()
153+ defer sm.unlock()
154+ // sm.newSession()
155+ sm.session.AutoRedial(done)
156+}
157+
158+func (sm *SessionManager) Close() {
159+ sm.lock()
160+ defer sm.unlock()
161+ sm.session.Close()
162+}
163+
164+func (sm *SessionManager) ClearCookie() {
165+ sm.lock()
166+ defer sm.unlock()
167+ sm.session.ClearCookie()
168+}
169+
170+func (sm *SessionManager) Dial() error {
171+ sm.lock()
172+ defer sm.unlock()
173+ return sm.session.Dial()
174+}
175+
176+func (sm *SessionManager) GetBroadcastCh() chan *BroadcastNotification {
177+ sm.lock()
178+ defer sm.unlock()
179+ return sm.session.GetBroadcastCh()
180+}
181+func (sm *SessionManager) GetNotificationsCh() chan AddressedNotification {
182+ sm.lock()
183+ defer sm.unlock()
184+ return sm.session.GetNotificationsCh()
185+}
186+func (sm *SessionManager) GetErrCh() chan error {
187+ sm.lock()
188+ defer sm.unlock()
189+ return sm.session.GetErrCh()
190+}
191+
192+func (sm *SessionManager) SetChForTesting(bch chan *BroadcastNotification, nch chan AddressedNotification, ech chan error) {
193+ sm.lock()
194+ defer sm.unlock()
195+ sm.session.SetChForTesting(bch, nch, ech)
196+}
197+
198 // ConcreteSession holds a client<->server session and its configuration.
199 type ConcreteSession struct {
200 // configuration
201@@ -191,7 +308,7 @@
202 func NewSession(serverAddrSpec string, conf ClientSessionConfig,
203 deviceId string, seenStateFactory func() (seenstate.SeenState, error),
204 log logger.Logger) (ClientSession, error) {
205- return newSession(serverAddrSpec, conf, deviceId, seenStateFactory, log)
206+ return newSessionManager(serverAddrSpec, conf, deviceId, seenStateFactory, log)
207 }
208
209 func newSession(serverAddrSpec string, conf ClientSessionConfig,

Subscribers

People subscribed via source and target branches