← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/gomaasapi/single-wrapper-class into lp:gomaasapi

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/gomaasapi/single-wrapper-class into lp:gomaasapi.

Commit message:
Rearrange JSONObject so its conversions are all in a single class.  Prepares for supporting non-JSON data.  Also, document more functions.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/gomaasapi/single-wrapper-class/+merge/147643

After this, the JSONObject you get back from a result will also store its original raw HTTP response as []byte.  You can then query that root JSONObject, in addition to the types you already get (string, number, array, object, MAAS object, bool), as raw data.

The interfaces have become classes.  I also added some documentation along the way, in some cases because things weren't clear to myself and so I thought the next reader would appreciate a note.

A large portion of the diff (which is ginormous, sorry) is accounted for by removing casts between Go types and the jsonX types that formerly wrapped them.  We can't have wrapper types that are direct aliases of the contained types any more, since we'll need to store some extra data in JSONObject.

Because there is now just a single wrapper type, it's no longer just MAASObject that knows its Client.  Every JSONObject now does.  A MAASObject is only constructed on the fly, when you ask for one.  Call GetMAASObject() twice on the same JSONObject and you'll get two separate instances (with identical contents of course).  Which is probably safer, although if you were modifying MAASObjects in place you may have been asking for trouble anyway.

There is one unfortunate API change: a JSONObject itself can no longer be nil; it can just wrap a nil value.  So before using a JSONObject, call IsNil() on it now, instead of comparing it to nil.  I hope it's early enough in this API's lifetime that such changes are still feasible.


Jeroen
-- 
https://code.launchpad.net/~jtv/gomaasapi/single-wrapper-class/+merge/147643
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/gomaasapi/single-wrapper-class into lp:gomaasapi.
=== modified file 'client.go'
--- client.go	2013-02-05 14:27:56 +0000
+++ client.go	2013-02-11 12:31:19 +0000
@@ -19,6 +19,7 @@
 	Signer  OAuthSigner
 }
 
+// dispatchRequest sends a request to the server, and interprets the response.
 func (client Client) dispatchRequest(request *http.Request) ([]byte, error) {
 	client.Signer.OAuthSign(request)
 	httpClient := http.Client{}
@@ -36,10 +37,16 @@
 	return body, nil
 }
 
+// GetURL returns the URL to a given resource on the API, based on its URI.
+// The resource URI may be absolute or relative; either way the result is a
+// full absolute URL including the network part.
 func (client Client) GetURL(uri *url.URL) *url.URL {
 	return client.BaseURL.ResolveReference(uri)
 }
 
