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

Add public for ->writeToDisk() as option #534

Merged
merged 10 commits into from
Jan 24, 2024
Merged

Conversation

bmckay959
Copy link
Contributor

I'm using S3 for hosting my sitemaps(Using Vapor for the site so local disk isn't an option).

I'm currently generating the sitemap and after creation, I'm then changing the visibility to public. This could cause, though rare, a race condition of me creating the sitemap and before making it public, a search engine crawler would receive a "Access Denied" error from S3.

By adding the visibility option as seen here(https://laravel.com/docs/10.x/filesystem#file-visibility), it resolves this issue.

Add visibility option for sitemap index.
Add public private visibility option
Update SitemapIndex.php For S3 Public Storage Option
Copy link
Member

@freekmurze freekmurze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add tests and docs for this change?

src/Sitemap.php Outdated Show resolved Hide resolved
@bmckay959 bmckay959 marked this pull request as draft January 19, 2024 13:38
@bmckay959 bmckay959 marked this pull request as ready for review January 19, 2024 13:51
@freekmurze freekmurze merged commit 20e0cc1 into spatie:main Jan 24, 2024
3 checks passed
@freekmurze
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants