Skip to content

Commit

Permalink
[fix] Update related config when template default_values are changed
Browse files Browse the repository at this point in the history
The config related to a template were not being flagged for update
when default_values were changed.
Moreover, during debugging, I found out that in some cases
the action was being performed even if there were no real changes,
the only difference being the new value was an instance of dict
while the value loaded from the model is an instance of OrderedDict.

This change fixes both issues.
  • Loading branch information
nemesifier committed Mar 16, 2021
1 parent 460c86c commit 1105698
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
17 changes: 15 additions & 2 deletions openwisp_controller/config/base/template.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import json
from collections import OrderedDict
from copy import copy

Expand Down Expand Up @@ -112,8 +113,10 @@ def save(self, *args, **kwargs):
update_related_config_status = False
if not self._state.adding:
current = self.__class__.objects.get(pk=self.pk)
for attr in ['backend', 'config']:
if getattr(self, attr) != getattr(current, attr):
for attr in ['backend', 'config', 'default_values']:
new_value = _get_value_for_comparison(getattr(self, attr))
current_value = _get_value_for_comparison(getattr(current, attr))
if new_value != current_value:
update_related_config_status = True
break
# save current changes
Expand Down Expand Up @@ -209,3 +212,13 @@ def __get_clone_name(self):
# It's allowed to be blank because VPN client templates can be
# automatically generated via the netjsonconfig library if left empty.
AbstractTemplate._meta.get_field('config').blank = True


def _get_value_for_comparison(value):
"""
if value is a nested OrderedDict, convert it to dict
so two simple dicts can be compared
"""
if not isinstance(value, OrderedDict):
return value
return json.loads(json.dumps(value))
18 changes: 17 additions & 1 deletion openwisp_controller/config/tests/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,12 +657,28 @@ def test_task_called(self, mocked_task):
conf.set_status_applied()
mocked_task.assert_not_called()

with self.subTest('task is called when template is assigned to conf'):
with self.subTest('task is called when template conf is changed'):
template.config['interfaces'][0]['name'] = 'eth1'
template.full_clean()
template.save()
mocked_task.assert_called_with(template.pk)

mocked_task.reset_mock()

with self.subTest('task is called when template default_values are changed'):
template.refresh_from_db()
template.default_values = {'a': 'a'}
template.full_clean()
template.save()
mocked_task.assert_called_with(template.pk)

mocked_task.reset_mock()

with self.subTest('task is not called when there are no changes'):
template.full_clean()
template.save()
mocked_task.assert_not_called()

@mock.patch.object(task_logger, 'warning')
def test_task_failure(self, mocked_warning):
update_template_related_config_status.delay(uuid.uuid4())
Expand Down

0 comments on commit 1105698

Please sign in to comment.