Python-IDL: getattr after mutate fix

This commit returns the updated column value when getattr is done
after a mutate operation is performed (but before the commit).

Signed-off-by: Amitabha Biswas <azbiswas@gmail.com>
Reported-by: Richard Theis <rtheis@us.ibm.com>
Reported-at: http://openvswitch.org/pipermail/dev/2016-September/080120.html
Fixes: a59912a0ee ("python: Add support for partial map and set updates")
Signed-off-by: Russell Bryant <russell@ovn.org>

python/ovs/db/idl.py

@@ -770,6 +770,7 @@ class Row(object):
         assert self._changes is not None
         assert self._mutations is not None
 
+        column = self._table.columns[column_name]
         datum = self._changes.get(column_name)
         inserts = None
         if '_inserts' in self._mutations.keys():
@@ -784,24 +785,33 @@ class Row(object):
                                          (self.__class__.__name__,
                                           column_name))
                 else:
-                    datum = inserts
+                    datum = data.Datum.from_python(column.type,
-            if column_name in self._data:
+                                                   inserts,
+                                                   _row_to_uuid)
+            elif column_name in self._data:
                 datum = self._data[column_name]
-                try:
+                if column.type.is_set():
+                    dlist = datum.as_list()
                     if inserts is not None:
-                        datum.extend(inserts)
+                        dlist.extend(list(inserts))
                     if removes is not None:
-                        datum = [x for x in datum if x not in removes]
+                        removes_datum = data.Datum.from_python(column.type,
-                except error.Error:
+                                                              removes,
-                    pass
+                                                              _row_to_uuid)
-                try:
+                        removes_list = removes_datum.as_list()
+                        dlist = [x for x in dlist if x not in removes_list]
+                    datum = data.Datum.from_python(column.type, dlist,
+                                                   _row_to_uuid)
+                elif column.type.is_map():
+                    dmap = datum.to_python(_uuid_to_row)
                     if inserts is not None:
-                        datum.merge(inserts)
+                        dmap.update(inserts)
                     if removes is not None:
-                        for key in removes.keys():
+                        for key in removes:
-                            del datum[key]
+                            if key not in (inserts or {}):
-                except error.Error:
+                                del dmap[key]
-                    pass
+                    datum = data.Datum.from_python(column.type, dmap,
+                                                   _row_to_uuid)
             else:
                 if inserts is None:
                     raise AttributeError("%s instance has no attribute '%s'" %
@@ -870,7 +880,7 @@ class Row(object):
             vlog.err("attempting to write bad value to column %s (%s)"
                      % (column_name, e))
             return
-        if column_name in self._data:
+        if self._data and column_name in self._data:
             # Remove existing key/value before updating.
             removes = self._mutations.setdefault('_removes', {})
             column_value = removes.setdefault(column_name, set())

tests/ovsdb-idl.at

@@ -1094,15 +1094,18 @@ OVSDB_CHECK_IDL_PY([partial-map idl],
 [['["idltest", {"op":"insert", "table":"simple2",
                 "row":{"name":"myString1","smap":["map",[["key1","value1"],["key2","value2"]]]} }]']
 ],
-  [?simple2:name,smap,imap 'partialmapinsertelement' 'partialmapinsertmultipleelements' 'partialmapdelelements'],
+  [?simple2:name,smap,imap 'partialmapinsertelement' 'partialmapinsertmultipleelements' 'partialmapdelelements' 'partialmapmutatenew'],
 [[000: name=myString1 smap=[(key1 value1) (key2 value2)] imap=[]
 001: commit, status=success
 002: name=String2 smap=[(key1 myList1) (key2 value2)] imap=[(3 myids2)]
 003: commit, status=success
-004: name=String2 smap=[(key1 myList1) (key2 myList2) (key3 myList3)] imap=[(3 myids2)]
+004: name=String2 smap=[(key1 myList1) (key2 myList2) (key3 myList3) (key4 myList4)] imap=[(3 myids2)]
 005: commit, status=success
 006: name=String2 smap=[(key2 myList2)] imap=[(3 myids2)]
-007: done
+007: commit, status=success
+008: name=String2 smap=[(key2 myList2)] imap=[(3 myids2)]
+008: name=String2New smap=[(key1 newList1) (key2 newList2)] imap=[]
+009: done
 ]])
 
 m4_define([OVSDB_CHECK_IDL_PARTIAL_UPDATE_SET_COLUMN],
@@ -1145,7 +1148,7 @@ OVSDB_CHECK_IDL_PY([partial-set idl],
                {"op":"mutate", "table":"simple3", "where":[["_uuid", "==", ["named-uuid", "newrow"]]],
                     "mutations": [["uset", "insert", ["set", [["uuid", "000d2f6a-76af-412f-b59d-e7bcd3e84eff"]]]]]}]']
 ],
