[Fixes #10214] metadata_only filter not working properly (#10215) (#1…

…0269) * [Fixes #10214] metadata_only filter not working properly * [Fixes #10214] metadata_only filter not working properly * [Fixes #10214] metadata_only filter not working properly Co-authored-by: Alessio Fabiani <[email protected]> Co-authored-by: mattiagiupponi <[email protected]> Co-authored-by: Alessio Fabiani <[email protected]>
GeoNode · Nov 8, 2022 · ed9e56b · ed9e56b
1 parent e707b6a
commit ed9e56b
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 43 deletions.
diff --git a/geonode/base/api/permissions.py b/geonode/base/api/permissions.py
@@ -17,16 +17,17 @@
 #
 #########################################################################
 import logging
+from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.shortcuts import get_object_or_404
 
 from rest_framework import permissions
 from rest_framework.filters import BaseFilterBackend
 from geonode.security.permissions import BASIC_MANAGE_PERMISSIONS, DOWNLOAD_PERMISSIONS, EDIT_PERMISSIONS, VIEW_PERMISSIONS
-
+from distutils.util import strtobool
 from geonode.security.utils import (
     get_users_with_perms,
-    get_resources_with_perms)
+    get_visible_resources)
 from geonode.groups.models import GroupProfile
 from rest_framework.permissions import DjangoModelPermissions
 from guardian.shortcuts import get_objects_for_user
