Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support "bytes" proto (v2/v3) type. #13

Merged
merged 3 commits into from Nov 27, 2018
Merged

Support "bytes" proto (v2/v3) type. #13

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c529597
Support "bytes" proto (v2/v3) type.
dilyevsky Nov 27, 2018
f927ff6
Move special case block into scalarFromStarlark. Update Gogo message …
dilyevsky Nov 27, 2018
250595c
Update README to include gogo msg generation command.
dilyevsky Nov 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@ github.com/gogo/protobuf v1.1.1 h1:72R+M5VuhED/KujmZVcIquuo8mBgX4oVda//DQb3PXo=
github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
github.com/golang/protobuf v1.2.0 h1:P3YflyNX/ehuJFLhxviNdFxQPkGK5cDcApsge1SqnvM=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/jmillikin-stripe/godebug v0.0.0-20180620173319-8279e1966bc1 h1:JgsVrDAUy59N248f3l4RGZ0hij5u1HTit8iJr1mFSBY=
github.com/jmillikin-stripe/godebug v0.0.0-20180620173319-8279e1966bc1/go.mod h1:gFqr/IKD8P+Hluq9gThCR944BAu6jUqd5H/R3PrPfuM=
github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 h1:MtvEpTB6LX3vkb4ax0b5D2DHbNAUsen0Gx5wZoq3lV4=
github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
Expand Down
24 changes: 19 additions & 5 deletions internal/go/skycfg/proto_message.go
View file
Open in desktop
Expand Up @@ -217,7 +217,8 @@ func valueToStarlark(val reflect.Value) starlark.Value {
if msg, ok := iface.(proto.Message); ok {
return NewSkyProtoMessage(msg)
}
if val.Type().Kind() == reflect.Struct {
t := val.Type()
if t.Kind() == reflect.Struct {
// Might have been generated by gogo-protobuf
//
// Need to check if this is a non-pointer map value, which
Expand All @@ -229,7 +230,11 @@ func valueToStarlark(val reflect.Value) starlark.Value {
}
}
}
if val.Type().Kind() == reflect.Slice {
// Handle []byte ([]uint8) -> string special case.
if t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Uint8 {
return starlark.String(string(val.Interface().([]byte)))
}
if t.Kind() == reflect.Slice {
var items []starlark.Value
for ii := 0; ii < val.Len(); ii++ {
items = append(items, valueToStarlark(val.Index(ii)))
Expand All @@ -239,7 +244,7 @@ func valueToStarlark(val reflect.Value) starlark.Value {
list: starlark.NewList(items),
}
}
if val.Type().Kind() == reflect.Map {
if t.Kind() == reflect.Map {
dict := &starlark.Dict{}
for _, keyVal := range val.MapKeys() {
elemVal := val.MapIndex(keyVal)
Expand Down Expand Up @@ -298,12 +303,21 @@ func scalarToStarlark(val reflect.Value) starlark.Value {
func valueFromStarlark(t reflect.Type, sky starlark.Value) (reflect.Value, error) {
switch sky := sky.(type) {
case starlark.Int, starlark.Float, starlark.String, starlark.Bool:
scalar, err := scalarFromStarlark(t, sky)
var scalar reflect.Value
var err error
// If dst is []byte ([]Uint8) and src is string, attempt to
// convert value below.
if t.Kind() == reflect.Slice && t.Elem().Kind() == reflect.Uint8 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic feels like it belongs in scalarFromStarlark(). Since the Go type is passed in already, you can special-case the conversion from string to []uint8 and return a reflected slice (which should be assignable).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

scalar, err = scalarFromStarlark(reflect.TypeOf(""), sky)
} else {
scalar, err = scalarFromStarlark(t, sky)
}
if err != nil {
return reflect.Value{}, err
}

// Handle the use of typedefs in Kubernetes
// Handle the use of typedefs in Kubernetes and "string" ->
// "bytes" conversion.
if scalarType := scalar.Type(); !scalarType.AssignableTo(t) {
if scalarType.Kind() != reflect.String || !scalarType.ConvertibleTo(t) {
return reflect.Value{}, typeError(t, sky)
Expand Down
7 changes: 7 additions & 0 deletions internal/go/skycfg/proto_test.go
View file
Open in desktop
Expand Up @@ -301,6 +301,7 @@ func TestMessageAttrNames(t *testing.T) {
"f_nested_enum",
"f_oneof_a",
"f_oneof_b",
"f_bytes",
}
sort.Strings(want)
if !reflect.DeepEqual(want, got) {
Expand Down Expand Up @@ -341,6 +342,7 @@ func TestMessageV2(t *testing.T) {
f_toplevel_enum = proto.package("skycfg.test_proto").ToplevelEnumV2.TOPLEVEL_ENUM_V2_B,
f_nested_enum = proto.package("skycfg.test_proto").MessageV2.NestedEnum.NESTED_ENUM_B,
f_oneof_a = "string in oneof",
f_bytes = "also some string",
)`)
gotMsg := val.(*skyProtoMessage).msg
wantMsg := &pb.MessageV2{
Expand Down Expand Up @@ -373,6 +375,7 @@ func TestMessageV2(t *testing.T) {
FToplevelEnum: pb.ToplevelEnumV2_TOPLEVEL_ENUM_V2_B.Enum(),
FNestedEnum: pb.MessageV2_NESTED_ENUM_B.Enum(),
FOneof: &pb.MessageV2_FOneofA{"string in oneof"},
FBytes: []byte("also some string"),
}
if diff := ProtoDiff(wantMsg, gotMsg); diff != "" {
t.Fatalf("diff from expected message:\n%s", diff)
Expand All @@ -397,6 +400,7 @@ func TestMessageV2(t *testing.T) {
"f_nested_enum": `<skycfg.test_proto.MessageV2.NestedEnum NESTED_ENUM_B=1>`,
"f_oneof_a": `"string in oneof"`,
"f_oneof_b": `None`,
"f_bytes": `"also some string"`,
}
attrs := val.(starlark.HasAttrs)
for attrName, wantAttr := range wantAttrs {
Expand Down Expand Up @@ -444,6 +448,7 @@ func TestMessageV3(t *testing.T) {
f_toplevel_enum = proto.package("skycfg.test_proto").ToplevelEnumV3.TOPLEVEL_ENUM_V3_B,
f_nested_enum = proto.package("skycfg.test_proto").MessageV3.NestedEnum.NESTED_ENUM_B,
f_oneof_a = "string in oneof",
f_bytes = "also some string",
)`)
gotMsg := val.(*skyProtoMessage).msg
wantMsg := &pb.MessageV3{
Expand Down Expand Up @@ -476,6 +481,7 @@ func TestMessageV3(t *testing.T) {
FToplevelEnum: pb.ToplevelEnumV3_TOPLEVEL_ENUM_V3_B,
FNestedEnum: pb.MessageV3_NESTED_ENUM_B,
FOneof: &pb.MessageV3_FOneofA{"string in oneof"},
FBytes: []byte("also some string"),
}
if diff := ProtoDiff(wantMsg, gotMsg); diff != "" {
t.Fatalf("diff from expected message:\n%s", diff)
Expand All @@ -500,6 +506,7 @@ func TestMessageV3(t *testing.T) {
"f_nested_enum": `<skycfg.test_proto.MessageV3.NestedEnum NESTED_ENUM_B=1>`,
"f_oneof_a": `"string in oneof"`,
"f_oneof_b": `None`,
"f_bytes": `"also some string"`,
}
attrs := val.(starlark.HasAttrs)
for attrName, wantAttr := range wantAttrs {
Expand Down
4 changes: 3 additions & 1 deletion testdata/test_proto_v2.proto
View file
Open in desktop
Expand Up @@ -38,7 +38,9 @@ message MessageV2 {
string f_oneof_b = 18;
}

// NEXT: 19
optional bytes f_bytes = 19;

// NEXT: 20
}

enum ToplevelEnumV2 {
Expand Down
4 changes: 3 additions & 1 deletion testdata/test_proto_v3.proto
View file
Open in desktop
Expand Up @@ -39,7 +39,9 @@ message MessageV3 {
string f_oneof_b = 18;
}

// NEXT: 19
bytes f_bytes = 19;

// NEXT: 20
}

enum ToplevelEnumV3 {
Expand Down