diff --git a/backend/src/baserow/api/decorators.py b/backend/src/baserow/api/decorators.py index 3438895d8f..532eceb6f5 100755 --- a/backend/src/baserow/api/decorators.py +++ b/backend/src/baserow/api/decorators.py @@ -1,6 +1,6 @@ -import typing from datetime import datetime, timezone from functools import wraps +from typing import Any, Callable, Dict, Optional, Type from zoneinfo import ZoneInfo, ZoneInfoNotFoundError from django.db import OperationalError @@ -249,6 +249,7 @@ def func_wrapper(*args, **kwargs): def validate_body_custom_fields( registry, base_serializer_class=None, + serializer_class_context: Optional[Dict[str, Any]] = None, type_attribute_name="type", partial=False, allow_empty_type=False, @@ -266,6 +267,8 @@ def validate_body_custom_fields( :param base_serializer_class: The base serializer class that will be used when generating the serializer. :type base_serializer_class: ModelSerializer + :param serializer_class_context: If provided, this context will be passed to the + `get_serializer_class` method of the type instance. :param type_attribute_name: The attribute name containing the type value in the request data. :type type_attribute_name: str @@ -301,6 +304,7 @@ def func_wrapper(*args, **kwargs): registry, request.data, base_serializer_class=base_serializer_class, + serializer_class_context=serializer_class_context, type_attribute_name=type_attribute_name, partial=partial, allow_empty_type=allow_empty_type, @@ -400,7 +404,7 @@ def func_wrapper(*args, **kwargs): return validate_decorator -def require_request_data_type(*rtypes: typing.Type) -> typing.Callable: +def require_request_data_type(*rtypes: Type) -> Callable: """ Decorate a view function to restrict allowed request.data to specific types, allowing request.data type checks before actual view is called. This may be diff --git a/backend/src/baserow/api/utils.py b/backend/src/baserow/api/utils.py index 323945e93a..7c05ea963a 100644 --- a/backend/src/baserow/api/utils.py +++ b/backend/src/baserow/api/utils.py @@ -175,6 +175,7 @@ def validate_data( many: bool = False, return_validated: bool = False, instance=None, + context: Optional[Dict[str, Any]] = None, ) -> Dict[str, Any]: """ Validates the provided data via the provided serializer class. If the data doesn't @@ -189,10 +190,13 @@ def validate_data( :param many: Indicates whether the serializer should be constructed as a list. :param return_validated: Returns validated_data from DRF serializer :param instance: The instance that is being updated. + :param context: Optional context dict passed to the serializer. :return: The data after being validated by the serializer. """ - serializer = serializer_class(instance, data=data, partial=partial, many=many) + serializer = serializer_class( + instance, data=data, partial=partial, many=many, context=context or {} + ) if not serializer.is_valid(): detail = serialize_validation_errors_recursive(serializer.errors) raise exception_to_raise(detail) @@ -208,6 +212,7 @@ def validate_data_custom_fields( registry: "Registry", data: Dict[str, Any], base_serializer_class: Optional[Type[ModelSerializer]] = None, + serializer_class_context: Optional[Dict[str, Any]] = None, type_attribute_name: str = "type", partial: bool = False, allow_empty_type: bool = False, @@ -217,12 +222,14 @@ def validate_data_custom_fields( Validates the provided data with the serializer generated by the registry based on the provided type_name and provided base_serializer_class. - :param type_name: The type name of the type instance that is needed to generated + :param type_name: The type name of the type instance that is needed to generate the serializer. :param registry: The registry where to get the type instance from. :param data: The data that needs to be validated. :param base_serializer_class: The base serializer class that is used when generating the serializer for validation. + :param serializer_class_context: If provided, this context will be passed to the + `get_serializer_class` method of the type instance. :param type_attribute_name: The attribute key name that contains the type value. :param partial: Whether the data is a partial update. :param allow_empty_type: Whether the type can be empty. @@ -237,8 +244,8 @@ def validate_data_custom_fields( try: type_instance = registry.get(type_name) except InstanceTypeDoesNotExist: - # If the provided type name doesn't exist we will raise a machine - # readable validation error. + # If the provided type name doesn't exist we will raise a + # machine-readable validation error. raise RequestBodyValidationException( { type_attribute_name: [ @@ -259,7 +266,11 @@ def validate_data_custom_fields( serializer_class = type_instance.get_serializer_class(**serializer_kwargs) return validate_data( - serializer_class, data, partial=partial, return_validated=return_validated + serializer_class, + data, + partial=partial, + return_validated=return_validated, + context=serializer_class_context or {}, ) diff --git a/backend/src/baserow/contrib/automation/api/nodes/views.py b/backend/src/baserow/contrib/automation/api/nodes/views.py index 82a817d3ce..5ab878f8af 100644 --- a/backend/src/baserow/contrib/automation/api/nodes/views.py +++ b/backend/src/baserow/contrib/automation/api/nodes/views.py @@ -44,6 +44,7 @@ from baserow.contrib.automation.api.workflows.errors import ( ERROR_AUTOMATION_WORKFLOW_DOES_NOT_EXIST, ) +from baserow.contrib.automation.application_types import AutomationApplicationType from baserow.contrib.automation.nodes.actions import ( CreateAutomationNodeActionType, DeleteAutomationNodeActionType, @@ -244,6 +245,7 @@ def patch(self, request, node_id: int): automation_node_type_registry, request.data, base_serializer_class=UpdateAutomationNodeSerializer, + serializer_class_context={"application_type": AutomationApplicationType}, partial=True, return_validated=True, ) diff --git a/backend/src/baserow/contrib/automation/application_types.py b/backend/src/baserow/contrib/automation/application_types.py index a66a1c2d08..2030f706dd 100644 --- a/backend/src/baserow/contrib/automation/application_types.py +++ b/backend/src/baserow/contrib/automation/application_types.py @@ -12,6 +12,9 @@ AutomationApplicationTypeInitApplication, ) from baserow.contrib.automation.constants import IMPORT_SERIALIZED_IMPORTING +from baserow.contrib.automation.data_providers.registries import ( + automation_data_provider_type_registry, +) from baserow.contrib.automation.models import Automation, AutomationWorkflow from baserow.contrib.automation.operations import ListAutomationWorkflowsOperationType from baserow.contrib.automation.types import AutomationDict @@ -41,6 +44,7 @@ class AutomationApplicationType(ApplicationType): supports_integrations = True request_serializer_field_names = [] serializer_mixins = [lazy_get_instance_serializer_class] + data_provider_type_registry = automation_data_provider_type_registry # Automation applications are imported third (after database, builder) import_application_priority = 0 diff --git a/backend/src/baserow/contrib/builder/api/data_sources/views.py b/backend/src/baserow/contrib/builder/api/data_sources/views.py index c0db804981..a053e66a69 100644 --- a/backend/src/baserow/contrib/builder/api/data_sources/views.py +++ b/backend/src/baserow/contrib/builder/api/data_sources/views.py @@ -50,6 +50,7 @@ ) from baserow.contrib.builder.api.elements.errors import ERROR_ELEMENT_DOES_NOT_EXIST from baserow.contrib.builder.api.pages.errors import ERROR_PAGE_DOES_NOT_EXIST +from baserow.contrib.builder.application_types import BuilderApplicationType from baserow.contrib.builder.data_sources.builder_dispatch_context import ( BuilderDispatchContext, ) @@ -310,6 +311,7 @@ def patch(self, request, data_source_id: int): service_type_registry, request.data, base_serializer_class=UpdateDataSourceSerializer, + serializer_class_context={"application_type": BuilderApplicationType}, return_validated=True, ) diff --git a/backend/src/baserow/contrib/builder/api/elements/serializers.py b/backend/src/baserow/contrib/builder/api/elements/serializers.py index e148edec2a..33381d02af 100644 --- a/backend/src/baserow/contrib/builder/api/elements/serializers.py +++ b/backend/src/baserow/contrib/builder/api/elements/serializers.py @@ -317,7 +317,7 @@ def to_representation(self, instance): else: instance_type = self.get_type_from_instance(instance) - serializer = instance_type.get_serializer(instance) + serializer = instance_type.get_serializer(instance, context=self.context) if isinstance(instance, Mapping): ret = serializer.to_representation(instance["config"]) @@ -359,9 +359,9 @@ def to_internal_value(self, data): code="INVALID_FIELD_PROPERTY", ) - ret = instance_type.get_serializer(field_config_data).to_internal_value( - field_config_data - ) + ret = instance_type.get_serializer( + field_config_data, context=self.context + ).to_internal_value(field_config_data) data["config"] = ret diff --git a/backend/src/baserow/contrib/builder/api/elements/views.py b/backend/src/baserow/contrib/builder/api/elements/views.py index 554872ba04..af86d6e8cb 100644 --- a/backend/src/baserow/contrib/builder/api/elements/views.py +++ b/backend/src/baserow/contrib/builder/api/elements/views.py @@ -38,6 +38,7 @@ UpdateElementSerializer, ) from baserow.contrib.builder.api.pages.errors import ERROR_PAGE_DOES_NOT_EXIST +from baserow.contrib.builder.application_types import BuilderApplicationType from baserow.contrib.builder.data_sources.exceptions import DataSourceDoesNotExist from baserow.contrib.builder.elements.exceptions import ( CollectionElementPropertyOptionsNotUnique, @@ -148,7 +149,9 @@ def get(self, request, page_id): } ) @validate_body_custom_fields( - element_type_registry, base_serializer_class=CreateElementSerializer + element_type_registry, + base_serializer_class=CreateElementSerializer, + serializer_class_context={"application_type": BuilderApplicationType}, ) def post(self, request, data: Dict, page_id: int): """Creates a new element.""" @@ -228,6 +231,7 @@ def patch(self, request, element_id: int): element_type_registry, request.data, base_serializer_class=UpdateElementSerializer, + serializer_class_context={"application_type": BuilderApplicationType}, partial=True, return_validated=True, ) diff --git a/backend/src/baserow/contrib/builder/api/workflow_actions/views.py b/backend/src/baserow/contrib/builder/api/workflow_actions/views.py index f35488a91d..b7fd70dc43 100644 --- a/backend/src/baserow/contrib/builder/api/workflow_actions/views.py +++ b/backend/src/baserow/contrib/builder/api/workflow_actions/views.py @@ -42,6 +42,7 @@ OrderWorkflowActionsSerializer, UpdateBuilderWorkflowActionsSerializer, ) +from baserow.contrib.builder.application_types import BuilderApplicationType from baserow.contrib.builder.data_sources.builder_dispatch_context import ( BuilderDispatchContext, ) @@ -285,6 +286,7 @@ def patch(self, request, workflow_action_id: int): builder_workflow_action_type_registry, request.data, base_serializer_class=UpdateBuilderWorkflowActionsSerializer, + serializer_class_context={"application_type": BuilderApplicationType}, partial=True, ) diff --git a/backend/src/baserow/contrib/builder/application_types.py b/backend/src/baserow/contrib/builder/application_types.py index 25150b476f..a6a26b88f9 100755 --- a/backend/src/baserow/contrib/builder/application_types.py +++ b/backend/src/baserow/contrib/builder/application_types.py @@ -17,6 +17,9 @@ BuilderApplicationTypeInitApplication, ) from baserow.contrib.builder.constants import IMPORT_SERIALIZED_IMPORTING +from baserow.contrib.builder.data_providers.registries import ( + builder_data_provider_type_registry, +) from baserow.contrib.builder.models import Builder from baserow.contrib.builder.operations import ListPagesBuilderOperationType from baserow.contrib.builder.pages.handler import PageHandler @@ -78,6 +81,7 @@ class BuilderApplicationType(ApplicationType): serializer_mixins = [lazy_get_instance_serializer_class] public_serializer_mixins = [lazy_get_instance_public_serializer_class] request_serializer_mixins = [] + data_provider_type_registry = builder_data_provider_type_registry # Builder applications are imported second. import_application_priority = 1 diff --git a/backend/src/baserow/contrib/database/api/fields/views.py b/backend/src/baserow/contrib/database/api/fields/views.py index 0225d8750b..32f237c1ed 100644 --- a/backend/src/baserow/contrib/database/api/fields/views.py +++ b/backend/src/baserow/contrib/database/api/fields/views.py @@ -68,6 +68,7 @@ ) from baserow.contrib.database.api.tokens.authentications import TokenAuthentication from baserow.contrib.database.api.tokens.errors import ERROR_NO_PERMISSION_TO_TABLE +from baserow.contrib.database.application_types import DatabaseApplicationType from baserow.contrib.database.fields.actions import ( ChangePrimaryFieldActionType, CreateFieldActionType, @@ -278,6 +279,7 @@ def get(self, request, table_id): @validate_body_custom_fields( field_type_registry, base_serializer_class=CreateFieldSerializer, + serializer_class_context={"application_type": DatabaseApplicationType}, ) @map_exceptions( { @@ -463,6 +465,7 @@ def patch(self, request, field_id): field_type_registry, request.data, base_serializer_class=UpdateFieldSerializer, + serializer_class_context={"application_type": DatabaseApplicationType}, ) # Because each field type can raise custom exceptions at while updating the diff --git a/backend/src/baserow/contrib/database/application_types.py b/backend/src/baserow/contrib/database/application_types.py index e641eb34fc..a01b6719db 100755 --- a/backend/src/baserow/contrib/database/application_types.py +++ b/backend/src/baserow/contrib/database/application_types.py @@ -44,6 +44,7 @@ IMPORT_SERIALIZED_IMPORTING_TABLE_DATA, IMPORT_SERIALIZED_IMPORTING_TABLE_STRUCTURE, ) +from .data_providers.registries import database_data_provider_type_registry from .data_sync.registries import data_sync_type_registry from .db.atomic import read_repeatable_single_database_atomic_transaction from .export_serialized import DatabaseExportSerializedStructure @@ -71,6 +72,7 @@ class DatabaseApplicationType(ApplicationType): # Mark the request serializer field names as empty, otherwise # the polymorphic request serializer will try and serialize tables. request_serializer_field_names = [] + data_provider_type_registry = database_data_provider_type_registry # Database applications are imported first. import_application_priority = 2 diff --git a/backend/src/baserow/contrib/integrations/local_baserow/api/serializers.py b/backend/src/baserow/contrib/integrations/local_baserow/api/serializers.py index f878ef52b2..14207571bd 100644 --- a/backend/src/baserow/contrib/integrations/local_baserow/api/serializers.py +++ b/backend/src/baserow/contrib/integrations/local_baserow/api/serializers.py @@ -35,6 +35,7 @@ def to_representation(self, instance): representation = super().to_representation(instance) representation["sortings"] = LocalBaserowTableServiceSortSerializer( instance.service_sorts.all(), + context=self.context, many=True, ).data return representation @@ -44,7 +45,9 @@ def to_internal_value(self, data): data = super().to_internal_value(data) if sortings is not None: data["service_sorts"] = [ - LocalBaserowTableServiceSortSerializer().to_internal_value(ss) + LocalBaserowTableServiceSortSerializer( + context=self.context + ).to_internal_value(ss) for ss in sortings ] return data @@ -93,6 +96,7 @@ def to_representation(self, instance): representation["filters"] = LocalBaserowTableServiceFilterSerializer( instance.service_filters.all(), many=True, + context=self.context, ).data return representation @@ -101,7 +105,9 @@ def to_internal_value(self, data): data = super().to_internal_value(data) if filters is not None: data["service_filters"] = [ - LocalBaserowTableServiceFilterSerializer().to_internal_value(sf) + LocalBaserowTableServiceFilterSerializer( + context=self.context + ).to_internal_value(sf) for sf in filters ] return data diff --git a/backend/src/baserow/core/formula/__init__.py b/backend/src/baserow/core/formula/__init__.py index 627cdb61f0..fb5d5a542a 100644 --- a/backend/src/baserow/core/formula/__init__.py +++ b/backend/src/baserow/core/formula/__init__.py @@ -24,8 +24,10 @@ BaserowFormulaSyntaxError, ] +from baserow.core.formula.parser.formula_execution_visitor import ( + BaserowFormulaExecutionVisitor, +) from baserow.core.formula.parser.parser import get_parse_tree_for_formula -from baserow.core.formula.parser.python_executor import BaserowPythonExecutor def resolve_formula( @@ -51,4 +53,4 @@ def resolve_formula( return formula["formula"] tree = get_parse_tree_for_formula(formula["formula"]) - return BaserowPythonExecutor(functions, formula_context).visit(tree) + return BaserowFormulaExecutionVisitor(functions, formula_context).visit(tree) diff --git a/backend/src/baserow/core/formula/argument_types.py b/backend/src/baserow/core/formula/argument_types.py index f2c42571ae..34bb1ab273 100644 --- a/backend/src/baserow/core/formula/argument_types.py +++ b/backend/src/baserow/core/formula/argument_types.py @@ -87,13 +87,13 @@ def parse(self, value): class BooleanBaserowRuntimeFormulaArgumentType(BaserowRuntimeFormulaArgumentType): def test(self, value): try: - ensure_boolean(value) + ensure_boolean(value, False) return True except ValidationError: return False def parse(self, value): - return ensure_boolean(value) + return ensure_boolean(value, False) class TimezoneBaserowRuntimeFormulaArgumentType(BaserowRuntimeFormulaArgumentType): diff --git a/backend/src/baserow/core/formula/parser/exceptions.py b/backend/src/baserow/core/formula/parser/exceptions.py index 624edcfa9e..5c94684b28 100644 --- a/backend/src/baserow/core/formula/parser/exceptions.py +++ b/backend/src/baserow/core/formula/parser/exceptions.py @@ -15,9 +15,17 @@ def __init__(self, function_def, num_args): error_prefix = "1 argument was" else: error_prefix = f"{num_args} arguments were" + + if function_def.num_args is None: + # This function doesn't take a specific set of `args`, but instead a + # variable number of arguments with a minimum number of args. + expected = f"at least {function_def.min_args}" + else: + expected = str(function_def.num_args) + super().__init__( - f"{error_prefix} given to the {function_def}, it must instead " - f"be given {function_def.num_args}" + f"{error_prefix} given to the '{function_def.type}' function, it must " + f"instead be given {expected}" ) @@ -51,8 +59,8 @@ def __init__(self): class UnknownOperator(BaserowFormulaException): - def __init__(self, operatorText): - super().__init__(f"it used the unknown operator {operatorText}") + def __init__(self, operator_text): + super().__init__(f"it used the unknown operator {operator_text}") class BaserowFormulaSyntaxError(BaserowFormulaException): diff --git a/backend/src/baserow/core/formula/parser/python_executor.py b/backend/src/baserow/core/formula/parser/formula_execution_visitor.py similarity index 97% rename from backend/src/baserow/core/formula/parser/python_executor.py rename to backend/src/baserow/core/formula/parser/formula_execution_visitor.py index a4c5d93e23..6609aa9d29 100644 --- a/backend/src/baserow/core/formula/parser/python_executor.py +++ b/backend/src/baserow/core/formula/parser/formula_execution_visitor.py @@ -12,7 +12,7 @@ ) -class BaserowPythonExecutor(BaserowFormulaVisitor): +class BaserowFormulaExecutionVisitor(BaserowFormulaVisitor): def __init__( self, functions: FunctionCollection, @@ -54,11 +54,7 @@ def visitFunctionCall(self, ctx: BaserowFormula.FunctionCallContext): def _do_func(self, function_argument_expressions, function_name: str): args = [expr.accept(self) for expr in function_argument_expressions] formula_function_type = self._get_formula_function_type(function_name) - - formula_function_type.validate_args(args) - args_parsed = formula_function_type.parse_args(args) - return formula_function_type.execute(self.context, args_parsed) def _get_formula_function_type(self, function_name: str) -> FormulaFunction: diff --git a/backend/src/baserow/core/formula/parser/formula_validation_visitor.py b/backend/src/baserow/core/formula/parser/formula_validation_visitor.py new file mode 100644 index 0000000000..9cfd8b4d38 --- /dev/null +++ b/backend/src/baserow/core/formula/parser/formula_validation_visitor.py @@ -0,0 +1,200 @@ +from typing import TYPE_CHECKING, List, Optional + +from baserow.core.formula.exceptions import InvalidRuntimeFormula +from baserow.core.formula.parser.exceptions import ( + FieldByIdReferencesAreDeprecated, + FormulaFunctionTypeDoesNotExist, + InvalidNumberOfArguments, + UnknownOperator, +) +from baserow.core.formula.parser.generated.BaserowFormula import BaserowFormula +from baserow.core.formula.parser.generated.BaserowFormulaVisitor import ( + BaserowFormulaVisitor, +) + +if TYPE_CHECKING: + from baserow.core.formula import FunctionCollection + from baserow.core.formula.registries import DataProviderTypeRegistry + + +class DeferredValue: + """ + Marker class representing a value that will be resolved at execution time. + + During validation, when we encounter nested function calls (e.g., `is_even(get('foo.bar'))`), + the inner function's return value isn't available yet. Instead of failing validation + because we can't type-check an unknown value, we return this marker to indicate + "this will be a valid value at runtime, skip type validation for now." + """ + + pass + + +class BaserowFormulaValidationVisitor(BaserowFormulaVisitor): + """ + A Baserow formula visitor which is responsible for validating a formula's + function and its arguments. + """ + + def __init__( + self, + functions: "FunctionCollection", + data_provider_type_registry: Optional["DataProviderTypeRegistry"] = None, + ): + self.functions = functions + self.data_provider_type_registry = data_provider_type_registry + + def visitRoot(self, ctx: BaserowFormula.RootContext): + return ctx.expr().accept(self) + + def visitStringLiteral(self, ctx: BaserowFormula.StringLiteralContext): + # noinspection PyTypeChecker + return self.process_string(ctx) + + def visitBinaryOp(self, ctx: BaserowFormula.BinaryOpContext): + if ctx.PLUS(): + op = "add" + elif ctx.MINUS(): + op = "minus" + elif ctx.SLASH(): + op = "divide" + elif ctx.EQUAL(): + op = "equal" + elif ctx.BANG_EQUAL(): + op = "not_equal" + elif ctx.STAR(): + op = "multiply" + elif ctx.GT(): + op = "greater_than" + elif ctx.LT(): + op = "less_than" + elif ctx.GTE(): + op = "greater_than_or_equal" + elif ctx.LTE(): + op = "less_than_or_equal" + elif ctx.AMP_AMP(): + op = "and" + elif ctx.PIPE_PIPE(): + op = "or" + else: + raise UnknownOperator(ctx.getText()) + + return self.visitFunctionCall(ctx, op) + + def process_string(self, ctx): + literal_without_outer_quotes = ctx.getText()[1:-1] + if ctx.SINGLEQ_STRING_LITERAL() is not None: + literal = literal_without_outer_quotes.replace("\\'", "'") + else: + literal = literal_without_outer_quotes.replace('\\"', '"') + return literal + + def visitDecimalLiteral(self, ctx: BaserowFormula.DecimalLiteralContext): + return float(ctx.getText()) + + def visitBooleanLiteral(self, ctx: BaserowFormula.BooleanLiteralContext): + return ctx.TRUE() is not None + + def visitBrackets(self, ctx: BaserowFormula.BracketsContext): + return ctx.expr().accept(self) + + def visitIdentifier(self, ctx: BaserowFormula.IdentifierContext): + return ctx.getText() + + def visitIntegerLiteral(self, ctx: BaserowFormula.IntegerLiteralContext): + return int(ctx.getText()) + + def visitFieldByIdReference(self, ctx: BaserowFormula.FieldByIdReferenceContext): + raise FieldByIdReferencesAreDeprecated() + + def visitLeftWhitespaceOrComments( + self, ctx: BaserowFormula.LeftWhitespaceOrCommentsContext + ): + return ctx.expr().accept(self) + + def visitRightWhitespaceOrComments( + self, ctx: BaserowFormula.RightWhitespaceOrCommentsContext + ): + return ctx.expr().accept(self) + + def visitFieldReference(self, ctx: BaserowFormula.FieldReferenceContext): + """ + Handle field('name') syntax. There is no native support for this function + in non-database formulas, so we raise an error. + """ + + raise InvalidRuntimeFormula("'field' is not a a supported function") + + def _parse_args_for_validation( + self, formula_function_type, accepted_args: List + ) -> List: + """ + Parse arguments for validation, skipping DeferredValue instances. + + During validation, nested function calls return DeferredValue markers + since their actual values aren't available yet. We pass these through + unchanged rather than attempting to parse/cast them. + + :param formula_function_type: The function type with arg definitions. + :param accepted_args: The arguments from visiting child expressions. + :return: Parsed arguments with DeferredValue instances preserved. + """ + + if formula_function_type.args is None: + return accepted_args + + result = [] + for index, arg in enumerate(accepted_args): + if isinstance(arg, DeferredValue): + # Preserve deferred values - they'll be resolved at execution time + result.append(arg) + elif index < len(formula_function_type.args): + result.append(formula_function_type.args[index].parse(arg)) + else: + result.append(arg) + return result + + def visitFunctionCall( + self, ctx: BaserowFormula.FunctionCallContext, function_name: str = None + ): + """ + Visits a function call node in the parse tree. For each function we encounter, + we validate its args using the corresponding function type's `validate_args` + method. + + :param ctx: The function call context from the parse tree. + :param function_name: Optional function name to use instead of + the one in the context. Mainly used for operator visits. + :raises InvalidNumberOfArguments: If the number of arguments provided to the + function does not match the expected number. + :return: DeferredValue marker to indicate this function's result will be + resolved at execution time. + """ + + accepted_args = [expr.accept(self) for expr in ctx.expr()] + function_name = function_name or ctx.func_name().getText().lower() + try: + formula_function_type = self.functions.get(function_name) + except FormulaFunctionTypeDoesNotExist: + raise InvalidRuntimeFormula(f"Unsupported function '{function_name}'.") + if not formula_function_type.validate_number_of_args(accepted_args): + raise InvalidNumberOfArguments(formula_function_type, len(accepted_args)) + + args_parsed = self._parse_args_for_validation( + formula_function_type, accepted_args + ) + # Only run validate_args if none of the arguments are DeferredValue. + # DeferredValue represents nested function calls whose values aren't + # available until execution time, so we can't type-check them. + has_deferred = any(isinstance(arg, DeferredValue) for arg in args_parsed) + if not has_deferred: + formula_function_type.validate_args( + args_parsed, + validation_context={ + "data_provider_type_registry": self.data_provider_type_registry + }, + ) + + # Return DeferredValue so parent function calls know this arg's value + # will only be available at execution time + return DeferredValue() diff --git a/backend/src/baserow/core/formula/registries.py b/backend/src/baserow/core/formula/registries.py index 85a735af78..2639643449 100644 --- a/backend/src/baserow/core/formula/registries.py +++ b/backend/src/baserow/core/formula/registries.py @@ -5,7 +5,6 @@ from baserow.core.formula.parser.exceptions import ( FormulaFunctionTypeDoesNotExist, InvalidFormulaArgumentType, - InvalidNumberOfArguments, ) from baserow.core.formula.types import ( FormulaArg, @@ -20,6 +19,10 @@ class RuntimeFormulaFunction(ABC, Instance): + # The minimum number of arguments that the function expects. This value + # will only be used if `args` is not defined. + min_args = None + @classmethod @property @abstractmethod @@ -61,17 +64,21 @@ def execute(self, context: FormulaContext, args: FormulaArgs) -> Any: pass - def validate_args(self, args: FormulaArgs): + def validate_args( + self, + args: FormulaArgs, + validation_context: Optional[Dict[str, Any]] = None, + ): """ This function can be called to validate all arguments given to the formula :param args: The arguments provided to the formula - :raises InvalidNumberOfArguments: If the number of arguments is invalid + :param validation_context: An optional context that can be used during + validation. Can provide additional information that is not available in the + args themselves. :raises InvalidFormulaArgumentType: If any of the arguments have a wrong type """ - if not self.validate_number_of_args(args): - raise InvalidNumberOfArguments(self, args) invalid_arg = self.validate_type_of_args(args) if invalid_arg: raise InvalidFormulaArgumentType(self, invalid_arg) @@ -90,7 +97,7 @@ def validate_number_of_args(self, args: FormulaArgs) -> bool: required_args = len([arg for arg in self.args if not arg.optional]) total_args = len(self.args) - return len(args) >= required_args and len(args) <= total_args + return required_args <= len(args) <= total_args def validate_type_of_args(self, args: FormulaArgs) -> Optional[FormulaArg]: """ @@ -170,6 +177,13 @@ def import_path( return path + def is_valid(self, path: List[str]) -> bool: + """ + Allows to hook into the path validation process. Not currently implemented. + + :param path: the list of path strings after the provider name. + """ + def extract_properties( self, path: List[str], diff --git a/backend/src/baserow/core/formula/runtime_formula_types.py b/backend/src/baserow/core/formula/runtime_formula_types.py index 85de1b5964..6fbbb046d7 100644 --- a/backend/src/baserow/core/formula/runtime_formula_types.py +++ b/backend/src/baserow/core/formula/runtime_formula_types.py @@ -1,10 +1,12 @@ import random import uuid -from typing import Optional +from typing import Any, Dict, Optional from zoneinfo import ZoneInfo from django.utils import timezone +from baserow.core.exceptions import InstanceTypeDoesNotExist +from baserow.core.formula import BaserowFormulaSyntaxError from baserow.core.formula.argument_types import ( AnyBaserowRuntimeFormulaArgumentType, ArrayOfNumbersBaserowRuntimeFormulaArgumentType, @@ -19,10 +21,12 @@ from baserow.core.formula.types import FormulaArg, FormulaArgs, FormulaContext from baserow.core.formula.utils.date import convert_date_format_moment_to_python from baserow.core.formula.validator import ensure_array, ensure_string +from baserow.core.utils import to_path class RuntimeConcat(RuntimeFormulaFunction): type = "concat" + min_args = 2 def validate_type_of_args(self, args) -> Optional[FormulaArg]: arg_type = TextBaserowRuntimeFormulaArgumentType() @@ -32,7 +36,7 @@ def validate_type_of_args(self, args) -> Optional[FormulaArg]: ) def validate_number_of_args(self, args): - return len(args) > 1 + return len(args) >= self.min_args def execute(self, context: FormulaContext, args: FormulaArgs): return "".join([ensure_string(a) for a in args]) @@ -42,6 +46,34 @@ class RuntimeGet(RuntimeFormulaFunction): type = "get" args = [TextBaserowRuntimeFormulaArgumentType()] + def validate_args( + self, + args: FormulaArgs, + validation_context: Optional[Dict[str, Any]] = None, + ): + super().validate_args(args) + + provider_name, *rest = to_path(args[0]) + if not provider_name: + raise BaserowFormulaSyntaxError( + f"The '{self.type}' function arguments " + "must start with a formula provider name." + ) + + data_provider_type_registry = validation_context["data_provider_type_registry"] + try: + provider = data_provider_type_registry.get(provider_name) + except InstanceTypeDoesNotExist: + # Re-raise the exception but with a more precise message. We + # are at a stage where we have enough variables that we can + # precisely say where the argument is wrong. + raise InstanceTypeDoesNotExist( + provider_name, + f"The formula provider '{provider_name}' " + f"used in '{args[0]}' does not exist in this module.", + ) + provider.is_valid(rest) + def execute(self, context: FormulaContext, args: FormulaArgs): return context[args[0]] diff --git a/backend/src/baserow/core/formula/serializers.py b/backend/src/baserow/core/formula/serializers.py index ad01b16591..f4e7343ecd 100644 --- a/backend/src/baserow/core/formula/serializers.py +++ b/backend/src/baserow/core/formula/serializers.py @@ -5,9 +5,17 @@ from rest_framework import serializers from rest_framework.exceptions import ValidationError +from baserow.core.exceptions import InstanceTypeDoesNotExist from baserow.core.formula.field import BASEROW_FORMULA_VERSION_INITIAL -from baserow.core.formula.parser.exceptions import BaserowFormulaSyntaxError +from baserow.core.formula.parser.exceptions import ( + BaserowFormulaSyntaxError, + InvalidNumberOfArguments, +) +from baserow.core.formula.parser.formula_validation_visitor import ( + BaserowFormulaValidationVisitor, +) from baserow.core.formula.parser.parser import get_parse_tree_for_formula +from baserow.core.formula.registries import formula_runtime_function_registry from baserow.core.formula.types import ( BASEROW_FORMULA_MODE_ADVANCED, BASEROW_FORMULA_MODE_RAW, @@ -112,8 +120,35 @@ def to_internal_value(self, data: Union[str, Dict[str, str]]): if not data["formula"] or data["mode"] == BASEROW_FORMULA_MODE_RAW: return data + # Inspect the `context` for an `ApplicationType`, we'll need to + # determine what this application type's data provider registry is, + # so we can validate the formula + context = self.context or {} + if context.get("application_type") is None: + raise ValidationError( + "The formula serializer field requires an application type " + "context to validate the formula arguments.", + code="missing_context", + ) + try: - get_parse_tree_for_formula(data["formula"]) + tree = get_parse_tree_for_formula(data["formula"]) + try: + BaserowFormulaValidationVisitor( + formula_runtime_function_registry, + data_provider_type_registry=self.context.get( + "application_type" + )().data_provider_type_registry, + ).visit(tree) + except ( + BaserowFormulaSyntaxError, + InvalidNumberOfArguments, + InstanceTypeDoesNotExist, + ) as exc: + raise ValidationError( + exc.args[0], + code="invalid_formula_argument", + ) from exc return data except BaserowFormulaSyntaxError as e: raise ValidationError(f"The formula is invalid: {e}", code="invalid") diff --git a/backend/src/baserow/core/registries.py b/backend/src/baserow/core/registries.py index c0dfefd888..5faec48619 100755 --- a/backend/src/baserow/core/registries.py +++ b/backend/src/baserow/core/registries.py @@ -317,6 +317,10 @@ def get_api_urls(self): # value. If this property is not overridden, then the instance is imported last. import_application_priority = 0 + # The data provider type registry applicable to this type. Must be set + # by subclasses for runtime formula validation to work correctly. + data_provider_type_registry = None + def prepare_value_for_db(self, values: dict, instance: "Application | None" = None): """ This function allows you to hook into the moment an application is created or diff --git a/backend/src/baserow/core/services/formula_importer.py b/backend/src/baserow/core/services/formula_importer.py index cf55b43210..d40e000857 100644 --- a/backend/src/baserow/core/services/formula_importer.py +++ b/backend/src/baserow/core/services/formula_importer.py @@ -1,7 +1,10 @@ from abc import ABC, abstractmethod from baserow.core.formula import BaserowFormula, BaserowFormulaVisitor -from baserow.core.formula.parser.exceptions import FieldByIdReferencesAreDeprecated +from baserow.core.formula.parser.exceptions import ( + BaserowFormulaSyntaxError, + FieldByIdReferencesAreDeprecated, +) from baserow.core.utils import to_path @@ -66,7 +69,7 @@ def _do_func_import(self, function_argument_expressions, function_name: str): return f"{function_name}({','.join(args)})" def visitBinaryOp(self, ctx: BaserowFormula.BinaryOpContext): - args = [expr.accept(self) for expr in (ctx.expr())] + args = [expr.accept(self) for expr in ctx.expr()] return f" {ctx.op.text} ".join(args) def visitFunc_name(self, ctx: BaserowFormula.Func_nameContext): @@ -78,6 +81,14 @@ def visitIdentifier(self, ctx: BaserowFormula.IdentifierContext): def visitIntegerLiteral(self, ctx: BaserowFormula.IntegerLiteralContext): return ctx.getText() + def visitFieldReference(self, ctx: BaserowFormula.FieldReferenceContext): + """ + Handle field('name') syntax. There is no native support for this function + in services, so we raise an error. + """ + + raise BaserowFormulaSyntaxError("'field' is not a a supported function") + def visitFieldByIdReference(self, ctx: BaserowFormula.FieldByIdReferenceContext): raise FieldByIdReferencesAreDeprecated() diff --git a/backend/src/baserow/test_utils/fixtures/automation_node.py b/backend/src/baserow/test_utils/fixtures/automation_node.py index 0be5f25570..4b2c10c8f0 100644 --- a/backend/src/baserow/test_utils/fixtures/automation_node.py +++ b/backend/src/baserow/test_utils/fixtures/automation_node.py @@ -15,6 +15,7 @@ CoreRouterActionNodeType, LocalBaserowCreateRowNodeType, LocalBaserowDeleteRowNodeType, + LocalBaserowGetRowNodeType, LocalBaserowRowsCreatedNodeTriggerType, LocalBaserowUpdateRowNodeType, ) @@ -111,6 +112,13 @@ def create_local_baserow_delete_row_action_node(self, user=None, **kwargs): **kwargs, ) + def create_local_baserow_get_row_action_node(self, user=None, **kwargs): + return self.create_automation_node( + user=user, + type=LocalBaserowGetRowNodeType.type, + **kwargs, + ) + def create_core_iterator_action_node( self, user=None, **kwargs ) -> CoreIteratorActionNode: diff --git a/backend/tests/baserow/api/formula/test_formula_serializers.py b/backend/tests/baserow/api/formula/test_formula_serializers.py new file mode 100644 index 0000000000..bfb2baeeaa --- /dev/null +++ b/backend/tests/baserow/api/formula/test_formula_serializers.py @@ -0,0 +1,24 @@ +import pytest +from rest_framework.exceptions import ValidationError + +from baserow.core.formula.field import BASEROW_FORMULA_VERSION_INITIAL +from baserow.core.formula.serializers import FormulaSerializerField +from baserow.core.formula.types import BASEROW_FORMULA_MODE_SIMPLE + + +@pytest.mark.parametrize("context", [None, {}, {"application_type": None}]) +def test_formula_serializer_field_without_context(context): + with pytest.raises(ValidationError) as exc: + field = FormulaSerializerField() + field._context = context + field.to_internal_value( + { + "formula": "get('data_source.123.field_456')", + "version": BASEROW_FORMULA_VERSION_INITIAL, + "mode": BASEROW_FORMULA_MODE_SIMPLE, + } + ) + assert str(exc.value.detail[0]) == ( + "The formula serializer field requires " + "an application type context to validate the formula arguments." + ) diff --git a/backend/tests/baserow/contrib/automation/api/nodes/test_nodes_views.py b/backend/tests/baserow/contrib/automation/api/nodes/test_nodes_views.py index 095c823564..441cc86d3e 100644 --- a/backend/tests/baserow/contrib/automation/api/nodes/test_nodes_views.py +++ b/backend/tests/baserow/contrib/automation/api/nodes/test_nodes_views.py @@ -375,7 +375,6 @@ def test_duplicate_node_invalid_node(api_client, data_fixture): def test_update_node(api_client, data_fixture): user, token = data_fixture.create_user_and_token() workflow = data_fixture.create_automation_workflow(user) - trigger = workflow.get_trigger() node = data_fixture.create_automation_node(user=user, workflow=workflow) assert node.label == "" @@ -394,6 +393,38 @@ def test_update_node(api_client, data_fixture): } +@pytest.mark.django_db +def test_updating_node_with_invalid_formula_arguments_throws_error( + api_client, data_fixture +): + user, token = data_fixture.create_user_and_token() + workflow = data_fixture.create_automation_workflow(user) + node = data_fixture.create_local_baserow_get_row_action_node( + user=user, workflow=workflow + ) + service_type = node.service.get_type() + response = api_client.patch( + reverse(API_URL_ITEM, kwargs={"node_id": node.id}), + {"service": {"type": service_type.type, "row_id": "get('foobar.123')"}}, + **get_api_kwargs(token), + ) + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json() == { + "error": "ERROR_REQUEST_BODY_VALIDATION", + "detail": { + "service": { + "row_id": [ + { + "error": "The formula provider 'foobar' used " + "in 'foobar.123' does not exist in this module.", + "code": "invalid_formula_argument", + } + ] + } + }, + } + + @pytest.mark.django_db def test_update_node_invalid_node(api_client, data_fixture): _, token = data_fixture.create_user_and_token() diff --git a/backend/tests/baserow/contrib/builder/api/data_sources/test_data_source_views.py b/backend/tests/baserow/contrib/builder/api/data_sources/test_data_source_views.py index bcf09e7d67..f6ac0212ae 100644 --- a/backend/tests/baserow/contrib/builder/api/data_sources/test_data_source_views.py +++ b/backend/tests/baserow/contrib/builder/api/data_sources/test_data_source_views.py @@ -257,6 +257,43 @@ def test_update_data_source(api_client, data_fixture): assert response.json()["name"] == "name test" +@pytest.mark.django_db +def test_updating_data_source_with_invalid_formula_arguments_throws_error( + api_client, data_fixture +): + user, token = data_fixture.create_user_and_token() + page = data_fixture.create_builder_page(user=user) + table = data_fixture.create_database_table(user=user) + data_source = data_fixture.create_builder_local_baserow_get_row_data_source( + page=page + ) + url = reverse( + "api:builder:data_source:item", kwargs={"data_source_id": data_source.id} + ) + response = api_client.patch( + url, + { + "table_id": table.id, + "row_id": "get('foobar.123')", + }, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json() == { + "error": "ERROR_REQUEST_BODY_VALIDATION", + "detail": { + "row_id": [ + { + "error": "The formula provider 'foobar' used " + "in 'foobar.123' does not exist in this module.", + "code": "invalid_formula_argument", + } + ] + }, + } + + @pytest.mark.django_db def test_update_data_source_page(api_client, data_fixture): user, token = data_fixture.create_user_and_token() diff --git a/backend/tests/baserow/contrib/builder/api/elements/test_element_views.py b/backend/tests/baserow/contrib/builder/api/elements/test_element_views.py index 3a66b7f3dc..f055465fac 100644 --- a/backend/tests/baserow/contrib/builder/api/elements/test_element_views.py +++ b/backend/tests/baserow/contrib/builder/api/elements/test_element_views.py @@ -258,6 +258,36 @@ def test_update_element(api_client, data_fixture): assert response.json()["level"] == 3 +@pytest.mark.django_db +def test_updating_element_with_invalid_formula_arguments_throws_error( + api_client, data_fixture +): + user, token = data_fixture.create_user_and_token() + page = data_fixture.create_builder_page(user=user) + element = data_fixture.create_builder_heading_element(page=page) + + url = reverse("api:builder:element:item", kwargs={"element_id": element.id}) + response = api_client.patch( + url, + {"value": "get('foobar.123')", "level": 3}, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json() == { + "error": "ERROR_REQUEST_BODY_VALIDATION", + "detail": { + "value": [ + { + "error": "The formula provider 'foobar' used " + "in 'foobar.123' does not exist in this module.", + "code": "invalid_formula_argument", + } + ] + }, + } + + @pytest.mark.django_db def test_update_element_styles(api_client, data_fixture): user, token = data_fixture.create_user_and_token() diff --git a/backend/tests/baserow/contrib/builder/api/elements/test_table_element.py b/backend/tests/baserow/contrib/builder/api/elements/test_table_element.py index 68835a6c66..774164b17e 100644 --- a/backend/tests/baserow/contrib/builder/api/elements/test_table_element.py +++ b/backend/tests/baserow/contrib/builder/api/elements/test_table_element.py @@ -64,14 +64,14 @@ def test_can_update_a_table_element_fields(api_client, data_fixture): { "name": "Name", "type": "text", - "value": "get('test1')", + "value": "get('data_source.123')", "uid": uuids[0], }, { "name": "Color", "type": "link", - "navigate_to_url": "get('test2')", - "link_name": "get('test3')", + "navigate_to_url": "get('data_source.124')", + "link_name": "get('data_source.125')", "target": "self", "variant": LinkElement.VARIANTS.BUTTON, "uid": uuids[1], @@ -79,7 +79,7 @@ def test_can_update_a_table_element_fields(api_client, data_fixture): { "name": "Question", "type": "text", - "value": "get('test3')", + "value": "get('data_source.126')", "uid": uuids[2], }, ], @@ -97,7 +97,7 @@ def test_can_update_a_table_element_fields(api_client, data_fixture): "name": "Name", "type": "text", "value": BaserowFormulaObject( - formula="get('test1')", + formula="get('data_source.123')", version=BASEROW_FORMULA_VERSION_INITIAL, mode=BASEROW_FORMULA_MODE_SIMPLE, ), @@ -110,12 +110,12 @@ def test_can_update_a_table_element_fields(api_client, data_fixture): "navigate_to_page_id": None, "navigation_type": NavigationElementMixin.NAVIGATION_TYPES.PAGE, "navigate_to_url": BaserowFormulaObject( - formula="get('test2')", + formula="get('data_source.124')", version=BASEROW_FORMULA_VERSION_INITIAL, mode=BASEROW_FORMULA_MODE_SIMPLE, ), "link_name": BaserowFormulaObject( - formula="get('test3')", + formula="get('data_source.125')", version=BASEROW_FORMULA_VERSION_INITIAL, mode=BASEROW_FORMULA_MODE_SIMPLE, ), @@ -130,7 +130,7 @@ def test_can_update_a_table_element_fields(api_client, data_fixture): "name": "Question", "type": "text", "value": BaserowFormulaObject( - formula="get('test3')", + formula="get('data_source.126')", version=BASEROW_FORMULA_VERSION_INITIAL, mode=BASEROW_FORMULA_MODE_SIMPLE, ), 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 84b11aa283..828169ac82 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 @@ -166,6 +166,40 @@ def test_patch_workflow_actions(api_client, data_fixture): assert response_json["description"]["formula"] == "'hello'" +@pytest.mark.django_db +def test_updating_workflow_actions_with_invalid_formula_arguments_throws_error( + api_client, data_fixture +): + user, token = data_fixture.create_user_and_token() + page = data_fixture.create_builder_page(user=user) + workflow_action = data_fixture.create_notification_workflow_action(page=page) + + url = reverse( + "api:builder:workflow_action:item", + kwargs={"workflow_action_id": workflow_action.id}, + ) + response = api_client.patch( + url, + {"description": "get('foobar.123')"}, + format="json", + HTTP_AUTHORIZATION=f"JWT {token}", + ) + + assert response.status_code == HTTP_400_BAD_REQUEST + assert response.json() == { + "error": "ERROR_REQUEST_BODY_VALIDATION", + "detail": { + "description": [ + { + "error": "The formula provider 'foobar' used " + "in 'foobar.123' does not exist in this module.", + "code": "invalid_formula_argument", + } + ] + }, + } + + class PublicTestWorkflowActionType(NotificationWorkflowActionType): type = "test_workflow_action" diff --git a/backend/tests/baserow/contrib/database/formula/test_baserow_formula_results.py b/backend/tests/baserow/contrib/database/formula/test_baserow_formula_results.py index fcce381bfb..9684b0042b 100644 --- a/backend/tests/baserow/contrib/database/formula/test_baserow_formula_results.py +++ b/backend/tests/baserow/contrib/database/formula/test_baserow_formula_results.py @@ -930,13 +930,13 @@ def test_can_use_has_option_on_lookup_of_single_select_fields(data_fixture): ( "CONCAT()", "ERROR_WITH_FORMULA", - "Error with formula: 0 arguments were given to the function concat, it must " + "Error with formula: 0 arguments were given to the 'concat' function, it must " "instead be given more than 1 arguments.", ), ( "CONCAT('a')", "ERROR_WITH_FORMULA", - "Error with formula: 1 argument was given to the function concat, it must " + "Error with formula: 1 argument was given to the 'concat' function, it must " "instead be given more than 1 arguments.", ), ("UPPER()", "ERROR_WITH_FORMULA", None), @@ -944,7 +944,7 @@ def test_can_use_has_option_on_lookup_of_single_select_fields(data_fixture): ( "UPPER('a','a')", "ERROR_WITH_FORMULA", - "Error with formula: 2 arguments were given to the function upper, it must " + "Error with formula: 2 arguments were given to the 'upper' function, it must " "instead be given exactly 1 argument.", ), ("LOWER('a','a')", "ERROR_WITH_FORMULA", None), diff --git a/backend/tests/baserow/core/formula/test_formula_visitors.py b/backend/tests/baserow/core/formula/test_formula_visitors.py new file mode 100644 index 0000000000..8ce6ad3f02 --- /dev/null +++ b/backend/tests/baserow/core/formula/test_formula_visitors.py @@ -0,0 +1,100 @@ +from typing import List +from unittest.mock import MagicMock + +import pytest + +from baserow.core.exceptions import InstanceTypeDoesNotExist +from baserow.core.formula import BaserowFormulaSyntaxError +from baserow.core.formula.parser.exceptions import InvalidNumberOfArguments +from baserow.core.formula.parser.formula_validation_visitor import ( + BaserowFormulaValidationVisitor, +) +from baserow.core.formula.parser.parser import get_parse_tree_for_formula +from baserow.core.formula.registries import ( + DataProviderType, + DataProviderTypeRegistry, + formula_runtime_function_registry, +) +from baserow.core.services.dispatch_context import DispatchContext + + +class MockDataProvider(DataProviderType): + type = "test_provider" + + def is_valid(self, path: List[str]): + return True + + def get_data_chunk(self, dispatch_context: DispatchContext, path: List[str]): + pass + + +@pytest.fixture +def mock_registry(): + registry = DataProviderTypeRegistry() + registry.register(MockDataProvider()) + return registry + + +def parse_and_visit_formula(formula: str, registry=None): + tree = get_parse_tree_for_formula(formula) + visitor = BaserowFormulaValidationVisitor( + formula_runtime_function_registry, data_provider_type_registry=registry + ) + return visitor.visit(tree) + + +def test_get_with_no_arguments_raises_invalid_number_of_args(mock_registry): + with pytest.raises( + InvalidNumberOfArguments, + match=r"0 arguments were given .* it must instead be given 1", + ): + parse_and_visit_formula("get()", mock_registry) + + +def test_get_with_two_arguments_raises_invalid_number_of_args(mock_registry): + with pytest.raises( + InvalidNumberOfArguments, + match=r"2 arguments were given .* it must instead be given 1", + ): + parse_and_visit_formula( + "get('test_provider.field1', 'extra_arg')", mock_registry + ) + + +def test_get_with_empty_provider_name_raises_syntax_error( + mock_registry, +): + with pytest.raises( + BaserowFormulaSyntaxError, + match=r"The 'get' function arguments must start with a formula provider name.", + ): + parse_and_visit_formula("get('.field1')", mock_registry) + + +def test_get_with_nonexistent_provider_raises_instance_type_does_not_exist( + mock_registry, +): + with pytest.raises( + InstanceTypeDoesNotExist, + match=r"The formula provider 'foobar' used in 'foobar.field1' does not exist", + ): + parse_and_visit_formula("get('foobar.field1')", mock_registry) + + +def test_get_with_valid_provider_calls_is_valid(mock_registry): + provider = mock_registry.get("test_provider") + provider.is_valid = MagicMock(return_value=True) + parse_and_visit_formula("get('test_provider.foo.bar.baz')", mock_registry) + provider.is_valid.assert_called_once_with(["foo", "bar", "baz"]) + + +def test_get_as_child_formula_is_valid(mock_registry): + provider = mock_registry.get("test_provider") + provider.is_valid = MagicMock(return_value=True) + parse_and_visit_formula( + "concat(get('test_provider.field1'), ' ', get('test_provider.field2'))", + mock_registry, + ) + assert provider.is_valid.call_count == 2 + provider.is_valid.assert_any_call(["field1"]) + provider.is_valid.assert_any_call(["field2"]) diff --git a/backend/tests/baserow/core/formula/test_runtime_formula_types.py b/backend/tests/baserow/core/formula/test_runtime_formula_types.py index 81e61fcfd9..fcc6deb26e 100644 --- a/backend/tests/baserow/core/formula/test_runtime_formula_types.py +++ b/backend/tests/baserow/core/formula/test_runtime_formula_types.py @@ -1735,16 +1735,15 @@ def test_runtime_and_execute(args, expected): ([True, False], None), ([True, "false"], None), ([True, "False"], None), - # Invalid types for 1st arg - (["foo", True], "foo"), - ([{}, True], {}), - (["", True], ""), - ([100, True], 100), - # Invalid types for 2nd arg - ([True, "foo"], "foo"), - ([True, {}], {}), - ([True, ""], ""), - ([True, 100], 100), + # With strict=False, all values are valid (converted via bool()) + (["foo", True], None), + ([{}, True], None), + (["", True], None), + ([True, "foo"], None), + ([True, {}], None), + ([True, ""], None), + ([100, True], None), + ([True, 100], None), ], ) def test_runtime_and_validate_type_of_args(args, expected): @@ -1797,15 +1796,15 @@ def test_runtime_or_execute(args, expected): ([True, False], None), ([True, "false"], None), ([True, "False"], None), - # Invalid types for 1st or 2nd arg - (["foo", True], "foo"), - ([{}, True], {}), - (["", True], ""), - ([100, True], 100), - ([True, "foo"], "foo"), - ([True, {}], {}), - ([True, ""], ""), - ([True, 100], 100), + # With strict=False, all values are valid (converted via bool()) + (["foo", True], None), + ([{}, True], None), + (["", True], None), + ([True, "foo"], None), + ([True, {}], None), + ([True, ""], None), + ([100, True], None), + ([True, 100], None), ], ) def test_runtime_or_validate_type_of_args(args, expected): diff --git a/backend/tests/baserow/formula/test_formula_execution_visitor.py b/backend/tests/baserow/formula/test_formula_execution_visitor.py new file mode 100644 index 0000000000..df385b8d20 --- /dev/null +++ b/backend/tests/baserow/formula/test_formula_execution_visitor.py @@ -0,0 +1,42 @@ +import pytest + +from baserow.core.formula.parser.formula_execution_visitor import ( + BaserowFormulaExecutionVisitor, +) +from baserow.core.formula.parser.parser import get_parse_tree_for_formula +from baserow.core.formula.registries import formula_runtime_function_registry +from baserow.test_utils.helpers import load_test_cases + +TEST_DATA = load_test_cases("formula_visitor_cases") + +VALID_FORMULA_EXECUTION_TESTS = TEST_DATA["VALID_FORMULA_EXECUTION_TESTS"] +INVALID_FORMULA_EXECUTION_TESTS = TEST_DATA["INVALID_FORMULA_EXECUTION_TESTS"] + + +@pytest.mark.django_db +@pytest.mark.parametrize("test_data", VALID_FORMULA_EXECUTION_TESTS) +def test_valid_formulas(test_data): + formula = test_data["formula"] + result = test_data["result"] + context = test_data["context"] + + tree = get_parse_tree_for_formula(formula) + assert ( + BaserowFormulaExecutionVisitor( + formula_runtime_function_registry, context + ).visit(tree) + == result + ) + + +@pytest.mark.django_db +@pytest.mark.parametrize("test_data", INVALID_FORMULA_EXECUTION_TESTS) +def test_invalid_formulas(test_data): + formula = test_data["formula"] + context = test_data["context"] + + with pytest.raises(Exception): + tree = get_parse_tree_for_formula(formula) + BaserowFormulaExecutionVisitor( + formula_runtime_function_registry, context + ).visit(tree) diff --git a/backend/tests/baserow/formula/test_formula_validation_visitor.py b/backend/tests/baserow/formula/test_formula_validation_visitor.py new file mode 100644 index 0000000000..205bfed90a --- /dev/null +++ b/backend/tests/baserow/formula/test_formula_validation_visitor.py @@ -0,0 +1,61 @@ +import pytest + +from baserow.core.formula.parser.formula_validation_visitor import ( + BaserowFormulaValidationVisitor, +) +from baserow.core.formula.parser.parser import get_parse_tree_for_formula +from baserow.core.formula.registries import ( + DataProviderType, + DataProviderTypeRegistry, + formula_runtime_function_registry, +) +from baserow.test_utils.helpers import load_test_cases + +TEST_DATA = load_test_cases("formula_visitor_cases") + +VALID_FORMULA_VALIDATION_TESTS = TEST_DATA["VALID_FORMULA_VALIDATION_TESTS"] +INVALID_FORMULA_VALIDATION_TESTS = TEST_DATA["INVALID_FORMULA_VALIDATION_TESTS"] + + +class TestDataProviderType(DataProviderType): + type = "test_data_provider" + + def get_data_chunk(self, dispatch_context, path): + return None + + +# Create registry once at module level to avoid concurrent registration issues +_test_data_provider_registry = DataProviderTypeRegistry() +_test_data_provider_registry.register(TestDataProviderType()) + + +@pytest.mark.django_db +@pytest.mark.parametrize("test_data", VALID_FORMULA_VALIDATION_TESTS) +def test_valid_formulas(test_data): + formula = test_data["formula"] + + tree = get_parse_tree_for_formula(formula) + try: + BaserowFormulaValidationVisitor( + formula_runtime_function_registry, + data_provider_type_registry=_test_data_provider_registry, + ).visit(tree) + except Exception as e: + pytest.fail(f"Validation raised an unexpected exception: {e}") + + +@pytest.mark.django_db +@pytest.mark.parametrize("test_data", INVALID_FORMULA_VALIDATION_TESTS) +def test_invalid_formulas(test_data): + formula = test_data["formula"] + expected_error = test_data["backend_error"] + + tree = get_parse_tree_for_formula(formula) + + with pytest.raises(Exception) as exc_info: + BaserowFormulaValidationVisitor( + formula_runtime_function_registry, + data_provider_type_registry=_test_data_provider_registry, + ).visit(tree) + if expected_error: + assert expected_error in str(exc_info) diff --git a/backend/tests/baserow/formula/test_python_executor.py b/backend/tests/baserow/formula/test_python_executor.py deleted file mode 100644 index 2096ab06b1..0000000000 --- a/backend/tests/baserow/formula/test_python_executor.py +++ /dev/null @@ -1,50 +0,0 @@ -import pytest - -from baserow.core.formula.parser.exceptions import ( - BaserowFormulaSyntaxError, - InvalidNumberOfArguments, -) -from baserow.core.formula.parser.parser import get_parse_tree_for_formula -from baserow.core.formula.parser.python_executor import BaserowPythonExecutor -from baserow.core.formula.registries import formula_runtime_function_registry -from baserow.test_utils.helpers import load_test_cases - -TEST_DATA = load_test_cases("formula_runtime_cases") - -VALID_FORMULA_TESTS = TEST_DATA["VALID_FORMULA_TESTS"] -INVALID_FORMULA_TESTS = TEST_DATA["INVALID_FORMULA_TESTS"] - - -@pytest.mark.parametrize("test_data", VALID_FORMULA_TESTS) -def test_valid_formulas(test_data): - formula = test_data["formula"] - result = test_data["result"] - context = test_data["context"] - - tree = get_parse_tree_for_formula(formula) - assert ( - BaserowPythonExecutor(formula_runtime_function_registry, context).visit(tree) - == result - ) - - -@pytest.mark.parametrize("test_data", INVALID_FORMULA_TESTS) -def test_invalid_formulas(test_data): - formula = test_data["formula"] - context = test_data["context"] - - with pytest.raises(Exception): - tree = get_parse_tree_for_formula(formula) - BaserowPythonExecutor(formula_runtime_function_registry, context).visit(tree) - - -def test_formula_function_does_not_exist(): - with pytest.raises(BaserowFormulaSyntaxError): - tree = get_parse_tree_for_formula("notExistingFunction(1,2,3)") - BaserowPythonExecutor(formula_runtime_function_registry, {}).visit(tree) - - -def test_invalid_number_of_arguments(): - with pytest.raises(InvalidNumberOfArguments): - tree = get_parse_tree_for_formula("get(1,2)") - BaserowPythonExecutor(formula_runtime_function_registry, {}).visit(tree) diff --git a/changelog/entries/unreleased/feature/4532_improved_baserow_formula_function_argument_validation.json b/changelog/entries/unreleased/feature/4532_improved_baserow_formula_function_argument_validation.json new file mode 100644 index 0000000000..38ebf8e32d --- /dev/null +++ b/changelog/entries/unreleased/feature/4532_improved_baserow_formula_function_argument_validation.json @@ -0,0 +1,9 @@ +{ + "type": "feature", + "message": "Improved Baserow formula function argument validation.", + "issue_origin": "github", + "issue_number": 4532, + "domain": "core", + "bullet_points": [], + "created_at": "2026-01-13" +} \ No newline at end of file diff --git a/premium/web-frontend/modules/baserow_premium/components/field/FieldAISubForm.vue b/premium/web-frontend/modules/baserow_premium/components/field/FieldAISubForm.vue index bb96b1e450..4b0a561520 100644 --- a/premium/web-frontend/modules/baserow_premium/components/field/FieldAISubForm.vue +++ b/premium/web-frontend/modules/baserow_premium/components/field/FieldAISubForm.vue @@ -89,6 +89,7 @@ :mode="localMode" :nodes-hierarchy="nodesHierarchy" :placeholder="$t('fieldAISubForm.promptPlaceholder')" + :validation-context="{ dataProviderRegistry: dataProviders }" @input="updatedFormulaStr" @update:mode="updateMode" /> diff --git a/tests/cases/formula_runtime_cases.json b/tests/cases/formula_runtime_cases.json deleted file mode 100644 index 93c5dc7b5b..0000000000 --- a/tests/cases/formula_runtime_cases.json +++ /dev/null @@ -1,68 +0,0 @@ -{ - "VALID_FORMULA_TESTS": [ - { - "formula": "'test'", - "result": "test", - "context": {} - }, - { - "formula": "1", - "result": 1, - "context": {} - }, - { - "formula": "true", - "result": true, - "context": {} - }, - { - "formula": "false", - "result": false, - "context": {} - }, - { - "formula": "concat('a', 'b')", - "result": "ab", - "context": {} - }, - { - "formula": "get('a')", - "result": "test", - "context": { - "a": "test" - } - }, - { - "formula": "1 + 2", - "result": 3, - "context": {} - }, - { - "formula": "get('a') + get('b')", - "result": 4, - "context": {"a": 1, "b": 3} - } - ], - "INVALID_FORMULA_TESTS": [ - { - "formula": "invalid()", - "context": {} - }, - { - "formula": "1 + 'hello'", - "context": {} - }, - { - "formula": "get(1,2)", - "context": {} - }, - { - "formula": "concat()", - "context": {} - }, - { - "formula": "concat('a')", - "context": {} - } - ] -} diff --git a/tests/cases/formula_visitor_cases.json b/tests/cases/formula_visitor_cases.json new file mode 100644 index 0000000000..662425e3a7 --- /dev/null +++ b/tests/cases/formula_visitor_cases.json @@ -0,0 +1,194 @@ +{ + "VALID_FORMULA_EXECUTION_TESTS": [ + { + "formula": "'test'", + "result": "test", + "context": {} + }, + { + "formula": "1", + "result": 1, + "context": {} + }, + { + "formula": "true", + "result": true, + "context": {} + }, + { + "formula": "false", + "result": false, + "context": {} + }, + { + "formula": "concat('a', 'b')", + "result": "ab", + "context": {} + }, + { + "formula": "get('a')", + "result": "test", + "context": { + "a": "test" + } + }, + { + "formula": "1 + 2", + "result": 3, + "context": {} + }, + { + "formula": "get('a') + get('b')", + "result": 4, + "context": {"a": 1, "b": 3} + } + ], + "INVALID_FORMULA_EXECUTION_TESTS": [ + { + "formula": "invalid()", + "context": {} + }, + { + "formula": "1 + 'hello'", + "context": {} + }, + { + "formula": "get(1,2)", + "context": {} + } + ], + "VALID_FORMULA_VALIDATION_TESTS": [ + { + "formula": "'test'", + "context": {} + }, + { + "formula": "get('test_data_provider.1')", + "context": {} + }, + { + "formula": "is_even(2)", + "context": {}, + "description": "Literal numeric value validates correctly" + }, + { + "formula": "is_odd(3)", + "context": {}, + "description": "Literal numeric value validates correctly" + }, + { + "formula": "is_even(get('test_data_provider.value'))", + "context": {}, + "description": "Nested get() call is deferred, skipping type validation" + }, + { + "formula": "is_odd(get('test_data_provider.value'))", + "context": {}, + "description": "Nested get() call is deferred, skipping type validation" + }, + { + "formula": "add(get('test_data_provider.a'), get('test_data_provider.b'))", + "context": {}, + "description": "Multiple nested get() calls are deferred" + }, + { + "formula": "is_even(add(1, 2))", + "context": {}, + "description": "Nested function calls with literals are deferred" + }, + { + "formula": "is_odd(1 + 2)", + "context": {}, + "description": "Literal numeric values with an operator validate correctly" + }, + { + "formula": "if(1 + 3, 'true', 'false')", + "context": {}, + "description": "The boolean expression in the first argument is not strict." + } + ], + "INVALID_FORMULA_VALIDATION_TESTS": [ + { + "formula": "unknownfunction()", + "context": {}, + "frontendError": "Unsupported function 'unknownfunction'.", + "backend_error": "Unsupported function 'unknownfunction'." + }, + { + "formula": "field('field_123')", + "context": {}, + "frontendError": "Unsupported function 'field'.", + "backend_error": "'field' is not a a supported function" + }, + { + "formula": "get()", + "context": {}, + "frontendError": "formulaParserErrors.invalidArgCountExact", + "backend_error": "0 arguments were given to the 'get' function, it must instead be given 1" + }, + { + "formula": "get('a', 'b')", + "context": {}, + "frontendError": "formulaParserErrors.invalidArgCountExact", + "backend_error": "2 arguments were given to the 'get' function, it must instead be given 1" + }, + { + "formula": "get('jeff')", + "context": {}, + "frontendError": "runtimeGetErrors.invalidPath", + "backend_error": "The formula provider 'jeff' used in 'jeff' does not exist in this module." + }, + { + "formula": "get('.1')", + "context": {}, + "frontendError": "runtimeGetErrors.missingProvider", + "backend_error": "The 'get' function arguments must start with a formula provider name." + }, + { + "formula": "get('foo.1')", + "context": {}, + "frontendError": "runtimeGetErrors.unknownProvider", + "backend_error": "The formula provider 'foo' used in 'foo.1' does not exist in this module." + }, + { + "formula": "concat()", + "context": {}, + "frontendError": null, + "backend_error": "0 arguments were given to the 'concat' function, it must instead be given at least 2" + }, + { + "formula": "concat('a')", + "context": {}, + "frontendError": null, + "backend_error": null + }, + { + "formula": "is_even('not a number')", + "context": {}, + "frontendError": null, + "backend_error": "is not a valid number or convertible to a number", + "description": "Literal string fails numeric type validation" + }, + { + "formula": "add(1, 'two')", + "context": {}, + "frontendError": null, + "backend_error": "is not a valid number or convertible to a number", + "description": "Mixed literal types fail validation" + }, + { + "formula": "is_even(1 + 'two')", + "context": {}, + "frontendError": null, + "backend_error": "is not a valid number or convertible to a number", + "description": "Mixed literal types fail validation" + }, + { + "formula": "avg('foobar')", + "context": {}, + "frontendError": null, + "backend_error": "is not a valid number or convertible to a number", + "description": "Cannot be ensured to an array of numbers." + } + ] +} diff --git a/web-frontend/locales/en.json b/web-frontend/locales/en.json index d72334c8af..28b52e224a 100644 --- a/web-frontend/locales/en.json +++ b/web-frontend/locales/en.json @@ -702,6 +702,12 @@ "categoryDate": "Date", "categoryCondition": "Condition" }, + "runtimeGetErrors": { + "invalidPath": "The 'get' argument '{path}' must be a dot-delimited path (e.g., 'provider.property').", + "missingProvider": "The 'get' argument '{path}' is missing its formula provider prefix.", + "unknownProvider": "The '{providerName}' formula provider does not exist.", + "invalidProviderPath": "The '{providerName}' formula provider found '{path}' to be invalid." + }, "nodeExplorer": { "noResults": "No results found", "resetSearch": "Reset search" diff --git a/web-frontend/modules/automation/components/AutomationBuilderFormulaInput.vue b/web-frontend/modules/automation/components/AutomationBuilderFormulaInput.vue index 8bd439ef69..4bfa605a1c 100644 --- a/web-frontend/modules/automation/components/AutomationBuilderFormulaInput.vue +++ b/web-frontend/modules/automation/components/AutomationBuilderFormulaInput.vue @@ -6,6 +6,7 @@ :nodes-hierarchy="nodesHierarchy" context-position="left" :mode="localMode" + :validation-context="{ dataProviderRegistry: dataProviders }" @update:mode="updateMode" @input="updatedFormulaStr" /> diff --git a/web-frontend/modules/builder/components/ApplicationBuilderFormulaInput.vue b/web-frontend/modules/builder/components/ApplicationBuilderFormulaInput.vue index d12095818e..c533880806 100644 --- a/web-frontend/modules/builder/components/ApplicationBuilderFormulaInput.vue +++ b/web-frontend/modules/builder/components/ApplicationBuilderFormulaInput.vue @@ -7,6 +7,7 @@ :loading="dataExplorerLoading" :nodes-hierarchy="nodesHierarchy" :context-position="isInSidePanel ? 'left' : 'bottom'" + :validation-context="{ dataProviderRegistry: dataProviders }" @input="updatedFormulaStr" @update:mode="updateMode" /> diff --git a/web-frontend/modules/core/assets/scss/components/all.scss b/web-frontend/modules/core/assets/scss/components/all.scss index c0abfeb16a..ec49c55714 100644 --- a/web-frontend/modules/core/assets/scss/components/all.scss +++ b/web-frontend/modules/core/assets/scss/components/all.scss @@ -205,5 +205,6 @@ @import 'code_editor'; @import 'field_constraints'; @import 'workspace_search'; -@import 'formula_input_context'; +@import 'formula_input_explorer_context'; +@import 'formula_input_error_context'; @import 'tabs_body'; diff --git a/web-frontend/modules/core/assets/scss/components/formula_input_error_context.scss b/web-frontend/modules/core/assets/scss/components/formula_input_error_context.scss new file mode 100644 index 0000000000..c2e1b1d858 --- /dev/null +++ b/web-frontend/modules/core/assets/scss/components/formula_input_error_context.scss @@ -0,0 +1,13 @@ +.formula-input-error-context { + border: none; + + .alert { + margin: 0; + width: 100%; + } + + .alert__message { + max-height: 75px; + overflow-y: auto; + } +} diff --git a/web-frontend/modules/core/assets/scss/components/formula_input_context.scss b/web-frontend/modules/core/assets/scss/components/formula_input_explorer_context.scss similarity index 89% rename from web-frontend/modules/core/assets/scss/components/formula_input_context.scss rename to web-frontend/modules/core/assets/scss/components/formula_input_explorer_context.scss index d99bc861fa..d53bd07dce 100644 --- a/web-frontend/modules/core/assets/scss/components/formula_input_context.scss +++ b/web-frontend/modules/core/assets/scss/components/formula_input_explorer_context.scss @@ -1,4 +1,4 @@ -.formula-input-context { +.formula-input-explorer-context { width: 400px; display: flex; flex-direction: column; @@ -54,7 +54,7 @@ } } -.formula-input-context__footer { +.formula-input-explorer-context__footer { flex-shrink: 0; padding: 12px 16px; border-top: 1px solid $palette-neutral-200; @@ -67,6 +67,6 @@ @include rounded($rounded); } -.formula-input-context__advanced-mode-modal { +.formula-input-explorer-context__advanced-mode-modal { z-index: $z-index-modal + 1; } diff --git a/web-frontend/modules/core/assets/scss/components/formula_input_field.scss b/web-frontend/modules/core/assets/scss/components/formula_input_field.scss index 0094ae4cf9..0810e75d95 100644 --- a/web-frontend/modules/core/assets/scss/components/formula_input_field.scss +++ b/web-frontend/modules/core/assets/scss/components/formula_input_field.scss @@ -141,3 +141,7 @@ padding-right: 2px; } } + +.formula-input-field__view-full-error { + margin-left: 5px; +} diff --git a/web-frontend/modules/core/components/Context.vue b/web-frontend/modules/core/components/Context.vue index 9d54a695e7..dadfc2949c 100644 --- a/web-frontend/modules/core/components/Context.vue +++ b/web-frontend/modules/core/components/Context.vue @@ -49,6 +49,16 @@ export default { default: () => false, required: false, }, + /** + * When `checkForEdges` is called, this prop allows us to force the + * position to remain fixed, even if the context could result in it + * getting clipped. + */ + forcePosition: { + type: Boolean, + default: false, + required: false, + }, }, emits: ['hidden', 'shown'], data() { @@ -502,6 +512,11 @@ export default { horizontalOffset > 0 + // If forcePosition is true, don't adjust the position based on edges. + if (this.forcePosition) { + return { vertical, horizontal } + } + // If bottom, top, left or right doesn't fit, but their opposite does we switch to // that. if (vertical === 'bottom' && !canBottom && canTop) { diff --git a/web-frontend/modules/core/components/formula/FormulaInputErrorContext.vue b/web-frontend/modules/core/components/formula/FormulaInputErrorContext.vue new file mode 100644 index 0000000000..9b2a15daf0 --- /dev/null +++ b/web-frontend/modules/core/components/formula/FormulaInputErrorContext.vue @@ -0,0 +1,56 @@ + + + diff --git a/web-frontend/modules/core/components/formula/FormulaInputContext.vue b/web-frontend/modules/core/components/formula/FormulaInputExplorerContext.vue similarity index 86% rename from web-frontend/modules/core/components/formula/FormulaInputContext.vue rename to web-frontend/modules/core/components/formula/FormulaInputExplorerContext.vue index 677fcf0bbd..f7017323e0 100644 --- a/web-frontend/modules/core/components/formula/FormulaInputContext.vue +++ b/web-frontend/modules/core/components/formula/FormulaInputExplorerContext.vue @@ -1,9 +1,11 @@