Skip to content

Commit b0ebe9e

Browse files
imorlandimorland
committed
[1.x] Register model lifecycle listeners on the events dispatcher
The integration classes hooked Eloquent model events via the static Model::event(Closure) API (User::saving, PasswordToken::created, Tag::created, etc.). That binds closures to the model classes' static dispatcher, which cannot be serialized under PHPUnit process isolation on PHP 7.x, causing "Serialization of 'Closure' is not allowed" errors. Register the same listeners on the events dispatcher instead, using the "eloquent.<event>: <Model>" keys those static helpers dispatch under, with named handler methods rather than closures. Behaviour is unchanged. The flarum/flags HasMany::macro is left as a closure: it relies on $this being rebound to the HasMany instance and is registered on the relation class rather than a model instance.
1 parent 5a36865 commit b0ebe9e

5 files changed

Lines changed: 179 additions & 128 deletions

File tree

‎extensions/audit/src/Integration/CoreUserIntegration.php‎

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -68,31 +68,14 @@ public function __invoke(Container $container): void
6868
$events->listen(Event\Registered::class, [$this, 'registered']);
6969
$events->listen(Event\Renamed::class, [$this, 'renamed']);
7070

71-
PasswordToken::created(function (PasswordToken $token) {
72-
$this->log($token->user, 'password_change_requested');
73-
});
74-
75-
LoginProvider::created(function (LoginProvider $provider) {
76-
$this->log($provider->user, 'provider_connected', [
77-
'provider' => $provider->provider,
78-
'identifier' => $provider->identifier,
79-
]);
80-
});
81-
82-
LoginProvider::updated(function (LoginProvider $provider) {
83-
if (Arr::exists($provider->getChanges(), 'last_login_at')) {
84-
$this->log($provider->user, 'logged_in_with_provider', [
85-
'provider' => $provider->provider,
86-
'identifier' => $provider->identifier,
87-
]);
88-
}
89-
});
90-
91-
User::saving(function (User $user) {
92-
// There's no way of accessing the original email from EmailChanged, so we save it beforehand.
93-
// We can't use the core user saving event because it's not dispatched in ConfirmEmailHandler.
94-
$this->originalEmail = $user->getOriginal('email');
95-
});
71+
// These hook Eloquent model lifecycle events. We listen on the events dispatcher
72+
// (rather than the static Model::event(Closure) API) so the listeners aren't bound to
73+
// the model classes' static dispatcher — which can't be serialized under PHPUnit
74+
// process isolation on PHP 7.x.
75+
$events->listen('eloquent.created: '.PasswordToken::class, [$this, 'passwordTokenCreated']);
76+
$events->listen('eloquent.created: '.LoginProvider::class, [$this, 'loginProviderCreated']);
77+
$events->listen('eloquent.updated: '.LoginProvider::class, [$this, 'loginProviderUpdated']);
78+
$events->listen('eloquent.saving: '.User::class, [$this, 'userSaving']);
9679
}
9780

9881
protected function log(User $user, string $action, array $payload = []): void
@@ -102,6 +85,36 @@ protected function log(User $user, string $action, array $payload = []): void
10285
], $payload));
10386
}
10487

