Skip to content

Commit

Permalink
Change call location of initTypes for typeDictionary (#186)
Browse files Browse the repository at this point in the history 
* Change call location of initTypes for typeDictionary

Add `newTypeDictionary()` helper and call `initTypes` closer to
where the dictionary is initialized.

* Merge with master and change aliasing test to only test top-level statements.
  • Loading branch information
@wenovus
wenovus authored Aug 16, 2021
1 parent 163ddd1 commit 1fb7893
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 77 deletions.
18 changes: 13 additions & 5 deletions pkg/yang/ast.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@ import (
// A yangStatement contains all information needed to build a particular
// type of statement into an AST node.
type yangStatement struct {
// funcs is the map of YANG field names to the function that handle them.
// funcs is the map of YANG field names to the function that populates
// the statement into the AST node.
funcs map[string]func(*Statement, reflect.Value, reflect.Value) error
// required is a list of fields that must be present in the statement.
required []string
// sRequired maps statement type to a list of required fields.
// If a field is required by statement type foo, then only foo should
// have the field.
// sRequired maps a statement name to a list of required sub-field
// names. The statement name can be an alias of the primary field type.
// e.g. If a field is required by statement type foo, then only foo
// should have the field. If bar is an alias of foo, it must not
// have this field.
sRequired map[string][]string
// addext is the function to handle possible extensions.
addext func(*Statement, reflect.Value, reflect.Value) error
Expand Down Expand Up @@ -78,6 +81,12 @@ type meta struct {

// aliases is a map of "aliased" names, that is, two types of statements
// that parse (nearly) the same.
// NOTE: This only works for root-level aliasing for now, which is good enough
// for module/submodule. This is because yangStatement.funcs doesn't store the
// handler function for aliased fields, and sRequired also may only store the
// correct values when processing a root-level statement due to aliasing. These
// issues would need to be fixed in order to support aliasing for non-top-level
// statements.
var aliases = map[string]string{
"submodule": "module",
}
Expand All @@ -92,7 +101,6 @@ func BuildAST(s *Statement) (Node, error) {
// root node. It also takes as input a type dictionary into which any
// encountered typedefs within the statement are cached.
func buildASTWithTypeDict(s *Statement, d *typeDictionary) (Node, error) {
initTypes(reflect.TypeOf(&meta{}), d)
v, err := build(s, nilValue, d)
if err != nil {
return nil, err
Expand Down
189 changes: 124 additions & 65 deletions pkg/yang/ast_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,17 @@ type MainNode struct {
ChildNode *SubNode `yang:"child_node"`
ChildSlice []*SubNode `yang:"child_slice"`
ReqNode *ReqNode `yang:"req_node"`
AltReqNode *ReqNode `yang:"alt_req_node"`
MainField *Value `yang:"main_field,required=main_node"`
AltField *Value `yang:"alt_field,required=alt_node"`
}

func (m *MainNode) Kind() string {
if m.AltField != nil {
return "alt_node"
}
return "main_node"
}

func (MainNode) Kind() string { return "main_node" }
func (m *MainNode) ParentNode() Node { return m.Parent }
func (m *MainNode) NName() string { return m.Name }
func (m *MainNode) Statement() *Statement { return m.Source }
Expand Down Expand Up @@ -103,15 +110,15 @@ func (m *MainNode) checkEqual(n Node) string {
return fmt.Sprintf("req_node: %s", s)
}
}
if (m.AltReqNode == nil) != (o.AltReqNode == nil) {
if m.AltReqNode == nil {
return "unexpected alt_req_node entry"
if (m.AltField == nil) != (o.AltField == nil) {
if m.AltField == nil {
return "unexpected alt_field entry"
}
return "missing expected alt_req_node entry"
return "missing expected alt_field entry"
}
if m.AltReqNode != nil {
if s := m.AltReqNode.checkEqual(o.AltReqNode); s != "" {
return fmt.Sprintf("alt_req_node: %s", s)
if m.AltField != nil {
if m.AltField.Name != o.AltField.Name {
return fmt.Sprintf("got alt_field of %s, want %s", o.AltField.Name, m.AltField.Name)
}
}
return ""
Expand Down Expand Up @@ -164,9 +171,6 @@ type ReqNode struct {
}

func (s *ReqNode) Kind() string {
if s.AltReqField != nil {
return "alt_req_node"
}
return "req_node"
}
func (s *ReqNode) ParentNode() Node { return s.Parent }
Expand Down Expand Up @@ -235,11 +239,10 @@ func TestAST(t *testing.T) {
type meta struct {
MainNode []*MainNode `yang:"main_node"`
}
initTypes(reflect.TypeOf(&meta{}), &typeDict)

old_aliases := aliases
aliases = map[string]string{
"alt_req_node": "req_node",
"alt_node": "main_node",
}

for _, tt := range []struct {
Expand All @@ -258,17 +261,22 @@ main_node the_node {
// See https://tools.ietf.org/html/rfc6020#section-7.17
ex:ext1 value1;
ex:ext2 value2;
main_field foo;
}
`,
out: &MainNode{
Source: SA("main_node", "the_node",
SA("ex:ext1", "value1"),
SA("ex:ext2", "value2")),
SA("ex:ext2", "value2"),
SA("main_field", "foo")),
Name: "the_node",
Extensions: []*Statement{
SA("ex:ext1", "value1"),
SA("ex:ext2", "value2"),
},
MainField: &Value{
Name: "foo",
},
},
},
{
Expand All @@ -288,6 +296,7 @@ main_node the_node {
child_slice element2 {
sub_field el2;
}
main_field foo;
}`,
out: &MainNode{
Source: SA("main_node", "the_node",
Expand All @@ -300,6 +309,7 @@ main_node the_node {
SA("sub_field", "el1")),
SA("child_slice", "element2",
SA("sub_field", "el2")),
SA("main_field", "foo"),
),
Name: "the_node",
Field: &Value{
Expand Down Expand Up @@ -339,56 +349,48 @@ main_node the_node {
},
},
},
MainField: &Value{
Name: "foo",
},
},
},
{
line: line(),
in: `
// This test tests for the presence of a required field.
// main_node requires the field named "main_field".
main_node the_node {
// This test tests for the presence of a required field.
// req_node requires the field named "req_field".
// alt_req_node requires the field named "alt_req_field".
req_node value1 {
req_field foo {
}
}
alt_req_node value2 {
req_field foo {
}
alt_req_field bar {
}
main_field value1 {
}
}
`,
out: &MainNode{
Source: SA("main_node", "the_node",
SA("req_node", "value1",
SA("req_field", "foo")),
SA("alt_req_node", "value2",
SA("req_field", "foo"),
SA("alt_req_field", "bar")),
SA("main_field", "value1"),
),
Name: "the_node",
ReqNode: &ReqNode{
Source: SA("req_node", "value1",
SA("req_field", "foo")),
MainField: &Value{
Name: "value1",
ReqField: &Value{
Name: "foo",
},
},
AltReqNode: &ReqNode{
Source: SA("alt_req_node", "value2",
SA("req_field", "foo"),
SA("alt_req_field", "bar")),
},
},
{
line: line(),
in: `
// This test tests for the presence of a required= field.
// alt_node requires the field named "alt_field".
alt_node the_node {
alt_field value2 {
}
}
`,
out: &MainNode{
Source: SA("alt_node", "the_node",
SA("alt_field", "value2"),
),
Name: "the_node",
AltField: &Value{
Name: "value2",
ReqField: &Value{
Name: "foo",
},
AltReqField: &Value{
Name: "bar",
},
},
},
},
Expand All @@ -412,9 +414,44 @@ main_node the_node {
line: line(),
in: `
main_node the_node {
// This test tests that the absence of a required field.
// This test tests for the presence of a required field.
// req_node requires the field named "req_field".
req_node value1 {
req_field foo {
}
}
main_field foo;
}
`,
out: &MainNode{
Source: SA("main_node", "the_node",
SA("req_node", "value1",
SA("req_field", "foo")),
SA("main_field", "foo"),
),
Name: "the_node",
ReqNode: &ReqNode{
Source: SA("req_node", "value1",
SA("req_field", "foo")),
Name: "value1",
ReqField: &Value{
Name: "foo",
},
},
MainField: &Value{
Name: "foo",
},
},
},
{
line: line(),
in: `
main_node the_node {
// This test tests that the absence of a required field fails.
// req_node requires the field named "req_field".
req_node value1 ;
req_node value1 {
}
main_field foo;
}
`,
err: `ast.yang:5:2: missing required req_node field: req_field`,
Expand All @@ -423,32 +460,50 @@ main_node the_node {
line: line(),
in: `
main_node the_node {
// This test tests that the alt_req_field, specified with
// required=alt_req_node, causes the AST construction to error when a
// req_node contains it.
// This test tests that the absence of a required field.
// main_node requires the field named "main_field".
req_node value1 {
req_field foo {
}
alt_req_field foo {
}
}
}
`,
err: `ast.yang:6:2: unknown req_node field: alt_req_field`,
err: `ast.yang:2:1: missing required main_node field: main_field`,
},
{
line: line(),
in: `
// This test tests that the alt_field, specified with
// required=alt_node, causes the AST construction to error when a
// main_node contains it.
main_node the_node {
// This test tests that required=alt_req_node enforces that
// alt_req_node must contain it.
alt_req_node value2 {
req_field foo {
}
}
main_field foo;
alt_field foo;
}
`,
err: `ast.yang:5:2: missing required alt_req_node field: alt_req_field`,
err: `ast.yang:5:1: unknown main_node field: alt_field`,
},
{
line: line(),
in: `
// This test tests that required=alt_node enforces that
// alt_node must contain it.
alt_node the_node {
main_field foo;
alt_field foo;
}
`,
err: `ast.yang:4:1: unknown alt_node field: main_field`,
},
{
line: line(),
in: `
// This test tests that required=alt_node enforces that
// alt_node must contain it.
alt_node the_node {
}
`,
err: `ast.yang:4:1: missing required alt_node field: alt_field`,
},
} {
ss, err := Parse(tt.in, "ast.yang")
Expand All @@ -460,7 +515,11 @@ main_node the_node {
t.Errorf("%d: got %d results, want 1", tt.line, len(ss))
continue
}
ast, err := BuildAST(ss[0])

typeDict := newTypeDictionary()
initTypes(reflect.TypeOf(&meta{}), typeDict)

ast, err := buildASTWithTypeDict(ss[0], typeDict)
switch {
case err == nil && tt.err == "":
if s := tt.out.checkEqual(ast); s != "" {
Expand Down
10 changes: 7 additions & 3 deletions pkg/yang/bgp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@

package yang

import "testing"
import (
"reflect"
"testing"
)

// TestBGP simply makes sure we are able to parse a version of Anees's
// BGP model. We don't actually attempt to validate we got the right
Expand All @@ -28,8 +31,9 @@ func TestBGP(t *testing.T) {
if len(ss) != 1 {
t.Fatalf("got %d results, want 1", len(ss))
}
_, err = BuildAST(ss[0])
if err != nil {
typeDict := newTypeDictionary()
initTypes(reflect.TypeOf(&meta{}), typeDict)
if _, err := buildASTWithTypeDict(ss[0], typeDict); err != nil {
t.Fatal(err)
}
}
Expand Down
1 change: 0 additions & 1 deletion pkg/yang/entry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ module base {

func TestBadYang(t *testing.T) {
for _, tt := range badInputs {
typeDict = typeDictionary{dict: map[Node]map[string]*Typedef{}}
ms := NewModules()
if err := ms.Parse(tt.in, tt.name); err != nil {
t.Fatalf("unexpected error %s", err)
Expand Down
Loading

0 comments on commit 1fb7893

Please sign in to comment.