getXOVER() declared its return type as `array|string|NNTPService`, which
does not include DariusIII\NetNntp\Error. The underlying NNTP client
legitimately returns an Error object whenever the server responds with
an error to an XOVER command (e.g. a group with no matching articles,
or a range past the group's high-water mark) -- both call sites in
BinariesService already guard for exactly this case via
NNTPService::isError($result). Because the declared return type
excluded Error, PHP raised a TypeError before either caller ever got a
chance to run that check:
App\Services\NNTP\NNTPService::getXOVER(): Return value must be of
type App\Services\NNTP\NNTPService|array|string, DariusIII\NetNntp\Error
returned
In practice this crashed the first XOVER call that hit any error
response, which made historical backfill (`update:backfill` /
`multiprocessing:backfill`) unusable beyond the very first successful
chunk for a group -- backfill by its nature keeps requesting older and
older ranges until it walks off the group's actual history, at which
point the server error becomes inevitable.
Every sibling method on this class that the NNTP client can answer with
an Error object (doConnect, doQuit, getOverview, getGroups, getMessages,
getMessagesByMessageID) already declares `mixed` for this same reason.
This change brings getXOVER() in line with that existing convention
rather than introducing a new pattern.
Added a regression test that uses reflection to assert getXOVER()'s
return type permits DariusIII\NetNntp\Error (or is unrestricted via
`mixed`), plus a sanity check that NNTPService::isError() correctly
identifies Error instances. Verified the test fails against the old
`array|string|NNTPService` signature and passes against `mixed`.
Manually verified against a live NNTP server: `update:backfill` on a
real group ran 15+ chunks past the point where it previously crashed on
the very first error response, with no exceptions.
RolesAndPermissionsSeeder grants every 'view *' permission via
Role::givePermissionTo() only -- it never grants permissions directly to a
user with User::givePermissionTo(). That is true for every seeded role,
including Admin.
User::getCategoryExclusionById() computed the allowed permission set as:
$userAllowed = $user->getDirectPermissions()->pluck('name')->toArray();
$roleAllowed = $user->getAllPermissions()->pluck('name')->toArray();
$allowed = array_intersect($roleAllowed, $userAllowed);
getAllPermissions() already includes permissions granted via the user's
role(s), so intersecting it with getDirectPermissions() (permissions
assigned directly to the user, bypassing roles) means $allowed is empty
for any user whose permissions come only from their role. Since every
seeded role works this way, this silently excluded every category root
for every user on a fresh install, and any subsequent Newznab/Torznab API
search or browse request returned zero results with no visible error.
Fix: use getAllPermissions() directly, since it already reflects both
role-granted and directly-granted permissions.
Added a regression test (test_role_only_permissions_are_not_excluded)
that mirrors the real seeder setup -- role-only permissions, nothing
granted directly to the user -- to make sure this doesn't regress.
DariusIII