Skip to content

Commit

Permalink
[Fixes #9604] Report empty thumbnail in case a thumbnail has not been…
Browse files Browse the repository at this point in the history
… generated for a resource (#9613) (#9810)

* -[Fixes #9604] Report empty thumbnail in case a thumbnail has not been generated for a resource

* - update tests

* -remove unused import

* -remove unused import

* - create Thumbnail Link if thumbnail_url exists

* - fix tests

* - fix pep8

* - add command to clean resources thumbs

* - fix lgtm

* - rename migration file

* - fix migration dependence

Co-authored-by: Alessio Fabiani <alessio.fabiani@geosolutionsgroup.com>
(cherry picked from commit 38cbd9d)

Co-authored-by: NAGGINDA MARTHA <marthamareal@gmail.com>
  • Loading branch information
afabiani and marthamareal committed Aug 23, 2022
1 parent 6d773d6 commit a5df4ec
Show file tree
Hide file tree
Showing 13 changed files with 61 additions and 70 deletions.
5 changes: 0 additions & 5 deletions geonode/api/resourcebase_api.py
Expand Up @@ -23,7 +23,6 @@
from django.db.models import Q
from django.http import HttpResponse
from django.conf import settings
from django.templatetags.static import static
from tastypie.authentication import MultiAuthentication, SessionAuthentication
from tastypie.bundle import Bundle

Expand Down Expand Up @@ -52,7 +51,6 @@
from geonode.base.bbox_utils import filter_bbox
from geonode.groups.models import GroupProfile
from geonode.utils import check_ogc_backend
from geonode.thumbs.utils import MISSING_THUMB
from geonode.security.utils import get_visible_resources
from .authentication import OAuthAuthentication
from .authorization import GeoNodeAuthorization, GeonodeApiKeyAuthentication
Expand Down Expand Up @@ -576,9 +574,6 @@ def format_objects(self, objects):
if 'site_url' not in formatted_obj or len(formatted_obj['site_url']) == 0:
formatted_obj['site_url'] = settings.SITEURL

if formatted_obj['thumbnail_url'] and len(formatted_obj['thumbnail_url']) == 0:
formatted_obj['thumbnail_url'] = static(MISSING_THUMB)

formatted_obj['owner__username'] = obj.owner.username
formatted_obj['owner_name'] = obj.owner.get_full_name() or obj.owner.username

Expand Down
32 changes: 32 additions & 0 deletions geonode/base/migrations/0083_clean_resources_with_missing_thumb.py
@@ -0,0 +1,32 @@
import logging

from django.db import migrations
from django.templatetags.static import static

from geonode.thumbs.utils import MISSING_THUMB

logger = logging.getLogger(__name__)


def set_null_thumbnail(apps, _):
"Sets thumbnail_url to null for resources with thumbnail_url=missing_thumb"
try:
resource_model = apps.get_model('base', 'ResourceBase')
link_model = apps.get_model('base', 'Link')
# update thumbnail urls
resource_model.objects.filter(thumbnail_url__icontains=static(MISSING_THUMB)).update(thumbnail_url=None)
# Remove thumbnail links
link_model.objects.filter(resource__thumbnail_url__isnull=True, name='Thumbnail').delete()
except Exception as e:
logger.exception(e)


class Migration(migrations.Migration):

dependencies = [
('base', '0082_remove_dialogos_comment'),
]

operations = [
migrations.RunPython(set_null_thumbnail, migrations.RunPython.noop),
]
24 changes: 2 additions & 22 deletions geonode/base/models.py
Expand Up @@ -45,7 +45,6 @@
from django.core.exceptions import ValidationError
from django.utils.translation import ugettext_lazy as _
from django.contrib.contenttypes.models import ContentType
from django.templatetags.static import static
from django.utils.html import strip_tags
from mptt.models import MPTTModel, TreeForeignKey

Expand Down Expand Up @@ -73,7 +72,6 @@
get_allowed_extensions,
is_monochromatic_image)
from geonode.thumbs.utils import (
MISSING_THUMB,
thumb_size,
remove_thumbs,
get_unique_upload_path)
Expand Down Expand Up @@ -1707,9 +1705,8 @@ def get_thumbnail_url(self):
"""Return a thumbnail url.
It could be a local one if it exists, a remote one (WMS GetImage) for example
or a 'Missing Thumbnail' one.
"""
_thumbnail_url = self.thumbnail_url or static(MISSING_THUMB)
_thumbnail_url = self.thumbnail_url
local_thumbnails = self.link_set.filter(name='Thumbnail')
remote_thumbnails = self.link_set.filter(name='Remote Thumbnail')
if local_thumbnails.exists():
Expand Down Expand Up @@ -1789,7 +1786,7 @@ def save_thumbnail(self, filename, image):
)
)
# Cleaning up the old stuff
if self.thumbnail_path and MISSING_THUMB not in self.thumbnail_path and storage_manager.exists(self.thumbnail_path):
if self.thumbnail_path and storage_manager.exists(self.thumbnail_path):
storage_manager.delete(self.thumbnail_path)
# Store the new url and path
self.thumbnail_url = url
Expand All @@ -1806,23 +1803,6 @@ def save_thumbnail(self, filename, image):
)
try:
Link.objects.filter(resource=self, name='Thumbnail').delete()
_thumbnail_url = static(MISSING_THUMB)
obj, _created = Link.objects.get_or_create(
resource=self,
name='Thumbnail',
defaults=dict(
url=_thumbnail_url,
extension='png',
mime='image/png',
link_type='image',
)
)
self.thumbnail_url = _thumbnail_url
obj.url = _thumbnail_url
obj.save()
ResourceBase.objects.filter(id=self.id).update(
thumbnail_url=_thumbnail_url
)
except Exception as e:
logger.error(
f'Error when generating the thumbnail for resource {self.id}. ({e})'
Expand Down
9 changes: 4 additions & 5 deletions geonode/base/tests.py
Expand Up @@ -21,7 +21,6 @@
import requests

from uuid import uuid4
from urllib.parse import urlparse
from unittest.mock import patch, Mock
from django.core.exceptions import ObjectDoesNotExist

Expand Down Expand Up @@ -91,19 +90,19 @@ def tearDown(self):

def test_initial_behavior(self):
"""
Tests that an empty resource has a missing image as default thumbnail.
Tests that an empty resource has a missing image as null.
"""
self.assertFalse(self.rb.has_thumbnail())
missing = self.rb.get_thumbnail_url()
self.assertTrue('missing_thumb' in os.path.splitext(missing)[0])
self.assertIsNone(missing)

def test_empty_image(self):
"""
Tests that an empty image does not change the current resource thumbnail.
"""
current = self.rb.get_thumbnail_url()
self.rb.save_thumbnail('test-thumb', None)
self.assertEqual(current, urlparse(self.rb.get_thumbnail_url()).path)
self.assertEqual(current, self.rb.get_thumbnail_url())

@patch('PIL.Image.open', return_value=test_image)
def test_monochromatic_image(self, image):
Expand All @@ -114,7 +113,7 @@ def test_monochromatic_image(self, image):

current = self.rb.get_thumbnail_url()
self.rb.save_thumbnail(filename, image)
self.assertEqual(current, urlparse(self.rb.get_thumbnail_url()).path)
self.assertEqual(current, self.rb.get_thumbnail_url())

# cleanup: remove saved thumbnail
thumb_utils.remove_thumbs(filename)
Expand Down
4 changes: 1 addition & 3 deletions geonode/geoserver/helpers.py
Expand Up @@ -42,7 +42,6 @@
from django.conf import settings
from django.utils import timezone
from django.db import transaction
from django.templatetags.static import static
from django.contrib.auth import get_user_model
from django.utils.module_loading import import_string
from django.contrib.contenttypes.models import ContentType
Expand All @@ -64,7 +63,6 @@
from geonode import GeoNodeException
from geonode.base.models import Link
from geonode.base.models import ResourceBase
from geonode.thumbs.utils import MISSING_THUMB
from geonode.security.views import _perms_info_json
from geonode.catalogue.models import catalogue_post_save
from geonode.layers.models import Dataset, Attribute, Style
Expand Down Expand Up @@ -2142,7 +2140,7 @@ def sync_instance_with_geoserver(
}

if updatebbox and is_monochromatic_image(instance.thumbnail_url):
to_update['thumbnail_url'] = static(MISSING_THUMB)
to_update['thumbnail_url'] = None

# Save all the modified information in the instance without triggering signals.
with transaction.atomic():
Expand Down
4 changes: 1 addition & 3 deletions geonode/geoserver/manager.py
Expand Up @@ -28,15 +28,13 @@
from django.conf import settings
from django.db.models.query import QuerySet
from django.contrib.auth.models import Group
from django.templatetags.static import static
from django.contrib.auth import get_user_model

from geonode.maps.models import Map
from geonode.base import enumerations
from geonode.layers.models import Dataset
from geonode.upload.models import Upload
from geonode.base.models import ResourceBase
from geonode.thumbs.utils import MISSING_THUMB
from geonode.utils import get_dataset_workspace
from geonode.services.enumerations import CASCADED
from geonode.security.utils import skip_registered_members_common_group
Expand Down Expand Up @@ -478,7 +476,7 @@ def set_permissions(self, uuid: str, /, instance: ResourceBase = None, owner: se

def set_thumbnail(self, uuid: str, /, instance: ResourceBase = None, overwrite: bool = True, check_bbox: bool = True) -> bool:
if instance and (isinstance(instance.get_real_instance(), Dataset) or isinstance(instance.get_real_instance(), Map)):
if overwrite or instance.thumbnail_url == static(MISSING_THUMB):
if overwrite or not instance.thumbnail_url:
create_gs_thumbnail(instance.get_real_instance(), overwrite=overwrite, check_bbox=check_bbox)
return True
return False
Expand Down
6 changes: 1 addition & 5 deletions geonode/geoserver/signals.py
Expand Up @@ -23,7 +23,6 @@
from geoserver.layer import Layer as GsLayer

from django.db.models import Q
from django.templatetags.static import static
from django.dispatch import Signal

# use different name to avoid module clash
Expand All @@ -35,7 +34,6 @@
from geonode.geoserver.tasks import geoserver_create_thumbnail
from geonode.layers.models import Dataset
from geonode.services.enumerations import CASCADED
from geonode.thumbs.utils import MISSING_THUMB

from . import BACKEND_PACKAGE
from .tasks import geoserver_cascading_delete, geoserver_post_save_datasets
Expand Down Expand Up @@ -117,8 +115,7 @@ def geoserver_pre_save_maplayer(instance, sender, **kwargs):
def geoserver_post_save_map(instance, sender, created, **kwargs):
instance.set_missing_info()
if not created:
if not instance.thumbnail_url or \
instance.thumbnail_url == static(MISSING_THUMB):
if not instance.thumbnail_url:
logger.debug(f"... Creating Thumbnail for Map [{instance.title}]")
geoserver_create_thumbnail.apply_async((instance.id, False, True, ))

Expand All @@ -135,7 +132,6 @@ def geoserver_set_thumbnail(instance, **kwargs):
'thumbnail_url' in kwargs['update_fields']:
_recreate_thumbnail = True
if not instance.thumbnail_url or \
instance.thumbnail_url == static(MISSING_THUMB) or \
is_monochromatic_image(instance.thumbnail_url):
_recreate_thumbnail = True
if _recreate_thumbnail:
Expand Down
3 changes: 1 addition & 2 deletions geonode/maps/api/tests.py
Expand Up @@ -27,7 +27,6 @@
from geonode.base.populate_test_data import create_models
from geonode.layers.models import Dataset
from geonode.maps.models import Map, MapLayer
from geonode.thumbs.utils import MISSING_THUMB

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -192,7 +191,7 @@ def test_create_map(self):
self.assertEqual(response_maplayer["extra_params"], {"msId": "Stamen.Watercolor__0"})
self.assertEqual(response_maplayer["current_style"], "some-style-first-layer")
self.assertIsNotNone(response_maplayer["dataset"])
self.assertNotIn(MISSING_THUMB, response.data["map"]['thumbnail_url'])
self.assertIsNotNone(response.data["map"]['thumbnail_url'])


DUMMY_MAPDATA = {
Expand Down
4 changes: 1 addition & 3 deletions geonode/resource/manager.py
Expand Up @@ -37,7 +37,6 @@
from django.db import transaction
from django.db.models.query import QuerySet
from django.contrib.auth.models import Group
from django.templatetags.static import static
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Permission
from django.contrib.contenttypes.models import ContentType
Expand All @@ -48,7 +47,6 @@

from geonode.thumbs.thumbnails import _generate_thumbnail_name
from geonode.documents.tasks import create_document_thumbnail
from geonode.thumbs import utils as thumb_utils
from geonode.security.permissions import (
PermSpecCompact,
DATA_STYLABLE_RESOURCES_SUBTYPES)
Expand Down Expand Up @@ -819,7 +817,7 @@ def set_thumbnail(self, uuid: str, /, instance: ResourceBase = None, overwrite:
_resource.save_thumbnail(file_name, thumbnail)
else:
if instance and instance.files and isinstance(instance.get_real_instance(), Document):
if overwrite or instance.thumbnail_url == static(thumb_utils.MISSING_THUMB):
if overwrite or not instance.thumbnail_url:
create_document_thumbnail.apply((instance.id,))
self._concrete_resource_manager.set_thumbnail(uuid, instance=_resource, overwrite=overwrite, check_bbox=check_bbox)
return True
Expand Down
6 changes: 1 addition & 5 deletions geonode/thumbs/tests/test_integration.py
Expand Up @@ -33,8 +33,6 @@
from django.urls import reverse
from django.contrib.auth import get_user_model
from django.test.utils import override_settings
from django.templatetags.static import static


from geonode import geoserver
from geonode.maps.models import Map, MapLayer
Expand All @@ -45,7 +43,6 @@
from geonode.tests.base import GeoNodeBaseTestSupport
from geonode.thumbs.thumbnails import create_gs_thumbnail_geonode, create_thumbnail
from geonode.layers.models import Dataset
from geonode.thumbs.utils import MISSING_THUMB
from geonode.thumbs.background import (
OSMTileBackground,
WikiMediaTileBackground,
Expand All @@ -60,7 +57,6 @@

logger = logging.getLogger(__name__)

missing_thumbnail_url = static(MISSING_THUMB)

LOCAL_TIMEOUT = 300
EXPECTED_RESULTS_DIR = "geonode/thumbs/tests/expected_results/"
Expand Down Expand Up @@ -535,7 +531,7 @@ def setUpClass(cls):
cls.map_composition.refresh_from_db()

def _fetch_thumb_and_compare(self, url, expected_image):
if url == missing_thumbnail_url:
if not url:
logger.error(f'It was not possible to fetch the remote dataset WMS GetMap! thumb_url: {url}')
return
_, img = http_client.request(url)
Expand Down
3 changes: 1 addition & 2 deletions geonode/thumbs/thumbnails.py
Expand Up @@ -23,7 +23,6 @@
from typing import List, Union, Optional, Tuple

from django.conf import settings
from django.templatetags.static import static
from django.utils.module_loading import import_string

from geonode.documents.models import Document
Expand Down Expand Up @@ -94,7 +93,7 @@ def create_thumbnail(

# handle custom, uploaded thumbnails, which may have different extensions from the default thumbnail
thumbnail_exists = False
if instance.thumbnail_url and instance.thumbnail_url != static(utils.MISSING_THUMB):
if instance.thumbnail_url:
thumbnail_exists = utils.thumb_exists(instance.thumbnail_url.rsplit('/')[-1])

if (thumbnail_exists or utils.thumb_exists(default_thumbnail_name)) and not overwrite:
Expand Down
2 changes: 1 addition & 1 deletion geonode/thumbs/utils.py
Expand Up @@ -138,7 +138,7 @@ def expand_bbox_to_ratio(

def assign_missing_thumbnail(instance) -> None:
"""
Function assigning 'geonode.thumbs.utils.MISSING_THUMB' to a provided instance
Function assigning None in thumbnail_url to a provided instance
:param instance: instance of Dataset or Map models
"""
Expand Down
29 changes: 15 additions & 14 deletions geonode/utils.py
Expand Up @@ -1597,21 +1597,22 @@ def set_resource_default_links(instance, layer, prune=False, **kwargs):
logger.debug(f" -- Resource Links[Legend link]...error: {e}")

# Thumbnail link
logger.debug(" -- Resource Links[Thumbnail link]...")
if (Link.objects.filter(resource=instance.resourcebase_ptr,
url=instance.get_thumbnail_url(),
name='Thumbnail').count() < 2):
Link.objects.update_or_create(
resource=instance.resourcebase_ptr,
url=instance.get_thumbnail_url(),
name='Thumbnail',
defaults=dict(
extension='png',
mime='image/png',
link_type='image',
if instance.get_thumbnail_url():
logger.debug(" -- Resource Links[Thumbnail link]...")
if (Link.objects.filter(resource=instance.resourcebase_ptr,
url=instance.get_thumbnail_url(),
name='Thumbnail').count() < 2):
Link.objects.update_or_create(
resource=instance.resourcebase_ptr,
url=instance.get_thumbnail_url(),
name='Thumbnail',
defaults=dict(
extension='png',
mime='image/png',
link_type='image',
)
)
)
logger.debug(" -- Resource Links[Thumbnail link]...done!")
logger.debug(" -- Resource Links[Thumbnail link]...done!")

logger.debug(" -- Resource Links[OWS Links]...")
try:
Expand Down

0 comments on commit a5df4ec

Please sign in to comment.