Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements for DownloadHandler #11421

Closed
5 tasks done
mattiagiupponi opened this issue Aug 28, 2023 · 0 comments · Fixed by #11423
Closed
5 tasks done

Improvements for DownloadHandler #11421

mattiagiupponi opened this issue Aug 28, 2023 · 0 comments · Fixed by #11423
Assignees
Labels
API v2 feature A new feature to be added to the codebase
Milestone

Comments

@mattiagiupponi
Copy link
Contributor

mattiagiupponi commented Aug 28, 2023

As a follow-up of this issue #11296 we want to introduce a new set of improvement for the download handler.
This improvements will help to make room for an API that supports multiple downloads (in the future)

Datasets

  • Rename DownloadHandler to DatasetDownloadHandler, and move it under the layers module
  • The DatasetDownloadHandler implements an is_ajax_safe method which will return a boolean -> whether the download_url can be called with AJAX or not. For the default handler it will be:
    • true for WPS
    • false for all the other cases
  • Remove the logic for the creation of the "original" Link entries here.
  • Dataset.download_url will just return build_absolute_uri. The logic of whether to return any "original" Link (created manually or received from previous DBs) will be moved to the DatasetDownloadHandler.

Documents

  • Also, documents must return whether AJAX is safe or not. I wouldn't bother too much here, since we have a convoluted download / link / href logic for documents. Maybe we can implement a Document.download_is_ajax_safe property returning true if the resource is LOCAL, false otherwise.

API

  • Implement a download_urls array property for the ResourceBaseSerializer with the following structure:
 "download_urls": [
   {
      "URL": .... , 
      "ajax_safe": true|false, 
      "default":true|false
   },
]	
  • For the moment download_urls will contain only one entry;
    • For datasets, it will be populated with the properties obtained from the single DatasetDownloadHandler. NOTICE that for REMOTE resources (e.g. services) download_urls should return an empty array. Let's think if it will be the serializer to handle this case, or let DatasetDownloadHandler return the download_url object itself (which would be None for REMOTE datasets).
    • For documents, it will be populated from Document.download_url and Document.download_is_ajax_safe
    • For the other resource types it will return an empty array
  • Deprecate the download_url field.
@mattiagiupponi mattiagiupponi added feature A new feature to be added to the codebase API v2 labels Aug 28, 2023
@mattiagiupponi mattiagiupponi added this to the 4.2.0 milestone Aug 28, 2023
@mattiagiupponi mattiagiupponi self-assigned this Aug 28, 2023
mattiagiupponi added a commit that referenced this issue Aug 28, 2023
mattiagiupponi added a commit that referenced this issue Aug 28, 2023
@mattiagiupponi mattiagiupponi linked a pull request Aug 28, 2023 that will close this issue
12 tasks
mattiagiupponi added a commit that referenced this issue Aug 28, 2023
mattiagiupponi added a commit that referenced this issue Aug 31, 2023
mattiagiupponi added a commit that referenced this issue Sep 1, 2023
giohappy added a commit that referenced this issue Sep 8, 2023
* [Fixes #11421] Improvements for DownloadHandler

* [Fixes #11421] Fix tests

* [Fixes #11421] Fix tests

* [Fixes #11421] Fix tests

* [Fixes #11421] Improve module import in download handlers

* [Fixes #11421] Improve module import in download handlers

* [Fixes #11335] fix flake8

* [Fixes #11421] fix broken test

* [Fixes #11421] Test fix tests

* [Fixes #11421] rollback test changes

* [Fixes #11421] Test order response value

* [Fixes #11421] Fix flake8 and black issues

* improve ajax_safe logic, extended tests

* remove unused import

* remove unused import (2)

* fixed custom download handler test

* fix

* black fix

* removed old parameter

---------

Co-authored-by: G. Allegri <giohappy@gmail.com>
@ridoo ridoo mentioned this issue Apr 12, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API v2 feature A new feature to be added to the codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant