From 6a49116e26f6c1d304c50daeb891530470e85618 Mon Sep 17 00:00:00 2001 From: Peter Evans <peter@baserow.io> Date: Wed, 26 Feb 2025 09:29:58 +0000 Subject: [PATCH] Resolve "Cache the public builder data sources, elements and workflow actions endpoints." --- .../builder/api/domains/public_views.py | 152 +++++++++++++++--- .../contrib/builder/domains/handler.py | 19 ++- .../builder/formula_property_extractor.py | 7 +- .../src/baserow/contrib/builder/handler.py | 50 +++--- .../baserow/contrib/builder/pages/handler.py | 59 ++++++- backend/src/baserow/core/utils.py | 16 +- .../api/domains/test_domain_public_views.py | 12 +- .../test_workflow_actions_views.py | 52 +++--- .../builder/domains/test_domain_handler.py | 34 +++- .../builder/pages/test_page_handler.py | 36 +++++ .../contrib/builder/test_builder_handler.py | 11 +- .../baserow/core/test_core_utils_cache.py | 81 +++++----- ...performance_of_published_applications.json | 7 + 13 files changed, 397 insertions(+), 139 deletions(-) create mode 100644 changelog/entries/unreleased/feature/3472_builder_improved_the_performance_of_published_applications.json 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 331eeb87e..03862c426 100644 --- a/backend/src/baserow/contrib/builder/api/domains/public_views.py +++ b/backend/src/baserow/contrib/builder/api/domains/public_views.py @@ -1,8 +1,11 @@ +from typing import Any, Dict, List + from django.db import transaction from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, extend_schema from rest_framework.permissions import AllowAny +from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView @@ -27,7 +30,11 @@ from baserow.contrib.builder.api.data_sources.errors import ( from baserow.contrib.builder.api.data_sources.serializers import ( DispatchDataSourceRequestSerializer, ) -from baserow.contrib.builder.api.domains.serializers import PublicBuilderSerializer +from baserow.contrib.builder.api.domains.serializers import ( + PublicBuilderSerializer, + PublicDataSourceSerializer, + PublicElementSerializer, +) from baserow.contrib.builder.api.pages.errors import ERROR_PAGE_DOES_NOT_EXIST from baserow.contrib.builder.api.workflow_actions.serializers import ( BuilderWorkflowActionSerializer, @@ -42,12 +49,17 @@ from baserow.contrib.builder.data_sources.exceptions import ( ) from baserow.contrib.builder.data_sources.handler import DataSourceHandler from baserow.contrib.builder.data_sources.service import DataSourceService +from baserow.contrib.builder.domains.handler import DomainHandler from baserow.contrib.builder.domains.service import DomainService from baserow.contrib.builder.elements.registries import element_type_registry from baserow.contrib.builder.elements.service import ElementService from baserow.contrib.builder.errors import ERROR_BUILDER_DOES_NOT_EXIST from baserow.contrib.builder.exceptions import BuilderDoesNotExist -from baserow.contrib.builder.handler import BuilderHandler +from baserow.contrib.builder.handler import ( + BUILDER_PUBLIC_BUILDER_BY_DOMAIN_TTL_SECONDS, + BUILDER_PUBLIC_RECORDS_CACHE_TTL_SECONDS, + BuilderHandler, +) from baserow.contrib.builder.pages.exceptions import PageDoesNotExist from baserow.contrib.builder.pages.handler import PageHandler from baserow.contrib.builder.service import BuilderService @@ -65,8 +77,7 @@ from baserow.core.services.exceptions import ( ServiceSortPropertyDoesNotExist, ) from baserow.core.services.registries import service_type_registry - -from .serializers import PublicDataSourceSerializer, PublicElementSerializer +from baserow.core.utils import safe_get_or_set_cache class PublicBuilderByDomainNameView(APIView): @@ -93,18 +104,39 @@ class PublicBuilderByDomainNameView(APIView): }, ) @map_exceptions({BuilderDoesNotExist: ERROR_BUILDER_DOES_NOT_EXIST}) - def get(self, request, domain_name): + def get(self, request: Request, domain_name: str): """ Responds with a serialized version of the builder related to the query. Try to match a published builder for the given domain name. Used to display the public site. """ + data = safe_get_or_set_cache( + cache_key=DomainHandler.get_public_builder_by_domain_cache_key(domain_name), + version_cache_key=DomainHandler.get_public_builder_by_domain_version_cache_key( + domain_name + ), + default=lambda: self._get_public_builder_by_domain(request, domain_name), + timeout=BUILDER_PUBLIC_BUILDER_BY_DOMAIN_TTL_SECONDS, + ) + return Response(data) + + def _get_public_builder_by_domain(self, request: Request, domain_name: str): + """ + Returns a serialized builder which has a domain matching `domain_name`. + + Only requested if the public get-by-domain cache is stale, or if the + application has been re-published. + + :param request: the HTTP request. + :param domain_name: the domain name to match. + :return: a publicly serialized builder. + """ + builder = DomainService().get_public_builder_by_domain_name( request.user, domain_name ) - - return Response(PublicBuilderSerializer(builder).data) + return PublicBuilderSerializer(builder).data class PublicBuilderByIdView(APIView): @@ -180,20 +212,43 @@ class PublicElementsView(APIView): PageDoesNotExist: ERROR_PAGE_DOES_NOT_EXIST, } ) - def get(self, request, page_id): + def get(self, request: Request, page_id: int): """ Responds with a list of serialized elements that belongs to the given page id. """ + if PageHandler().is_published_page(page_id): + data = safe_get_or_set_cache( + cache_key=PageHandler.get_page_public_records_cache_key( + page_id, request.user_source_user, "elements" + ), + default=lambda: self._get_public_page_elements(request, page_id), + timeout=BUILDER_PUBLIC_RECORDS_CACHE_TTL_SECONDS, + ) + else: + data = self._get_public_page_elements(request, page_id) + + return Response(data) + + def _get_public_page_elements( + self, request: Request, page_id: int + ) -> List[Dict[str, Any]]: + """ + Returns a list of serialized elements that belong to the given page id. + + Only requested if the public elements cache is stale. + + :param request: the HTTP request. + :param page_id: the page id. + :return: a list of serialized elements. + """ + page = PageHandler().get_page(page_id) - elements = ElementService().get_elements(request.user, page) - - data = [ + return [ element_type_registry.get_serializer(element, PublicElementSerializer).data for element in elements ] - return Response(data) class PublicDataSourcesView(APIView): @@ -227,26 +282,47 @@ class PublicDataSourcesView(APIView): PageDoesNotExist: ERROR_PAGE_DOES_NOT_EXIST, } ) - def get(self, request, page_id): + def get(self, request: Request, page_id: int): """ Responds with a list of serialized data_sources that belong to the page if the user has access to it. """ - page = PageHandler().get_page(page_id) + if PageHandler().is_published_page(page_id): + data = safe_get_or_set_cache( + cache_key=PageHandler.get_page_public_records_cache_key( + page_id, request.user_source_user, "data_sources" + ), + default=lambda: self._get_public_page_data_sources(request, page_id), + timeout=BUILDER_PUBLIC_RECORDS_CACHE_TTL_SECONDS, + ) + else: + data = self._get_public_page_data_sources(request, page_id) + return Response(data) + + def _get_public_page_data_sources(self, request: Request, page_id: int): + """ + Returns a list of serialized data sources that belong to the given page id. + + Only requested if the public data sources cache is stale. + + :param request: the HTTP request. + :param page_id: the page id. + :return: a list of serialized data sources. + """ + + page = PageHandler().get_page(page_id) data_sources = DataSourceService().get_data_sources(request.user, page) - handler = BuilderHandler() - public_properties = handler.get_builder_public_properties( + public_properties = BuilderHandler().get_builder_public_properties( request.user_source_user, page.builder ) - allowed_fields = [] for fields in public_properties["external"].values(): allowed_fields.extend(fields) - data = [ + return [ service_type_registry.get_serializer( data_source.service, PublicDataSourceSerializer, @@ -256,8 +332,6 @@ class PublicDataSourcesView(APIView): if data_source.service and data_source.service.integration_id ] - return Response(data) - class PublicBuilderWorkflowActionsView(APIView): permission_classes = (AllowAny,) @@ -295,14 +369,44 @@ class PublicBuilderWorkflowActionsView(APIView): PageDoesNotExist: ERROR_PAGE_DOES_NOT_EXIST, } ) - def get(self, request, page_id: int): - page = PageHandler().get_page(page_id) + def get(self, request: Request, page_id: int): + """ " + Responds with a list of serialized workflow actions that belongs to the given + page id. + """ + if PageHandler().is_published_page(page_id): + data = safe_get_or_set_cache( + cache_key=PageHandler.get_page_public_records_cache_key( + page_id, request.user_source_user, "workflow_actions" + ), + default=lambda: self._get_public_page_workflow_actions( + request, page_id + ), + timeout=BUILDER_PUBLIC_RECORDS_CACHE_TTL_SECONDS, + ) + else: + data = self._get_public_page_workflow_actions(request, page_id) + + return Response(data) + + def _get_public_page_workflow_actions(self, request: Request, page_id: int): + """ + Returns a list of serialized workflow actions that belong to the given page id. + + Only requested if the public workflow actions cache is stale. + + :param request: the HTTP request. + :param page_id: the page id. + :return: a list of serialized workflow actions. + """ + + page = PageHandler().get_page(page_id) workflow_actions = BuilderWorkflowActionService().get_workflow_actions( request.user, page ) - data = [ + return [ builder_workflow_action_type_registry.get_serializer( workflow_action, BuilderWorkflowActionSerializer, @@ -311,8 +415,6 @@ class PublicBuilderWorkflowActionsView(APIView): for workflow_action in workflow_actions ] - return Response(data) - class PublicDispatchDataSourceView(APIView): permission_classes = (AllowAny,) diff --git a/backend/src/baserow/contrib/builder/domains/handler.py b/backend/src/baserow/contrib/builder/domains/handler.py index c3149c349..7d3083cc1 100644 --- a/backend/src/baserow/contrib/builder/domains/handler.py +++ b/backend/src/baserow/contrib/builder/domains/handler.py @@ -19,7 +19,7 @@ from baserow.core.models import Workspace from baserow.core.registries import ImportExportConfig, application_type_registry from baserow.core.storage import get_default_storage from baserow.core.trash.handler import TrashHandler -from baserow.core.utils import Progress, extract_allowed +from baserow.core.utils import Progress, extract_allowed, invalidate_versioned_cache class DomainHandler: @@ -275,4 +275,21 @@ class DomainHandler: domain.last_published = datetime.now(tz=timezone.utc) domain.save() + # Invalidate the public builder-by-domain cache after a new publication. + DomainHandler.invalidate_public_builder_by_domain_cache(domain.domain_name) + return domain + + @classmethod + def get_public_builder_by_domain_cache_key(cls, domain_name: str) -> str: + return f"ab_public_builder_by_domain_{domain_name}" + + @classmethod + def get_public_builder_by_domain_version_cache_key(cls, domain_name: str) -> str: + return f"ab_public_builder_by_domain_{domain_name}_version" + + @classmethod + def invalidate_public_builder_by_domain_cache(cls, domain_name: str): + invalidate_versioned_cache( + cls.get_public_builder_by_domain_version_cache_key(domain_name) + ) diff --git a/backend/src/baserow/contrib/builder/formula_property_extractor.py b/backend/src/baserow/contrib/builder/formula_property_extractor.py index 5c9631239..6596999f1 100644 --- a/backend/src/baserow/contrib/builder/formula_property_extractor.py +++ b/backend/src/baserow/contrib/builder/formula_property_extractor.py @@ -1,7 +1,5 @@ from typing import TYPE_CHECKING, Dict, List, Set -from django.contrib.auth.models import AbstractUser - from antlr4.tree import Tree from baserow.contrib.builder.data_providers.registries import ( @@ -11,6 +9,7 @@ from baserow.contrib.builder.elements.models import Element from baserow.contrib.builder.formula_importer import BaserowFormulaImporter from baserow.core.formula import BaserowFormula from baserow.core.formula.exceptions import InvalidBaserowFormula +from baserow.core.user_sources.user_source_user import UserSourceUser from baserow.core.utils import merge_dicts_no_duplicates, to_path if TYPE_CHECKING: @@ -176,10 +175,10 @@ def get_data_source_property_names( def get_builder_used_property_names( - user: AbstractUser, builder: "Builder" + user: UserSourceUser, builder: "Builder" ) -> Dict[str, Dict[int, List[str]]]: """ - Given a User and a Builder, return all property names used in the all the + Given a UserSourceUser and a Builder, return all property names used in the all the pages. This involves looping over all Elements, Workflow Actions, and Data Sources diff --git a/backend/src/baserow/contrib/builder/handler.py b/backend/src/baserow/contrib/builder/handler.py index bdaa4f492..07adc024a 100644 --- a/backend/src/baserow/contrib/builder/handler.py +++ b/backend/src/baserow/contrib/builder/handler.py @@ -1,8 +1,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.db.models.query import QuerySet from baserow.contrib.builder.formula_property_extractor import ( @@ -14,12 +12,19 @@ from baserow.core.handler import CoreHandler from baserow.core.models import Workspace from baserow.core.user_sources.handler import UserSourceHandler from baserow.core.user_sources.models import UserSource +from baserow.core.user_sources.user_source_user import UserSourceUser 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 +USED_PROPERTIES_CACHE_KEY_PREFIX = "used_properties_for_page" +# The duration of the cached public element, data source and workflow action API views. +BUILDER_PUBLIC_RECORDS_CACHE_TTL_SECONDS = 60 * 60 + +# The duration of the cached public `get_public_builder_by_domain_name` view. +BUILDER_PUBLIC_BUILDER_BY_DOMAIN_TTL_SECONDS = 60 * 60 + +# The duration of the cached public properties for the builder API views. +BUILDER_PREVIEW_USED_PROPERTIES_CACHE_TTL_SECONDS = 60 SENTINEL = "__no_results__" @@ -50,20 +55,15 @@ class BuilderHandler: ) @classmethod - def _get_builder_version_cache(cls, builder: Builder): - return f"{CACHE_KEY_PREFIX}_version_{builder.id}" + def _get_builder_public_properties_version_cache(cls, builder: Builder) -> str: + return f"{USED_PROPERTIES_CACHE_KEY_PREFIX}_version_{builder.id}" def get_builder_used_properties_cache_key( - self, user: AbstractUser, builder: Builder - ) -> Optional[str]: + self, user: UserSourceUser, builder: Builder + ) -> str: """ Returns a cache key that can be used to key the results of making the expensive function call to get_builder_used_property_names(). - - If the user is a Django user, return None. This is because the Page - Designer should always have the latest data in the Preview (e.g. when - they are not authenticated). Also, the Django user doesn't have the role - attribute, unlike the User Source User. """ if user.is_anonymous or not user.role: @@ -72,14 +72,16 @@ class BuilderHandler: else: role = f"_{user.role}" - return f"{CACHE_KEY_PREFIX}_{builder.id}{role}" + return f"{USED_PROPERTIES_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 invalidate_builder_public_properties_cache(cls, builder: Builder): + invalidate_versioned_cache( + cls._get_builder_public_properties_version_cache(builder) + ) def get_builder_public_properties( - self, user: AbstractUser, builder: Builder + self, user: UserSourceUser, builder: Builder ) -> Dict[str, Dict[int, List[str]]]: """ Return a Dict where keys are ["all", "external", "internal"] and values @@ -100,7 +102,7 @@ class BuilderHandler: result = safe_get_or_set_cache( self.get_builder_used_properties_cache_key(user, builder), - self._get_builder_version_cache(builder), + self._get_builder_public_properties_version_cache(builder), default=compute_properties, timeout=settings.BUILDER_PUBLICLY_USED_PROPERTIES_CACHE_TTL_SECONDS if builder.workspace_id @@ -146,3 +148,13 @@ class BuilderHandler: ) ) return UserSourceHandler().aggregate_user_counts(workspace, queryset) + + @classmethod + def get_public_builder_by_domain_version_cache(cls, domain_name: str) -> str: + return f"get_public_builder_by_domain_{domain_name}" + + @classmethod + def invalidate_public_builder_by_domain_cache(cls, domain_name: str): + invalidate_versioned_cache( + cls.get_public_builder_by_domain_version_cache(domain_name) + ) diff --git a/backend/src/baserow/contrib/builder/pages/handler.py b/backend/src/baserow/contrib/builder/pages/handler.py index 885172fbe..7c4e72ff5 100644 --- a/backend/src/baserow/contrib/builder/pages/handler.py +++ b/backend/src/baserow/contrib/builder/pages/handler.py @@ -42,7 +42,15 @@ from baserow.contrib.builder.workflow_actions.handler import ( ) from baserow.core.exceptions import IdDoesNotExist from baserow.core.storage import ExportZipFile -from baserow.core.utils import ChildProgressBuilder, MirrorDict, find_unused_name +from baserow.core.user_sources.user_source_user import UserSourceUser +from baserow.core.utils import ( + ChildProgressBuilder, + MirrorDict, + find_unused_name, + safe_get_or_set_cache, +) + +BUILDER_PAGE_IS_PUBLISHED_CACHE_TTL_SECONDS = 60 * 60 class PageHandler: @@ -229,6 +237,55 @@ class PageHandler: return full_order + @classmethod + def get_page_public_records_cache_key( + cls, page_id: int, user: UserSourceUser, record_name: str + ): + """ + Generates the cache key used by the public elements, data sources and workflow + actions endpoints. If the `user` is authenticated, and they have a role, we will + include the role in the cache key. + + :param page_id: the ID of the public page being requested. + :param user: the `UserSourceUser` performing the HTTP request. + :param record_name: one of "elements", "data_sources" or "workflow_actions". + Used to differentiate between public view endpoints. + :return: the cache key. + """ + + role = f"_{user.role}" if not user.is_anonymous and user.role else "" + return f"ab_public_page_{page_id}{role}_{record_name}_records" + + def is_published_page(self, public_page_id: int) -> bool: + """ + Returns whether this public page ID points to a published domain + application or not. + + :param public_page_id: The ID of the public page. + :return: whether this public page ID is published or not. + """ + + return safe_get_or_set_cache( + f"ab_public_page_{public_page_id}_published", + default=lambda: self._is_published_application_page(public_page_id), + timeout=BUILDER_PAGE_IS_PUBLISHED_CACHE_TTL_SECONDS, + ) + + def _is_published_application_page(self, public_page_id: int) -> bool: + """ + Given a *public* page ID, is responsible for returning the published domain + application it's associated with. + + :param public_page_id: The ID of the public page. + :return: The published domain application associated with the public page. + """ + + return ( + Builder.objects.filter(page__id=public_page_id) + .exclude(published_from=None) + .exists() + ) + def duplicate_page( self, page: Page, progress_builder: Optional[ChildProgressBuilder] = None ): diff --git a/backend/src/baserow/core/utils.py b/backend/src/baserow/core/utils.py index 35838b453..9f7f66f41 100644 --- a/backend/src/baserow/core/utils.py +++ b/backend/src/baserow/core/utils.py @@ -38,6 +38,7 @@ from redis.exceptions import LockNotOwnedError from requests.utils import guess_json_utf from baserow.contrib.database.db.schema import optional_atomic +from baserow.version import VERSION as BASEROW_VERSION from .exceptions import CannotCalculateIntermediateOrder @@ -1208,6 +1209,9 @@ def are_hostnames_same(hostname1: str, hostname2: str) -> bool: return not ips1.isdisjoint(ips2) +SENTINEL = object() + + def safe_get_or_set_cache( cache_key: str, version_cache_key: str = None, @@ -1232,23 +1236,23 @@ def safe_get_or_set_cache( :return: The cached value if it exists; otherwise, the newly set value. """ - cached = cache.get(cache_key) - - cache_key_to_use = cache_key + cache_key_to_use = f"{BASEROW_VERSION}_{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: + cached = cache.get(cache_key_to_use, SENTINEL) + + if cached is SENTINEL: 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) + cached = cache.get(cache_key_to_use, SENTINEL) # We check again to make sure it hasn't been populated in the meantime # while acquiring the lock - if cached is None: + if cached is SENTINEL: if callable(default): cached = default() else: diff --git a/backend/tests/baserow/contrib/builder/api/domains/test_domain_public_views.py b/backend/tests/baserow/contrib/builder/api/domains/test_domain_public_views.py index 07d6b7e2e..391ef8161 100644 --- a/backend/tests/baserow/contrib/builder/api/domains/test_domain_public_views.py +++ b/backend/tests/baserow/contrib/builder/api/domains/test_domain_public_views.py @@ -107,12 +107,12 @@ def test_get_public_builder_by_domain_name(api_client, data_fixture): page2 = data_fixture.create_builder_page(user=user, builder=builder_to) domain = data_fixture.create_builder_custom_domain( - domain_name="test.getbaserow.io", published_to=builder_to + domain_name="xyztest.getbaserow.io", published_to=builder_to ) url = reverse( "api:builder:domains:get_builder_by_domain_name", - kwargs={"domain_name": "test.getbaserow.io"}, + kwargs={"domain_name": "xyztest.getbaserow.io"}, ) # Anonymous request @@ -215,14 +215,14 @@ def test_get_builder_missing_domain_name(api_client, data_fixture): def test_get_non_public_builder(api_client, data_fixture): user, token = data_fixture.create_user_and_token() page = data_fixture.create_builder_page(user=user) - page2 = data_fixture.create_builder_page(builder=page.builder, user=user) - domain = data_fixture.create_builder_custom_domain( - domain_name="test.getbaserow.io", builder=page.builder + data_fixture.create_builder_page(builder=page.builder, user=user) + data_fixture.create_builder_custom_domain( + domain_name="notpublic.getbaserow.io", builder=page.builder ) url = reverse( "api:builder:domains:get_builder_by_domain_name", - kwargs={"domain_name": "test.getbaserow.io"}, + kwargs={"domain_name": "notpublic.getbaserow.io"}, ) response = api_client.get( url, diff --git a/backend/tests/baserow/contrib/builder/api/workflow_actions/test_workflow_actions_views.py b/backend/tests/baserow/contrib/builder/api/workflow_actions/test_workflow_actions_views.py index 862e78110..bb5e12d8c 100644 --- a/backend/tests/baserow/contrib/builder/api/workflow_actions/test_workflow_actions_views.py +++ b/backend/tests/baserow/contrib/builder/api/workflow_actions/test_workflow_actions_views.py @@ -180,32 +180,25 @@ class PublicTestWorkflowActionType(NotificationWorkflowActionType): @pytest.mark.django_db -def test_public_workflow_actions_view( - api_client, data_fixture, mutable_builder_workflow_action_registry -): +def test_public_workflow_actions_view(api_client, data_fixture): user, token = data_fixture.create_user_and_token() - page = data_fixture.create_builder_page(user=user) + workspace = data_fixture.create_workspace(user=user) - mutable_builder_workflow_action_registry.unregister( - NotificationWorkflowActionType().type - ) - mutable_builder_workflow_action_registry.register(PublicTestWorkflowActionType()) - - workflow_action = BuilderWorkflowActionHandler().create_workflow_action( - PublicTestWorkflowActionType(), test="hello", page=page + builder = data_fixture.create_builder_application(workspace=workspace) + page = data_fixture.create_builder_page(builder=builder) + BuilderWorkflowActionHandler().create_workflow_action( + NotificationWorkflowActionType(), page=page ) - url = reverse( - "api:builder:workflow_action:item", - kwargs={"workflow_action_id": workflow_action.id}, - ) - response = api_client.get( - url, - format="json", - HTTP_AUTHORIZATION=f"JWT {token}", + published_builder = data_fixture.create_builder_application(workspace=None) + published_page = data_fixture.create_builder_page(builder=published_builder) + BuilderWorkflowActionHandler().create_workflow_action( + NotificationWorkflowActionType(), page=published_page ) - assert "test" not in response.json() + data_fixture.create_builder_custom_domain( + builder=builder, published_to=published_builder + ) url = reverse( "api:builder:domains:list_workflow_actions", @@ -217,8 +210,23 @@ def test_public_workflow_actions_view( HTTP_AUTHORIZATION=f"JWT {token}", ) - [workflow_action_in_response] = response.json() - assert "test" in workflow_action_in_response + response_json = response.json() + assert len(response_json) == 1 + assert response_json[0]["type"] == NotificationWorkflowActionType.type + + url = reverse( + "api:builder:domains:list_workflow_actions", + kwargs={"page_id": published_page.id}, + ) + response = api_client.get( + url, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + response_json = response.json() + assert len(response_json) == 1 + assert response_json[0]["type"] == NotificationWorkflowActionType.type @pytest.mark.django_db diff --git a/backend/tests/baserow/contrib/builder/domains/test_domain_handler.py b/backend/tests/baserow/contrib/builder/domains/test_domain_handler.py index 13f286bbc..ed510ca0d 100644 --- a/backend/tests/baserow/contrib/builder/domains/test_domain_handler.py +++ b/backend/tests/baserow/contrib/builder/domains/test_domain_handler.py @@ -9,7 +9,7 @@ from baserow.contrib.builder.domains.handler import DomainHandler from baserow.contrib.builder.domains.models import Domain from baserow.contrib.builder.exceptions import BuilderDoesNotExist from baserow.contrib.builder.models import Builder -from baserow.core.utils import Progress +from baserow.core.utils import Progress, safe_get_or_set_cache @pytest.mark.django_db @@ -171,15 +171,27 @@ def test_domain_publishing(data_fixture): page1 = data_fixture.create_builder_page(builder=builder) page2 = data_fixture.create_builder_page(builder=builder) - element1 = data_fixture.create_builder_heading_element( - page=page1, level=2, value="'foo'" - ) - element2 = data_fixture.create_builder_text_element(page=page1) - element3 = data_fixture.create_builder_heading_element(page=page2) + data_fixture.create_builder_heading_element(page=page1, level=2, value="'foo'") + data_fixture.create_builder_text_element(page=page1) + data_fixture.create_builder_heading_element(page=page2) progress = Progress(100) - DomainHandler().publish(domain1, progress) + domain1 = DomainHandler().publish(domain1, progress) + + # Pretend that someone visited the public builder-by-domain endpoint. + builder_by_domain_cache_key = ( + DomainHandler.get_public_builder_by_domain_version_cache_key( + domain1.domain_name + ) + ) + + version_key = DomainHandler.get_public_builder_by_domain_version_cache_key( + domain1.domain_name + ) + + # We populate the builder domain cache + safe_get_or_set_cache(builder_by_domain_cache_key, version_key, default="before") domain1.refresh_from_db() @@ -192,9 +204,15 @@ def test_domain_publishing(data_fixture): assert progress.progress == progress.total - # Lets publish it a second time. + # Let's publish it a second time. DomainHandler().publish(domain1, progress) + # Following a re-publish, the builder-by-domain cache is invalidated + assert ( + safe_get_or_set_cache(builder_by_domain_cache_key, version_key, default="after") + == "after" + ) + assert Builder.objects.count() == 2 diff --git a/backend/tests/baserow/contrib/builder/pages/test_page_handler.py b/backend/tests/baserow/contrib/builder/pages/test_page_handler.py index 44ef9ec28..f9215a8c1 100644 --- a/backend/tests/baserow/contrib/builder/pages/test_page_handler.py +++ b/backend/tests/baserow/contrib/builder/pages/test_page_handler.py @@ -1,5 +1,6 @@ import pytest +from baserow.contrib.builder.domains.handler import DomainHandler from baserow.contrib.builder.elements.models import ColumnElement, TextElement from baserow.contrib.builder.elements.registries import element_type_registry from baserow.contrib.builder.pages.constants import ILLEGAL_PATH_SAMPLE_CHARACTER @@ -17,6 +18,7 @@ from baserow.contrib.builder.pages.exceptions import ( ) from baserow.contrib.builder.pages.handler import PageHandler from baserow.contrib.builder.pages.models import Page +from baserow.core.user_sources.user_source_user import UserSourceUser @pytest.mark.django_db @@ -517,3 +519,37 @@ def test_validate_query_params_edge_cases(): invalid_params = [{"name": f"filter{char}", "type": "text"}] with pytest.raises(InvalidQueryParamName): handler.validate_query_params(path, path_params, invalid_params) + + +def test_get_page_public_records_cache_key(): + user_with_role = UserSourceUser( + None, None, 1, "username", "foo@bar.com", role="admin" + ) + assert ( + PageHandler.get_page_public_records_cache_key(123, user_with_role, "elements") + == "ab_public_page_123_admin_elements_records" + ) + user_without_role = UserSourceUser(None, None, 1, "username", "foo@bar.com") + assert ( + PageHandler.get_page_public_records_cache_key( + 123, user_without_role, "elements" + ) + == "ab_public_page_123_elements_records" + ) + + +@pytest.mark.django_db +def test_is_published_application_page(data_fixture): + user = data_fixture.create_user() + workspace = data_fixture.create_workspace(user=user) + + builder = data_fixture.create_builder_application(workspace=workspace) + page = data_fixture.create_builder_page(builder=builder) + domain = data_fixture.create_builder_custom_domain(builder=builder) + + domain = DomainHandler().publish(domain) + published_builder = domain.published_to + published_page = published_builder.page_set.get() + + assert not PageHandler()._is_published_application_page(page.id) + assert PageHandler()._is_published_application_page(published_page.id) diff --git a/backend/tests/baserow/contrib/builder/test_builder_handler.py b/backend/tests/baserow/contrib/builder/test_builder_handler.py index 07d43d7ac..19966fcec 100644 --- a/backend/tests/baserow/contrib/builder/test_builder_handler.py +++ b/backend/tests/baserow/contrib/builder/test_builder_handler.py @@ -4,7 +4,10 @@ from django.contrib.auth import get_user_model import pytest -from baserow.contrib.builder.handler import CACHE_KEY_PREFIX, BuilderHandler +from baserow.contrib.builder.handler import ( + USED_PROPERTIES_CACHE_KEY_PREFIX, + BuilderHandler, +) from baserow.core.exceptions import ApplicationDoesNotExist from baserow.core.user_sources.user_source_user import UserSourceUser @@ -46,17 +49,17 @@ def test_get_builder_select_related_theme_config( ( True, "", - f"{CACHE_KEY_PREFIX}_100", + f"{USED_PROPERTIES_CACHE_KEY_PREFIX}_100", ), ( True, "foo_role", - f"{CACHE_KEY_PREFIX}_100", + f"{USED_PROPERTIES_CACHE_KEY_PREFIX}_100", ), ( False, "foo_role", - f"{CACHE_KEY_PREFIX}_100_foo_role", + f"{USED_PROPERTIES_CACHE_KEY_PREFIX}_100_foo_role", ), ], ) diff --git a/backend/tests/baserow/core/test_core_utils_cache.py b/backend/tests/baserow/core/test_core_utils_cache.py index f40e1201c..f202640dc 100755 --- a/backend/tests/baserow/core/test_core_utils_cache.py +++ b/backend/tests/baserow/core/test_core_utils_cache.py @@ -1,5 +1,3 @@ -from django.core.cache import cache - from baserow.core.utils import invalidate_versioned_cache, safe_get_or_set_cache @@ -7,15 +5,13 @@ 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, + timeout=6, ) assert result == "my_default_value" - assert cache.get(cache_key) == "my_default_value" def test_safe_get_or_set_cache_callable_stores_return_value(): @@ -24,7 +20,6 @@ def test_safe_get_or_set_cache_callable_stores_return_value(): """ cache_key = "test_callable_default" - cache.delete(cache_key) def some_callable(): return "callable_value" @@ -32,10 +27,9 @@ def test_safe_get_or_set_cache_callable_stores_return_value(): result = safe_get_or_set_cache( cache_key=cache_key, default=some_callable, - timeout=60, + timeout=6, ) assert result == "callable_value" - assert cache.get(cache_key) == "callable_value" def test_safe_get_or_set_cache_uses_existing_value(): @@ -44,17 +38,20 @@ def test_safe_get_or_set_cache_uses_existing_value(): """ 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="existing_value", + timeout=60, + ) result = safe_get_or_set_cache( cache_key=cache_key, default="unused_default", - timeout=60, + timeout=6, ) + 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(): @@ -65,19 +62,15 @@ def test_versioned_cache_set_and_retrieve(): 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, + timeout=6, ) 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(): @@ -88,45 +81,47 @@ def test_versioned_cache_hit(): 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="already_versioned", + timeout=6, + ) result = safe_get_or_set_cache( cache_key=base_key, version_cache_key=version_cache_key, default="unused_default", - timeout=60, + timeout=6, ) + assert result == "already_versioned" - assert cache.get(full_key) == "already_versioned" -def test_invalidate_versioned_cache_increments_existing(): +def test_versioned_cache_invalidation(): """ - If a version_cache_key already exists, calling invalidate_versioned_cache should - increment the version. + If a versioned key already exists, safe_get_or_set_cache should retrieve + that existing value rather than setting a new one. """ - version_key = "test_invalidate_existing" - cache.set(version_key, 3) + base_key = "test_versioned_base2" + version_cache_key = "test_versioned_key2" - invalidate_versioned_cache(version_key) - assert cache.get(version_key) == 4 + result = safe_get_or_set_cache( + cache_key=base_key, + version_cache_key=version_cache_key, + default="already_versioned", + timeout=6, + ) + invalidate_versioned_cache(version_cache_key) -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. - """ + result = safe_get_or_set_cache( + cache_key=base_key, + version_cache_key=version_cache_key, + default="new_value", + timeout=6, + ) - version_key = "test_invalidate_absent" - cache.delete(version_key) - - invalidate_versioned_cache(version_key) - assert cache.get(version_key) == 1 + assert result == "new_value" diff --git a/changelog/entries/unreleased/feature/3472_builder_improved_the_performance_of_published_applications.json b/changelog/entries/unreleased/feature/3472_builder_improved_the_performance_of_published_applications.json new file mode 100644 index 000000000..441188665 --- /dev/null +++ b/changelog/entries/unreleased/feature/3472_builder_improved_the_performance_of_published_applications.json @@ -0,0 +1,7 @@ +{ + "type": "feature", + "message": "[Builder] Improved the performance of published applications.", + "issue_number": 3472, + "bullet_points": [], + "created_at": "2025-02-25" +} \ No newline at end of file