From a9d9bb9d47ced055db5ad3da1accd67e221288f3 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 20 Jun 2014 15:12:12 -0700 Subject: [PATCH 1/7] Add intelligent type marshalling/unmarshalling --- pkg/api/helper.go | 111 +++++++++++++++++++++++++++++++++++++++++ pkg/api/helper_test.go | 74 +++++++++++++++++++++++++++ 2 files changed, 185 insertions(+) create mode 100644 pkg/api/helper.go create mode 100644 pkg/api/helper_test.go diff --git a/pkg/api/helper.go b/pkg/api/helper.go new file mode 100644 index 0000000000..3aeddb0b0f --- /dev/null +++ b/pkg/api/helper.go @@ -0,0 +1,111 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package api + +import ( + "encoding/json" + "fmt" + "reflect" +) + +var knownTypes = map[string]reflect.Type{} + +func init() { + types := []interface{}{ + PodList{}, Pod{}, ReplicationControllerList{}, + ReplicationController{}, ServiceList{}, Service{}, + } + for _, obj := range types { + t := reflect.TypeOf(obj) + knownTypes[t.Name()] = t + } +} + +// Returns the name of the type (sans pointer), and its kind field. Takes pointer-to-struct.. +func nameAndJSONBase(obj interface{}) (string, reflect.Value, error) { + v := reflect.ValueOf(obj) + if v.Kind() != reflect.Ptr { + return "", reflect.Value{}, fmt.Errorf("expected pointer, but got %v", v.Type().Name()) + } + v = v.Elem() + name := v.Type().Name() + if v.Kind() != reflect.Struct { + return "", reflect.Value{}, fmt.Errorf("expected struct, but got %v", name) + } + jsonBase := v.FieldByName("JSONBase") + if !jsonBase.IsValid() { + return "", reflect.Value{}, fmt.Errorf("struct %v lacks embedded JSON type", name) + } + return name, jsonBase, nil +} + +// Encode turns the given api object into an appropriate JSON string. +// Will return an error if the object doesn't have an embedded JSONBase. +// Obj must be a pointer to a struct. Note, this sets the object's Kind +// field. +func Encode(obj interface{}) (data []byte, err error) { + name, jsonBase, err := nameAndJSONBase(obj) + if err != nil { + return nil, err + } + if _, contains := knownTypes[name]; !contains { + return nil, fmt.Errorf("struct %v can't be unmarshalled because it's not in knownTypes", name) + } + jsonBase.FieldByName("Kind").Set(reflect.ValueOf(name)) + return json.Marshal(obj) +} + +// Decode converts a JSON string back into a pointer to an api object. Deduces the type +// based upon the Kind field (set by encode). +func Decode(data []byte) (interface{}, error) { + findKind := struct { + Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` + }{} + err := json.Unmarshal(data, &findKind) + if err != nil { + return nil, fmt.Errorf("Couldn't get kind: %#v", err) + } + objType, found := knownTypes[findKind.Kind] + if !found { + return nil, fmt.Errorf("%v is not a known type", findKind.Kind) + } + obj := reflect.New(objType).Interface() + err = json.Unmarshal(data, obj) + if err != nil { + return nil, err + } + return obj, nil +} + +// DecodeInto parses a JSON string and stores it in obj. Returns an error +// if data.Kind is set and doesn't match the type of obj. Obj should be a +// pointer to an api type. +func DecodeInto(data []byte, obj interface{}) error { + err := json.Unmarshal(data, obj) + if err != nil { + return err + } + name, jsonBase, err := nameAndJSONBase(obj) + if err != nil { + return err + } + foundName := jsonBase.FieldByName("Kind").Interface().(string) + if foundName != "" && foundName != name { + return fmt.Errorf("data had kind %v, but passed object was of type %v", foundName, name) + } + return nil +} diff --git a/pkg/api/helper_test.go b/pkg/api/helper_test.go new file mode 100644 index 0000000000..93ed887c6c --- /dev/null +++ b/pkg/api/helper_test.go @@ -0,0 +1,74 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package api + +import ( + "reflect" + "testing" +) + +func runTest(t *testing.T, source interface{}) { + name := reflect.TypeOf(source).Name() + data, err := Encode(source) + if err != nil { + t.Errorf("%v: %v", name, err) + return + } + obj2, err := Decode(data) + if err != nil { + t.Errorf("%v: %v", name, err) + return + } + if !reflect.DeepEqual(source, obj2) { + t.Errorf("%v: wanted %#v, got %#v", name, source, obj2) + return + } + obj3 := reflect.New(reflect.TypeOf(source).Elem()).Interface() + err = DecodeInto(data, obj3) + if err != nil { + t.Errorf("%v: %v", name, err) + return + } + if !reflect.DeepEqual(source, obj3) { + t.Errorf("%v: wanted %#v, got %#v", name, source, obj3) + return + } +} + +func TestTypes(t *testing.T) { + // TODO: auto-fill all fields. + table := []interface{}{ + &Pod{ + JSONBase: JSONBase{ + ID: "mylittlepod", + }, + Labels: map[string]string{ + "name": "pinky", + }, + }, + &Service{}, + &ServiceList{}, + &ReplicationControllerList{}, + &ReplicationController{}, + &PodList{}, + } + for _, item := range table { + runTest(t, item) + } +} + +// TODO: test rejection of bad JSON. From fb991fb84e7432c412ca0ab0f7565e94b7afcc38 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 20 Jun 2014 15:24:56 -0700 Subject: [PATCH 2/7] Change type to []byte --- pkg/api/helper.go | 6 ++++-- pkg/apiserver/apiserver.go | 6 +++--- pkg/apiserver/apiserver_test.go | 4 ++-- pkg/registry/controller_registry.go | 4 ++-- pkg/registry/controller_registry_test.go | 2 +- pkg/registry/pod_registry.go | 4 ++-- pkg/registry/pod_registry_test.go | 2 +- pkg/registry/service_registry.go | 2 +- 8 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/api/helper.go b/pkg/api/helper.go index 3aeddb0b0f..515778d7be 100644 --- a/pkg/api/helper.go +++ b/pkg/api/helper.go @@ -65,7 +65,7 @@ func Encode(obj interface{}) (data []byte, err error) { if _, contains := knownTypes[name]; !contains { return nil, fmt.Errorf("struct %v can't be unmarshalled because it's not in knownTypes", name) } - jsonBase.FieldByName("Kind").Set(reflect.ValueOf(name)) + jsonBase.FieldByName("Kind").SetString(name) return json.Marshal(obj) } @@ -104,7 +104,9 @@ func DecodeInto(data []byte, obj interface{}) error { return err } foundName := jsonBase.FieldByName("Kind").Interface().(string) - if foundName != "" && foundName != name { + if foundName == "" { + jsonBase.FieldByName("Kind").SetString(name) + } else if foundName != name { return fmt.Errorf("data had kind %v, but passed object was of type %v", foundName, name) } return nil diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 74b019f60d..19823b97e4 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -36,7 +36,7 @@ type RESTStorage interface { List(labels.Selector) (interface{}, error) Get(id string) (interface{}, error) Delete(id string) (<-chan interface{}, error) - Extract(body string) (interface{}, error) + Extract(body []byte) (interface{}, error) Create(interface{}) (<-chan interface{}, error) Update(interface{}) (<-chan interface{}, error) } @@ -143,10 +143,10 @@ func (server *ApiServer) error(err error, w http.ResponseWriter) { fmt.Fprintf(w, "Internal Error: %#v", err) } -func (server *ApiServer) readBody(req *http.Request) (string, error) { +func (server *ApiServer) readBody(req *http.Request) ([]byte, error) { defer req.Body.Close() body, err := ioutil.ReadAll(req.Body) - return string(body), err + return body, err } func (server *ApiServer) waitForObject(out <-chan interface{}, timeout time.Duration) (interface{}, error) { diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 93748debe0..a9194bd6a0 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -69,9 +69,9 @@ func (storage *SimpleRESTStorage) Delete(id string) (<-chan interface{}, error) return storage.channel, storage.err } -func (storage *SimpleRESTStorage) Extract(body string) (interface{}, error) { +func (storage *SimpleRESTStorage) Extract(body []byte) (interface{}, error) { var item Simple - json.Unmarshal([]byte(body), &item) + json.Unmarshal(body, &item) return item, storage.err } diff --git a/pkg/registry/controller_registry.go b/pkg/registry/controller_registry.go index d99ad90ff5..d71e7f90b1 100644 --- a/pkg/registry/controller_registry.go +++ b/pkg/registry/controller_registry.go @@ -61,9 +61,9 @@ func (storage *ControllerRegistryStorage) Delete(id string) (<-chan interface{}, return apiserver.MakeAsync(func() interface{} { return apiserver.Status{Success: true} }), storage.registry.DeleteController(id) } -func (storage *ControllerRegistryStorage) Extract(body string) (interface{}, error) { +func (storage *ControllerRegistryStorage) Extract(body []byte) (interface{}, error) { result := api.ReplicationController{} - err := json.Unmarshal([]byte(body), &result) + err := json.Unmarshal(body, &result) result.Kind = "cluster#replicationController" return result, err } diff --git a/pkg/registry/controller_registry_test.go b/pkg/registry/controller_registry_test.go index 8d2257ec0d..5a96d21147 100644 --- a/pkg/registry/controller_registry_test.go +++ b/pkg/registry/controller_registry_test.go @@ -123,7 +123,7 @@ func TestExtractControllerJson(t *testing.T) { } body, err := json.Marshal(controller) expectNoError(t, err) - controllerOut, err := storage.Extract(string(body)) + controllerOut, err := storage.Extract(body) expectNoError(t, err) // Extract adds a Kind controller.Kind = "cluster#replicationController" diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index fd9c6eb3c9..f1b1de4fa7 100644 --- a/pkg/registry/pod_registry.go +++ b/pkg/registry/pod_registry.go @@ -136,9 +136,9 @@ func (storage *PodRegistryStorage) Delete(id string) (<-chan interface{}, error) return apiserver.MakeAsync(func() interface{} { return apiserver.Status{Success: true} }), storage.registry.DeletePod(id) } -func (storage *PodRegistryStorage) Extract(body string) (interface{}, error) { +func (storage *PodRegistryStorage) Extract(body []byte) (interface{}, error) { pod := api.Pod{} - err := json.Unmarshal([]byte(body), &pod) + err := json.Unmarshal(body, &pod) pod.Kind = "cluster#pod" return pod, err } diff --git a/pkg/registry/pod_registry_test.go b/pkg/registry/pod_registry_test.go index 783bbe1c90..d984feac56 100644 --- a/pkg/registry/pod_registry_test.go +++ b/pkg/registry/pod_registry_test.go @@ -104,7 +104,7 @@ func TestExtractJson(t *testing.T) { } body, err := json.Marshal(pod) expectNoError(t, err) - podOut, err := storage.Extract(string(body)) + podOut, err := storage.Extract(body) expectNoError(t, err) // Extract adds in a kind pod.Kind = "cluster#pod" diff --git a/pkg/registry/service_registry.go b/pkg/registry/service_registry.go index 37691e24d9..6ea48c3ea7 100644 --- a/pkg/registry/service_registry.go +++ b/pkg/registry/service_registry.go @@ -105,7 +105,7 @@ func (sr *ServiceRegistryStorage) Delete(id string) (<-chan interface{}, error) return apiserver.MakeAsync(func() interface{} { return apiserver.Status{Success: true} }), sr.registry.DeleteService(id) } -func (sr *ServiceRegistryStorage) Extract(body string) (interface{}, error) { +func (sr *ServiceRegistryStorage) Extract(body []byte) (interface{}, error) { var svc api.Service err := json.Unmarshal([]byte(body), &svc) svc.Kind = "cluster#service" From 2bcb44b6bd747ca95850a8d0d7a7af04f075fb67 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 20 Jun 2014 15:30:49 -0700 Subject: [PATCH 3/7] make Registry use Encode --- pkg/registry/controller_registry.go | 6 +----- pkg/registry/controller_registry_test.go | 4 +--- pkg/registry/pod_registry.go | 6 +----- pkg/registry/pod_registry_test.go | 5 +---- pkg/registry/service_registry.go | 6 +----- 5 files changed, 5 insertions(+), 22 deletions(-) diff --git a/pkg/registry/controller_registry.go b/pkg/registry/controller_registry.go index d71e7f90b1..f5dc00dd36 100644 --- a/pkg/registry/controller_registry.go +++ b/pkg/registry/controller_registry.go @@ -17,8 +17,6 @@ limitations under the License. package registry import ( - "encoding/json" - "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" @@ -53,7 +51,6 @@ func (storage *ControllerRegistryStorage) Get(id string) (interface{}, error) { if err != nil { return nil, err } - controller.Kind = "cluster#replicationController" return controller, err } @@ -63,8 +60,7 @@ func (storage *ControllerRegistryStorage) Delete(id string) (<-chan interface{}, func (storage *ControllerRegistryStorage) Extract(body []byte) (interface{}, error) { result := api.ReplicationController{} - err := json.Unmarshal(body, &result) - result.Kind = "cluster#replicationController" + err := api.DecodeInto(body, &result) return result, err } diff --git a/pkg/registry/controller_registry_test.go b/pkg/registry/controller_registry_test.go index 5a96d21147..69b04e03e3 100644 --- a/pkg/registry/controller_registry_test.go +++ b/pkg/registry/controller_registry_test.go @@ -121,12 +121,10 @@ func TestExtractControllerJson(t *testing.T) { ID: "foo", }, } - body, err := json.Marshal(controller) + body, err := api.Encode(&controller) expectNoError(t, err) controllerOut, err := storage.Extract(body) expectNoError(t, err) - // Extract adds a Kind - controller.Kind = "cluster#replicationController" if !reflect.DeepEqual(controller, controllerOut) { t.Errorf("Expected %#v, found %#v", controller, controllerOut) } diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index f1b1de4fa7..f74dea980f 100644 --- a/pkg/registry/pod_registry.go +++ b/pkg/registry/pod_registry.go @@ -16,7 +16,6 @@ limitations under the License. package registry import ( - "encoding/json" "fmt" "log" "strings" @@ -73,7 +72,6 @@ func (storage *PodRegistryStorage) List(selector labels.Selector) (interface{}, } } - result.Kind = "cluster#podList" return result, err } @@ -128,7 +126,6 @@ func (storage *PodRegistryStorage) Get(id string) (interface{}, error) { } pod.CurrentState.HostIP = getInstanceIP(storage.cloud, pod.CurrentState.Host) - pod.Kind = "cluster#pod" return pod, err } @@ -138,8 +135,7 @@ func (storage *PodRegistryStorage) Delete(id string) (<-chan interface{}, error) func (storage *PodRegistryStorage) Extract(body []byte) (interface{}, error) { pod := api.Pod{} - err := json.Unmarshal(body, &pod) - pod.Kind = "cluster#pod" + err := api.DecodeInto(body, &pod) return pod, err } diff --git a/pkg/registry/pod_registry_test.go b/pkg/registry/pod_registry_test.go index d984feac56..8034f3c4a7 100644 --- a/pkg/registry/pod_registry_test.go +++ b/pkg/registry/pod_registry_test.go @@ -16,7 +16,6 @@ limitations under the License. package registry import ( - "encoding/json" "fmt" "reflect" "testing" @@ -102,12 +101,10 @@ func TestExtractJson(t *testing.T) { ID: "foo", }, } - body, err := json.Marshal(pod) + body, err := api.Encode(&pod) expectNoError(t, err) podOut, err := storage.Extract(body) expectNoError(t, err) - // Extract adds in a kind - pod.Kind = "cluster#pod" if !reflect.DeepEqual(pod, podOut) { t.Errorf("Expected %#v, found %#v", pod, podOut) } diff --git a/pkg/registry/service_registry.go b/pkg/registry/service_registry.go index 6ea48c3ea7..59faa1a8ff 100644 --- a/pkg/registry/service_registry.go +++ b/pkg/registry/service_registry.go @@ -17,7 +17,6 @@ limitations under the License. package registry import ( - "encoding/json" "fmt" "strconv" "strings" @@ -64,7 +63,6 @@ func (sr *ServiceRegistryStorage) List(selector labels.Selector) (interface{}, e if err != nil { return nil, err } - list.Kind = "cluster#serviceList" var filtered []api.Service for _, service := range list.Items { if selector.Matches(labels.Set(service.Labels)) { @@ -80,7 +78,6 @@ func (sr *ServiceRegistryStorage) Get(id string) (interface{}, error) { if err != nil { return nil, err } - service.Kind = "cluster#service" return service, err } @@ -107,8 +104,7 @@ func (sr *ServiceRegistryStorage) Delete(id string) (<-chan interface{}, error) func (sr *ServiceRegistryStorage) Extract(body []byte) (interface{}, error) { var svc api.Service - err := json.Unmarshal([]byte(body), &svc) - svc.Kind = "cluster#service" + err := api.DecodeInto(body, &svc) return svc, err } From d7b4915111d4c3b367576ff06dbe652961149c4d Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 20 Jun 2014 16:08:54 -0700 Subject: [PATCH 4/7] Don't make people have to worry about the Kind field. --- pkg/api/helper.go | 94 +++++++++++++++++++++++++++++++----------- pkg/api/helper_test.go | 30 ++++++++++++++ 2 files changed, 101 insertions(+), 23 deletions(-) diff --git a/pkg/api/helper.go b/pkg/api/helper.go index 515778d7be..fdb86d351f 100644 --- a/pkg/api/helper.go +++ b/pkg/api/helper.go @@ -25,16 +25,74 @@ import ( var knownTypes = map[string]reflect.Type{} func init() { - types := []interface{}{ - PodList{}, Pod{}, ReplicationControllerList{}, - ReplicationController{}, ServiceList{}, Service{}, - } + AddKnownTypes( + PodList{}, Pod{}, + ReplicationControllerList{}, ReplicationController{}, + ServiceList{}, Service{}, + ) +} + +func AddKnownTypes(types ...interface{}) { for _, obj := range types { t := reflect.TypeOf(obj) knownTypes[t.Name()] = t } } +// Encode turns the given api object into an appropriate JSON string. +// Will return an error if the object doesn't have an embedded JSONBase. +// Obj may be a pointer to a struct, or a struct. If a struct, a copy +// will be made so that the object's Kind field can be set. If a pointer, +// we change the Kind field, marshal, and then set the kind field back to +// "". Having to keep track of the kind field makes tests very annoying, +// so the rule is it's set only in wire format (json), not when in native +// format. +func Encode(obj interface{}) (data []byte, err error) { + obj = checkPtr(obj) + fieldToReset, err := prepareEncode(obj) + if err != nil { + return nil, err + } + data, err = json.Marshal(obj) + fieldToReset.SetString("") + return +} + +// Just like Encode, but produces indented output. +func EncodeIndent(obj interface{}) (data []byte, err error) { + obj = checkPtr(obj) + fieldToReset, err := prepareEncode(obj) + if err != nil { + return nil, err + } + data, err = json.MarshalIndent(obj, "", " ") + fieldToReset.SetString("") + return +} + +func checkPtr(obj interface{}) interface{} { + v := reflect.ValueOf(obj) + if v.Kind() == reflect.Ptr { + return obj + } + v2 := reflect.New(v.Type()) + v2.Elem().Set(v) + return v2.Interface() +} + +func prepareEncode(obj interface{}) (reflect.Value, error) { + name, jsonBase, err := nameAndJSONBase(obj) + if err != nil { + return reflect.Value{}, err + } + if _, contains := knownTypes[name]; !contains { + return reflect.Value{}, fmt.Errorf("struct %v won't be unmarshalable because it's not in knownTypes", name) + } + kind := jsonBase.FieldByName("Kind") + kind.SetString(name) + return kind, nil +} + // Returns the name of the type (sans pointer), and its kind field. Takes pointer-to-struct.. func nameAndJSONBase(obj interface{}) (string, reflect.Value, error) { v := reflect.ValueOf(obj) @@ -53,22 +111,6 @@ func nameAndJSONBase(obj interface{}) (string, reflect.Value, error) { return name, jsonBase, nil } -// Encode turns the given api object into an appropriate JSON string. -// Will return an error if the object doesn't have an embedded JSONBase. -// Obj must be a pointer to a struct. Note, this sets the object's Kind -// field. -func Encode(obj interface{}) (data []byte, err error) { - name, jsonBase, err := nameAndJSONBase(obj) - if err != nil { - return nil, err - } - if _, contains := knownTypes[name]; !contains { - return nil, fmt.Errorf("struct %v can't be unmarshalled because it's not in knownTypes", name) - } - jsonBase.FieldByName("Kind").SetString(name) - return json.Marshal(obj) -} - // Decode converts a JSON string back into a pointer to an api object. Deduces the type // based upon the Kind field (set by encode). func Decode(data []byte) (interface{}, error) { @@ -88,6 +130,12 @@ func Decode(data []byte) (interface{}, error) { if err != nil { return nil, err } + _, jsonBase, err := nameAndJSONBase(obj) + if err != nil { + return nil, err + } + // Don't leave these set. Track type with go's type. + jsonBase.FieldByName("Kind").SetString("") return obj, nil } @@ -104,10 +152,10 @@ func DecodeInto(data []byte, obj interface{}) error { return err } foundName := jsonBase.FieldByName("Kind").Interface().(string) - if foundName == "" { - jsonBase.FieldByName("Kind").SetString(name) - } else if foundName != name { + if foundName != "" && foundName != name { return fmt.Errorf("data had kind %v, but passed object was of type %v", foundName, name) } + // Don't leave these set. Track type with go's type. + jsonBase.FieldByName("Kind").SetString("") return nil } diff --git a/pkg/api/helper_test.go b/pkg/api/helper_test.go index 93ed887c6c..d24083161d 100644 --- a/pkg/api/helper_test.go +++ b/pkg/api/helper_test.go @@ -71,4 +71,34 @@ func TestTypes(t *testing.T) { } } +func TestNonPtr(t *testing.T) { + obj := interface{}(Pod{Labels: map[string]string{"name": "foo"}}) + data, err := Encode(obj) + obj2, err2 := Decode(data) + if err != nil || err2 != nil { + t.Errorf("Failure: %v %v", err2, err2) + } + if _, ok := obj2.(*Pod); !ok { + t.Errorf("Got wrong type") + } + if !reflect.DeepEqual(obj2, &Pod{Labels: map[string]string{"name": "foo"}}) { + t.Errorf("Something changed: %#v", obj2) + } +} + +func TestPtr(t *testing.T) { + obj := interface{}(&Pod{Labels: map[string]string{"name": "foo"}}) + data, err := Encode(obj) + obj2, err2 := Decode(data) + if err != nil || err2 != nil { + t.Errorf("Failure: %v %v", err2, err2) + } + if _, ok := obj2.(*Pod); !ok { + t.Errorf("Got wrong type") + } + if !reflect.DeepEqual(obj2, &Pod{Labels: map[string]string{"name": "foo"}}) { + t.Errorf("Something changed: %#v", obj2) + } +} + // TODO: test rejection of bad JSON. From 14361e336ab571b14e511788c78d7b391a5c8b51 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 20 Jun 2014 16:18:36 -0700 Subject: [PATCH 5/7] Make apiserver work with new encode/decode --- pkg/apiserver/apiserver.go | 4 ++-- pkg/apiserver/apiserver_test.go | 18 +++++++++++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 19823b97e4..c5fd4b85cd 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -17,7 +17,6 @@ limitations under the License. package apiserver import ( - "encoding/json" "fmt" "io/ioutil" "log" @@ -27,6 +26,7 @@ import ( "strings" "time" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -130,7 +130,7 @@ func (server *ApiServer) notFound(req *http.Request, w http.ResponseWriter) { func (server *ApiServer) write(statusCode int, object interface{}, w http.ResponseWriter) { w.WriteHeader(statusCode) - output, err := json.MarshalIndent(object, "", " ") + output, err := api.EncodeIndent(object) if err != nil { server.error(err, w) return diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index a9194bd6a0..006a6bfc0d 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -26,9 +26,14 @@ import ( "sync" "testing" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) +func init() { + api.AddKnownTypes(Simple{}, SimpleList{}) +} + // TODO: This doesn't reduce typing enough to make it worth the less readable errors. Remove. func expectNoError(t *testing.T, err error) { if err != nil { @@ -37,11 +42,13 @@ func expectNoError(t *testing.T, err error) { } type Simple struct { - Name string + JSONBase api.JSONBase `json:",inline"` + Name string } type SimpleList struct { - Items []Simple + JSONBase api.JSONBase `json:",inline"` + Items []Simple } type SimpleRESTStorage struct { @@ -54,7 +61,7 @@ type SimpleRESTStorage struct { } func (storage *SimpleRESTStorage) List(labels.Selector) (interface{}, error) { - result := SimpleList{ + result := &SimpleList{ Items: storage.list, } return result, storage.err @@ -71,7 +78,7 @@ func (storage *SimpleRESTStorage) Delete(id string) (<-chan interface{}, error) func (storage *SimpleRESTStorage) Extract(body []byte) (interface{}, error) { var item Simple - json.Unmarshal(body, &item) + api.DecodeInto(body, &item) return item, storage.err } @@ -90,7 +97,7 @@ func extractBody(response *http.Response, object interface{}) (string, error) { if err != nil { return string(body), err } - err = json.Unmarshal(body, object) + err = api.DecodeInto(body, object) return string(body), err } @@ -150,6 +157,7 @@ func TestNonEmptyList(t *testing.T) { body, err := extractBody(resp, &listOut) if len(listOut.Items) != 1 { t.Errorf("Unexpected response: %#v", listOut) + return } if listOut.Items[0].Name != simpleStorage.list[0].Name { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0], string(body)) From 41534c1cc5f4fd96051463e6f37bcc6db6dd239a Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Fri, 20 Jun 2014 16:50:56 -0700 Subject: [PATCH 6/7] Encode/decode working everywhere now. --- cmd/cloudcfg/cloudcfg.go | 2 +- pkg/api/helper.go | 10 ++- pkg/api/helper_test.go | 14 +++- pkg/api/types.go | 18 +++++ pkg/apiserver/apiserver.go | 7 +- pkg/apiserver/apiserver_test.go | 9 ++- pkg/cloudcfg/cloudcfg.go | 6 +- pkg/cloudcfg/cloudcfg_test.go | 2 +- pkg/cloudcfg/parse.go | 6 +- pkg/cloudcfg/parse_test.go | 3 +- pkg/cloudcfg/resource_printer.go | 117 ++++++++++------------------ pkg/registry/controller_registry.go | 4 +- pkg/registry/pod_registry.go | 2 +- pkg/registry/service_registry.go | 2 +- 14 files changed, 98 insertions(+), 104 deletions(-) diff --git a/cmd/cloudcfg/cloudcfg.go b/cmd/cloudcfg/cloudcfg.go index 2ba1cc3216..4c4076e2c9 100644 --- a/cmd/cloudcfg/cloudcfg.go +++ b/cmd/cloudcfg/cloudcfg.go @@ -168,7 +168,7 @@ func executeAPIRequest(method string, auth *kube_client.AuthInfo) bool { printer = &cloudcfg.HumanReadablePrinter{} } - var body string + var body []byte if body, err = cloudcfg.DoRequest(request, auth); err == nil { if err = printer.Print(body, os.Stdout); err != nil { log.Fatalf("Failed to print: %#v\nRaw received text:\n%v\n", err, string(body)) diff --git a/pkg/api/helper.go b/pkg/api/helper.go index fdb86d351f..9754ba141c 100644 --- a/pkg/api/helper.go +++ b/pkg/api/helper.go @@ -20,6 +20,8 @@ import ( "encoding/json" "fmt" "reflect" + + "gopkg.in/v1/yaml" ) var knownTypes = map[string]reflect.Type{} @@ -29,6 +31,7 @@ func init() { PodList{}, Pod{}, ReplicationControllerList{}, ReplicationController{}, ServiceList{}, Service{}, + Status{}, ) } @@ -117,7 +120,8 @@ func Decode(data []byte) (interface{}, error) { findKind := struct { Kind string `json:"kind,omitempty" yaml:"kind,omitempty"` }{} - err := json.Unmarshal(data, &findKind) + // yaml is a superset of json, so we use it to decode here. That way, we understand both. + err := yaml.Unmarshal(data, &findKind) if err != nil { return nil, fmt.Errorf("Couldn't get kind: %#v", err) } @@ -126,7 +130,7 @@ func Decode(data []byte) (interface{}, error) { return nil, fmt.Errorf("%v is not a known type", findKind.Kind) } obj := reflect.New(objType).Interface() - err = json.Unmarshal(data, obj) + err = yaml.Unmarshal(data, obj) if err != nil { return nil, err } @@ -143,7 +147,7 @@ func Decode(data []byte) (interface{}, error) { // if data.Kind is set and doesn't match the type of obj. Obj should be a // pointer to an api type. func DecodeInto(data []byte, obj interface{}) error { - err := json.Unmarshal(data, obj) + err := yaml.Unmarshal(data, obj) if err != nil { return err } diff --git a/pkg/api/helper_test.go b/pkg/api/helper_test.go index d24083161d..8c8e6f04d3 100644 --- a/pkg/api/helper_test.go +++ b/pkg/api/helper_test.go @@ -61,7 +61,19 @@ func TestTypes(t *testing.T) { }, }, &Service{}, - &ServiceList{}, + &ServiceList{ + Items: []Service{ + { + Labels: map[string]string{ + "foo": "bar", + }, + }, { + Labels: map[string]string{ + "foo": "baz", + }, + }, + }, + }, &ReplicationControllerList{}, &ReplicationController{}, &PodList{}, diff --git a/pkg/api/types.go b/pkg/api/types.go index 36ce624765..fdbd8eb90a 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -174,3 +174,21 @@ type Endpoints struct { Name string Endpoints []string } + +// Status is a return value for calls that don't return other objects. +// Arguably, this could go in apiserver, but I'm including it here so clients needn't +// import both. +type Status struct { + JSONBase `json:",inline" yaml:",inline"` + // One of: "success", "failure", "working" (for operations not yet completed) + // TODO: if "working", include an operation identifier so final status can be + // checked. + Status string `json:"status,omitempty" yaml:"status,omitempty"` +} + +// Values of Status.Status +const ( + StatusSuccess = "success" + StatusFailure = "failure" + StatusWorking = "working" +) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index c5fd4b85cd..3ae9305253 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -50,11 +50,6 @@ func MakeAsync(fn func() interface{}) <-chan interface{} { return channel } -// Status is a return value for calls that don't return other objects -type Status struct { - Success bool -} - // ApiServer is an HTTPHandler that delegates to RESTStorage objects. // It handles URLs of the form: // ${prefix}/${storage_key}[/${object_name}] @@ -248,7 +243,7 @@ func (server *ApiServer) handleREST(parts []string, requestUrl *url.URL, req *ht } out, err := storage.Delete(parts[1]) var obj interface{} - obj = Status{Success: true} + obj = api.Status{Status: api.StatusSuccess} if err == nil && sync { obj, err = server.waitForObject(out, timeout) } diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 006a6bfc0d..5f6b876e4c 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -42,13 +42,13 @@ func expectNoError(t *testing.T, err error) { } type Simple struct { - JSONBase api.JSONBase `json:",inline"` - Name string + api.JSONBase `yaml:",inline" json:",inline"` + Name string `yaml:"name,omitempty" json:"name,omitempty"` } type SimpleList struct { - JSONBase api.JSONBase `json:",inline"` - Items []Simple + api.JSONBase `yaml:",inline" json:",inline"` + Items []Simple `yaml:"items,omitempty" json:"items,omitempty"` } type SimpleRESTStorage struct { @@ -155,6 +155,7 @@ func TestNonEmptyList(t *testing.T) { var listOut SimpleList body, err := extractBody(resp, &listOut) + expectNoError(t, err) if len(listOut.Items) != 1 { t.Errorf("Unexpected response: %#v", listOut) return diff --git a/pkg/cloudcfg/cloudcfg.go b/pkg/cloudcfg/cloudcfg.go index fb097703c7..6925b62d5c 100644 --- a/pkg/cloudcfg/cloudcfg.go +++ b/pkg/cloudcfg/cloudcfg.go @@ -113,7 +113,7 @@ func RequestWithBodyData(data []byte, url, method string) (*http.Request, error) } // Execute a request, adds authentication (if auth != nil), and HTTPS cert ignoring. -func DoRequest(request *http.Request, auth *client.AuthInfo) (string, error) { +func DoRequest(request *http.Request, auth *client.AuthInfo) ([]byte, error) { if auth != nil { request.SetBasicAuth(auth.User, auth.Password) } @@ -123,11 +123,11 @@ func DoRequest(request *http.Request, auth *client.AuthInfo) (string, error) { client := &http.Client{Transport: tr} response, err := client.Do(request) if err != nil { - return "", err + return []byte{}, err } defer response.Body.Close() body, err := ioutil.ReadAll(response.Body) - return string(body), err + return body, err } // StopController stops a controller named 'name' by setting replicas to zero diff --git a/pkg/cloudcfg/cloudcfg_test.go b/pkg/cloudcfg/cloudcfg_test.go index 96e47aa330..178d983df5 100644 --- a/pkg/cloudcfg/cloudcfg_test.go +++ b/pkg/cloudcfg/cloudcfg_test.go @@ -164,7 +164,7 @@ func TestDoRequest(t *testing.T) { if err != nil { t.Error("Unexpected error") } - if body != expectedBody { + if string(body) != expectedBody { t.Errorf("Expected body: '%s', saw: '%s'", expectedBody, body) } fakeHandler.ValidateRequest(t, "/foo/bar", "GET", nil) diff --git a/pkg/cloudcfg/parse.go b/pkg/cloudcfg/parse.go index 4e1f4d0e17..466384f840 100644 --- a/pkg/cloudcfg/parse.go +++ b/pkg/cloudcfg/parse.go @@ -1,12 +1,10 @@ package cloudcfg import ( - "encoding/json" "fmt" "reflect" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "gopkg.in/v1/yaml" ) var storageToType = map[string]reflect.Type{ @@ -25,9 +23,9 @@ func ToWireFormat(data []byte, storage string) ([]byte, error) { } obj := reflect.New(prototypeType).Interface() - err := yaml.Unmarshal(data, obj) + err := api.DecodeInto(data, obj) if err != nil { return nil, err } - return json.Marshal(obj) + return api.Encode(obj) } diff --git a/pkg/cloudcfg/parse_test.go b/pkg/cloudcfg/parse_test.go index f8ac85b89d..e299fd22b2 100644 --- a/pkg/cloudcfg/parse_test.go +++ b/pkg/cloudcfg/parse_test.go @@ -1,7 +1,6 @@ package cloudcfg import ( - "encoding/json" "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -16,7 +15,7 @@ func TestParseBadStorage(t *testing.T) { } func DoParseTest(t *testing.T, storage string, obj interface{}) { - json_data, _ := json.Marshal(obj) + json_data, _ := api.Encode(obj) yaml_data, _ := yaml.Marshal(obj) t.Logf("Intermediate yaml:\n%v\n", string(yaml_data)) diff --git a/pkg/cloudcfg/resource_printer.go b/pkg/cloudcfg/resource_printer.go index 9368d57d50..6f2643cacc 100644 --- a/pkg/cloudcfg/resource_printer.go +++ b/pkg/cloudcfg/resource_printer.go @@ -31,23 +31,23 @@ import ( // ResourcePrinter is an interface that knows how to print API resources type ResourcePrinter interface { // Print receives an arbitrary JSON body, formats it and prints it to a writer - Print(string, io.Writer) error + Print([]byte, io.Writer) error } // Identity printer simply copies the body out to the output stream type IdentityPrinter struct{} -func (i *IdentityPrinter) Print(data string, w io.Writer) error { - _, err := fmt.Fprint(w, data) +func (i *IdentityPrinter) Print(data []byte, w io.Writer) error { + _, err := w.Write(data) return err } // YAMLPrinter parses JSON, and re-formats as YAML type YAMLPrinter struct{} -func (y *YAMLPrinter) Print(data string, w io.Writer) error { +func (y *YAMLPrinter) Print(data []byte, w io.Writer) error { var obj interface{} - if err := json.Unmarshal([]byte(data), &obj); err != nil { + if err := json.Unmarshal(data, &obj); err != nil { return err } output, err := yaml.Marshal(obj) @@ -64,9 +64,10 @@ type HumanReadablePrinter struct{} var podColumns = []string{"Name", "Image(s)", "Host", "Labels"} var replicationControllerColumns = []string{"Name", "Image(s)", "Selector", "Replicas"} var serviceColumns = []string{"Name", "Labels", "Selector", "Port"} +var statusColumns = []string{"Status"} -func (h *HumanReadablePrinter) unknown(data string, w io.Writer) error { - _, err := fmt.Fprintf(w, "Unknown object: %s", data) +func (h *HumanReadablePrinter) unknown(data []byte, w io.Writer) error { + _, err := fmt.Fprintf(w, "Unknown object: %s", string(data)) return err } @@ -90,97 +91,62 @@ func (h *HumanReadablePrinter) makeImageList(manifest api.ContainerManifest) str return strings.Join(images, ",") } -func (h *HumanReadablePrinter) printPod(pod api.Pod, w io.Writer) error { +func (h *HumanReadablePrinter) printPod(pod *api.Pod, w io.Writer) error { _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", pod.ID, h.makeImageList(pod.DesiredState.Manifest), pod.CurrentState.Host+"/"+pod.CurrentState.HostIP, labels.Set(pod.Labels)) return err } -func (h *HumanReadablePrinter) printPodList(podList api.PodList, w io.Writer) error { +func (h *HumanReadablePrinter) printPodList(podList *api.PodList, w io.Writer) error { for _, pod := range podList.Items { - if err := h.printPod(pod, w); err != nil { + if err := h.printPod(&pod, w); err != nil { return err } } return nil } -func (h *HumanReadablePrinter) printReplicationController(ctrl api.ReplicationController, w io.Writer) error { +func (h *HumanReadablePrinter) printReplicationController(ctrl *api.ReplicationController, w io.Writer) error { _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%d\n", ctrl.ID, h.makeImageList(ctrl.DesiredState.PodTemplate.DesiredState.Manifest), labels.Set(ctrl.DesiredState.ReplicaSelector), ctrl.DesiredState.Replicas) return err } -func (h *HumanReadablePrinter) printReplicationControllerList(list api.ReplicationControllerList, w io.Writer) error { +func (h *HumanReadablePrinter) printReplicationControllerList(list *api.ReplicationControllerList, w io.Writer) error { for _, ctrl := range list.Items { - if err := h.printReplicationController(ctrl, w); err != nil { + if err := h.printReplicationController(&ctrl, w); err != nil { return err } } return nil } -func (h *HumanReadablePrinter) printService(svc api.Service, w io.Writer) error { +func (h *HumanReadablePrinter) printService(svc *api.Service, w io.Writer) error { _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%d\n", svc.ID, labels.Set(svc.Labels), labels.Set(svc.Selector), svc.Port) return err } -func (h *HumanReadablePrinter) printServiceList(list api.ServiceList, w io.Writer) error { +func (h *HumanReadablePrinter) printServiceList(list *api.ServiceList, w io.Writer) error { for _, svc := range list.Items { - if err := h.printService(svc, w); err != nil { + if err := h.printService(&svc, w); err != nil { return err } } return nil } -// TODO replace this with something that returns a concrete printer object, rather than -// having the secondary switch below. -func (h *HumanReadablePrinter) extractObject(data, kind string) (interface{}, error) { - // TODO: I think this can be replaced with some reflection and a map[string]type - switch kind { - case "cluster#pod": - var obj api.Pod - if err := json.Unmarshal([]byte(data), &obj); err != nil { - return nil, err - } - return obj, nil - case "cluster#podList": - var list api.PodList - if err := json.Unmarshal([]byte(data), &list); err != nil { - return nil, err - } - return list, nil - case "cluster#replicationController": - var ctrl api.ReplicationController - if err := json.Unmarshal([]byte(data), &ctrl); err != nil { - return nil, err - } - return ctrl, nil - case "cluster#replicationControllerList": - var list api.ReplicationControllerList - if err := json.Unmarshal([]byte(data), &list); err != nil { - return nil, err - } - return list, nil - case "cluster#service": - var ctrl api.Service - if err := json.Unmarshal([]byte(data), &ctrl); err != nil { - return nil, err - } - return ctrl, nil - case "cluster#serviceList": - var list api.ServiceList - if err := json.Unmarshal([]byte(data), &list); err != nil { - return nil, err - } - return list, nil - default: - return nil, fmt.Errorf("unknown kind: %s", kind) +func (h *HumanReadablePrinter) printStatus(status *api.Status, w io.Writer) error { + err := h.printHeader(statusColumns, w) + if err != nil { + return err } + _, err = fmt.Fprintf(w, "%v\n", status.Status) + return err } -func (h *HumanReadablePrinter) Print(data string, output io.Writer) error { +// TODO replace this with something that returns a concrete printer object, rather than +// having the secondary switch below. +func (h *HumanReadablePrinter) Print(data []byte, output io.Writer) error { w := tabwriter.NewWriter(output, 20, 5, 3, ' ', 0) defer w.Flush() var mapObj map[string]interface{} @@ -198,30 +164,31 @@ func (h *HumanReadablePrinter) Print(data string, output io.Writer) error { return fmt.Errorf("unexpected object with no 'kind' field: %s", data) } - kind := (mapObj["kind"]).(string) - obj, err := h.extractObject(data, kind) + obj, err := api.Decode(data) if err != nil { return err } - switch obj.(type) { - case api.Pod: + switch o := obj.(type) { + case *api.Pod: h.printHeader(podColumns, w) - return h.printPod(obj.(api.Pod), w) - case api.PodList: + return h.printPod(o, w) + case *api.PodList: h.printHeader(podColumns, w) - return h.printPodList(obj.(api.PodList), w) - case api.ReplicationController: + return h.printPodList(o, w) + case *api.ReplicationController: h.printHeader(replicationControllerColumns, w) - return h.printReplicationController(obj.(api.ReplicationController), w) - case api.ReplicationControllerList: + return h.printReplicationController(o, w) + case *api.ReplicationControllerList: h.printHeader(replicationControllerColumns, w) - return h.printReplicationControllerList(obj.(api.ReplicationControllerList), w) - case api.Service: + return h.printReplicationControllerList(o, w) + case *api.Service: h.printHeader(serviceColumns, w) - return h.printService(obj.(api.Service), w) - case api.ServiceList: + return h.printService(o, w) + case *api.ServiceList: h.printHeader(serviceColumns, w) - return h.printServiceList(obj.(api.ServiceList), w) + return h.printServiceList(o, w) + case *api.Status: + return h.printStatus(o, w) default: return h.unknown(data, w) } diff --git a/pkg/registry/controller_registry.go b/pkg/registry/controller_registry.go index f5dc00dd36..bf7722e83b 100644 --- a/pkg/registry/controller_registry.go +++ b/pkg/registry/controller_registry.go @@ -34,7 +34,7 @@ func MakeControllerRegistryStorage(registry ControllerRegistry) apiserver.RESTSt } func (storage *ControllerRegistryStorage) List(selector labels.Selector) (interface{}, error) { - result := api.ReplicationControllerList{JSONBase: api.JSONBase{Kind: "cluster#replicationControllerList"}} + result := api.ReplicationControllerList{} controllers, err := storage.registry.ListControllers() if err == nil { for _, controller := range controllers { @@ -55,7 +55,7 @@ func (storage *ControllerRegistryStorage) Get(id string) (interface{}, error) { } func (storage *ControllerRegistryStorage) Delete(id string) (<-chan interface{}, error) { - return apiserver.MakeAsync(func() interface{} { return apiserver.Status{Success: true} }), storage.registry.DeleteController(id) + return apiserver.MakeAsync(func() interface{} { return api.Status{Status: api.StatusSuccess} }), storage.registry.DeleteController(id) } func (storage *ControllerRegistryStorage) Extract(body []byte) (interface{}, error) { diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index f74dea980f..73e12eb04b 100644 --- a/pkg/registry/pod_registry.go +++ b/pkg/registry/pod_registry.go @@ -130,7 +130,7 @@ func (storage *PodRegistryStorage) Get(id string) (interface{}, error) { } func (storage *PodRegistryStorage) Delete(id string) (<-chan interface{}, error) { - return apiserver.MakeAsync(func() interface{} { return apiserver.Status{Success: true} }), storage.registry.DeletePod(id) + return apiserver.MakeAsync(func() interface{} { return api.Status{Status: api.StatusSuccess} }), storage.registry.DeletePod(id) } func (storage *PodRegistryStorage) Extract(body []byte) (interface{}, error) { diff --git a/pkg/registry/service_registry.go b/pkg/registry/service_registry.go index 59faa1a8ff..b55912054d 100644 --- a/pkg/registry/service_registry.go +++ b/pkg/registry/service_registry.go @@ -99,7 +99,7 @@ func (sr *ServiceRegistryStorage) Delete(id string) (<-chan interface{}, error) } } } - return apiserver.MakeAsync(func() interface{} { return apiserver.Status{Success: true} }), sr.registry.DeleteService(id) + return apiserver.MakeAsync(func() interface{} { return api.Status{Status: api.StatusSuccess} }), sr.registry.DeleteService(id) } func (sr *ServiceRegistryStorage) Extract(body []byte) (interface{}, error) { From ee75bb8dbeb1c7adfd92eb3a014a187b70a3e625 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 23 Jun 2014 09:54:01 -0700 Subject: [PATCH 7/7] Address comments; Also internally start using JSONBase instead of reflect.Value of JSONBase. --- pkg/api/helper.go | 57 +++++++++++++++----------------------- pkg/apiserver/apiserver.go | 2 +- 2 files changed, 24 insertions(+), 35 deletions(-) diff --git a/pkg/api/helper.go b/pkg/api/helper.go index 9754ba141c..b4fe0c42d9 100644 --- a/pkg/api/helper.go +++ b/pkg/api/helper.go @@ -28,9 +28,12 @@ var knownTypes = map[string]reflect.Type{} func init() { AddKnownTypes( - PodList{}, Pod{}, - ReplicationControllerList{}, ReplicationController{}, - ServiceList{}, Service{}, + PodList{}, + Pod{}, + ReplicationControllerList{}, + ReplicationController{}, + ServiceList{}, + Service{}, Status{}, ) } @@ -52,25 +55,13 @@ func AddKnownTypes(types ...interface{}) { // format. func Encode(obj interface{}) (data []byte, err error) { obj = checkPtr(obj) - fieldToReset, err := prepareEncode(obj) - if err != nil { - return nil, err - } - data, err = json.Marshal(obj) - fieldToReset.SetString("") - return -} - -// Just like Encode, but produces indented output. -func EncodeIndent(obj interface{}) (data []byte, err error) { - obj = checkPtr(obj) - fieldToReset, err := prepareEncode(obj) + jsonBase, err := prepareEncode(obj) if err != nil { return nil, err } data, err = json.MarshalIndent(obj, "", " ") - fieldToReset.SetString("") - return + jsonBase.Kind = "" + return data, err } func checkPtr(obj interface{}) interface{} { @@ -83,35 +74,34 @@ func checkPtr(obj interface{}) interface{} { return v2.Interface() } -func prepareEncode(obj interface{}) (reflect.Value, error) { +func prepareEncode(obj interface{}) (*JSONBase, error) { name, jsonBase, err := nameAndJSONBase(obj) if err != nil { - return reflect.Value{}, err + return nil, err } if _, contains := knownTypes[name]; !contains { - return reflect.Value{}, fmt.Errorf("struct %v won't be unmarshalable because it's not in knownTypes", name) + return nil, fmt.Errorf("struct %v won't be unmarshalable because it's not in knownTypes", name) } - kind := jsonBase.FieldByName("Kind") - kind.SetString(name) - return kind, nil + jsonBase.Kind = name + return jsonBase, nil } // Returns the name of the type (sans pointer), and its kind field. Takes pointer-to-struct.. -func nameAndJSONBase(obj interface{}) (string, reflect.Value, error) { +func nameAndJSONBase(obj interface{}) (string, *JSONBase, error) { v := reflect.ValueOf(obj) if v.Kind() != reflect.Ptr { - return "", reflect.Value{}, fmt.Errorf("expected pointer, but got %v", v.Type().Name()) + return "", nil, fmt.Errorf("expected pointer, but got %v", v.Type().Name()) } v = v.Elem() name := v.Type().Name() if v.Kind() != reflect.Struct { - return "", reflect.Value{}, fmt.Errorf("expected struct, but got %v", name) + return "", nil, fmt.Errorf("expected struct, but got %v", name) } jsonBase := v.FieldByName("JSONBase") if !jsonBase.IsValid() { - return "", reflect.Value{}, fmt.Errorf("struct %v lacks embedded JSON type", name) + return "", nil, fmt.Errorf("struct %v lacks embedded JSON type", name) } - return name, jsonBase, nil + return name, jsonBase.Addr().Interface().(*JSONBase), nil } // Decode converts a JSON string back into a pointer to an api object. Deduces the type @@ -139,7 +129,7 @@ func Decode(data []byte) (interface{}, error) { return nil, err } // Don't leave these set. Track type with go's type. - jsonBase.FieldByName("Kind").SetString("") + jsonBase.Kind = "" return obj, nil } @@ -155,11 +145,10 @@ func DecodeInto(data []byte, obj interface{}) error { if err != nil { return err } - foundName := jsonBase.FieldByName("Kind").Interface().(string) - if foundName != "" && foundName != name { - return fmt.Errorf("data had kind %v, but passed object was of type %v", foundName, name) + if jsonBase.Kind != "" && jsonBase.Kind != name { + return fmt.Errorf("data had kind %v, but passed object was of type %v", jsonBase.Kind, name) } // Don't leave these set. Track type with go's type. - jsonBase.FieldByName("Kind").SetString("") + jsonBase.Kind = "" return nil } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 3ae9305253..e7b91fe51c 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -125,7 +125,7 @@ func (server *ApiServer) notFound(req *http.Request, w http.ResponseWriter) { func (server *ApiServer) write(statusCode int, object interface{}, w http.ResponseWriter) { w.WriteHeader(statusCode) - output, err := api.EncodeIndent(object) + output, err := api.Encode(object) if err != nil { server.error(err, w) return