launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/gomaasapi/record-action-params into lp:gomaasapi.

Commit message:
Make the testservice record the url.Values posted when performing an operation (e.g. starting a node).

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~rvb/gomaasapi/record-action-params/+merge/158117

I choose to add another field named "nodeOperationRequestValues" instead of making the existing one (TestServer.nodeOperations) more complex because the new structure would have been a pain to manipulate in Go.

The main reason behind that change is so that we can check that the provider sends the correct "user data" when starting a node: https://code.launchpad.net/~rvb/juju-core/fix-test-start-instance/+merge/158116.
-- 
https://code.launchpad.net/~rvb/gomaasapi/record-action-params/+merge/158117
Your team MAAS Maintainers is requested to review the proposed merge of lp:~rvb/gomaasapi/record-action-params into lp:gomaasapi.

=== modified file 'testservice.go'
--- testservice.go	2013-03-12 16:27:02 +0000
+++ testservice.go	2013-04-10 14:03:24 +0000
@@ -53,13 +53,14 @@
 // library.
 type TestServer struct {
 	*httptest.Server
-	serveMux       *http.ServeMux
-	client         Client
-	nodes          map[string]MAASObject
-	ownedNodes     map[string]bool
-	nodeOperations map[string][]string
-	files          map[string]MAASObject
-	version        string
+	serveMux                   *http.ServeMux
+	client                     Client
+	nodes                      map[string]MAASObject
+	ownedNodes                 map[string]bool
+	nodeOperations             map[string][]string
+	nodeOperationRequestValues map[string][]url.Values
+	files                      map[string]MAASObject
+	version                    string
 }
 
 func getNodeURI(version, systemId string) string {
@@ -78,6 +79,7 @@
 	server.nodes = make(map[string]MAASObject)
 	server.ownedNodes = make(map[string]bool)
 	server.nodeOperations = make(map[string][]string)
+	server.nodeOperationRequestValues = make(map[string][]url.Values)
 	server.files = make(map[string]MAASObject)
 }
 
@@ -87,14 +89,37 @@
 	return server.nodeOperations
 }
 
-func (server *TestServer) addNodeOperation(systemId, operation string) {
+// NodeOperationRequestValues returns the map containing the list of the
+// url.Values extracted from the request used when performing operations
+// on nodes.
+func (server *TestServer) NodeOperationRequestValues() map[string][]url.Values {
+	return server.nodeOperationRequestValues
+}
+
+func (server *TestServer) addNodeOperation(systemId, operation string, request *http.Request) {
 	operations, present := server.nodeOperations[systemId]
+	operationRequestValues, _ := server.nodeOperationRequestValues[systemId]
+	requestValues := url.Values{}
+	if request.Body != nil {
+		defer request.Body.Close()
+		body, err := ioutil.ReadAll(request.Body)
+		if err != nil {
+			panic(err)
+		}
+		requestValues, err = url.ParseQuery(string(body))
+		if err != nil {
+			panic(err)
+		}
+	}
 	if !present {
 		operations = []string{operation}
+		operationRequestValues = []url.Values{requestValues}
 	} else {
 		operations = append(operations, operation)
+		operationRequestValues = append(operationRequestValues, requestValues)
 	}
 	server.nodeOperations[systemId] = operations
+	server.nodeOperationRequestValues[systemId] = operationRequestValues
 }
 
 // NewNode creates a MAAS node.  The provided string should be a valid json
@@ -245,7 +270,7 @@
 		// The only operations supported are "start", "stop" and "release".
 		if operation == "start" || operation == "stop" || operation == "release" {
 			// Record operation on node.
-			server.addNodeOperation(systemId, operation)
+			server.addNodeOperation(systemId, operation, r)
 
 			if operation == "release" {
 				delete(server.OwnedNodes(), systemId)

=== modified file 'testservice_test.go'
--- testservice_test.go	2013-03-12 16:27:02 +0000
+++ testservice_test.go	2013-04-10 14:03:24 +0000
@@ -97,26 +97,41 @@
 func (suite *TestServerSuite) TestClearClearsData(c *C) {
 	input := `{"system_id": "mysystemid"}`
 	suite.server.NewNode(input)
-	suite.server.addNodeOperation("mysystemid", "start")
+	suite.server.addNodeOperation("mysystemid", "start", &http.Request{})
 
 	suite.server.Clear()
 
 	c.Check(len(suite.server.nodes), Equals, 0)
 	c.Check(len(suite.server.nodeOperations), Equals, 0)
+	c.Check(len(suite.server.nodeOperationRequestValues), Equals, 0)
 }
 
 func (suite *TestServerSuite) TestAddNodeOperationPopulatesOperations(c *C) {
 	input := `{"system_id": "mysystemid"}`
 	suite.server.NewNode(input)
 
-	suite.server.addNodeOperation("mysystemid", "start")
-	suite.server.addNodeOperation("mysystemid", "stop")
+	suite.server.addNodeOperation("mysystemid", "start", &http.Request{})
+	suite.server.addNodeOperation("mysystemid", "stop", &http.Request{})
 
 	nodeOperations := suite.server.NodeOperations()
 	operations := nodeOperations["mysystemid"]
 	c.Check(operations, DeepEquals, []string{"start", "stop"})
 }
 
+func (suite *TestServerSuite) TestAddNodeOperationPopulatesOperationRequestValues(c *C) {
+	input := `{"system_id": "mysystemid"}`
+	suite.server.NewNode(input)
+	reader := strings.NewReader("key=value")
+	request, err := http.NewRequest("POST", "http://example.com/";, reader)
+	c.Assert(err, IsNil)
+
+	suite.server.addNodeOperation("mysystemid", "start", request)
+
+	values := suite.server.NodeOperationRequestValues()
+	value := values["mysystemid"]
+	c.Check(value[0], DeepEquals, url.Values{"key": []string{"value"}})
+}
+
 func (suite *TestServerSuite) TestNewNodeRequiresJSONString(c *C) {
 	input := `invalid:json`
 	defer func() {