+// Get performs an HTTP "GET" to the API.  This may be either an API method
+// invocation (if you pass its name in "operation") or plain resource
+// retrieval (if you leave "operation" blank).
 func (client Client) Get(uri *url.URL, operation string, parameters url.Values) ([]byte, error) {
 	opParameter := parameters.Get("op")
 	if opParameter != "" {
@@ -58,7 +65,8 @@
 	return client.dispatchRequest(request)
 }
 
-// nonIdempotentRequest is a utility method to issue a PUT or a POST request.
+// nonIdempotentRequest implements the common functionality of PUT and POST
+// requests (but not GET or DELETE requests).
 func (client Client) nonIdempotentRequest(method string, uri *url.URL, parameters url.Values) ([]byte, error) {
 	url := client.GetURL(uri)
 	request, err := http.NewRequest(method, url.String(), strings.NewReader(string(parameters.Encode())))
@@ -69,16 +77,21 @@
 	return client.dispatchRequest(request)
 }
 
+// Post performs an HTTP "POST" to the API.  This may be either an API method
+// invocation (if you pass its name in "operation") or plain resource
+// retrieval (if you leave "operation" blank).
 func (client Client) Post(uri *url.URL, operation string, parameters url.Values) ([]byte, error) {
 	queryParams := url.Values{"op": {operation}}
 	uri.RawQuery = queryParams.Encode()
 	return client.nonIdempotentRequest("POST", uri, parameters)
 }
 
+// Put updates an object on the API, using an HTTP "PUT" request.
 func (client Client) Put(uri *url.URL, parameters url.Values) ([]byte, error) {
 	return client.nonIdempotentRequest("PUT", uri, parameters)
 }
 
+// Delete deletes an object on the API, using an HTTP "DELETE" request.
 func (client Client) Delete(uri *url.URL) error {
 	url := client.GetURL(uri)
 	request, err := http.NewRequest("DELETE", url.String(), strings.NewReader(""))
@@ -92,13 +105,14 @@
 	return nil
 }
 
+// Anonymous "signature method" implementation.
 type anonSigner struct{}
 
 func (signer anonSigner) OAuthSign(request *http.Request) error {
 	return nil
 }
 
-// Trick to ensure *anonSigner implements the OAuthSigner interface.
+// *anonSigner implements the OAuthSigner interface.
 var _ OAuthSigner = anonSigner{}
 
 // NewAnonymousClient creates a client that issues anonymous requests.

=== modified file 'jsonobject.go'
--- jsonobject.go	2013-02-06 03:54:20 +0000
+++ jsonobject.go	2013-02-11 12:31:19 +0000
@@ -23,79 +23,43 @@
 // There is one exception: a MAASObject is really a special kind of map,
 // so you can read it as either.
 // Reading a null item is also an error.  So before you try obj.Get*(),
-// first check that obj != nil.
-type JSONObject interface {
-	// Type of this value:
-	// "string", "float64", "map", "maasobject", "array", or "bool".
-	Type() string
-	// Read as string.
-	GetString() (string, error)
-	// Read number as float64.
-	GetFloat64() (float64, error)
-	// Read object as map.
-	GetMap() (map[string]JSONObject, error)
-	// Read object as MAAS object.
-	GetMAASObject() (MAASObject, error)
-	// Read list as array.
-	GetArray() ([]JSONObject, error)
-	// Read as bool.
-	GetBool() (bool, error)
+// first check obj.IsNil().
+type JSONObject struct {
+	value  interface{}
+	client Client
 }
 
-// Internally, each JSONObject already knows what type it is.  It just
-// can't tell the caller yet because the caller may not have the right
-// hard-coded variable type.
-// So for each JSON type, there is a separate implementation of JSONObject
-// that converts only to that type.  Any other conversion is an error.
-// One type is special: jsonMAASObject is an object in the MAAS sense.  It
-// behaves just like a jsonMap if you want it to, but it also implements
-// MAASObject.
-type jsonString string
-type jsonFloat64 float64
-type jsonMap map[string]JSONObject
-type jsonArray []JSONObject
-type jsonBool bool
-
 // Our JSON processor distinguishes a MAASObject from a jsonMap by the fact
 // that it contains a key "resource_uri".  (A regular map might contain the
 // same key through sheer coincide, but never mind: you can still treat it
 // as a jsonMap and never notice the difference.)
 const resourceURI = "resource_uri"
 
-// maasify is internal.  It turns a completely untyped json.Unmarshal result
-// into a JSONObject (with the appropriate implementation of course).
-// This function is recursive.  Maps and arrays are deep-copied, with each
-// individual value being converted to a JSONObject type.
+// maasify turns a completely untyped json.Unmarshal result into a JSONObject
+// (with the appropriate implementation of course).  This function is
+// recursive.  Maps and arrays are deep-copied, with each individual value
+// being converted to a JSONObject type.
 func maasify(client Client, value interface{}) JSONObject {
 	if value == nil {
-		return nil
+		return JSONObject{}
 	}
 	switch value.(type) {
-	case string:
-		return jsonString(value.(string))
-	case float64:
-		return jsonFloat64(value.(float64))
+	case string, float64, bool:
+		return JSONObject{value: value}
 	case map[string]interface{}:
 		original := value.(map[string]interface{})
 		result := make(map[string]JSONObject, len(original))
 		for key, value := range original {
 			result[key] = maasify(client, value)
 		}
-		if _, ok := result[resourceURI]; ok {
-			// If the map contains "resource-uri", we can treat
-			// it as a MAAS object.
-			return newJSONMAASObject(result, client)
-		}
-		return jsonMap(result)
+		return JSONObject{value: result, client: client}
 	case []interface{}:
 		original := value.([]interface{})
 		result := make([]JSONObject, len(original))
 		for index, value := range original {
 			result[index] = maasify(client, value)
 		}
-		return jsonArray(result)
-	case bool:
-		return jsonBool(value.(bool))
+		return JSONObject{value: result}
 	}
 	msg := fmt.Sprintf("Unknown JSON type, can't be converted to JSONObject: %v", value)
 	panic(msg)
@@ -106,92 +70,57 @@
 	var obj interface{}
 	err := json.Unmarshal(input, &obj)
 	if err != nil {
-		return nil, err
+		return JSONObject{}, err
 	}
 	return maasify(client, obj), nil
 }
 
 // Return error value for failed type conversion.
 func failConversion(wantedType string, obj JSONObject) error {
-	msg := fmt.Sprintf("Requested %v, got %v.", wantedType, obj.Type())
+	msg := fmt.Sprintf("Requested %v, got %T.", wantedType, obj.value)
 	return errors.New(msg)
 }
 
-// Error return values for failure to convert to string.
-func failString(obj JSONObject) (string, error) {
-	return "", failConversion("string", obj)
-}
-
-// Error return values for failure to convert to float64.
-func failFloat64(obj JSONObject) (float64, error) {
-	return 0.0, failConversion("float64", obj)
-}
-
-// Error return values for failure to convert to map.
-func failMap(obj JSONObject) (map[string]JSONObject, error) {
-	return make(map[string]JSONObject, 0), failConversion("map", obj)
-}
-
-// Error return values for failure to convert to MAAS object.
-func failMAASObject(obj JSONObject) (MAASObject, error) {
-	return jsonMAASObject{}, failConversion("maasobject", obj)
-}
-
-// Error return values for failure to convert to array.
-func failArray(obj JSONObject) ([]JSONObject, error) {
-	return make([]JSONObject, 0), failConversion("array", obj)
-}
-
-// Error return values for failure to convert to bool.
-func failBool(obj JSONObject) (bool, error) {
-	return false, failConversion("bool", obj)
-}
-
-// JSONObject implementation for jsonString.
-func (jsonString) Type() string                               { return "string" }
-func (obj jsonString) GetString() (string, error)             { return string(obj), nil }
-func (obj jsonString) GetFloat64() (float64, error)           { return failFloat64(obj) }
-func (obj jsonString) GetMap() (map[string]JSONObject, error) { return failMap(obj) }
-func (obj jsonString) GetMAASObject() (MAASObject, error)     { return failMAASObject(obj) }
-func (obj jsonString) GetArray() ([]JSONObject, error)        { return failArray(obj) }
-func (obj jsonString) GetBool() (bool, error)                 { return failBool(obj) }
-
-// JSONObject implementation for jsonFloat64.
-func (jsonFloat64) Type() string                               { return "float64" }
-func (obj jsonFloat64) GetString() (string, error)             { return failString(obj) }
-func (obj jsonFloat64) GetFloat64() (float64, error)           { return float64(obj), nil }
-func (obj jsonFloat64) GetMap() (map[string]JSONObject, error) { return failMap(obj) }
-func (obj jsonFloat64) GetMAASObject() (MAASObject, error)     { return failMAASObject(obj) }
-func (obj jsonFloat64) GetArray() ([]JSONObject, error)        { return failArray(obj) }
-func (obj jsonFloat64) GetBool() (bool, error)                 { return failBool(obj) }
-
-// JSONObject implementation for jsonMap.
-func (jsonMap) Type() string                     { return "map" }
-func (obj jsonMap) GetString() (string, error)   { return failString(obj) }
-func (obj jsonMap) GetFloat64() (float64, error) { return failFloat64(obj) }
-func (obj jsonMap) GetMap() (map[string]JSONObject, error) {
-	return (map[string]JSONObject)(obj), nil
-}
-func (obj jsonMap) GetMAASObject() (MAASObject, error) { return failMAASObject(obj) }
-func (obj jsonMap) GetArray() ([]JSONObject, error)    { return failArray(obj) }
-func (obj jsonMap) GetBool() (bool, error)             { return failBool(obj) }
-
-// JSONObject implementation for jsonArray.
-func (jsonArray) Type() string                               { return "array" }
-func (obj jsonArray) GetString() (string, error)             { return failString(obj) }
-func (obj jsonArray) GetFloat64() (float64, error)           { return failFloat64(obj) }
-func (obj jsonArray) GetMap() (map[string]JSONObject, error) { return failMap(obj) }
-func (obj jsonArray) GetMAASObject() (MAASObject, error)     { return failMAASObject(obj) }
-func (obj jsonArray) GetArray() ([]JSONObject, error) {
-	return ([]JSONObject)(obj), nil
-}
-func (obj jsonArray) GetBool() (bool, error) { return failBool(obj) }
-
-// JSONObject implementation for jsonBool.
-func (jsonBool) Type() string                               { return "bool" }
-func (obj jsonBool) GetString() (string, error)             { return failString(obj) }
-func (obj jsonBool) GetFloat64() (float64, error)           { return failFloat64(obj) }
-func (obj jsonBool) GetMap() (map[string]JSONObject, error) { return failMap(obj) }
-func (obj jsonBool) GetMAASObject() (MAASObject, error)     { return failMAASObject(obj) }
-func (obj jsonBool) GetArray() ([]JSONObject, error)        { return failArray(obj) }
-func (obj jsonBool) GetBool() (bool, error)                 { return bool(obj), nil }
+func (obj JSONObject) IsNil() bool {
+	return obj.value == nil
+}
+
+func (obj JSONObject) GetString() (value string, err error) {
+	value, ok := obj.value.(string)
+	if !ok {
+		err = failConversion("string", obj)
+	}
+	return
+}
+
+func (obj JSONObject) GetFloat64() (value float64, err error) {
+	value, ok := obj.value.(float64)
+	if !ok {
+		err = failConversion("float64", obj)
+	}
+	return
+}
+
+func (obj JSONObject) GetMap() (value map[string]JSONObject, err error) {
+	value, ok := obj.value.(map[string]JSONObject)
+	if !ok {
+		err = failConversion("map", obj)
+	}
+	return
+}
+
+func (obj JSONObject) GetArray() (value []JSONObject, err error) {
+	value, ok := obj.value.([]JSONObject)
+	if !ok {
+		err = failConversion("array", obj)
+	}
+	return
+}
+
+func (obj JSONObject) GetBool() (value bool, err error) {
+	value, ok := obj.value.(bool)
+	if !ok {
+		err = failConversion("bool", obj)
+	}
+	return
+}

=== modified file 'jsonobject_test.go'
--- jsonobject_test.go	2013-02-05 14:35:15 +0000
+++ jsonobject_test.go	2013-02-11 12:31:19 +0000
@@ -14,49 +14,59 @@
 
 // maasify() converts nil.
 func (suite *JSONObjectSuite) TestMaasifyConvertsNil(c *C) {
-	c.Check(maasify(Client{}, nil), Equals, nil)
+	c.Check(maasify(Client{}, nil).IsNil(), Equals, true)
 }
 
 // maasify() converts strings.
 func (suite *JSONObjectSuite) TestMaasifyConvertsString(c *C) {
 	const text = "Hello"
-	c.Check(string(maasify(Client{}, text).(jsonString)), Equals, text)
+	out, err := maasify(Client{}, text).GetString()
+	c.Assert(err, IsNil)
+	c.Check(out, Equals, text)
 }
 
 // maasify() converts float64 numbers.
 func (suite *JSONObjectSuite) TestMaasifyConvertsNumber(c *C) {
 	const number = 3.1415926535
-	c.Check(float64(maasify(Client{}, number).(jsonFloat64)), Equals, number)
+	num, err := maasify(Client{}, number).GetFloat64()
+	c.Assert(err, IsNil)
+	c.Check(num, Equals, number)
 }
 
 // maasify() converts array slices.
 func (suite *JSONObjectSuite) TestMaasifyConvertsArray(c *C) {
 	original := []interface{}{3.0, 2.0, 1.0}
-	output := maasify(Client{}, original).(jsonArray)
+	output, err := maasify(Client{}, original).GetArray()
+	c.Assert(err, IsNil)
 	c.Check(len(output), Equals, len(original))
 }
 
 // When maasify() converts an array slice, the result contains JSONObjects.
 func (suite *JSONObjectSuite) TestMaasifyArrayContainsJSONObjects(c *C) {
-	arr := maasify(Client{}, []interface{}{9.9}).(jsonArray)
-	var entry JSONObject
-	entry = arr[0]
-	c.Check((float64)(entry.(jsonFloat64)), Equals, 9.9)
+	arr, err := maasify(Client{}, []interface{}{9.9}).GetArray()
+	c.Assert(err, IsNil)
+	_ = JSONObject(arr[0])
+	entry, err := arr[0].GetFloat64()
+	c.Assert(err, IsNil)
+	c.Check(entry, Equals, 9.9)
 }
 
 // maasify() converts maps.
 func (suite *JSONObjectSuite) TestMaasifyConvertsMap(c *C) {
 	original := map[string]interface{}{"1": "one", "2": "two", "3": "three"}
-	output := maasify(Client{}, original).(jsonMap)
+	output, err := maasify(Client{}, original).GetMap()
+	c.Assert(err, IsNil)
 	c.Check(len(output), Equals, len(original))
 }
 
 // When maasify() converts a map, the result contains JSONObjects.
 func (suite *JSONObjectSuite) TestMaasifyMapContainsJSONObjects(c *C) {
-	mp := maasify(Client{}, map[string]interface{}{"key": "value"}).(jsonMap)
-	var entry JSONObject
-	entry = mp["key"]
-	c.Check((string)(entry.(jsonString)), Equals, "value")
+	jsonobj := maasify(Client{}, map[string]interface{}{"key": "value"})
+	mp, err := jsonobj.GetMap()
+	_ = JSONObject(mp["key"])
+	c.Assert(err, IsNil)
+	entry, err := mp["key"].GetString()
+	c.Check(entry, Equals, "value")
 }
 
 // maasify() converts MAAS objects.
@@ -65,66 +75,89 @@
 		"resource_uri": "http://example.com/foo";,
 		"size":         "3",
 	}
-	output := maasify(Client{}, original).(jsonMAASObject)
-	c.Check(len(output.jsonMap), Equals, len(original))
-	c.Check((string)(output.jsonMap["size"].(jsonString)), Equals, "3")
+	obj, err := maasify(Client{}, original).GetMAASObject()
+	c.Assert(err, IsNil)
+	c.Check(len(obj.GetMap()), Equals, len(original))
+	size, err := obj.GetMap()["size"].GetString()
+	c.Assert(err, IsNil)
+	c.Check(size, Equals, "3")
 }
 
 // maasify() passes its client to a MAASObject it creates.
