Repository: CS-SI/eodag
Pull request #542 (2022-11-03 10:24:48)

fix: new methods to get assets filename from header
Only HEAD method was used to try extracting `filename` from `content-disposition` header.
Now also try GET method to get this header when download starts.

Also strips out filename from query args when it is obtained from parsed url.

github-actions[bot]: <strong> </strong>

| File | Coverage |   |
| - | :-: | :-: |
| **All files** | `84%` | :white_check_mark: |
| eodag/plugins/download/http.py | `76%` | :white_check_mark: |
| tests/units/test\_download\_plugins.py | `96%` | :white_check_mark: |

<strong> </strong>

| File | Coverage |   |
| - | :-: | :-: |
| **All files** | `83%` | :white_check_mark: |
| eodag/plugins/download/http.py | `76%` | :white_check_mark: |
| tests/units/test\_download\_plugins.py | `96%` | :white_check_mark: |
| eodag/plugins/download/http.py | `76%` | :white_check_mark: |
| tests/units/test\_download\_plugins.py | `95%` | :white_check_mark: |

_Minimum allowed coverage is `70%`_

<p align="right">Generated by :monkey: cobertura-action against 2ae34299e8538cd0c17ca1381b7f265644fef7d6 </p>
github-actions[bot]: ## Unit Test Results
    2 files  ±0      2 suites  ±0   10m 4s [:stopwatch:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "duration of all tests") +41s
299 tests +3  296 [:heavy_check_mark:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "passed tests") +3  3 [:zzz:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "skipped / disabled tests") ±0  0 [:x:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "failed tests") ±0 
598 runs  +6  592 [:heavy_check_mark:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "passed tests") +6  6 [:zzz:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "skipped / disabled tests") ±0  0 [:x:](https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#the-symbols "failed tests") ±0 

Results for commit e47e64f7. ± Comparison against base commit 3cdbd4c6.

[test-results]:data:application/gzip;base64,H4sIAB2bY2MC/03MSw7CIBSF4a00jB0QHhdxMwZ5JMS2GAqjxr17xYId/t9Jzk5CnP1GbhO7TGSrsYxwNZsS04oJVCDgVNqoda/7Vq1tBH96xhcSHxBMnBHoAJ9zyofkun4/pb4e0S+lZkN+j9D7dNj6/GfTssSCQbxQHkRQYJRjlEsq4OGAG0aFlMqwIB2niOT9AS8MtUMFAQAA


Patch:
diff --git a/eodag/plugins/download/http.py b/eodag/plugins/download/http.py
index 96caf35a..68f0e5f6 100644
--- a/eodag/plugins/download/http.py
+++ b/eodag/plugins/download/http.py
@@ -21,6 +21,7 @@ import os
 import shutil
 import zipfile
 from cgi import parse_header
+from urllib.parse import urlparse
 
 import requests
 from requests import HTTPError, RequestException
@@ -305,7 +306,9 @@ class HTTPDownload(Download):
                 asset_url, auth=auth, timeout=HTTP_REQ_TIMEOUT
             ).headers.get("content-disposition", None)
             if asset_content_disposition:
-                asset_filename = parse_header(asset_content_disposition)[-1]["filename"]
+                asset_filename = parse_header(asset_content_disposition)[-1].get(
+                    "filename", None
+                )
             else:
                 asset_filename = None
 
@@ -342,15 +345,26 @@ class HTTPDownload(Download):
                         logger.warning("Skipping %s" % asset_url)
                     error_messages.add(str(e))
                 else:
