Merge lp:~rvb/gwacl/poller into lp:gwacl

Proposed by Raphaël Badin
Status: Merged
Approved by: Raphaël Badin
Approved revision: 64
Merged at revision: 69
Proposed branch: lp:~rvb/gwacl/poller
Merge into: lp:gwacl
Diff against target: 705 lines (+394/-63)
8 files modified
managementapi.go (+2/-2)
managementapi_test.go (+19/-15)
poller.go (+92/-0)
poller_test.go (+187/-0)
test_helpers.go (+64/-0)
x509session.go (+7/-7)
x509session_test.go (+9/-39)
xmlobjects.go (+14/-0)
To merge this branch: bzr merge lp:~rvb/gwacl/poller
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+155680@code.launchpad.net

Commit message

Add generic poller utility. Add a specialized poller to poll the Windows Azure server until an operation is completed.

Description of the change

The main change in this branch is the addition of the file poller.go (and the related tests) which adds a generic Poller interface and a generic method to actually perform the polling plus an specialized implementation for polling the Windows Azure server until a specific operation is completed.

A second branch will:
- change the asynchronous operations from managementapi.go so that they will return the corresponding operation id.
- update live_example_managementapi.go to wait for the completion of asynchronous operations.

This change was pre-imp'ed with Jeroen but instead of returning channels like we originally thought StartPolling would do, I decided to create a blocking method. It's then up to the caller to use goroutines if he does not want to block during the polling. This is ­— I think — a better design because it leaves it up to the caller to create and manage channels as he likes.

I've moved the test utilities rig*** in the test_helpers.go file because they are now used in many places.

Also, I changed the session's get/post/put so that they now return the full response instead of just the body. We need that in order to check the return status (and we will also need it to extract the operationID value from the headers).

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Please don't refer to a generic "the caller" as "she"! It's especially confusing because unlike "une prick," a caller is grammatically masculine.

English has a better way of dealing with indeterminate genders: use "they," with the corresponding plural verbs. Unlike other languages, English has no problem with switching conjugations halfway through a sentence for this purpose — "If the customer _has_ a complaint, remember to say that _they are_ right."

(Also, "wants not to block" is geek-speak. In real English you'd say "does not want to block," or perhaps "wants to avoid blocking.")

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

I think I was wrong on one point, actually: "wants to not block" would be the correct geek-speak. :-)

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Oh, and ESPECIALLY don't arbitrarily switch genders for the same person¹. Did somebody make you a bet that it would set me off? :)


¹) With the appropriate exception for trans-gender individuals, of course.

Revision history for this message
Raphaël Badin (rvb) wrote :

Wording is fixed… can I haz code review? ;)

Revision history for this message
Raphaël Badin (rvb) wrote :

> Oh, and ESPECIALLY don't arbitrarily switch genders for the same person¹. Did
> somebody make you a bet that it would set me off? :)

Well, I guess I know which button to push if I want to set you off now :).

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Oh, no worries. Just about any button will do for that.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

You mentioned that you were taking this channel-less approach and I think it was the right choice.

Overall a long but good branch, except for one annoying detail that you'll find below.

.

Could you document Poller to set the right expectations with the reader? The "flyweight" approach in particular looks a bit confusing. My understanding is that the Poller gives you a response and then you need to ask that same Poller whether that response means that you're done... I think I understand why it's done that way (it allows for an immutable poller) but it would help to have it stated.

I like the separation between the Poller interface and the internal implementation. Exposing the x509Response doesn't really fit in though. My recollection is that we had meant that to be internal to gwacl, given the duplication with http.Response, and now x509Response is no longer exported. This part needs fixing, I think — though not necessarily in the same branch.

.

Well done for documenting the pointless panic at the end of StartPolling. I guess in the future the compiler will get smarter, detect that the loop never exits, and complain that the panic() is unreachable. The compiler giveth, and then the compiler taketh away.

.

On reflection, maybe StartPolling() isn't quite the right name, in that it implies that it won't do the actual polling.

.

Typo in the documentation for operationPoller: "A object."

By the way, I'm not sure if documentation for a type should also begin with its name.

.

The documentation for operationPoller.Poll() and operationPoller.IsDone() gets lost in trying to describe the implementation. Document what they mean to the caller, not how they do their work! As a caller, I really don't care whether Poll() issues a GET request or a HEAD request — gwacl's exact purpose is to hide that sort of thing from me. What I care about as a caller is primarily: that I need to call these methods in order to wait for my prior request to complete and secondarily: that it makes outgoing requests and that it blocks.

.

In operationPoller.IsDone(), I think I see one of those xml.Unmarshal() calls that need an error check.

.

I like the tests. Excellent comment in TestStartPollingRetries().

.

