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

Fixes #1957: Ensure unique S3 object keys by adding numeric suffix. #1963

Merged
merged 6 commits into from
Aug 8, 2024

Conversation

abhinavxd
Copy link
Contributor

  • Removed unused functions and variables in filesystem.go

@knadh
Copy link
Owner

knadh commented Jul 29, 2024

Thanks for the PR!

Thinking about it, rather than each provider implementing its own logic, it'd be simpler to do it centrally in handleUploadMedia() and check in the database. Roughly:

  • Add a GetMediaByFilename(fname, provider string) function to core/media.go
  • This will require a new query to be added to queries.sql also
  • Use this in handleUploadMedia() to assert the incoming filename and tweak it if it already exists.
  • Remove the existing assertion logic from the filesystem provider.

@abhinavxd
Copy link
Contributor Author

Yep, makes sense. I will update the PR.

@abhinavxd
Copy link
Contributor Author

abhinavxd commented Jul 29, 2024

image
I added the DB level check for unique filename, but it fails when multiple files with the same name are selected for upload.

The previous provider level check handles this properly, Could not recreate it with provider level checks.

image

Abhinav Raut added 3 commits July 31, 2024 01:50
- remove provider level checks for unique filename.
- adds new utils function `appendSuffixToFilename`
@abhinavxd
Copy link
Contributor Author

@knadh Changes made as discussed.

cmd/utils.go Outdated Show resolved Hide resolved
Copy link
Owner

@knadh knadh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple minor things to fix.

@abhinavxd
Copy link
Contributor Author

Made the changes requested.

@knadh knadh merged commit 679457c into knadh:master Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants