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

[11.x] Add contextual attributes to resolve drivers #52265

Merged
merged 11 commits into from
Aug 5, 2024
Merged

[11.x] Add contextual attributes to resolve drivers #52265

merged 11 commits into from
Aug 5, 2024

Conversation

ziadoz
Copy link
Contributor

@ziadoz ziadoz commented Jul 25, 2024

I work on an app where we pass multiple filesystems to various controllers and commands, and currently we have to register a contextual binding for each one to supply the appropriate filesystems.

This PR adds a new Storage contextual attribute for resolving storage disks, which can be specified directly on the parameters:

class MyController
{
    public function __construct(#[Storage('foo')] public Filesystem $foo, #[Storage('bar')] public Filesystem $bar)
    {
    }
}

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@ziadoz ziadoz changed the title Add Storage contextual attribute to resolve disks Add contextual attribute to resolve storage disks Jul 25, 2024
@ziadoz ziadoz marked this pull request as ready for review July 25, 2024 09:41
@driesvints driesvints changed the title Add contextual attribute to resolve storage disks [11.x] Add contextual attribute to resolve storage disks Jul 25, 2024
@taylorotwell
Copy link
Member

I think probably the case could be made for having attributes for most of the built-in driver based stuff... wonder if we should add cache and auth attributes to this PR?

@ziadoz
Copy link
Contributor Author

ziadoz commented Jul 30, 2024

@taylorotwell I've added cache, database and log attributes to this PR.

@ollieread mentioned he had plans for auth on the implementation PR, and he's knowledgeable about multi tenancy, so I'll leave those to him if that's ok.

@ollieread
Copy link
Contributor

@ollieread mentioned he had plans for auth on the implementation PR, and he's knowledgeable about multi tenancy, so I'll leave those to him if that's ok.

If you have time to do it now, feel free to do so, especially if you're already doing it.

I was going to add a Guard attribute, and an Authed attribute, where the first injects the whole guard and the second the current user.

@ziadoz
Copy link
Contributor Author

ziadoz commented Jul 30, 2024

@ollieread I've added an Authed and Guard attribute. Hopefully they look sensible.

@ollieread
Copy link
Contributor

@ollieread I've added an Authed and Guard attribute. Hopefully they look sensible.

Looks good to me! Good job 👍

@ziadoz ziadoz changed the title [11.x] Add contextual attribute to resolve storage disks [11.x] Add contextual attributes to resolve drivers Aug 2, 2024
@taylorotwell taylorotwell merged commit b8a6d7c into laravel:11.x Aug 5, 2024
28 of 29 checks passed
@ziadoz ziadoz deleted the storage-container-attr branch August 5, 2024 15:06
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.

3 participants