Skip to content

Commit 5b5c763

Browse files
pandafypandafy
committed
[req-change] Added cache key to prevent multiple config_modifed
1 parent 80fe472 commit 5b5c763

File tree

5 files changed

+306
-10
lines changed

5 files changed

+306
-10
lines changed

‎openwisp_controller/config/base/config.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from collections import defaultdict
55

66
from cache_memoize import cache_memoize
7+
from django.core.cache import cache
78
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
89
from django.db import models, transaction
910
from django.utils.translation import gettext_lazy as _
@@ -110,6 +111,7 @@ class AbstractConfig(ChecksumCacheMixin, BaseConfig):
110111
)
111112

112113
_CHECKSUM_CACHE_TIMEOUT = ChecksumCacheMixin._CHECKSUM_CACHE_TIMEOUT
114+
_CONFIG_MODIFIED_TIMEOUT = 3
113115
_config_context_functions = list()
114116
_old_backend = None
115117

@@ -679,11 +681,27 @@ def _update_checksum_db(self, new_checksum=None):
679681
new_checksum = new_checksum or self.checksum
680682
self._meta.model.objects.filter(pk=self.pk).update(checksum_db=new_checksum)
681683

684+
@property
685+
def _config_modified_timeout_cache_key(self):
686+
return f"config_modified_timeout_{self.pk}"
687+
688+
def _set_config_modified_timeout_cache(self):
689+
cache.set(
690+
self._config_modified_timeout_cache_key,
691+
True,
692+
timeout=self._CONFIG_MODIFIED_TIMEOUT,
693+
)
694+
695+
def _delete_config_modified_timeout_cache(self):
696+
cache.delete(self._config_modified_timeout_cache_key)
697+
682698
def _send_config_modified_signal(self, action):
683699
"""
684700
Emits ``config_modified`` signal.
685701
Called also by Template when templates of a device are modified
686702
"""
703+
if cache.get(self._config_modified_timeout_cache_key) is not None:
704+
return
687705
assert action in [
688706
"config_changed",
689707
"related_template_changed",
@@ -698,6 +716,11 @@ def _send_config_modified_signal(self, action):
698716
config=self,
699717
device=self.device,
700718
)
719+
cache.set(
720+
self._config_modified_timeout_cache_key,
721+
True,
722+
timeout=self._CONFIG_MODIFIED_TIMEOUT,
723+
)
701724

702725
def _send_config_deactivating_signal(self):
703726
"""

‎openwisp_controller/config/tests/test_admin.py‎

Lines changed: 173 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,47 @@
5454
Group = load_model("openwisp_users", "Group")
5555

5656

57+
class TestDeviceAdminMixin:
58+
_device_params = {
59+
"name": "test-device",
60+
"hardware_id": "1234",
61+
"mac_address": CreateConfigTemplateMixin.TEST_MAC_ADDRESS,
62+
"key": CreateConfigTemplateMixin.TEST_KEY,
63+
"model": "",
64+
"os": "",
65+
"notes": "",
66+
"config-0-id": "",
67+
"config-0-device": "",
68+
"config-0-backend": "netjsonconfig.OpenWrt",
69+
"config-0-templates": "",
70+
"config-0-config": json.dumps({}),
71+
"config-0-context": "",
72+
"config-TOTAL_FORMS": 1,
73+
"config-INITIAL_FORMS": 0,
74+
"config-MIN_NUM_FORMS": 0,
75+
"config-MAX_NUM_FORMS": 1,
76+
# openwisp_controller.connection
77+
"deviceconnection_set-TOTAL_FORMS": 0,
78+
"deviceconnection_set-INITIAL_FORMS": 0,
79+
"deviceconnection_set-MIN_NUM_FORMS": 0,
80+
"deviceconnection_set-MAX_NUM_FORMS": 1000,
81+
"command_set-TOTAL_FORMS": 0,
82+
"command_set-INITIAL_FORMS": 0,
83+
"command_set-MIN_NUM_FORMS": 0,
84+
"command_set-MAX_NUM_FORMS": 1000,
85+
}
86+
# WARNING - WATCHOUT
87+
# this class attribute is changed dynamically
88+
# by other apps which add inlines to DeviceAdmin
89+
_additional_params = {}
90+
91+
def _get_device_params(self, org):
92+
p = self._device_params.copy()
93+
p.update(self._additional_params)
94+
p["organization"] = org.pk
95+
return p
96+
97+
5798
class TestImportExportMixin:
5899
"""
59100
Reused in OpenWISP Monitoring
@@ -237,6 +278,7 @@ class TestAdmin(
237278
CreateDeviceGroupMixin,
238279
CreateConfigTemplateMixin,
239280
TestVpnX509Mixin,
281+
TestDeviceAdminMixin,
240282
TestAdminMixin,
241283
TestCase,
242284
):
@@ -278,10 +320,6 @@ class TestAdmin(
278320
"command_set-MIN_NUM_FORMS": 0,
279321
"command_set-MAX_NUM_FORMS": 1000,
280322
}
281-
# WARNING - WATCHOUT
282-
# this class attribute is changed dynamically
283-
# by other apps which add inlines to DeviceAdmin
284-
_additional_params = {}
285323

