1
0
Fork 0
mirror of https://gitlab.com/bramw/baserow.git synced 2025-03-16 05:23:33 +00:00

Add cache for used properties computation in preview

This commit is contained in:
Jérémie Pardou 2025-02-13 14:11:25 +00:00
parent acc56a9249
commit 12bbde2155
12 changed files with 389 additions and 74 deletions

View file

@ -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 = []

View file

@ -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

View file

@ -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,
)

View file

@ -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,
)

View file

@ -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
)

View file

@ -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)

View file

@ -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)

View file

@ -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 = {

View file

@ -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,

View file

@ -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):
"""

View file

@ -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

View file

@ -0,0 +1,7 @@
{
"type": "refactor",
"message": "[Builder] Improved performance in preview mode",
"issue_number": null,
"bullet_points": [],
"created_at": "2025-02-13"
}