From 50b86a4e36b097212b4a7df2fd6668616b671201 Mon Sep 17 00:00:00 2001 From: mattiagiupponi Date: Fri, 28 May 2021 18:30:42 +0200 Subject: [PATCH 1/4] Uploads now rely on ResourceBase.Files and upload the files via storage_manager --- .env_dev | 2 + .../migrations/0062_resourcebase_files.py | 18 ++++++++ geonode/base/models.py | 22 ++++++++++ geonode/upload/admin.py | 6 +-- .../migrations/0032_auto_20210528_1556.py | 24 ++++++++++ geonode/upload/models.py | 44 +++++++------------ geonode/upload/tests/test_upload_manager.py | 41 +++++++++++++++++ geonode/upload/upload.py | 3 +- 8 files changed, 128 insertions(+), 32 deletions(-) create mode 100644 geonode/base/migrations/0062_resourcebase_files.py create mode 100644 geonode/upload/migrations/0032_auto_20210528_1556.py create mode 100644 geonode/upload/tests/test_upload_manager.py diff --git a/.env_dev b/.env_dev index 6261e876d73..76ff3d820c2 100644 --- a/.env_dev +++ b/.env_dev @@ -166,3 +166,5 @@ OAUTH2_CLIENT_SECRET=rCnp5txobUo83EpQEblM8fVj3QT5zb5qRfxNsuPzCqZaiRyIoxM4jdgMiZK # GeoNode APIs API_LOCKDOWN=False TASTYPIE_APIKEY= + +STORAGE_MANAGER_CONCRETE_CLASS = geonode.storage.dropbox.DropboxStorageManager diff --git a/geonode/base/migrations/0062_resourcebase_files.py b/geonode/base/migrations/0062_resourcebase_files.py new file mode 100644 index 00000000000..07db217d3a3 --- /dev/null +++ b/geonode/base/migrations/0062_resourcebase_files.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2 on 2021-05-28 15:35 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('base', '0061_merge_0060_auto_20210512_1658_0060_resourcebase_state'), + ] + + operations = [ + migrations.AddField( + model_name='resourcebase', + name='files', + field=models.JSONField(default=dict), + ), + ] diff --git a/geonode/base/models.py b/geonode/base/models.py index 17a78aa6818..56836d6aab5 100644 --- a/geonode/base/models.py +++ b/geonode/base/models.py @@ -29,6 +29,7 @@ from django.db import models from django.conf import settings from django.core import serializers +from django.db.models.fields.json import JSONField from django.utils.functional import cached_property from django.utils.html import escape from django.utils.timezone import now @@ -85,6 +86,7 @@ from urllib.parse import urlparse, urlsplit, urljoin from imagekit.cachefiles.backends import Simple +from geonode.storage.manager import storage_manager logger = logging.getLogger(__name__) @@ -590,6 +592,24 @@ def get_queryset(self): def polymorphic_queryset(self): return super(ResourceBaseManager, self).get_queryset() + @staticmethod + def upload_files(resource, files): + try: + out = {} + for f in files: + _, ext = os.path.splitext(f) + if os.path.isfile(f) and os.path.exists(f): + + with open(f, 'rb') as ff: + folder = os.path.basename(os.path.dirname(f)) + filename = os.path.basename(f) + file_uploaded_path = storage_manager.save(f'{folder}/{filename}', ff) + out[ext] = file_uploaded_path + resource.files = out + resource.save() + except Exception as e: + logger.exception(e) + class ResourceBase(PolymorphicModel, PermissionLevelMixin, ItemBase): """ @@ -914,6 +934,8 @@ class ResourceBase(PolymorphicModel, PermissionLevelMixin, ItemBase): _("Metadata"), default=False, help_text=_('If true, will be excluded from search')) + + files = JSONField(null=False, default=dict) __is_approved = False __is_published = False diff --git a/geonode/upload/admin.py b/geonode/upload/admin.py index 78ddc41938e..05884c4fdce 100644 --- a/geonode/upload/admin.py +++ b/geonode/upload/admin.py @@ -32,11 +32,11 @@ def import_link(obj): class UploadAdmin(admin.ModelAdmin): - list_display = ('id', 'import_id', 'name', 'layer', 'user', 'date', 'state', import_link) + list_display = ('id', 'import_id', 'name', 'resource', 'user', 'date', 'state', import_link) list_display_links = ('id',) date_hierarchy = 'date' - list_filter = ('name', 'layer', 'user', 'date', 'state') - search_fields = ('name', 'layer', 'user', 'date', 'state') + list_filter = ('name', 'resource', 'user', 'date', 'state') + search_fields = ('name', 'resource', 'user', 'date', 'state') def delete_queryset(self, request, queryset): for obj in queryset: diff --git a/geonode/upload/migrations/0032_auto_20210528_1556.py b/geonode/upload/migrations/0032_auto_20210528_1556.py new file mode 100644 index 00000000000..5e3a165c3e5 --- /dev/null +++ b/geonode/upload/migrations/0032_auto_20210528_1556.py @@ -0,0 +1,24 @@ +# Generated by Django 3.2 on 2021-05-28 15:56 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('base', '0062_resourcebase_files'), + ('upload', '0031_auto_20210527_1520'), + ] + + operations = [ + migrations.RemoveField( + model_name='upload', + name='layer', + ), + migrations.AddField( + model_name='upload', + name='resource', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='base.resourcebase'), + ), + ] diff --git a/geonode/upload/models.py b/geonode/upload/models.py index 260b76ef5ba..2cf3c3d6137 100644 --- a/geonode/upload/models.py +++ b/geonode/upload/models.py @@ -17,6 +17,7 @@ # along with this program. If not, see . # ######################################################################### +from geonode.base.models import ResourceBase from geonode.storage.manager import storage_manager import os import json @@ -53,12 +54,12 @@ def invalidate_from_session(self, upload_session): import_id=upload_session.import_session.id ).update(state=enumerations.STATE_INVALID) - def update_from_session(self, upload_session, layer=None): + def update_from_session(self, upload_session, resource=None): self.get( user=upload_session.user, name=upload_session.name, import_id=upload_session.import_session.id).update_from_session( - upload_session, layer=layer) + upload_session, resource=resource) def create_from_session(self, user, import_session): return self.create( @@ -81,7 +82,7 @@ class Upload(models.Model): state = models.CharField(max_length=16) create_date = models.DateTimeField('create_date', default=now) date = models.DateTimeField('date', default=now) - layer = models.ForeignKey(Layer, null=True, on_delete=models.CASCADE) + resource = models.ForeignKey(ResourceBase, null=True, on_delete=models.CASCADE) upload_dir = models.TextField(null=True) name = models.CharField(max_length=64, null=True) complete = models.BooleanField(default=False) @@ -109,7 +110,7 @@ def get_session(self): return pickle.loads( base64.decodebytes(self.session.encode('UTF-8'))) - def update_from_session(self, upload_session, layer=None): + def update_from_session(self, upload_session, resource=None): self.session = base64.encodebytes(pickle.dumps(upload_session)).decode('UTF-8') self.state = upload_session.import_session.state self.name = upload_session.name @@ -119,30 +120,19 @@ def update_from_session(self, upload_session, layer=None): if not self.upload_dir: self.upload_dir = os.path.dirname(upload_session.base_file) - if layer and not self.layer: - self.layer = layer + if resource and not self.resource: + self.resource = resource - if upload_session.base_file and self.layer and self.layer.name: + if upload_session.base_file and self.resource and self.resource.title: uploaded_files = upload_session.base_file[0] - base_file = uploaded_files.base_file aux_files = uploaded_files.auxillary_files sld_files = uploaded_files.sld_files xml_files = uploaded_files.xml_files - file_name, _ = os.path.splitext(os.path.basename(base_file)) + if self.resource and not self.resource.files: + files_to_upload = aux_files + sld_files + xml_files + [uploaded_files.base_file] + ResourceBase.objects.upload_files(resource=resource, files=files_to_upload) - if not UploadFile.objects.filter(upload=self, name=file_name).exists(): - assigned_name = self.layer.name - uploaded_file = UploadFile.objects.create_from_upload(upload=self, name=assigned_name, base_file=base_file) - - if uploaded_file and uploaded_file.name: - additional_files = aux_files + sld_files + xml_files - for _f in additional_files: - UploadFile.objects.create_from_upload( - upload=self, - name=assigned_name, - base_file=_f - ) # TODO: add delte temporary file on filesystem if "COMPLETE" == self.state: @@ -162,7 +152,7 @@ def progress(self): elif self.state == enumerations.STATE_PROCESSED: return 100.0 elif self.complete or self.state in (enumerations.STATE_COMPLETE, enumerations.STATE_RUNNING): - if self.layer and self.layer.processed: + if self.resource and self.resource.processed: self.set_processing_state(enumerations.STATE_PROCESSED) return 100.0 elif self.state == enumerations.STATE_RUNNING: @@ -198,7 +188,7 @@ def get_resume_url(self): return f"{reverse('data_upload')}?id={self.import_id}" else: next = get_next_step(self.get_session) - if not self.layer and session.state == enumerations.STATE_COMPLETE: + if not self.resource and session.state == enumerations.STATE_COMPLETE: if next == 'check' or (next == 'final' and self.state == enumerations.STATE_PENDING): from .views import final_step_view final_step_view(None, self.get_session) @@ -231,8 +221,8 @@ def get_import_url(self): return None def get_detail_url(self): - if self.layer and self.state == enumerations.STATE_PROCESSED: - return getattr(self.layer, 'detail_url', None) + if self.resource and self.state == enumerations.STATE_PROCESSED: + return getattr(self.resource, 'detail_url', None) else: return None @@ -273,8 +263,8 @@ def delete(self, *args, **kwargs): def set_processing_state(self, state): self.state = True Upload.objects.filter(id=self.id).update(state=state) - if self.layer: - self.layer.set_processing_state(state) + if self.resource: + self.resource.set_processing_state(state) def __str__(self): return f'Upload [{self.pk}] gs{self.import_id} - {self.name}, {self.user}' diff --git a/geonode/upload/tests/test_upload_manager.py b/geonode/upload/tests/test_upload_manager.py new file mode 100644 index 00000000000..b597bcd609d --- /dev/null +++ b/geonode/upload/tests/test_upload_manager.py @@ -0,0 +1,41 @@ +import os +from django.test import TestCase +from gisdata import GOOD_DATA +from geonode.upload.models import Upload, UploadFile +from geonode.base.populate_test_data import create_single_layer +from unittest.mock import patch + + +class TestUploadFileManager(TestCase): + def setUp(self): + self.sut = UploadFile + self.file = os.path.join(GOOD_DATA, 'raster', 'relief_san_andres.tif') + self.layer = create_single_layer('foo_layer') + self.upload = Upload.objects.create( + state='RUNNING', + layer=self.layer + ) + + def test_will_return_none_if_file_does_not_exists(self): + actual = self.sut.objects.create_from_upload( + self.upload, + "foo_name", + "not_exising_file" + ) + self.assertIsNone(actual) + # check also that UploadFile does not exist + uf = UploadFile.objects.filter(upload=self.upload).exists() + self.assertFalse(uf) + + @patch('geonode.storage.manager.storage_manager.save') + def test_will_create_the_upload_file_row(self, save): + save.return_value = self.file + actual = self.sut.objects.create_from_upload( + self.upload, + "foo_name", + self.file + ) + self.assertIsNotNone(actual) + # check also that UploadFile does not exist + uf = UploadFile.objects.filter(upload=self.upload).exists() + self.assertTrue(uf) \ No newline at end of file diff --git a/geonode/upload/upload.py b/geonode/upload/upload.py index 6d356cefc79..f98a14c5135 100644 --- a/geonode/upload/upload.py +++ b/geonode/upload/upload.py @@ -753,8 +753,7 @@ def final_step(upload_session, user, charset="UTF-8", layer_id=None): raise UploadException.from_exc(_('Error configuring Layer'), e) assert saved_layer - - Upload.objects.update_from_session(upload_session, layer=saved_layer) + Upload.objects.update_from_session(upload_session, resource=saved_layer) # Set default permissions on the newly created layer and send notifications permissions = upload_session.permissions From 1282d9ec1ee6447d3f48b3858f00d7ee8dd89b90 Mon Sep 17 00:00:00 2001 From: mattiagiupponi Date: Mon, 31 May 2021 11:58:54 +0200 Subject: [PATCH 2/4] Get rid of UploadFile, let the file being stored by storage_manager and move field to ResourceBase --- geonode/base/models.py | 12 ++-- geonode/layers/models.py | 2 +- geonode/layers/utils.py | 4 +- geonode/layers/views.py | 2 +- geonode/proxy/templatetags/proxy_lib_tags.py | 14 ++--- geonode/proxy/views.py | 12 ++-- geonode/upload/admin.py | 10 +-- geonode/upload/api/serializers.py | 38 +++++++----- geonode/upload/api/views.py | 1 + geonode/upload/forms.py | 8 --- geonode/upload/models.py | 64 +++++--------------- geonode/upload/tests/test_upload_manager.py | 41 ------------- geonode/upload/urls.py | 5 -- geonode/upload/views.py | 57 +---------------- 14 files changed, 65 insertions(+), 205 deletions(-) delete mode 100644 geonode/upload/tests/test_upload_manager.py diff --git a/geonode/base/models.py b/geonode/base/models.py index 56836d6aab5..c037f05ebbe 100644 --- a/geonode/base/models.py +++ b/geonode/base/models.py @@ -593,7 +593,7 @@ def polymorphic_queryset(self): return super(ResourceBaseManager, self).get_queryset() @staticmethod - def upload_files(resource, files): + def upload_files(resource_id, files): try: out = {} for f in files: @@ -604,9 +604,11 @@ def upload_files(resource, files): folder = os.path.basename(os.path.dirname(f)) filename = os.path.basename(f) file_uploaded_path = storage_manager.save(f'{folder}/{filename}', ff) - out[ext] = file_uploaded_path - resource.files = out - resource.save() + out[ext] = storage_manager.path(file_uploaded_path) + + # making an update instead of save in order to avoid others + # signal like post_save and commiunication with geoserver + ResourceBase.objects.filter(id=resource_id).update(files=out) except Exception as e: logger.exception(e) @@ -934,7 +936,7 @@ class ResourceBase(PolymorphicModel, PermissionLevelMixin, ItemBase): _("Metadata"), default=False, help_text=_('If true, will be excluded from search')) - + files = JSONField(null=False, default=dict) __is_approved = False diff --git a/geonode/layers/models.py b/geonode/layers/models.py index 01a21160afb..d69cb03085b 100644 --- a/geonode/layers/models.py +++ b/geonode/layers/models.py @@ -647,7 +647,7 @@ def pre_delete_layer(instance, sender, **kwargs): from geonode.upload.models import Upload # Need to call delete one by one in ordee to invoke the # 'delete' overridden method - for upload in Upload.objects.filter(layer_id=instance.id): + for upload in Upload.objects.filter(resource_id=instance.id): upload.delete() # Delete object permissions diff --git a/geonode/layers/utils.py b/geonode/layers/utils.py index f39c785a42e..926dbce8a0e 100644 --- a/geonode/layers/utils.py +++ b/geonode/layers/utils.py @@ -624,8 +624,8 @@ def gs_handle_layer(layer, base_files, user, action_type="append"): # opening upload session for the selected layer from geonode.upload.models import Upload - upload_session, _ = Upload.objects.get_or_create(layer=layer, user=user) - upload_session.resource = layer + upload_session, _ = Upload.objects.get_or_create(resource=layer.resourcebase_ptr, user=user) + upload_session.resource = layer.resourcebase_ptr upload_session.processed = False upload_session.save() diff --git a/geonode/layers/views.py b/geonode/layers/views.py index 41d648c531d..39bb103441f 100644 --- a/geonode/layers/views.py +++ b/geonode/layers/views.py @@ -1079,7 +1079,7 @@ def layer_metadata( from geonode.upload.models import Upload - up_sessions = Upload.objects.filter(layer=layer) + up_sessions = Upload.objects.filter(resource_id=layer.resourcebase_ptr_id) if up_sessions.count() > 0 and up_sessions[0].user != layer.owner: up_sessions.update(user=layer.owner) diff --git a/geonode/proxy/templatetags/proxy_lib_tags.py b/geonode/proxy/templatetags/proxy_lib_tags.py index b7016caa435..282a7160836 100644 --- a/geonode/proxy/templatetags/proxy_lib_tags.py +++ b/geonode/proxy/templatetags/proxy_lib_tags.py @@ -18,6 +18,7 @@ # ######################################################################### +from geonode.base.models import ResourceBase import traceback from django import template @@ -28,8 +29,6 @@ from urllib.parse import urlsplit, urljoin -from geonode.layers.models import Layer -from geonode.upload.models import Upload from geonode.utils import resolve_object register = template.Library() @@ -41,7 +40,7 @@ def original_link_available(context, resourceid, url): request = context['request'] instance = resolve_object( request, - Layer, + ResourceBase, {'pk': resourceid}, permission='base.download_resourcebase', permission_msg=_not_permitted) @@ -52,12 +51,11 @@ def original_link_available(context, resourceid, url): return True layer_files = [] - if isinstance(instance, Layer): + if isinstance(instance, ResourceBase): try: - upload_session = Upload.objects.get(layer=instance) - for lyr in upload_session.uploadfile_set.all(): - layer_files.append(lyr) - if not storage_manager.exists(lyr.file): + for ext, file in instance.files.items(): + layer_files.append(file) + if not storage_manager.exists(file): return False except Exception: traceback.print_exc() diff --git a/geonode/proxy/views.py b/geonode/proxy/views.py index 0f1b0c438c2..7699c8e2d18 100644 --- a/geonode/proxy/views.py +++ b/geonode/proxy/views.py @@ -276,14 +276,14 @@ def download(request, resourceid, sender=Layer): layer_files = [] try: - upload_session = Upload.objects.get(layer=instance) + files = instance.resourcebase_ptr.files # Copy all Layer related files into a temporary folder - for lyr in upload_session.uploadfile_set.all(): - if storage_manager.exists(lyr.file): - layer_files.append(lyr) - filename = os.path.basename(lyr.file) + for ext, file_path in files.items(): + if storage_manager.exists(file_path): + layer_files.append(file_path) + filename = os.path.basename(file_path) with open(f"{target_folder}/{filename}", 'wb+') as f: - f.write(storage_manager.open(lyr.file).read()) + f.write(storage_manager.open(file_path).read()) else: return HttpResponse( loader.render_to_string( diff --git a/geonode/upload/admin.py b/geonode/upload/admin.py index 05884c4fdce..64ca8c2cd40 100644 --- a/geonode/upload/admin.py +++ b/geonode/upload/admin.py @@ -18,7 +18,7 @@ # ######################################################################### -from geonode.upload.models import Upload, UploadFile +from geonode.upload.models import Upload from django.contrib import admin @@ -43,12 +43,4 @@ def delete_queryset(self, request, queryset): obj.delete() -class UploadFileAdmin(admin.ModelAdmin): - list_display = ('id', 'upload', 'slug') - list_display_links = ('id',) - list_filter = ('slug', ) - search_fields = ('slug', ) - - admin.site.register(Upload, UploadAdmin) -admin.site.register(UploadFile, UploadFileAdmin) diff --git a/geonode/upload/api/serializers.py b/geonode/upload/api/serializers.py index 0b137ef620d..f3ab76a951b 100644 --- a/geonode/upload/api/serializers.py +++ b/geonode/upload/api/serializers.py @@ -17,13 +17,14 @@ # along with this program. If not, see . # ######################################################################### +import os +from geonode.base.models import ResourceBase from rest_framework import serializers -from dynamic_rest.serializers import DynamicModelSerializer from dynamic_rest.fields.fields import DynamicRelationField, DynamicComputedField from geonode.base.utils import build_absolute_uri -from geonode.upload.models import Upload, UploadFile +from geonode.upload.models import Upload from geonode.layers.api.serializers import LayerSerializer from geonode.base.api.serializers import BaseDynamicModelSerializer @@ -32,17 +33,26 @@ logger = logging.getLogger(__name__) -class UploadFileSerializer(DynamicModelSerializer): - +class UploadFileField(serializers.RelatedField): class Meta: - model = UploadFile - name = 'upload_file' - fields = ( - 'pk', 'name', 'slug', 'file' - ) - - name = serializers.CharField(read_only=True) - slug = serializers.CharField(read_only=True) + model = ResourceBase + name = 'resource-files' + + def to_representation(self, obj): + files = [] + for _, file in obj.files.items(): + name, _ = os.path.splitext(os.path.basename(file)) + files.append( + { + "name": name, + "slug": os.path.basename(file), + "file": file + } + ) + return { + 'name': obj.title, + 'files': files, + } class SessionSerializer(serializers.Field): @@ -213,7 +223,7 @@ class Meta: fields = ( 'id', 'name', 'date', 'create_date', 'user', 'state', 'progress', 'complete', 'import_id', - 'uploadfile_set', 'resume_url', 'delete_url', 'import_url', 'detail_url' + 'resume_url', 'delete_url', 'import_url', 'detail_url', "uploadfile_set" ) progress = ProgressField(read_only=True) @@ -221,4 +231,4 @@ class Meta: delete_url = ProgressUrlField('delete', read_only=True) import_url = ProgressUrlField('import', read_only=True) detail_url = ProgressUrlField('detail', read_only=True) - uploadfile_set = DynamicRelationField(UploadFileSerializer, embed=True, many=True, read_only=True) + uploadfile_set = UploadFileField(source='resource', read_only=True) diff --git a/geonode/upload/api/views.py b/geonode/upload/api/views.py index 5f6a33cbf7c..d80ded7377d 100644 --- a/geonode/upload/api/views.py +++ b/geonode/upload/api/views.py @@ -64,6 +64,7 @@ class UploadViewSet(DynamicModelViewSet): queryset = Upload.objects.all() serializer_class = UploadSerializer pagination_class = GeoNodeApiPagination + lookup_field = "resource_id" @extend_schema(methods=['put'], responses={201: None}, diff --git a/geonode/upload/forms.py b/geonode/upload/forms.py index cdcefb49b27..280e445012e 100644 --- a/geonode/upload/forms.py +++ b/geonode/upload/forms.py @@ -27,19 +27,11 @@ from ..utils import check_ogc_backend from ..layers.forms import JSONField -from .models import UploadFile from .upload_validators import validate_uploaded_files logger = logging.getLogger(__name__) -class UploadFileForm(forms.ModelForm): - - class Meta: - model = UploadFile - fields = '__all__' - - class LayerUploadForm(forms.Form): base_file = forms.FileField() dbf_file = forms.FileField(required=False) diff --git a/geonode/upload/models.py b/geonode/upload/models.py index 2cf3c3d6137..5c67c1d5e85 100644 --- a/geonode/upload/models.py +++ b/geonode/upload/models.py @@ -34,7 +34,6 @@ from django.utils.timezone import now from geonode.base import enumerations -from geonode.layers.models import Layer from geonode.tasks.tasks import AcquireLock from geonode.geoserver.helpers import gs_uploader, ogc_server_settings @@ -131,7 +130,7 @@ def update_from_session(self, upload_session, resource=None): if self.resource and not self.resource.files: files_to_upload = aux_files + sld_files + xml_files + [uploaded_files.base_file] - ResourceBase.objects.upload_files(resource=resource, files=files_to_upload) + ResourceBase.objects.upload_files(resource_id=resource.id, files=files_to_upload) # TODO: add delte temporary file on filesystem @@ -228,7 +227,6 @@ def get_detail_url(self): def delete(self, *args, **kwargs): importer_locations = [] - upload_files = [_file.file for _file in UploadFile.objects.filter(upload=self)] super(Upload, self).delete(*args, **kwargs) try: session = gs_uploader.get_session(self.import_id) @@ -243,17 +241,27 @@ def delete(self, *args, **kwargs): session.delete() except Exception: logging.warning('error deleting upload session') - for _file in upload_files: + + # we delete directly the folder with the files of the resource + for _, _file in self.resource.files.items(): try: - if os.path.isfile(_file.path): - os.remove(_file.path) + dirname = os.path.dirname(_file) + if storage_manager.exists(dirname): + storage_manager.delete(dirname) + break except Exception as e: logger.warning(e) + + # Do we want to delete the files also from the resource? + ResourceBase.objects.filter(id=self.resource.id).update(files={}) + for _location in importer_locations: try: shutil.rmtree(_location) except Exception as e: logger.warning(e) + + # here we are deleting the local that soon will be removed if self.upload_dir and os.path.exists(self.upload_dir): try: shutil.rmtree(self.upload_dir) @@ -268,47 +276,3 @@ def set_processing_state(self, state): def __str__(self): return f'Upload [{self.pk}] gs{self.import_id} - {self.name}, {self.user}' - - -class UploadFileManager(models.Manager): - - def __init__(self): - models.Manager.__init__(self) - - def create_from_upload(self, - upload, - name, - base_file): - try: - if os.path.isfile(base_file) and os.path.exists(base_file): - slug = os.path.basename(base_file) - - with open(base_file, 'rb') as ff: - filepath = f"{os.path.basename(os.path.dirname(base_file))}" - file_uploaded_path = storage_manager.save(f'{filepath}/{slug}', ff) - - return self.create( - upload=upload, - file=file_uploaded_path, - name=name, - slug=slug - ) - - except Exception as e: - logger.exception(e) - - -class UploadFile(models.Model): - - objects = UploadFileManager() - - upload = models.ForeignKey(Upload, null=True, blank=True, on_delete=models.CASCADE) - name = models.CharField(max_length=4096, blank=True) - slug = models.SlugField(max_length=4096, blank=True) - file = models.CharField(max_length=4096, blank=True) - - def __str__(self): - return str(self.slug) - - def get_absolute_url(self): - return reverse('data_upload_new', args=[self.slug, ]) diff --git a/geonode/upload/tests/test_upload_manager.py b/geonode/upload/tests/test_upload_manager.py deleted file mode 100644 index b597bcd609d..00000000000 --- a/geonode/upload/tests/test_upload_manager.py +++ /dev/null @@ -1,41 +0,0 @@ -import os -from django.test import TestCase -from gisdata import GOOD_DATA -from geonode.upload.models import Upload, UploadFile -from geonode.base.populate_test_data import create_single_layer -from unittest.mock import patch - - -class TestUploadFileManager(TestCase): - def setUp(self): - self.sut = UploadFile - self.file = os.path.join(GOOD_DATA, 'raster', 'relief_san_andres.tif') - self.layer = create_single_layer('foo_layer') - self.upload = Upload.objects.create( - state='RUNNING', - layer=self.layer - ) - - def test_will_return_none_if_file_does_not_exists(self): - actual = self.sut.objects.create_from_upload( - self.upload, - "foo_name", - "not_exising_file" - ) - self.assertIsNone(actual) - # check also that UploadFile does not exist - uf = UploadFile.objects.filter(upload=self.upload).exists() - self.assertFalse(uf) - - @patch('geonode.storage.manager.storage_manager.save') - def test_will_create_the_upload_file_row(self, save): - save.return_value = self.file - actual = self.sut.objects.create_from_upload( - self.upload, - "foo_name", - self.file - ) - self.assertIsNotNone(actual) - # check also that UploadFile does not exist - uf = UploadFile.objects.filter(upload=self.upload).exists() - self.assertTrue(uf) \ No newline at end of file diff --git a/geonode/upload/urls.py b/geonode/upload/urls.py index c1f00bf18f3..3d22b292fb9 100644 --- a/geonode/upload/urls.py +++ b/geonode/upload/urls.py @@ -18,19 +18,14 @@ # ######################################################################### from django.conf.urls import url, include -from geonode.upload.views import UploadFileCreateView, UploadFileDeleteView from . import views urlpatterns = [ # 'geonode.upload.views', - url(r'^new/$', UploadFileCreateView.as_view(), - name='data_upload_new'), url(r'^progress$', views.data_upload_progress, name='data_upload_progress'), url(r'^(?P\w+)?$', views.view, name='data_upload'), url(r'^delete/(?P\d+)?$', views.delete, name='data_upload_delete'), - url(r'^remove/(?P\d+)$', - UploadFileDeleteView.as_view(), name='data_upload_remove'), url(r'^', include('geonode.upload.api.urls')), ] diff --git a/geonode/upload/views.py b/geonode/upload/views.py index 33e7f55f699..c8dbb5b44bd 100644 --- a/geonode/upload/views.py +++ b/geonode/upload/views.py @@ -45,15 +45,12 @@ from http.client import BadStatusLine from django.contrib import auth -from django.urls import reverse from django.conf import settings from django.shortcuts import render from django.utils.html import escape -from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.core.exceptions import PermissionDenied from django.utils.translation import ugettext_lazy as _ -from django.views.generic import CreateView, DeleteView from django.contrib.auth.decorators import login_required from geonode.upload import UploadException @@ -68,12 +65,10 @@ from .forms import ( LayerUploadForm, SRSForm, - TimeForm, - UploadFileForm, + TimeForm ) from .models import ( - Upload, - UploadFile) + Upload) from .files import ( get_scan_hint, scan_file) @@ -88,7 +83,6 @@ is_async_step, is_latitude, is_longitude, - JSONResponse, json_response, get_previous_step, layer_eligible_for_time_dimension, @@ -794,55 +788,8 @@ def delete(req, id): )) -class UploadFileCreateView(CreateView): - form_class = UploadFileForm - model = UploadFile - - def form_valid(self, form): - self.object = form.save() - f = self.request.FILES.get('file') - data = [ - { - 'name': f.name, - 'url': f"{settings.MEDIA_URL}uploads/{f.name.replace(' ', '_')}", - 'thumbnail_url': f"{settings.MEDIA_URL}pictures/{f.name.replace(' ', '_')}", - 'delete_url': reverse( - 'data_upload_remove', - args=[ - self.object.id]), - 'delete_type': "DELETE"}] - response = JSONResponse(data, {}, response_content_type(self.request)) - response['Content-Disposition'] = 'inline; filename=files.json' - return response - - def form_invalid(self, form): - data = [{}] - response = JSONResponse(data, {}, response_content_type(self.request)) - response['Content-Disposition'] = 'inline; filename=files.json' - return response - - def response_content_type(request): if "application/json" in request.META['HTTP_ACCEPT']: return "application/json" else: return "text/plain" - - -class UploadFileDeleteView(DeleteView): - model = UploadFile - - def delete(self, request, *args, **kwargs): - """ - This does not actually delete the file, only the database record. But - that is easy to implement. - """ - self.object = self.get_object() - self.object.delete() - if request.is_ajax(): - response = JSONResponse( - True, {}, response_content_type(self.request)) - response['Content-Disposition'] = 'inline; filename=files.json' - return response - else: - return HttpResponseRedirect(reverse('data_upload_new')) From 594c3204e388b3857e96bb2ab5855e7f1a91e0ad Mon Sep 17 00:00:00 2001 From: mattiagiupponi Date: Mon, 31 May 2021 14:53:29 +0200 Subject: [PATCH 3/4] Remove UploadFile model, increase strongness of Upload model --- .env_dev | 2 - geonode/base/models.py | 1 + .../migrations/0033_auto_20210531_1252.py | 23 ++++++++++ geonode/upload/models.py | 42 ++++++++++++------- 4 files changed, 50 insertions(+), 18 deletions(-) create mode 100644 geonode/upload/migrations/0033_auto_20210531_1252.py diff --git a/.env_dev b/.env_dev index 76ff3d820c2..6261e876d73 100644 --- a/.env_dev +++ b/.env_dev @@ -166,5 +166,3 @@ OAUTH2_CLIENT_SECRET=rCnp5txobUo83EpQEblM8fVj3QT5zb5qRfxNsuPzCqZaiRyIoxM4jdgMiZK # GeoNode APIs API_LOCKDOWN=False TASTYPIE_APIKEY= - -STORAGE_MANAGER_CONCRETE_CLASS = geonode.storage.dropbox.DropboxStorageManager diff --git a/geonode/base/models.py b/geonode/base/models.py index c037f05ebbe..abfb8a2316d 100644 --- a/geonode/base/models.py +++ b/geonode/base/models.py @@ -609,6 +609,7 @@ def upload_files(resource_id, files): # making an update instead of save in order to avoid others # signal like post_save and commiunication with geoserver ResourceBase.objects.filter(id=resource_id).update(files=out) + return out except Exception as e: logger.exception(e) diff --git a/geonode/upload/migrations/0033_auto_20210531_1252.py b/geonode/upload/migrations/0033_auto_20210531_1252.py new file mode 100644 index 00000000000..92e2a8a821b --- /dev/null +++ b/geonode/upload/migrations/0033_auto_20210531_1252.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2 on 2021-05-31 12:52 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('base', '0062_resourcebase_files'), + ('upload', '0032_auto_20210528_1556'), + ] + + operations = [ + migrations.AlterField( + model_name='upload', + name='resource', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='base.resourcebase'), + ), + migrations.DeleteModel( + name='UploadFile', + ), + ] diff --git a/geonode/upload/models.py b/geonode/upload/models.py index 5c67c1d5e85..d15abb22992 100644 --- a/geonode/upload/models.py +++ b/geonode/upload/models.py @@ -32,7 +32,7 @@ from django.urls import reverse from django.conf import settings from django.utils.timezone import now - +from geonode import GeoNodeException from geonode.base import enumerations from geonode.tasks.tasks import AcquireLock from geonode.geoserver.helpers import gs_uploader, ogc_server_settings @@ -81,7 +81,7 @@ class Upload(models.Model): state = models.CharField(max_length=16) create_date = models.DateTimeField('create_date', default=now) date = models.DateTimeField('date', default=now) - resource = models.ForeignKey(ResourceBase, null=True, on_delete=models.CASCADE) + resource = models.ForeignKey(ResourceBase, null=True, on_delete=models.SET_NULL) upload_dir = models.TextField(null=True) name = models.CharField(max_length=64, null=True) complete = models.BooleanField(default=False) @@ -109,7 +109,7 @@ def get_session(self): return pickle.loads( base64.decodebytes(self.session.encode('UTF-8'))) - def update_from_session(self, upload_session, resource=None): + def update_from_session(self, upload_session, resource: ResourceBase = None): self.session = base64.encodebytes(pickle.dumps(upload_session)).decode('UTF-8') self.state = upload_session.import_session.state self.name = upload_session.name @@ -120,7 +120,12 @@ def update_from_session(self, upload_session, resource=None): self.upload_dir = os.path.dirname(upload_session.base_file) if resource and not self.resource: - self.resource = resource + if not isinstance(resource, ResourceBase) and hasattr(resource, 'resourcebase_ptr'): + self.resource = resource.resourcebase_ptr + elif not isinstance(resource, ResourceBase): + raise GeoNodeException("Invalid resource uploaded, plase select one of the available") + else: + self.resource = resource if upload_session.base_file and self.resource and self.resource.title: uploaded_files = upload_session.base_file[0] @@ -132,7 +137,11 @@ def update_from_session(self, upload_session, resource=None): files_to_upload = aux_files + sld_files + xml_files + [uploaded_files.base_file] ResourceBase.objects.upload_files(resource_id=resource.id, files=files_to_upload) - # TODO: add delte temporary file on filesystem + # Now we delete the files from local file system + # only if it does not match with the default temporary path + if os.path.exists(self.upload_dir): + if settings.STATIC_ROOT != os.path.dirname(os.path.abspath(self.upload_dir)): + shutil.rmtree(self.upload_dir) if "COMPLETE" == self.state: self.complete = True @@ -243,17 +252,18 @@ def delete(self, *args, **kwargs): logging.warning('error deleting upload session') # we delete directly the folder with the files of the resource - for _, _file in self.resource.files.items(): - try: - dirname = os.path.dirname(_file) - if storage_manager.exists(dirname): - storage_manager.delete(dirname) - break - except Exception as e: - logger.warning(e) - - # Do we want to delete the files also from the resource? - ResourceBase.objects.filter(id=self.resource.id).update(files={}) + if self.resource: + for _, _file in self.resource.files.items(): + try: + dirname = os.path.dirname(_file) + if storage_manager.exists(dirname): + storage_manager.delete(dirname) + break + except Exception as e: + logger.warning(e) + + # Do we want to delete the files also from the resource? + ResourceBase.objects.filter(id=self.resource.id).update(files={}) for _location in importer_locations: try: From f39a70c7c7337f18321f7d6828e2ef641f6b6c8c Mon Sep 17 00:00:00 2001 From: mattiagiupponi Date: Mon, 31 May 2021 15:30:07 +0200 Subject: [PATCH 4/4] Fix proxy tag tests --- geonode/proxy/tests.py | 45 ++++++++++++++++++++++++------------------ geonode/proxy/views.py | 3 ++- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/geonode/proxy/tests.py b/geonode/proxy/tests.py index 7a94ef7052b..5b75d29f7d1 100644 --- a/geonode/proxy/tests.py +++ b/geonode/proxy/tests.py @@ -30,7 +30,7 @@ from geonode.proxy.templatetags.proxy_lib_tags import original_link_available from django.test.client import RequestFactory from mock import patch -from geonode.upload.models import Upload, UploadFile +from geonode.upload.models import Upload import json from django.core.files.uploadedfile import SimpleUploadedFile @@ -154,20 +154,25 @@ def test_download_url_with_not_existing_file(self): @on_ogc_backend(geoserver.BACKEND_PACKAGE) def test_download_url_with_existing_files(self, fopen, fexists): fexists.return_value = True - fopen.return_value = SimpleUploadedFile('file.shp', b'scc') + fopen.return_value = SimpleUploadedFile('foo_file.shp', b'scc') layer = Layer.objects.all().first() + layer.files = { + ".dbf": "/tmpe1exb9e9/foo_file.dbf", + ".prj": "/tmpe1exb9e9/foo_file.prj", + ".shp": "/tmpe1exb9e9/foo_file.shp", + ".shx": "/tmpe1exb9e9/foo_file.shx" + } + + layer.save() + + layer.refresh_from_db() + upload = Upload.objects.create( state='RUNNING', - layer=layer + resource=layer ) - _ = UploadFile.objects.create( - upload=upload, - file='/file.shp', - slug='foo_slug', - name="foo_name" - ) self.client.login(username='admin', password='admin') # ... all should be good response = self.client.get(reverse('download', args=(layer.id,))) @@ -218,25 +223,27 @@ def test_tag_original_link_available_with_different_netlock_should_return_true(s def test_should_return_false_if_no_files_are_available(self): _ = Upload.objects.create( state='RUNNING', - layer=self.resource + resource=self.resource ) actual = original_link_available(self.context, self.resource.resourcebase_ptr_id, self.url) self.assertFalse(actual) @patch('geonode.storage.manager.storage_manager.exists', return_value=True) - def test_should_return_true_if_no_files_are_available(self, fexists): + def test_should_return_true_if_files_are_available(self, fexists): upload = Upload.objects.create( state='RUNNING', - layer=self.resource - ) - - _ = UploadFile.objects.create( - upload=upload, - file='/file.shp', - slug='foo_slug', - name="foo_name" + resource=self.resource ) + self.resource.files = { + ".dbf": "/tmpe1exb9e9/foo_file.dbf", + ".prj": "/tmpe1exb9e9/foo_file.prj", + ".shp": "/tmpe1exb9e9/foo_file.shp", + ".shx": "/tmpe1exb9e9/foo_file.shx" + } + self.resource.save() + + self.resource.refresh_from_db() actual = original_link_available(self.context, self.resource.resourcebase_ptr_id, self.url) self.assertTrue(actual) diff --git a/geonode/proxy/views.py b/geonode/proxy/views.py index 7699c8e2d18..b9bfdf2f5a3 100644 --- a/geonode/proxy/views.py +++ b/geonode/proxy/views.py @@ -18,6 +18,7 @@ # ######################################################################### +from geonode.base.models import ResourceBase import io import os import re @@ -266,7 +267,7 @@ def download(request, resourceid, sender=Layer): permission='base.download_resourcebase', permission_msg=_not_permitted) - if isinstance(instance, Layer): + if isinstance(instance, ResourceBase): # Create Target Folder dirpath = tempfile.mkdtemp(dir=settings.STATIC_ROOT) dir_time_suffix = get_dir_time_suffix()