286324
def setUp(self):
287325
self.client.force_login(self._get_admin())
@@ -291,12 +329,6 @@ def tearDownClass(cls):
291329
super().tearDownClass()
292330
devnull.close()
293331

294-
def _get_device_params(self, org):
295-
p = self._device_params.copy()
296-
p.update(self._additional_params)
297-
p["organization"] = org.pk
298-
return p
299-
300332
def test_device_and_template_different_organization(self):
301333
org1 = self._get_org()
302334
template = self._create_template(organization=org1)
@@ -2251,6 +2283,7 @@ def test_templates_fetch_queries_10(self):
22512283
class TestTransactionAdmin(
22522284
CreateConfigTemplateMixin,
22532285
TestAdminMixin,
2286+
TestDeviceAdminMixin,
22542287
TransactionTestCase,
22552288
):
22562289
app_label = "config"
@@ -2569,6 +2602,136 @@ def test_restoring_template_sends_config_modified(self):
25692602
self.assertEqual(config.status, "modified")
25702603
self.assertNotEqual(config.checksum, config_checksum)
25712604

2605+
def test_config_modified_signal(self):
2606+
"""
2607+
Verifies multiple config_modified signal is not send for
2608+
a single change
2609+
"""
2610+
template1 = self._create_template(
2611+
default_values={"ssid": "OpenWISP"}, default=True
2612+
)
2613+
template2 = self._create_template(
2614+
name="template2",
2615+
config={"interfaces": [{"name": "{{ ifname }}", "type": "ethernet"}]},
2616+
default_values={"ifname": "eth1"},
2617+
default=True,
2618+
)
2619+
path = reverse(f"admin:{self.app_label}_device_add")
2620+
params = self._get_device_params(org=self._get_org())
2621+
params.update({"config-0-templates": f"{template1.pk},{template2.pk}"})
2622+
response = self.client.post(path, data=params, follow=True)
2623+
self.assertEqual(response.status_code, 200)
2624+
2625+
config = Device.objects.get(name=params["name"]).config
2626+
self.assertEqual(config.templates.count(), 2)
2627+
path = reverse(f"admin:{self.app_label}_device_change", args=[config.device_id])
2628+
params.update(
2629+
{
2630+
"config-0-id": str(config.pk),
2631+
"config-0-device": str(config.device_id),
2632+
"config-INITIAL_FORMS": 1,
2633+
"_continue": True,
2634+
}
2635+
)
2636+
2637+
config._delete_config_modified_timeout_cache()
2638+
with self.subTest(
2639+
"Updating the unused context variable does not send config_modified signal"
2640+
):
2641+
with patch(
2642+
"openwisp_controller.config.signals.config_modified.send"
2643+
) as mocked_signal:
2644+
params.update(
2645+
{
2646+
"config-0-context": json.dumps(
2647+
{"ssid": "Updated", "ifname": "eth1"}
2648+
)
2649+
}
2650+
)
2651+
response = self.client.post(path, data=params, follow=True)
2652+
self.assertEqual(response.status_code, 200)
2653+
mocked_signal.assert_not_called()
2654+
config.refresh_from_db()
2655+
self.assertEqual(config.context["ssid"], "Updated")
2656+
2657+
config._delete_config_modified_timeout_cache()
2658+
with self.subTest(
2659+
"Changing used context variable sends config_modified signal"
2660+
):
2661+
with patch(
2662+
"openwisp_controller.config.signals.config_modified.send"
2663+
) as mocked_signal:
2664+
params.update(
2665+
{
2666+
"config-0-context": json.dumps(
2667+
{"ssid": "Updated", "ifname": "eth2"}
2668+
)
2669+
}
2670+
)
2671+
response = self.client.post(path, data=params, follow=True)
2672+
self.assertEqual(response.status_code, 200)
2673+
2674+
config.refresh_from_db()
2675+
self.assertEqual(
2676+
config.context["ssid"],
2677+
"Updated",
2678+
)
2679+
self.assertEqual(config.status, "modified")
2680+
mocked_signal.assert_called_once()
2681+
2682+
config._delete_config_modified_timeout_cache()
2683+
with self.subTest("Changing device configuration sends config_modified signal"):
2684+
with patch(
2685+
"openwisp_controller.config.signals.config_modified.send"
2686+
) as mocked_signal:
2687+
params.update(
2688+
{
2689+
"config-0-config": json.dumps(
2690+
{"interfaces": [{"name": "eth5", "type": "ethernet"}]}
2691+
)
2692+
}
2693+
)
2694+
response = self.client.post(path, data=params, follow=True)
2695+
self.assertEqual(response.status_code, 200)
2696+
config.refresh_from_db()
2697+
self.assertEqual(
2698+
config.config["interfaces"][0]["name"],
2699+
"eth5",
2700+
)
2701+
self.assertEqual(config.status, "modified")
2702+
mocked_signal.assert_called_once()
2703+
2704+
config._delete_config_modified_timeout_cache()
2705+
with self.subTest("Changing applied template sends config_modified signal"):
2706+
with patch(
2707+
"openwisp_controller.config.signals.config_modified.send"
2708+
) as mocked_signal:
2709+
response = self.client.post(
2710+
reverse(
2711+
f"admin:{self.app_label}_template_change", args=[template2.pk]
2712+
),
2713+
data={
2714+
"name": template2.name,
2715+
"organization": "",
2716+
"type": template2.type,
2717+
"backend": template2.backend,
2718+
"config": json.dumps(template2.config),
2719+
"default_values": json.dumps(
2720+
{
2721+
"ifname": "eth3",
2722+
}
2723+
),
2724+
"required": False,
2725+
"default": True,
2726+
"_continue": True,
2727+
},
2728+
follow=True,
2729+
)
2730+
self.assertEqual(response.status_code, 200)
2731+
template2.refresh_from_db()
2732+
self.assertEqual(template2.default_values["ifname"], "eth3")
2733+
mocked_signal.assert_called_once()
2734+
25722735

