Skip to content

Commit

Permalink
Add support for mapping leaf-lists to paths. (#756)
Browse files Browse the repository at this point in the history
* Add annotations to protobuf oneof fields.

 * (M) protogen/*
   - Annotate a field within a oneof with the schema paths.

* Add support for mapping `oneof` fields to paths.

 * (M) protogen.*
  - Add annotations for a field to every subfield of a oneof such
    that handling these fields is as per any other message field.
 * (M) protomap/integration_tests/*
  - Validate against generated gRIBI messages.
 * (M) protomap/*
  - Add support for mapping oneof fields and test coverage.

* Ensure that path annotations are created for leaf-list of unions.

 * (M) protogen/protogen(_test)?.go
  - Add support for adding schema path annotations for unions that
    are within leaf-lists. This is represented as a repeated
    message rather than a single oneof.
 * (M) protogen/testdata/*
  - Update testing data.

* Working commit.

* Add generated protobuf examples.

* Note that panic should be avoided.

* Add support for mapping leaf-list and unions to paths.

* Fix integration test.

* Fix defect with multiple leaf-list of union.

* Address review comments.

* Push file with error strings corrected.
  • Loading branch information
robshakir authored Jan 10, 2023
1 parent 6284b6a commit f58f5df
Show file tree
Hide file tree
Showing 20 changed files with 4,088 additions and 345 deletions.
70 changes: 54 additions & 16 deletions proto/yext/yext.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions proto/yext/yext.proto
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ extend google.protobuf.FieldOptions {
// parent of the entity). The field number for this extension is reserved
// in the global protobuf registry.
string schemapath = 1040;
// leaflist indicates that the field represents a YANG leaf-list. It is
// valid only for repeated fields, and can be used to determine that the
// generated field is a list of scalar values by any parsing code.
bool leaflist = 1041;
// leaflistunion indicates that hte field represents a YANG leaf-list
// of union types. It is valid only for repeated message fields and can be
// used to determine that specific leaf-list union handling is required.
bool leaflistunion = 1042;
}

extend google.protobuf.EnumValueOptions {
Expand Down
19 changes: 19 additions & 0 deletions protogen/protogen.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ const (
// genutil.MakeNameUnique which would append "_" to the name of the key we explicitly
// append _ plus the string defined in protoMatchingListNameKeySuffix to the list name.
protoMatchingListNameKeySuffix = "key"
// protoLeafListAnnotationOption specifies the name of the FieldOption used to annotate
// whether repeated fields are leaf-lists.
protoLeafListAnnotationOption = "(yext.leaflist)"
// protoLeafListUnionAnnotationOption specifies the name of the FieldOption used to annotate
// whether repeated fields are leaf-lists of unions.
protoLeafListUnionAnnotationOption = "(yext.leaflistunion)"
)

// protoMsgField describes a field of a protobuf message.
Expand Down Expand Up @@ -785,6 +791,19 @@ func addProtoLeafOrLeafListField(fieldDef *protoMsgField, msgDef *protoMsg, args

if args.field.Type == ygen.LeafListNode {
fieldDef.IsRepeated = true
switch d.repeatedMsg {
case nil:
fieldDef.Options = append(fieldDef.Options, &protoOption{
Name: protoLeafListAnnotationOption,
Value: "true",
})
default:
fieldDef.Options = append(fieldDef.Options, &protoOption{
Name: protoLeafListUnionAnnotationOption,
Value: "true",
})
}

}
return repeatedMsg, imports, nil
}
Expand Down
20 changes: 16 additions & 4 deletions protogen/protogen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ func TestGenProto3Msg(t *testing.T) {
Name: "field_two",
Type: "FieldTwoUnion",
IsRepeated: true,
Options: []*protoOption{{
Name: "(yext.leaflistunion)",
Value: "true",
}},
}},
},
"FieldTwoUnion": {
Expand Down Expand Up @@ -381,6 +385,10 @@ func TestGenProto3Msg(t *testing.T) {
Name: "leaf_list",
Type: "ywrapper.StringValue",
IsRepeated: true,
Options: []*protoOption{{
Name: "(yext.leaflist)",
Value: "true",
}},
}, {
Tag: 17594927,
Name: "container_child",
Expand Down Expand Up @@ -434,14 +442,18 @@ func TestGenProto3Msg(t *testing.T) {
Name: "AMessage",
YANGPath: "/root/a-message",
Fields: []*protoMsgField{{
Tag: 17594927,
Name: "container_child",
Type: "root.a_message.ContainerChild",
}, {
Tag: 299656613,
Name: "leaf_list",
Type: "ywrapper.StringValue",
IsRepeated: true,
}, {
Tag: 17594927,
Name: "container_child",
Type: "root.a_message.ContainerChild",
Options: []*protoOption{{
Name: "(yext.leaflist)",
Value: "true",
}},
}},
Imports: []string{"base/root/a_message/a_message.proto"},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ message TopLevel {
uint64 u_uint64 = 423874955 [(yext.schemapath) = "/top-level/idrefsc/idref/config/u"];
}
ywrapper.StringValue l = 372609659 [(yext.schemapath) = "/top-level/idrefsc/idref/config/l"];
repeated UUnion u = 372609634 [(yext.schemapath) = "/top-level/idrefsc/idref/config/u"];
repeated UUnion u = 372609634 [(yext.leaflistunion) = true,(yext.schemapath) = "/top-level/idrefsc/idref/config/u"];
}
message IdrefKey {
openconfig.enums.NestedMessagesKEY i = 1 [(yext.schemapath) = "/top-level/idrefsc/idref/config/i|/top-level/idrefsc/idref/i"];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ message TopLevel {
}
openconfig.enums.NestedMessagesKEY i = 372609662 [(yext.schemapath) = "/top-level/idrefsc/idref/config/i"];
ywrapper.StringValue l = 372609659 [(yext.schemapath) = "/top-level/idrefsc/idref/config/l"];
repeated UUnion u = 372609634 [(yext.schemapath) = "/top-level/idrefsc/idref/config/u"];
repeated UUnion u = 372609634 [(yext.leaflistunion) = true,(yext.schemapath) = "/top-level/idrefsc/idref/config/u"];
}
message State {
message UUnion {
Expand All @@ -88,7 +88,7 @@ message TopLevel {
}
openconfig.enums.NestedMessagesKEY i = 500927979 [(yext.schemapath) = "/top-level/idrefsc/idref/state/i"];
ywrapper.StringValue l = 500927982 [(yext.schemapath) = "/top-level/idrefsc/idref/state/l"];
repeated UUnion u = 500927991 [(yext.schemapath) = "/top-level/idrefsc/idref/state/u"];
repeated UUnion u = 500927991 [(yext.leaflistunion) = true,(yext.schemapath) = "/top-level/idrefsc/idref/state/u"];
}
Config config = 135893622 [(yext.schemapath) = "/top-level/idrefsc/idref/config"];
State state = 419960739 [(yext.schemapath) = "/top-level/idrefsc/idref/state"];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import "github.com/openconfig/ygot/proto/ywrapper/ywrapper.proto";
message Child {
ywrapper.BoolValue boolean = 135159880;
ywrapper.IntValue integer = 367917455;
repeated ywrapper.StringValue leaf_list = 370551192;
repeated ywrapper.StringValue leaf_list = 370551192 [(yext.leaflist) = true];
ywrapper.StringValue leaf_with_dashes = 503746721;
ywrapper.StringValue string = 486500768;
ywrapper.UintValue uinteger = 343208358;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import "github.com/openconfig/ygot/proto/ywrapper/ywrapper.proto";
// Config represents the /proto-test-a/parent/child/config YANG schema element.
message Config {
ywrapper.IntValue integer = 367917455;
repeated ywrapper.StringValue leaf_list = 370551192;
repeated ywrapper.StringValue leaf_list = 370551192 [(yext.leaflist) = true];
ywrapper.StringValue leaf_with_dashes = 503746721;
ywrapper.StringValue string = 486500768;
ywrapper.UintValue uinteger = 343208358;
Expand All @@ -26,7 +26,7 @@ message Config {
message State {
ywrapper.BoolValue boolean = 135159880;
ywrapper.IntValue integer = 486380674;
repeated ywrapper.StringValue leaf_list = 256667601;
repeated ywrapper.StringValue leaf_list = 256667601 [(yext.leaflist) = true];
ywrapper.StringValue leaf_with_dashes = 475722830;
ywrapper.StringValue string = 428609663;
ywrapper.UintValue uinteger = 343366297;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ message Config {
}
EnumeratedLeaf enumerated_leaf = 10800442;
EnumeratedWithDefault enumerated_with_default = 83098118;
repeated EnumeratedWithDefaultListMultipleDefault enumerated_with_default_list_multiple_default = 63055264;
repeated EnumeratedWithDefaultListSingleDefault enumerated_with_default_list_single_default = 465479240;
repeated openconfig.enums.ProtoTestCEnumWithDefault enumerated_with_default_list_single_default_at_type = 75892847;
repeated EnumeratedWithDefaultListMultipleDefault enumerated_with_default_list_multiple_default = 63055264 [(yext.leaflist) = true];
repeated EnumeratedWithDefaultListSingleDefault enumerated_with_default_list_single_default = 465479240 [(yext.leaflist) = true];
repeated openconfig.enums.ProtoTestCEnumWithDefault enumerated_with_default_list_single_default_at_type = 75892847 [(yext.leaflist) = true];
}

// State represents the /proto-test-c/entity/state YANG schema element.
Expand All @@ -62,7 +62,7 @@ message State {
}
EnumeratedLeaf enumerated_leaf = 247899547;
EnumeratedWithDefault enumerated_with_default = 82519423;
repeated EnumeratedWithDefaultListMultipleDefault enumerated_with_default_list_multiple_default = 195710037;
repeated EnumeratedWithDefaultListSingleDefault enumerated_with_default_list_single_default = 336052385;
repeated openconfig.enums.ProtoTestCEnumWithDefault enumerated_with_default_list_single_default_at_type = 52286674;
repeated EnumeratedWithDefaultListMultipleDefault enumerated_with_default_list_multiple_default = 195710037 [(yext.leaflist) = true];
repeated EnumeratedWithDefaultListSingleDefault enumerated_with_default_list_single_default = 336052385 [(yext.leaflist) = true];
repeated openconfig.enums.ProtoTestCEnumWithDefault enumerated_with_default_list_single_default_at_type = 52286674 [(yext.leaflist) = true];
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ import "openconfig/enums/enums.proto";
// Config represents the /proto-test-d/test/config YANG schema element.
message Config {
ywrapper.StringValue bar = 88698462;
repeated ywrapper.StringValue foo = 188685517;
repeated ywrapper.StringValue foo = 188685517 [(yext.leaflist) = true];
openconfig.enums.ProtoTestDFOO id = 411659456;
openconfig.enums.ProtoTestDATypedef ty = 495547642;
}

// State represents the /proto-test-d/test/state YANG schema element.
message State {
ywrapper.StringValue bar = 153828379;
repeated ywrapper.StringValue foo = 321003268;
repeated ywrapper.StringValue foo = 321003268 [(yext.leaflist) = true];
openconfig.enums.ProtoTestDFOO id = 490046099;
openconfig.enums.ProtoTestDATypedef ty = 3508169;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ message LluUnion {
// Bar represents the /proto-test-e/bars/bar YANG schema element.
message Bar {
ywrapper.StringValue foo = 91327513;
repeated LluUnion llu = 139983164;
repeated LluUnion llu = 139983164 [(yext.leaflistunion) = true];
ywrapper.StringValue single_type_union = 186685410;
}
21 changes: 20 additions & 1 deletion protomap/integration_tests/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/openconfig/gribi/v1/proto/gribi_aft"
"github.com/openconfig/ygot/protomap"
"github.com/openconfig/ygot/protomap/integration_tests/testdata/gribi_aft"
"github.com/openconfig/ygot/testutil"
"github.com/openconfig/ygot/ygot"
"google.golang.org/protobuf/proto"
Expand Down Expand Up @@ -70,6 +70,25 @@ func TestGRIBIAFT(t *testing.T) {
mustPath("afts/mpls/label-entry[label=32]/state/label"): uint64(32),
mustPath("afts/mpls/label-entry[label=32]/label"): uint64(32),
},
}, {
desc: "NH entry with label stack",
inProto: &gribi_aft.Afts{
NextHop: []*gribi_aft.Afts_NextHopKey{{
Index: 1,
NextHop: &gribi_aft.Afts_NextHop{
PushedMplsLabelStack: []*gribi_aft.Afts_NextHop_PushedMplsLabelStackUnion{{
PushedMplsLabelStackUint64: 42,
}, {
PushedMplsLabelStackUint64: 84,
}},
},
}},
},
wantPaths: map[*gpb.Path]interface{}{
mustPath("afts/next-hops/next-hop[index=1]/state/pushed-mpls-label-stack"): []interface{}{uint64(42), uint64(84)},
mustPath("afts/next-hops/next-hop[index=1]/index"): uint64(1),
mustPath("afts/next-hops/next-hop[index=1]/state/index"): uint64(1),
},
}}

for _, tt := range tests {
Expand Down
Loading

0 comments on commit f58f5df

Please sign in to comment.