Merge f9291df into b909487

exampleocpath/update.sh

@@ -18,6 +18,7 @@ go run ../ypathgen/generator/generator.go -path=public,deps -output_file=ocpath.
   -prefer_operational_state=true \
   -exclude_modules=ietf-interfaces \
   -schema_struct_path=github.com/openconfig/ygot/exampleoc \
+  -list_builder_key_threshold=3 \
   public/release/models/network-instance/openconfig-network-instance.yang \
   public/release/models/optical-transport/openconfig-optical-amplifier.yang \
   public/release/models/optical-transport/openconfig-terminal-device.yang \

ygot/path_types.go

@@ -58,6 +58,10 @@ func ResolveRelPath(n PathStruct) ([]*gpb.PathElem, []error) {
 	return n.relPath()
 }
 
+func (n *NodePath) ModifyKey(name string, value interface{}) {
+	n.keys[name] = value
+}
+
 // relPath converts the information stored in NodePath into the partial
 // []*gpb.PathElem representing the node's relative path.
 func (n *NodePath) relPath() ([]*gpb.PathElem, []error) {

ypathgen/generator/generator.go

@@ -31,17 +31,18 @@ import (
 )
 
 var (
-	yangPaths              = flag.String("path", "", "Comma separated list of paths to be recursively searched for included modules or submodules within the defined YANG modules.")
-	excludeModules         = flag.String("exclude_modules", "", "Comma separated set of module names that should be excluded from code generation this can be used to ensure overlapping namespaces can be ignored.")
-	packageName            = flag.String("package_name", "telemetry", "The name of the Go package that should be generated.")
-	outputFile             = flag.String("output_file", "", "The single file that the Go package should be written to.")
-	ignoreCircDeps         = flag.Bool("ignore_circdeps", false, "If set to true, circular dependencies between submodules are ignored.")
-	preferOperationalState = flag.Bool("prefer_operational_state", false, "If set to true, state (config false) fields in the YANG schema are preferred over intended config leaves when building paths. This flag is only valid when exclude_state=false.")
-	fakeRootName           = flag.String("fakeroot_name", "device", "The name of the fake root entity. This name will be capitalized for exporting.")
-	schemaStructPkgAlias   = flag.String("schema_struct_pkg_alias", "", "The package alias of the schema struct package.")
-	schemaStructPath       = flag.String("schema_struct_path", "", "The import path to use for ygen-generated schema structs.")
-	gnmiProtoPath          = flag.String("gnmi_proto_path", genutil.GoDefaultGNMIImportPath, "The import path to use for gNMI's proto package.")
-	ygotImportPath         = flag.String("ygot_path", genutil.GoDefaultYgotImportPath, "The import path to use for ygot.")
+	yangPaths               = flag.String("path", "", "Comma separated list of paths to be recursively searched for included modules or submodules within the defined YANG modules.")
+	excludeModules          = flag.String("exclude_modules", "", "Comma separated set of module names that should be excluded from code generation this can be used to ensure overlapping namespaces can be ignored.")
+	packageName             = flag.String("package_name", "telemetry", "The name of the Go package that should be generated.")
+	outputFile              = flag.String("output_file", "", "The single file that the Go package should be written to.")
+	ignoreCircDeps          = flag.Bool("ignore_circdeps", false, "If set to true, circular dependencies between submodules are ignored.")
+	preferOperationalState  = flag.Bool("prefer_operational_state", false, "If set to true, state (config false) fields in the YANG schema are preferred over intended config leaves when building paths. This flag is only valid when exclude_state=false.")
+	fakeRootName            = flag.String("fakeroot_name", "device", "The name of the fake root entity. This name will be capitalized for exporting.")
+	schemaStructPkgAlias    = flag.String("schema_struct_pkg_alias", "", "The package alias of the schema struct package.")
+	schemaStructPath        = flag.String("schema_struct_path", "", "The import path to use for ygen-generated schema structs.")
+	gnmiProtoPath           = flag.String("gnmi_proto_path", genutil.GoDefaultGNMIImportPath, "The import path to use for gNMI's proto package.")
+	ygotImportPath          = flag.String("ygot_path", genutil.GoDefaultYgotImportPath, "The import path to use for ygot.")
+	listBuilderKeyThreshold = flag.Uint("list_builder_key_threshold", 0, "The threshold equal or over which the builder API is used for key population. 0 means infinity.")
 )
 
 // writeGoCodeSingleFile takes a ypathgen.GeneratedPathCode struct and writes
@@ -111,7 +112,8 @@ func main() {
 		YANGParseOptions: yang.Options{
 			IgnoreSubmoduleCircularDependencies: *ignoreCircDeps,
 		},
-		GeneratingBinary: genutil.CallerName(),
+		GeneratingBinary:        genutil.CallerName(),
+		ListBuilderKeyThreshold: *listBuilderKeyThreshold,
 	}
 
 	pathCode, _, errs := cg.GeneratePathCode(generateModules, includePaths)

ypathgen/path_tests/path_test.go

@@ -178,6 +178,48 @@ func TestPathCreation(t *testing.T) {
 			return root.NetworkInstance("RED").Mpls().SignalingProtocols().SegmentRouting().Interface("eth1").SidCounter(iNullInUnion).InOctets()
 		},
 		wantPath: "/network-instances/network-instance[name=RED]/mpls/signaling-protocols/segment-routing/interfaces/interface[interface-id=eth1]/sid-counters/sid-counter[mpls-label=IMPLICIT_NULL]/state/in-octets",
+	}, {
+		name: "Builder API constructor",
+		makePath: func(root *ocp.Root) ygot.PathStruct {
+			return root.Lldp().Interface("foo").Neighbor("mars").TlvBuilder()
+		},
+		wantPath: "/lldp/interfaces/interface[name=foo]/neighbors/neighbor[id=mars]/custom-tlvs/tlv[type=*][oui=*][oui-subtype=*]",
+	}, {
+		name: "Builder API constructor going beyond the list",
+		makePath: func(root *ocp.Root) ygot.PathStruct {
+			return root.Lldp().Interface("foo").Neighbor("mars").TlvBuilder().Value()
+		},
+		wantPath: "/lldp/interfaces/interface[name=foo]/neighbors/neighbor[id=mars]/custom-tlvs/tlv[type=*][oui=*][oui-subtype=*]/state/value",
+	}, {
+		name: "Builder API builder for type",
+		makePath: func(root *ocp.Root) ygot.PathStruct {
+			return root.Lldp().Interface("foo").Neighbor("mars").TlvBuilder().WithType(3)
+		},
+		wantPath: "/lldp/interfaces/interface[name=foo]/neighbors/neighbor[id=mars]/custom-tlvs/tlv[type=3][oui=*][oui-subtype=*]",
+	}, {
+		name: "Builder API builder for oui",
+		makePath: func(root *ocp.Root) ygot.PathStruct {
+			return root.Lldp().Interface("foo").Neighbor("mars").TlvBuilder().WithOui("bar")
+		},
+		wantPath: "/lldp/interfaces/interface[name=foo]/neighbors/neighbor[id=mars]/custom-tlvs/tlv[type=*][oui=bar][oui-subtype=*]",
+	}, {
+		name: "Builder API builder for subtype",
+		makePath: func(root *ocp.Root) ygot.PathStruct {
+			return root.Lldp().Interface("foo").Neighbor("mars").TlvBuilder().WithOuiSubtype("baz").Value()
+		},
+		wantPath: "/lldp/interfaces/interface[name=foo]/neighbors/neighbor[id=mars]/custom-tlvs/tlv[type=*][oui=*][oui-subtype=baz]/state/value",
+	}, {
+		name: "Builder API builder for type and oui",
+		makePath: func(root *ocp.Root) ygot.PathStruct {
+			return root.Lldp().Interface("foo").Neighbor("mars").TlvBuilder().WithType(3).WithOui("bar")
+		},
+		wantPath: "/lldp/interfaces/interface[name=foo]/neighbors/neighbor[id=mars]/custom-tlvs/tlv[type=3][oui=bar][oui-subtype=*]",
+	}, {
+		name: "Builder API builder for type and oui and oui-subtype",
+		makePath: func(root *ocp.Root) ygot.PathStruct {
+			return root.Lldp().Interface("foo").Neighbor("mars").TlvBuilder().WithOui("bar").WithType(3).WithOuiSubtype("baz")
+		},
+		wantPath: "/lldp/interfaces/interface[name=foo]/neighbors/neighbor[id=mars]/custom-tlvs/tlv[type=3][oui=bar][oui-subtype=baz]",
 	}}
 
 	for _, tt := range tests {

ypathgen/pathgen.go

@@ -48,6 +48,14 @@ const (
 	// node as well as a list's wildcard child constructor methods that
 	// distinguishes each from its non-wildcard counterpart.
 	WildcardSuffix = "Any"
+	// BuilderCtorSuffix is the suffix applied to the builder constructor
+	// method's name in order to indicate itself to the user.
+	BuilderCtorSuffix = "Builder"
+	// BuilderKeyPrefix is the prefix applied to the key-modifying builder
+	// method for a list PathStruct that uses the builder API.
+	// NOTE: This cannot be "", as the builder method name would conflict
+	// with the child constructor method for the keys.
+	BuilderKeyPrefix = "With"
 )
 
 // NewDefaultConfig creates a GenConfig with default configuration. schemaStructPkgPath is a
@@ -97,6 +105,12 @@ type GenConfig struct {
 	// included in the header of output files for debugging purposes. If a
 	// string is not specified, the location of the library is utilised.
 	GeneratingBinary string
+	// ListBuilderKeyThreshold means to use the builder API format instead
+	// of the key-combination API format for constructing list keys when
+	// the number of keys is at least the threshold value.
+	// 0 (default) means no threshold, i.e. always use the key-combination
+	// API format.
+	ListBuilderKeyThreshold uint
 }
 
 // GoImports contains package import options.
@@ -181,7 +195,7 @@ func (cg *GenConfig) GeneratePathCode(yangFiles, includePaths []string) (*Genera
 				util.NewErrs(fmt.Errorf("GeneratePathCode: Implementation bug -- node %s not found in dirNameMap", directoryName)))
 		}
 