88+
public function passwordTokenCreated(PasswordToken $token)
89+
{
90+
$this->log($token->user, 'password_change_requested');
91+
}
92+
93+
public function loginProviderCreated(LoginProvider $provider)
94+
{
95+
$this->log($provider->user, 'provider_connected', [
96+
'provider' => $provider->provider,
97+
'identifier' => $provider->identifier,
98+
]);
99+
}
100+
101+
public function loginProviderUpdated(LoginProvider $provider)
102+
{
103+
if (Arr::exists($provider->getChanges(), 'last_login_at')) {
104+
$this->log($provider->user, 'logged_in_with_provider', [
105+
'provider' => $provider->provider,
106+
'identifier' => $provider->identifier,
107+
]);
108+
}
109+
}
110+
111+
public function userSaving(User $user)
112+
{
113+
// There's no way of accessing the original email from EmailChanged, so we save it beforehand.
114+
// We can't use the core user saving event because it's not dispatched in ConfirmEmailHandler.
115+
$this->originalEmail = $user->getOriginal('email');
116+
}
117+
105118
public function activated(Event\Activated $event)
106119
{
107120
// Do not log anything when enabled via API on creation or via social login

‎extensions/audit/src/Integration/FlagsIntegration.php‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use Flarum\Flags\Flag;
1414
use Flarum\Post\Post;
1515
use Illuminate\Contracts\Container\Container;
16+
use Illuminate\Contracts\Events\Dispatcher;
1617
use Illuminate\Database\Eloquent\Relations\HasMany;
1718

1819
/**
@@ -34,21 +35,14 @@ public function __invoke(Container $container): void
3435
return;
3536
}
3637

37-
Flag::created(function (Flag $flag) {
38-
// We only log flags created manually via the extension.
39-
// We don't log the creation of Approval/Akismet flags.
40-
if ($flag->type !== 'user') {
41-
return;
42-
}
43-
44-
AuditLogger::log('post.flagged', [
45-
'discussion_id' => $flag->post->discussion->id,
46-
'post_id' => $flag->post->id,
47-
'reason' => $flag->reason ?? ($flag->reason_detail ? 'other' : null),
48-
]);
49-
});
38+
// Listen on the events dispatcher rather than the static Flag::created(Closure) API, so the
39+
// listener isn't bound to the model's static dispatcher (not serializable on PHP 7.x).
40+
$container->make(Dispatcher::class)->listen('eloquent.created: '.Flag::class, [$this, 'flagCreated']);
5041

51-
// We don't use the FlagsWillBeDeleted event as extensions might still prevent deletion at that point.
42+
// We don't use the FlagsWillBeDeleted event as extensions might still prevent deletion at that
43+
// point. This macro must stay a closure: it relies on $this being rebound to the HasMany
44+
// instance at call time. It registers on the HasMany class (not a model instance), so it is
45+
// not part of the serialized model graph that tripped the static model-event closures.
5246
HasMany::macro('delete', function () {
5347
/** @var HasMany $this */
5448
$parent = $this->getParent();
@@ -76,4 +70,19 @@ public function __invoke(Container $container): void
7670
return $result;
7771
});
7872
}
73+
74+
public function flagCreated(Flag $flag)
75+
{
76+
// We only log flags created manually via the extension.
77+
// We don't log the creation of Approval/Akismet flags.
78+
if ($flag->type !== 'user') {
79+
return;
80+
}
81+
82+
AuditLogger::log('post.flagged', [
83+
'discussion_id' => $flag->post->discussion->id,
84+
'post_id' => $flag->post->id,
85+
'reason' => $flag->reason ?? ($flag->reason_detail ? 'other' : null),
86+
]);
87+
}
7988
}

‎extensions/audit/src/Integration/FoFUsernameRequestIntegration.php‎

Lines changed: 61 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use Flarum\User\User;
1414
use FoF\UserRequest\UsernameRequest;
1515
use Illuminate\Contracts\Container\Container;
16+
use Illuminate\Contracts\Events\Dispatcher;
1617

