Repository: openconfig/goyang
Pull request #242 (2023-04-21 21:55:36)

Add `OrderedByUser` field
Per https://datatracker.ietf.org/doc/html/rfc7950#section-7.7.7 the field is either ordered by the user or it is not.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/59206255/badge)](https://coveralls.io/builds/59206255)

Coverage: 83.744% (-0.3%) from 84.006% when pulling **2828bf1ad087bc9642ee03d21de413a84aaead16 on ordered-by-user** into **fc70dac8a958090641b25e7c7e6f0ca8d714cc5e on master**.


Patch:
diff --git a/pkg/yang/entry.go b/pkg/yang/entry.go
index eb06437..e2c5309 100644
--- a/pkg/yang/entry.go
+++ b/pkg/yang/entry.go
@@ -148,7 +148,31 @@ type RPCEntry struct {
 type ListAttr struct {
 	MinElements uint64 // leaf-list or list MUST have at least min-elements
 	MaxElements uint64 // leaf-list or list has at most max-elements
-	OrderedBy   *Value // order of entries determined by "system" or "user"
+	// OrderedBy is deprecated. Use OrderedByUser instead.
+	OrderedBy *Value
+	// OrderedByUser indicates whether the entries are "ordered-by user".
+	// Otherwise the order is determined by the system.
+	OrderedByUser bool
+}
+
+// parseOrderedBy parses the ordered-by value and classifies the list/leaf-list
+// by whether the `ordered-by user` modifier is active.
+//
+// For more information see
+// https://datatracker.ietf.org/doc/html/rfc7950#section-7.7.7
+func (l *ListAttr) parseOrderedBy(s *Value) error {
+	if s == nil {
+		return nil
+	}
+	l.OrderedBy = s
+	switch s.Name {
+	case "user":
+		l.OrderedByUser = true
+	case "system":
+	default:
+		return fmt.Errorf("%s: ordered-by has invalid argument: %q", Source(s), s.Name)
+	}
+	return nil
 }
 
 // NewDefaultListAttr returns a new ListAttr object with min/max elements being
@@ -610,7 +634,9 @@ func ToEntry(n Node) (e *Entry) {
 
 		e = ToEntry(leaf)
 		e.ListAttr = NewDefaultListAttr()
-		e.ListAttr.OrderedBy = s.OrderedBy
+		if err := e.ListAttr.parseOrderedBy(s.OrderedBy); err != nil {
+			e.addError(err)
+		}
 		var err error
 		if e.ListAttr.MaxElements, err = semCheckMaxElements(s.MaxElements); err != nil {
 			e.addError(err)
@@ -647,7 +673,9 @@ func ToEntry(n Node) (e *Entry) {
 	switch s := n.(type) {
 	case *List:
 		e.ListAttr = NewDefaultListAttr()
-		e.ListAttr.OrderedBy = s.OrderedBy
+		if err := e.ListAttr.parseOrderedBy(s.OrderedBy); err != nil {
+			e.addError(err)
+		}
 		var err error
 		if e.ListAttr.MaxElements, err = semCheckMaxElements(s.MaxElements); err != nil {
 			e.addError(err)


Test patch:
diff --git a/pkg/yang/entry_test.go b/pkg/yang/entry_test.go
index b2a5ae0..b87d948 100644
--- a/pkg/yang/entry_test.go
+++ b/pkg/yang/entry_test.go
@@ -3958,3 +3958,165 @@ func TestLess(t *testing.T) {
 		}
 	}
 }
+
+type customTestCases struct {
+	wantEntryPath       string
+	wantEntryCustomTest func(t *testing.T, e *Entry)
+}
+
+func TestOrderedBy(t *testing.T) {
+	tests := []struct {
+		name          string
+		inModules     map[string]string
+		testcases     []customTestCases
+		wantErrSubstr string
+	}{{
+		name: "ordered-by user",
+		inModules: map[string]string{
+			"test.yang": `
+			module test {
+				prefix "t";
+				namespace "urn:t";
+
+				list ordered-list {
+					key "name";
+					ordered-by user;
+					leaf name {
+						type string;
+					}
+				}
+
+				list unordered-list {
+					key "name";
+					ordered-by system;
+					leaf name {
+						type string;
+					}
+				}
+
+				list unordered-list2 {
+					key "name";
+					leaf name {
+						type string;
+					}
+				}
+
+				leaf-list ordered-leaflist {
+					ordered-by user;
+					type string;
+				}
+
+				leaf-list unordered-leaflist {
+					ordered-by system;
+					type string;
+				}
+
+				leaf-list unordered-leaflist2 {
+					type string;
+				}
+			}
+			`,
+		},
+		testcases: []customTestCases{{
+			wantEntryPath: "/test/ordered-list",
+			wantEntryCustomTest: func(t *testing.T, e *Entry) {
+				if got, want := e.ListAttr.OrderedByUser, true; got != want {
+					t.Errorf("got %v, want %v", got, want)
+				}
+			},
+		}, {
+			wantEntryPath: "/test/unordered-list",
+			wantEntryCustomTest: func(t *testing.T, e *Entry) {
+				if got, want := e.ListAttr.OrderedByUser, false; got != want {
+					t.Errorf("got %v, want %v", got, want)
+				}
+			},
+		}, {
+			wantEntryPath: "/test/unordered-list2",
+			wantEntryCustomTest: func(t *testing.T, e *Entry) {
+				if got, want := e.ListAttr.OrderedByUser, false; got != want {
+					t.Errorf("got %v, want %v", got, want)
+				}
+			},
+		}, {
+			wantEntryPath: "/test/ordered-leaflist",
+			wantEntryCustomTest: func(t *testing.T, e *Entry) {
+				if got, want := e.ListAttr.OrderedByUser, true; got != want {
+					t.Errorf("got %v, want %v", got, want)
+				}
+			},
+		}, {
+			wantEntryPath: "/test/unordered-leaflist",
+			wantEntryCustomTest: func(t *testing.T, e *Entry) {
+				if got, want := e.ListAttr.OrderedByUser, false; got != want {
+					t.Errorf("got %v, want %v", got, want)
+				}
+			},
+		}, {
+			wantEntryPath: "/test/unordered-leaflist2",
+			wantEntryCustomTest: func(t *testing.T, e *Entry) {
+				if got, want := e.ListAttr.OrderedByUser, false; got != want {
+					t.Errorf("got %v, want %v", got, want)
+				}
+			},
+		}},
+	}, {
+		name: "ordered-by client: invalid argument",
+		inModules: map[string]string{
+			"test.yang": `
+			module test {
+				prefix "t";
+				namespace "urn:t";
+
+				list ordered-list {
+					key "name";
+					ordered-by client;
+					leaf name {
+						type string;
+					}
+				}
+			}
+			`,
+		},
+		wantErrSubstr: "ordered-by has invalid argument",
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ms := NewModules()
+			var errs []error
+			for n, m := range tt.inModules {
+				if err := ms.Parse(m, n); err != nil {
+					errs = append(errs, err)
+				}
+			}
+
+			if len(errs) > 0 {
+				t.Fatalf("ms.Parse(), got unexpected error parsing input modules: %v", errs)
+			}
+
+			if errs := ms.Process(); len(errs) > 0 {
+				if len(errs) == 1 {
+					if diff := errdiff.Substring(errs[0], tt.wantErrSubstr); diff != "" {
+						t.Fatalf("did not get expected error, %s", diff)
+					}
+					return
+				}
+				t.Fatalf("ms.Process(), got too many errors processing entries: %v", errs)
+			}
+
+			dir := map[string]*Entry{}
+			for _, m := range ms.Modules {
+				addTreeE(ToEntry(m), dir)
+			}
+
+			for _, tc := range tt.testcases {
+				e, ok := dir[tc.wantEntryPath]
+				if !ok {
+					t.Fatalf("could not find entry %s within the dir: %v", tc.wantEntryPath, dir)
+				}
+				tc.wantEntryCustomTest(t, e)
+			}
+		})
+	}
+}
diff --git a/pkg/yang/marshal_test.go b/pkg/yang/marshal_test.go
index f77582e..6151aa1 100644
--- a/pkg/yang/marshal_test.go
+++ b/pkg/yang/marshal_test.go
@@ -342,7 +342,8 @@ func TestMarshalJSON(t *testing.T) {
       "ListAttr": {
         "MinElements": 0,
         "MaxElements": 18446744073709551615,
-        "OrderedBy": null
+        "OrderedBy": null,
+        "OrderedByUser": false
       }
     }
   },
@@ -356,7 +357,8 @@ func TestMarshalJSON(t *testing.T) {
   "ListAttr": {
     "MinElements": 48,
     "MaxElements": 42,
-    "OrderedBy": null
+    "OrderedBy": null,
+    "OrderedByUser": false
   },
   "Identities": [
     {
@@ -534,7 +536,8 @@ func TestParseAndMarshal(t *testing.T) {
           "ListAttr": {
             "MinElements": 10,
             "MaxElements": 18446744073709551615,
-            "OrderedBy": null
+            "OrderedBy": null,
+            "OrderedByUser": false
           }
         },
         "d": {
@@ -623,7 +626,8 @@ func TestParseAndMarshal(t *testing.T) {
           "ListAttr": {
             "MinElements": 0,
             "MaxElements": 18446744073709551615,
-            "OrderedBy": null
+            "OrderedBy": null,
+            "OrderedByUser": false
           }
         },
         "zip2": {
@@ -645,7 +649,8 @@ func TestParseAndMarshal(t *testing.T) {
           "ListAttr": {
             "MinElements": 0,
             "MaxElements": 1000,
-            "OrderedBy": null
+            "OrderedBy": null,
+            "OrderedByUser": false
           }
         }
       }