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] Allows Unit & Backed Enums for registering named RateLimiter & RateLimited middleware #52935

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

sethsandaru
Copy link
Contributor

Hi team,

This PR enables us to use both BackedEnum & UnitEnum to register named RateLimiter, and the RateLimited middleware.

I believe many of us really want to use Enum for this case and avoid using hardcoded strings across our codebase.

image

@devfrey
Copy link
Contributor

devfrey commented Sep 26, 2024

What problem does this solve, as opposed to using string constants?

class GlobalRateLimitEnum
{
    public const HUBSPOT = 'hubspot';
}

public function middleware(): array
{
    return [
        new RateLimited(GlobalRateLimitEnum::HUBSPOT),
    ];
}

Exact same syntax, same "type safety" (or lack thereof), and requires no changes the framework.

@sethsandaru
Copy link
Contributor Author

sethsandaru commented Sep 26, 2024

it doesn't solve any typing problems here. It gives devs the flexibility to use Enum 🤷‍♂️ depending on people's taste I guess. Many parts of Laravel already support Enum.

And I'm a fan of UnitEnum for this (BackedEnum - nice to have). And enums are special, don't treat them like a simple place to hold strings or integers, they can do more than that.

===

Going forward, I can even do something like this:
enum GlobalNamedLimiter
{
    case API; // short and simple

    public function getLimit(): Closure
    {
        return fn () => match ($this) {
            self::API => Limit::perMinute(100)
        };
    }
}

RateLimiter::for(
    GlobalNamedLimiter::API, 
    GlobalNamedLimiter::API->getLimit() // return () => Limit::perMinute(100);
);

or even this

public function middleware(): array
{
    return [GlobalNamedLimiter::API->middleware()];
}

🤷‍♂️

@taylorotwell taylorotwell merged commit ead6020 into laravel:11.x Sep 27, 2024
30 of 31 checks passed
@sethsandaru sethsandaru deleted the 11.x-accept-enum-rate-limit branch September 28, 2024 02:11
@siarheipashkevich
Copy link
Contributor

siarheipashkevich commented Oct 7, 2024

@sethsandaru what about supporting enum for Illuminate\Support\Fluent?

https://laravel.com/docs/11.x/validation#complex-conditional-array-validation

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.

4 participants