Skip to content

Commit 5018a49

Browse files
committed
refactor: modernise package for PHP 8.4 — final classes, readonly, typed properties
final: - Subscriber, SubscribableServiceProvider, FakeSubscriber - UnsubscribeController, LegacyUnsubscribeController - UserUnsubscribed, UserUnsubscribing SubscribableMailMessage and the SubscribableNotification trait are left open — the composable design depends on subclassing. readonly / constructor property promotion: - Events use public readonly constructor promotion; $user type corrected from the misleading Illuminate\Foundation\Auth\User to object - UnsubscribeController and LegacyUnsubscribeController use private readonly - Subscriber promotes $app to private readonly Typed callable properties: - Subscriber's five handler properties are now private \Closure|null - parseHandler() returns \Closure via \Closure::fromCallable(), using Str::parseCallback()'s second argument for the default method name - Invocation uses first-class callable style: ($this->onFoo)($args) Other cleanup: - $hander typo corrected to $handler throughout - Missing return types added (routeName, checkSubscriptionStatus, etc.) - Protected visibility on Subscriber/FakeSubscriber internals tightened to private (no subclassing possible on final classes) - Redundant @param/@return docblocks removed where signatures suffice - SubscribableServiceProvider::register() simplified to a short closure - FakeSubscriber gains assertCheckedSubscriptionStatus() so tests that previously used Subscriber::shouldReceive() (incompatible with final) can still verify argument routing via the fake https://claude.ai/code/session_01R4pAjWwGY8xKspsdU8xnsy
1 parent 50f5f5d commit 5018a49

10 files changed

Lines changed: 89 additions & 216 deletions

src/Controllers/LegacyUnsubscribeController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
use Illuminate\Routing\Controller;
77
use YlsIdeas\SubscribableNotifications\Subscriber;
88