In TestStartOperationRetries, "InProgress" is misspelled as "InProgres" in a comment and "Succeeded" is misspelled twice as "Succeded" — which may be working just because it's consistent, but it's late for me.

.

Jeroen

review: Approve
lp:~rvb/gwacl/poller updated
63. By Raphaël Badin

Review comments.

Revision history for this message
Raphaël Badin (rvb) wrote :

Thanks for the review.

>
> Could you document Poller to set the right expectations with the reader? The
> "flyweight" approach in particular looks a bit confusing. My understanding is
> that the Poller gives you a response and then you need to ask that same Poller
> whether that response means that you're done... I think I understand why it's
> done that way (it allows for an immutable poller) but it would help to have it
> stated.

Added a comments.

> I like the separation between the Poller interface and the internal
> implementation. Exposing the x509Response doesn't really fit in though. My
> recollection is that we had meant that to be internal to gwacl, given the
> duplication with http.Response, and now x509Response is no longer exported.
> This part needs fixing, I think — though not necessarily in the same branch.

You're right, now that x509Response is not exposed, the method in the poller interface should not be exposed either. Same for startPolling which is more of an internal utility method used by more concrete implementations such as StartOperationPolling.

> On reflection, maybe StartPolling() isn't quite the right name, in that it
> implies that it won't do the actual polling.

Hum, I don't really have a better idea right now so I'll land this as is and we will see if we come up with a better name later.

> Typo in the documentation for operationPoller: "A object."

Fixed.

> The documentation for operationPoller.Poll() and operationPoller.IsDone() gets
> lost in trying to describe the implementation. Document what they mean to the
> caller, not how they do their work! As a caller, I really don't care whether
> Poll() issues a GET request or a HEAD request — gwacl's exact purpose is to
> hide that sort of thing from me. What I care about as a caller is primarily:
> that I need to call these methods in order to wait for my prior request to
> complete and secondarily: that it makes outgoing requests and that it blocks.

This is really internal stuff, no user will ever call these methods directly, hence the focus on explaining what the methods do internally.

> In operationPoller.IsDone(), I think I see one of those xml.Unmarshal() calls
> that need an error check.

Indeed, I've left it as is, we will fix this in another branch.

> In TestStartOperationRetries, "InProgress" is misspelled as "InProgres" in a
> comment and "Succeeded" is misspelled twice as "Succeded" — which may be
> working just because it's consistent, but it's late for me.

oops, fixed.

Thanks again for the review.

lp:~rvb/gwacl/poller updated
64. By Raphaël Badin

Merge trunk, fix conflicts.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

On Wednesday 27 Mar 2013 14:09:17 you wrote:
> > Oh, and ESPECIALLY don't arbitrarily switch genders for the same person¹.
> > Did somebody make you a bet that it would set me off? :)
>
> Well, I guess I know which button to push if I want to set you off now :).

There are SO many to choose.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

On Wednesday 27 Mar 2013 17:06:24 you wrote:
> > On reflection, maybe StartPolling() isn't quite the right name, in that it
> > implies that it won't do the actual polling.
>
> Hum, I don't really have a better idea right now so I'll land this as is and
> we will see if we come up with a better name later.

