[Issue #6414] File system operations do not adhere to Django file storage API

[mtnorthcott]

Fixes #6414

PR to replace file system operations performed using Python's os, shutil etc. library functions with the more generic Django file storage API function calls with the goal of supporting Django storage plugins such as django-storage-swift.

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

For all pull requests:

• Confirm you have read the contribution guidelines
• You have sent a Contribution Licence Agreement (CLA) as necessary (not required for small changes, e.g., fixing typos in the documentation)
• Make sure the first PR targets the master branch, eventual backports will be managed later. This can be ignored if the PR is fixing an issue that only happens in a specific branch, but not in newer ones.

The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):

• There is a ticket in https://github.com/GeoNode/geonode/issues describing the issue/improvement/feature (a notable exemption is, changes not visible to end-users)
• The issue connected to the PR must have Labels and Milestone assigned
• PR for bug fixes and small new features are presented as a single commit
• Commit message must be in the form "[Fixes #<issue_number>] Title of the Issue"
• New unit tests have been added covering the changes, unless there is an explanation on why the tests are not necessary/implemented
• This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
• This PR passes the QA checks: flake8 geonode
• Commits changing the settings, UI, existing user workflows, or adding new functionality, need to include documentation updates

Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.

[codecov]

Codecov Report

Merging #6425 into master will decrease coverage by 0.00%.
The diff coverage is 65.71%.

@@            Coverage Diff             @@
##           master    #6425      +/-   ##
==========================================
- Coverage   59.01%   59.00%   -0.01%     
==========================================
  Files         184      185       +1     
  Lines       20569    20598      +29     
  Branches     3416     3420       +4     
==========================================
+ Hits        12138    12154      +16     
- Misses       7358     7374      +16     
+ Partials     1073     1070       -3     

[afabiani]

Hi @mtnorthcott it's been a bit difficult to understand when a PR is ready for the review or not.

I'm kindly asking to either create just one PR for each issue, with no further commits, or put the PRs in DRAFT until they are still a work in progress. Thanks for your understanding.

[lgtm-com]

This pull request fixes 1 alert when merging 62f0a916c17c0af0cd4dc4e844b744149b3e0155 into d4c9311 - view on LGTM.com

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[mtnorthcott]

Hi @mtnorthcott it's been a bit difficult to understand when a PR is ready for the review or not.

I'm kindly asking to either create just one PR for each issue, with no further commits, or put the PRs in DRAFT until they are still a work in progress. Thanks for your understanding.

@afabiani I had intended to tag the description with 'Closes #6414' once complete and make a comment but I did not communicate this so my apologies for the confusion. I have tagged this PR as a draft for the time being.

[mtnorthcott]

I have done some testing and, although there are likely instances of local storage operations that I've missed, this PR is now in a state in which feature-breaking bugs are fixed for third-party plugins as described. This predominantly includes creation of documents and document thumbnails using unoconv, layer thumbnail generation and orphaned thumbnail deletion. I am happy for @afabiani to review as time permits. Please let me know of any changes you would like made.

[afabiani]

@mtnorthcott for the time being I see some tests failing on Travis like this

======================================================================
ERROR: test_create_document_url_view (geonode.documents.tests.DocumentsTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/GeoNode/geonode/geonode/documents/tests.py", line 140, in test_create_document_url_view
    response = self.client.post(reverse('document_upload'), data=form_data)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/test/client.py", line 543, in post
    response = super().post(path, data=data, content_type=content_type, secure=secure, **extra)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/test/client.py", line 357, in post
    secure=secure, **extra)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/test/client.py", line 422, in generic
    return self.request(**r)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/test/client.py", line 503, in request
    raise exc_value
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view
    return view_func(request, *args, **kwargs)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/views/generic/base.py", line 71, in view
    return self.dispatch(request, *args, **kwargs)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/views/generic/base.py", line 97, in dispatch
    return handler(request, *args, **kwargs)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/views/generic/edit.py", line 172, in post
    return super().post(request, *args, **kwargs)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/views/generic/edit.py", line 142, in post
    return self.form_valid(form)
  File "/home/travis/build/GeoNode/geonode/geonode/documents/views.py", line 240, in form_valid
    self.object.save(notify=True)
  File "/home/travis/build/GeoNode/geonode/geonode/base/models.py", line 913, in save
    super(ResourceBase, self).save(*args, **kwargs)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/polymorphic/models.py", line 91, in save
    return super(PolymorphicModel, self).save(*args, **kwargs)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/db/models/base.py", line 741, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/db/models/base.py", line 790, in save_base
    update_fields=update_fields, raw=raw, using=using,
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/dispatch/dispatcher.py", line 175, in send
    for receiver in self._live_receivers(sender)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/django/dispatch/dispatcher.py", line 175, in <listcomp>
    for receiver in self._live_receivers(sender)
  File "/home/travis/build/GeoNode/geonode/geonode/documents/models.py", line 220, in create_thumbnail
    result = create_document_thumbnail.delay(object_id=instance.id)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/celery/app/task.py", line 426, in delay
    return self.apply_async(args, kwargs)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/celery/app/task.py", line 564, in apply_async
    link=link, link_error=link_error, **options)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/celery/app/task.py", line 776, in apply
    ret = tracer(task_id, args, kwargs, request)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/celery/app/trace.py", line 429, in trace_task
    I, R, state, retval = on_error(task_request, exc, uuid)
  File "/home/travis/virtualenv/python3.7.1/lib/python3.7/site-packages/celery/app/trace.py", line 412, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/home/travis/build/GeoNode/geonode/geonode/documents/tasks.py", line 81, in create_document_thumbnail
    image_file.close()
AttributeError: 'NoneType' object has no attribute 'close'

[lgtm-com]

This pull request fixes 1 alert when merging dfab8af70e617dad174f2d9ef94355ab86000eb2 into 0e03367 - view on LGTM.com

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[lgtm-com]

This pull request fixes 1 alert when merging 0a0ee4d50a80d2932615f83638c420ab033be96a into 0e03367 - view on LGTM.com

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[lgtm-com]

This pull request fixes 1 alert when merging 040711522ed41a1c86e5882b29260671e2f2d1fa into 0e03367 - view on LGTM.com

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[lgtm-com]

This pull request fixes 1 alert when merging c2ad4a93a91d16af56356336933ea75f501a27ab into 0e03367 - view on LGTM.com

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[srtonz]

There are still a couple of changes that should be made before merging this code. Apologies @mtnorthcott for not getting to review this earlier.

[lgtm-com]

This pull request fixes 1 alert when merging ab15cdaf1976d815e9b6f48686357b9ec1638d64 into 24ba526 - view on LGTM.com

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[lgtm-com]

This pull request fixes 1 alert when merging 1db58caba39dc4e7075f6f8d68af3965976eebcf into 24ba526 - view on LGTM.com

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[lgtm-com]

This pull request introduces 1 alert and fixes 1 when merging cc5717d85bc80a4d94e7842c1afe453ac3dd2adf into 24ba526 - view on LGTM.com

new alerts:

• 1 for Unused local variable

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[lgtm-com]

This pull request fixes 1 alert when merging 570ec5d into 24ba526 - view on LGTM.com

fixed alerts:

• 1 for Module is imported with 'import' and 'import from'

[srtonz]

@mtnorthcott apologies this took a while to review, but looking good to me now.