Merge branch '1925-two-linked-tables-with-publically-shared-views-can-cause-bugs-caused-by-prefetch-caching-fields' into 'develop'

Resolve "Two linked tables with publically shared views can cause bugs caused by prefetch caching fields"

Closes #1925

See merge request baserow/baserow!1607

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

@@ -2761,7 +2761,14 @@ class CachingPublicViewRowChecker:
         self._always_visible_views = []
         self._view_row_check_cache = defaultdict(dict)
         handler = ViewHandler()
-        for view in self._public_views:
+        for view in specific_iterator(
+            self._public_views,
+            per_content_type_queryset_hook=(
+                lambda model, queryset: view_type_registry.get_by_model(
+                    model
+                ).enhance_queryset(queryset)
+            ),
+        ):
             if only_include_views_which_want_realtime_events:
                 view_type = view_type_registry.get_by_model(view.specific_class)
                 if not view_type.when_shared_publicly_requires_realtime_events:

backend/src/baserow/contrib/database/ws/public/fields/signals.py

@@ -64,8 +64,11 @@ def _get_views_where_field_visible_and_hidden_fields_in_view(
     """
 
     views_where_field_was_visible = []
+    views_with_prefetched_fields = View.objects.filter(
+        public=True, table_id=field.table_id
+    ).prefetch_related("table__field_set")
     for view in specific_iterator(
-        field.table.view_set.filter(public=True).prefetch_related("table__field_set"),
+        views_with_prefetched_fields,
         per_content_type_queryset_hook=(
             lambda model, queryset: view_type_registry.get_by_model(
                 model

backend/src/baserow/contrib/database/ws/public/rows/signals.py

@@ -3,6 +3,8 @@ from typing import Any, Dict, List, Optional
 from django.db import transaction
 from django.dispatch import receiver
 
+from opentelemetry import trace
+
 from baserow.contrib.database.api.constants import PUBLIC_PLACEHOLDER_ENTITY_ID
 from baserow.contrib.database.api.rows.serializers import serialize_rows_for_response
 from baserow.contrib.database.rows import signals as row_signals
@@ -10,9 +12,13 @@ from baserow.contrib.database.table.models import GeneratedTableModel
 from baserow.contrib.database.views.handler import PublicViewRows, ViewHandler
 from baserow.contrib.database.views.registries import view_type_registry
 from baserow.contrib.database.ws.rows.signals import RealtimeRowMessages
+from baserow.core.telemetry.utils import baserow_trace
 from baserow.ws.registries import page_registry
 
+tracer = trace.get_tracer(__name__)
 
+
+@baserow_trace(tracer)
 def _send_rows_created_event_to_views(
     serialized_rows: List[Dict[Any, Any]],
     before: Optional[GeneratedTableModel],
@@ -40,6 +46,7 @@ def _send_rows_created_event_to_views(
         )
 
 
+@baserow_trace(tracer)
 def _send_rows_deleted_event_to_views(
     serialized_deleted_rows: List[Dict[Any, Any]],
     public_views: List[PublicViewRows],
@@ -64,6 +71,7 @@ def _send_rows_deleted_event_to_views(
 
 
 @receiver(row_signals.rows_created)
+@baserow_trace(tracer)
 def public_rows_created(
     sender,
     rows,
@@ -91,6 +99,7 @@ def public_rows_created(
 
 
 @receiver(row_signals.before_rows_delete)
+@baserow_trace(tracer)
 def public_before_rows_delete(sender, rows, user, table, model, **kwargs):
     row_checker = ViewHandler().get_public_views_row_checker(
         table, model, only_include_views_which_want_realtime_events=True
@@ -104,6 +113,7 @@ def public_before_rows_delete(sender, rows, user, table, model, **kwargs):
 
 
 @receiver(row_signals.rows_deleted)
+@baserow_trace(tracer)
 def public_rows_deleted(sender, rows, user, table, model, before_return, **kwargs):
     public_views = dict(before_return)[public_before_rows_delete][
         "deleted_rows_public_views"
@@ -117,6 +127,7 @@ def public_rows_deleted(sender, rows, user, table, model, before_return, **kwarg
 
 
 @receiver(row_signals.before_rows_update)
+@baserow_trace(tracer)
 def public_before_rows_update(
     sender, rows, user, table, model, updated_field_ids, **kwargs
 ):
@@ -135,6 +146,7 @@ def public_before_rows_update(
 
 
 @receiver(row_signals.rows_updated)
+@baserow_trace(tracer)
 def public_rows_updated(
     sender,
     rows,
@@ -225,6 +237,7 @@ def public_rows_updated(
         view_slug_to_updated_public_view_rows.values()
     )
 
+    @baserow_trace(tracer)
     def _send_created_updated_deleted_row_signals_to_views():
         _send_rows_deleted_event_to_views(
             serialized_old_rows, public_views_where_rows_were_deleted

backend/tests/baserow/contrib/database/field/test_link_row_field_type.py

@@ -1627,3 +1627,44 @@ def test_deleting_only_one_side_of_a_link_row_field_update_deleted_side_dependen
     assert lookup.formula_type == "invalid"
     # ensure no entry in the trash is created for the update
     assert TrashEntry.objects.count() == 0
+
+
+@pytest.mark.django_db
+@pytest.mark.field_link_row
+def test_two_linked_tables_both_publically_shared_can_have_related_linked_field_removed(
+    api_client, data_fixture
+):
+    user, token = data_fixture.create_user_and_token()
+    table_a = data_fixture.create_database_table(user=user)
+    table_b = data_fixture.create_database_table(user=user, database=table_a.database)
+    public_grid_view_a = data_fixture.create_grid_view(
+        user,
+        table=table_a,
+        public=True,
+    )
+    public_grid_view_b = data_fixture.create_grid_view(
+        user,
+        table=table_b,
+        public=True,
+    )
+
+    field_handler = FieldHandler()
+
+    field = data_fixture.create_text_field(
+        table=table_b, order=1, primary=True, name="Name"
+    )
+
+    link_a_and_b = field_handler.create_field(
+        user,
+        table_a,
+        "link_row",
+        name="A<->B",
+        link_row_table=table_b,
+        has_related_field=True,
+    )
+
+    field_handler.update_field(
+        user,
+        link_a_and_b,
+        has_related_field=False,
+    )

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

@@ -1677,12 +1677,12 @@ def test_get_public_views_which_include_row(data_fixture, django_assert_num_quer
         table, model, only_include_views_which_want_realtime_events=True
     )
     assert checker.get_public_views_where_row_is_visible(row) == [
-        public_view1.view_ptr,
-        public_view3.view_ptr,
+        public_view1.view_ptr.specific,
+        public_view3.view_ptr.specific,
     ]
     assert checker.get_public_views_where_row_is_visible(row2) == [
-        public_view2.view_ptr,
-        public_view3.view_ptr,
+        public_view2.view_ptr.specific,
+        public_view3.view_ptr.specific,
     ]
 
 
@@ -1756,15 +1756,15 @@ def test_get_public_views_which_include_rows(data_fixture):
 
     assert checker.get_public_views_where_rows_are_visible([row, row2]) == [
         PublicViewRows(
-            view=ViewHandler().get_view_as_user(user, public_view1.id),
+            view=ViewHandler().get_view_as_user(user, public_view1.id).specific,
             allowed_row_ids={1},
         ),
         PublicViewRows(
-            view=ViewHandler().get_view_as_user(user, public_view2.id),
+            view=ViewHandler().get_view_as_user(user, public_view2.id).specific,
             allowed_row_ids={2},
         ),
         PublicViewRows(
-            view=ViewHandler().get_view_as_user(user, public_view3.id),
+            view=ViewHandler().get_view_as_user(user, public_view3.id).specific,
             allowed_row_ids=PublicViewRows.ALL_ROWS_ALLOWED,
         ),
     ]
@@ -1810,7 +1810,7 @@ def test_public_view_row_checker_caches_when_only_unfiltered_fields_updated(
     )
 
     assert row_checker.get_public_views_where_row_is_visible(visible_row) == [
-        public_grid_view.view_ptr
+        public_grid_view.view_ptr.specific
     ]
     assert row_checker.get_public_views_where_row_is_visible(invisible_row) == []
 
@@ -1818,7 +1818,7 @@ def test_public_view_row_checker_caches_when_only_unfiltered_fields_updated(
     # be changing unfiltered_field it knows it can cache the results
     with django_assert_num_queries(0):
         assert row_checker.get_public_views_where_row_is_visible(visible_row) == [
-            public_grid_view.view_ptr
+            public_grid_view.view_ptr.specific
         ]
         assert row_checker.get_public_views_where_row_is_visible(invisible_row) == []
 
@@ -1859,13 +1859,14 @@ def test_public_view_row_checker_includes_public_views_with_no_filters_with_no_q
         updated_field_ids=[unfiltered_field.id],
     )
 
+    view_ptr_specific = public_grid_view.view_ptr.specific
     # It should precalculate that this view is always visible.
     with django_assert_num_queries(0):
         assert row_checker.get_public_views_where_row_is_visible(visible_row) == [
-            public_grid_view.view_ptr
+            view_ptr_specific
         ]
         assert row_checker.get_public_views_where_row_is_visible(other_row) == [
-            public_grid_view.view_ptr
+            view_ptr_specific
         ]
 
 
@@ -1909,7 +1910,7 @@ def test_public_view_row_checker_does_not_cache_when_any_filtered_fields_updated
     )
 
     assert row_checker.get_public_views_where_row_is_visible(visible_row) == [
-        public_grid_view.view_ptr
+        public_grid_view.view_ptr.specific
     ]
     assert row_checker.get_public_views_where_row_is_visible(invisible_row) == []
 
@@ -1921,7 +1922,7 @@ def test_public_view_row_checker_does_not_cache_when_any_filtered_fields_updated
     visible_row.save()
 
     assert row_checker.get_public_views_where_row_is_visible(invisible_row) == [
-        public_grid_view.view_ptr
+        public_grid_view.view_ptr.specific
     ]
     assert row_checker.get_public_views_where_row_is_visible(visible_row) == []
 
@@ -1944,7 +1945,8 @@ def test_public_view_row_checker_runs_expected_queries_on_init(
         view=public_grid_view, field=filtered_field, type="equal", value="FilterValue"
     )
     model = table.get_model()
-    with django_assert_num_queries(2):
+    num_queries = 6
+    with django_assert_num_queries(num_queries):
         # First query to get the public views, second query to get their filters.
         ViewHandler().get_public_views_row_checker(
             table,
@@ -1965,7 +1967,7 @@ def test_public_view_row_checker_runs_expected_queries_on_init(
     )
 
     # Adding another view shouldn't result in more queries
-    with django_assert_num_queries(2):
+    with django_assert_num_queries(num_queries):
         # First query to get the public views, second query to get their filters.
         ViewHandler().get_public_views_row_checker(
             table,
@@ -2013,11 +2015,12 @@ def test_public_view_row_checker_runs_expected_queries_when_checking_rows(
         updated_field_ids=[filtered_field.id, unfiltered_field.id],
     )
 
+    view_ptr_specific = public_grid_view.view_ptr.specific
     with django_assert_num_queries(1):
         # Only should run a single exists query to check if the row is in the single
         # public view
         assert row_checker.get_public_views_where_row_is_visible(visible_row) == [
-            public_grid_view.view_ptr
+            view_ptr_specific
         ]
     with django_assert_num_queries(1):
         # Only should run a single exists query to check if the row is in the single
@@ -2040,11 +2043,12 @@ def test_public_view_row_checker_runs_expected_queries_when_checking_rows(
         only_include_views_which_want_realtime_events=True,
         updated_field_ids=[filtered_field.id, unfiltered_field.id],
     )
+    specific_another_view = another_public_grid_view.view_ptr.specific
     with django_assert_num_queries(2):
         # Now should run two queries, one per public view
         assert row_checker.get_public_views_where_row_is_visible(visible_row) == [
-            public_grid_view.view_ptr,
-            another_public_grid_view.view_ptr,
+            view_ptr_specific,
+            specific_another_view,
         ]
     with django_assert_num_queries(2):
         # Now should run two queries, one per public view

backend/tests/baserow/contrib/database/ws/public/test_public_ws_rows_signals.py

@@ -1067,9 +1067,11 @@ def test_batch_update_rows_visible_in_public_view_to_some_not_be_visible_event_s
         [initially_visible_row, initially_visible_row2]
     ) == [
         PublicViewRows(
-            ViewHandler().get_view_as_user(
+            ViewHandler()
+            .get_view_as_user(
                 user, public_view_with_filters_initially_hiding_all_rows.id
-            ),
+            )
+            .specific,
             allowed_row_ids={1, 2},
         )
     ]
@@ -1188,7 +1190,7 @@ def test_given_row_visible_in_public_view_when_updated_to_be_not_visible_event_s
         table, model, only_include_views_which_want_realtime_events=True
     )
     assert row_checker.get_public_views_where_row_is_visible(initially_visible_row) == [
-        public_view_with_row_showing.view_ptr
+        public_view_with_row_showing.view_ptr.specific
     ]
 
     # Update the row so it is no longer visible
@@ -1284,14 +1286,12 @@ def test_batch_update_rows_visible_in_public_view_to_be_not_visible_event_sent(
     row_checker = ViewHandler().get_public_views_row_checker(
         table, model, only_include_views_which_want_realtime_events=True
     )
-    assert row_checker.get_public_views_where_rows_are_visible(
+    public_views = row_checker.get_public_views_where_rows_are_visible(
         [initially_visible_row, initially_visible_row2]
-    ) == [
-        PublicViewRows(
-            ViewHandler().get_view_as_user(user, public_view_with_row_showing.id),
-            allowed_row_ids={1, 2},
-        )
-    ]
+    )
+    assert len(public_views) == 1
+    assert public_views[0].allowed_row_ids == {1, 2}
+    assert public_views[0].view.id == public_view_with_row_showing.id
 
     # Update rows so that they are no longer visible
     with transaction.atomic():
@@ -1395,7 +1395,7 @@ def test_given_row_visible_in_public_view_when_updated_to_still_be_visible_event
         table, model, only_include_views_which_want_realtime_events=True
     )
     assert row_checker.get_public_views_where_row_is_visible(initially_visible_row) == [
-        public_view_with_row_showing.view_ptr
+        public_view_with_row_showing.view_ptr.specific
     ]
 
     # Update the row so it is still visible but changed
@@ -1496,14 +1496,12 @@ def test_batch_update_rows_visible_in_public_view_still_be_visible_event_sent(
     row_checker = ViewHandler().get_public_views_row_checker(
         table, model, only_include_views_which_want_realtime_events=True
     )
-    assert row_checker.get_public_views_where_rows_are_visible(
+    public_views = row_checker.get_public_views_where_rows_are_visible(
         [initially_visible_row, initially_visible_row2]
-    ) == [
-        PublicViewRows(
-            ViewHandler().get_view_as_user(user, public_view_with_row_showing.id),
-            allowed_row_ids={1, 2},
-        )
-    ]
+    )
+    assert len(public_views) == 1
+    assert public_views[0].allowed_row_ids == {1, 2}
+    assert public_views[0].view.id == public_view_with_row_showing.id
 
     # Update the row so that they are still visible but changed
     with transaction.atomic():
@@ -1598,14 +1596,12 @@ def test_batch_update_subset_rows_visible_in_public_view_no_filters(
     row_checker = ViewHandler().get_public_views_row_checker(
         table, model, only_include_views_which_want_realtime_events=True
     )
-    assert row_checker.get_public_views_where_rows_are_visible(
+    public_views = row_checker.get_public_views_where_rows_are_visible(
         [initially_visible_row, initially_visible_row2]
-    ) == [
-        PublicViewRows(
-            ViewHandler().get_view_as_user(user, public_view_with_row_showing.id),
-            allowed_row_ids=PublicViewRows.ALL_ROWS_ALLOWED,
-        )
-    ]
+    )
+    assert len(public_views) == 1
+    assert public_views[0].allowed_row_ids == PublicViewRows.ALL_ROWS_ALLOWED
+    assert public_views[0].view.id == public_view_with_row_showing.id
 
     # Update the row so that they are still visible but changed
     with transaction.atomic():
@@ -1962,7 +1958,7 @@ def test_given_row_visible_in_public_view_when_moved_row_updated_sent(
         table, model, only_include_views_which_want_realtime_events=True
     )
     assert row_checker.get_public_views_where_row_is_visible(visible_moving_row) == [
-        public_view.view_ptr
+        public_view.view_ptr.specific
     ]
 
     # Move the visible row behind the invisible one

changelog/entries/unreleased/bug/1925_two_linked_tables_with_publically_shared_views_can_cause_bug.json

@@ -0,0 +1,7 @@
+{
+    "type": "bug",
+    "message": "two linked tables with publically shared views can cause bugs caused by prefetch caching fields",
+    "issue_number": 1925,
+    "bullet_points": [],
+    "created_at": "2023-08-24"
+}