Merge lp:~julian-edwards/gwacl/updatehostedservice into lp:gwacl

Proposed by Julian Edwards on 2013-07-04
Status: Merged
Approved by: Julian Edwards on 2013-07-04
Approved revision: 162
Merged at revision: 161
Proposed branch: lp:~julian-edwards/gwacl/updatehostedservice
Merge into: lp:gwacl
Diff against target: 197 lines (+130/-3)
4 files modified
management_base.go (+21/-3)
management_base_test.go (+30/-0)
xmlobjects.go (+24/-0)
xmlobjects_test.go (+55/-0)
To merge this branch: bzr merge lp:~julian-edwards/gwacl/updatehostedservice
Reviewer Review Type Date Requested Status
Raphaël Badin (community) 2013-07-04 Approve on 2013-07-04
Review via email: mp+172957@code.launchpad.net

Commit message

Add UpdateHostedService api call.

To post a comment you must log in.
Raphaël Badin (rvb) wrote :

Thanks, nice change.

[0]

+func (api *ManagementAPI) UpdateHostedService(serviceName string, params *UpdateHostedService) error {
11 + var err error
12 + URI := "services/hostedservices/" + serviceName

You forgot to call checkPathComponents(serviceName)

[1]

11 + var err error

No need for this declaration I think.

[2]

This can be done later but it would be great to use this method in example/management/run.go.

[3]

[2]

152 +func (suite *xmlSuite) TestUpdateHostedService(c *C) {
153 + label := MakeRandomString(10)
154 + expected := makeUpdateHostedService(label)
155 + input := UpdateHostedService{
156 + XMLNS: XMLNS,
157 + Label: label,
158 + Description: "description",

This is a detail but it's a bit weird that "description" references a string that is buried deep in the XML code above, maybe the description (and the other values of the XML string) should be an argument of makeUpdateHostedService().

review: Approve
Julian Edwards (julian-edwards) wrote :

On 04/07/13 16:55, Raphaël Badin wrote:
> Review: Approve
>
> Thanks, nice change.

Thank you.

>
> [0]
>
> +func (api *ManagementAPI) UpdateHostedService(serviceName string, params *UpdateHostedService) error {
> 11 + var err error
> 12 + URI := "services/hostedservices/" + serviceName
>
> You forgot to call checkPathComponents(serviceName)

Ok thanks.

>
> [1]
>
> 11 + var err error
>
> No need for this declaration I think.

Well - it kinda is. When you do multiple assignments to err, if it's
the second return value in a := statement it doesn't get set
consistently in my experience - at least it's fucking confusing me
anyway, assignment semantics in Go are batshit. Adding the var ensures
it's never masking the result with subsequent assignments.

>
> [2]
>
> This can be done later but it would be great to use this method in example/management/run.go.

Indeed, but the branch was large enough.

>
> [3]
>
> [2]
>
> 152 +func (suite *xmlSuite) TestUpdateHostedService(c *C) {
> 153 + label := MakeRandomString(10)
> 154 + expected := makeUpdateHostedService(label)
> 155 + input := UpdateHostedService{
> 156 + XMLNS: XMLNS,
> 157 + Label: label,
> 158 + Description: "description",
>
>
> This is a detail but it's a bit weird that "description" references a string that is buried deep in the XML code above, maybe the description (and the other values of the XML string) should be an argument of makeUpdateHostedService().
>

True, and I was just harping on about that in IRC as well :) I fixed
it, thanks for point it out.

J

162. By Julian Edwards on 2013-07-04

improvements after the review

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'management_base.go'
2--- management_base.go 2013-07-03 00:10:00 +0000
3+++ management_base.go 2013-07-04 07:13:25 +0000
4@@ -121,6 +121,23 @@
5 return hostedServices.HostedServices, err
6 }
7
8+// UpdateHostedService updates the provided values on the named service.
9+// Use NewUpdateHostedService() to create an UpdateHostedService params object.
10+func (api *ManagementAPI) UpdateHostedService(serviceName string, params *UpdateHostedService) error {
11+ var err error
12+ checkPathComponents(serviceName)
13+ URI := "services/hostedservices/" + serviceName
14+ body, err := params.Serialize()
15+ if err != nil {
16+ return err
17+ }
18+ response, err := api.session.put(URI, []byte(body))
19+ if err != nil {
20+ return err
21+ }
22+ return api.blockUntilCompleted(response)
23+}
24+
25 // GetHostedServiceProperties loads a HostedService object from the Azure
26 // management API.
27 func (api *ManagementAPI) GetHostedServiceProperties(
28@@ -143,7 +160,8 @@
29 }
30
31 // AddHostedService adds a hosted service.
32-// This is an asynchronous operation, it blocks until the operation is completed.
33+// This is an asynchronous operation on Azure, but this call blocks until the
34+// operation is completed.
35 func (api *ManagementAPI) AddHostedService(definition *CreateHostedService) error {
36 URI := "services/hostedservices"
37 body, err := marshalXML(definition)
38@@ -326,8 +344,8 @@
39 checkPathComponents(
40 request.ServiceName, request.DeploymentName, request.RoleName)
41 url := ("services/hostedservices/" + request.ServiceName +
42- "/deployments/" + request.DeploymentName + "/roles/" +
43- request.RoleName)
44+ "/deployments/" + request.DeploymentName + "/roles/" +
45+ request.RoleName)
46 response, err := api.session.get(url)
47 if err != nil {
48 return nil, err
49
50=== modified file 'management_base_test.go'
51--- management_base_test.go 2013-07-04 04:33:05 +0000
52+++ management_base_test.go 2013-07-04 07:13:25 +0000
53@@ -4,6 +4,7 @@
54 package gwacl
55
56 import (
57+ "encoding/base64"
58 "encoding/xml"
59 "errors"
60 "fmt"
61@@ -369,6 +370,35 @@
62 c.Assert(descriptors[0].URL, Equals, url)
63 }
64
65+func (suite *managementBaseAPISuite) TestUpdateHostedService(c *C) {
66+ api := makeAPI(c)
67+ randomLabel := MakeRandomString(10)
68+ randomDescription := MakeRandomString(10)
69+ property := ExtendedProperty{
70+ Name: "property-name",
71+ Value: "property-value",
72+ }
73+ base64Label := base64.StdEncoding.EncodeToString([]byte(randomLabel))
74+ requestPayload := []byte(makeUpdateHostedService(base64Label, randomDescription, property))
75+ fixedResponse := x509Response{
76+ StatusCode: http.StatusOK,
77+ }
78+ rigFixedResponseDispatcher(&fixedResponse)
79+ recordedRequests := make([]*X509Request, 0)
80+ rigRecordingDispatcher(&recordedRequests)
81+
82+ serviceName := MakeRandomString(10)
83+ properties := []ExtendedProperty{
84+ property,
85+ }
86+ update := NewUpdateHostedService(randomLabel, randomDescription, properties)
87+ err := api.UpdateHostedService(serviceName, update)
88+
89+ c.Assert(err, IsNil)
90+ expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices/" + serviceName
91+ checkOneRequest(c, &recordedRequests, expectedURL, requestPayload, "PUT")
92+}
93+
94 func (suite *managementBaseAPISuite) TestGetHostedServiceProperties_withoutDetails(c *C) {
95 api := makeAPI(c)
96 body := `
97
98=== modified file 'xmlobjects.go'
99--- xmlobjects.go 2013-07-04 03:58:15 +0000
100+++ xmlobjects.go 2013-07-04 07:13:25 +0000
101@@ -381,6 +381,30 @@
102 return xml.Unmarshal(data, s)
103 }
104
105+// UpdateHostedService contains the details necessary to call the
106+// UpdateHostedService management API call.
107+// See http://msdn.microsoft.com/en-us/library/windowsazure/gg441303.aspx
108+type UpdateHostedService struct {
109+ XMLNS string `xml:"xmlns,attr"`
110+ Label string `xml:"Label,omitempty"` // base64-encoded
111+ Description string `xml:"Description,omitempty"`
112+ ExtendedProperties []ExtendedProperty `xml:"ExtendedProperties>ExtendedProperty,omitempty"`
113+}
114+
115+func (u *UpdateHostedService) Serialize() (string, error) {
116+ return toxml(u)
117+}
118+
119+func NewUpdateHostedService(label, description string, properties []ExtendedProperty) *UpdateHostedService {
120+ base64label := base64.StdEncoding.EncodeToString([]byte(label))
121+ return &UpdateHostedService{
122+ XMLNS: XMLNS,
123+ Label: base64label,
124+ Description: description,
125+ ExtendedProperties: properties,
126+ }
127+}
128+
129 //
130 // Deployment bits
131 //
132
133=== modified file 'xmlobjects_test.go'
134--- xmlobjects_test.go 2013-07-04 04:27:34 +0000
135+++ xmlobjects_test.go 2013-07-04 07:13:25 +0000
136@@ -1201,6 +1201,61 @@
137 c.Check(cssi.GeoReplicationEnabled, Equals, "false")
138 }
139
140+func makeUpdateHostedService(label, description string, property ExtendedProperty) string {
141+ template := dedent.Dedent(`
142+ <UpdateHostedService xmlns="http://schemas.microsoft.com/windowsazure">
143+ <Label>%s</Label>
144+ <Description>%s</Description>
145+ <ExtendedProperties>
146+ <ExtendedProperty>
147+ <Name>%s</Name>
148+ <Value>%s</Value>
149+ </ExtendedProperty>
150+ </ExtendedProperties>
151+ </UpdateHostedService>`)
152+ return fmt.Sprintf(template, label, description, property.Name, property.Value)
153+}
154+
155+func (suite *xmlSuite) TestUpdateHostedService(c *C) {
156+ label := MakeRandomString(10)
157+ description := MakeRandomString(10)
158+ property := ExtendedProperty{
159+ Name: "property-name",
160+ Value: "property-value",
161+ }
162+ expected := makeUpdateHostedService(label, description, property)
163+ input := UpdateHostedService{
164+ XMLNS: XMLNS,
165+ Label: label,
166+ Description: description,
167+ ExtendedProperties: []ExtendedProperty{
168+ property,
169+ },
170+ }
171+
172+ observed, err := input.Serialize()
173+ c.Assert(err, IsNil)
174+ c.Assert(observed, Equals, expected)
175+}
176+
177+func (suite *xmlSuite) TestNewUpdateHostedService(c *C) {
178+ label := MakeRandomString(10)
179+ description := MakeRandomString(10)
180+ properties := []ExtendedProperty{
181+ {
182+ Name: MakeRandomString(10),
183+ Value: MakeRandomString(10),
184+ },
185+ }
186+ updateHostedService := NewUpdateHostedService(
187+ label, description, properties)
188+ c.Check(
189+ updateHostedService.Label, Equals,
190+ base64.StdEncoding.EncodeToString([]byte(label)))
191+ c.Check(updateHostedService.Description, Equals, description)
192+ c.Check(updateHostedService.ExtendedProperties, DeepEquals, properties)
193+}
194+
195 func (suite *xmlSuite) TestBlockListSerialize(c *C) {
196 blockList := &BlockList{
197 XMLName: xml.Name{Local: "BlockList"},

Subscribers

People subscribed via source and target branches

to all changes: