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
=== modified file 'management_base.go'
--- management_base.go 2013-07-03 00:10:00 +0000
+++ management_base.go 2013-07-04 07:13:25 +0000
@@ -121,6 +121,23 @@
121 return hostedServices.HostedServices, err121 return hostedServices.HostedServices, err
122}122}
123123
124// UpdateHostedService updates the provided values on the named service.
125// Use NewUpdateHostedService() to create an UpdateHostedService params object.
126func (api *ManagementAPI) UpdateHostedService(serviceName string, params *UpdateHostedService) error {
127 var err error
128 checkPathComponents(serviceName)
129 URI := "services/hostedservices/" + serviceName
130 body, err := params.Serialize()
131 if err != nil {
132 return err
133 }
134 response, err := api.session.put(URI, []byte(body))
135 if err != nil {
136 return err
137 }
138 return api.blockUntilCompleted(response)
139}
140
124// GetHostedServiceProperties loads a HostedService object from the Azure141// GetHostedServiceProperties loads a HostedService object from the Azure
125// management API.142// management API.
126func (api *ManagementAPI) GetHostedServiceProperties(143func (api *ManagementAPI) GetHostedServiceProperties(
@@ -143,7 +160,8 @@
143}160}
144161
145// AddHostedService adds a hosted service.162// AddHostedService adds a hosted service.
146// This is an asynchronous operation, it blocks until the operation is completed.163// This is an asynchronous operation on Azure, but this call blocks until the
164// operation is completed.
147func (api *ManagementAPI) AddHostedService(definition *CreateHostedService) error {165func (api *ManagementAPI) AddHostedService(definition *CreateHostedService) error {
148 URI := "services/hostedservices"166 URI := "services/hostedservices"
149 body, err := marshalXML(definition)167 body, err := marshalXML(definition)
@@ -326,8 +344,8 @@
326 checkPathComponents(344 checkPathComponents(
327 request.ServiceName, request.DeploymentName, request.RoleName)345 request.ServiceName, request.DeploymentName, request.RoleName)
328 url := ("services/hostedservices/" + request.ServiceName +346 url := ("services/hostedservices/" + request.ServiceName +
329 "/deployments/" + request.DeploymentName + "/roles/" +347 "/deployments/" + request.DeploymentName + "/roles/" +
330 request.RoleName)348 request.RoleName)
331 response, err := api.session.get(url)349 response, err := api.session.get(url)
332 if err != nil {350 if err != nil {
333 return nil, err351 return nil, err
334352
=== modified file 'management_base_test.go'
--- management_base_test.go 2013-07-04 04:33:05 +0000
+++ management_base_test.go 2013-07-04 07:13:25 +0000
@@ -4,6 +4,7 @@
4package gwacl4package gwacl
55
6import (6import (
7 "encoding/base64"
7 "encoding/xml"8 "encoding/xml"
8 "errors"9 "errors"
9 "fmt"10 "fmt"
@@ -369,6 +370,35 @@
369 c.Assert(descriptors[0].URL, Equals, url)370 c.Assert(descriptors[0].URL, Equals, url)
370}371}
371372
373func (suite *managementBaseAPISuite) TestUpdateHostedService(c *C) {
374 api := makeAPI(c)
375 randomLabel := MakeRandomString(10)
376 randomDescription := MakeRandomString(10)
377 property := ExtendedProperty{
378 Name: "property-name",
379 Value: "property-value",
380 }
381 base64Label := base64.StdEncoding.EncodeToString([]byte(randomLabel))
382 requestPayload := []byte(makeUpdateHostedService(base64Label, randomDescription, property))
383 fixedResponse := x509Response{
384 StatusCode: http.StatusOK,
385 }
386 rigFixedResponseDispatcher(&fixedResponse)
387 recordedRequests := make([]*X509Request, 0)
388 rigRecordingDispatcher(&recordedRequests)
389
390 serviceName := MakeRandomString(10)
391 properties := []ExtendedProperty{
392 property,
393 }
394 update := NewUpdateHostedService(randomLabel, randomDescription, properties)
395 err := api.UpdateHostedService(serviceName, update)
396
397 c.Assert(err, IsNil)
398 expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices/" + serviceName
399 checkOneRequest(c, &recordedRequests, expectedURL, requestPayload, "PUT")
400}
401
372func (suite *managementBaseAPISuite) TestGetHostedServiceProperties_withoutDetails(c *C) {402func (suite *managementBaseAPISuite) TestGetHostedServiceProperties_withoutDetails(c *C) {
373 api := makeAPI(c)403 api := makeAPI(c)
374 body := `404 body := `
375405
=== modified file 'xmlobjects.go'
--- xmlobjects.go 2013-07-04 03:58:15 +0000
+++ xmlobjects.go 2013-07-04 07:13:25 +0000
@@ -381,6 +381,30 @@
381 return xml.Unmarshal(data, s)381 return xml.Unmarshal(data, s)
382}382}
383383
384// UpdateHostedService contains the details necessary to call the
385// UpdateHostedService management API call.
386// See http://msdn.microsoft.com/en-us/library/windowsazure/gg441303.aspx
387type UpdateHostedService struct {
388 XMLNS string `xml:"xmlns,attr"`
389 Label string `xml:"Label,omitempty"` // base64-encoded
390 Description string `xml:"Description,omitempty"`
391 ExtendedProperties []ExtendedProperty `xml:"ExtendedProperties>ExtendedProperty,omitempty"`
392}
393
394func (u *UpdateHostedService) Serialize() (string, error) {
395 return toxml(u)
396}
397
398func NewUpdateHostedService(label, description string, properties []ExtendedProperty) *UpdateHostedService {
399 base64label := base64.StdEncoding.EncodeToString([]byte(label))
400 return &UpdateHostedService{
401 XMLNS: XMLNS,
402 Label: base64label,
403 Description: description,
404 ExtendedProperties: properties,
405 }
406}
407
384//408//
385// Deployment bits409// Deployment bits
386//410//
387411
=== modified file 'xmlobjects_test.go'
--- xmlobjects_test.go 2013-07-04 04:27:34 +0000
+++ xmlobjects_test.go 2013-07-04 07:13:25 +0000
@@ -1201,6 +1201,61 @@
1201 c.Check(cssi.GeoReplicationEnabled, Equals, "false")1201 c.Check(cssi.GeoReplicationEnabled, Equals, "false")
1202}1202}
12031203
1204func makeUpdateHostedService(label, description string, property ExtendedProperty) string {
1205 template := dedent.Dedent(`
1206 <UpdateHostedService xmlns="http://schemas.microsoft.com/windowsazure">
1207 <Label>%s</Label>
1208 <Description>%s</Description>
1209 <ExtendedProperties>
1210 <ExtendedProperty>
1211 <Name>%s</Name>
1212 <Value>%s</Value>
1213 </ExtendedProperty>
1214 </ExtendedProperties>
1215 </UpdateHostedService>`)
1216 return fmt.Sprintf(template, label, description, property.Name, property.Value)
1217}
1218
1219func (suite *xmlSuite) TestUpdateHostedService(c *C) {
1220 label := MakeRandomString(10)
1221 description := MakeRandomString(10)
1222 property := ExtendedProperty{
1223 Name: "property-name",
1224 Value: "property-value",
1225 }
1226 expected := makeUpdateHostedService(label, description, property)
1227 input := UpdateHostedService{
1228 XMLNS: XMLNS,
1229 Label: label,
1230 Description: description,
1231 ExtendedProperties: []ExtendedProperty{
1232 property,
1233 },
1234 }
1235
1236 observed, err := input.Serialize()
1237 c.Assert(err, IsNil)
1238 c.Assert(observed, Equals, expected)
1239}
1240
1241func (suite *xmlSuite) TestNewUpdateHostedService(c *C) {
1242 label := MakeRandomString(10)
1243 description := MakeRandomString(10)
1244 properties := []ExtendedProperty{
1245 {
1246 Name: MakeRandomString(10),
1247 Value: MakeRandomString(10),
1248 },
1249 }
1250 updateHostedService := NewUpdateHostedService(
1251 label, description, properties)
1252 c.Check(
1253 updateHostedService.Label, Equals,
1254 base64.StdEncoding.EncodeToString([]byte(label)))
1255 c.Check(updateHostedService.Description, Equals, description)
1256 c.Check(updateHostedService.ExtendedProperties, DeepEquals, properties)
1257}
1258
1204func (suite *xmlSuite) TestBlockListSerialize(c *C) {1259func (suite *xmlSuite) TestBlockListSerialize(c *C) {
1205 blockList := &BlockList{1260 blockList := &BlockList{
1206 XMLName: xml.Name{Local: "BlockList"},1261 XMLName: xml.Name{Local: "BlockList"},

Subscribers

People subscribed via source and target branches

to all changes: