From 9d52b0fc0801e8a37d06e99fc962985fcd73704c Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Mon, 7 Sep 2015 18:36:35 -0700 Subject: [PATCH 1/5] add GroupVersion struct --- pkg/api/unversioned/group_version.go | 75 +++++++++++++++++++++++ pkg/api/unversioned/group_version_test.go | 68 ++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 pkg/api/unversioned/group_version.go create mode 100644 pkg/api/unversioned/group_version_test.go diff --git a/pkg/api/unversioned/group_version.go b/pkg/api/unversioned/group_version.go new file mode 100644 index 00000000000..acbbc166004 --- /dev/null +++ b/pkg/api/unversioned/group_version.go @@ -0,0 +1,75 @@ +/* +Copyright 2015 The Kubernetes Authors 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 unversioned + +import ( + "encoding/json" + "fmt" + "strings" +) + +// TODO: We need to remove the GroupVersion in types.go. We use the name GroupAndVersion here temporarily. +type GroupAndVersion struct { + Group string + Version string +} + +func (gv *GroupAndVersion) String() string { + // special case of "v1" for backward compatibility + if gv.Group == "" { + return gv.Version + } else { + return gv.Group + "/" + gv.Version + } +} + +func ParseGroupVersion(gv string) (GroupAndVersion, error) { + s := strings.Split(gv, "/") + // "v1" is the only special case. Otherwise GroupVersion is expected to contain + // one "/" dividing the string into two parts. + if len(s) == 1 && gv == "v1" { + return GroupAndVersion{"", "v1"}, nil + } else if len(s) == 2 { + return GroupAndVersion{s[0], s[1]}, nil + } else { + return GroupAndVersion{}, fmt.Errorf("Unexpected GroupVersion string: %v", gv) + } +} + +// MarshalJSON implements the json.Marshaller interface. +func (gv GroupAndVersion) MarshalJSON() ([]byte, error) { + s := gv.String() + if strings.Count(s, "/") > 1 { + return []byte{}, fmt.Errorf("illegal GroupVersion %v: contains more than one /", s) + } + return json.Marshal(s) +} + +// UnmarshalJSON implements the json.Unmarshaller interface. +func (gv *GroupAndVersion) UnmarshalJSON(value []byte) error { + var s string + if err := json.Unmarshal(value, &s); err != nil { + return err + } + parsed, err := ParseGroupVersion(s) + if err != nil { + return err + } + gv.Group = parsed.Group + gv.Version = parsed.Version + return nil +} diff --git a/pkg/api/unversioned/group_version_test.go b/pkg/api/unversioned/group_version_test.go new file mode 100644 index 00000000000..daf1c3da779 --- /dev/null +++ b/pkg/api/unversioned/group_version_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2015 The Kubernetes Authors 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 unversioned + +import ( + "encoding/json" + "reflect" + "testing" +) + +type GroupVersionHolder struct { + GV GroupAndVersion `json:"val"` +} + +func TestGroupVersionUnmarshalJSON(t *testing.T) { + cases := []struct { + input []byte + expect GroupAndVersion + }{ + {[]byte(`{"val": "v1"}`), GroupAndVersion{"", "v1"}}, + {[]byte(`{"val": "extensions/v1beta1"}`), GroupAndVersion{"extensions", "v1beta1"}}, + } + + for _, c := range cases { + var result GroupVersionHolder + if err := json.Unmarshal([]byte(c.input), &result); err != nil { + t.Errorf("Failed to unmarshal input '%v': %v", c.input, err) + } + if !reflect.DeepEqual(result.GV, c.expect) { + t.Errorf("Failed to unmarshal input '%s': expected %+v, got %+v", c.input, c.expect, result.GV) + } + } +} + +func TestGroupVersionMarshalJSON(t *testing.T) { + cases := []struct { + input GroupAndVersion + expect []byte + }{ + {GroupAndVersion{"", "v1"}, []byte(`{"val":"v1"}`)}, + {GroupAndVersion{"extensions", "v1beta1"}, []byte(`{"val":"extensions/v1beta1"}`)}, + } + + for _, c := range cases { + input := GroupVersionHolder{c.input} + result, err := json.Marshal(&input) + if err != nil { + t.Errorf("Failed to marshal input '%v': %v", input, err) + } + if !reflect.DeepEqual(result, c.expect) { + t.Errorf("Failed to marshal input '%+v': expected: %s, got: %s", input, c.expect, result) + } + } +} From 6419924a5ea148ed7c0be117585becc23801d94d Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Thu, 5 Nov 2015 13:46:44 -0800 Subject: [PATCH 2/5] add ugorji unmarshaller and address comments --- pkg/api/unversioned/group_version.go | 25 +++++++++++++++-------- pkg/api/unversioned/group_version_test.go | 14 +++++++++++-- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/pkg/api/unversioned/group_version.go b/pkg/api/unversioned/group_version.go index acbbc166004..1427a03c666 100644 --- a/pkg/api/unversioned/group_version.go +++ b/pkg/api/unversioned/group_version.go @@ -30,7 +30,7 @@ type GroupAndVersion struct { func (gv *GroupAndVersion) String() string { // special case of "v1" for backward compatibility - if gv.Group == "" { + if gv.Group == "" && gv.Version == "v1" { return gv.Version } else { return gv.Group + "/" + gv.Version @@ -41,11 +41,12 @@ func ParseGroupVersion(gv string) (GroupAndVersion, error) { s := strings.Split(gv, "/") // "v1" is the only special case. Otherwise GroupVersion is expected to contain // one "/" dividing the string into two parts. - if len(s) == 1 && gv == "v1" { + switch { + case len(s) == 1 && gv == "v1": return GroupAndVersion{"", "v1"}, nil - } else if len(s) == 2 { + case len(s) == 2: return GroupAndVersion{s[0], s[1]}, nil - } else { + default: return GroupAndVersion{}, fmt.Errorf("Unexpected GroupVersion string: %v", gv) } } @@ -59,8 +60,7 @@ func (gv GroupAndVersion) MarshalJSON() ([]byte, error) { return json.Marshal(s) } -// UnmarshalJSON implements the json.Unmarshaller interface. -func (gv *GroupAndVersion) UnmarshalJSON(value []byte) error { +func (gv *GroupAndVersion) unmarshal(value []byte) error { var s string if err := json.Unmarshal(value, &s); err != nil { return err @@ -69,7 +69,16 @@ func (gv *GroupAndVersion) UnmarshalJSON(value []byte) error { if err != nil { return err } - gv.Group = parsed.Group - gv.Version = parsed.Version + *gv = parsed return nil } + +// UnmarshalJSON implements the json.Unmarshaller interface. +func (gv *GroupAndVersion) UnmarshalJSON(value []byte) error { + return gv.unmarshal(value) +} + +// UnmarshalTEXT implements the Ugorji's encoding.TextUnmarshaler interface. +func (gv *GroupAndVersion) UnmarshalText(value []byte) error { + return gv.unmarshal(value) +} diff --git a/pkg/api/unversioned/group_version_test.go b/pkg/api/unversioned/group_version_test.go index daf1c3da779..3c56b469e04 100644 --- a/pkg/api/unversioned/group_version_test.go +++ b/pkg/api/unversioned/group_version_test.go @@ -20,6 +20,8 @@ import ( "encoding/json" "reflect" "testing" + + "github.com/ugorji/go/codec" ) type GroupVersionHolder struct { @@ -37,11 +39,19 @@ func TestGroupVersionUnmarshalJSON(t *testing.T) { for _, c := range cases { var result GroupVersionHolder + // test golang lib's JSON codec if err := json.Unmarshal([]byte(c.input), &result); err != nil { - t.Errorf("Failed to unmarshal input '%v': %v", c.input, err) + t.Errorf("JSON codec failed to unmarshal input '%v': %v", c.input, err) } if !reflect.DeepEqual(result.GV, c.expect) { - t.Errorf("Failed to unmarshal input '%s': expected %+v, got %+v", c.input, c.expect, result.GV) + t.Errorf("JSON codec failed to unmarshal input '%s': expected %+v, got %+v", c.input, c.expect, result.GV) + } + // test the Ugorji codec + if err := codec.NewDecoderBytes(c.input, new(codec.JsonHandle)).Decode(&result); err != nil { + t.Errorf("Ugorji codec failed to unmarshal input '%v': %v", c.input, err) + } + if !reflect.DeepEqual(result.GV, c.expect) { + t.Errorf("Ugorji codec failed to unmarshal input '%s': expected %+v, got %+v", c.input, c.expect, result.GV) } } } From fb360bca98c0044e20508a6e324876a508936ea7 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Thu, 5 Nov 2015 14:15:01 -0800 Subject: [PATCH 3/5] add comments --- pkg/api/unversioned/group_version.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/api/unversioned/group_version.go b/pkg/api/unversioned/group_version.go index 1427a03c666..d5331541845 100644 --- a/pkg/api/unversioned/group_version.go +++ b/pkg/api/unversioned/group_version.go @@ -23,11 +23,14 @@ import ( ) // TODO: We need to remove the GroupVersion in types.go. We use the name GroupAndVersion here temporarily. +// GroupVersion contains the "group" and the "version", which uniquely identifies the API. type GroupAndVersion struct { Group string Version string } +// String puts "group" and "version" into a single "group/version" string. For the legacy v1 +// it returns "v1". func (gv *GroupAndVersion) String() string { // special case of "v1" for backward compatibility if gv.Group == "" && gv.Version == "v1" { @@ -37,6 +40,8 @@ func (gv *GroupAndVersion) String() string { } } +// ParseGroupVersion turns "group/version" string into a GroupVersion struct. It reports error +// if it cannot parse the string. func ParseGroupVersion(gv string) (GroupAndVersion, error) { s := strings.Split(gv, "/") // "v1" is the only special case. Otherwise GroupVersion is expected to contain From 15e6ca5ac530adec3f05eaa4437048ec4518fc35 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Thu, 5 Nov 2015 14:52:58 -0800 Subject: [PATCH 4/5] rename to GroupVersion and rename the one in type.go to GroupVersionForDiscovery --- pkg/api/unversioned/group_version.go | 22 +++++++++++----------- pkg/api/unversioned/group_version_test.go | 14 +++++++------- pkg/api/unversioned/types.go | 6 +++--- pkg/client/unversioned/client_test.go | 2 +- pkg/client/unversioned/discovery_client.go | 4 ++-- pkg/client/unversioned/helper_test.go | 4 ++-- pkg/master/master.go | 8 ++++---- pkg/master/master_test.go | 4 ++-- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/pkg/api/unversioned/group_version.go b/pkg/api/unversioned/group_version.go index d5331541845..2cd68baa3f5 100644 --- a/pkg/api/unversioned/group_version.go +++ b/pkg/api/unversioned/group_version.go @@ -22,16 +22,16 @@ import ( "strings" ) -// TODO: We need to remove the GroupVersion in types.go. We use the name GroupAndVersion here temporarily. +// TODO: We need to remove the GroupVersion in types.go. We use the name GroupVersion here temporarily. // GroupVersion contains the "group" and the "version", which uniquely identifies the API. -type GroupAndVersion struct { +type GroupVersion struct { Group string Version string } // String puts "group" and "version" into a single "group/version" string. For the legacy v1 // it returns "v1". -func (gv *GroupAndVersion) String() string { +func (gv *GroupVersion) String() string { // special case of "v1" for backward compatibility if gv.Group == "" && gv.Version == "v1" { return gv.Version @@ -42,22 +42,22 @@ func (gv *GroupAndVersion) String() string { // ParseGroupVersion turns "group/version" string into a GroupVersion struct. It reports error // if it cannot parse the string. -func ParseGroupVersion(gv string) (GroupAndVersion, error) { +func ParseGroupVersion(gv string) (GroupVersion, error) { s := strings.Split(gv, "/") // "v1" is the only special case. Otherwise GroupVersion is expected to contain // one "/" dividing the string into two parts. switch { case len(s) == 1 && gv == "v1": - return GroupAndVersion{"", "v1"}, nil + return GroupVersion{"", "v1"}, nil case len(s) == 2: - return GroupAndVersion{s[0], s[1]}, nil + return GroupVersion{s[0], s[1]}, nil default: - return GroupAndVersion{}, fmt.Errorf("Unexpected GroupVersion string: %v", gv) + return GroupVersion{}, fmt.Errorf("Unexpected GroupVersion string: %v", gv) } } // MarshalJSON implements the json.Marshaller interface. -func (gv GroupAndVersion) MarshalJSON() ([]byte, error) { +func (gv GroupVersion) MarshalJSON() ([]byte, error) { s := gv.String() if strings.Count(s, "/") > 1 { return []byte{}, fmt.Errorf("illegal GroupVersion %v: contains more than one /", s) @@ -65,7 +65,7 @@ func (gv GroupAndVersion) MarshalJSON() ([]byte, error) { return json.Marshal(s) } -func (gv *GroupAndVersion) unmarshal(value []byte) error { +func (gv *GroupVersion) unmarshal(value []byte) error { var s string if err := json.Unmarshal(value, &s); err != nil { return err @@ -79,11 +79,11 @@ func (gv *GroupAndVersion) unmarshal(value []byte) error { } // UnmarshalJSON implements the json.Unmarshaller interface. -func (gv *GroupAndVersion) UnmarshalJSON(value []byte) error { +func (gv *GroupVersion) UnmarshalJSON(value []byte) error { return gv.unmarshal(value) } // UnmarshalTEXT implements the Ugorji's encoding.TextUnmarshaler interface. -func (gv *GroupAndVersion) UnmarshalText(value []byte) error { +func (gv *GroupVersion) UnmarshalText(value []byte) error { return gv.unmarshal(value) } diff --git a/pkg/api/unversioned/group_version_test.go b/pkg/api/unversioned/group_version_test.go index 3c56b469e04..4a26fbb106a 100644 --- a/pkg/api/unversioned/group_version_test.go +++ b/pkg/api/unversioned/group_version_test.go @@ -25,16 +25,16 @@ import ( ) type GroupVersionHolder struct { - GV GroupAndVersion `json:"val"` + GV GroupVersion `json:"val"` } func TestGroupVersionUnmarshalJSON(t *testing.T) { cases := []struct { input []byte - expect GroupAndVersion + expect GroupVersion }{ - {[]byte(`{"val": "v1"}`), GroupAndVersion{"", "v1"}}, - {[]byte(`{"val": "extensions/v1beta1"}`), GroupAndVersion{"extensions", "v1beta1"}}, + {[]byte(`{"val": "v1"}`), GroupVersion{"", "v1"}}, + {[]byte(`{"val": "extensions/v1beta1"}`), GroupVersion{"extensions", "v1beta1"}}, } for _, c := range cases { @@ -58,11 +58,11 @@ func TestGroupVersionUnmarshalJSON(t *testing.T) { func TestGroupVersionMarshalJSON(t *testing.T) { cases := []struct { - input GroupAndVersion + input GroupVersion expect []byte }{ - {GroupAndVersion{"", "v1"}, []byte(`{"val":"v1"}`)}, - {GroupAndVersion{"extensions", "v1beta1"}, []byte(`{"val":"extensions/v1beta1"}`)}, + {GroupVersion{"", "v1"}, []byte(`{"val":"v1"}`)}, + {GroupVersion{"extensions", "v1beta1"}, []byte(`{"val":"extensions/v1beta1"}`)}, } for _, c := range cases { diff --git a/pkg/api/unversioned/types.go b/pkg/api/unversioned/types.go index c90bf296571..a650b95339d 100644 --- a/pkg/api/unversioned/types.go +++ b/pkg/api/unversioned/types.go @@ -299,15 +299,15 @@ type APIGroup struct { // name is the name of the group. Name string `json:"name"` // versions are the versions supported in this group. - Versions []GroupVersion `json:"versions"` + Versions []GroupVersionForDiscovery `json:"versions"` // preferredVersion is the version preferred by the API server, which // probably is the storage version. - PreferredVersion GroupVersion `json:"preferredVersion,omitempty"` + PreferredVersion GroupVersionForDiscovery `json:"preferredVersion,omitempty"` } // GroupVersion contains the "group/version" and "version" string of a version. // It is made a struct to keep extensiblity. -type GroupVersion struct { +type GroupVersionForDiscovery struct { // groupVersion specifies the API group and version in the form "group/version" GroupVersion string `json:"groupVersion"` // version specifies the version in the form of "version". This is to save diff --git a/pkg/client/unversioned/client_test.go b/pkg/client/unversioned/client_test.go index d9eefd53c0f..640a24d3a4b 100644 --- a/pkg/client/unversioned/client_test.go +++ b/pkg/client/unversioned/client_test.go @@ -400,7 +400,7 @@ func TestGetServerResources(t *testing.T) { list = &unversioned.APIGroupList{ Groups: []unversioned.APIGroup{ { - Versions: []unversioned.GroupVersion{ + Versions: []unversioned.GroupVersionForDiscovery{ {GroupVersion: "extensions/v1beta1"}, }, }, diff --git a/pkg/client/unversioned/discovery_client.go b/pkg/client/unversioned/discovery_client.go index cc3eb6665e3..a9c0ed263a6 100644 --- a/pkg/client/unversioned/discovery_client.go +++ b/pkg/client/unversioned/discovery_client.go @@ -55,9 +55,9 @@ type DiscoveryClient struct { // Convert unversioned.APIVersions to unversioned.APIGroup. APIVersions is used by legacy v1, so // group would be "". func apiVersionsToAPIGroup(apiVersions *unversioned.APIVersions) (apiGroup unversioned.APIGroup) { - groupVersions := []unversioned.GroupVersion{} + groupVersions := []unversioned.GroupVersionForDiscovery{} for _, version := range apiVersions.Versions { - groupVersion := unversioned.GroupVersion{ + groupVersion := unversioned.GroupVersionForDiscovery{ GroupVersion: version, Version: version, } diff --git a/pkg/client/unversioned/helper_test.go b/pkg/client/unversioned/helper_test.go index fe4365245d2..2960cf6d592 100644 --- a/pkg/client/unversioned/helper_test.go +++ b/pkg/client/unversioned/helper_test.go @@ -385,7 +385,7 @@ func TestHelperGetServerAPIVersions(t *testing.T) { APIGroupList := unversioned.APIGroupList{ Groups: []unversioned.APIGroup{ { - Versions: []unversioned.GroupVersion{ + Versions: []unversioned.GroupVersionForDiscovery{ { GroupVersion: "group1/v1", }, @@ -395,7 +395,7 @@ func TestHelperGetServerAPIVersions(t *testing.T) { }, }, { - Versions: []unversioned.GroupVersion{ + Versions: []unversioned.GroupVersionForDiscovery{ { GroupVersion: "group2/v1", }, diff --git a/pkg/master/master.go b/pkg/master/master.go index 46dbfa522d5..f8425272abd 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -665,7 +665,7 @@ func (m *Master) init(c *Config) { if err != nil { glog.Fatalf("Unable to setup experimental api: %v", err) } - expAPIVersions := []unversioned.GroupVersion{ + expAPIVersions := []unversioned.GroupVersionForDiscovery{ { GroupVersion: expVersion.Version, Version: apiutil.GetVersion(expVersion.Version), @@ -678,7 +678,7 @@ func (m *Master) init(c *Config) { group := unversioned.APIGroup{ Name: g.Group, Versions: expAPIVersions, - PreferredVersion: unversioned.GroupVersion{GroupVersion: storageVersion, Version: apiutil.GetVersion(storageVersion)}, + PreferredVersion: unversioned.GroupVersionForDiscovery{GroupVersion: storageVersion, Version: apiutil.GetVersion(storageVersion)}, } apiserver.AddGroupWebService(m.handlerContainer, c.APIGroupPrefix+"/"+latest.GroupOrDie("extensions").Group, group) allGroups = append(allGroups, group) @@ -992,13 +992,13 @@ func (m *Master) InstallThirdPartyResource(rsrc *expapi.ThirdPartyResource) erro glog.Fatalf("Unable to setup thirdparty api: %v", err) } path := makeThirdPartyPath(group) - groupVersion := unversioned.GroupVersion{ + groupVersion := unversioned.GroupVersionForDiscovery{ GroupVersion: group + "/" + rsrc.Versions[0].Name, Version: rsrc.Versions[0].Name, } apiGroup := unversioned.APIGroup{ Name: group, - Versions: []unversioned.GroupVersion{groupVersion}, + Versions: []unversioned.GroupVersionForDiscovery{groupVersion}, } apiserver.AddGroupWebService(m.handlerContainer, path, apiGroup) m.addThirdPartyResourceStorage(path, thirdparty.Storage[strings.ToLower(kind)+"s"].(*thirdpartyresourcedataetcd.REST)) diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go index 6e7d95eea42..618f493d62f 100644 --- a/pkg/master/master_test.go +++ b/pkg/master/master_test.go @@ -399,13 +399,13 @@ func TestDiscoveryAtAPIS(t *testing.T) { } expectGroupName := "extensions" - expectVersions := []unversioned.GroupVersion{ + expectVersions := []unversioned.GroupVersionForDiscovery{ { GroupVersion: testapi.Extensions.GroupAndVersion(), Version: testapi.Extensions.Version(), }, } - expectPreferredVersion := unversioned.GroupVersion{ + expectPreferredVersion := unversioned.GroupVersionForDiscovery{ GroupVersion: config.StorageVersions["extensions"], Version: apiutil.GetVersion(config.StorageVersions["extensions"]), } From 2f012ae0366aa876c21b32174e7b1b693f64bbff Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Thu, 5 Nov 2015 15:49:52 -0800 Subject: [PATCH 5/5] run gen-swagger-docs --- pkg/api/unversioned/types_swagger_doc_generated.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/api/unversioned/types_swagger_doc_generated.go b/pkg/api/unversioned/types_swagger_doc_generated.go index cf9ad3f41b2..25242a59449 100644 --- a/pkg/api/unversioned/types_swagger_doc_generated.go +++ b/pkg/api/unversioned/types_swagger_doc_generated.go @@ -76,14 +76,14 @@ func (APIVersions) SwaggerDoc() map[string]string { return map_APIVersions } -var map_GroupVersion = map[string]string{ +var map_GroupVersionForDiscovery = map[string]string{ "": "GroupVersion contains the \"group/version\" and \"version\" string of a version. It is made a struct to keep extensiblity.", "groupVersion": "groupVersion specifies the API group and version in the form \"group/version\"", "version": "version specifies the version in the form of \"version\". This is to save the clients the trouble of splitting the GroupVersion.", } -func (GroupVersion) SwaggerDoc() map[string]string { - return map_GroupVersion +func (GroupVersionForDiscovery) SwaggerDoc() map[string]string { + return map_GroupVersionForDiscovery } var map_ListMeta = map[string]string{