-  ['partialrenamesetadd' 'partialduplicateadd' 'partialsetdel' 'partialsetref' 'partialsetoverrideops' 'partialsetmutatenew'],
+  ['partialrenamesetadd' 'partialduplicateadd' 'partialsetdel' 'partialsetref' 'partialsetoverrideops' 'partialsetadddelete' 'partialsetmutatenew'],
 [[000: name=mySet1 uset=[<0> <1>]
 001: commit, status=success
 002: name=String2 uset=[<0> <1> <2>]
@@ -1158,9 +1161,11 @@ OVSDB_CHECK_IDL_PY([partial-set idl],
 009: commit, status=success
 010: name=String2 uset=[<3>]
 011: commit, status=success
-012: name=String2 uset=[<3>]
+012: name=String2 uset=[<4> <5>]
-012: name=String3 uset=[<4>]
+013: commit, status=success
-013: done
+014: name=String2 uset=[<4> <5>]
+014: name=String3 uset=[<6>]
+015: done
 ]])
 
 m4_define([OVSDB_CHECK_IDL_NOTIFY_PY],

tests/test-ovsdb.py

@@ -431,38 +431,63 @@ def idl_set(idl, commands, step):
             l1.k = [l1]
         elif name == 'partialmapinsertelement':
             row = idltest_find_simple2(idl, 'myString1')
+            len_smap = len(getattr(row, 'smap'))
             row.setkey('smap', 'key1', 'myList1')
+            len_imap = len(getattr(row, 'imap'))
             row.setkey('imap', 3, 'myids2')
             row.__setattr__('name', 'String2')
+            assert len(getattr(row, 'smap')) == len_smap
+            assert len(getattr(row, 'imap')) == len_imap + 1
         elif name == 'partialmapinsertmultipleelements':
             row = idltest_find_simple2(idl, 'String2')
+            len_smap = len(getattr(row, 'smap'))
             row.setkey('smap', 'key2', 'myList2')
             row.setkey('smap', 'key3', 'myList3')
+            row.setkey('smap', 'key4', 'myList4')
+            assert len(getattr(row, 'smap')) == len_smap + 2
         elif name == 'partialmapdelelements':
             row = idltest_find_simple2(idl, 'String2')
+            len_smap = len(getattr(row, 'smap'))
             row.delkey('smap', 'key1', 'myList1')
             row.delkey('smap', 'key2', 'wrongvalue')
             row.delkey('smap', 'key3')
+            row.delkey('smap', 'key4')
+            assert len(getattr(row, 'smap')) == len_smap - 3
+        elif name == 'partialmapmutatenew':
+            new_row2 = txn.insert(idl.tables["simple2"])
+            setattr(new_row2, 'name', 'String2New')
+            new_row2.setkey('smap', 'key1', 'newList1')
+            assert len(getattr(new_row2, 'smap')) == 1
+            new_row2.setkey('smap', 'key2', 'newList2')
+            assert len(getattr(new_row2, 'smap')) == 2
         elif name == 'partialrenamesetadd':
             row = idltest_find_simple3(idl, 'mySet1')
+            old_size = len(getattr(row, 'uset', []))
             row.addvalue('uset',
                          uuid.UUID("001e43d2-dd3f-4616-ab6a-83a490bb0991"))
             row.__setattr__('name', 'String2')
+            assert len(getattr(row, 'uset', [])) == old_size + 1
         elif name == 'partialduplicateadd':
             row = idltest_find_simple3(idl, 'String2')
+            old_size = len(getattr(row, 'uset', []))
             row.addvalue('uset',
                          uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
             row.addvalue('uset',
                          uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
+            assert len(getattr(row, 'uset', [])) == old_size + 1
         elif name == 'partialsetdel':
             row = idltest_find_simple3(idl, 'String2')
+            old_size = len(getattr(row, 'uset', []))
             row.delvalue('uset',
                          uuid.UUID("001e43d2-dd3f-4616-ab6a-83a490bb0991"))
+            assert len(getattr(row, 'uset', [])) == old_size - 1
         elif name == 'partialsetref':
             new_row = txn.insert(idl.tables["simple4"])
             new_row.__setattr__('name', 'test')
             row = idltest_find_simple3(idl, 'String2')
+            old_size = len(getattr(row, 'uref', []))
             row.addvalue('uref', new_row.uuid)
+            assert len(getattr(row, 'uref', [])) == old_size + 1
         elif name == 'partialsetoverrideops':
             row = idltest_find_simple3(idl, 'String2')
             row.addvalue('uset',
@@ -471,12 +496,23 @@ def idl_set(idl, commands, step):
                          uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
             row.__setattr__('uset',
                 [uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8")])
+            assert len(getattr(row, 'uset', [])) == 1
+        elif name == 'partialsetadddelete':
+            row = idltest_find_simple3(idl, 'String2')
+            row.addvalue('uset',
+                         uuid.UUID('b6272353-af9c-40b7-90fe-32a43e6518a1'))
+            row.addvalue('uset',
+                         uuid.UUID('1d6a71a2-dffb-426e-b2fa-b727091f9901'))
+            row.delvalue('uset',
+                         uuid.UUID('0026b3ba-571b-4729-8227-d860a5210ab8'))
+            assert len(getattr(row, 'uset', [])) == 2
         elif name == 'partialsetmutatenew':
             new_row41 = txn.insert(idl.tables["simple4"])
             new_row41.__setattr__('name', 'new_row41')
             new_row3 = txn.insert(idl.tables["simple3"])
             setattr(new_row3, 'name', 'String3')
             new_row3.addvalue('uset', new_row41.uuid)
+            assert len(getattr(new_row3, 'uset', [])) == 1
         else:
             sys.stderr.write("unknown command %s\n" % name)
             sys.exit(1)