Repository: robshakir/pyangbind

Issue:
Leaf-list stores multiple duplicate values
Hello,
I noticed in [RFC 6020 section 7.7 for leaf-list](https://tools.ietf.org/html/rfc6020#section-7.7) that there is a requirement on leaf-list which we do not follow in pyangbind.

`The values in a leaf-list MUST be unique.`

### Example:
Given the following yang model:
```yang
module test {
    namespace "http://foo/bar/test/1.0";
    prefix test;
    leaf-list my-leaf-list {
        type string;
    }
}
```
#### Expected
```python
>>> from test import test
>>> from pyangbind.lib.xpathhelper import YANGPathHelper
>>>
>>> graph = test(ph=YANGPathHelper())
>>>
>>> graph.my_leaf_list = ['foo', 'foo']
>>> graph.get()
{'my-leaf-list': [u'foo']}
```
#### Actual
```python
>>> graph.get()
{'my-leaf-list': [u'foo', u'foo']}
```

I think the leaf-list structure might need to resemble a set instead of a list, unless I misunderstood the rfc.

Discussion:

Patch:
diff --git a/pyangbind/lib/yangtypes.py b/pyangbind/lib/yangtypes.py
index aa6fe79..8b73346 100644
--- a/pyangbind/lib/yangtypes.py
+++ b/pyangbind/lib/yangtypes.py
@@ -377,19 +377,25 @@ def TypedListType(*args, **kwargs):
     _list = list()
 
     def __init__(self, *args, **kwargs):
+      self._unique = kwargs.pop('unique', False)
       self._allowed_type = allowed_type
       self._list = list()
       if len(args):
         if isinstance(args[0], list):
           tmp = []
           for i in args[0]:
-            tmp.append(self.check(i))
+            if not self._unique or i not in tmp:
+              tmp.append(self.check(i))
           self._list.extend(tmp)
         else:
           tmp = self.check(args[0])
           self._list.append(tmp)
 
     def check(self, v):
+      # Short circuit uniqueness check
+      if self._unique and v in self._list:
+        raise ValueError("Values in this list must be unique.")
+
       passed = False
       count = 0
       for i in self._allowed_type:
@@ -446,16 +452,16 @@ def TypedListType(*args, **kwargs):
       del self._list[i]
 
     def __setitem__(self, i, v):
-      self.check(v)
-      self._list.insert(i, v)
+      self.insert(i, v)
 
     def insert(self, i, v):
       val = self.check(v)
       self._list.insert(i, val)
 
     def append(self, v):
-      val = self.check(v)
-      self._list.append(val)
+      if not self._unique or v not in self._list:
+        val = self.check(v)
+        self._list.append(val)
 
     def __str__(self):
       return str(self._list)
diff --git a/pyangbind/plugin/pybind.py b/pyangbind/plugin/pybind.py
index 98ef25e..3707c08 100644
--- a/pyangbind/plugin/pybind.py
+++ b/pyangbind/plugin/pybind.py
@@ -844,7 +844,7 @@ def get_children(ctx, fd, i_children, module, parent, path=str(),
         # TypedList (see lib.yangtypes) with a particular set of types allowed.
         class_str["name"] = "__%s" % (i["name"])
         class_str["type"] = "YANGDynClass"
-        class_str["arg"] = "base="
+        class_str["arg"] = "unique=True, base="
         if isinstance(i["type"]["native_type"][1], list):
           allowed_type = "["
           for subtype in i["type"]["native_type"][1]:
diff --git a/tox.ini b/tox.ini
index 51e59dc..d11d8c3 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,5 +1,5 @@
 [tox]
-envlist = py{27,34,35,36,37,py2,py3},flake8
+envlist = py{27,34,35,36,37,py2},flake8
 skip_missing_interpreters = True
 
 [testenv]


Test patch:
diff --git a/tests/leaf-list/run.py b/tests/leaf-list/run.py
index 331167b..f84c6cf 100755
--- a/tests/leaf-list/run.py
+++ b/tests/leaf-list/run.py
@@ -16,22 +16,21 @@ class LeafListTests(PyangBindTestCase):
     self.leaflist_obj = self.bindings.leaflist()
 
   def test_container_exists(self):
-    self.assertTrue(hasattr(self.leaflist_obj, "container"), "Base container is missing.")
+    self.assertTrue(hasattr(self.leaflist_obj, "container"))
 
   def test_leaflist_exists(self):
-    self.assertTrue(hasattr(self.leaflist_obj.container, "leaflist"), "Leaf-list instance is missing.")
+    self.assertTrue(hasattr(self.leaflist_obj.container, "leaflist"))
 
   def test_leaflist_length_is_zero(self):
-    self.assertEqual(len(self.leaflist_obj.container.leaflist), 0, "Length of leaflist was not zero.")
+    self.assertEqual(len(self.leaflist_obj.container.leaflist), 0)
 
   def test_append_to_leaflist(self):
     self.leaflist_obj.container.leaflist.append("itemOne")
-    self.assertEqual(len(self.leaflist_obj.container.leaflist), 1, "Did not successfully append string to list.")
+    self.assertEqual(len(self.leaflist_obj.container.leaflist), 1)
 
   def test_retrieve_leaflist_item_value(self):
     self.leaflist_obj.container.leaflist.append("itemOne")
-    self.assertEqual(self.leaflist_obj.container.leaflist[0], "itemOne",
-      "Cannot successfully address an item from the list.")
+    self.assertEqual(self.leaflist_obj.container.leaflist[0], "itemOne")
 
   def test_append_int_to_string_leaflist(self):
     with self.assertRaises(ValueError):
@@ -41,14 +40,14 @@ class LeafListTests(PyangBindTestCase):
     self.leaflist_obj.container.leaflist.append("itemOne")
     self.leaflist_obj.container.leaflist.append("itemTwo")
 
-    self.assertEqual(self.leaflist_obj.container.leaflist[1], "itemTwo", "getitem did not return the correct value.")
+    self.assertEqual(self.leaflist_obj.container.leaflist[1], "itemTwo")
 
   def test_setitem(self):
     self.leaflist_obj.container.leaflist.append("itemOne")
     self.leaflist_obj.container.leaflist.append("itemTwo")
     self.leaflist_obj.container.leaflist[1] = "indexOne"
 
-    self.assertEqual(self.leaflist_obj.container.leaflist[1], "indexOne", "setitem did not set the correct node.")
+    self.assertEqual(self.leaflist_obj.container.leaflist[1], "indexOne")
 
   def test_insert(self):
     self.leaflist_obj.container.leaflist.append("itemOne")
@@ -56,7 +55,7 @@ class LeafListTests(PyangBindTestCase):
     self.leaflist_obj.container.leaflist[1] = "indexOne"
     self.leaflist_obj.container.leaflist.insert(0, "indexZero")
 
-    self.assertEqual(self.leaflist_obj.container.leaflist[0], "indexZero", "Incorrectly set index 0 value")
+    self.assertEqual(self.leaflist_obj.container.leaflist[0], "indexZero")
 
   def test_leaflist_grows_from_various_modification_methods(self):
     self.leaflist_obj.container.leaflist.append("itemOne")
@@ -64,7 +63,7 @@ class LeafListTests(PyangBindTestCase):
     self.leaflist_obj.container.leaflist[1] = "indexOne"
     self.leaflist_obj.container.leaflist.insert(0, "indexZero")
 
-    self.assertEqual(len(self.leaflist_obj.container.leaflist), 4, "List item was not added by insert()")
+    self.assertEqual(len(self.leaflist_obj.container.leaflist), 4)
 
   def test_delete_item_from_leaflist(self):
     self.leaflist_obj.container.leaflist.append("itemOne")
@@ -74,7 +73,7 @@ class LeafListTests(PyangBindTestCase):
 
     del self.leaflist_obj.container.leaflist[0]
 
-    self.assertEqual(len(self.leaflist_obj.container.leaflist), 3, "List item not successfully removed by delitem")
+    self.assertEqual(len(self.leaflist_obj.container.leaflist), 3)
 
   def test_get_full_leaflist(self):
     self.leaflist_obj.container.leaflist.append("itemOne")
@@ -87,15 +86,13 @@ class LeafListTests(PyangBindTestCase):
       self.leaflist_obj.get(),
       {'container': {'leaflist': ['itemOne', 'indexOne', 'itemTwo'],
                      'listtwo': [],
-                     'listthree': []}},
-      "get did not correctly return the dictionary"
+                     'listthree': []}}
     )
 
   def test_leaflist_assignment(self):
     self.leaflist_obj.container.leaflist = ["itemOne", "itemTwo"]
 
