Opened 2 years ago

Closed 2 years ago

#34237 closed Bug (invalid)

FileField does not take upload_to into account when setting unique=True

Reported by: 0x4A-0x41-0x4B Owned by: nobody
Component: File uploads/storage Version: 4.1
Severity: Normal Keywords: FileField, Unique, Storage, Upload
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by 0x4A-0x41-0x4B)

#27188 added support for setting unique=True to the FileField.
This does not seem to take the upload_to argument into account.

As upload_to could be used to completely change the name of the uploaded file it should, in my opinion, be considered part of the filename.
Or should at the very least be mentioned in the docs for clarity.

If upload_to is defined, setting unique=True does effectively nothing right now. The file is passed on and a unique filename is generated (->The default behavior of adding an underscore followed by seven random characters) instead of raising an error.

Change History (4)

comment:1 by 0x4A-0x41-0x4B, 2 years ago

Description: modified (diff)

in reply to:  description ; comment:2 by Mariusz Felisiak, 2 years ago

Resolution: invalid
Status: newclosed

Replying to 0x4A-0x41-0x4B:

#27188 added support for setting unique=True to the FileField.
This does not seem to take the upload_to argument into account.

As far as I'm aware, upload_to directory is concatenated with the name and accounted for by an unique constraint.

As upload_to could be used to completely change the name of the uploaded file it should, in my opinion, be considered part of the filename.
Or should at the very least be mentioned in the docs for clarity.

If upload_to is defined, setting unique=True does effectively nothing right now.

That's not true. unique=True only means that a database-level constraint is created.

The file is passed on and a unique filename is generated (->The default behavior of adding an underscore followed by seven random characters) instead of raising an error.

It depends on the implementation of storage.generate_filename(). The database constraint is the last frontier where uniqueness is protected.

in reply to:  2 ; comment:3 by 0x4A-0x41-0x4B, 2 years ago

Resolution: invalid
Status: closednew

Replying to Mariusz Felisiak:

As far as I'm aware, upload_to directory is concatenated with the name and accounted for by an unique constraint.

This is strictly taken from the documentation (i have not yet looked into the source to back this up). If you define a custom upload_to callback, it will be passed on directly to the storage's save method (as mentioned in the documentation).

That's not true. unique=True only means that a database-level constraint is created.

Yep, that's true, sorry for the mistake on my part.

It depends on the implementation of storage.generate_filename(). The database constraint is the last frontier where uniqueness is protected.

Well, what's the point of upload_to then? Why would i ever use the FileField's upload_to, if i can use the storage's generate_filename method?

This puts up a barrier for anyone trying to enforce a purely unique set of file entries. The documentation for this is, if nothing else, misleading:

This attribute provides a way of setting the upload directory and file name ... Source

As its clearly NOT used to set the filename if the storage can (and will) completely disregard it.

in reply to:  3 comment:4 by Mariusz Felisiak, 2 years ago

Resolution: invalid
Status: newclosed

Well, what's the point of upload_to then? Why would i ever use the FileField's upload_to, if i can use the storage's generate_filename method?

If you prefer to write and use a custom storage, please do. The upload_to option is intended for most of users who would like to change only the directory, not other storage mechanisms.

This puts up a barrier for anyone trying to enforce a purely unique set of file entries. The documentation for this is, if nothing else, misleading:

This attribute provides a way of setting the upload directory and file name ... Source

As its clearly NOT used to set the filename if the storage can (and will) completely disregard it.

Built-in storage uses upload_to. I'm not sure what you're proposing, do you want to document that 3rd-party storages can ignore upload_to? TBH, they can do almost anything, that's how custom implementation works, I see no point in documenting this. Feel-free to propose a docs clarification, however I'm skeptical.

For me this ticket is still "invalid".

Note: See TracTickets for help on using tickets.
Back to Top