Skip to content

Commit

Permalink
Use correct schema entry for determining package name, and minor refa…
Browse files Browse the repository at this point in the history 
…ctor of getNodeDataMap. (#594)

Previously it was using the parent's directory. Now changed to use the child's schema to generate the package name.

Also moved fakeroot's `NodeData` generation within `getNodeDataMap` itself.
  • Loading branch information
@wenovus
wenovus authored Oct 8, 2021
1 parent a776802 commit 26a85b4
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 36 deletions.
55 changes: 27 additions & 28 deletions ypathgen/pathgen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func (cg *GenConfig) GeneratePathCode(yangFiles, includePaths []string) (map[str
}

// Get NodeDataMap for the schema.
nodeDataMap, es := getNodeDataMap(directories, leafTypeMap, schemaStructPkgAccessor, cg.PathStructSuffix, cg.PackageName, cg.SplitByModule, cg.TrimOCPackage)
nodeDataMap, es := getNodeDataMap(directories, leafTypeMap, cg.FakeRootName, schemaStructPkgAccessor, cg.PathStructSuffix, cg.PackageName, cg.SplitByModule, cg.TrimOCPackage)
if es != nil {
errs = util.AppendErrs(errs, es)
}
Expand All @@ -275,23 +275,6 @@ func (cg *GenConfig) GeneratePathCode(yangFiles, includePaths []string) (map[str
util.NewErrs(fmt.Errorf("GeneratePathCode: Implementation bug -- node %s not found in dirNameMap", directoryName)))
}

if ygen.IsFakeRoot(directory.Entry) {
// Since we always generate the fake root, we add the
// fake root GoStruct to the data map as well.
nodeDataMap[directory.Name+cg.PathStructSuffix] = &NodeData{
GoTypeName: "*" + schemaStructPkgAccessor + yang.CamelCase(cg.FakeRootName),
LocalGoTypeName: "*" + yang.CamelCase(cg.FakeRootName),
GoFieldName: "",
SubsumingGoStructName: yang.CamelCase(cg.FakeRootName),
IsLeaf: false,
IsScalarField: false,
HasDefault: false,
YANGTypeName: "",
YANGPath: "/",
GoPathPackageName: goPackageName(directory, cg.SplitByModule, cg.TrimOCPackage, cg.PackageName),
}
}

var listBuilderKeyThreshold uint
if cg.GenerateWildcardPaths {
listBuilderKeyThreshold = cg.ListBuilderKeyThreshold
Expand Down Expand Up @@ -332,16 +315,16 @@ func (cg *GenConfig) GeneratePathCode(yangFiles, includePaths []string) (map[str
// packageNameReplacePattern matches all characters allowed in yang modules, but not go packages.
var packageNameReplacePattern = regexp.MustCompile("[._-]")

// goPackageName returns the go package to use when generating code for the input Directory.
// goPackageName returns the go package to use when generating code for the input schema Entry.
// If splitByModule is false, the pkgName is always returned. Otherwise,
// a transformed version of the module that the directory belongs to is returned.
// If trimOCPkg is true, "openconfig-" is remove from the package name.
// fakeRootPkgName is the name of the package that contains just the fake root path struct.
func goPackageName(dir *ygen.Directory, splitByModule, trimOCPkg bool, pkgName string) string {
if !splitByModule || ygen.IsFakeRoot(dir.Entry) {
func goPackageName(entry *yang.Entry, splitByModule, trimOCPkg bool, pkgName string) string {
if !splitByModule || ygen.IsFakeRoot(entry) {
return pkgName
}
name := util.SchemaTreeRoot(dir.Entry).Name
name := util.SchemaTreeRoot(entry).Name
if trimOCPkg {
name = strings.TrimPrefix(name, "openconfig-")
}
Expand Down Expand Up @@ -622,11 +605,27 @@ func mustTemplate(name, src string) *template.Template {
// packageName, splitByModule, and trimOCPackage are used to determine
// the generated Go package name for the generated PathStructs.
// If a directory or field doesn't exist in the leafTypeMap, then an error is returned.
// Note: Top-level nodes, but *not* the fake root, are part of the output.
func getNodeDataMap(directories map[string]*ygen.Directory, leafTypeMap map[string]map[string]*ygen.MappedType, schemaStructPkgAccessor, pathStructSuffix, packageName string, splitByModule, trimOCPackage bool) (NodeDataMap, util.Errors) {
func getNodeDataMap(directories map[string]*ygen.Directory, leafTypeMap map[string]map[string]*ygen.MappedType, fakeRootName, schemaStructPkgAccessor, pathStructSuffix, packageName string, splitByModule, trimOCPackage bool) (NodeDataMap, util.Errors) {
nodeDataMap := NodeDataMap{}
var errs util.Errors
for path, dir := range directories {
if ygen.IsFakeRoot(dir.Entry) {
// Since we always generate the fake root, we add the
// fake root GoStruct to the data map as well.
nodeDataMap[dir.Name+pathStructSuffix] = &NodeData{
GoTypeName: "*" + schemaStructPkgAccessor + yang.CamelCase(fakeRootName),
LocalGoTypeName: "*" + yang.CamelCase(fakeRootName),
GoFieldName: "",
SubsumingGoStructName: yang.CamelCase(fakeRootName),
IsLeaf: false,
IsScalarField: false,
HasDefault: false,
YANGTypeName: "",
YANGPath: "/",
GoPathPackageName: goPackageName(dir.Entry, splitByModule, trimOCPackage, packageName),
}
}

goFieldNameMap := ygen.GoFieldNameMap(dir)
fieldTypeMap, ok := leafTypeMap[path]
if !ok {
Expand Down Expand Up @@ -686,7 +685,7 @@ func getNodeDataMap(directories map[string]*ygen.Directory, leafTypeMap map[stri
HasDefault: isLeaf && (field.Default != "" || mType.DefaultValue != nil),
YANGTypeName: yangTypeName,
YANGPath: field.Path(),
GoPathPackageName: goPackageName(dir, splitByModule, trimOCPackage, packageName),
GoPathPackageName: goPackageName(field, splitByModule, trimOCPackage, packageName),
}
}
}
Expand Down Expand Up @@ -843,8 +842,8 @@ func generateDirectorySnippet(directory *ygen.Directory, directories map[string]
// If it is, add that package as a dependency and set the accessor.
if ygen.IsFakeRoot(directory.Entry) {
if fieldDirectory := directories[field.Path()]; fieldDirectory != nil {
parentPackge := goPackageName(directory, splitByModule, trimOCPkg, pkgName)
childPackage := goPackageName(fieldDirectory, splitByModule, trimOCPkg, pkgName)
parentPackge := goPackageName(directory.Entry, splitByModule, trimOCPkg, pkgName)
childPackage := goPackageName(field, splitByModule, trimOCPkg, pkgName)
if parentPackge != childPackage {
deps[childPackage] = true
childPkgAccessor = childPackage + "."
Expand Down Expand Up @@ -892,7 +891,7 @@ func generateDirectorySnippet(directory *ygen.Directory, directories map[string]
PathStructName: structData.TypeName,
StructBase: structBuf.String(),
ChildConstructors: methodBuf.String(),
Package: goPackageName(directory, splitByModule, trimOCPkg, pkgName),
Package: goPackageName(directory.Entry, splitByModule, trimOCPkg, pkgName),
}
for dep := range deps {
snippet.Deps = append(snippet.Deps, dep)
Expand Down
42 changes: 34 additions & 8 deletions ypathgen/pathgen_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1664,6 +1664,7 @@ func TestGetNodeDataMap(t *testing.T) {
name string
inDirectories map[string]*ygen.Directory
inLeafTypeMap map[string]map[string]*ygen.MappedType
inFakeRootName string
inSchemaStructPkgAccessor string
inPathStructSuffix string
inPackageName string
Expand All @@ -1679,6 +1680,7 @@ func TestGetNodeDataMap(t *testing.T) {
"leaf": leafTypeMap["/root-module/container"]["leaf"],
},
},
inFakeRootName: "device",
inSchemaStructPkgAccessor: "struct.",
inPathStructSuffix: "Path",
wantNodeDataMap: NodeDataMap{
Expand All @@ -1698,6 +1700,7 @@ func TestGetNodeDataMap(t *testing.T) {
name: "non-leaf and non-scalar leaf",
inDirectories: directoryWithBinaryLeaf,
inLeafTypeMap: leafTypeMap2,
inFakeRootName: "device",
inSchemaStructPkgAccessor: "struct.",
inPathStructSuffix: "_Path",
wantNodeDataMap: NodeDataMap{
Expand All @@ -1719,8 +1722,17 @@ func TestGetNodeDataMap(t *testing.T) {
IsScalarField: false,
HasDefault: false,
},
"Root_Path": {
GoTypeName: "*struct.Device",
LocalGoTypeName: "*Device",
GoFieldName: "",
SubsumingGoStructName: "Device",
IsLeaf: false,
IsScalarField: false,
HasDefault: false,
},
},
wantSorted: []string{"Container_Leaf_Path", "Container_Path"},
wantSorted: []string{"Container_Leaf_Path", "Container_Path", "Root_Path"},
}, {
name: "non-existent path",
inDirectories: map[string]*ygen.Directory{"/root-module/container": directories["/root-module/container"]},
Expand All @@ -1732,6 +1744,7 @@ func TestGetNodeDataMap(t *testing.T) {
"leaf": {NativeType: "Binary"},
},
},
inFakeRootName: "device",
inSchemaStructPkgAccessor: "oc.",
inPathStructSuffix: "Path",
wantErrSubstrings: []string{`path "/root-module/container" does not exist`},
Expand All @@ -1746,13 +1759,15 @@ func TestGetNodeDataMap(t *testing.T) {
"laugh": leafTypeMap["/root-module/container"]["leaf"],
},
},
inFakeRootName: "device",
inSchemaStructPkgAccessor: "oc.",
inPathStructSuffix: "Path",
wantErrSubstrings: []string{`field name "leaf" does not exist`},
}, {
name: "big test with everything",
inDirectories: directories,
inLeafTypeMap: leafTypeMap,
inFakeRootName: "root",
inPathStructSuffix: "Path",
inSplitByModule: true,
inPackageName: "device",
Expand All @@ -1765,7 +1780,7 @@ func TestGetNodeDataMap(t *testing.T) {
IsLeaf: false,
IsScalarField: false,
HasDefault: false,
GoPathPackageName: "device",
GoPathPackageName: "rootmodule_path",
},
"ContainerWithConfigPath": {
GoTypeName: "*ContainerWithConfig",
Expand All @@ -1775,7 +1790,7 @@ func TestGetNodeDataMap(t *testing.T) {
IsLeaf: false,
IsScalarField: false,
HasDefault: false,
GoPathPackageName: "device",
GoPathPackageName: "rootmodule_path",
},
"ContainerWithConfig_LeafPath": {
GoTypeName: "Binary",
Expand Down Expand Up @@ -1827,7 +1842,7 @@ func TestGetNodeDataMap(t *testing.T) {
IsScalarField: false,
HasDefault: false,
YANGTypeName: "ieeefloat32",
GoPathPackageName: "device",
GoPathPackageName: "rootmodule_path",
},
"LeafWithDefaultPath": {
GoTypeName: "string",
Expand All @@ -1838,7 +1853,7 @@ func TestGetNodeDataMap(t *testing.T) {
IsScalarField: true,
HasDefault: true,
YANGTypeName: "string",
GoPathPackageName: "device",
GoPathPackageName: "rootmodule_path",
},
"ListPath": {
GoTypeName: "*List",
Expand All @@ -1848,7 +1863,7 @@ func TestGetNodeDataMap(t *testing.T) {
IsLeaf: false,
IsScalarField: false,
HasDefault: false,
GoPathPackageName: "device",
GoPathPackageName: "rootmodule_path",
},
"ListWithStatePath": {
GoTypeName: "*ListWithState",
Expand All @@ -1858,7 +1873,7 @@ func TestGetNodeDataMap(t *testing.T) {
IsLeaf: false,
IsScalarField: false,
HasDefault: false,
GoPathPackageName: "device",
GoPathPackageName: "rootmodule_path",
},
"ListWithState_KeyPath": {
GoTypeName: "float64",
Expand Down Expand Up @@ -1899,6 +1914,16 @@ func TestGetNodeDataMap(t *testing.T) {
IsScalarField: false,
HasDefault: false,
GoPathPackageName: "rootmodule_path",
},
"RootPath": {
GoTypeName: "*Root",
LocalGoTypeName: "*Root",
GoFieldName: "",
SubsumingGoStructName: "Root",
IsLeaf: false,
IsScalarField: false,
HasDefault: false,
GoPathPackageName: "device",
}},
wantSorted: []string{
"ContainerPath",
Expand All @@ -1915,12 +1940,13 @@ func TestGetNodeDataMap(t *testing.T) {
"List_Key1Path",
"List_Key2Path",
"List_UnionKeyPath",
"RootPath",
},
}}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, gotErrs := getNodeDataMap(tt.inDirectories, tt.inLeafTypeMap, tt.inSchemaStructPkgAccessor, tt.inPathStructSuffix, tt.inPackageName, tt.inSplitByModule, false)
got, gotErrs := getNodeDataMap(tt.inDirectories, tt.inLeafTypeMap, tt.inFakeRootName, tt.inSchemaStructPkgAccessor, tt.inPathStructSuffix, tt.inPackageName, tt.inSplitByModule, false)
// TODO(wenbli): Enhance gNMI's errdiff with checking a slice of substrings and use here.
var gotErrStrs []string
for _, err := range gotErrs {
Expand Down

0 comments on commit 26a85b4

Please sign in to comment.