From 26a85b4b542000f94eb1f6052eb9a78f23a6f26a Mon Sep 17 00:00:00 2001 From: Wen Bo Li <50884368+wenovus@users.noreply.github.com> Date: Fri, 8 Oct 2021 11:03:55 -0700 Subject: [PATCH] Use correct schema entry for determining package name, and minor refactor 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. --- ypathgen/pathgen.go | 55 ++++++++++++++++++++-------------------- ypathgen/pathgen_test.go | 42 ++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 36 deletions(-) diff --git a/ypathgen/pathgen.go b/ypathgen/pathgen.go index 0ce02c6fb..a5ca9a8df 100644 --- a/ypathgen/pathgen.go +++ b/ypathgen/pathgen.go @@ -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) } @@ -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 @@ -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-") } @@ -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 { @@ -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), } } } @@ -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 + "." @@ -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) diff --git a/ypathgen/pathgen_test.go b/ypathgen/pathgen_test.go index 33c0e4c84..07151ed69 100644 --- a/ypathgen/pathgen_test.go +++ b/ypathgen/pathgen_test.go @@ -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 @@ -1679,6 +1680,7 @@ func TestGetNodeDataMap(t *testing.T) { "leaf": leafTypeMap["/root-module/container"]["leaf"], }, }, + inFakeRootName: "device", inSchemaStructPkgAccessor: "struct.", inPathStructSuffix: "Path", wantNodeDataMap: NodeDataMap{ @@ -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{ @@ -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"]}, @@ -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`}, @@ -1746,6 +1759,7 @@ 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`}, @@ -1753,6 +1767,7 @@ func TestGetNodeDataMap(t *testing.T) { name: "big test with everything", inDirectories: directories, inLeafTypeMap: leafTypeMap, + inFakeRootName: "root", inPathStructSuffix: "Path", inSplitByModule: true, inPackageName: "device", @@ -1765,7 +1780,7 @@ func TestGetNodeDataMap(t *testing.T) { IsLeaf: false, IsScalarField: false, HasDefault: false, - GoPathPackageName: "device", + GoPathPackageName: "rootmodule_path", }, "ContainerWithConfigPath": { GoTypeName: "*ContainerWithConfig", @@ -1775,7 +1790,7 @@ func TestGetNodeDataMap(t *testing.T) { IsLeaf: false, IsScalarField: false, HasDefault: false, - GoPathPackageName: "device", + GoPathPackageName: "rootmodule_path", }, "ContainerWithConfig_LeafPath": { GoTypeName: "Binary", @@ -1827,7 +1842,7 @@ func TestGetNodeDataMap(t *testing.T) { IsScalarField: false, HasDefault: false, YANGTypeName: "ieeefloat32", - GoPathPackageName: "device", + GoPathPackageName: "rootmodule_path", }, "LeafWithDefaultPath": { GoTypeName: "string", @@ -1838,7 +1853,7 @@ func TestGetNodeDataMap(t *testing.T) { IsScalarField: true, HasDefault: true, YANGTypeName: "string", - GoPathPackageName: "device", + GoPathPackageName: "rootmodule_path", }, "ListPath": { GoTypeName: "*List", @@ -1848,7 +1863,7 @@ func TestGetNodeDataMap(t *testing.T) { IsLeaf: false, IsScalarField: false, HasDefault: false, - GoPathPackageName: "device", + GoPathPackageName: "rootmodule_path", }, "ListWithStatePath": { GoTypeName: "*ListWithState", @@ -1858,7 +1873,7 @@ func TestGetNodeDataMap(t *testing.T) { IsLeaf: false, IsScalarField: false, HasDefault: false, - GoPathPackageName: "device", + GoPathPackageName: "rootmodule_path", }, "ListWithState_KeyPath": { GoTypeName: "float64", @@ -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", @@ -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 {