Merge branch '196-check-if-the-select_for_update-is-used-when-updating' into 'develop'

Resolve "Check if the `select_for_update` is used when updating"

Closes #196

See merge request bramw/baserow!123

backend/src/baserow/contrib/database/api/tables/views.py

@@ -195,6 +195,7 @@ class TableView(APIView):
         table = TableHandler().update_table(
             request.user,
             TableHandler().get_table(request.user, table_id),
+            base_queryset=Table.objects.select_for_update(),
             name=data['name']
         )
         serializer = TableSerializer(table)

backend/src/baserow/contrib/database/api/tokens/views.py

@@ -154,7 +154,11 @@ class TokenView(APIView):
     def patch(self, request, data, token_id):
         """Updates the values of a token."""
 
-        token = TokenHandler().get_token(request.user, token_id)
+        token = TokenHandler().get_token(
+            request.user,
+            token_id,
+            base_queryset=Token.objects.select_for_update()
+        )
         permissions = data.pop('permissions', None)
         rotate_key = data.pop('rotate_key', False)
 

backend/src/baserow/contrib/database/api/views/views.py

@@ -475,7 +475,11 @@ class ViewFilterView(APIView):
         """Updates the view filter if the user belongs to the group."""
 
         handler = ViewHandler()
-        view_filter = handler.get_filter(request.user, view_filter_id)
+        view_filter = handler.get_filter(
+            request.user,
+            view_filter_id,
+            base_queryset=ViewFilter.objects.select_for_update()
+        )
 
         if 'field' in data:
             # We can safely assume the field exists because the
@@ -695,7 +699,11 @@ class ViewSortView(APIView):
         """Updates the view sort if the user belongs to the group."""
 
         handler = ViewHandler()
-        view_sort = handler.get_sort(request.user, view_sort_id)
+        view_sort = handler.get_sort(
+            request.user,
+            view_sort_id,
+            base_queryset=ViewSort.objects.select_for_update()
+        )
 
         if 'field' in data:
             # We can safely assume the field exists because the

backend/src/baserow/contrib/database/table/handler.py

@@ -18,7 +18,7 @@ from .exceptions import (
 
 
 class TableHandler:
-    def get_table(self, user, table_id):
+    def get_table(self, user, table_id, base_queryset=None):
         """
         Selects a table with a given id from the database.
 
@@ -26,14 +26,20 @@ class TableHandler:
         :type user: User
         :param table_id: The identifier of the table that must be returned.
         :type table_id: int
+        :param base_queryset: The base queryset from where to select the table
+            object from. This can for example be used to do a `select_related`.
+        :type base_queryset: Queryset
         :raises TableDoesNotExist: When the table with the provided id does not exist.
         :raises UserNotInGroupError: When the user does not belong to the related group.
         :return: The requested table of the provided id.
         :rtype: Table
         """
 
+        if not base_queryset:
+            base_queryset = Table.objects
+
         try:
-            table = Table.objects.select_related('database__group').get(id=table_id)
+            table = base_queryset.select_related('database__group').get(id=table_id)
         except Table.DoesNotExist:
             raise TableDoesNotExist(f'The table with id {table_id} doe not exist.')
 

backend/src/baserow/contrib/database/views/handler.py

@@ -259,7 +259,7 @@ class ViewHandler:
 
         return queryset
 
-    def get_filter(self, user, view_filter_id):
+    def get_filter(self, user, view_filter_id, base_queryset=None):
         """
         Returns an existing view filter by the given id.
 
@@ -267,14 +267,20 @@ class ViewHandler:
         :type user: User
         :param view_filter_id: The id of the view filter.
         :type view_filter_id: int
+        :param base_queryset: The base queryset from where to select the view filter
+            object. This can for example be used to do a `select_related`.
+        :type base_queryset: Queryset
         :raises ViewFilterDoesNotExist: The the requested view does not exists.
         :raises UserNotInGroupError: When the user does not belong to the related group.
         :return: The requested view filter instance.
         :type: ViewFilter
         """
 
+        if not base_queryset:
+            base_queryset = ViewFilter.objects
+
         try:
-            view_filter = ViewFilter.objects.select_related(
+            view_filter = base_queryset.select_related(
                 'view__table__database__group'
             ).get(
                 pk=view_filter_id
@@ -476,7 +482,7 @@ class ViewHandler:
 
         return queryset
 
-    def get_sort(self, user, view_sort_id):
+    def get_sort(self, user, view_sort_id, base_queryset=None):
         """
         Returns an existing view sort with the given id.
 
@@ -484,14 +490,20 @@ class ViewHandler:
         :type user: User
         :param view_sort_id: The id of the view sort.
         :type view_sort_id: int
+        :param base_queryset: The base queryset from where to select the view sort
+            object from. This can for example be used to do a `select_related`.
+        :type base_queryset: Queryset
         :raises ViewSortDoesNotExist: The the requested view does not exists.
         :raises UserNotInGroupError: When the user does not belong to the related group.
         :return: The requested view sort instance.
         :type: ViewSort
         """
 
+        if not base_queryset:
+            base_queryset = ViewSort.objects
+
         try:
-            view_sort = ViewSort.objects.select_related(
+            view_sort = base_queryset.select_related(
                 'view__table__database__group'
             ).get(
                 pk=view_sort_id

backend/tests/baserow/contrib/database/table/test_table_handler.py

@@ -28,6 +28,14 @@ def test_get_database_table(data_fixture):
     with pytest.raises(TableDoesNotExist):
         handler.get_table(user=user, table_id=99999)
 
+    # If the error is raised we know for sure that the base query has resolved.
+    with pytest.raises(AttributeError):
+        handler.get_table(
+            user=user,
+            table_id=table.id,
+            base_queryset=Table.objects.prefetch_related('UNKNOWN')
+        )
+
     table_copy = handler.get_table(user=user, table_id=table.id)
     assert table_copy.id == table.id
 

backend/tests/baserow/contrib/database/view/test_view_handler.py

@@ -402,6 +402,13 @@ def test_get_filter(data_fixture):
     with pytest.raises(UserNotInGroupError):
         handler.get_filter(user=user_2, view_filter_id=equal_filter.id)
 
+    with pytest.raises(AttributeError):
+        handler.get_filter(
+            user=user,
+            view_filter_id=equal_filter.id,
+            base_queryset=ViewFilter.objects.prefetch_related('UNKNOWN')
+        )
+
     filter = handler.get_filter(user=user, view_filter_id=equal_filter.id)
 
     assert filter.id == equal_filter.id
@@ -671,6 +678,13 @@ def test_get_sort(data_fixture):
     with pytest.raises(UserNotInGroupError):
         handler.get_sort(user=user_2, view_sort_id=equal_sort.id)
 
+    with pytest.raises(AttributeError):
+        handler.get_sort(
+            user=user,
+            view_sort_id=equal_sort.id,
+            base_queryset=ViewSort.objects.prefetch_related('UNKNOWN')
+        )
+
     sort = handler.get_sort(user=user, view_sort_id=equal_sort.id)
 
     assert sort.id == equal_sort.id

changelog.md

@@ -1,5 +1,6 @@
 # Changelog
 
+* Added select_for_update where it was still missing.
 * Fixed API docs scrollbar size issue.
 * Also lint the backend tests.
 * Implemented a switch to disable all filters without deleting them.