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.
Fix 5 instances of stripos('literal', ) where arguments were
reversed, causing the needle to be searched inside a short literal
instead of the literal inside the variable:
- Line 599: stripos('dlc', ) → stripos(, 'dlc') — DLC
branch was never entered for real game titles
- Line 601: stripos('Rock Band Network', ) → stripos(, ...)
— Rock Band check was dead code inside the dead DLC branch
- Line 603: strpos('-', ) → str_contains(, '-') — DLC
hyphen splitting never triggered
- Line 622: stripos('PSX2PSP', ) → stripos(, ...)
— worked by accident (PSX is prefix of PSX2PSP)
- Line 626: stripos('XBLA', ) + stripos('dlc', ) →
stripos(, ...) + stripos(, ...)
Also adds ConsoleServiceDlcParsingTest (9 tests) covering DLC title
parsing, Rock Band Network handling, hyphen splitting, and XBLA
platform upgrade.
DariusIII