-func (suite *JSONObjectSuite) TestMaasifyPassesInfoToMAASObject(c *C) {
+func (suite *JSONObjectSuite) TestMaasifyPassesClientToMAASObject(c *C) {
 	client := Client{}
 	original := map[string]interface{}{"resource_uri": "/foo"}
-	output := maasify(client, original).(jsonMAASObject)
+	output, err := maasify(client, original).GetMAASObject()
+	c.Assert(err, IsNil)
 	c.Check(output.client, Equals, client)
 }
 
 // maasify() passes its client into an array of MAASObjects it creates.
-func (suite *JSONObjectSuite) TestMaasifyPassesInfoIntoArray(c *C) {
+func (suite *JSONObjectSuite) TestMaasifyPassesClientIntoArray(c *C) {
 	client := Client{}
 	obj := map[string]interface{}{"resource_uri": "/foo"}
 	list := []interface{}{obj}
-	output := maasify(client, list).(jsonArray)
-	c.Check(output[0].(jsonMAASObject).client, Equals, client)
+	jsonobj, err := maasify(client, list).GetArray()
+	c.Assert(err, IsNil)
+	out, err := jsonobj[0].GetMAASObject()
+	c.Assert(err, IsNil)
+	c.Check(out.client, Equals, client)
 }
 
 // maasify() passes its client into a map of MAASObjects it creates.
-func (suite *JSONObjectSuite) TestMaasifyPassesInfoIntoMap(c *C) {
+func (suite *JSONObjectSuite) TestMaasifyPassesClientIntoMap(c *C) {
 	client := Client{}
 	obj := map[string]interface{}{"resource_uri": "/foo"}
 	mp := map[string]interface{}{"key": obj}
-	output := maasify(client, mp).(jsonMap)
-	c.Check(output["key"].(jsonMAASObject).client, Equals, client)
+	jsonobj, err := maasify(client, mp).GetMap()
+	c.Assert(err, IsNil)
+	out, err := jsonobj["key"].GetMAASObject()
+	c.Assert(err, IsNil)
+	c.Check(out.client, Equals, client)
 }
 
 // maasify() passes its client all the way down into any MAASObjects in the
 // object structure it creates.
-func (suite *JSONObjectSuite) TestMaasifyPassesInfoAllTheWay(c *C) {
+func (suite *JSONObjectSuite) TestMaasifyPassesClientAllTheWay(c *C) {
 	client := Client{}
 	obj := map[string]interface{}{"resource_uri": "/foo"}
 	mp := map[string]interface{}{"key": obj}
 	list := []interface{}{mp}
-	output := maasify(client, list).(jsonArray)
-	maasobj := output[0].(jsonMap)["key"]
-	c.Check(maasobj.(jsonMAASObject).client, Equals, client)
+	jsonobj, err := maasify(client, list).GetArray()
+	c.Assert(err, IsNil)
+	outerMap, err := jsonobj[0].GetMap()
+	c.Assert(err, IsNil)
+	out, err := outerMap["key"].GetMAASObject()
+	c.Assert(err, IsNil)
+	c.Check(out.client, Equals, client)
 }
 
 // maasify() converts Booleans.
 func (suite *JSONObjectSuite) TestMaasifyConvertsBool(c *C) {
-	c.Check(bool(maasify(Client{}, true).(jsonBool)), Equals, true)
-	c.Check(bool(maasify(Client{}, false).(jsonBool)), Equals, false)
+	t, err := maasify(Client{}, true).GetBool()
+	c.Assert(err, IsNil)
+	f, err := maasify(Client{}, false).GetBool()
+	c.Assert(err, IsNil)
+	c.Check(t, Equals, true)
+	c.Check(f, Equals, false)
 }
 
 // Parse takes you from a JSON blob to a JSONObject.
 func (suite *JSONObjectSuite) TestParseMaasifiesJSONBlob(c *C) {
 	blob := []byte("[12]")
 	obj, err := Parse(Client{}, blob)
-	c.Check(err, IsNil)
-	c.Check(float64(obj.(jsonArray)[0].(jsonFloat64)), Equals, 12.0)
+	c.Assert(err, IsNil)
+
+	arr, err := obj.GetArray()
+	c.Assert(err, IsNil)
+	out, err := arr[0].GetFloat64()
+	c.Assert(err, IsNil)
+	c.Check(out, Equals, 12.0)
 }
 
 // String-type JSONObjects convert only to string.
 func (suite *JSONObjectSuite) TestConversionsString(c *C) {
-	obj := jsonString("Test string")
+	obj := maasify(Client{}, "Test string")
 
 	value, err := obj.GetString()
 	c.Check(err, IsNil)
@@ -144,7 +177,7 @@
 
 // Number-type JSONObjects convert only to float64.
 func (suite *JSONObjectSuite) TestConversionsFloat64(c *C) {
-	obj := jsonFloat64(1.1)
+	obj := maasify(Client{}, 1.1)
 
 	value, err := obj.GetFloat64()
 	c.Check(err, IsNil)
@@ -164,8 +197,7 @@
 
 // Map-type JSONObjects convert only to map.
 func (suite *JSONObjectSuite) TestConversionsMap(c *C) {
-	input := map[string]JSONObject{"x": jsonString("y")}
-	obj := jsonMap(input)
+	obj := maasify(Client{}, map[string]interface{}{"x": "y"})
 
 	value, err := obj.GetMap()
 	c.Check(err, IsNil)
@@ -187,7 +219,7 @@
 
 // Array-type JSONObjects convert only to array.
 func (suite *JSONObjectSuite) TestConversionsArray(c *C) {
-	obj := jsonArray([]JSONObject{jsonString("item")})
+	obj := maasify(Client{}, []interface{}{"item"})
 
 	value, err := obj.GetArray()
 	c.Check(err, IsNil)
@@ -209,7 +241,7 @@
 
 // Boolean-type JSONObjects convert only to bool.
 func (suite *JSONObjectSuite) TestConversionsBool(c *C) {
-	obj := jsonBool(false)
+	obj := maasify(Client{}, false)
 
 	value, err := obj.GetBool()
 	c.Check(err, IsNil)

=== modified file 'maas.go'
--- maas.go	2013-02-05 11:35:09 +0000
+++ maas.go	2013-02-11 12:31:19 +0000
@@ -5,6 +5,6 @@
 
 // NewMAAS returns an interface to the MAAS API as a MAASObject.
 func NewMAAS(client Client) MAASObject {
-	input := map[string]JSONObject{resourceURI: jsonString(client.BaseURL.String())}
-	return newJSONMAASObject(jsonMap(input), client)
+	attrs := map[string]interface{}{resourceURI: client.BaseURL.String()}
+	return newJSONMAASObject(attrs, client)
 }

=== modified file 'maasobject.go'
--- maasobject.go	2013-02-07 16:01:48 +0000
+++ maasobject.go	2013-02-11 12:31:19 +0000
@@ -5,88 +5,77 @@
 
 import (
 	"errors"
+	"fmt"
 	"net/url"
 )
 
 // MAASObject represents a MAAS object as returned by the MAAS API, such as a
 // Node or a Tag.
-// This is a special kind of JSONObject.  A MAAS API call will usually return
-// either a MAASObject or a list of MAASObjects.  (The list itself will be
-// wrapped in a JSONObject).
-type MAASObject interface {
-	JSONObject
-
-	// Utility method to extract a string field from this MAAS object.
-	GetField(name string) (string, error)
-	// URL for this MAAS object.
-	URL() *url.URL
-	// Resource URI for this MAAS object.
-	URI() *url.URL
-	// Retrieve the MAAS object located at thisObject.URI()+name.
-	GetSubObject(name string) MAASObject
-	// Retrieve this MAAS object.
-	Get() (MAASObject, error)
-	// Write this MAAS object.
-	Post(params url.Values) (JSONObject, error)
-	// Update this MAAS object with the given values.
-	Update(params url.Values) (MAASObject, error)
-	// Delete this MAAS object.
-	Delete() error
-	// Invoke a GET-based method on this MAAS object.
-	CallGet(operation string, params url.Values) (JSONObject, error)
-	// Invoke a POST-based method on this MAAS object.
-	CallPost(operation string, params url.Values) (JSONObject, error)
-}
-
-// JSONObject implementation for a MAAS object.  From a decoding perspective,
-// a jsonMAASObject is just like a jsonMap except it contains a key
-// "resource_uri", and it keeps track of the Client you got it from so that
-// you can invoke API methods directly on their MAAS objects.
-// jsonMAASObject implements both JSONObject and MAASObject.
-type jsonMAASObject struct {
-	jsonMap
+// You can extract a MAASObject out of a JSONObject using
+// JSONObject.GetMAASObject.  A MAAS API call will usually return either a
+// MAASObject or a list of MAASObjects.  The list itself would be wrapped in
+// a JSONObject, so if an API call returns a list of objects "l," you first
+// obtain the array using l.GetArray().  Then, for each item "i" in the array,
+// obtain the matching MAASObject using i.GetMAASObject().
+type MAASObject struct {
+	values map[string]JSONObject
 	client Client
 	uri    *url.URL
 }
 
 // newJSONMAASObject creates a new MAAS object.  It will panic if the given map
 // does not contain a valid URL for the 'resource_uri' key.
-func newJSONMAASObject(jmap jsonMap, client Client) jsonMAASObject {
-	const panicPrefix = "Error processing MAAS object: "
-	uriObj, ok := jmap[resourceURI]
+func newJSONMAASObject(jmap map[string]interface{}, client Client) MAASObject {
+	obj, err := maasify(client, jmap).GetMAASObject()
+	if err != nil {
+		panic(err)
+	}
+	return obj
+}
+
+var noResourceURI = errors.New("not a MAAS object: no 'resource_uri' key")
+
+// extractURI obtains the "resource_uri" string from a JSONObject map.
+func extractURI(attrs map[string]JSONObject) (*url.URL, error) {
+	uriEntry, ok := attrs[resourceURI]
 	if !ok {
-		panic(errors.New(panicPrefix + "no 'resource_uri' key present in the given jsonMap."))
-	}
-	uriString, err := uriObj.GetString()
-	if err != nil {
-		panic(errors.New(panicPrefix + "the value of 'resource_uri' is not a string."))
-	}
-	uri, err := url.Parse(uriString)
-	if err != nil {
-		panic(errors.New(panicPrefix + "the value of 'resource_uri' is not a valid URL."))
-	}
-	return jsonMAASObject{jmap, client, uri}
-}
-
-var _ JSONObject = (*jsonMAASObject)(nil)
-var _ MAASObject = (*jsonMAASObject)(nil)
-
-// JSONObject implementation for jsonMAASObject.
-func (jsonMAASObject) Type() string                               { return "maasobject" }
-func (obj jsonMAASObject) GetString() (string, error)             { return failString(obj) }
-func (obj jsonMAASObject) GetFloat64() (float64, error)           { return failFloat64(obj) }
-func (obj jsonMAASObject) GetMap() (map[string]JSONObject, error) { return obj.jsonMap.GetMap() }
-func (obj jsonMAASObject) GetMAASObject() (MAASObject, error)     { return obj, nil }
-func (obj jsonMAASObject) GetArray() ([]JSONObject, error)        { return failArray(obj) }
-func (obj jsonMAASObject) GetBool() (bool, error)                 { return failBool(obj) }
-
-// MAASObject implementation for jsonMAASObject.
-
-func (obj jsonMAASObject) GetField(name string) (string, error) {
-	return obj.jsonMap[name].GetString()
-}
-
-func (obj jsonMAASObject) URI() *url.URL {
+		return nil, noResourceURI
+	}
+	uri, err := uriEntry.GetString()
+	if err != nil {
+		return nil, fmt.Errorf("invalid resource_uri: %v", uri)
+	}
+	resourceURL, err := url.Parse(uri)
+	if err != nil {
+		return nil, fmt.Errorf("resource_uri does not contain a valid URL: %v", uri)
+	}
+	return resourceURL, nil
+}
+
+// JSONObject getter for a MAAS object.  From a decoding perspective, a
+// MAASObject is just like a map except it contains a key "resource_uri", and
+// it keeps track of the Client you got it from so that you can invoke API
+// methods directly on their MAAS objects.
+func (obj JSONObject) GetMAASObject() (MAASObject, error) {
+	attrs, err := obj.GetMap()
+	if err != nil {
+		return MAASObject{}, err
+	}
+	uri, err := extractURI(attrs)
+	if err != nil {
+		return MAASObject{}, err
+	}
+	return MAASObject{values: attrs, client: obj.client, uri: uri}, nil
+}
+
+// GetField extracts a string field from this MAAS object.
+func (obj MAASObject) GetField(name string) (string, error) {
+	return obj.values[name].GetString()
+}
+
+// URI is the resource URI for this MAAS object.  It is an absolute path, but
+// without a network part.
+func (obj MAASObject) URI() *url.URL {
 	// Duplicate the URL.
 	uri, err := url.Parse(obj.uri.String())
 	if err != nil {
@@ -95,11 +84,20 @@
 	return uri
 }
 
-func (obj jsonMAASObject) URL() *url.URL {
+// URL returns a full absolute URL (including network part) for this MAAS
+// object on the API.
+func (obj MAASObject) URL() *url.URL {
 	return obj.client.GetURL(obj.URI())
 }
 
-func (obj jsonMAASObject) GetSubObject(name string) MAASObject {
+// GetMap returns all of the object's attributes in the form of a map.
+func (obj MAASObject) GetMap() map[string]JSONObject {
+	return obj.values
+}
+
+// GetSubObject returns a new MAASObject representing the API resource found
+// at a given sub-path of the current object's resource URI.
+func (obj MAASObject) GetSubObject(name string) MAASObject {
 	uri := obj.URI()
 	newUrl, err := url.Parse(name)
 	if err != nil {
@@ -107,66 +105,74 @@
 	}
 	resUrl := uri.ResolveReference(newUrl)
 	resUrl.Path = EnsureTrailingSlash(resUrl.Path)
-	input := map[string]JSONObject{resourceURI: jsonString(resUrl.String())}
-	return newJSONMAASObject(jsonMap(input), obj.client)
+	input := map[string]interface{}{resourceURI: resUrl.String()}
+	return newJSONMAASObject(input, obj.client)
 }
 
 var NotImplemented = errors.New("Not implemented")
 
-func (obj jsonMAASObject) Get() (MAASObject, error) {
+// Get retrieves a fresh copy of this MAAS object from the API.
+func (obj MAASObject) Get() (MAASObject, error) {
 	uri := obj.URI()
 	result, err := obj.client.Get(uri, "", url.Values{})
 	if err != nil {
-		return nil, err
+		return MAASObject{}, err
 	}
 	jsonObj, err := Parse(obj.client, result)
 	if err != nil {
-		return nil, err
+		return MAASObject{}, err
 	}
 	return jsonObj.GetMAASObject()
 }
 
-func (obj jsonMAASObject) Post(params url.Values) (JSONObject, error) {
+// Post overwrites this object's existing value on the API with those given
+// in "params."  It returns the object's new value as received from the API.
+func (obj MAASObject) Post(params url.Values) (JSONObject, error) {
 	uri := obj.URI()
 	result, err := obj.client.Post(uri, "", params)
 	if err != nil {
-		return nil, err
+		return JSONObject{}, err
 	}
 	return Parse(obj.client, result)
 }
 
-func (obj jsonMAASObject) Update(params url.Values) (MAASObject, error) {
+// Update modifies this object on the API, based on the values given in
+// "params."  It returns the object's new value as received from the API.
+func (obj MAASObject) Update(params url.Values) (MAASObject, error) {
 	uri := obj.URI()
 	result, err := obj.client.Put(uri, params)
 	if err != nil {
-		return nil, err
+		return MAASObject{}, err
 	}
 	jsonObj, err := Parse(obj.client, result)
 	if err != nil {
-		return nil, err
+		return MAASObject{}, err
 	}
 	return jsonObj.GetMAASObject()
 }
 
-func (obj jsonMAASObject) Delete() error {
+// Delete removes this object on the API.
+func (obj MAASObject) Delete() error {
 	uri := obj.URI()
 	return obj.client.Delete(uri)
 }
 
-func (obj jsonMAASObject) CallGet(operation string, params url.Values) (JSONObject, error) {
+// CallGet invokes an idempotent API method on this object.
+func (obj MAASObject) CallGet(operation string, params url.Values) (JSONObject, error) {
 	uri := obj.URI()
 	result, err := obj.client.Get(uri, operation, params)
 	if err != nil {
-		return nil, err
+		return JSONObject{}, err
 	}
 	return Parse(obj.client, result)
 }
 
-func (obj jsonMAASObject) CallPost(operation string, params url.Values) (JSONObject, error) {
+// CallPost invokes a non-idempotent API method on this object.
+func (obj MAASObject) CallPost(operation string, params url.Values) (JSONObject, error) {
 	uri := obj.URI()
 	result, err := obj.client.Post(uri, operation, params)
 	if err != nil {
-		return nil, err
+		return JSONObject{}, err
 	}
 	return Parse(obj.client, result)
 }

=== modified file 'maasobject_test.go'
--- maasobject_test.go	2013-02-07 16:01:48 +0000
+++ maasobject_test.go	2013-02-11 12:31:19 +0000
@@ -18,16 +18,10 @@
 	return "http://example.com/"; + fmt.Sprint(rand.Int31())
 }
 
-func makeFakeMAASObject() jsonMAASObject {
-	attrs := make(map[string]JSONObject)
-	attrs[resourceURI] = jsonString(makeFakeResourceURI())
-	return jsonMAASObject{jsonMap: jsonMap(attrs)}
-}
-
-// jsonMAASObjects convert only to map or to MAASObject.
+// JSONObjects containing MAAS objects convert only to map or to MAASObject.
 func (suite *MAASObjectSuite) TestConversionsMAASObject(c *C) {
-	input := map[string]JSONObject{resourceURI: jsonString("someplace")}
-	obj := jsonMAASObject{jsonMap: jsonMap(input)}
+	input := map[string]interface{}{resourceURI: "someplace"}
+	obj := maasify(Client{}, input)
 
 	mp, err := obj.GetMap()
 	c.Check(err, IsNil)
@@ -35,9 +29,10 @@
 	c.Check(err, IsNil)
 	c.Check(text, Equals, "someplace")
 
-	maasobj, err := obj.GetMAASObject()
-	c.Check(err, IsNil)
-	_ = maasobj.(jsonMAASObject)
+	var maasobj MAASObject
+	maasobj, err = obj.GetMAASObject()
+	c.Assert(err, IsNil)
+	c.Check(maasobj, NotNil)
 
 	_, err = obj.GetString()
 	c.Check(err, NotNil)
@@ -56,8 +51,9 @@
 		msg := recoveredError.(error).Error()
 		c.Check(msg, Matches, ".*no 'resource_uri' key.*")
 	}()
-	input := map[string]JSONObject{"test": jsonString("test")}
-	newJSONMAASObject(jsonMap(input), Client{})
+
+	input := map[string]interface{}{"test": "test"}
+	newJSONMAASObject(input, Client{})
 }
 
 func (suite *MAASObjectSuite) TestNewJSONMAASObjectPanicsIfResourceURINotString(c *C) {
@@ -65,10 +61,11 @@
 		recoveredError := recover()
 		c.Check(recoveredError, NotNil)
 		msg := recoveredError.(error).Error()
-		c.Check(msg, Matches, ".*the value of 'resource_uri' is not a string.*")
+		c.Check(msg, Matches, ".*invalid resource_uri.*")
 	}()
-	input := map[string]JSONObject{resourceURI: jsonFloat64(77.7)}
-	newJSONMAASObject(jsonMap(input), Client{})
+
+	input := map[string]interface{}{resourceURI: 77.77}
+	newJSONMAASObject(input, Client{})
 }
 
 func (suite *MAASObjectSuite) TestNewJSONMAASObjectPanicsIfResourceURINotURL(c *C) {
@@ -76,16 +73,17 @@
 		recoveredError := recover()
 		c.Check(recoveredError, NotNil)
 		msg := recoveredError.(error).Error()
-		c.Check(msg, Matches, ".*the value of 'resource_uri' is not a valid URL.*")
+		c.Check(msg, Matches, ".*resource_uri.*valid URL.*")
 	}()
-	input := map[string]JSONObject{resourceURI: jsonString("")}
-	newJSONMAASObject(jsonMap(input), Client{})
+
+	input := map[string]interface{}{resourceURI: ""}
+	newJSONMAASObject(input, Client{})
 }
 
 func (suite *MAASObjectSuite) TestNewJSONMAASObjectSetsUpURI(c *C) {
 	URI, _ := url.Parse("http://example.com/a/resource";)
-	input := map[string]JSONObject{resourceURI: jsonString(URI.String())}
-	obj := newJSONMAASObject(jsonMap(input), Client{})
+	attrs := map[string]interface{}{resourceURI: URI.String()}
+	obj := newJSONMAASObject(attrs, Client{})
 	c.Check(obj.uri, DeepEquals, URI)
 }
 
@@ -93,9 +91,9 @@
 	baseURL, _ := url.Parse("http://example.com/";)
 	uri := "http://example.com/a/resource";
 	resourceURL, _ := url.Parse(uri)
-	input := map[string]JSONObject{resourceURI: jsonString(uri)}
+	input := map[string]interface{}{resourceURI: uri}
 	client := Client{BaseURL: baseURL}
-	obj := newJSONMAASObject(jsonMap(input), client)
+	obj := newJSONMAASObject(input, client)
 
 	URL := obj.URL()
 
@@ -105,9 +103,9 @@
 func (suite *MAASObjectSuite) TestGetSubObjectRelative(c *C) {
 	baseURL, _ := url.Parse("http://example.com/";)
 	uri := "http://example.com/a/resource/";
-	input := map[string]JSONObject{resourceURI: jsonString(uri)}
+	input := map[string]interface{}{resourceURI: uri}
 	client := Client{BaseURL: baseURL}
-	obj := newJSONMAASObject(jsonMap(input), client)
+	obj := newJSONMAASObject(input, client)
 	subName := "test"
 
 	subObj := obj.GetSubObject(subName)
@@ -122,9 +120,9 @@
 func (suite *MAASObjectSuite) TestGetSubObjectAbsolute(c *C) {
 	baseURL, _ := url.Parse("http://example.com/";)
 	uri := "http://example.com/a/resource/";
-	input := map[string]JSONObject{resourceURI: jsonString(uri)}
+	input := map[string]interface{}{resourceURI: uri}
 	client := Client{BaseURL: baseURL}
-	obj := newJSONMAASObject(jsonMap(input), client)
+	obj := newJSONMAASObject(input, client)
 	subName := "/b/test"
 
 	subObj := obj.GetSubObject(subName)
@@ -138,10 +136,10 @@
 	uri := "http://example.com/a/resource";
 	fieldName := "field name"
 	fieldValue := "a value"
-	input := map[string]JSONObject{
-		resourceURI: jsonString(uri), fieldName: jsonString(fieldValue),
+	input := map[string]interface{}{
+		resourceURI: uri, fieldName: fieldValue,
 	}
-	obj := jsonMAASObject{jsonMap: jsonMap(input)}
+	obj := newJSONMAASObject(input, Client{})
 	value, err := obj.GetField(fieldName)
 	c.Check(err, IsNil)
 	c.Check(value, Equals, fieldValue)

=== modified file 'testservice.go'
--- testservice.go	2013-02-07 10:01:10 +0000
+++ testservice.go	2013-02-11 12:31:19 +0000
@@ -21,9 +21,6 @@
 	TestServer *TestServer
 }
 
-// TestMAASObject implements the MAASObject interface.
-var _ MAASObject = (*TestMAASObject)(nil)
-
 // NewTestMAAS returns a TestMAASObject that implements the MAASObject
 // interface and thus can be used as a test object instead of the one returned
 // by gomaasapi.NewMAAS().
@@ -84,28 +81,21 @@
 // string representing a map and contain a string value for the key 
 // 'system_id'.  e.g. `{"system_id": "mysystemid"}`.
 // If one of these conditions is not met, NewNode panics.
-func (server *TestServer) NewNode(json string) MAASObject {
-	obj, err := Parse(server.client, []byte(json))
-	if err != nil {
-		panic(err)
-	}
-	mapobj, err := obj.GetMap()
-	if err != nil {
-		panic(err)
-	}
-	systemId, hasSystemId := mapobj["system_id"]
+func (server *TestServer) NewNode(jsonText string) MAASObject {
+	var attrs map[string]interface{}
+	err := json.Unmarshal([]byte(jsonText), &attrs)
+	if err != nil {
+		panic(err)
+	}
+	systemIdEntry, hasSystemId := attrs["system_id"]
 	if !hasSystemId {
 		panic("The given map json string does not contain a 'system_id' value.")
 	}
-	stringSystemId, err := systemId.GetString()
-	if err != nil {
-		panic(err)
-	}
-	resourceUri := getNodeURI(server.version, stringSystemId)
-	mapobj[resourceURI] = jsonString(resourceUri)
-	maasobj := newJSONMAASObject(mapobj, server.client)
-	server.nodes[stringSystemId] = maasobj
-	return maasobj
+	systemId := systemIdEntry.(string)
+	attrs[resourceURI] = getNodeURI(server.version, systemId)
+	obj := newJSONMAASObject(attrs, server.client)
+	server.nodes[systemId] = obj
+	return obj
 }
 
 // Returns a map associating all the nodes' system ids with the nodes'
@@ -129,8 +119,7 @@
 	if !found {
 		panic("No node with such 'system_id'.")
 	}
-	mapObj, _ := node.GetMap()
-	mapObj[key] = jsonString(value)
+	node.GetMap()[key] = maasify(server.client, value)
 }
 
 func getNodeListingURL(version string) string {
@@ -191,9 +180,19 @@
 	}
 }
 
+func (obj JSONObject) MarshalJSON() ([]byte, error) {
+	if obj.IsNil() {
+		return json.Marshal(nil)
+	}
+	return json.Marshal(obj.value)
+}
+
+func (obj MAASObject) MarshalJSON() ([]byte, error) {
+	return json.Marshal(obj.GetMap())
+}
+
 func marshalNode(node MAASObject) string {
-	mapObj, _ := node.GetMap()
-	res, _ := json.Marshal(mapObj)
+	res, _ := json.Marshal(node)
 	return string(res)
 
 }
@@ -253,8 +252,7 @@
 	var convertedNodes = []map[string]JSONObject{}
 	for systemId, node := range server.nodes {
 		if !hasId || contains(ids, systemId) {
-			mapp, _ := node.GetMap()
-			convertedNodes = append(convertedNodes, mapp)
+			convertedNodes = append(convertedNodes, node.GetMap())
 		}
 	}
 	res, _ := json.Marshal(convertedNodes)

=== modified file 'testservice_test.go'
--- testservice_test.go	2013-02-07 10:01:10 +0000
+++ testservice_test.go	2013-02-11 12:31:19 +0000
@@ -78,8 +78,8 @@
 	suite.server.ChangeNode("mysystemid", "newfield", "newvalue")
 
 	node, _ := suite.server.nodes["mysystemid"]
-	mapObj, _ := node.GetMap()
-	field, _ := mapObj["newfield"].GetString()
+	field, err := node.GetField("newfield")
+	c.Assert(err, IsNil)
 	c.Check(field, Equals, "newvalue")
 }
 
@@ -234,13 +234,16 @@
 
 	c.Check(err, IsNil)
 	listNodes, err := listNodeObjects.GetArray()
-	c.Check(err, IsNil)
+	c.Assert(err, IsNil)
 	c.Check(len(listNodes), Equals, 1)
-	node, _ := listNodes[0].GetMAASObject()
-	systemId, _ := node.GetField("system_id")
+	node, err := listNodes[0].GetMAASObject()
+	c.Assert(err, IsNil)
+	systemId, err := node.GetField("system_id")
+	c.Assert(err, IsNil)
 	c.Check(systemId, Equals, "mysystemid")
 	resourceURI, _ := node.GetField(resourceURI)
-	expectedResourceURI := fmt.Sprintf("/api/%s/nodes/mysystemid/", suite.TestMAASObject.TestServer.version)
+	apiVersion := suite.TestMAASObject.TestServer.version
+	expectedResourceURI := fmt.Sprintf("/api/%s/nodes/mysystemid/", apiVersion)
 	c.Check(resourceURI, Equals, expectedResourceURI)
 }