Attachments API use filename from Content-Disposition before url

[tylerjmchugh]

Currently the attachments putResourceFromURL API uses the last node from the URL (excluding query parameters) to get the filename. This becomes an issue when providing URL's that do not contain the filename in the last node. Instead the content disposition header should have higher priority over the url for retrieving the filename.

For example when providing a URL for a file that is uploaded to a cloud provider like OneDrive the download URL looks like .../.../download.aspx?.... This means when putResourceFromUrl is called the extracted filename is always download.aspx rather than the actual file download's name. If content disposition is used instead the actual filename can be extracted from the header.

Additionally there are some issues with the "consumes" and "produces" spring docs for putResource and putResourceFromURL leading to an incorrect OpenAPI spec which causes the APIs to fail or return invalid responses. This also causes the swagger page to be incorrect and causes issues for codegen as the generated methods for putResource do not correctly provide the file as multipart/form-data.

This should show that only multipart is accepted:

Issues with response codes because JSON is not defined as the return type (This call was successful):

This PR aims to fix these issues by first attempting to get the file name from the Content-Disposition header before falling back to the URL for the filename and by updating the spring docs.

Checklist

• I have read the contribution guidelines
• Pull request provided for main branch, backports managed with label
• Good housekeeping of code, cleaning up comments, tests, and documentation
• Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
• Clean commit messages, longer verbose messages are encouraged
• API Changes are identified in commit messages
• Testing provided for features or enhancements using automatic tests
• User documentation provided for new features or enhancements in manual
• Build documentation provided for development instructions in README.md files
• Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

[josegar74]

I have tested with some URL's and files from Google Drive, but Content-disposition is not set. I don't have access to OneDrive, but if you have tested, looks ok.

See the comment in the code.

[josegar74]

If an exception occurs, should not be better to return null, so it falls back to getFilenameFromUrl?

[tylerjmchugh]

Good point, fixed in latest commit

[tylerjmchugh]

I have tested with some URL's and files from Google Drive, but Content-disposition is not set. I don't have access to OneDrive, but if you have tested, looks ok.

See the comment in the code.

I am having trouble finding a sample resource that uses Content-disposition other than on my own OneDrive but yes it is tested and works when the content disposition header is present. Also I fixed the issue you identified.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[geonetworkbuild]

The backport to 4.2.x failed:

The process '/usr/bin/git' failed with exit code 1

stderr

The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

    git commit --allow-empty

Otherwise, please use 'git cherry-pick --skip'

stdout

Auto-merging services/src/main/java/org/fao/geonet/api/records/attachments/AttachmentsApi.java
[backport-8470-to-4.2.x 93afa1ff93] Update spring doc consumes and produces
 Author: tylerjmchugh <[email protected]>
 Date: Mon Oct 28 13:02:24 2024 -0400
 1 file changed, 4 insertions(+), 2 deletions(-)
Auto-merging core/src/main/java/org/fao/geonet/api/records/attachments/AbstractStore.java
[backport-8470-to-4.2.x b30d08ed42] Update putResource logic to get filename from contentDisposition header before using url
 Author: tylerjmchugh <[email protected]>
 Date: Mon Oct 28 13:03:51 2024 -0400
 1 file changed, 23 insertions(+), 1 deletion(-)
On branch backport-8470-to-4.2.x
You are currently cherry-picking commit 49af24534d.
  (all conflicts fixed: run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

nothing to commit, working tree clean

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.2.x 4.2.x
# Navigate to the new working tree
cd .worktrees/backport-4.2.x
# Create a new branch
git switch --create backport-8470-to-4.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick e5c788aaa20052d15f066fa73ab213c231ef30b7,cabdf0168c90aea78a71aaec340da515702e58d2,49af24534d997b7ca111799240cdf467c793a3f6,7794d7663ee3a14a479e57052a41601c3ce498e7,e040fab62c9c4a6ec9da935f8bfc73b7ea342b75,702b3ae671ed844c453ce065e52e970eee120e5e,77d5621e9b17871fd8e0b200e7c069a502961a14
# Push it to GitHub
git push --set-upstream origin backport-8470-to-4.2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.2.x

Then, create a pull request where the base branch is 4.2.x and the compare/head branch is backport-8470-to-4.2.x.