Skip to content

Commit

Permalink
ovsdb-idl: Fix bugs in Python IDL partial set and map.
Browse files Browse the repository at this point in the history
This patch fixes a couple of bugs in commit a59912a
(python: add support for partial map and partial set updates)
and reverses a simplication added in commit 884d9ba
(Simplify partial map Py3 IDL test) to make the Python3 test
cases passes.

The following changes have been made:

1. Allow multiple map updates on the same column in a transaction.
2. Partial map Py3 IDL test can now support multiple elements.
3. SetAttr overrides pre-existing insert and remove updates.
4. addvalue/delvalue contains unique elements

Signed-off-by: Amitabha Biswas <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
azbiswas authored and blp committed Aug 15, 2016
1 parent ae59d13 commit 330b9c9
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 100 deletions.
118 changes: 57 additions & 61 deletions python/ovs/db/idl.py
Original file line number Diff line number Diff line change
Expand Up @@ -832,72 +832,68 @@ def __setattr__(self, column_name, value):
return
self._idl.txn._write(self, column, datum)

def _init_mutations_if_needed(self):
if '_inserts' not in self._mutations.keys():
self._mutations['_inserts'] = {}
if '_removes' not in self._mutations.keys():
self._mutations['_removes'] = {}
return

def addvalue(self, column_name, key):
self._idl.txn._txn_rows[self.uuid] = self
self._init_mutations_if_needed()
if column_name in self._data.keys():
if column_name not in self._mutations['_inserts'].keys():
self._mutations['_inserts'][column_name] = []
self._mutations['_inserts'][column_name].append(key)
column = self._table.columns[column_name]
try:
data.Datum.from_python(column.type, key, _row_to_uuid)
except error.Error as e:
# XXX rate-limit
vlog.err("attempting to write bad value to column %s (%s)"
% (column_name, e))
return
inserts = self._mutations.setdefault('_inserts', {})
column_value = inserts.setdefault(column_name, set())
column_value.add(key)

def delvalue(self, column_name, key):
self._idl.txn._txn_rows[self.uuid] = self
self._init_mutations_if_needed()
if column_name in self._data.keys():
if column_name not in self._mutations['_removes'].keys():
self._mutations['_removes'][column_name] = []
self._mutations['_removes'][column_name].append(key)
column = self._table.columns[column_name]
try:
data.Datum.from_python(column.type, key, _row_to_uuid)
except error.Error as e:
# XXX rate-limit
vlog.err("attempting to delete bad value from column %s (%s)"
% (column_name, e))
return
removes = self._mutations.setdefault('_removes', {})
column_value = removes.setdefault(column_name, set())
column_value.add(key)

def setkey(self, column_name, key, value):
self._idl.txn._txn_rows[self.uuid] = self
column = self._table.columns[column_name]
try:
datum = data.Datum.from_python(column.type, {key: value},
_row_to_uuid)
data.Datum.from_python(column.type, {key: value}, _row_to_uuid)
except error.Error as e:
# XXX rate-limit
vlog.err("attempting to write bad value to column %s (%s)"
% (column_name, e))
return
self._init_mutations_if_needed()
if column_name in self._data.keys():
if column_name not in self._mutations['_removes'].keys():
self._mutations['_removes'][column_name] = []
self._mutations['_removes'][column_name].append(key)
if self._mutations['_inserts'] is None:
self._mutations['_inserts'] = {}
self._mutations['_inserts'][column_name] = datum

def delkey(self, column_name, key):
if column_name in self._data:
# Remove existing key/value before updating.
removes = self._mutations.setdefault('_removes', {})
column_value = removes.setdefault(column_name, set())
column_value.add(key)
inserts = self._mutations.setdefault('_inserts', {})
column_value = inserts.setdefault(column_name, {})
column_value[key] = value

def delkey(self, column_name, key, value=None):
self._idl.txn._txn_rows[self.uuid] = self
self._init_mutations_if_needed()
if column_name not in self._mutations['_removes'].keys():
self._mutations['_removes'][column_name] = []
self._mutations['_removes'][column_name].append(key)

def delkey(self, column_name, key, value):
self._idl.txn._txn_rows[self.uuid] = self
try:
old_value = data.Datum.to_python(self._data[column_name],
_uuid_to_row)
except error.Error:
return
if key not in old_value.keys():
return
if old_value[key] != value:
return

self._init_mutations_if_needed()
if column_name not in self._mutations['_removes'].keys():
self._mutations['_removes'][column_name] = []
self._mutations['_removes'][column_name].append(key)
if value:
try:
old_value = data.Datum.to_python(self._data[column_name],
_uuid_to_row)
except error.Error:
return
if key not in old_value:
return
if old_value[key] != value:
return
removes = self._mutations.setdefault('_removes', {})
column_value = removes.setdefault(column_name, set())
column_value.add(key)
return