1718
/**
1819
* fof/username-request integration.
@@ -46,57 +47,66 @@ public function __invoke(Container $container): void
4647
return;
4748
}
4849

49-
User::updated(function (User $user) {
50-
$this->oldNickname = $user->getOriginal('nickname');
51-
$this->oldUsername = $user->getOriginal('username');
52-
});
50+
// Listen on the events dispatcher rather than the static Model::event(Closure) API, so the
51+
// listeners aren't bound to the model classes' static dispatcher (not serializable on PHP 7.x).
52+
$events = $container->make(Dispatcher::class);
5353

54-
UsernameRequest::saved(function (UsernameRequest $request) {
55-
switch ($request->status) {
56-
case 'Sent':
57-
if ($request->for_nickname) {
58-
AuditLogger::log('user.nickname_requested', [
59-
'user_id' => $request->user_id,
60-
'new_nickname' => $request->requested_username ?: null,
61-
]);
62-
} else {
63-
AuditLogger::log('user.username_requested', [
64-
'user_id' => $request->user_id,
65-
'new_username' => $request->requested_username,
66-
]);
67-
}
68-
break;
69-
case 'Approved':
70-
if ($request->for_nickname) {
71-
AuditLogger::log('user.nickname_request_approved', [
72-
'user_id' => $request->user_id,
73-
'old_nickname' => $this->oldNickname ?: null,
74-
'new_nickname' => $request->requested_username ?: null,
75-
]);
76-
} else {
77-
AuditLogger::log('user.username_request_approved', [
78-
'user_id' => $request->user_id,
79-
'old_username' => $this->oldUsername,
80-
'new_username' => $request->requested_username,
81-
]);
82-
}
83-
break;
84-
case 'Rejected':
85-
if ($request->for_nickname) {
86-
AuditLogger::log('user.nickname_request_rejected', [
87-
'user_id' => $request->user_id,
88-
'new_nickname' => $request->requested_username ?: null,
89-
'reason' => $request->reason,
90-
]);
91-
} else {
92-
AuditLogger::log('user.username_request_rejected', [
93-
'user_id' => $request->user_id,
94-
'new_username' => $request->requested_username,
95-
'reason' => $request->reason,
96-
]);
97-
}
98-
break;
99-
}
100-
});
54+
$events->listen('eloquent.updated: '.User::class, [$this, 'userUpdated']);
55+
$events->listen('eloquent.saved: '.UsernameRequest::class, [$this, 'requestSaved']);
56+
}
57+
58+
public function userUpdated(User $user)
59+
{
60+
$this->oldNickname = $user->getOriginal('nickname');
61+
$this->oldUsername = $user->getOriginal('username');
62+
}
63+
64+
public function requestSaved(UsernameRequest $request)
65+
{
66+
switch ($request->status) {
67+
case 'Sent':
68+
if ($request->for_nickname) {
69+
AuditLogger::log('user.nickname_requested', [
70+
'user_id' => $request->user_id,
71+
'new_nickname' => $request->requested_username ?: null,
72+
]);
73+
} else {
74+
AuditLogger::log('user.username_requested', [
75+
'user_id' => $request->user_id,
76+
'new_username' => $request->requested_username,
77+
]);
78+
}
79+
break;
80+
case 'Approved':
81+
if ($request->for_nickname) {
82+
AuditLogger::log('user.nickname_request_approved', [
83+
'user_id' => $request->user_id,
84+
'old_nickname' => $this->oldNickname ?: null,
85+
'new_nickname' => $request->requested_username ?: null,
86+
]);
87+
} else {
88+
AuditLogger::log('user.username_request_approved', [
89+
'user_id' => $request->user_id,
90+
'old_username' => $this->oldUsername,
91+
'new_username' => $request->requested_username,
92+
]);
93+
}
94+
break;
95+
case 'Rejected':
96+
if ($request->for_nickname) {
97+
AuditLogger::log('user.nickname_request_rejected', [
98+
'user_id' => $request->user_id,
99+
'new_nickname' => $request->requested_username ?: null,
100+
'reason' => $request->reason,
101+
]);
102+
} else {
103+
AuditLogger::log('user.username_request_rejected', [
104+
'user_id' => $request->user_id,
105+
'new_username' => $request->requested_username,
106+
'reason' => $request->reason,
107+
]);
108+
}
109+
break;
110+
}
101111
}
102112
}

‎extensions/audit/src/Integration/NicknamesIntegration.php‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,26 @@ public function __invoke(Container $container): void
3737
// We need to register the event listener in booted() because that's where Extend\Event
3838
// registers them. Ours should run after Nicknames because of the extension's optional
3939
// dependency tree. The audit extender already defers this callback until booted().
40-
$container->make(Dispatcher::class)->listen(Saving::class, [$this, 'saving']);
40+
$events = $container->make(Dispatcher::class);
4141

42-
// There's no event for the nickname change at this time so we need to use a bit of Eloquent magic.
43-
User::saved(function (User $user) {
44-
// The $originalNickname variable holds the old value but it also signifies that the nickname was updated.
45-
if ($this->originalNickname !== false) {
46-
AuditLogger::log('user.nickname_changed', [
47-
'user_id' => $user->id,
48-
'old_nickname' => $this->originalNickname ?: null,
49-
'new_nickname' => $user->nickname ?: null,
50-
]);
51-
}
52-
});
42+
$events->listen(Saving::class, [$this, 'saving']);
43+
44+
// There's no event for the nickname change at this time so we hook the user saved
45+
// lifecycle. We listen on the dispatcher rather than User::saved(Closure) so the
46+
// listener isn't bound to the model's static dispatcher (not serializable on PHP 7.x).
47+
$events->listen('eloquent.saved: '.User::class, [$this, 'userSaved']);
48+
}
49+
50+
public function userSaved(User $user)
51+
{
52+
// The $originalNickname variable holds the old value but it also signifies that the nickname was updated.
53+
if ($this->originalNickname !== false) {
54+
AuditLogger::log('user.nickname_changed', [
55+
'user_id' => $user->id,
56+
'old_nickname' => $this->originalNickname ?: null,
57+
'new_nickname' => $user->nickname ?: null,
58+
]);
59+
}
5360
}
5461

5562
public function saving(Saving $event)

‎extensions/audit/src/Integration/TagsAdminIntegration.php‎

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Flarum\Audit\AuditLogger;
1313
use Flarum\Tags\Tag;
1414
use Illuminate\Contracts\Container\Container;
15+
use Illuminate\Contracts\Events\Dispatcher;
1516
use Illuminate\Support\Arr;
1617

1718
/**
@@ -33,34 +34,45 @@ public function __invoke(Container $container): void
3334
return;
3435
}
3536

36-
Tag::created(function (Tag $tag) {
37-
AuditLogger::log('tag.created', [
38-
'tag_id' => $tag->id,
39-
]);
40-
});
37+
// Listen on the events dispatcher rather than the static Tag::event(Closure) API, so the
38+
// listeners aren't bound to the model's static dispatcher (not serializable on PHP 7.x).
39+
$events = $container->make(Dispatcher::class);
4140

42-
Tag::updated(function (Tag $tag) {
43-
// If only the following properties were edited, this means we were in UpdateTagMetadata
44-
// and we don't want to log that.
45-
if (count(Arr::except($tag->getChanges(), [
46-
'discussion_count',
47-
'last_posted_at',
48-
'last_posted_discussion_id',
49-
'last_posted_user_id',
50-
'post_count', // Added by askvortsov/flarum-categories extension
51-
])) === 0) {
52-
return;
53-
}
41+
$events->listen('eloquent.created: '.Tag::class, [$this, 'created']);
42+
$events->listen('eloquent.updated: '.Tag::class, [$this, 'updated']);
43+
$events->listen('eloquent.deleted: '.Tag::class, [$this, 'deleted']);
44+
}
45+
46+
public function created(Tag $tag)
47+
{
48+
AuditLogger::log('tag.created', [
49+
'tag_id' => $tag->id,
50+
]);
51+
}
52+
53+
public function updated(Tag $tag)
54+
{
55+
// If only the following properties were edited, this means we were in UpdateTagMetadata
56+
// and we don't want to log that.
57+
if (count(Arr::except($tag->getChanges(), [
58+
'discussion_count',
59+
'last_posted_at',
60+
'last_posted_discussion_id',
61+
'last_posted_user_id',
62+
'post_count', // Added by askvortsov/flarum-categories extension
63+
])) === 0) {
64+
return;
65+
}
5466

55-
AuditLogger::log('tag.updated', [
56-
'tag_id' => $tag->id,
57-
]);
58-
});
67+
AuditLogger::log('tag.updated', [
68+
'tag_id' => $tag->id,
69+
]);
70+
}
5971

60-
Tag::deleted(function (Tag $tag) {
61-
AuditLogger::log('tag.deleted', [
62-
'tag_id' => $tag->id,
63-
]);
64-
});
72+
public function deleted(Tag $tag)
73+
{
74+
AuditLogger::log('tag.deleted', [
75+
'tag_id' => $tag->id,
76+
]);
6577
}
6678
}

0 commit comments

Comments
 (0)