Storages behaviour is not optimal

[Aramgutang]

There are numerous issues with the way storages are set up in ixc-django-docker.

For starters, this:

ixc-django-docker/ixc_django_docker/settings/storages.py

Lines 4 to 7 in bcf550f

	AWS_HEADERS = {
	'Expires': 'Thu, 31 Dec 2099 00:00:00 GMT',
	'Cache-Control': 'max-age=86400',
	}

Per the current django-storages docs, that actually has no effect, since it's only used with boto, not boto3, and we use the latter.

The suggested way to define a never-expire cache instead is via object parameters (also note the use of a properly large max-age):

AWS_S3_OBJECT_PARAMETERS = {
    'CacheControl': 'max-age=31536000',
    'Expires': datetime(2099, 12, 31),
}

However, I would hesitate to use that, as it would be the default for all uploads, not just those that use unique filenames, and we probably only want to use it when using unique filename storage. It would probably be best to add a new mixin:

class S3CacheForever(object):
    def __init__(self, *args, **kwargs):
        kwargs.setdefault('object_parameters', {
            'CacheControl': 'max-age=31536000',
            'Expires': datetime(2099, 12, 31),
        })
        super(S3CacheForever, self).__init__(*args, **kwargs)

And then add that to the list of mixins when constructing the unique storages:

ixc-django-docker/ixc_django_docker/storage.py

Lines 137 to 152 in bcf550f

	class S3UniquePrivateStorage(
	UniqueMixin,
	S3GetContentHashMixin,
	S3MediaLocationMixin,
	S3PrivateMixin,
	S3BotoStorage):
	pass
	class S3UniquePublicStorage(
	UniqueMixin,
	S3GetContentHashMixin,
	S3MediaLocationMixin,
	S3PublicMixin,
	S3BotoStorage):
	pass

Speaking of the list of mixins above, note UniqueMixin followed by S3GetContentHashMixin. The only functionality the latter provides is an overridden get_content_hash() method. However, UniqueMixin also overrides get_content_hash(), and doesn't call super() within it, so the functionality of S3GetContentHashMixin is never invoked. I'm not sure what the history of that mixin is and what it's trying to accomplish, so I don't have any specific suggestions other than for someone with more contextual knowledge to review the situation and either make its functionality invokable, or remove it altogether.

[Aramgutang]

Just wanted to note, since it was hard to track down, a reference of the allowed object parameters can be found here: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/s3.html#S3.Object.initiate_multipart_upload

[mrmachine]

@Aramgutang Thanks, this was recently brought to my attention again and I finally got around to looking at it. You are indeed correct, the order of the mixins is reversed. Just switching them so S3GetContentHashMixin is before UniqueMixin should be all that is needed.

You are probably also correct about the expiry settings. However we are now using R2 instead of S3 and I don't think it supports the per object expiry, and we handle this in our Cloudflare worker which provides public and private access to the R2 bucket.

But your suggestion does seem reasonable for S3 buckets.