There is a certain school of thought that says this method should not even be
exported, and that the API should just block (with a timeout) until the async
operation completes.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Yes, it is the fact that Poller is exported that made me think that it was to be part of gwacl's external API.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'managementapi.go'
2--- managementapi.go 2013-03-26 06:50:50 +0000
3+++ managementapi.go 2013-03-27 17:10:31 +0000
4@@ -32,7 +32,7 @@
5 return nil, err
6 }
7 hostedServices := HostedServiceDescriptorList{}
8- xml.Unmarshal(res, &hostedServices)
9+ xml.Unmarshal(res.Body, &hostedServices)
10 return hostedServices.HostedServices, nil
11 }
12
13@@ -44,7 +44,7 @@
14 return nil, err
15 }
16 hostedService := HostedService{}
17- xml.Unmarshal(res, &hostedService)
18+ xml.Unmarshal(res.Body, &hostedService)
19 return &hostedService, nil
20 }
21
22
23=== modified file 'managementapi_test.go'
24--- managementapi_test.go 2013-03-26 09:54:07 +0000
25+++ managementapi_test.go 2013-03-27 17:10:31 +0000
26@@ -48,12 +48,16 @@
27 return &recordedRequests
28 }
29
30-// checkRequest asserts that the given slice contains one request, with the
31+// checkOneRequest asserts that the given slice contains one request, with the
32 // given characteristics.
33-func checkRequest(c *C, recordedRequests *[]*x509Request, URL string, payload []byte, Method string) {
34+func checkOneRequest(c *C, recordedRequests *[]*x509Request, URL string, payload []byte, Method string) {
35 requests := *recordedRequests
36 c.Assert(len(requests), Equals, 1)
37 request := requests[0]
38+ checkRequest(c, request, URL, payload, Method)
39+}
40+
41+func checkRequest(c *C, request *x509Request, URL string, payload []byte, Method string) {
42 c.Assert(request.URL, Equals, URL)
43 c.Assert(string(request.Payload), Equals, string(payload))
44 c.Assert(request.Method, Equals, Method)
45@@ -61,33 +65,33 @@
46
47 func (suite *managementAPISuite) TestListHostedServiceDescriptors(c *C) {
48 api := suite.makeAPI(c)
49- recordedRequests := suite.setUpDispatcher()
50+ recordedRequests := setUpDispatcher("operationID")
51
52 _, err := api.ListHostedServiceDescriptors()
53
54 c.Assert(err, IsNil)
55 expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices"
56- checkRequest(c, recordedRequests, expectedURL, []byte{}, "GET")
57+ checkOneRequest(c, recordedRequests, expectedURL, []byte{}, "GET")
58 }
59
60 // TODO test that ListHostedServiceDescriptors parses the structures correctly
61
62 func (suite *managementAPISuite) TestLoadHostedService(c *C) {
63 api := suite.makeAPI(c)
64- recordedRequests := suite.setUpDispatcher()
65+ recordedRequests := setUpDispatcher("operationID")
66 serviceName := "serviceName"
67 _, err := api.LoadHostedService(serviceName)
68
69 c.Assert(err, IsNil)
70 expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices/" + serviceName + "?embed-detail=true"
71- checkRequest(c, recordedRequests, expectedURL, []byte{}, "GET")
72+ checkOneRequest(c, recordedRequests, expectedURL, []byte{}, "GET")
73 }
74
75 // TODO test that TestLoadHostedService parses the structures correctly
76
77 func (suite *managementAPISuite) TestAddHostedService(c *C) {
78 api := suite.makeAPI(c)
79- recordedRequests := suite.setUpDispatcher()
80+ recordedRequests := setUpDispatcher("operationID")
81 createHostedService := NewCreateHostedServiceWithLocation("testName", "testLabel", "East US")
82 err := api.AddHostedService(createHostedService)
83
84@@ -95,23 +99,23 @@
85 expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices"
86 expectedPayload, err := marshalXML(createHostedService)
87 c.Assert(err, IsNil)
88- checkRequest(c, recordedRequests, expectedURL, expectedPayload, "POST")
89+ checkOneRequest(c, recordedRequests, expectedURL, expectedPayload, "POST")
90 }
91
92 func (suite *managementAPISuite) TestDeleteHostedService(c *C) {
93 api := suite.makeAPI(c)
94- recordedRequests := suite.setUpDispatcher()
95+ recordedRequests := setUpDispatcher("operationID")
96 hostedServiceName := "testName"
97 err := api.DeleteHostedService(hostedServiceName)
98
99 c.Assert(err, IsNil)
100 expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices/" + hostedServiceName
101- checkRequest(c, recordedRequests, expectedURL, []byte{}, "DELETE")
102+ checkOneRequest(c, recordedRequests, expectedURL, []byte{}, "DELETE")
103 }
104
105 func (suite *managementAPISuite) TestAddDeployment(c *C) {
106 api := suite.makeAPI(c)
107- recordedRequests := suite.setUpDispatcher()
108+ recordedRequests := setUpDispatcher("operationID")
109 serviceName := "serviceName"
110 configurationSet := NewLinuxProvisioningConfiguration("testHostname12345", "test", "test123#@!", false)
111 OSVirtualHardDisk := NewOSVirtualHardDisk("hostCaching", "diskLabel", "diskName", "http://mediaLink", "sourceImageName")
112@@ -123,7 +127,7 @@
113 expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices/" + serviceName + "/deployments"
114 expectedPayload, err := marshalXML(deployment)
115 c.Assert(err, IsNil)
116- checkRequest(c, recordedRequests, expectedURL, expectedPayload, "POST")
117+ checkOneRequest(c, recordedRequests, expectedURL, expectedPayload, "POST")
118 }
119
120 func (suite *managementAPISuite) TestAddStorageAccount(c *C) {
121@@ -139,12 +143,12 @@
122 expectedURL := AZURE_URL + api.session.subscriptionId + "/services/storageservices"
123 expectedPayload, err := marshalXML(cssi)
124 c.Assert(err, IsNil)
125- checkRequest(c, &recordedRequests, expectedURL, expectedPayload, "POST")
126+ checkOneRequest(c, &recordedRequests, expectedURL, expectedPayload, "POST")
127 }
128
129 func (suite *managementAPISuite) TestPerformNodeOperation(c *C) {
130 api := suite.makeAPI(c)
131- recordedRequests := suite.setUpDispatcher()
132+ recordedRequests := setUpDispatcher("operationID")
133 serviceName := "serviceName"
134 deploymentName := "deploymentName"
135 roleName := "roleName"
136@@ -155,5 +159,5 @@
137 expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices/" + serviceName + "/deployments/" + deploymentName + "/roleinstances/" + roleName + "/Operations"
138 expectedPayload, err := marshalXML(operation)
139 c.Assert(err, IsNil)
140- checkRequest(c, recordedRequests, expectedURL, expectedPayload, "POST")
141+ checkOneRequest(c, recordedRequests, expectedURL, expectedPayload, "POST")
142 }
143
144=== added file 'poller.go'
145--- poller.go 1970-01-01 00:00:00 +0000
146+++ poller.go 2013-03-27 17:10:31 +0000
147@@ -0,0 +1,92 @@
148+// Copyright 2013 Canonical Ltd. This software is licensed under the
149+// GNU Lesser General Public License version 3 (see the file COPYING).
150+
151+package gwacl
152+
153+import (
154+ "encoding/xml"
155+ "time"
156+)
157+
158+// Generic poller interface/methods.
159+
160+// A Poller exposes two methods to query a remote server and decide when
161+// the response given by the server means that the polling is finished.
162+type Poller interface {
163+ poll() (*x509Response, error)
164+ isDone(*x509Response) bool
165+}
166+
167+// startPolling calls the poll() method of the given 'poller' object every
168+// 'interval' until poller.isDone() return true.
169+func startPolling(poller Poller, interval time.Duration) (*x509Response, error) {
170+ // TODO: Add a timeout so that polling won't continue forever if isDone()
171+ // always returns false.
172+ ticker := time.Tick(interval)
173+ for _ = range ticker {
174+ response, err := poller.poll()
175+ if err != nil {
176+ return nil, err
177+ }
178+ if poller.isDone(response) {
179+ return response, nil
180+ }
181+ }
182+ // This code cannot be reached but Go insists on having a return or a panic
183+ // statement at the end of this method. Sigh.
184+ panic("invalid poller state!")
185+}
186+
187+// Operation poller structs/methods.
188+
189+// StartOperationPolling calls startPolling on the given arguments and converts
190+// the returned object into an *Operation.
191+func StartOperationPolling(poller Poller, interval time.Duration) (*Operation, error) {
192+ response, err := startPolling(poller, interval)
193+ if err != nil {
194+ return nil, err
195+ }
196+ operation := Operation{}
197+ xml.Unmarshal(response.Body, &operation)
198+ return &operation, nil
199+}
200+
201+// operationPoller is an object implementing the Poller interface, used to
202+// poll the Window Azure server until the operation referenced by the given
203+// operationID is completed.
204+type operationPoller struct {
205+ api *ManagementAPI
206+ operationID string
207+}
208+
209+var _ Poller = &operationPoller{}
210+
211+// NewOperationPoller returns a poller object associated with the given
212+// management API object and the given operationID. It can track (by polling
213+// the server) the status of the operation associated with the provided
214+// operationID string.
215+func NewOperationPoller(api *ManagementAPI, operationID string) Poller {
216+ return operationPoller{api: api, operationID: operationID}
217+}
218+
219+// Poll issues a blocking request to microsoft Azure to fetch the information
220+// related to the operation associated with the poller.
221+func (poller operationPoller) poll() (*x509Response, error) {
222+ URI := "operations/" + poller.operationID
223+ return poller.api.session.get(URI)
224+}
225+
226+// IsDone returns true if the given response has a status code indicating
227+// success and if the returned XML response corresponds to a valid Operation
228+// with a status indicating that the operation is completed.
229+func (poller operationPoller) isDone(response *x509Response) bool {
230+ // TODO: Add a timeout so that polling won't continue forever if the
231+ // server cannot be reached.
232+ if response.StatusCode >= 200 && response.StatusCode < 300 {
233+ operation := Operation{}
234+ xml.Unmarshal(response.Body, &operation)
235+ status := operation.Status
236+ return (status != "" && status != InProgressOperationStatus)
237+ }
238+ return false
239+}
240
241=== added file 'poller_test.go'
242--- poller_test.go 1970-01-01 00:00:00 +0000
243+++ poller_test.go 2013-03-27 17:10:31 +0000
244@@ -0,0 +1,187 @@
245+// Copyright 2013 Canonical Ltd. This software is licensed under the
246+// GNU Lesser General Public License version 3 (see the file COPYING).
247+
248+package gwacl
249+
250+import (
251+ "fmt"
252+ . "launchpad.net/gocheck"
253+ "launchpad.net/gwacl/dedent"
254+ "net/http"
255+ "time"
256+)
257+
258+type pollerSuite struct{}
259+
260+var _ = Suite(&pollerSuite{})
261+
262+func (suite *pollerSuite) makeAPI(c *C) *ManagementAPI {
263+ subscriptionId := "subscriptionId"
264+ subscriptionId = subscriptionId
265+ api, err := NewManagementAPI(subscriptionId, "certFile")
266+ c.Assert(err, IsNil)
267+ return api
268+}
269+
270+// testPoller is a struct which implements the Poller interface. It records
271+// the number of calls to testPoller.Poll().
272+type testPoller struct {
273+ recordedPollCalls int
274+ notDoneCalls int
275+ errorCalls int
276+}
277+
278+var testPollerResponse = x509Response{}
279+var testPollerError = fmt.Errorf("Test error")
280+
281+// newTestPoller return a pointer to a testPoller object that will return
282+// false when IsDone() will be called 'notDoneCalls' number of times and that
283+// will error when Poll() will be called 'errorCalls' number of times.
284+func newTestPoller(notDoneCalls int, errorCalls int) *testPoller {
285+ return &testPoller{0, notDoneCalls, errorCalls}
286+}
287+
288+func (poller *testPoller) poll() (*x509Response, error) {
289+ if poller.errorCalls > 0 {
290+ poller.errorCalls -= 1
291+ return nil, testPollerError
292+ }
293+ poller.recordedPollCalls += 1
294+ return &testPollerResponse, nil
295+}
296+
297+func (poller *testPoller) isDone(response *x509Response) bool {
298+ if poller.notDoneCalls == 0 {
299+ return true
300+ }
301+ poller.notDoneCalls = poller.notDoneCalls - 1
302+ return false
303+}
304+
305+func (suite *pollerSuite) TeststartPollingReturnsError(c *C) {
306+ poller := newTestPoller(0, 1)
307+ response, err := startPolling(poller, time.Millisecond)
308+
309+ c.Assert(err, Equals, testPollerError)
310+ c.Assert(response, IsNil)
311+}
312+
313+func (suite *pollerSuite) TeststartPollingRetries(c *C) {
314+ poller := newTestPoller(2, 0)
315+ response, err := startPolling(poller, time.Millisecond)
316+
317+ c.Assert(err, IsNil)
318+ c.Assert(response, Equals, &testPollerResponse)
319+ // Poll() has been called 3 times: two calls for which IsDone() returned
320+ // false and one for which IsDone() return true.
321+ c.Assert(poller.recordedPollCalls, Equals, 3)
322+}
323+
324+func (suite *pollerSuite) TestNewOperationPoller(c *C) {
325+ api := suite.makeAPI(c)
326+ operationID := "operationID"
327+
328+ poller := NewOperationPoller(api, operationID)
329+
330+ operationPollerInstance := poller.(operationPoller)
331+ c.Check(operationPollerInstance.api, Equals, api)
332+ c.Check(operationPollerInstance.operationID, Equals, operationID)
333+}
334+
335+func (suite *pollerSuite) TestOperationPollerPoll(c *C) {
336+ api := suite.makeAPI(c)
337+ operationID := "operationID"
338+ poller := NewOperationPoller(api, operationID)
339+ recordedRequests := setUpDispatcher("operationID")
340+
341+ _, err := poller.poll()
342+
343+ c.Assert(err, IsNil)
344+ expectedURL := AZURE_URL + api.session.subscriptionId + "/operations/" + operationID
345+ checkOneRequest(c, recordedRequests, expectedURL, []byte{}, "GET")
346+}
347+
348+var operationXMLTemplate = dedent.Dedent(`
349+<?xml version="1.0" encoding="utf-8"?>
350+ <Operation xmlns="http://schemas.microsoft.com/windowsazure">
351+ <ID>bogus-request-id</ID>
352+ <Status>%s</Status>
353+ </Operation>
354+`)
355+
356+func (suite *pollerSuite) TestOperationPollerIsDoneReturnsTrueIfOperationDone(c *C) {
357+ poller := NewOperationPoller(suite.makeAPI(c), "operationID")
358+ operationStatuses := []string{"Succeeded", "Failed"}
359+ for _, operationStatus := range operationStatuses {
360+ body := fmt.Sprintf(operationXMLTemplate, operationStatus)
361+ response := x509Response{
362+ Body: []byte(body),
363+ StatusCode: http.StatusOK,
364+ }
365+
366+ isDone := poller.isDone(&response)
367+ c.Assert(isDone, Equals, true)
368+ }
369+}
370+
371+func (suite *pollerSuite) TestOperationPollerIsDoneReturnsFalse(c *C) {
372+ poller := NewOperationPoller(suite.makeAPI(c), "operationID")
373+ notDoneResponses := []x509Response{
374+ // 'InProgress' response.
375+ x509Response{
376+ Body: []byte(fmt.Sprintf(operationXMLTemplate, "InProgress")),
377+ StatusCode: http.StatusOK,
378+ },
379+ // Invalid XML content.
380+ x509Response{
381+ Body: []byte("invalid XML"),
382+ StatusCode: http.StatusOK,
383+ },
384+ // Error statuses.
385+ x509Response{StatusCode: http.StatusNotFound},
386+ x509Response{StatusCode: http.StatusBadRequest},
387+ x509Response{StatusCode: http.StatusInternalServerError},
388+ }
389+ for _, response := range notDoneResponses {
390+ isDone := poller.isDone(&response)
391+ c.Assert(isDone, Equals, false)
392+ }
393+}
394+
395+func (suite *pollerSuite) TestStartOperationPollingRetries(c *C) {
396+ // This is an end-to-end test of the operation poller; there is a certain
397+ // amount of duplication with the unit tests for startPolling() but
398+ // it's probably worth it to thoroughly test StartOperationPolling().
399+
400+ // Fake 2 responses in sequence: a 'InProgress' response and then a
401+ // 'Succeeded' response.
402+ firstResponse := DispatcherResponse{
403+ response: &x509Response{
404+ Body: []byte(fmt.Sprintf(operationXMLTemplate, "InProgress")),
405+ StatusCode: http.StatusOK,
406+ },
407+ errorObject: nil}
408+ secondResponse := DispatcherResponse{
409+ response: &x509Response{
410+ Body: []byte(fmt.Sprintf(operationXMLTemplate, "Succeeded")),
411+ StatusCode: http.StatusOK,
412+ },
413+ errorObject: nil}
414+ responses := []DispatcherResponse{firstResponse, secondResponse}
415+ rigPreparedResponseDispatcher(responses)
416+ recordedRequests := make([]*x509Request, 0)
417+ rigRecordingDispatcher(&recordedRequests)
418+
419+ // Setup poller and start it.
420+ operationID := "operationID"
421+ poller := NewOperationPoller(suite.makeAPI(c), operationID)
422+ response, err := startPolling(poller, time.Millisecond)
423+
424+ c.Assert(err, IsNil)
425+ c.Assert(response, Equals, secondResponse.response)
426+ operationPollerInstance := poller.(operationPoller)
427+ expectedURL := AZURE_URL + operationPollerInstance.api.session.subscriptionId + "/operations/" + operationID
428+ c.Assert(len(recordedRequests), Equals, 2)
429+ checkRequest(c, recordedRequests[0], expectedURL, []byte{}, "GET")
430+ checkRequest(c, recordedRequests[1], expectedURL, []byte{}, "GET")
431+}
432
433=== modified file 'test_helpers.go'
434--- test_helpers.go 2013-03-21 02:14:39 +0000
435+++ test_helpers.go 2013-03-27 17:10:31 +0000
436@@ -6,6 +6,7 @@
437 import (
438 "fmt"
439 "math/rand"
440+ "net/http"
441 "strings"
442 "time"
443 )
444@@ -181,6 +182,69 @@
445 ExtendedProperties: ExtendedProperties}
446 }
447
448+// rigRecordingDispatcher sets up a request dispatcher that records incoming
449+// requests by appending them to *record. It returns the result of whatever
450+// dispatcher was already active.
451+// If you also want the dispatcher to return a particular result, rig it for
452+// that result first (using one of the other rig...Dispatcher functions) and
453+// then chain the recording dispatcher in front of it.
454+func rigRecordingDispatcher(record *[]*x509Request) {
455+ previousDispatcher := _X509Dispatcher
456+ _X509Dispatcher = func(session *x509Session, request *x509Request) (*x509Response, error) {
457+ *record = append(*record, request)
458+ return previousDispatcher(session, request)
459+ }
460+}
461+
462+// rigFixedResponseDispatcher sets up a request dispatcher that always returns
463+// a prepared response.
464+func rigFixedResponseDispatcher(response *x509Response) {
465+ _X509Dispatcher = func(*x509Session, *x509Request) (*x509Response, error) {
466+ return response, nil
467+ }
468+}
469+
470+// rigFailingDispatcher sets up a request dispatcher that returns a given
471+// error.
472+func rigFailingDispatcher(failure error) {
473+ _X509Dispatcher = func(*x509Session, *x509Request) (*x509Response, error) {
474+ return nil, failure
475+ }
476+}
477+
478+type DispatcherResponse struct {
479+ response *x509Response
480+ errorObject error
481+}
482+
483+// rigPreparedResponseDispatcher sets up a request dispatcher that all the provided
484+// reponses, one after the other each time the dispatcher is called.
485+func rigPreparedResponseDispatcher(responses []DispatcherResponse) {
486+ index := 0
487+ _X509Dispatcher = func(*x509Session, *x509Request) (*x509Response, error) {
488+ response := responses[index]
489+ index += 1
490+ return response.response, response.errorObject
491+ }
492+}
493+
494+// setUpDispatcher sets up a request dispatcher that:
495+// - records requests
496+// - returns empty responses
497+func setUpDispatcher(operationID string) *[]*x509Request {
498+ header := http.Header{}
499+ header.Set("X-Ms-Request-Id", operationID)
500+ fixedResponse := x509Response{
501+ StatusCode: http.StatusOK,
502+ Body: []byte{},
503+ Header: header,
504+ }
505+ rigFixedResponseDispatcher(&fixedResponse)
506+ recordedRequests := make([]*x509Request, 0)
507+ rigRecordingDispatcher(&recordedRequests)
508+ return &recordedRequests
509+}
510+
511 func init() {
512 // Staggeringly, rand will produce the same numbers from the start of
513 // program invocation unless you seed it ...
514
515=== modified file 'x509session.go'
516--- x509session.go 2013-03-27 07:16:45 +0000
517+++ x509session.go 2013-03-27 17:10:31 +0000
518@@ -43,7 +43,7 @@
519 // It returns the response body and/or an error. If the error is a
520 // ServerError, the returned body will be the one received from the server.
521 // For any other kind of error, the returned body will be nil.
522-func (session *x509Session) get(url string) ([]byte, error) {
523+func (session *x509Session) get(url string) (*x509Response, error) {
524 fullUrl := composeURL(url, session.subscriptionId)
525 request := newX509RequestGET(fullUrl)
526 response, err := _X509Dispatcher(session, request)
527@@ -51,14 +51,14 @@
528 return nil, err
529 }
530 err = session.getServerError(response.StatusCode, response.Body, "GET request failed")
531- return response.Body, err
532+ return response, err
533 }
534
535 // post performs a POST request to the Azure management API.
536 // It returns the response body and/or an error. If the error is a
537 // ServerError, the returned body will be the one received from the server.
538 // For any other kind of error, the returned body will be nil.
539-func (session *x509Session) post(url string, body []byte, contentType string) ([]byte, error) {
540+func (session *x509Session) post(url string, body []byte, contentType string) (*x509Response, error) {
541 fullUrl := composeURL(url, session.subscriptionId)
542 request := newX509RequestPOST(fullUrl, body, contentType)
543 response, err := _X509Dispatcher(session, request)
544@@ -66,7 +66,7 @@
545 return nil, err
546 }
547 err = session.getServerError(response.StatusCode, response.Body, "POST request failed")
548- return response.Body, err
549+ return response, err
550 }
551
552 // delete performs a DELETE request to the Azure management API.
553@@ -81,12 +81,12 @@
554 }
555
556 // put performs a PUT request to the Azure management API.
557-func (session *x509Session) put(url string, body []byte) error {
558+func (session *x509Session) put(url string, body []byte) (*x509Response, error) {
559 fullUrl := composeURL(url, session.subscriptionId)
560 request := newX509RequestPUT(fullUrl, body)
561 response, err := _X509Dispatcher(session, request)
562 if err != nil {
563- return err
564+ return nil, err
565 }
566- return session.getServerError(response.StatusCode, response.Body, "POST request failed")
567+ return response, session.getServerError(response.StatusCode, response.Body, "POST request failed")
568 }
569
570=== modified file 'x509session_test.go'
571--- x509session_test.go 2013-03-27 16:34:04 +0000
572+++ x509session_test.go 2013-03-27 17:10:31 +0000
573@@ -44,36 +44,6 @@
574
575 var _ = Suite(&x509SessionSuite{})
576
577-// rigRecordingDispatcher sets up a request dispatcher that records incoming
578-// requests by appending them to *record. It returns the result of whatever
579-// dispatcher was already active.
580-// If you also want the dispatcher to return a particular result, rig it for
581-// that result first (using one of the other rig...Dispatcher functions) and
582-// then chain the recording dispatcher in front of it.
583-func rigRecordingDispatcher(record *[]*x509Request) {
584- previousDispatcher := _X509Dispatcher
585- _X509Dispatcher = func(session *x509Session, request *x509Request) (*x509Response, error) {
586- *record = append(*record, request)
587- return previousDispatcher(session, request)
588- }
589-}
590-
591-// rigFixedResponseDispatcher sets up a request dispatcher that always returns
592-// a prepared response.
593-func rigFixedResponseDispatcher(response *x509Response) {
594- _X509Dispatcher = func(*x509Session, *x509Request) (*x509Response, error) {
595- return response, nil
596- }
597-}
598-
599-// rigFailingDispatcher sets up a request dispatcher that returns a given
600-// error.
601-func rigFailingDispatcher(failure error) {
602- _X509Dispatcher = func(*x509Session, *x509Request) (*x509Response, error) {
603- return nil, failure
604- }
605-}
606-
607 // Create a cert and pem file in a temporary dir in /tmp and return the
608 // names of the files. The caller is responsible for cleaning up the files.
609 func makeX509Certificate() (string, string) {
610@@ -198,14 +168,14 @@
611 recordedRequests := make([]*x509Request, 0)
612 rigRecordingDispatcher(&recordedRequests)
613
614- receivedBody, err := session.get(uri)
615+ receivedResponse, err := session.get(uri)
616 c.Assert(err, IsNil)
617
618 c.Assert(len(recordedRequests), Equals, 1)
619 request := recordedRequests[0]
620 c.Check(request.URL, Equals, AZURE_URL+subscriptionID+"/"+uri)
621 c.Check(request.Method, Equals, "GET")
622- c.Check(receivedBody, DeepEquals, fixedResponse.Body)
623+ c.Check(*receivedResponse, DeepEquals, fixedResponse)
624 }
625
626 func (suite *x509SessionSuite) TestGetReportsClientSideError(c *C) {
627@@ -228,12 +198,12 @@
628 }
629 rigFixedResponseDispatcher(&fixedResponse)
630
631- body, err := session.get("/fail")
632+ response, err := session.get("/fail")
633 c.Assert(err, NotNil)
634
635 serverError := err.(*ServerError)
636 c.Check(serverError.StatusCode(), Equals, fixedResponse.StatusCode)
637- c.Check(body, DeepEquals, fixedResponse.Body)
638+ c.Check(*response, DeepEquals, fixedResponse)
639 }
640
641 func (suite *x509SessionSuite) TestPostIssuesRequest(c *C) {
642@@ -252,7 +222,7 @@
643 recordedRequests := make([]*x509Request, 0)
644 rigRecordingDispatcher(&recordedRequests)
645
646- receivedBody, err := session.post(uri, requestBody, requestContentType)
647+ receivedResponse, err := session.post(uri, requestBody, requestContentType)
648 c.Assert(err, IsNil)
649
650 c.Assert(len(recordedRequests), Equals, 1)
651@@ -261,7 +231,7 @@
652 c.Check(request.Method, Equals, "POST")
653 c.Check(request.ContentType, Equals, requestContentType)
654 c.Check(request.Payload, DeepEquals, requestBody)
655- c.Check(receivedBody, DeepEquals, fixedResponse.Body)
656+ c.Check(*receivedResponse, DeepEquals, fixedResponse)
657 }
658
659 func (suite *x509SessionSuite) TestPostReportsClientSideError(c *C) {
660@@ -284,12 +254,12 @@
661 }
662 rigFixedResponseDispatcher(&fixedResponse)
663
664- body, err := session.post("/fail", []byte("request body"), "contentType")
665+ reponse, err := session.post("/fail", []byte("request body"), "contentType")
666 c.Assert(err, NotNil)
667
668 serverError := err.(*ServerError)
669 c.Check(serverError.StatusCode(), Equals, fixedResponse.StatusCode)
670- c.Check(body, DeepEquals, fixedResponse.Body)
671+ c.Check(*reponse, DeepEquals, fixedResponse)
672 }
673
674 func (suite *x509SessionSuite) TestDeleteIssuesRequest(c *C) {
675@@ -326,7 +296,7 @@
676 recordedRequests := make([]*x509Request, 0)
677 rigRecordingDispatcher(&recordedRequests)
678
679- err = session.put(uri, requestBody)
680+ _, err = session.put(uri, requestBody)
681 c.Assert(err, IsNil)
682
683 c.Assert(len(recordedRequests), Equals, 1)
684
685=== modified file 'xmlobjects.go'
686--- xmlobjects.go 2013-03-27 07:16:45 +0000
687+++ xmlobjects.go 2013-03-27 17:10:31 +0000
688@@ -520,3 +520,17 @@
689 func (g *GetBlockList) Deserialize(data []byte) error {
690 return xml.Unmarshal(data, g)
691 }
692+
693+//
694+// Operation Services bits
695+//
696+
697+var InProgressOperationStatus = "InProgress"
698+
699+type Operation struct {
700+ ID string `xml:"ID"`
701+ Status string `xml:"Status"`
702+ HttpStatusCode int `xml:"HttpStatusCode"`
703+ ErrorCode string `xml:"Error>Code"`
704+ ErrorMessage string `xml:"Error>Message"`
705+}

Subscribers

People subscribed via source and target branches

to all changes: