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

[Julian Edwards]

Add UpdateHostedService api call.

[Raphaël Badin (rvb)]

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)]

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

[lp:~julian-edwards/gwacl/updatehostedservice]

162. By Julian Edwards on 2013-07-04

improvements after the review

=== 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 @@
     return hostedServices.HostedServices, err
 }
 
+// UpdateHostedService updates the provided values on the named service.
+// Use NewUpdateHostedService() to create an UpdateHostedService params object.
+func (api *ManagementAPI) UpdateHostedService(serviceName string, params *UpdateHostedService) error {
+    var err error
+    checkPathComponents(serviceName)
+    URI := "services/hostedservices/" + serviceName
+    body, err := params.Serialize()
+    if err != nil {
+        return err
+    }
+    response, err := api.session.put(URI, []byte(body))
+    if err != nil {
+        return err
+    }
+    return api.blockUntilCompleted(response)
+}
+
 // GetHostedServiceProperties loads a HostedService object from the Azure
 // management API.
 func (api *ManagementAPI) GetHostedServiceProperties(
@@ -143,7 +160,8 @@
 }
 
 // AddHostedService adds a hosted service.
-// This is an asynchronous operation, it blocks until the operation is completed.
+// This is an asynchronous operation on Azure, but this call blocks until the
+// operation is completed.
 func (api *ManagementAPI) AddHostedService(definition *CreateHostedService) error {
     URI := "services/hostedservices"
     body, err := marshalXML(definition)
@@ -326,8 +344,8 @@
     checkPathComponents(
         request.ServiceName, request.DeploymentName, request.RoleName)
     url := ("services/hostedservices/" + request.ServiceName +
-            "/deployments/" + request.DeploymentName + "/roles/" +
-            request.RoleName)
+        "/deployments/" + request.DeploymentName + "/roles/" +
+        request.RoleName)
     response, err := api.session.get(url)
     if err != nil {
         return nil, err

=== 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 @@
 package gwacl
 
 import (
+    "encoding/base64"
     "encoding/xml"
     "errors"
     "fmt"
@@ -369,6 +370,35 @@
     c.Assert(descriptors[0].URL, Equals, url)
 }
 
+func (suite *managementBaseAPISuite) TestUpdateHostedService(c *C) {
+    api := makeAPI(c)
+    randomLabel := MakeRandomString(10)
+    randomDescription := MakeRandomString(10)
+    property := ExtendedProperty{
+        Name:  "property-name",
+        Value: "property-value",
+    }
+    base64Label := base64.StdEncoding.EncodeToString([]byte(randomLabel))
+    requestPayload := []byte(makeUpdateHostedService(base64Label, randomDescription, property))
+    fixedResponse := x509Response{
+        StatusCode: http.StatusOK,
+    }
+    rigFixedResponseDispatcher(&fixedResponse)
+    recordedRequests := make([]*X509Request, 0)
+    rigRecordingDispatcher(&recordedRequests)
+
+    serviceName := MakeRandomString(10)
+    properties := []ExtendedProperty{
+        property,
+    }
+    update := NewUpdateHostedService(randomLabel, randomDescription, properties)
+    err := api.UpdateHostedService(serviceName, update)
+
+    c.Assert(err, IsNil)
+    expectedURL := AZURE_URL + api.session.subscriptionId + "/services/hostedservices/" + serviceName
+    checkOneRequest(c, &recordedRequests, expectedURL, requestPayload, "PUT")
+}
+
 func (suite *managementBaseAPISuite) TestGetHostedServiceProperties_withoutDetails(c *C) {
     api := makeAPI(c)
     body := `

=== 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 @@
     return xml.Unmarshal(data, s)
 }
 
+// UpdateHostedService contains the details necessary to call the
+// UpdateHostedService management API call.
+// See http://msdn.microsoft.com/en-us/library/windowsazure/gg441303.aspx
+type UpdateHostedService struct {
+    XMLNS              string             `xml:"xmlns,attr"`
+    Label              string             `xml:"Label,omitempty"` // base64-encoded
+    Description        string             `xml:"Description,omitempty"`
+    ExtendedProperties []ExtendedProperty `xml:"ExtendedProperties>ExtendedProperty,omitempty"`
+}
+
+func (u *UpdateHostedService) Serialize() (string, error) {
+    return toxml(u)
+}
+
+func NewUpdateHostedService(label, description string, properties []ExtendedProperty) *UpdateHostedService {
+    base64label := base64.StdEncoding.EncodeToString([]byte(label))
+    return &UpdateHostedService{
+        XMLNS:              XMLNS,
+        Label:              base64label,
+        Description:        description,
+        ExtendedProperties: properties,
+    }
+}
+
 //
 // Deployment bits
 //

=== 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 @@
     c.Check(cssi.GeoReplicationEnabled, Equals, "false")
 }
 
+func makeUpdateHostedService(label, description string, property ExtendedProperty) string {
+    template := dedent.Dedent(`
+        <UpdateHostedService xmlns="http://schemas.microsoft.com/windowsazure">
+          <Label>%s</Label>
+          <Description>%s</Description>
+          <ExtendedProperties>
+            <ExtendedProperty>
+              <Name>%s</Name>
+              <Value>%s</Value>
+            </ExtendedProperty>
+          </ExtendedProperties>
+        </UpdateHostedService>`)
+    return fmt.Sprintf(template, label, description, property.Name, property.Value)
+}
+
+func (suite *xmlSuite) TestUpdateHostedService(c *C) {
+    label := MakeRandomString(10)
+    description := MakeRandomString(10)
+    property := ExtendedProperty{
+        Name:  "property-name",
+        Value: "property-value",
+    }
+    expected := makeUpdateHostedService(label, description, property)
+    input := UpdateHostedService{
+        XMLNS:       XMLNS,
+        Label:       label,
+        Description: description,
+        ExtendedProperties: []ExtendedProperty{
+            property,
+        },
+    }
+
+    observed, err := input.Serialize()
+    c.Assert(err, IsNil)
+    c.Assert(observed, Equals, expected)
+}
+
+func (suite *xmlSuite) TestNewUpdateHostedService(c *C) {
+    label := MakeRandomString(10)
+    description := MakeRandomString(10)
+    properties := []ExtendedProperty{
+        {
+            Name:  MakeRandomString(10),
+            Value: MakeRandomString(10),
+        },
+    }
+    updateHostedService := NewUpdateHostedService(
+        label, description, properties)
+    c.Check(
+        updateHostedService.Label, Equals,
+        base64.StdEncoding.EncodeToString([]byte(label)))
+    c.Check(updateHostedService.Description, Equals, description)
+    c.Check(updateHostedService.ExtendedProperties, DeepEquals, properties)
+}
+
 func (suite *xmlSuite) TestBlockListSerialize(c *C) {
     blockList := &BlockList{
         XMLName: xml.Name{Local: "BlockList"},