-                    if asset_filename is not None:
-                        asset_rel_path = asset_filename
-                    else:
-                        asset_rel_path = (
-                            asset_url.replace(product.location, "")
-                            .replace("https://", "")
-                            .replace("http://", "")
+                    asset_rel_path = urlparse(asset_url).path.strip("/")
+                    asset_rel_dir = os.path.dirname(asset_rel_path)
+
+                    if asset_filename is None:
+                        # try getting content-disposition header from GET if not in HEAD result
+                        asset_content_disposition = stream.headers.get(
+                            "content-disposition", None
                         )
-                    asset_abs_path = os.path.join(fs_dir_path, asset_rel_path)
+                        if asset_content_disposition:
+                            asset_filename = parse_header(asset_content_disposition)[
+                                -1
+                            ].get("filename", None)
+
+                    if asset_filename is None:
+                        # default filename extracted from path
+                        asset_filename = os.path.basename(asset_rel_path)
+
+                    asset_abs_path = os.path.join(
+                        fs_dir_path, asset_rel_dir, asset_filename
+                    )
                     asset_abs_path_dir = os.path.dirname(asset_abs_path)
                     if not os.path.isdir(asset_abs_path_dir):
                         os.makedirs(asset_abs_path_dir)


Test patch:
diff --git a/tests/units/test_download_plugins.py b/tests/units/test_download_plugins.py
index 3b3c480b..10e1ba3b 100644
--- a/tests/units/test_download_plugins.py
+++ b/tests/units/test_download_plugins.py
@@ -158,6 +158,86 @@ class TestDownloadPluginHttp(BaseDownloadPluginTest):
         self.assertEqual(path, os.path.join(self.output_dir, "dummy_product"))
         self.assertTrue(os.path.isfile(path))
 
+    @mock.patch("eodag.plugins.download.http.requests.head", autospec=True)
+    @mock.patch("eodag.plugins.download.http.requests.get", autospec=True)
+    def test_plugins_download_http_assets_filename_from_href(
+        self, mock_requests_get, mock_requests_head
+    ):
+        """HTTPDownload.download() must create an outputfile"""
+
+        plugin = self.get_download_plugin(self.product)
+        self.product.location = self.product.remote_location = "http://somewhere"
+        self.product.properties["id"] = "someproduct"
+        self.product.assets = {
+            "foo": {"href": "http://somewhere/something?foo=bar#baz"}
+        }
+        mock_requests_get.return_value.__enter__.return_value.headers = {
+            "content-disposition": ""
+        }
+        mock_requests_head.return_value.headers = {"content-disposition": ""}
+
+        path = plugin.download(self.product, outputs_prefix=self.output_dir)
+
+        self.assertEqual(path, os.path.join(self.output_dir, "dummy_product"))
+        self.assertTrue(os.path.isdir(path))
+        self.assertTrue(
+            os.path.isfile(os.path.join(self.output_dir, "dummy_product", "something"))
+        )
+
+    @mock.patch("eodag.plugins.download.http.requests.head", autospec=True)
+    @mock.patch("eodag.plugins.download.http.requests.get", autospec=True)
+    def test_plugins_download_http_assets_filename_from_get(
+        self, mock_requests_get, mock_requests_head
+    ):
+        """HTTPDownload.download() must create an outputfile"""
+
+        plugin = self.get_download_plugin(self.product)
+        self.product.location = self.product.remote_location = "http://somewhere"
+        self.product.properties["id"] = "someproduct"
+        self.product.assets = {"foo": {"href": "http://somewhere/something"}}
+        mock_requests_get.return_value.__enter__.return_value.headers = {
+            "content-disposition": '; filename = "somethingelse"'
+        }
+        mock_requests_head.return_value.headers = {"content-disposition": ""}
+
+        path = plugin.download(self.product, outputs_prefix=self.output_dir)
+
+        self.assertEqual(path, os.path.join(self.output_dir, "dummy_product"))
+        self.assertTrue(os.path.isdir(path))
+        self.assertTrue(
+            os.path.isfile(
+                os.path.join(self.output_dir, "dummy_product", "somethingelse")
+            )
+        )
+
+    @mock.patch("eodag.plugins.download.http.requests.head", autospec=True)
+    @mock.patch("eodag.plugins.download.http.requests.get", autospec=True)
+    def test_plugins_download_http_assets_filename_from_head(
+        self, mock_requests_get, mock_requests_head
+    ):
+        """HTTPDownload.download() must create an outputfile"""
+
+        plugin = self.get_download_plugin(self.product)
+        self.product.location = self.product.remote_location = "http://somewhere"
+        self.product.properties["id"] = "someproduct"
+        self.product.assets = {"foo": {"href": "http://somewhere/something"}}
+        mock_requests_get.return_value.__enter__.return_value.headers = {
+            "content-disposition": '; filename = "somethingelse"'
+        }
+        mock_requests_head.return_value.headers = {
+            "content-disposition": '; filename = "anotherthing"'
+        }
+
+        path = plugin.download(self.product, outputs_prefix=self.output_dir)
+
+        self.assertEqual(path, os.path.join(self.output_dir, "dummy_product"))
+        self.assertTrue(os.path.isdir(path))
+        self.assertTrue(
+            os.path.isfile(
+                os.path.join(self.output_dir, "dummy_product", "anotherthing")
+            )
+        )
+
 
 class TestDownloadPluginHttpRetry(BaseDownloadPluginTest):
     def setUp(self):