@classmethod
Expand Down Expand Up @@ -1282,7 +1278,7 @@ def commit(self):
column = row._table.columns[col]
if column.type.is_map():
opdat = ["set"]
opdat.append(dat)
opdat.append(list(dat))
else:
opdat = ["set"]
inner_opdat = []
Expand All @@ -1299,15 +1295,17 @@ def commit(self):
op["mutations"].append(mutation)
addop = True
if '_inserts' in row._mutations.keys():
for col, dat in six.iteritems(row._mutations['_inserts']):
for col, val in six.iteritems(row._mutations['_inserts']):
column = row._table.columns[col]
if column.type.is_map():
opdat = ["map"]
opdat.append(dat.as_list())
datum = data.Datum.from_python(column.type, val,
_row_to_uuid)
opdat.append(datum.as_list())
else:
opdat = ["set"]
inner_opdat = []
for ele in dat:
for ele in val:
try:
datum = data.Datum.from_python(column.type,
ele, _row_to_uuid)
Expand Down Expand Up @@ -1471,13 +1469,11 @@ def _write(self, row, column, datum):
return

txn._txn_rows[row.uuid] = row
if '_inserts' in row._mutations.keys():
if column.name in row._mutations['_inserts']:
row._mutations['_inserts'][column.name] = datum.copy()
else:
row._changes[column.name] = datum.copy()
else:
row._changes[column.name] = datum.copy()
if '_inserts' in row._mutations:
row._mutations['_inserts'].pop(column.name, None)
if '_removes' in row._mutations:
row._mutations['_removes'].pop(column.name, None)
row._changes[column.name] = datum.copy()

