Make LocalSettingsContext more robust to priority
The relation data for for the LocalSettings context could cause the priority sorting to break if the priority key wasn't cmpable (e.g. using <, > or ==). This patch fixes the associated bug, by making the sorting extra robust and ensuring that un-cmp-able values are 'greater' (e.g. further down the list) that cmp-able values, and equal to each other. E.g. a partially ordered set. Change-Id: I6bbf7e5f81a772ffc6ea859c9ab7c05f2eb9fdc5 Closes-bug: #2023404
This commit is contained in:
Alex Kavanagh
parent
cbcb084af8
commit
e8d0ca39a1
|
|
@ -14,6 +14,7 @@
|
|||
|
||||
# vim: set ts=4:et
|
||||
|
||||
import functools
|
||||
import json
|
||||
|
||||
from charmhelpers.core.hookenv import (
|
||||
|
|
@ -382,11 +383,79 @@ class LocalSettingsContext(OSContextGenerator):
|
|||
ctxt = {
|
||||
'settings': [
|
||||
'# {0}\n{1}'.format(u, rd['local-settings'])
|
||||
for u, rd in sorted(relations,
|
||||
key=lambda r: r[1]['priority'])]
|
||||
for u, rd in sorted(
|
||||
relations,
|
||||
key=functools.cmp_to_key(self._priority_sort_cmp))]
|
||||
}
|
||||
return ctxt
|
||||
|
||||
@staticmethod
|
||||
def _priority_sort_cmp(unit_rdata_l, unit_rdata_r):
|
||||
"""Sort two replates for __call__ .
|
||||
|
||||
This is used from functools.cmp_to_key() function.
|
||||
|
||||
It compares items in the (unit, rdata) as rdata['priority'] where
|
||||
|
||||
* No 'priority' value > one that is present; technically this isn't
|
||||
possible, but is defensive here.
|
||||
* None > that something that is present.
|
||||
* types are int-ed() and attempted to be compared
|
||||
* Two identical types are cmp-ed
|
||||
* If types won't compare or can't be converted to an int, then they
|
||||
are assumed to be non comparable and return 'equal' (0)
|
||||
|
||||
:param unit_rdata_l: the (unit, relation_data) on the left.
|
||||
:type unit_data_l: Tuple[str, Any]
|
||||
:param unit_rdata_r: the (unit, relation_data) on the right.
|
||||
:type unit_data_r: Tuple[str, Any]
|
||||
:returns: a cmp return value (-1 is l<r, +1 is l>r, 0 is l==r)
|
||||
:rtype: int
|
||||
"""
|
||||
try:
|
||||
priority_l = unit_rdata_l[1].get('priority')
|
||||
if priority_l is None:
|
||||
# cmp(None, Anything) is 1
|
||||
return 1
|
||||
except IndexError:
|
||||
# cmp( <missing>, Anything) is 1: i.e. missing to end of list
|
||||
return 1
|
||||
try:
|
||||
priority_r = unit_rdata_r[1]['priority']
|
||||
if priority_r is None:
|
||||
# cmp(Anything, None) is -1
|
||||
return -1
|
||||
except IndexError:
|
||||
# cmp( Anything, <missing>) is -1: i.e. missing already at end
|
||||
return -1
|
||||
|
||||
# define a _cmp function.
|
||||
def _cmp(a, b):
|
||||
if a < b:
|
||||
return -1
|
||||
elif a > b:
|
||||
return 1
|
||||
else:
|
||||
return 0
|
||||
|
||||
# try converting to ints before doing strings, as the strings may be
|
||||
# numbers.
|
||||
try:
|
||||
return _cmp(int(priority_l), int(priority_r))
|
||||
except ValueError:
|
||||
pass
|
||||
|
||||
# if they are the same type, try comparing them.
|
||||
if type(priority_l) == type(priority_r):
|
||||
try:
|
||||
return _cmp(priority_l, priority_r)
|
||||
except TypeError:
|
||||
# types don't support comparisions
|
||||
return 0
|
||||
# different types, that won't become ints, and also don't support
|
||||
# so just say they are the same.
|
||||
return 0
|
||||
|
||||
|
||||
class WebSSOFIDServiceProviderContext(OSContextGenerator):
|
||||
interfaces = ['websso-fid-service-provider']
|
||||
|
|
|
|||
|
|
@ -1363,6 +1363,115 @@ class TestHorizonContexts(CharmTestCase):
|
|||
'# horizon-plugin/0\n'
|
||||
'FOO = True']})
|
||||
|
||||
def test_LocalSettingsContext_unusual_priority(self):
|
||||
# First, left priority missing.
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'local-settings': 'FOO = True'},
|
||||
{'priority': 60,
|
||||
'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': []})
|
||||
# First, right priority missing.
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'priority': 99,
|
||||
'local-settings': 'FOO = True'},
|
||||
{'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': []})
|
||||
# Left priority is None.
|
||||
self.relation_ids.return_value = ['plugin:0', 'plugin-too:0']
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'priority': None,
|
||||
'local-settings': 'FOO = True'},
|
||||
{'priority': 60,
|
||||
'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': ['# horizon-plugin-too/0\n'
|
||||
'BAR = False',
|
||||
'# horizon-plugin/0\n'
|
||||
'FOO = True']})
|
||||
# Right priority is None.
|
||||
self.relation_ids.return_value = ['plugin:0', 'plugin-too:0']
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'priority': 99,
|
||||
'local-settings': 'FOO = True'},
|
||||
{'priority': None,
|
||||
'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': ['# horizon-plugin/0\n'
|
||||
'FOO = True',
|
||||
'# horizon-plugin-too/0\n'
|
||||
'BAR = False']})
|
||||
# Left priority is stringy number.
|
||||
self.relation_ids.return_value = ['plugin:0', 'plugin-too:0']
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'priority': "99",
|
||||
'local-settings': 'FOO = True'},
|
||||
{'priority': 60,
|
||||
'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': ['# horizon-plugin-too/0\n'
|
||||
'BAR = False',
|
||||
'# horizon-plugin/0\n'
|
||||
'FOO = True']})
|
||||
# Right priority is stringy number.
|
||||
self.relation_ids.return_value = ['plugin:0', 'plugin-too:0']
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'priority': 99,
|
||||
'local-settings': 'FOO = True'},
|
||||
{'priority': "60",
|
||||
'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': ['# horizon-plugin-too/0\n'
|
||||
'BAR = False',
|
||||
'# horizon-plugin/0\n'
|
||||
'FOO = True']})
|
||||
# Both priorities are strings
|
||||
self.relation_ids.return_value = ['plugin:0', 'plugin-too:0']
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'priority': "99",
|
||||
'local-settings': 'FOO = True'},
|
||||
{'priority': "60",
|
||||
'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': ['# horizon-plugin-too/0\n'
|
||||
'BAR = False',
|
||||
'# horizon-plugin/0\n'
|
||||
'FOO = True']})
|
||||
# Left priority is weired json object
|
||||
self.relation_ids.return_value = ['plugin:0', 'plugin-too:0']
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'priority': "{'a': 1}",
|
||||
'local-settings': 'FOO = True'},
|
||||
{'priority': "60",
|
||||
'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': ['# horizon-plugin-too/0\n'
|
||||
'BAR = False',
|
||||
'# horizon-plugin/0\n'
|
||||
'FOO = True']})
|
||||
# right priority is weired json object
|
||||
self.relation_ids.return_value = ['plugin:0', 'plugin-too:0']
|
||||
self.related_units.side_effect = [['horizon-plugin/0'],
|
||||
['horizon-plugin-too/0']]
|
||||
self.relation_get.side_effect = [{'priority': "99",
|
||||
'local-settings': 'FOO = True'},
|
||||
{'priority': "[1,2,3]",
|
||||
'local-settings': 'BAR = False'}]
|
||||
self.assertEqual(horizon_contexts.LocalSettingsContext()(),
|
||||
{'settings': ['# horizon-plugin/0\n'
|
||||
'FOO = True',
|
||||
'# horizon-plugin-too/0\n'
|
||||
'BAR = False']})
|
||||
|
||||
def test_WebSSOFIDServiceProviderContext(self):
|
||||
def relation_ids_side_effect(rname):
|
||||
return {
|
||||
|
|
|
|||
Loading…
Reference in New Issue