starred/flyimg

Fork 0

mirror of https://github.com/flyimg/flyimg.git synced 2026-04-25 09:45:50 +03:00

[GH-ISSUE #533] S3 Portable Visibility Converter for Flyssystem #193

New issue

Closed

opened 2026-02-25 22:34:35 +03:00 by kerem · 4 comments

kerem commented

2026-02-25 22:34:35 +03:00

Owner

Originally created by @sitesurfer on GitHub (Oct 22, 2024).
Original GitHub issue: https://github.com/flyimg/flyimg/issues/533

Originally assigned to: @sadok-f on GitHub.

I have been looking through the code with reference to the AWS S3 side of things. Having successfully gotten this all to work with Digital Ocean Spaces - using the optional setting for endpoint - I turn to wanting to be able to make all files public as we are using the DO CDN as well.

So far we have great success with the uploads, and viewing via the normal route. However it strikes us that using the 'path' option and storing this value in our db would potentially reduce url complexity and a small amount of latency (its not a lot but every little helps I feel).

However - the files are stored into DO with a private flag. In other apps I have set the visibility on the Laravel S3 adapter to 'public' and all works fine, perusing the flysystem docs brings me to the League\Flysystem\AwsS3V3\PortableVisibilityConverter which can be added into the service Provider setup and provide the correct settings to make the bucket public.

I've tried amending the service provider, checking first that the correct converter is actually in the docker image (it is), but have been so far unsuccessful in getting past a class loading error.

Is there a possibility that we could have an optional config item that simply placed this in/out of the service provider as required?
Or if I am barking up the wrong tree, how has the public issue been addressed previously? Nothing in the issues as far as I can see after a search.

Reference: https://flysystem.thephpleague.com/docs/adapter/aws-s3-v3/

Originally created by @sitesurfer on GitHub (Oct 22, 2024). Original GitHub issue: https://github.com/flyimg/flyimg/issues/533 Originally assigned to: @sadok-f on GitHub. I have been looking through the code with reference to the AWS S3 side of things. Having successfully gotten this all to work with Digital Ocean Spaces - using the optional setting for endpoint - I turn to wanting to be able to make all files public as we are using the DO CDN as well. So far we have great success with the uploads, and viewing via the normal route. However it strikes us that using the 'path' option and storing this value in our db would potentially reduce url complexity and a small amount of latency (its not a lot but every little helps I feel). However - the files are stored into DO with a private flag. In other apps I have set the visibility on the Laravel S3 adapter to 'public' and all works fine, perusing the flysystem docs brings me to the League\Flysystem\AwsS3V3\PortableVisibilityConverter which can be added into the service Provider setup and provide the correct settings to make the bucket public. I've tried amending the service provider, checking first that the correct converter is actually in the docker image (it is), but have been so far unsuccessful in getting past a class loading error. Is there a possibility that we could have an optional config item that simply placed this in/out of the service provider as required? Or if I am barking up the wrong tree, how has the public issue been addressed previously? Nothing in the issues as far as I can see after a search. Reference: https://flysystem.thephpleague.com/docs/adapter/aws-s3-v3/

kerem

2026-02-25 22:34:35 +03:00

closed this issue
added the
enhancement

question
labels

kerem commented

2026-02-25 22:34:35 +03:00

Author

Owner

@sitesurfer commented on GitHub (Oct 22, 2024):

Its probably worth mentioning that I personally have gotten around this by using s3cmd via cli and providing a bucket policy. It might just be a neat feature for users in the future.

@sitesurfer commented on GitHub (Oct 22, 2024): Its probably worth mentioning that I personally have gotten around this by using s3cmd via cli and providing a bucket policy. It might just be a neat feature for users in the future.

kerem commented

2026-02-25 22:34:35 +03:00

Author

Owner

@sadok-f commented on GitHub (Oct 23, 2024):

Thank you @sitesurfer for suggesting this feature.
yes you're right, Flyimg is missing some of the optional settings when it comes to S3.
I'm currently working to include those options as part of the AwsS3V3Adapter instantiation:

    // Optional path prefix
    'path/prefix',
    // Visibility converter (League\Flysystem\AwsS3V3\VisibilityConverter)
    new League\Flysystem\AwsS3V3\PortableVisibilityConverter(
        // Optional default for directories
        League\Flysystem\Visibility::PUBLIC // or ::PRIVATE
    )

@sadok-f commented on GitHub (Oct 23, 2024): Thank you @sitesurfer for suggesting this feature. yes you're right, Flyimg is missing some of the optional settings when it comes to S3. I'm currently working to include those options as part of the AwsS3V3Adapter instantiation: ``` // Optional path prefix 'path/prefix', // Visibility converter (League\Flysystem\AwsS3V3\VisibilityConverter) new League\Flysystem\AwsS3V3\PortableVisibilityConverter( // Optional default for directories League\Flysystem\Visibility::PUBLIC // or ::PRIVATE ) ```

kerem commented

2026-02-25 22:34:35 +03:00

Author

Owner

@sadok-f commented on GitHub (Oct 24, 2024):

Hi @sitesurfer
in our latest release 1.5.0 we added a new optional parameters to the s3 storage option such as path_prefix and visibility
can you please check if this matches your needs?
Thanks!

@sadok-f commented on GitHub (Oct 24, 2024): Hi @sitesurfer in our latest release [1.5.0](https://github.com/flyimg/flyimg/releases/tag/1.5.0) we added a new optional parameters to the s3 storage option such as `path_prefix` and `visibility` can you please check if this matches your needs? Thanks!

kerem commented

2026-02-25 22:34:35 +03:00

Author

Owner

@sitesurfer commented on GitHub (Oct 24, 2024):

Thats absolutely amazing, it makes so much sense when compared to the normal usage in flysystem. Thanks for this - a lot!

@sitesurfer commented on GitHub (Oct 24, 2024): Thats absolutely amazing, it makes so much sense when compared to the normal usage in flysystem. Thanks for this - a lot!

kerem referenced this issue

2026-02-25 22:35:06 +03:00

[PR #193] [MERGED] issue-#191: mage path parameters is ignored, closes #191 #327

kerem referenced this issue

2026-03-15 15:43:38 +03:00

[PR #193] [MERGED] issue-#191: mage path parameters is ignored, closes #191 #933