def insert(self, table, new_uuid=None):
"""Inserts and returns a new row in 'table', which must be one of the
Expand Down
20 changes: 12 additions & 8 deletions tests/ovsdb-idl.at
Original file line number Diff line number Diff line change
Expand Up @@ -1110,15 +1110,17 @@ OVSDB_CHECK_IDL_PARTIAL_UPDATE_MAP_COLUMN([map, simple2 idl-partial-update-map-c

OVSDB_CHECK_IDL_PY([partial-map idl],
[['["idltest", {"op":"insert", "table":"simple2",
"row":{"name":"myString1","smap":["map",[["key1","value1"]]]} }]']
"row":{"name":"myString1","smap":["map",[["key1","value1"],["key2","value2"]]]} }]']
],
[?simple2:name,smap,imap 'partialmapinsertelement' 'partialmapdelelement'],
[[000: name=myString1 smap={key1: value1} imap={}
[?simple2:name,smap,imap 'partialmapinsertelement' 'partialmapinsertmultipleelements' 'partialmapdelelements'],
[[000: name=myString1 smap=[(key1 value1) (key2 value2)] imap=[]
001: commit, status=success
002: name=String2 smap={key1: myList1} imap={3: myids2}
002: name=String2 smap=[(key1 myList1) (key2 value2)] imap=[(3 myids2)]
003: commit, status=success
004: name=String2 smap={} imap={3: myids2}
005: done
004: name=String2 smap=[(key1 myList1) (key2 myList2) (key3 myList3)] imap=[(3 myids2)]
005: commit, status=success
006: name=String2 smap=[(key2 myList2)] imap=[(3 myids2)]
007: done
]])

m4_define([OVSDB_CHECK_IDL_PARTIAL_UPDATE_SET_COLUMN],
Expand Down Expand Up @@ -1161,7 +1163,7 @@ OVSDB_CHECK_IDL_PY([partial-set idl],
[['["idltest", {"op":"insert", "table":"simple3",
"row":{"name":"mySet1","uset":["set", [[ "uuid", "0005b872-f9e5-43be-ae02-3184b9680e75" ], [ "uuid", "000d2f6a-76af-412f-b59d-e7bcd3e84eff" ]]]} }, {"op":"insert", "table":"simple4", "row":{"name":"seed"}}]']
],
['partialrenamesetadd' 'partialsetadd2' 'partialsetdel' 'partialsetref'],
['partialrenamesetadd' 'partialduplicateadd' 'partialsetdel' 'partialsetref' 'partialsetoverrideops'],
[[000: name=mySet1 uset=[<0> <1>]
001: commit, status=success
002: name=String2 uset=[<0> <1> <2>]
Expand All @@ -1171,7 +1173,9 @@ OVSDB_CHECK_IDL_PY([partial-set idl],
006: name=String2 uset=[<0> <1> <3>]
007: commit, status=success
008: name=String2 uset=[<0> <1> <3>]
009: done
009: commit, status=success
010: name=String2 uset=[<3>]
011: done
]])

m4_define([OVSDB_CHECK_IDL_NOTIFY_PY],
Expand Down
63 changes: 32 additions & 31 deletions tests/test-ovsdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,15 @@ def do_parse_schema(schema_string):
print(ovs.json.to_string(schema.to_json(), sort_keys=True))


def get_simple_table_printable_row(row):
simple_columns = ["i", "r", "b", "s", "u", "ia",
"ra", "ba", "sa", "ua", "uuid"]
def get_simple_printable_row_string(row, columns):
s = ""
for column in simple_columns:
for column in columns:
if hasattr(row, column) and not (type(getattr(row, column))
is ovs.db.data.Atom):
s += "%s=%s " % (column, getattr(row, column))
value = getattr(row, column)
if isinstance(value, dict):
value = sorted(value.items())
s += "%s=%s " % (column, value)
s = s.strip()
s = re.sub('""|,|u?\'', "", s)
s = re.sub('UUID\(([^)]+)\)', r'\1', s)
Expand All @@ -166,36 +167,20 @@ def get_simple_table_printable_row(row):
return s


def get_simple_table_printable_row(row):
simple_columns = ["i", "r", "b", "s", "u", "ia",
"ra", "ba", "sa", "ua", "uuid"]
return get_simple_printable_row_string(row, simple_columns)


def get_simple2_table_printable_row(row):
simple2_columns = ["name", "smap", "imap"]
s = ""
for column in simple2_columns:
if hasattr(row, column) and not (type(getattr(row, column))
is ovs.db.data.Atom):
s += "%s=%s " % (column, getattr(row, column))
s = s.strip()
s = re.sub('""|,|u?\'', "", s)
s = re.sub('UUID\(([^)]+)\)', r'\1', s)
s = re.sub('False', 'false', s)
s = re.sub('True', 'true', s)
s = re.sub(r'(ba)=([^[][^ ]*) ', r'\1=[\2] ', s)
return s
return get_simple_printable_row_string(row, simple2_columns)


def get_simple3_table_printable_row(row):
simple3_columns = ["name", "uset"]
s = ""
for column in simple3_columns:
if hasattr(row, column) and not (type(getattr(row, column))
is ovs.db.data.Atom):
s += "%s=%s " % (column, getattr(row, column))
s = s.strip()
s = re.sub('""|,|u?\'', "", s)
s = re.sub('UUID\(([^)]+)\)', r'\1', s)
s = re.sub('False', 'false', s)
s = re.sub('True', 'true', s)
s = re.sub(r'(ba)=([^[][^ ]*) ', r'\1=[\2] ', s)
return s
return get_simple_printable_row_string(row, simple3_columns)


def print_idl(idl, step):
Expand Down Expand Up @@ -448,18 +433,26 @@ def notify(event, row, updates=None):
row.setkey('smap', 'key1', 'myList1')
row.setkey('imap', 3, 'myids2')
row.__setattr__('name', 'String2')
elif name == 'partialmapdelelement':
elif name == 'partialmapinsertmultipleelements':
row = idltest_find_simple2(idl, 'String2')
row.setkey('smap', 'key2', 'myList2')
row.setkey('smap', 'key3', 'myList3')
elif name == 'partialmapdelelements':
row = idltest_find_simple2(idl, 'String2')
row.delkey('smap', 'key1', 'myList1')
row.delkey('smap', 'key2', 'wrongvalue')
row.delkey('smap', 'key3')
elif name == 'partialrenamesetadd':
row = idltest_find_simple3(idl, 'mySet1')
row.addvalue('uset',
uuid.UUID("001e43d2-dd3f-4616-ab6a-83a490bb0991"))
row.__setattr__('name', 'String2')
elif name == 'partialsetadd2':
elif name == 'partialduplicateadd':
row = idltest_find_simple3(idl, 'String2')
row.addvalue('uset',
uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
row.addvalue('uset',
uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
elif name == 'partialsetdel':
row = idltest_find_simple3(idl, 'String2')
row.delvalue('uset',
Expand All @@ -469,6 +462,14 @@ def notify(event, row, updates=None):
new_row.__setattr__('name', 'test')
row = idltest_find_simple3(idl, 'String2')
row.addvalue('uref', new_row.uuid)
elif name == 'partialsetoverrideops':
row = idltest_find_simple3(idl, 'String2')
row.addvalue('uset',
uuid.UUID("579e978d-776c-4f19-a225-268e5890e670"))
row.delvalue('uset',
uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
row.__setattr__('uset',
[uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8")])
else:
sys.stderr.write("unknown command %s\n" % name)
sys.exit(1)
Expand Down

0 comments on commit 330b9c9

Please sign in to comment.