-		structSnippet, es := generateDirectorySnippet(directory, directories, cg.SchemaStructPkgAlias)
+		structSnippet, es := generateDirectorySnippet(directory, directories, cg.SchemaStructPkgAlias, cg.ListBuilderKeyThreshold)
 		if es != nil {
 			errs = util.AppendErrs(errs, es)
 		}
@@ -431,6 +445,14 @@ func (n *{{ .Struct.TypeName }}) {{ .MethodName -}} ({{ .KeyParamListStr }}) *{{
 		),
 	}
 }
+`)
+
+	goKeyBuilderTemplate = mustTemplate("goKeyBuilder", `
+// {{ .MethodName }} sets {{ .TypeName }}'s key "{{ .KeySchemaName }}" to the specified value.
+func (n *{{ .TypeName }}) {{ .MethodName }}({{ .KeyParamName }} {{ .KeyParamType }}) *{{ .TypeName }} {
+    n.ModifyKey("{{ .KeySchemaName }}", {{ .KeyParamName }})
+    return n
+}
 `)
 )
 
@@ -597,7 +619,7 @@ type goPathFieldData struct {
 // the fields of the struct. directory is the parsed information of a schema
 // node, and directories is a map from path to a parsed schema node for all
 // nodes in the schema.
-func generateDirectorySnippet(directory *ygen.Directory, directories map[string]*ygen.Directory, schemaStructPkgAlias string) (GoPathStructCodeSnippet, util.Errors) {
+func generateDirectorySnippet(directory *ygen.Directory, directories map[string]*ygen.Directory, schemaStructPkgAlias string, listBuilderKeyThreshold uint) (GoPathStructCodeSnippet, util.Errors) {
 	var errs util.Errors
 	// structBuf is used to store the code associated with the struct defined for
 	// the target YANG entity.
@@ -627,7 +649,7 @@ func generateDirectorySnippet(directory *ygen.Directory, directories map[string]
 		}
 		goFieldName := goFieldNameMap[fieldName]
 
-		if es := generateChildConstructors(&methodBuf, directory, fieldName, goFieldName, directories, schemaStructPkgAlias); es != nil {
+		if es := generateChildConstructors(&methodBuf, directory, fieldName, goFieldName, directories, schemaStructPkgAlias, listBuilderKeyThreshold); es != nil {
 			errs = util.AppendErrs(errs, es)
 		}
 
@@ -670,7 +692,7 @@ func generateDirectorySnippet(directory *ygen.Directory, directories map[string]
 // field name to be used as the generated method's name and the incremental
 // type name of of the child path struct, and a map of all directories of the
 // whole schema keyed by their schema paths.
-func generateChildConstructors(methodBuf *strings.Builder, directory *ygen.Directory, directoryFieldName string, goFieldName string, directories map[string]*ygen.Directory, schemaStructPkgAlias string) []error {
+func generateChildConstructors(methodBuf *strings.Builder, directory *ygen.Directory, directoryFieldName string, goFieldName string, directories map[string]*ygen.Directory, schemaStructPkgAlias string, listBuilderKeyThreshold uint) []error {
 	field, ok := directory.Fields[directoryFieldName]
 	if !ok {
 		return []error{fmt.Errorf("generateChildConstructors: field %s not found in directory %v", directoryFieldName, directory)}
@@ -698,22 +720,26 @@ func generateChildConstructors(methodBuf *strings.Builder, directory *ygen.Direc
 	// This is expected to be nil for leaf fields.
 	fieldDirectory := directories[field.Path()]
 
-	if field.IsList() { // else, the field is a container or leaf.
-		if fieldDirectory.ListAttr == nil {
-			// TODO(wenbli): keyless lists as a path are not supported by gNMI, but this
-			// library is currently intended for gNMI, so need to decide on a long-term solution.
-
-			// As a short-term solution, we just need to prevent the user from accessing any node in the keyless list's subtree.
-			// Here, we simply skip generating the child constructor, such that its subtree is unreachable.
-			return nil
-			// Erroring out, on the other hand, is impractical due to their existence in the current OpenConfig models.
-			// return fmt.Errorf("generateChildConstructors: schemas containing keyless lists are unsupported, path: %s", field.Path())
-		}
-
+	switch {
+	case !field.IsList():
+		return generateChildConstructorsForLeafOrContainer(methodBuf, fieldData, isUnderFakeRoot)
+	case fieldDirectory.ListAttr == nil:
+		// TODO(wenbli): keyless lists as a path are not supported by gNMI, but this
+		// library is currently intended for gNMI, so need to decide on a long-term solution.
+
+		// As a short-term solution, we just need to prevent the user from accessing any node in the keyless list's subtree.
+		// Here, we simply skip generating the child constructor, such that its subtree is unreachable.
+		return nil
+		// Erroring out, on the other hand, is impractical due to their existence in the current OpenConfig models.
+		// return fmt.Errorf("generateChildConstructors: schemas containing keyless lists are unsupported, path: %s", field.Path())
+	case listBuilderKeyThreshold != 0 && uint(len(fieldDirectory.ListAttr.KeyElems)) >= listBuilderKeyThreshold:
+		// If the number of keys is equal to or over the builder API threshold,
+		// then use the builder API format to make the list path API less
+		// confusing for the user.
+		return generateChildConstructorsForListBuilderFormat(methodBuf, fieldDirectory.ListAttr, fieldData, isUnderFakeRoot, schemaStructPkgAlias)
+	default:
 		return generateChildConstructorsForList(methodBuf, fieldDirectory.ListAttr, fieldData, isUnderFakeRoot, schemaStructPkgAlias)
 	}
-
-	return generateChildConstructorsForLeafOrContainer(methodBuf, fieldData, isUnderFakeRoot)
 }
 
 // generateChildConstructorsForLeafOrContainer writes into methodBuf the child
@@ -740,6 +766,70 @@ func generateChildConstructorsForLeafOrContainer(methodBuf *strings.Builder, fie
 	return errors
 }
 
+// generateChildConstructorsForListBuilderFormat writes into methodBuf the
+// child constructor method snippets for the list represented by listAttr using
+// the builder API format. fieldData contains the childConstructor template
+// output information for if the node were a container (which contains a subset
+// of the basic information required for the list constructor methods).
+func generateChildConstructorsForListBuilderFormat(methodBuf *strings.Builder, listAttr *ygen.YangListAttr, fieldData goPathFieldData, isUnderFakeRoot bool, schemaStructPkgAlias string) []error {
+	var errors []error
+	// List of function parameters as would appear in the method definition.
+	keyParams, err := makeKeyParams(listAttr, schemaStructPkgAlias)
+	if err != nil {
+		return append(errors, err)
+	}
+	keyN := len(keyParams)
+
+	var keyEntryStrs []string
+	for i := 0; i != keyN; i++ {
+		keyEntryStrs = append(keyEntryStrs, fmt.Sprintf(`"%s": "*"`, keyParams[i].name))
+	}
+	// Create the string for the method parameter list and ygot.NodePath's key list.
+	fieldData.KeyParamListStr = ""
+	fieldData.KeyEntriesStr = strings.Join(keyEntryStrs, ", ")
+
+	// Set the child type to be the wildcard version.
+	fieldData.TypeName += WildcardSuffix
+
+	// Since all keys are wildcarded, just use BuilderCtorSuffix alone as the suffix.
+	fieldData.MethodName += BuilderCtorSuffix
+
+	// Generate child constructor method for non-wildcard version of parent struct.
+	if err := goPathChildConstructorTemplate.Execute(methodBuf, fieldData); err != nil {
+		errors = append(errors, err)
+	}
+
+	// The root node doesn't have a wildcard version of itself.
+	if !isUnderFakeRoot {
+		// Generate child constructor method for wildcard version of parent struct.
+		fieldData.Struct.TypeName += WildcardSuffix
+		if err := goPathChildConstructorTemplate.Execute(methodBuf, fieldData); err != nil {
+			return append(errors, err)
+		}
+	}
+
+	for i := 0; i != keyN; i++ {
+		builder := struct {
+			MethodName    string
+			TypeName      string
+			KeySchemaName string
+			KeyParamType  string
+			KeyParamName  string
+		}{
+			MethodName:    BuilderKeyPrefix + keyParams[i].varName,
+			TypeName:      fieldData.TypeName,
+			KeySchemaName: keyParams[i].name,
+			KeyParamName:  keyParams[i].varName,
+			KeyParamType:  keyParams[i].typeName,
+		}
+		if err := goKeyBuilderTemplate.Execute(methodBuf, builder); err != nil {
+			return append(errors, err)
+		}
+	}
+
+	return errors
+}
+
 // generateChildConstructorsForList writes into methodBuf the child constructor
 // method snippets for the list represented by listAttr. fieldData contains the
 // childConstructor template output information for if the node were a