-    self.assertEqual(self.leaflist_obj.container.leaflist, ["itemOne", "itemTwo"],
-      "Leaflist assignment did not function correctly")
+    self.assertEqual(self.leaflist_obj.container.leaflist, ["itemOne", "itemTwo"])
 
   def test_leaflist_assignment_of_wrong_type(self):
     with self.assertRaises(ValueError):
@@ -103,7 +100,7 @@ class LeafListTests(PyangBindTestCase):
 
   def test_restricted_string(self):
     self.leaflist_obj.container.listtwo.append("a-valid-string")
-    self.assertEqual(len(self.leaflist_obj.container.listtwo), 1, "Restricted lefalist did not function correctly.")
+    self.assertEqual(len(self.leaflist_obj.container.listtwo), 1)
 
   def test_restricted_string_invalid_value(self):
     with self.assertRaises(ValueError):
@@ -117,8 +114,23 @@ class LeafListTests(PyangBindTestCase):
           self.leaflist_obj.container.listthree.append(pair[0])
         except ValueError:
           allowed = False
-        self.assertEqual(allowed, pair[1], "leaf-list of union type had invalid result (%s != %s for %s)" %
-          (allowed, pair[1], pair[0]))
+        self.assertEqual(allowed, pair[1])
+
+  def test_leaf_lists_are_unique_after_assignment(self):
+    self.leaflist_obj.container.leaflist = ['foo', 'bar', 'foo']
+    self.assertEqual(self.leaflist_obj.container.get(filter=True), {'leaflist': ['foo', 'bar']})
+
+  def test_leaf_lists_are_unique_after_append(self):
+    self.leaflist_obj.container.leaflist.append('foo')
+    self.leaflist_obj.container.leaflist.append('bar')
+    self.leaflist_obj.container.leaflist.append('foo')
+    self.assertEqual(self.leaflist_obj.container.get(filter=True), {'leaflist': ['foo', 'bar']})
+
+  def test_leaf_lists_insert_non_unique_value_raises_keyerror(self):
+    self.leaflist_obj.container.leaflist[0] = 'foo'
+    self.leaflist_obj.container.leaflist[1] = 'bar'
+    with self.assertRaises(ValueError):
+      self.leaflist_obj.container.leaflist[2] = 'foo'
 
 
 if __name__ == '__main__':