Test performance improvements

geonode/base/api/serializers.py

@@ -258,13 +258,20 @@ def get_attribute(self, instance):
         return build_absolute_uri(avatar_url(instance, self.avatar_size))
 
 
+def _real_instance(instance):
+    # memoize the polymorphic downcast: three fields call it per row
+    if not hasattr(instance, "_real_instance_cache"):
+        instance._real_instance_cache = instance.get_real_instance()
+    return instance._real_instance_cache
+
+
 class EmbedUrlField(DynamicComputedField):
     def __init__(self, **kwargs):
         super().__init__(**kwargs)
 
     def get_attribute(self, instance):
         try:
-            _instance = instance.get_real_instance()
+            _instance = _real_instance(instance)
         except Exception as e:
             logger.exception(e)
             _instance = None
@@ -302,7 +309,7 @@ def get_attribute(self, instance):
             logger.info(
                 f"Field {self.field_name} is deprecated and will be removed in the future GeoNode version. Please refer to download_urls"
             )
-            _instance = instance.get_real_instance()
+            _instance = _real_instance(instance)
             return _instance.download_url if hasattr(_instance, "download_url") else None
         except Exception as e:
             logger.exception(e)
@@ -315,7 +322,7 @@ def __init__(self, **kwargs):
 
     def get_attribute(self, instance):
         try:
-            _instance = instance.get_real_instance()
+            _instance = _real_instance(instance)
         except Exception as e:
             logger.exception(e)
             raise e
@@ -339,14 +346,24 @@ def get_attribute(self, instance):
 
         elif _instance.resource_type in ["dataset"]:
             download_urls = []
+            request = self.context.get("request")
+            # reuse the already-resolved instance instead of re-resolving the alternate per handler
+            # (_resolve_dataset); gate on the cached download perm to keep the same visibility
+            _can_download = bool(request) and permissions_registry.user_has_perm(
+                request.user, _instance, "download_resourcebase", include_virtual=True, use_cache=True
+            )
             # lets get only the default one first to set it
             default_handler = get_default_dataset_download_handler()
-            obj = default_handler(self.context.get("request"), _instance.alternate)
+            obj = default_handler(request, _instance.alternate)
+            if _can_download:
+                obj._resource = _instance
             if obj.download_url:
                 download_urls.append({"url": obj.download_url, "ajax_safe": obj.is_ajax_safe, "default": True})
             # then let's prepare the payload with everything
             for handler in get_download_handlers():
-                obj = handler(self.context.get("request"), _instance.alternate)
+                obj = handler(request, _instance.alternate)
+                if _can_download:
+                    obj._resource = _instance
                 if obj.download_url:
                     download_urls.append({"url": obj.download_url, "ajax_safe": obj.is_ajax_safe, "default": False})
 
@@ -365,7 +382,12 @@ def __init__(self, **kwargs):
     def get_attribute(self, instance):
         _user = self.context.get("request")
         if _user and not _user.user.is_anonymous:
-            return Favorite.objects.filter(object_id=instance.pk, user=_user.user).exists()
+            # fetch the user favorites once per request, then membership check in python
+            fav_ids = self.context.get("_favorite_ids")
+            if fav_ids is None:
+                fav_ids = set(Favorite.objects.filter(user=_user.user).values_list("object_id", flat=True))
+                self.context["_favorite_ids"] = fav_ids
+            return instance.pk in fav_ids
         return False
 
 
@@ -556,9 +578,8 @@ class Meta:
     def to_representation(self, instance):
         ret = []
         link_fields = ["extension", "link_type", "name", "mime", "url"]
-        links = Link.objects.filter(
-            resource_id=instance,  # link_type__in=["OGC:WMS", "OGC:WFS", "OGC:WCS", "image", "metadata"]
-        )
+        # select_related asset/resource so the per-link asset + resource lookups below don't each hit the db
+        links = Link.objects.filter(resource_id=instance).select_related("asset", "resource")
         request = self.context.get("request", None)
         for lnk in links:
             formatted_link = model_to_dict(lnk, fields=link_fields)