9-
class LegacyUnsubscribeController extends Controller
9+
final class LegacyUnsubscribeController extends Controller
1010
{
1111
public function __construct(private readonly Subscriber $subscriber)
1212
{
1313
$this->middleware('signed');
1414
}
1515

16-
public function __invoke(Request $request, $subscriberId, ?string $mailingList = null)
16+
public function __invoke(Request $request, mixed $subscriberId, ?string $mailingList = null): mixed
1717
{
1818
return app(UnsubscribeController::class)(
1919
$request,

src/Controllers/UnsubscribeController.php

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,43 +4,20 @@
44

55
use Illuminate\Database\Eloquent\Relations\Relation;
66
use Illuminate\Http\Request;
7-
use Illuminate\Http\Response;
87
use Illuminate\Routing\Controller;
98
use YlsIdeas\SubscribableNotifications\Contracts\CanUnsubscribe;
109
use YlsIdeas\SubscribableNotifications\Events\UserUnsubscribed;
1110
use YlsIdeas\SubscribableNotifications\Events\UserUnsubscribing;
1211
use YlsIdeas\SubscribableNotifications\Subscriber;
1312

14-
/**
15-
* Class UnsubscribeController.
16-
*/
17-
class UnsubscribeController extends Controller
13+
final class UnsubscribeController extends Controller
1814
{
19-
/**
20-
* @var Subscriber
21-
*/
22-
protected $subscriber;
23-
24-
/**
25-
* UnsubscribeController constructor.
26-
* @param Subscriber $subscriber
27-
*/
28-
public function __construct(Subscriber $subscriber)
15+
public function __construct(private readonly Subscriber $subscriber)
2916
{
3017
$this->middleware('signed');
31-
$this->subscriber = $subscriber;
3218
}
3319

34-
/**
35-
* Handle the incoming request.
36-
*
37-
* @param Request $request
38-
* @param string $subscriberType
39-
* @param mixed $subscriberId
40-
* @param string|null $mailingList
41-
* @return Response
42-
*/
43-
public function __invoke(Request $request, string $subscriberType, $subscriberId, ?string $mailingList = null)
20+
public function __invoke(Request $request, string $subscriberType, mixed $subscriberId, ?string $mailingList = null): mixed
4421
{
4522
$modelClass = Relation::getMorphedModel($subscriberType) ?? $subscriberType;
4623

@@ -68,7 +45,6 @@ public function __invoke(Request $request, string $subscriberType, $subscriberId
6845

6946
event(new UserUnsubscribed($subscriber, $mailingList));
7047

71-
// RFC 8058: one-click POST must not redirect; return 204 No Content
7248
if ($request->isMethod('post')) {
7349
return response('', 204);
7450
}

src/Events/UserUnsubscribed.php

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,10 @@
22

33
namespace YlsIdeas\SubscribableNotifications\Events;
44

5-
use Illuminate\Foundation\Auth\User;
6-
7-
class UserUnsubscribed
5+
final class UserUnsubscribed
86
{
9-
/**
10-
* @var User
11-
*/
12-
public $user;
13-
/**
14-
* @var string|null
15-
*/
16-
public $mailingList;
17-
18-
public function __construct($user, ?string $mailingList = null)
19-
{
20-
$this->user = $user;
21-
$this->mailingList = $mailingList;
22-
}
7+
public function __construct(
8+
public readonly object $user,
9+
public readonly ?string $mailingList = null,
10+
) {}
2311
}

src/Events/UserUnsubscribing.php

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,10 @@
22

33
namespace YlsIdeas\SubscribableNotifications\Events;
44

5-
use Illuminate\Foundation\Auth\User;
6-
7-
class UserUnsubscribing
5+
final class UserUnsubscribing
86
{
9-
/**
10-
* @var User
11-
*/
12-
public $user;
13-
/**
14-
* @var string|null
15-
*/
16-
public $mailingList;
17-
18-
public function __construct($user, ?string $mailingList = null)
19-
{
20-
$this->user = $user;
21-
$this->mailingList = $mailingList;
22-
}
7+
public function __construct(
8+
public readonly object $user,
9+
public readonly ?string $mailingList = null,
10+
) {}
2311
}

src/Facades/Subscriber.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*/
2727
class Subscriber extends Facade
2828
{
29-
protected static function getFacadeAccessor()
29+
protected static function getFacadeAccessor(): string
3030
{
3131
return \YlsIdeas\SubscribableNotifications\Subscriber::class;
3232
}

src/MailSubscriber.php

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@
99

1010
trait MailSubscriber
1111
{
12-
/**
13-
* @param string|null $mailingList
14-
* @return string
15-
*/
1612
public function unsubscribeLink(?string $mailingList = null): string
1713
{
1814
return URL::signedRoute(
@@ -25,10 +21,6 @@ public function unsubscribeLink(?string $mailingList = null): string
2521
);
2622
}
2723

28-
/**
29-
* @param Notification $notification
30-
* @return bool
31-
*/
3224
public function mailSubscriptionStatus(Notification $notification): bool
3325
{
3426
$list = $notification instanceof AppliesToMailingList

src/SubscribableServiceProvider.php

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,13 @@
22

33
namespace YlsIdeas\SubscribableNotifications;
44

5-
use Illuminate\Contracts\Foundation\Application;
65
use Illuminate\Notifications\Events\NotificationSending;
76
use Illuminate\Support\Facades\Event;
87
use Illuminate\Support\ServiceProvider;
98
use YlsIdeas\SubscribableNotifications\Contracts\CheckNotifiableSubscriptionStatus;
109
use YlsIdeas\SubscribableNotifications\Contracts\CheckSubscriptionStatusBeforeSendingNotifications;
1110

12-
class SubscribableServiceProvider extends ServiceProvider
11+
final class SubscribableServiceProvider extends ServiceProvider
1312
{
1413
public function boot(): void
1514
{
@@ -40,9 +39,6 @@ public function boot(): void
4039

4140
public function register(): void
4241
{
43-
$this->app->singleton(Subscriber::class, function (Application $app) {
44-
/** @phpstan-ignore-next-line */
45-
return new Subscriber($app);
46-
});
42+
$this->app->singleton(Subscriber::class, fn () => new Subscriber($this->app));
4743
}
4844
}

src/Subscriber.php

Lines changed: 36 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -4,57 +4,35 @@
44

55
use Illuminate\Contracts\Foundation\Application;
66
use Illuminate\Database\Eloquent\Relations\Relation;
7-
use Illuminate\Http\Response;
87
use Illuminate\Support\Str;
98

10-
class Subscriber
9+
final class Subscriber
1110
{
12-
public $uri = 'unsubscribe/{subscriberType}/{subscriberId}/{mailingList?}';
13-
public $hander = '\YlsIdeas\SubscribableNotifications\Controllers\UnsubscribeController';
14-
public $routeName = 'unsubscribe';
11+
public string $uri = 'unsubscribe/{subscriberType}/{subscriberId}/{mailingList?}';
12+
public string $handler = '\YlsIdeas\SubscribableNotifications\Controllers\UnsubscribeController';
13+
public string $routeName = 'unsubscribe';
1514

1615
/** Morph type stored when legacyRoutes() is called — used by LegacyUnsubscribeController. */
1716
public ?string $legacySubscriberType = null;
18-
/**
19-
* @var callable
20-
*/
21-
protected $onUnsubscribeFromMailingList;
22-
/**
23-
* @var callable
24-
*/
25-
protected $onUnsubscribeFromAllMailingLists;
26-
/**
27-
* @var callable
28-
*/
29-
protected $onCompletion;
30-
/**
31-
* @var callable
32-
*/
33-
protected $onCheckSubscriptionStatusForMailingLists;
34-
/**
35-
* @var callable
36-
*/
37-
protected $onCheckSubscriptionStatusForAllMailingLists;
38-
39-
/**
40-
* @var Application
41-
*/
42-
protected $app;
43-
44-
public function __construct(Application $app)
45-
{
46-
$this->app = $app;
47-
}
4817

49-
public function routes($router = null): void
18+
private ?\Closure $onUnsubscribeFromMailingList = null;
19+
private ?\Closure $onUnsubscribeFromAllMailingLists = null;
20+
private ?\Closure $onCompletion = null;
21+
private ?\Closure $onCheckSubscriptionStatusForMailingLists = null;
22+
private ?\Closure $onCheckSubscriptionStatusForAllMailingLists = null;
23+
24+
public function __construct(private readonly Application $app)
25+
{}
26+
27+
public function routes(mixed $router = null): void
5028
{
5129
$router = $router ?? $this->app->make('router');
52-
$router->match(['GET', 'POST'], $this->uri, $this->hander)
30+
$router->match(['GET', 'POST'], $this->uri, $this->handler)
5331
->name($this->routeName)
5432
->where('subscriberType', '[^\d/][^/]*');
5533
}
5634

57-
public function legacyRoutes(string $defaultModel, $router = null): void
35+
public function legacyRoutes(string $defaultModel, mixed $router = null): void
5836
{
5937
$router = $router ?? $this->app->make('router');
6038

@@ -68,110 +46,69 @@ public function legacyRoutes(string $defaultModel, $router = null): void
6846
)->name($this->routeName . '.legacy');
6947
}
7048

71-
/**
72-
* @return string
73-
*/
74-
public function routeName()
49+
public function routeName(): string
7550
{
7651
return $this->routeName;
7752
}
7853

79-
/**
80-
* @param string|callable $handler
81-
* @throws \Illuminate\Contracts\Container\BindingResolutionException
82-
*/
83-
public function onUnsubscribeFromMailingList($handler)
54+
public function onUnsubscribeFromMailingList(string|callable $handler): void
8455
{
8556
$this->onUnsubscribeFromMailingList = $this->parseHandler($handler);
8657
}
8758

88-
/**
89-
* @param string|callable $handler
90-
* @throws \Illuminate\Contracts\Container\BindingResolutionException
91-
*/
92-
public function onUnsubscribeFromAllMailingLists($handler)
59+
public function onUnsubscribeFromAllMailingLists(string|callable $handler): void
9360
{
9461
$this->onUnsubscribeFromAllMailingLists = $this->parseHandler($handler);
9562
}
9663

97-
/**
98-
* @param string|callable $handler
99-
* @throws \Illuminate\Contracts\Container\BindingResolutionException
100-
*/
101-
public function onCompletion($handler)
64+
public function onCompletion(string|callable $handler): void
10265
{
10366
$this->onCompletion = $this->parseHandler($handler);
10467
}
10568

106-
/**
107-
* @param string|callable $handler
108-
* @throws \Illuminate\Contracts\Container\BindingResolutionException
109-
*/
110-
public function onCheckSubscriptionStatusOfAllMailingLists($handler)
69+
public function onCheckSubscriptionStatusOfAllMailingLists(string|callable $handler): void
11170
{
11271
$this->onCheckSubscriptionStatusForAllMailingLists = $this->parseHandler($handler);
11372
}
11473

115-
/**
116-
* @param string|callable $handler
117-
* @throws \Illuminate\Contracts\Container\BindingResolutionException
118-
*/
119-
public function onCheckSubscriptionStatusOfMailingList($handler)
74+
public function onCheckSubscriptionStatusOfMailingList(string|callable $handler): void
12075
{
12176
$this->onCheckSubscriptionStatusForMailingLists = $this->parseHandler($handler);
12277
}
12378

124-
/**
125-
* @param mixed $user
126-
* @param string $mailingList
127-
*/
128-
public function unsubscribeFromMailingList($user, string $mailingList)
79+
public function unsubscribeFromMailingList(mixed $user, string $mailingList): void
12980
{
130-
call_user_func($this->onUnsubscribeFromMailingList, $user, $mailingList);
81+
($this->onUnsubscribeFromMailingList)($user, $mailingList);
13182
}
13283

133-
/**
134-
* @param mixed $user
135-
*/
136-
public function unsubscribeFromAllMailingLists($user)
84+
public function unsubscribeFromAllMailingLists(mixed $user): void
13785
{
138-
call_user_func($this->onUnsubscribeFromAllMailingLists, $user);
86+
($this->onUnsubscribeFromAllMailingLists)($user);
13987
}
14088

141-
/**
142-
* @param mixed $user
143-
* @param string|null $mailingList
144-
* @return Response
145-
*/
146-
public function complete($user, ?string $mailingList = null)
89+
public function complete(mixed $user, ?string $mailingList = null): mixed
14790
{
148-
return call_user_func($this->onCompletion, $user, $mailingList);
91+
return ($this->onCompletion)($user, $mailingList);
14992
}
15093

151-
/**
152-
* @param mixed $user
153-
* @param string|null $mailingList
154-
* @return bool
155-
*/
156-
public function checkSubscriptionStatus($user, ?string $mailingList = null)
94+
public function checkSubscriptionStatus(mixed $user, ?string $mailingList = null): bool
15795
{
15896
if ($mailingList !== null) {
159-
return (bool) call_user_func($this->onCheckSubscriptionStatusForAllMailingLists, $user) &&
160-
(bool) call_user_func($this->onCheckSubscriptionStatusForMailingLists, $user, $mailingList);
97+
return (bool) ($this->onCheckSubscriptionStatusForAllMailingLists)($user)
98+
&& (bool) ($this->onCheckSubscriptionStatusForMailingLists)($user, $mailingList);
16199
}
162100

163-
return (bool) call_user_func($this->onCheckSubscriptionStatusForAllMailingLists, $user);
101+
return (bool) ($this->onCheckSubscriptionStatusForAllMailingLists)($user);
164102
}
165103

166-
protected function parseHandler(string|callable$handler): callable
104+
private function parseHandler(string|callable $handler): \Closure
167105
{
168106
if (is_string($handler)) {
169-
$parsed = Str::parseCallback($handler);
170-
$parsed[0] = $this->app->make($parsed[0]);
107+
[$class, $method] = Str::parseCallback($handler, '__invoke');
171108

172-
return $parsed;
109+
return \Closure::fromCallable([$this->app->make($class), $method]);
173110
}
174111

175-
return $handler;
112+
return \Closure::fromCallable($handler);
176113
}
177114
}

0 commit comments

Comments
 (0)