Variants are not re-generated on overwrite image

[berthj193]

Thanks for putting efforts into this great package! I found this one most relevant to my requirement, among several django imaging libraries.

The only issue I'm having is variants don't get regenerated when you replace an existing image.

It works fine locally (django.core.files.storage.FileSystemStorage), but not on S3 (storages.backends.s3boto3.S3Boto3Storage). Only main image is replaced, and thumbnail variants remain untouched.

• main_image.png (replaced with new one)
• main_image.md.png (untouched, old)
• main_image.sm.png(untouched, old)

My workaround was to add random hash to file name so that set of 3 new images are created on S3 bucket, each time you replace. It's not good enough since old images hold the space.

It would be great if someone can take a quick double check, and fix if easy.

[codingjoe]

Hi @MingTheGit thanks for your kind words and thanks for reaching out. I let me try to rephrase your situation, to make sure I am getting it right.

You are uploading a new file, with the same name as a previously uploaded file, right? You are not using the rendervariations --replace management command, are you?
Does that mean your AWS_S3_FILE_OVERWRITE setting is set to True? Otherwise the storage backend should reject the file name and create a new one with a random bit at the end. I would recommend not to allow overriding file names (change the setting to False) to avoid collisions.

That being said, if the setting is set to False this will break the package. You see, if you call storage.save(name, content), it will actually modify the file name if a file with the same name already exists. That behavior is not supported by this package, since we don't persist the variation names.
Therefore it is important that the original image names always remain unique or the old images (including variations) are deleted in advance using obj.image_field.delete().

I hope that helps you a bit, let me know if you have any further questions.

Best
-joe

[berthj193]

@codingjoe Thanks for your quick answer!

That being said, if the setting is set to False this will break the package.

Did you mean "That being said, if the setting is set to True this will break the package."?

• AWS_S3_FILE_OVERWRITE = False I confirm it perfectly generates new main image and variations with hash appended.
• AWS_S3_FILE_OVERWRITE = True I'm not sure why it replaces only the main image, but not variants. Isn't it the job of S3Boto3Storage to replace if file is already present?
  (https://github.com/jschneier/django-storages/blob/master/storages/backends/s3boto3.py#L631)

I'm not using management command, posting snippet of my code.

profile_image = StdImageField(
    blank=True,
    default="",
    upload_to=profile_image_media_path,
    variations={"regular": (180, 180, True), "thumbnail": (76, 76, True)},
)

def profile_image_media_path(project, filename):
    ...
    _, extension = os.path.splitext(filename)
    return f"models/{model_id}/profile_image{extension}"

[codingjoe]

@MingTheGit check these lines here:

django-stdimage/stdimage/models.py

Lines 62 to 69 in 920c68b

	if storage.exists(variation_name):
	if replace:
	storage.delete(variation_name)
	logger.info('File "%s" already exists and has been replaced.',
	variation_name)
	else:
	logger.info('File "%s" already exists.', variation_name)
	return variation_name

You see, we check if the file already exists. The default behavior is not to override existing files. In any case, your filename pattern would require you to actually delete the picture before overriding it. That being said, i don't know if this is a good filename pattern. It depends on your bucket policy is set to private, otherwise guessing filenames is pretty easy. Check out another creation of mine, to create more advanced filename patterns: https://github.com/codingjoe/django-dynamic-filenames

[berthj193]

@codingjoe

Thanks for your checking, I appreciate it. I have tried some debugging and it seems like this is going to be a new feature request.

So, replace flag is always False in my case, because I'm not using management command.
L58 accepts replace from either L55 or management command. As far as I know replace in L52 is always False.

django-stdimage/stdimage/models.py

Lines 52 to 58 in 920c68b

	def render_variations(self, replace=False):
	"""Render all image variations and saves them to the storage."""
	for _, variation in self.field.variations.items():
	self.render_variation(self.name, variation, replace, self.storage)
	@classmethod
	def render_variation(cls, file_name, variation, replace=False,

Can you please consider adding replace | overwrite flag to StdImageField in the next roadmap?
Thank you in advance.

[berthj193]

How it works in the following scenarios

• django.core.files.storage.FileSystemStorage
• storages.backends.s3boto3.S3Boto3Storage with AWS_S3_FILE_OVERWRITE = False

Django ImageField generates hashed file name in these cases.
(self.name in L55, file_name in L58)

replace flag is not checked at all, because storage.exists(variation_name) is always False in L62.

[codingjoe]

@MingTheGit I didn't plan on implementing that feature, but feel free to make a suggestion.
Best Joe

[caspervk]

Why is this issue closed? It can never be correct behavior to replace the original while preserving the old variations, resulting in completely different images depending on the variation. Either all variations (including the original) should be replaced or none of them should.
In this regard I agree with @MingTheGit that it should be possible to replace images. In fact, I'd argue that this should be the default for variations, like it currently is for the original.

[codingjoe]

@caspervk: If you believe this to be a bug, I can't wait for your PR to resolve it :)