neutronX / django-markdownx

Comprehensive Markdown plugin built for Django
https://neutronx.github.io/django-markdownx/
Other
859 stars 153 forks source link

Human readable image file names #86

Open solidether opened 7 years ago

solidether commented 7 years ago

Is it possible to override this function in forms.py to the effect below? I have a small personal blog and since I'm setting the upload path to be date based, I'd prefer to keep the original file names intact. Where would I place the code for the override if it's possible? Could this be implemented as a setting? Or alternatively, I could be convinced that this is a bad idea. :-)

In forms.py:

@staticmethod
    def get_unique_file_name(file_name):
        """
        Generates a universally unique ID using Python ``UUID`` and attaches the
        extension of file name to it.

        :param file_name: Name of the uploaded file, including the extension.
        :type file_name: str
        :return: Universally unique ID, ending with the extension extracted
                 from ``file_name``.
        :rtype: str
        """
        """
        extension = 1
        extension_dot_index = 1

        file_name = "{unique_name}.{extension}".format(
            unique_name=uuid4(),
            extension=path.splitext(file_name)[extension][extension_dot_index:]
        )
        """
        return file_name
adi- commented 7 years ago

No, it is not bad. You get original file_name as a function parameter. However, there need to be a mechanism that makes filename unique, so you need somehow change the original name. The easiest one, which don't relies on DB would be:

file_name = "{original_name}_{unique_name}.{extension}".format(
  original_name=file_name,
  unique_name=uuid4(),
  extension=path.splitext(file_name)[extension][extension_dot_index:]
)
solidether commented 7 years ago

Thanks adi-, I understand. Right now I'm making a note to myself to adjust that code after updates. I've been having trouble figuring out how to override the function elsewhere in my application. It would have to be before the module loads correct? Any suggestions on how to do this and where to put the override would be appreciated!

adi- commented 7 years ago

Currently markdownx doesn't support overriding that function.

solidether commented 7 years ago

Ok, thanks for the code suggestion.

xenatisch commented 7 years ago

@adi- if we're doing this, I don't think a UUID4 is that necessary. We can have a 4 digit random number instead. To adhere to best practices, we can enforce a date-based subdirectory management (default behaviour) as well. For instance:

MEDIA
    +-- 2017
        +-- 10
            +-- 11
                +-- imageName_1234.ext

But the filename with a UUID would be way too long. Plus, it wouldn't be possible the remove the UUID from the filename programmatically later on (there is no unique character we can use a separator).

Alternatively, we can create an XML somewhere, or leverage a counter in the database to deal with file enumeration incrementally as opposed to randomly.

Any thoughts on this?

adi- commented 7 years ago

@xenatisch I guess we can change it to the algorithm you suggested. Also, I would add new markdown setting variable as a function handler, so anyone could easily override default code.

solidether commented 7 years ago

@adi, @xenatisch I think these suggestions sound great. Being able to override would make things a lot more flexible. Thanks!

xenatisch commented 7 years ago

@adi- Good thing this is a static function then.

It'll be fairly straight-forward to implement.

xenatisch commented 7 years ago

To follow up re #88:

Name

How about name_func for a name in the new implementation in #88 ? It's more concise, yet as explicit.

Implementation

Function

It also should be defined as follows: func(filename: str, ext: str) -> tuple, where the returned value is a tuple of either ('unique_name.ext',) or ('some', 'path', 'unique_name.ext') that will be fed to os.path.join as:

os.path.join(MEDIA, *func_resp)

whose result must be a unique path.

Note: MEDIA has to be enforced. It is a non-negotiable part because it enforces consistency and maintains security. Otherwise, inexperienced developers might feel inclined to save the file anywhere they feel like, which would be a recipe for disaster.

Fallback

Just in case, we can double check to ensure there is no existing file at the given path. If there is, we can (a) add some random number to the name, and (b) raise a warning explaining that the path produced must be unique, but it is not.