Skip to content
Open
9 changes: 9 additions & 0 deletions openwisp_controller/config/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ def __setmodels__(self):
self.org_limits = load_model("config", "OrganizationLimits")
self.cert_model = load_model("django_x509", "Cert")
self.org_model = load_model("openwisp_users", "Organization")
self.organization_config_settings_model = load_model(
"config", "OrganizationConfigSettings"
)

def connect_signals(self):
"""
Expand Down Expand Up @@ -270,6 +273,7 @@ def enable_cache_invalidation(self):
device_cache_invalidation_handler,
devicegroup_change_handler,
devicegroup_delete_handler,
organization_config_settings_change_handler,
vpn_server_change_handler,
)

Expand Down Expand Up @@ -336,6 +340,11 @@ def enable_cache_invalidation(self):
sender=self.vpn_model,
dispatch_uid="vpn.invalidate_checksum_cache",
)
post_save.connect(
organization_config_settings_change_handler,
sender=self.organization_config_settings_model,
dispatch_uid="organization_config_settings.invalidate_vpn_cache",
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

def register_dashboard_charts(self):
register_dashboard_chart(
Expand Down
20 changes: 20 additions & 0 deletions openwisp_controller/config/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,23 @@ def organization_disabled_handler(instance, **kwargs):
# No change in is_active
return
tasks.invalidate_controller_views_cache.delay(str(instance.id))


def organization_config_settings_change_handler(instance, **kwargs):
"""
Invalidates VPN cache when OrganizationConfigSettings context changes.
"""
if instance._state.adding:
return

# Check if context actually changed
try:
db_instance = instance.__class__.objects.only("context").get(id=instance.id)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can avoid this extra query, we already have similar idioms to handle similar cases, see: https://github.com/search?q=repo%3Aopenwisp%2Fopenwisp-controller+self._initial&type=code

This means that if you implement it this way, this receiver may be completely unnecessary.

if db_instance.context != instance.context:
transaction.on_commit(
lambda: tasks.invalidate_organization_vpn_cache.delay(
str(instance.organization_id)
)
)
except instance.__class__.DoesNotExist:
pass
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
14 changes: 14 additions & 0 deletions openwisp_controller/config/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,20 @@ def invalidate_vpn_server_devices_cache_change(vpn_pk):
VpnClient.invalidate_clients_cache(vpn)


@shared_task(soft_time_limit=7200)
def invalidate_organization_vpn_cache(organization_id):
"""
Invalidates VPN cache for all VPNs in an organization when
organization configuration variables change.
"""
Vpn = load_model("config", "Vpn")
from .controller.views import GetVpnView

for vpn in Vpn.objects.filter(organization_id=organization_id).iterator():
GetVpnView.invalidate_get_vpn_cache(vpn)
vpn.invalidate_checksum_cache()


@shared_task(soft_time_limit=7200)
def invalidate_devicegroup_cache_delete(instance_id, model_name, **kwargs):
from .api.views import DeviceGroupCommonName
Expand Down
17 changes: 0 additions & 17 deletions openwisp_controller/config/tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,3 @@ def test_organization_disabled_handler(self, mocked_task):
org.is_active = False
org.save()
mocked_task.assert_called_once()

mocked_task.reset_mock()
with self.subTest("Test task not executed on saving inactive org"):
org.name = "Changed named"
org.save()
mocked_task.assert_not_called()

with self.subTest("Test task not executed on creating inactive org"):
inactive_org = self._create_org(
is_active=False, name="inactive", slug="inactive"
)
mocked_task.assert_not_called()

with self.subTest("Test task not executed on changing inactive to active org"):
inactive_org.is_active = True
inactive_org.save()
mocked_task.assert_not_called()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right. We don't want to trigger cache invalidation in these cases.

Loading