25732736
class TestDeviceGroupAdmin(
25742737
CreateDeviceGroupMixin,

‎openwisp_controller/config/tests/test_api.py‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from datetime import timedelta
2+
from unittest.mock import patch
23

34
from django.contrib.auth.models import Permission
45
from django.test import TestCase
@@ -1667,3 +1668,104 @@ def test_device_patch_with_templates_of_same_org(self):
16671668
self.assertEqual(r.status_code, 200)
16681669
self.assertEqual(d1.config.templates.count(), 2)
16691670
self.assertEqual(r.data["config"]["templates"], [t1.id, t2.id])
1671+
1672+
def test_config_modified_signal(self):
1673+
"""
1674+
Verifies that config_modified signal is sent only once.
1675+
"""
1676+
template1 = self._create_template(
1677+
default_values={"ssid": "OpenWISP"},
1678+
)
1679+
template2 = self._create_template(
1680+
name="template2",
1681+
config={"interfaces": [{"name": "{{ ifname }}", "type": "ethernet"}]},
1682+
default_values={"ifname": "eth1"},
1683+
)
1684+
data = self._device_data
1685+
org = self._get_org()
1686+
data["organization"] = org.pk
1687+
data["config"]["templates"] = [str(template1.pk), str(template2.pk)]
1688+
response = self.client.post(
1689+
reverse("config_api:device_list"), data, content_type="application/json"
1690+
)
1691+
self.assertEqual(response.status_code, 201)
1692+
self.assertEqual(Device.objects.count(), 1)
1693+
config = Device.objects.first().config
1694+
self.assertEqual(config.templates.count(), 2)
1695+
device_detail = reverse("config_api:device_detail", args=[config.device_id])
1696+
1697+
config._delete_config_modified_timeout_cache()
1698+
with self.subTest(
1699+
"Updating the unused context variable does not send config_modified signal"
1700+
):
1701+
with patch(
1702+
"openwisp_controller.config.signals.config_modified.send"
1703+
) as mocked_signal:
1704+
data["config"]["context"]["ssid"] = "Updated"
1705+
response = self.client.patch(
1706+
device_detail,
1707+
{"config": data["config"]},
1708+
content_type="application/json",
1709+
)
1710+
self.assertEqual(response.status_code, 200)
1711+
config.refresh_from_db()
1712+
self.assertEqual(config.context["ssid"], "Updated")
1713+
mocked_signal.assert_not_called()
1714+
1715+
config._delete_config_modified_timeout_cache()
1716+
with self.subTest(
1717+
"Changing used context variable sends config_modified signal"
1718+
):
1719+
with patch(
1720+
"openwisp_controller.config.signals.config_modified.send"
1721+
) as mocked_signal:
1722+
data["config"]["context"] = {"ssid": "Updated", "ifname": "eth2"}
1723+
response = self.client.patch(
1724+
device_detail,
1725+
{"config": data["config"]},
1726+
content_type="application/json",
1727+
)
1728+
self.assertEqual(response.status_code, 200)
1729+
config.refresh_from_db()
1730+
self.assertEqual(
1731+
config.context["ssid"],
1732+
"Updated",
1733+
)
1734+
self.assertEqual(config.status, "modified")
1735+
mocked_signal.assert_called_once()
1736+
1737+
config._delete_config_modified_timeout_cache()
1738+
with self.subTest("Changing device configuration sends config_modified signal"):
1739+
with patch(
1740+
"openwisp_controller.config.signals.config_modified.send"
1741+
) as mocked_signal:
1742+
data["config"]["config"]["interfaces"] = [
1743+
{"name": "eth5", "type": "ethernet"}
1744+
]
1745+
response = self.client.patch(
1746+
device_detail, data, content_type="application/json"
1747+
)
1748+
self.assertEqual(response.status_code, 200)
1749+
config.refresh_from_db()
1750+
self.assertEqual(
1751+
config.config["interfaces"][0]["name"],
1752+
"eth5",
1753+
)
1754+
self.assertEqual(config.status, "modified")
1755+
mocked_signal.assert_called_once()
1756+
1757+
config._delete_config_modified_timeout_cache()
1758+
with self.subTest("Changing applied template sends config_modified signal"):
1759+
with patch(
1760+
"openwisp_controller.config.signals.config_modified.send"
1761+
) as mocked_signal:
1762+
data = {"default_values": {"ifname": "eth3"}}
1763+
response = self.client.patch(
1764+
reverse("config_api:template_detail", args=[template2.pk]),
1765+
data,
1766+
content_type="application/json",
1767+
)
1768+
self.assertEqual(response.status_code, 200)
1769+
template2.refresh_from_db()
1770+
self.assertEqual(template2.default_values["ifname"], "eth3")
1771+
mocked_signal.assert_called_once()

‎openwisp_controller/config/tests/test_config.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,7 @@ def test_config_modified_sent(self):
818818
)
819819
self.assertEqual(c.status, "modified")
820820

821+
c._delete_config_modified_timeout_cache()
821822
with catch_signal(config_modified) as handler:
822823
c.config = {"general": {"description": "changed again"}}
823824
c.full_clean()

0 commit comments

Comments
 (0)