django-cms / django-filer

File and Image Management Application for django
https://django-filer.readthedocs.io/
Other
1.73k stars 574 forks source link

Uploading SVGs without easy-thumbnails[svg] leaves db in inconsistent state #1433

Closed benzkji closed 3 months ago

benzkji commented 8 months ago

Without easy-thumbnails[svg] (i.e. reportlab) SVGs are still uploaded/stored as Image, but they have width=Null and height=Null stored. To be really consistent, one could store SVGs as File, when no support for that image format is available? Or deny SVG upload completly, without support?

Currently, it can be a pain, as one can start a project without knowing this, and ending up with a few hundred svgs in the db, without dimensions.

Dont know what the best solution could be...?

benzkji commented 8 months ago
from django.core.management import BaseCommand
from django.db.models import Q
from easy_thumbnails.VIL import Image as VILImage
from filer.models import Image

class Command(BaseCommand):
    help = (
        "update dimensions of svg images that have no dimensions set, "
        "for whatever reason"
    )

    def handle(self, *args, **options):
        no_dimensions = Image.objects.filter(file__endswith=".svg").filter(
            Q(_width=0) | Q(_width__isnull=True)
        )
        self.stdout.write(f"trying to set dimensions on {no_dimensions.count()} files")
        for image in no_dimensions:
            self.stdout.write(image.path)
            try:
                imgfile = image.file.file
            except ValueError:
                imgfile = image.file_ptr.file
            imgfile.seek(0)
            image._width, image._height = VILImage.load(imgfile).size
            image.save()
        return

Quickhack, copied some code from filer. but I get a lot of these errors: ERROR:root:Unsupported shape type Group for clipping

Dimensions are not updated. And it takes a very long time, for every single SVG. One can argue that my SVGs are too complex or whatever, but these are valid SVGs. I can provide examples if needed?

UPDATE: All above is true, except that it actually works! Dimensions are set. I've corrected my code, this could be used in filer in this or similar form...for people that forget easy-thumbnails[svg].

fsbraun commented 8 months ago

@benzkji Can I interest you in creating a PR with that code? I am not sure if that error throws an exception. Maybe just catch the output?

benzkji commented 8 months ago

yes. master is the branch to branch off, right?

benzkji commented 8 months ago

It doesnt throw an error, but logged 10'000 entries to my Sentry this morning. PR on the way.

benzkji commented 8 months ago

1434

jrief commented 7 months ago

No, this is a django-filer issue and has to be addressed here.

benzkji commented 7 months ago

@jrief have you seen the PR? Not sure which direction to go.

Also, fixing (with management command) is one thing, and preventing getting in an inconsistent state is another.

fsbraun commented 7 months ago

@benzkji Can we close this issue now that #1434 is merged?

benzkji commented 7 months ago

not really sure...as #1434 only does help when things already went wrong...but prevent getting there would be the goal. I don't have a definitive answer, but tend to the "deny uploads" when SVG support is not available? If you decide to not do anything, I'm fine too, feel free to close then ;-)

fsbraun commented 7 months ago

Or maybe just turn a SVG into a File object as opposed to Image object if SVG support is not installed. That's probably easily achievable. If SVG support is added later, however, Files will remain Files.

benzkji commented 7 months ago

Yes, but could lead to other undesired situations (client uploads a few SVGs, then has to re-upload and re-assign everything, when "no SVG support" is detected). I now know about, so...it's just about finding the most accessible solution for people who don't. I would prefer an Image withouth width, and being able to call the managemen command later on.

stale[bot] commented 4 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 3 months ago

This will now be closed due to inactivity, but feel free to reopen it.