From 33be915e234dbbf0228ebe7639f9e73b161c12a8 Mon Sep 17 00:00:00 2001 From: mattiagiupponi <51856725+mattiagiupponi@users.noreply.github.com> Date: Mon, 7 Oct 2024 11:24:34 +0200 Subject: [PATCH] [Fixes #12627] Include assets inside B/R (#12628) * [Fixes #12627] Include assets inside B/R * [Fixes #12627] force localasset in get_link_url * [Fixes #12627] fix pr comments * [Fixes #12627] add todo for assets_root replace * [Fixes #12627] change utils assets_root name --- geonode/assets/handlers.py | 1 + geonode/assets/local.py | 16 +++++++++ geonode/assets/models.py | 14 +++++++- geonode/assets/tests.py | 10 +++--- geonode/assets/utils.py | 5 +-- geonode/br/management/commands/backup.py | 36 ++++++++++++------- geonode/br/management/commands/restore.py | 28 ++++++++++----- .../commands/settings_docker_sample.ini | 4 +-- .../management/commands/settings_sample.ini | 4 +-- geonode/br/management/commands/utils/utils.py | 1 + geonode/documents/models.py | 4 +-- 11 files changed, 88 insertions(+), 35 deletions(-) diff --git a/geonode/assets/handlers.py b/geonode/assets/handlers.py index 3df019c521c..2e728ea3375 100644 --- a/geonode/assets/handlers.py +++ b/geonode/assets/handlers.py @@ -78,6 +78,7 @@ def get_default_handler(self) -> AssetHandlerInterface: return self._default_handler def get_handler(self, asset): + asset = asset.get_real_instance() if isinstance(asset, Asset) else asset asset_cls = asset if isinstance(asset, type) else asset.__class__ ret = self._registry.get(asset_cls, None) if not ret: diff --git a/geonode/assets/local.py b/geonode/assets/local.py index 558a22cecc4..aa8e486c485 100644 --- a/geonode/assets/local.py +++ b/geonode/assets/local.py @@ -28,6 +28,9 @@ def get_link_url(self, asset: LocalAsset): class IndexLocalLinkUrlHandler: def get_link_url(self, asset: LocalAsset): + asset = asset.get_real_instance() + if not isinstance(asset, LocalAsset): + raise TypeError("Only localasset are allowed") return build_absolute_uri(reverse("assets-link", args=(asset.pk,))) + f"/{os.path.basename(asset.location[0])}" @@ -76,6 +79,7 @@ def remove_data(self, asset: LocalAsset): Removes the files related to an Asset. Only files within the Assets directory are removed """ + asset = self.__force_real_instance(asset) if self._are_files_managed(asset): logger.info(f"Removing files for asset {asset.pk}") base = self._get_managed_dir(asset) @@ -85,6 +89,7 @@ def remove_data(self, asset: LocalAsset): logger.info(f"Not removing unmanaged files for asset {asset.pk}") def replace_data(self, asset: LocalAsset, files: list): + asset = self.__force_real_instance(asset) self.remove_data(asset) asset.location = files asset.save() @@ -123,6 +128,7 @@ def _clone_data(self, source_dir): def clone(self, source: LocalAsset) -> LocalAsset: # get a new asset instance to be edited and stored back + source = self.__force_real_instance(source) asset = LocalAsset.objects.get(pk=source.pk) # only copy files if they are managed @@ -204,12 +210,22 @@ def _get_managed_dir(cls, asset): return managed_dir + @classmethod + def __force_real_instance(cls, asset): + asset = asset.get_real_instance() + if not isinstance(asset, LocalAsset): + raise TypeError(f"Real instance of asset {asset} is not {cls.handled_asset_class()}") + return asset + class LocalAssetDownloadHandler(AssetDownloadHandlerInterface): def create_response( self, asset: LocalAsset, attachment: bool = False, basename: str = None, path: str = None ) -> HttpResponse: + asset = asset.get_real_instance() + if not isinstance(asset, LocalAsset): + raise TypeError("Only localasset are allowed") if not asset.location: return HttpResponse("Asset does not contain any data", status=500) diff --git a/geonode/assets/models.py b/geonode/assets/models.py index a4fe3a26de6..2d817913037 100644 --- a/geonode/assets/models.py +++ b/geonode/assets/models.py @@ -5,6 +5,18 @@ from django.contrib.auth import get_user_model +class AssetPolymorphicManager(PolymorphicManager): + """ + This override is required for the dump procedure. + Otherwise django is not able to dump the base objects + and will be upcasted to polymorphic models + https://github.com/jazzband/django-polymorphic/blob/cfd49b26d580d99b00dcd43a02409ce439a2c78f/polymorphic/base.py#L161-L175 + """ + + def get_queryset(self): + return super().get_queryset().non_polymorphic() + + class Asset(PolymorphicModel): """ A generic data linked to a ResourceBase @@ -16,7 +28,7 @@ class Asset(PolymorphicModel): owner = models.ForeignKey(get_user_model(), null=False, blank=False, on_delete=models.CASCADE) created = models.DateTimeField(auto_now_add=True) - objects = PolymorphicManager() + objects = AssetPolymorphicManager() class Meta: verbose_name_plural = "Assets" diff --git a/geonode/assets/tests.py b/geonode/assets/tests.py index 8efb66ac181..0401a7a0943 100644 --- a/geonode/assets/tests.py +++ b/geonode/assets/tests.py @@ -71,7 +71,7 @@ def test_creation_and_delete_data_cloned(self): asset.save() self.assertIsInstance(asset, LocalAsset) - reloaded = Asset.objects.get(pk=asset.pk) + reloaded = LocalAsset.objects.get(pk=asset.pk) self.assertIsNotNone(reloaded) self.assertIsInstance(reloaded, LocalAsset) file = reloaded.location[0] @@ -103,7 +103,7 @@ def test_creation_and_delete_data_external(self): asset.save() self.assertIsInstance(asset, LocalAsset) - reloaded = Asset.objects.get(pk=asset.pk) + reloaded = LocalAsset.objects.get(pk=asset.pk) self.assertIsNotNone(reloaded) self.assertIsInstance(reloaded, LocalAsset) file = reloaded.location[0] @@ -128,7 +128,7 @@ def test_clone_and_delete_data_managed(self): asset.save() self.assertIsInstance(asset, LocalAsset) - reloaded = Asset.objects.get(pk=asset.pk) + reloaded = LocalAsset.objects.get(pk=asset.pk) cloned = asset_handler.clone(reloaded) self.assertNotEqual(reloaded.pk, cloned.pk) @@ -161,7 +161,7 @@ def test_clone_and_delete_data_unmanaged(self): asset.save() self.assertIsInstance(asset, LocalAsset) - reloaded = Asset.objects.get(pk=asset.pk) + reloaded = LocalAsset.objects.get(pk=asset.pk) cloned = asset_handler.clone(reloaded) self.assertEqual(reloaded.location[0], cloned.location[0]) @@ -268,7 +268,7 @@ def _setup_test(self, u, _file=ONE_JSON): asset.save() self.assertIsInstance(asset, LocalAsset) - reloaded = Asset.objects.get(pk=asset.pk) + reloaded = LocalAsset.objects.get(pk=asset.pk) # put two more files in the asset dir asset_dir = os.path.dirname(reloaded.location[0]) diff --git a/geonode/assets/utils.py b/geonode/assets/utils.py index 06abd560ea0..e3a26a806e6 100644 --- a/geonode/assets/utils.py +++ b/geonode/assets/utils.py @@ -43,8 +43,8 @@ def get_default_asset(resource: ResourceBase, link_type=None) -> Asset or None: filters = {"link__resource": resource} if link_type: filters["link__link_type"] = link_type - - return Asset.objects.filter(**filters).first() + asset = Asset.objects.filter(**filters).first() + return asset.get_real_instance() if asset else None DEFAULT_TYPES = {"image": ["jpg", "jpeg", "gif", "png", "bmp", "svg"]} @@ -55,6 +55,7 @@ def find_type(ext): def create_link(resource, asset, link_type=None, extension=None, name=None, mime=None, asset_handler=None, **kwargs): + asset = asset.get_real_instance() asset_handler = asset_handler or asset_handler_registry.get_handler(asset) if not link_type or not extension or not name: diff --git a/geonode/br/management/commands/backup.py b/geonode/br/management/commands/backup.py index 1b23dde6915..c54c040cbc0 100644 --- a/geonode/br/management/commands/backup.py +++ b/geonode/br/management/commands/backup.py @@ -25,6 +25,9 @@ import re import logging +from geonode.assets.local import LocalAssetHandler +from geonode.assets.models import LocalAsset + from .utils import utils from requests.auth import HTTPBasicAuth @@ -166,23 +169,21 @@ def execute_backup(self, **options): logger.info(f" - Dumping '{app_name}' into '{dump_name}.json'") # Point stdout at a file for dumping data to. - with open(os.path.join(fixtures_target, f"{dump_name}.json"), "w") as output: - call_command("dumpdata", app_name, format="json", indent=2, stdout=output) + output_file = os.path.join(fixtures_target, f"{dump_name}.json") + call_command("dumpdata", app_name, output=output_file) # Store Media Root - logger.info("*** Dumping GeoNode media folder...") - media_root = settings.MEDIA_ROOT media_folder = os.path.join(target_folder, utils.MEDIA_ROOT) - if not os.path.exists(media_folder): - os.makedirs(media_folder, exist_ok=True) + logger.info("*** Dumping GeoNode media folder...") + self.backup_folder(folder=media_folder, root=settings.MEDIA_ROOT, config=config) - copy_tree( - media_root, - media_folder, - ignore=utils.ignore_time(config.gs_data_dt_filter[0], config.gs_data_dt_filter[1]), - ) - logger.info(f"Saved media files from '{media_root}'") + logger.info("*** Dumping GeoNode assets folder...") + assets_folder = os.path.join(target_folder, utils.ASSETS_ROOT) + self.backup_folder(folder=assets_folder, root=settings.ASSETS_ROOT, config=config) + for instance in LocalAsset.objects.iterator(): + if not LocalAssetHandler._are_files_managed(instance): + logger.warning(f"The file for the asset with id {instance.pk} were not backup since is not managed by GeoNode") # Create Final ZIP Archive logger.info("*** Creating final ZIP archive...") @@ -214,6 +215,17 @@ def execute_backup(self, **options): return str(os.path.join(backup_dir, f"{dir_time_suffix}.zip")) + def backup_folder(self, folder, root, config): + if not os.path.exists(folder): + os.makedirs(root, exist_ok=True) + + copy_tree( + root, + folder, + ignore=utils.ignore_time(config.gs_data_dt_filter[0], config.gs_data_dt_filter[1]), + ) + logger.info(f"Saved files from '{root}'") + def create_geoserver_backup(self, config, settings, target_folder, ignore_errors): # Create GeoServer Backup url = settings.OGC_SERVER["default"]["LOCATION"] diff --git a/geonode/br/management/commands/restore.py b/geonode/br/management/commands/restore.py index e8b48ba8287..012122ac83a 100755 --- a/geonode/br/management/commands/restore.py +++ b/geonode/br/management/commands/restore.py @@ -265,13 +265,16 @@ def execute_restore(self, **options): # Write Checks media_root = settings.MEDIA_ROOT media_folder = os.path.join(target_folder, utils.MEDIA_ROOT) - + assets_root = settings.ASSETS_ROOT + assets_folder = os.path.join(target_folder, utils.ASSETS_ROOT) try: logger.info("*** Performing some checks...") logger.info(f"[Sanity Check] Full Write Access to restore folder: '{restore_folder}' ...") chmod_tree(restore_folder) logger.info(f"[Sanity Check] Full Write Access to media root: '{media_root}' ...") chmod_tree(media_root) + logger.info(f"[Sanity Check] Full Write Access to assets root: '{assets_root}' ...") + chmod_tree(assets_root) except Exception as e: if notify: restore_notification.apply_async( @@ -393,15 +396,11 @@ def execute_restore(self, **options): # Restore Media Root logger.info("*** Restore media root...") - if config.gs_data_dt_filter[0] is None: - shutil.rmtree(media_root, ignore_errors=True) - - if not os.path.exists(media_root): - os.makedirs(media_root, exist_ok=True) + self.restore_folder(config, media_root, media_folder) + logger.info("*** Restore assets root...") + self.restore_folder(config, assets_root, assets_folder) - copy_tree(media_folder, media_root) - chmod_tree(media_root) - logger.info(f"Media files restored into '{media_root}'.") + # TODO improve this part, by saving the original asset_root path in a variable, then replace with the new one # store backup info restored_backup = RestoredBackup( @@ -441,6 +440,17 @@ def execute_restore(self, **options): logger.info("*** Final filesystem cleanup ...") shutil.rmtree(restore_folder) + def restore_folder(self, config, root, folder): + if config.gs_data_dt_filter[0] is None: + shutil.rmtree(root, ignore_errors=True) + + if not os.path.exists(root): + os.makedirs(root, exist_ok=True) + + copy_tree(folder, root) + chmod_tree(root) + logger.info(f"Files restored into '{root}'.") + def validate_backup_file_options(self, **options) -> None: """ Method validating --backup-file and --backup-files-dir options diff --git a/geonode/br/management/commands/settings_docker_sample.ini b/geonode/br/management/commands/settings_docker_sample.ini index 68bbfdf4eff..f5401a9c8fa 100644 --- a/geonode/br/management/commands/settings_docker_sample.ini +++ b/geonode/br/management/commands/settings_docker_sample.ini @@ -13,6 +13,6 @@ dumprasterdata = yes # data_layername_exclude_filter = {comma separated list of layernames, optionally with glob syntax} e.g.: tuscany_*,italy [fixtures] -apps = contenttypes,auth,people,groups,account,guardian,admin,actstream,announcements,avatar,base,documents,geoserver,invitations,pinax_notifications,harvesting,services,layers,maps,oauth2_provider,sites,socialaccount,taggit,tastypie,upload,user_messages,geonode_themes,geoapps,favorite,geonode_client -dumps = contenttypes,auth,people,groups,account,guardian,admin,actstream,announcements,avatar,base,documents,geoserver,invitations,pinax_notifications,harvesting,services,layers,maps,oauth2_provider,sites,socialaccount,taggit,tastypie,upload,user_messages,geonode_themes,geoapps,favorite,geonode_client +apps = contenttypes,auth,people,groups,account,guardian,admin,actstream,announcements,avatar,assets,base,documents,geoserver,invitations,pinax_notifications,harvesting,services,layers,maps,oauth2_provider,sites,socialaccount,taggit,tastypie,upload,user_messages,geonode_themes,geoapps,favorite,geonode_client +dumps = contenttypes,auth,people,groups,account,guardian,admin,actstream,announcements,avatar,assets,base,documents,geoserver,invitations,pinax_notifications,harvesting,services,layers,maps,oauth2_provider,sites,socialaccount,taggit,tastypie,upload,user_messages,geonode_themes,geoapps,favorite,geonode_client diff --git a/geonode/br/management/commands/settings_sample.ini b/geonode/br/management/commands/settings_sample.ini index 157414ab5e0..112b6618474 100644 --- a/geonode/br/management/commands/settings_sample.ini +++ b/geonode/br/management/commands/settings_sample.ini @@ -13,5 +13,5 @@ dumprasterdata = yes # data_layername_exclude_filter = {comma separated list of layernames, optionally with glob syntax} e.g.: tuscany_*,italy [fixtures] -apps = contenttypes,auth,people,groups,account,guardian,admin,actstream,announcements,avatar,base,documents,geoserver,invitations,pinax_notifications,harvesting,services,layers,maps,oauth2_provider,sites,socialaccount,taggit,tastypie,upload,user_messages,geonode_themes,geoapps,favorite,geonode_client -dumps = contenttypes,auth,people,groups,account,guardian,admin,actstream,announcements,avatar,base,documents,geoserver,invitations,pinax_notifications,harvesting,services,layers,maps,oauth2_provider,sites,socialaccount,taggit,tastypie,upload,user_messages,geonode_themes,geoapps,favorite,geonode_client +apps = contenttypes,auth,people,groups,account,guardian,admin,actstream,announcements,avatar,assets,base,documents,geoserver,invitations,pinax_notifications,harvesting,services,layers,maps,oauth2_provider,sites,socialaccount,taggit,tastypie,upload,user_messages,geonode_themes,geoapps,favorite,geonode_client +dumps = contenttypes,auth,people,groups,account,guardian,admin,actstream,announcements,avatar,assets,base,documents,geoserver,invitations,pinax_notifications,harvesting,services,layers,maps,oauth2_provider,sites,socialaccount,taggit,tastypie,upload,user_messages,geonode_themes,geoapps,favorite,geonode_client diff --git a/geonode/br/management/commands/utils/utils.py b/geonode/br/management/commands/utils/utils.py index cb313193554..4b4839973ba 100644 --- a/geonode/br/management/commands/utils/utils.py +++ b/geonode/br/management/commands/utils/utils.py @@ -42,6 +42,7 @@ TEMPLATE_DIRS = "template_dirs" LOCALE_PATHS = "locale_dirs" EXTERNAL_ROOT = "external" +ASSETS_ROOT = "assets" logger = logging.getLogger(__name__) diff --git a/geonode/documents/models.py b/geonode/documents/models.py index 626cccb5933..565eb32122f 100644 --- a/geonode/documents/models.py +++ b/geonode/documents/models.py @@ -26,7 +26,7 @@ from django.utils.functional import classproperty from django.utils.translation import gettext_lazy as _ -from geonode.assets.models import Asset +from geonode.assets.models import LocalAsset from geonode.client.hooks import hookset from geonode.base.models import ResourceBase from geonode.groups.conf import settings as groups_settings @@ -79,7 +79,7 @@ def compact_permission_labels(cls): @property def files(self): - asset = Asset.objects.filter(link__resource=self).first() + asset = LocalAsset.objects.filter(link__resource=self).first() return asset.location if asset else [] @property