Merge lp:~niemeyer/juju-core/fix-tools-marshaling into lp:~juju/juju-core/trunk

[Gustavo Niemeyer]

state: fix tools marshaling

https://codereview.appspot.com/6573050/

[Gustavo Niemeyer (niemeyer)]

Reviewers: mp+126363_code.launchpad.net,

Message:
Please take a look.

Description:
state: fix tools marshaling

https://code.launchpad.net/~niemeyer/juju-core/fix-tools-marshaling/+merge/126363

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6573050/

Affected files:
   A [revision details]
   M state/state.go
   M state/tools_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: state/state.go
=== modified file 'state/state.go'
--- state/state.go 2012-09-25 17:34:03 +0000
+++ state/state.go 2012-09-26 02:02:34 +0000
@@ -18,6 +18,7 @@
   "regexp"
  )

+// TODO(niemeyer): This must not be exported.
  type D []bson.DocElem

  // Tools describes a particular set of juju tools and where to find them.
@@ -26,6 +27,25 @@
   URL string
  }

+type toolsDoc struct {
+ Version version.Binary
+ URL string
+}
+
+func (t *Tools) GetBSON() (interface{}, error) {
+ return &toolsDoc{t.Binary, t.URL}, nil
+}
+
+func (t *Tools) SetBSON(raw bson.Raw) error {
+ var doc toolsDoc
+ if err := raw.Unmarshal(&doc); err != nil {
+ return err
+ }
+ t.Binary = doc.Version
+ t.URL = doc.URL
+ return nil
+}
+
  var (
   validService =
regexp.MustCompile("^[a-z][a-z0-9]*(-[a-z0-9]*[a-z][a-z0-9]*)*$")
   validUnit =
regexp.MustCompile("^[a-z][a-z0-9]*(-[a-z0-9]*[a-z][a-z0-9]*)*/[0-9]+$")

Index: state/tools_test.go
=== modified file 'state/tools_test.go'
--- state/tools_test.go 2012-09-21 20:31:39 +0000
+++ state/tools_test.go 2012-09-26 02:02:34 +0000
@@ -2,6 +2,7 @@

  import (
   "fmt"
+ "labix.org/v2/mgo/bson"
   . "launchpad.net/gocheck"
   "launchpad.net/juju-core/state"
   "launchpad.net/juju-core/version"
@@ -30,8 +31,6 @@
  }

  func testAgentTools(c *C, obj tooler, agent string) {
- c.Skip("Marshalling of agent tools is currently broken")
-
   // object starts with zero'd tools.
   t, err := obj.AgentTools()
   c.Assert(err, IsNil)
@@ -70,3 +69,25 @@
   c.Assert(err, IsNil)
   testAgentTools(c, unit, `unit "wordpress/0"`)
  }
+
+func (s *ToolsSuite) TestMarshalUnmarshal(c *C) {
+ tools := newTools("7.8.9-foo-bar", "http://arble.tgz")
+ data, err := bson.Marshal(&tools)
+ c.Assert(err, IsNil)
+
+ // Check the exact document.
+ want := bson.M{
+ "version": tools.Binary.String(),
+ "url": tools.URL,
+ }
+ got := bson.M{}
+ err = bson.Unmarshal(data, &got)
+ c.Assert(err, IsNil)
+ c.Assert(got, DeepEquals, want)
+
+ // Check that it unpacks properly too.
+ var t state.Tools
+ err = bson.Unmarshal(data, &t)
+ c.Assert(err, IsNil)
+ c.Assert(t, Equals, *tools)
+}

[Gustavo Niemeyer (niemeyer)]

*** Submitted:

state: fix tools marshaling

R=dfc
CC=
https://codereview.appspot.com/6573050

https://codereview.appspot.com/6573050/

=== modified file 'state/state.go'
--- state/state.go	2012-09-25 17:34:03 +0000
+++ state/state.go	2012-09-26 02:04:24 +0000
@@ -18,6 +18,7 @@
 	"regexp"
 )
 
+// TODO(niemeyer): This must not be exported.
 type D []bson.DocElem
 
 // Tools describes a particular set of juju tools and where to find them.
@@ -26,6 +27,25 @@
 	URL string
 }
 
+type toolsDoc struct {
+	Version version.Binary
+	URL     string
+}
+
+func (t *Tools) GetBSON() (interface{}, error) {
+	return &toolsDoc{t.Binary, t.URL}, nil
+}
+
+func (t *Tools) SetBSON(raw bson.Raw) error {
+	var doc toolsDoc
+	if err := raw.Unmarshal(&doc); err != nil {
+		return err
+	}
+	t.Binary = doc.Version
+	t.URL = doc.URL
+	return nil
+}
+
 var (
 	validService = regexp.MustCompile("^[a-z][a-z0-9]*(-[a-z0-9]*[a-z][a-z0-9]*)*$")
 	validUnit    = regexp.MustCompile("^[a-z][a-z0-9]*(-[a-z0-9]*[a-z][a-z0-9]*)*/[0-9]+$")

=== modified file 'state/tools_test.go'
--- state/tools_test.go	2012-09-21 20:31:39 +0000
+++ state/tools_test.go	2012-09-26 02:04:24 +0000
@@ -2,6 +2,7 @@
 
 import (
 	"fmt"
+	"labix.org/v2/mgo/bson"
 	. "launchpad.net/gocheck"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/version"
@@ -30,8 +31,6 @@
 }
 
 func testAgentTools(c *C, obj tooler, agent string) {
-	c.Skip("Marshalling of agent tools is currently broken")
-
 	// object starts with zero'd tools.
 	t, err := obj.AgentTools()
 	c.Assert(err, IsNil)
@@ -70,3 +69,25 @@
 	c.Assert(err, IsNil)
 	testAgentTools(c, unit, `unit "wordpress/0"`)
 }
+
+func (s *ToolsSuite) TestMarshalUnmarshal(c *C) {
+	tools := newTools("7.8.9-foo-bar", "http://arble.tgz")
+	data, err := bson.Marshal(&tools)
+	c.Assert(err, IsNil)
+
+	// Check the exact document.
+	want := bson.M{
+		"version": tools.Binary.String(),
+		"url":     tools.URL,
+	}
+	got := bson.M{}
+	err = bson.Unmarshal(data, &got)
+	c.Assert(err, IsNil)
+	c.Assert(got, DeepEquals, want)
+
+	// Check that it unpacks properly too.
+	var t state.Tools
+	err = bson.Unmarshal(data, &t)
+	c.Assert(err, IsNil)
+	c.Assert(t, Equals, *tools)
+}