@@ -571,7 +592,11 @@ def to_representation(self, instance):
                     "content": model_to_dict(lnk.asset, ["title", "description", "type", "created"]),
                 }
                 if request and permissions_registry.user_has_perm(
-                    request.user, lnk.resource.get_self_resource(), "download_resourcebase", include_virtual=True
+                    request.user,
+                    lnk.resource.get_self_resource(),
+                    "download_resourcebase",
+                    include_virtual=True,
+                    use_cache=True,
                 ):
                     extras["content"]["download_url"] = asset_handler_registry.get_handler(
                         lnk.asset
@@ -845,8 +870,14 @@ def to_representation(self, instance):
         data = super().to_representation(instance)
         if not isinstance(data, int):
             try:
+                # the permission-filtered base queryset is identical for every facet row of the
+                # request, so build it once instead of re-running the whole pipeline per row
+                base_qs = self.context.get("_count_base_qs")
+                if base_qs is None:
+                    base_qs = get_resources_with_perms(request.user, filter_options)
+                    self.context["_count_base_qs"] = base_qs
                 count_filter = {self.Meta.count_type: instance}
-                data["count"] = get_resources_with_perms(request.user, filter_options).filter(**count_filter).count()
+                data["count"] = base_qs.filter(**count_filter).count()
             except (TypeError, NoReverseMatch) as e:
                 logger.exception(e)
         return data

geonode/base/api/tests.py

@@ -75,6 +75,7 @@
     License,
     Group,
     LinkedResource,
+    Link,
 )
 
 from geonode.layers.models import Dataset
@@ -112,6 +113,36 @@ def setUpTestData(cls):
         create_models(b"map")
         create_models(b"dataset")
 
+    def test_download_urls_visibility_follows_download_permission(self):
+        """download_urls exposes the download link only to users allowed to download the dataset."""
+        bobby = get_user_model().objects.get(username="bobby")
+        norman = get_user_model().objects.get(username="norman")
+        dataset = create_single_dataset("perm_download_ds", owner=bobby)
+        download = "http://example.org/perm_download_ds.zip"
+        try:
+            Link.objects.create(
+                resource=dataset.get_self_resource(),
+                link_type="original",
+                name="original",
+                extension="zip",
+                mime="application/zip",
+                url=download,
+            )
+            dataset.set_permissions({"users": {norman: ["base.view_resourcebase"]}})
+            url = f"{reverse('base-resources-list')}/{dataset.id}?filter{{metadata_only}}=false"
+            # owner can download: the link is present
+            self.assertTrue(self.client.login(username="bobby", password="bob"))
+            response = self.client.get(url, format="json")
+            self.assertEqual(response.status_code, 200)
+            self.assertIn(download, [_d["url"] for _d in response.data["resource"]["download_urls"]])
+            # user with view but not download: the link is hidden
+            self.assertTrue(self.client.login(username="norman", password="norman"))
+            response = self.client.get(url, format="json")
+            self.assertEqual(response.status_code, 200)
+            self.assertNotIn(download, [_d["url"] for _d in response.data["resource"]["download_urls"]])
+        finally:
+            dataset.delete()
+
     def test_groups_list(self):
         """
         Ensure we can access the gropus list.

geonode/base/api/views.py

@@ -305,7 +305,13 @@ class ResourceBaseViewSet(ApiPresetsInitializer, MultiLangViewMixin, DeprecatedE
         ResourceBasePermissionsFilter,
         FavoriteFilter,
     ]
-    queryset = ResourceBase.objects.select_related("owner").order_by("-created")
+    queryset = (
+        ResourceBase.objects.select_related(
+            "owner", "category", "license", "group", "restriction_code_type", "spatial_representation_type"
+        )
+        # keywords/tkeywords/regions are prefetched by dynamic-rest; contactrole is a custom field, prefetch it here
+        .prefetch_related("contactrole_set__contact").order_by("-created")
+    )
     serializer_class = ResourceBaseSerializer
     pagination_class = GeoNodeApiPagination
 

geonode/base/models.py

@@ -1745,8 +1745,8 @@ def __get_contact_role_elements__(self, role: str) -> Optional[List[settings.AUT
             Optional[List[settings.AUTH_USER_MODEL]]: returns the requested contact role from the database
         """
         try:
-            contact_role = ContactRole.objects.filter(role=role, resource=self)
-            contacts = [cr.contact for cr in contact_role]
+            # single reverse-set read + python filter, instead of one query per role
+            contacts = [cr.contact for cr in self.contactrole_set.all() if cr.role == role]
         except ContactRole.DoesNotExist:
             contacts = None
         return contacts

geonode/security/registry.py

@@ -70,15 +70,15 @@ def sanity_checks(self):
         for item in self.REGISTRY:
             self.__check_item(item)
 
-    def user_has_perm(self, user, instance=None, perm="", include_virtual=False):
+    def user_has_perm(self, user, instance=None, perm="", include_virtual=False, use_cache=False):
         """
         Returns True if the user has the defined perm (permission)
         """
         if not perm:
             return False
 
         resolved_perms = self.get_perms(
-            instance=instance, user=user, include_virtual=True, include_user_add_resource=True
+            instance=instance, user=user, include_virtual=True, include_user_add_resource=True, use_cache=use_cache
         )
 
         if isinstance(perm, list):