Skip to content

Commit d1c2da8

Browse files
committed
[feature] Implement VPN cache invalidation using _initial pattern
- Use getattr for version-safe _initial_context access - Add behavioral tests covering all invalidation scenarios Fixes #1098
1 parent 58e1527 commit d1c2da8

2 files changed

Lines changed: 26 additions & 71 deletions

File tree

openwisp_controller/config/base/multitenancy.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class Meta:
4949

5050
def __init__(self, *args, **kwargs):
5151
super().__init__(*args, **kwargs)
52-
self._initial_context = self.context
52+
self._initial_context = deepcopy(self.context)
5353

5454
def __str__(self):
5555
return self.organization.name
@@ -62,15 +62,17 @@ def save(
6262
):
6363
context_changed = False
6464
if not self._state.adding:
65-
context_changed = self._initial_context != self.context
65+
initial_context = getattr(self, "_initial_context", None)
66+
if initial_context is not None:
67+
context_changed = initial_context != self.context
6668
super().save(force_insert, force_update, using, update_fields)
6769
if context_changed and self.organization.is_active:
6870
transaction.on_commit(
6971
lambda: tasks.invalidate_organization_vpn_cache.delay(
7072
str(self.organization_id)
7173
)
7274
)
73-
self._initial_context = self.context
75+
self._initial_context = deepcopy(self.context)
7476

7577

7678
class AbstractOrganizationLimits(models.Model):

openwisp_controller/config/tests/test_handlers.py

Lines changed: 21 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,10 @@ def test_organization_disabled_handler(self, mocked_task):
1313
with self.subTest("Test task not executed on creating active orgs"):
1414
org = self._create_org()
1515
mocked_task.assert_not_called()
16-
1716
with self.subTest("Test task executed on changing active to inactive org"):
1817
org.is_active = False
1918
org.save()
2019
mocked_task.assert_called_once()
21-
2220
with self.subTest("Test task not executed on changing inactive to active org"):
2321
mocked_task.reset_mock()
2422
inactive_org = self._create_org(is_active=False)
@@ -37,80 +35,35 @@ def _get_org_config_settings(self, org=None):
3735
return org.config_settings
3836

3937
@patch.object(tasks.invalidate_organization_vpn_cache, "delay")
40-
def test_vpn_cache_invalidation_on_context_change(self, mocked_task):
38+
def test_vpn_cache_invalidated_on_context_change(self, mocked_task):
4139
"""Test VPN cache invalidation when context changes"""
4240
config_settings = self._get_org_config_settings()
41+
config_settings.context = {"new": "context"}
42+
config_settings.save()
43+
mocked_task.assert_called_once_with(str(config_settings.organization_id))
4344

44-
with self.subTest("Test no invalidation on creation"):
45-
mocked_task.assert_not_called()
46-
47-
with self.subTest("Test invalidation when context changes"):
48-
config_settings.context = {"new": "context"}
49-
config_settings.save()
50-
mocked_task.assert_called_once_with(str(config_settings.organization_id))
51-
52-
with self.subTest("Test no invalidation when context unchanged"):
53-
mocked_task.reset_mock()
54-
config_settings.registration_enabled = False
55-
config_settings.save()
56-
mocked_task.assert_not_called()
45+
@patch.object(tasks.invalidate_organization_vpn_cache, "delay")
46+
def test_no_cache_invalidation_on_create(self, mocked_task):
47+
"""Test no VPN cache invalidation on object creation"""
48+
self._get_org_config_settings()
49+
mocked_task.assert_not_called()
5750

5851
@patch.object(tasks.invalidate_organization_vpn_cache, "delay")
59-
def test_no_vpn_cache_invalidation_for_inactive_org(self, mocked_task):
52+
def test_no_cache_invalidation_for_inactive_org(self, mocked_task):
6053
"""Test no VPN cache invalidation for inactive organizations"""
6154
inactive_org = self._create_org(is_active=False)
6255
config_settings = inactive_org.config_settings
63-
64-
with self.subTest("Test no invalidation for inactive org context change"):
65-
config_settings.context = {"new": "context"}
66-
config_settings.save()
67-
mocked_task.assert_not_called()
68-
69-
def test_initial_context_pattern_implementation(self):
70-
"""Test that _initial_context follows the established pattern"""
71-
config_settings = self._get_org_config_settings()
72-
73-
with self.subTest("Test _initial_context is set on init"):
74-
self.assertEqual(config_settings._initial_context, config_settings.context)
75-
76-
with self.subTest("Test _initial_context updates after save"):
77-
new_context = {"updated": "context"}
78-
config_settings.context = new_context
79-
config_settings.save()
80-
self.assertEqual(config_settings._initial_context, new_context)
81-
82-
@patch.object(tasks.invalidate_organization_vpn_cache, "delay")
83-
def test_feature_fails_when_removed(self, mocked_task):
84-
"""Test that fails with clear error when feature code is removed"""
85-
config_settings = self._get_org_config_settings()
86-
87-
# Simulate removing the _initial_context pattern
88-
def broken_init(self, *args, **kwargs):
89-
super(type(config_settings), self).__init__(*args, **kwargs)
90-
# Missing: self._initial_context = self.context
91-
92-
with patch.object(type(config_settings), "__init__", broken_init):
93-
new_config = type(config_settings)(organization=self._create_org())
94-
new_config.context = {"test": "context"}
95-
with self.assertRaises(
96-
AttributeError, msg="Feature code removed - _initial_context not set"
97-
):
98-
new_config.save()
56+
config_settings.context = {"new": "context"}
57+
config_settings.save()
58+
mocked_task.assert_not_called()
9959

10060
@patch.object(tasks.invalidate_organization_vpn_cache, "delay")
101-
def test_feature_fails_when_bugged(self, mocked_task):
102-
"""Test that fails with clear error when feature code is bugged"""
61+
def test_no_cache_invalidation_if_context_unchanged(self, mocked_task):
62+
"""Test no VPN cache invalidation when context is unchanged"""
10363
config_settings = self._get_org_config_settings()
104-
105-
# Simulate buggy implementation - always trigger invalidation
106-
def buggy_save(self, *args, **kwargs):
107-
super(type(config_settings), self).save(*args, **kwargs)
108-
# Bug: always invalidate regardless of context change
109-
if hasattr(self, "organization") and self.organization.is_active:
110-
tasks.invalidate_organization_vpn_cache.delay(str(self.organization_id))
111-
112-
with patch.object(type(config_settings), "save", buggy_save):
113-
# This should not trigger invalidation but buggy code will
114-
config_settings.registration_enabled = False
115-
config_settings.save()
116-
mocked_task.assert_called_once() # This proves the bug exists
64+
original_context = config_settings.context
65+
config_settings.registration_enabled = False
66+
config_settings.save()
67+
mocked_task.assert_not_called()
68+
# Verify context actually didn't change
69+
self.assertEqual(config_settings.context, original_context)

0 commit comments

Comments
 (0)