diff --git a/backend/src/baserow/contrib/builder/api/domains/public_views.py b/backend/src/baserow/contrib/builder/api/domains/public_views.py
index e8570fb59..331eeb87e 100644
--- a/backend/src/baserow/contrib/builder/api/domains/public_views.py
+++ b/backend/src/baserow/contrib/builder/api/domains/public_views.py
@@ -239,7 +239,7 @@ class PublicDataSourcesView(APIView):
handler = BuilderHandler()
public_properties = handler.get_builder_public_properties(
- request.user, page.builder
+ request.user_source_user, page.builder
)
allowed_fields = []
diff --git a/backend/src/baserow/contrib/builder/apps.py b/backend/src/baserow/contrib/builder/apps.py
index 0d2c695b2..f5376b6bc 100644
--- a/backend/src/baserow/contrib/builder/apps.py
+++ b/backend/src/baserow/contrib/builder/apps.py
@@ -315,4 +315,5 @@ class BuilderConfig(AppConfig):
# The signals must always be imported last because they use the registries
# which need to be filled first.
+ import baserow.contrib.builder.signals # noqa: F403, F401
import baserow.contrib.builder.ws.signals # noqa: F403, F401
diff --git a/backend/src/baserow/contrib/builder/data_sources/builder_dispatch_context.py b/backend/src/baserow/contrib/builder/data_sources/builder_dispatch_context.py
index 1e6c56a41..28402be09 100644
--- a/backend/src/baserow/contrib/builder/data_sources/builder_dispatch_context.py
+++ b/backend/src/baserow/contrib/builder/data_sources/builder_dispatch_context.py
@@ -273,6 +273,6 @@ class BuilderDispatchContext(DispatchContext):
return None
return BuilderHandler().get_builder_public_properties(
- self.request.user,
+ self.request.user_source_user,
self.page.builder,
)
diff --git a/backend/src/baserow/contrib/builder/handler.py b/backend/src/baserow/contrib/builder/handler.py
index 7ecee5203..bac5e96c7 100644
--- a/backend/src/baserow/contrib/builder/handler.py
+++ b/backend/src/baserow/contrib/builder/handler.py
@@ -3,7 +3,6 @@ from typing import Dict, List, Optional
from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.auth.models import AbstractUser
-from django.core.cache import cache
from baserow.contrib.builder.formula_property_extractor import (
get_builder_used_property_names,
@@ -11,9 +10,11 @@ from baserow.contrib.builder.formula_property_extractor import (
from baserow.contrib.builder.models import Builder
from baserow.contrib.builder.theme.registries import theme_config_block_registry
from baserow.core.handler import CoreHandler
+from baserow.core.utils import invalidate_versioned_cache, safe_get_or_set_cache
User = get_user_model()
CACHE_KEY_PREFIX = "used_properties_for_page"
+BUILDER_PREVIEW_USED_PROPERTIES_CACHE_TTL_SECONDS = 60
class BuilderHandler:
@@ -41,6 +42,10 @@ class BuilderHandler:
.specific
)
+ @classmethod
+ def _get_builder_version_cache(cls, builder: Builder):
+ return f"{CACHE_KEY_PREFIX}_version_{builder.id}"
+
def get_builder_used_properties_cache_key(
self, user: AbstractUser, builder: Builder
) -> Optional[str]:
@@ -54,9 +59,7 @@ class BuilderHandler:
attribute, unlike the User Source User.
"""
- if isinstance(user, User):
- return None
- elif user.is_anonymous:
+ if user.is_anonymous or not user.role:
# When the user is anonymous, only use the prefix + page ID.
role = ""
else:
@@ -64,6 +67,10 @@ class BuilderHandler:
return f"{CACHE_KEY_PREFIX}_{builder.id}{role}"
+ @classmethod
+ def invalidate_builder_public_properties_cache(cls, builder):
+ invalidate_versioned_cache(cls._get_builder_version_cache(builder))
+
def get_builder_public_properties(
self, user: AbstractUser, builder: Builder
) -> Dict[str, Dict[int, List[str]]]:
@@ -80,15 +87,11 @@ class BuilderHandler:
(required only by the backend).
"""
- cache_key = self.get_builder_used_properties_cache_key(user, builder)
- properties = cache.get(cache_key) if cache_key else None
- if properties is None:
- properties = get_builder_used_property_names(user, builder)
- if cache_key:
- cache.set(
- cache_key,
- properties,
- timeout=settings.BUILDER_PUBLICLY_USED_PROPERTIES_CACHE_TTL_SECONDS,
- )
-
- return properties
+ return safe_get_or_set_cache(
+ self.get_builder_used_properties_cache_key(user, builder),
+ self._get_builder_version_cache(builder),
+ default=lambda: get_builder_used_property_names(user, builder),
+ timeout=settings.BUILDER_PUBLICLY_USED_PROPERTIES_CACHE_TTL_SECONDS
+ if builder.workspace_id
+ else BUILDER_PREVIEW_USED_PROPERTIES_CACHE_TTL_SECONDS,
+ )
diff --git a/backend/src/baserow/contrib/builder/signals.py b/backend/src/baserow/contrib/builder/signals.py
new file mode 100644
index 000000000..81b6625ea
--- /dev/null
+++ b/backend/src/baserow/contrib/builder/signals.py
@@ -0,0 +1,124 @@
+from django.dispatch import receiver
+
+from baserow.contrib.builder.data_sources import signals as ds_signals
+from baserow.contrib.builder.elements import signals as element_signals
+from baserow.contrib.builder.handler import BuilderHandler
+from baserow.contrib.builder.models import Builder
+from baserow.contrib.builder.pages import signals as page_signals
+from baserow.contrib.builder.workflow_actions import signals as wa_signals
+from baserow.core.user_sources import signals as us_signals
+
+__all__ = [
+ "element_created",
+ "elements_created",
+ "element_deleted",
+ "element_updated",
+ "wa_created",
+ "wa_updated",
+ "wa_deleted",
+ "ds_created",
+ "ds_updated",
+ "ds_deleted",
+ "page_deleted",
+]
+
+# Elements
+
+
+@receiver(element_signals.element_created)
+def element_created(sender, element, user, before_id=None, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(element.page.builder)
+
+
+@receiver(element_signals.elements_created)
+def elements_created(sender, elements, page, user, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(page.builder)
+
+
+@receiver(element_signals.element_updated)
+def element_updated(sender, element, user, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(element.page.builder)
+
+
+@receiver(element_signals.element_deleted)
+def element_deleted(sender, page, element_id, user, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(page.builder)
+
+
+# Workflow actions
+
+
+@receiver(wa_signals.workflow_action_created)
+def wa_created(sender, workflow_action, user, before_id=None, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(
+ workflow_action.page.builder
+ )
+
+
+@receiver(wa_signals.workflow_action_updated)
+def wa_updated(sender, workflow_action, user, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(
+ workflow_action.page.builder
+ )
+
+
+@receiver(wa_signals.workflow_action_deleted)
+def wa_deleted(sender, workflow_action_id, page, user, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(page.builder)
+
+
+# Data sources
+
+
+@receiver(ds_signals.data_source_created)
+def ds_created(sender, data_source, user, before_id=None, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(
+ data_source.page.builder
+ )
+
+
+@receiver(ds_signals.data_source_updated)
+def ds_updated(sender, data_source, user, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(
+ data_source.page.builder
+ )
+
+
+@receiver(ds_signals.data_source_deleted)
+def ds_deleted(sender, data_source_id, page, user, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(page.builder)
+
+
+# Page
+
+
+@receiver(page_signals.page_deleted)
+def page_deleted(sender, builder, page_id, user, **kwargs):
+ BuilderHandler().invalidate_builder_public_properties_cache(builder)
+
+
+# User sources
+
+
+@receiver(us_signals.user_source_created)
+def us_created(sender, user_source, user, before_id=None, **kwargs):
+ if isinstance(user_source.application.specific, Builder):
+ BuilderHandler().invalidate_builder_public_properties_cache(
+ user_source.application.specific
+ )
+
+
+@receiver(us_signals.user_source_updated)
+def us_updated(sender, user_source, user, **kwargs):
+ if isinstance(user_source.application.specific, Builder):
+ BuilderHandler().invalidate_builder_public_properties_cache(
+ user_source.application.specific
+ )
+
+
+@receiver(us_signals.user_source_deleted)
+def us_deleted(sender, user_source_id, application, user, **kwargs):
+ if isinstance(application.specific, Builder):
+ BuilderHandler().invalidate_builder_public_properties_cache(
+ application.specific
+ )
diff --git a/backend/src/baserow/core/registry.py b/backend/src/baserow/core/registry.py
index 3baf6c42a..c9c2153ab 100644
--- a/backend/src/baserow/core/registry.py
+++ b/backend/src/baserow/core/registry.py
@@ -798,7 +798,7 @@ class ModelRegistryMixin(Generic[DjangoModel, InstanceSubClass]):
if isinstance(model_instance, type):
clazz = model_instance
else:
- clazz = type(model_instance)
+ clazz = model_instance.__class__
return self.get_for_class(clazz)
diff --git a/backend/src/baserow/core/utils.py b/backend/src/baserow/core/utils.py
index 6d0c700ae..35838b453 100644
--- a/backend/src/baserow/core/utils.py
+++ b/backend/src/baserow/core/utils.py
@@ -14,14 +14,27 @@ from decimal import Decimal
from fractions import Fraction
from itertools import chain, islice
from numbers import Number
-from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Type, Union
+from typing import (
+ Any,
+ Callable,
+ Dict,
+ Iterable,
+ List,
+ Optional,
+ Set,
+ Tuple,
+ Type,
+ Union,
+)
from django.conf import settings
+from django.core.cache import cache
from django.db import transaction
from django.db.models import ForeignKey, ManyToManyField, Model
from django.db.models.fields import NOT_PROVIDED
from django.db.transaction import get_connection
+from redis.exceptions import LockNotOwnedError
from requests.utils import guess_json_utf
from baserow.contrib.database.db.schema import optional_atomic
@@ -1193,3 +1206,81 @@ def are_hostnames_same(hostname1: str, hostname2: str) -> bool:
ips1 = get_all_ips(hostname1)
ips2 = get_all_ips(hostname2)
return not ips1.isdisjoint(ips2)
+
+
+def safe_get_or_set_cache(
+ cache_key: str,
+ version_cache_key: str = None,
+ default: Any | Callable = None,
+ timeout: int = 60,
+) -> Any:
+ """
+ Retrieves a value from the cache if it exists; otherwise, sets it using the
+ provided default value. If a version cache key is provided, the function uses
+ a versioned key to manage cache invalidation.
+
+ This function also uses a lock (if available on the cache backend) to ensure
+ multi call safety when setting a new value.
+
+ :param cache_key: The base key to look up in the cache.
+ :param version_cache_key: An optional key used to version the cache. If
+ provided,.
+ :param default: The default value to store in the cache if the key is absent.
+ Can be either a literal value or a callable. If it's a callable,
+ the function is called to retrieve the default value.
+ :param timeout: The cache timeout in seconds for newly set values. Defaults to 60.
+ :return: The cached value if it exists; otherwise, the newly set value.
+ """
+
+ cached = cache.get(cache_key)
+
+ cache_key_to_use = cache_key
+ if version_cache_key is not None:
+ version = cache.get(version_cache_key, 0)
+ cache_key_to_use = f"{cache_key}__version_{version}"
+
+ if cached is None:
+ use_lock = hasattr(cache, "lock")
+ if use_lock:
+ cache_lock = cache.lock(f"{cache_key_to_use}__lock", timeout=10)
+ cache_lock.acquire()
+ try:
+ cached = cache.get(cache_key_to_use)
+ # We check again to make sure it hasn't been populated in the meantime
+ # while acquiring the lock
+ if cached is None:
+ if callable(default):
+ cached = default()
+ else:
+ cached = default
+
+ cache.set(
+ cache_key_to_use,
+ cached,
+ timeout=timeout,
+ )
+ finally:
+ if use_lock:
+ try:
+ cache_lock.release()
+ except LockNotOwnedError:
+ # If the lock release fails, it might be because of the timeout
+ # and it's been stolen so we don't really care
+ pass
+
+ return cached
+
+
+def invalidate_versioned_cache(version_cache_key: str):
+ """
+ Invalidates (or increments) the version associated with a versioned cache,
+ forcing future reads on this versioned key to miss the cache.
+
+ :param version_cache_key: The key whose version is to be incremented in the cache.
+ """
+
+ try:
+ cache.incr(version_cache_key, 1)
+ except ValueError:
+ # No cache key, we create one
+ cache.set(version_cache_key, 1)
diff --git a/backend/tests/baserow/contrib/builder/data_sources/test_data_source_handler.py b/backend/tests/baserow/contrib/builder/data_sources/test_data_source_handler.py
index 6a83d28f2..c8dc452ba 100644
--- a/backend/tests/baserow/contrib/builder/data_sources/test_data_source_handler.py
+++ b/backend/tests/baserow/contrib/builder/data_sources/test_data_source_handler.py
@@ -574,7 +574,7 @@ def test_dispatch_data_source_doesnt_return_formula_field_names(
{},
HTTP_USERSOURCEAUTHORIZATION=f"JWT {user_source_user_token}",
)
- fake_request.user = user_source_user
+ fake_request.user_source_user = user_source_user
dispatch_context = BuilderDispatchContext(fake_request, page)
mock_get_builder_used_property_names.return_value = {
diff --git a/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py b/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py
index ce58dd5d4..ffdff3bb9 100644
--- a/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py
+++ b/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py
@@ -59,7 +59,7 @@ def test_dispatch_context_page_from_context(mock_get_field_names, data_fixture):
)
request = Request(HttpRequest())
- request.user = user_source_user
+ request.user_source_user = user_source_user
dispatch_context = BuilderDispatchContext(
request, page, offset=0, count=5, only_expose_public_allowed_properties=True
@@ -211,7 +211,7 @@ def test_builder_dispatch_context_field_names_computed_on_param(
mock_get_builder_used_property_names.return_value = mock_field_names
mock_request = MagicMock()
- mock_request.user.is_anonymous = True
+ mock_request.user_source_user.is_anonymous = True
mock_page = MagicMock()
mock_page.builder = MagicMock()
@@ -224,7 +224,7 @@ def test_builder_dispatch_context_field_names_computed_on_param(
if only_expose_public_allowed_properties:
assert dispatch_context.public_allowed_properties == mock_field_names
mock_get_builder_used_property_names.assert_called_once_with(
- mock_request.user, mock_page.builder
+ mock_request.user_source_user, mock_page.builder
)
else:
assert dispatch_context.public_allowed_properties is None
@@ -378,7 +378,7 @@ def test_builder_dispatch_context_public_allowed_properties_is_cached(
)
request = Request(HttpRequest())
- request.user = user_source_user
+ request.user_source_user = user_source_user
dispatch_context = BuilderDispatchContext(
request,
diff --git a/backend/tests/baserow/contrib/builder/test_builder_handler.py b/backend/tests/baserow/contrib/builder/test_builder_handler.py
index 72ff1b191..e782b06a3 100644
--- a/backend/tests/baserow/contrib/builder/test_builder_handler.py
+++ b/backend/tests/baserow/contrib/builder/test_builder_handler.py
@@ -44,43 +44,27 @@ def test_get_builder_select_related_theme_config(
@pytest.mark.parametrize(
- "is_anonymous,is_editor_user,user_role,expected_cache_key",
+ "is_anonymous,user_role,expected_cache_key",
[
(
True,
- False,
"",
f"{CACHE_KEY_PREFIX}_100",
),
(
True,
- False,
"foo_role",
f"{CACHE_KEY_PREFIX}_100",
),
(
- False,
False,
"foo_role",
f"{CACHE_KEY_PREFIX}_100_foo_role",
),
- (
- False,
- False,
- "bar_role",
- f"{CACHE_KEY_PREFIX}_100_bar_role",
- ),
- # Test the "editor" role
- (
- False,
- True,
- "",
- None,
- ),
],
)
def test_get_builder_used_properties_cache_key_returned_expected_cache_key(
- is_anonymous, is_editor_user, user_role, expected_cache_key
+ is_anonymous, user_role, expected_cache_key
):
"""
Test the BuilderHandler::get_builder_used_properties_cache_key() method.
@@ -88,13 +72,9 @@ def test_get_builder_used_properties_cache_key_returned_expected_cache_key(
Ensure the expected cache key is returned.
"""
- mock_request = MagicMock()
-
- if is_editor_user:
- mock_request.user = MagicMock(spec=User)
-
- mock_request.user.is_anonymous = is_anonymous
- mock_request.user.role = user_role
+ user_source_user = MagicMock()
+ user_source_user.is_anonymous = is_anonymous
+ user_source_user.role = user_role
mock_builder = MagicMock()
mock_builder.id = 100
@@ -102,35 +82,12 @@ def test_get_builder_used_properties_cache_key_returned_expected_cache_key(
handler = BuilderHandler()
cache_key = handler.get_builder_used_properties_cache_key(
- mock_request.user, mock_builder
+ user_source_user, mock_builder
)
assert cache_key == expected_cache_key
-def test_get_builder_used_properties_cache_key_returns_none():
- """
- Test the BuilderHandler::get_builder_used_properties_cache_key() method.
-
- Ensure that None is returned when request or builder are not available,
- or if the user is a builder user.
- """
-
- mock_request = MagicMock()
- mock_request.user = MagicMock(spec=User)
-
- mock_builder = MagicMock()
- mock_builder.id = 100
-
- handler = BuilderHandler()
-
- cache_key = handler.get_builder_used_properties_cache_key(
- mock_request.user, mock_builder
- )
-
- assert cache_key is None
-
-
@pytest.mark.django_db
def test_public_allowed_properties_is_cached(data_fixture, django_assert_num_queries):
"""
diff --git a/backend/tests/baserow/core/test_core_utils_cache.py b/backend/tests/baserow/core/test_core_utils_cache.py
new file mode 100755
index 000000000..f40e1201c
--- /dev/null
+++ b/backend/tests/baserow/core/test_core_utils_cache.py
@@ -0,0 +1,132 @@
+from django.core.cache import cache
+
+from baserow.core.utils import invalidate_versioned_cache, safe_get_or_set_cache
+
+
+def test_safe_get_or_set_cache_literally_stores_default():
+ """If the cache is empty, a literal default value is stored and returned."""
+
+ cache_key = "test_literal_default"
+ cache.delete(cache_key)
+
+ result = safe_get_or_set_cache(
+ cache_key=cache_key,
+ default="my_default_value",
+ timeout=60,
+ )
+ assert result == "my_default_value"
+ assert cache.get(cache_key) == "my_default_value"
+
+
+def test_safe_get_or_set_cache_callable_stores_return_value():
+ """
+ If the cache is empty, a callable default's return value is stored and returned.
+ """
+
+ cache_key = "test_callable_default"
+ cache.delete(cache_key)
+
+ def some_callable():
+ return "callable_value"
+
+ result = safe_get_or_set_cache(
+ cache_key=cache_key,
+ default=some_callable,
+ timeout=60,
+ )
+ assert result == "callable_value"
+ assert cache.get(cache_key) == "callable_value"
+
+
+def test_safe_get_or_set_cache_uses_existing_value():
+ """
+ If the cache key already has a value, it should be returned without overwriting.
+ """
+
+ cache_key = "test_existing"
+ cache.delete(cache_key)
+ cache.set(cache_key, "existing_value", 60)
+
+ result = safe_get_or_set_cache(
+ cache_key=cache_key,
+ default="unused_default",
+ timeout=60,
+ )
+ assert result == "existing_value"
+ # Confirm it didn't overwrite with 'unused_default'
+ assert cache.get(cache_key) == "existing_value"
+
+
+def test_versioned_cache_set_and_retrieve():
+ """
+ When a version_cache_key is given and the value does not exist,
+ it should store and retrieve the value under <cache_key>__version_X.
+ """
+
+ base_key = "test_versioned_base"
+ version_cache_key = "test_versioned_key"
+ cache.delete(base_key)
+ cache.delete(version_cache_key)
+
+ # No version exists, so this should initialize version=0
+ result = safe_get_or_set_cache(
+ cache_key=base_key,
+ version_cache_key=version_cache_key,
+ default="versioned_value",
+ timeout=60,
+ )
+ assert result == "versioned_value"
+ # Confirm the value is stored under the versioned key
+ assert cache.get(f"{base_key}__version_0") == "versioned_value"
+
+
+def test_versioned_cache_hit():
+ """
+ If a versioned key already exists, safe_get_or_set_cache should retrieve
+ that existing value rather than setting a new one.
+ """
+
+ base_key = "test_versioned_base2"
+ version_cache_key = "test_versioned_key2"
+ cache.delete(base_key)
+ cache.delete(version_cache_key)
+
+ # Manually set version=5
+ cache.set(version_cache_key, 5)
+ full_key = f"{base_key}__version_5"
+ cache.set(full_key, "already_versioned", 60)
+
+ result = safe_get_or_set_cache(
+ cache_key=base_key,
+ version_cache_key=version_cache_key,
+ default="unused_default",
+ timeout=60,
+ )
+ assert result == "already_versioned"
+ assert cache.get(full_key) == "already_versioned"
+
+
+def test_invalidate_versioned_cache_increments_existing():
+ """
+ If a version_cache_key already exists, calling invalidate_versioned_cache should
+ increment the version.
+ """
+
+ version_key = "test_invalidate_existing"
+ cache.set(version_key, 3)
+
+ invalidate_versioned_cache(version_key)
+ assert cache.get(version_key) == 4
+
+
+def test_invalidate_versioned_cache_sets_new_if_absent():
+ """
+ If a versioned cache key doesn't exist, calling invalidate_versioned_cache should
+ create it and set it to 1.
+ """
+
+ version_key = "test_invalidate_absent"
+ cache.delete(version_key)
+
+ invalidate_versioned_cache(version_key)
+ assert cache.get(version_key) == 1
diff --git a/changelog/entries/unreleased/refactor/builder_improved_performance_in_preview_mode.json b/changelog/entries/unreleased/refactor/builder_improved_performance_in_preview_mode.json
new file mode 100644
index 000000000..92ba73ef3
--- /dev/null
+++ b/changelog/entries/unreleased/refactor/builder_improved_performance_in_preview_mode.json
@@ -0,0 +1,7 @@
+{
+ "type": "refactor",
+ "message": "[Builder] Improved performance in preview mode",
+ "issue_number": null,
+ "bullet_points": [],
+ "created_at": "2025-02-13"
+}
\ No newline at end of file