@@ -194,26 +195,22 @@ class ResourceBasePermissionsFilter(BaseFilterBackend):
     A filter backend that limits results to those where the requesting user
     has read object level permissions.
     """
-    shortcut_kwargs = {
-        'accept_global_perms': True,
-    }
 
     def filter_queryset(self, request, queryset, view):
-        # We want to defer this import until runtime, rather than import-time.
-        # See https://github.com/encode/django-rest-framework/issues/4608
-        # (Also see #1624 for why we need to make this import explicitly)
-
-        user = request.user
-        # perm_format = '%(app_label)s.view_%(model_name)s'
-        # permission = self.perm_format % {
-        #     'app_label': queryset.model._meta.app_label,
-        #     'model_name': queryset.model._meta.model_name,
-        # }
-
-        obj_with_perms = get_resources_with_perms(user, shortcut_kwargs=self.shortcut_kwargs)
-        logger.debug(f" user: {user} -- obj_with_perms: {obj_with_perms}")
 
-        return queryset.filter(id__in=obj_with_perms.values('id'))
+        try:
+            metadata_only = strtobool(request.query_params.get("filter{metadata_only}", "None"))
+        except Exception:
+            metadata_only = None
+
+        return get_visible_resources(
+            queryset,
+            request.user,
+            metadata_only=metadata_only,
+            admin_approval_required=settings.ADMIN_MODERATE_UPLOADS,
+            unpublished_not_visible=settings.RESOURCE_PUBLISHING,
+            private_groups_not_visibile=settings.GROUP_PRIVATE_RESOURCES
+        )
 
 
 class UserHasPerms(DjangoModelPermissions):

diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py
@@ -436,6 +436,13 @@ def test_base_resources(self):
         response = self.client.get(url, format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
+        self.assertEqual(response.data['total'], 28)
+
+        url = "{base_url}?{params}".format(base_url=reverse('base-resources-list'), params="filter{metadata_only}=false")
+        # Anonymous
+        response = self.client.get(url, format='json')
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 26)
         response.data['resources'][0].get('executions')
         # Pagination
@@ -494,7 +501,7 @@ def test_base_resources(self):
         # Admin
         self.assertTrue(self.client.login(username='admin', password='admin'))
 
-        response = self.client.get(f"{url}?page_size=17", format='json')
+        response = self.client.get(f"{url}&page_size=17", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 26)
@@ -505,25 +512,27 @@ def test_base_resources(self):
         resource = ResourceBase.objects.filter(owner__username='bobby').first()
         self.assertEqual(resource.owner.username, 'bobby')
         # Admin
-        response = self.client.get(f"{url}/{resource.id}/", format='json')
+        url_with_id = "{base_url}/{res_id}?{params}".format(base_url=reverse('base-resources-list'), res_id=resource.id, params="filter{metadata_only}=false")
+
+        response = self.client.get(f"{url_with_id}", format='json')
         self.assertEqual(response.data['resource']['state'], enumerations.STATE_PROCESSED)
         self.assertEqual(response.data['resource']['sourcetype'], enumerations.SOURCE_TYPE_LOCAL)
         self.assertTrue('change_resourcebase' in list(response.data['resource']['perms']))
         # Annonymous
         self.assertIsNone(self.client.logout())
-        response = self.client.get(f"{url}/{resource.id}/", format='json')
+        response = self.client.get(f"{url_with_id}", format='json')
         self.assertFalse('change_resourcebase' in list(response.data['resource']['perms']))
         # user owner
         self.assertTrue(self.client.login(username='bobby', password='bob'))
-        response = self.client.get(f"{url}/{resource.id}/", format='json')
+        response = self.client.get(f"{url_with_id}", format='json')
         self.assertTrue('change_resourcebase' in list(response.data['resource']['perms']))
         # user not owner and not assigned
         self.assertTrue(self.client.login(username='norman', password='norman'))
-        response = self.client.get(f"{url}/{resource.id}/", format='json')
+        response = self.client.get(f"{url_with_id}", format='json')
         self.assertFalse('change_resourcebase' in list(response.data['resource']['perms']))
         # Check executions are returned when deffered
         # all resources
-        response = self.client.get(f'{url}?include[]=executions', format='json')
+        response = self.client.get(f'{url}&include[]=executions', format='json')
         self.assertEqual(response.status_code, 200)
         self.assertIsNotNone(response.data['resources'][0].get('executions'))
         # specific resource
@@ -555,7 +564,7 @@ def test_base_resources(self):
                 )
         }]
         self.assertTrue(self.client.login(username='bobby', password='bob'))
-        response = self.client.get(f'{url}/{resource.id}?include[]=executions', format='json')
+        response = self.client.get(f'{url_with_id}&include[]=executions', format='json')
         self.assertEqual(response.status_code, 200)
         self.assertIsNotNone(response.data['resource'].get('executions'))
         self.assertEqual(response.data['resource'].get('executions'), expected_executions_results)
@@ -567,7 +576,7 @@ def test_base_resources(self):
             self.assertEqual(6, resource.tkeywords.count())
             # Admin
             self.assertTrue(self.client.login(username='admin', password='admin'))
-            response = self.client.get(f"{url}/{resource.id}/", format='json')
+            response = self.client.get(f"{url_with_id}", format='json')
             self.assertIsNotNone(response.data['resource']['tkeywords'])
             self.assertEqual(6, len(response.data['resource']['tkeywords']))
             self.assertListEqual(
@@ -721,15 +730,15 @@ def test_filter_resources(self):
         self.assertTrue(self.client.login(username='admin', password='admin'))
 
         # Filter by owner == bobby
-        response = self.client.get(f"{url}?filter{{owner.username}}=bobby", format='json')
+        response = self.client.get(f"{url}?filter{{owner.username}}=bobby&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 3)
         # Pagination
         self.assertEqual(len(response.data['resources']), 3)
 
         # Filter by resource_type == document
-        response = self.client.get(f"{url}?filter{{resource_type}}=document", format='json')
+        response = self.client.get(f"{url}?filter{{resource_type}}=document&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 9)
@@ -738,7 +747,7 @@ def test_filter_resources(self):
 
         # Filter by resource_type == layer and title like 'common morx'
         response = self.client.get(
-            f"{url}?filter{{resource_type}}=dataset&filter{{title.icontains}}=common morx", format='json')
+            f"{url}?filter{{resource_type}}=dataset&filter{{title.icontains}}=common morx&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 1)
@@ -747,7 +756,7 @@ def test_filter_resources(self):
 
         # Filter by Keywords
         response = self.client.get(
-            f"{url}?filter{{keywords.name}}=here", format='json')
+            f"{url}?filter{{keywords.name}}=here&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 1)
@@ -756,7 +765,7 @@ def test_filter_resources(self):
 
         # Filter by Metadata Regions
         response = self.client.get(
-            f"{url}?filter{{regions.name.icontains}}=Italy", format='json')
+            f"{url}?filter{{regions.name.icontains}}=Italy&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 0)
@@ -765,29 +774,29 @@ def test_filter_resources(self):
 
         # Filter by Metadata Categories
         response = self.client.get(
-            f"{url}?filter{{category.identifier}}=elevation", format='json')
+            f"{url}?filter{{category.identifier}}=elevation&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 6)
         # Pagination
         self.assertEqual(len(response.data['resources']), 6)
 
         # Extent Filter
-        response = self.client.get(f"{url}?page_size=26&extent=-180,-90,180,90", format='json')
+        response = self.client.get(f"{url}?page_size=26&extent=-180,-90,180,90&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 26)
         # Pagination
         self.assertEqual(len(response.data['resources']), 26)
 
-        response = self.client.get(f"{url}?page_size=26&extent=0,0,100,100", format='json')
+        response = self.client.get(f"{url}?page_size=26&extent=0,0,100,100&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 26)
         # Pagination
         self.assertEqual(len(response.data['resources']), 26)
 
-        response = self.client.get(f"{url}?page_size=26&extent=-10,-10,-1,-1", format='json')
+        response = self.client.get(f"{url}?page_size=26&extent=-10,-10,-1,-1&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 12)
@@ -797,7 +806,7 @@ def test_filter_resources(self):
         # Extent Filter: Crossing Dateline
         extent = "-180.0000,56.9689,-162.5977,70.7435,155.9180,56.9689,180.0000,70.7435"
         response = self.client.get(
-            f"{url}?page_size=26&extent={extent}", format='json')
+            f"{url}?page_size=26&extent={extent}&filter{{metadata_only}}=false", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
         self.assertEqual(response.data['total'], 12)
@@ -816,7 +825,7 @@ def test_sort_resources(self):
             f"{url}?sort[]=title", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
-        self.assertEqual(response.data['total'], 26)
+        self.assertEqual(response.data['total'], 28)
         # Pagination
         self.assertEqual(len(response.data['resources']), 10)
 
@@ -830,7 +839,7 @@ def test_sort_resources(self):
             f"{url}?sort[]=-title", format='json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 5)
-        self.assertEqual(response.data['total'], 26)
+        self.assertEqual(response.data['total'], 28)
         # Pagination
         self.assertEqual(len(response.data['resources']), 10)
 
@@ -1663,7 +1672,7 @@ def test_embed_urls(self):
 
         resources = ResourceBase.objects.all()
         for resource in resources:
-            url = reverse('base-resources-detail', kwargs={'pk': resource.pk})
+            url = "{base_url}?{params}".format(base_url=reverse('base-resources-detail', kwargs={'pk': resource.pk}), params="filter{metadata_only}=false")
             response = self.client.get(url, format='json')
             if resource.title.endswith('metadata true'):
                 self.assertEqual(response.status_code, 404)

diff --git a/geonode/security/tests.py b/geonode/security/tests.py
@@ -1505,6 +1505,38 @@ def test_get_visible_resources_should_return_updated_resource_with_metadata_only
         actual = get_visible_resources(queryset=layers, user=get_user_model().objects.get(username=self.user))
         self.assertEqual(layers.filter(dirty_state=False).count(), len(actual))
 
+    def test_get_visible_resources_should_return_resource_with_metadata_only_true(self):
+        '''
+        If metadata only is provided, it should return only the metadata resources
+        '''
+        try:
+            dataset = create_single_dataset("dataset_with_metadata_only_True")
+            dataset.metadata_only = True
+            dataset.save()
+
+            layers = Dataset.objects.all()
+            actual = get_visible_resources(queryset=layers, metadata_only=True, user=get_user_model().objects.get(username=self.user))
+            self.assertEqual(1, actual.count())
+        finally:
+            if dataset:
+                dataset.delete()
+
+    def test_get_visible_resources_should_return_resource_with_metadata_only_none(self):
+        '''
+        If metadata only is provided, it should return only the metadata resources
+        '''
+        try:
+            dataset = create_single_dataset("dataset_with_metadata_only_True")
+            dataset.metadata_only = True
+            dataset.save()
+
+            layers = Dataset.objects.all()
+            actual = get_visible_resources(queryset=layers, metadata_only=None, user=get_user_model().objects.get(username=self.user))
+            self.assertEqual(layers.count(), actual.count())
+        finally:
+            if dataset:
+                dataset.delete()
+
     @override_settings(
         ADMIN_MODERATE_UPLOADS=True,
         RESOURCE_PUBLISHING=True,

diff --git a/geonode/security/utils.py b/geonode/security/utils.py
@@ -78,9 +78,11 @@ def get_visible_resources(queryset,
     except Exception:
         pass
 
-    # Hide Dirty State Resources
-    filter_set = queryset.filter(
-        Q(dirty_state=False) & Q(metadata_only=metadata_only))
+    filter_set = queryset.filter(dirty_state=False)
+
+    if metadata_only is not None:
+        # Hide Dirty State Resources
+        filter_set = filter_set.filter(metadata_only=metadata_only)
 
     if not is_admin:
         if user: