Skip to content

Commit b48cac9

Browse files
committed
Fix critical logic bug in change detection
The post_save handler was comparing identical values so changes were never detected. Split into pre_save (store original) and post_save (compare & invalidate) handlers. Also added .only('id') optimization and SoftTimeLimitExceeded handling as suggested.
1 parent e77e6d3 commit b48cac9

File tree

3 files changed

+36
-16
lines changed

3 files changed

+36
-16
lines changed

openwisp_controller/config/apps.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,8 @@ def enable_cache_invalidation(self):
273273
device_cache_invalidation_handler,
274274
devicegroup_change_handler,
275275
devicegroup_delete_handler,
276-
organization_config_settings_change_handler,
276+
organization_config_settings_pre_save_handler,
277+
organization_config_settings_post_save_handler,
277278
vpn_server_change_handler,
278279
)
279280

@@ -340,8 +341,13 @@ def enable_cache_invalidation(self):
340341
sender=self.vpn_model,
341342
dispatch_uid="vpn.invalidate_checksum_cache",
342343
)
344+
pre_save.connect(
345+
organization_config_settings_pre_save_handler,
346+
sender=self.organization_config_settings_model,
347+
dispatch_uid="organization_config_settings.store_original_context",
348+
)
343349
post_save.connect(
344-
organization_config_settings_change_handler,
350+
organization_config_settings_post_save_handler,
345351
sender=self.organization_config_settings_model,
346352
dispatch_uid="organization_config_settings.invalidate_vpn_cache",
347353
)

openwisp_controller/config/handlers.py

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -154,21 +154,27 @@ def organization_disabled_handler(instance, **kwargs):
154154
tasks.invalidate_controller_views_cache.delay(str(instance.id))
155155

156156

157-
def organization_config_settings_change_handler(instance, **kwargs):
157+
def organization_config_settings_pre_save_handler(instance, **kwargs):
158158
"""
159-
Invalidates VPN cache when OrganizationConfigSettings context changes.
159+
Stores original context before save to detect changes.
160160
"""
161161
if instance._state.adding:
162162
return
163-
164-
# Check if context actually changed
165163
try:
166164
db_instance = instance.__class__.objects.only("context").get(id=instance.id)
167-
if db_instance.context != instance.context:
168-
transaction.on_commit(
169-
lambda: tasks.invalidate_organization_vpn_cache.delay(
170-
str(instance.organization_id)
171-
)
172-
)
165+
instance._original_context = db_instance.context
173166
except instance.__class__.DoesNotExist:
174-
pass
167+
instance._original_context = None
168+
169+
170+
def organization_config_settings_post_save_handler(instance, **kwargs):
171+
"""
172+
Invalidates VPN cache when OrganizationConfigSettings context changes.
173+
"""
174+
original_context = getattr(instance, "_original_context", None)
175+
if original_context is not None and original_context != instance.context:
176+
transaction.on_commit(
177+
lambda: tasks.invalidate_organization_vpn_cache.delay(
178+
str(instance.organization_id)
179+
)
180+
)

openwisp_controller/config/tasks.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,17 @@ def invalidate_organization_vpn_cache(organization_id):
109109
Vpn = load_model("config", "Vpn")
110110
from .controller.views import GetVpnView
111111

112-
for vpn in Vpn.objects.filter(organization_id=organization_id).iterator():
113-
GetVpnView.invalidate_get_vpn_cache(vpn)
114-
vpn.invalidate_checksum_cache()
112+
try:
113+
for vpn in (
114+
Vpn.objects.filter(organization_id=organization_id).only("id").iterator()
115+
):
116+
GetVpnView.invalidate_get_vpn_cache(vpn)
117+
vpn.invalidate_checksum_cache()
118+
except SoftTimeLimitExceeded:
119+
logger.error(
120+
"soft time limit hit while executing "
121+
f"invalidate_organization_vpn_cache for organization {organization_id}"
122+
)
115123

116124

117125
@shared_task(soft_time_limit=7200)

0 commit comments

Comments
 (0)