Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When changing the file system to cloud storage, an error message "The source image file could not be found" always appears at the top of the image preview page. #11946

Closed
twn39 opened this issue May 11, 2024 · 4 comments
Labels
status:Needs Info status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. type:Bug

Comments

@twn39
Copy link

twn39 commented May 11, 2024

Issue Summary

I changed the default file storage system of Wagtail to use Tencent's COS, similar to AWS's S3. When previewing images, an error message always appears at the top.

WX20240511-161523@2x

Steps to Reproduce

  1. Changed the default file storage system to Tencent's COS
  2. Upload a image
  3. View the image
  • I have confirmed that this issue can be reproduced as described on a fresh Wagtail project: (yes)

Technical details

I found that when validating the image path during image preview, it does not include the URL paths for images stored on the web.

file path:wagtail/images/views/images.py:

    if image.is_stored_locally():
        # Give error if image file doesn't exist
        if not os.path.isfile(image.file.path):
            messages.error(
                request,
                _(
                    "The source image file could not be found. Please change the source or delete the image."
                )
                % {"image_title": image.title},
                buttons=[
                    messages.button(
                        reverse("wagtailimages:delete", args=(image.id,)), _("Delete")
                    )
                ],
            )
@twn39 twn39 added status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. type:Bug labels May 11, 2024
@gasman
Copy link
Collaborator

gasman commented May 11, 2024

Hi @twn39 - thanks for the report.

image.is_stored_locally() should return False if the storage backend is not storing files locally - this probably means that the storage backend has implemented a path method when it should really be raising NotImplementedError instead (see https://docs.djangoproject.com/en/5.0/ref/files/storage/#django.core.files.storage.Storage.path).

What package are you using for the storage backend? (Is it this one?)

@twn39
Copy link
Author

twn39 commented May 11, 2024

Hi @gasman

I implemented the COS (Cloud Object Storage) methods myself, and did not use any open-source libraries.

code:

from django.core.files.storage import Storage
from django.conf import settings
from qcloud_cos import CosConfig, CosServiceError
from qcloud_cos import CosS3Client
import logging
import uuid
import pathlib
from django.core.files.base import ContentFile
from django.core.files import File

logger = logging.getLogger(__name__)


class CosCloudStorage(Storage):
    def __init__(self):
        self.secret_id = settings.COS_SECRET_ID
        self.secret_key = settings.COS_SECRET_KEY
        self.region = settings.COS_REGION
        self.token = None
        self.scheme = settings.COS_SCHEME
        self.config = CosConfig(
            Region=self.region,
            SecretId=self.secret_id,
            SecretKey=self.secret_key,
            Token=self.token,
            Scheme=self.scheme
        )
        self.bucket = settings.COS_BUCKET
        self.client = CosS3Client(self.config)

    def _open(self, name, mode='rb'):
        response = self.client.get_object(Bucket=self.bucket, Key=name)
        fp = response['Body'].get_raw_stream()
        content_file = ContentFile(fp.data, name=name)
        return File(content_file)

    def _save(self, name, content):
        path = pathlib.Path(name)
        prefix = path.parts[0]
        file_ext = path.suffix
        if prefix == 'original_images':
            file_path = prefix + '/' + str(uuid.uuid4()) + file_ext
        else:
            file_path = name
        # file_path = name
        self.client.put_object(
            Bucket=self.bucket,
            Body=content.read(),
            Key=file_path,
            EnableMD5=False
        )
        return file_path

    def delete(self, name):
        self.client.delete_object(
            Bucket=self.bucket,
            Key=name
        )

    def exists(self, name):
        return self.client.object_exists(Bucket=self.bucket, Key=name)

    def listdir(self, path):
        pass

    def size(self, name):
        info = self._info(name)
        if info is not None:
            return info['Content-Length']
        return None

    def url(self, name):
        return settings.COS_BASE_URI + name

    def get_accessed_time(self, name):
        pass

    def get_created_time(self, name):
        pass

    def get_modified_time(self, name):
        info = self._info(name)
        if info is not None:
            return info['Last-Modified']
        return None

    def path(self, name):
        return settings.COS_BASE_URI + name

    # return data struct:
    # {'Content-Type': 'image/png', 'Content-Length': '132973', 'Connection': 'keep-alive',
    # 'Date': 'Mon, 26 Jul 2021 07:14:07 GMT', 'ETag': '"d9d11dc01f0fed2fb8788ee247130e99"',
    # 'Last-Modified': 'Mon, 26 Jul 2021 07:13:19 GMT', 'Server': 'tencent-cos',
    # 'x-cos-hash-crc64ecma': '3957965839468282517', 'x-cos-request-id': 'NjBmZTYwYmZfMmM5ZDA4MDlfMTVhZl83Y2U1NjBk'}
    def _info(self, name):
        try:
            response = self.client.head_object(
                Bucket=self.bucket,
                Key=name
            )
            return response
        except CosServiceError:
            return None

COS SDK dependency:

pip install cos-python-sdk-v5

@twn39
Copy link
Author

twn39 commented May 11, 2024

@gasman I think it's reasonable to implement the path method to return an HTTP web path in a cloud storage system, but some libraries might throw a NotImplementedError. I submitted a PR that simply excludes the validation for HTTP web paths. This might not be the best way to fix the issue. 😂

@gasman
Copy link
Collaborator

gasman commented May 11, 2024

You should remove the path method from the CosCloudStorage class, as this is not following the API as described in the Django documentation:

For storage systems that aren’t accessible from the local filesystem, this will raise NotImplementedError instead.

@gasman gasman closed this as not planned Won't fix, can't repro, duplicate, stale May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:Needs Info status:Unconfirmed Issue, usually a bug, that has not yet been validated as a confirmed problem. type:Bug
Projects
None yet
2 participants