Skip to content

Commit

Permalink
Fix diagnostics not being complete on version 6 (#2892)
Browse files Browse the repository at this point in the history
  • Loading branch information
ildyria authored Jan 11, 2025
1 parent e138f57 commit 18d2de7
Show file tree
Hide file tree
Showing 34 changed files with 412 additions and 223 deletions.
5 changes: 3 additions & 2 deletions app/Actions/Diagnostics/Errors.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use App\Actions\Diagnostics\Pipes\Checks\SupporterCheck;
use App\Actions\Diagnostics\Pipes\Checks\TimezoneCheck;
use App\Actions\Diagnostics\Pipes\Checks\UpdatableCheck;
use App\DTO\DiagnosticData;
use Illuminate\Pipeline\Pipeline;

class Errors
Expand Down Expand Up @@ -55,14 +56,14 @@ class Errors
*
* @param string[] $skip class names of checks that will be skipped
*
* @return string[] array of messages
* @return DiagnosticData[] array of messages
*/
public function get(array $skip = []): array
{
$filteredPipes = collect($this->pipes);
$this->pipes = $filteredPipes->reject(fn ($p) => in_array((new \ReflectionClass($p))->getShortName(), $skip, true))->all();

/** @var string[] $errors */
/** @var DiagnosticData[] $errors */
$errors = [];

return app(Pipeline::class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Actions\Diagnostics\Pipes\Checks;

use App\Contracts\DiagnosticPipe;
use App\DTO\DiagnosticData;
use App\Models\User;
use Illuminate\Support\Facades\Schema;

Expand All @@ -22,7 +23,7 @@ public function handle(array &$data, \Closure $next): array
$numberOfAdmin = User::query()->where('may_administrate', '=', true)->count();
if ($numberOfAdmin === 0) {
// @codeCoverageIgnoreStart
$data[] = 'Error: User Admin not found in database. Please run: "php lychee:create_user {username} {password}"';
$data[] = DiagnosticData::error('User Admin not found in database. Please run: "php lychee:create_user {username} {password}"', self::class);
// @codeCoverageIgnoreEnd
}

Expand Down
58 changes: 29 additions & 29 deletions app/Actions/Diagnostics/Pipes/Checks/AppUrlMatchCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Actions\Diagnostics\Pipes\Checks;

use App\Contracts\DiagnosticPipe;
use App\DTO\DiagnosticData;
use App\Facades\Helpers;
use Safe\Exceptions\PcreException;
use function Safe\preg_match;
Expand All @@ -27,8 +28,7 @@ public function handle(array &$data, \Closure $next): array
$dir_url = config('app.dir_url');
if (config('app.url') === 'http://localhost') {
// @codeCoverageIgnoreStart
$data[] = 'Warning: APP_URL is still set to default, this will break access to all your images';
$data[] = self::INVISIBLE_WARNING . 'and assets if you are using Lychee behind a sub-domain.';
$data[] = DiagnosticData::warn('APP_URL is still set to default, this will break access to all your images and assets if you are using Lychee behind a sub-domain.', self::class);
// @codeCoverageIgnoreEnd
}

Expand All @@ -40,63 +40,63 @@ public function handle(array &$data, \Closure $next): array

if ($bad !== '') {
// @codeCoverageIgnoreStart
$data[] = sprintf(
'Error: APP_URL (%s) contains a sub-path (%s).',
$censored_app_url,
$censored_bad,
);
$data[] = sprintf(
self::INVISIBLE_ERROR . 'Instead set APP_DIR to (%s) and APP_URL to (%s) in your .env',
$censored_bad,
$censored_current,
$data[] = DiagnosticData::error(
sprintf('APP_URL (%s) contains a sub-path (%s).', $censored_app_url, $censored_bad),
self::class,
[
sprintf('Instead set APP_DIR to (%s) and APP_URL to (%s) in your .env', $censored_bad, $censored_current),
]
);
// @codeCoverageIgnoreEnd
}

if ($bad !== '') {
// @codeCoverageIgnoreStart
$data[] = sprintf(
'Warning: APP_URL (%s) contains a sub-path (%s).',
$censored_app_url,
$censored_bad
$data[] = DiagnosticData::error(
sprintf('APP_URL (%s) contains a sub-path (%s).', $censored_app_url, $censored_bad),
self::class,
['This may impact your WebAuthn authentication.']
);
$data[] = self::INVISIBLE_WARNING . 'This may impact your WebAuthn authentication.';
// @codeCoverageIgnoreEnd
}

if (!$this->checkUrlMatchCurrentHost()) {
// @codeCoverageIgnoreStart
$data[] = sprintf(
'Error: APP_URL (%s) does not match the current url (%s).',
$censored_app_url,
$censored_current,
$data[] = DiagnosticData::error(
sprintf('APP_URL (%s) does not match the current url (%s).', $censored_app_url, $censored_current),
self::class,
['This will break WebAuthn authentication.']
);
$data[] = self::INVISIBLE_ERROR . 'This will break WebAuthn authentication.';
// @codeCoverageIgnoreEnd
}

$config_url_imgage = config('filesystems.disks.images.url');
if ($config_url_imgage === '') {
// @codeCoverageIgnoreStart
$data[] = 'Error: LYCHEE_UPLOADS_URL is set and empty. This will prevent images to be displayed. Remove the line from your .env';
$data[] = DiagnosticData::error(
'LYCHEE_UPLOADS_URL is set and empty. This will prevent images to be displayed. Remove the line from your .env',
self::class
);
// @codeCoverageIgnoreEnd
}

if (!str_starts_with($config_url_imgage, '/') && !str_starts_with($config_url_imgage, 'http')) {
// @codeCoverageIgnoreStart
$data[] = 'Error: LYCHEE_UPLOADS_URL is set but starts with neither a / nor http.';
$data[] = self::INVISIBLE_ERROR . 'This will prevent images from being displayed. Remove the line from your .env';
$data[] = DiagnosticData::error(
'LYCHEE_UPLOADS_URL is set but starts with neither a / nor http.',
self::class,
['This will prevent images from being displayed. Remove the line from your .env']
);
// @codeCoverageIgnoreEnd
}

if (($config_url . $dir_url . '/uploads') === $config_url_imgage && !$this->checkUrlMatchCurrentHost()) {
// @codeCoverageIgnoreStart
$data[] = sprintf(
'Error: APP_URL (%s) does not match the current url (%s).',
$censored_app_url,
$censored_current
$data[] = DiagnosticData::error(
sprintf('APP_URL (%s) does not match the current url (%s).', $censored_app_url, $censored_current),
self::class,
['This will prevent images from being properly displayed.']
);
$data[] = self::INVISIBLE_ERROR . 'This will prevent images from being properly displayed.';
// @codeCoverageIgnoreEnd
}

Expand Down
37 changes: 17 additions & 20 deletions app/Actions/Diagnostics/Pipes/Checks/BasicPermissionCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Actions\Diagnostics\Pipes\Checks;

use App\Contracts\DiagnosticPipe;
use App\DTO\DiagnosticData;
use App\Enum\StorageDiskType;
use App\Exceptions\Handler;
use App\Exceptions\Internal\InvalidConfigOption;
Expand Down Expand Up @@ -71,7 +72,7 @@ public function handle(array &$data, \Closure $next): array
/**
* Check all the folders with the correct permissions.
*
* @param array<int,string> $data
* @param DiagnosticData[] $data
*
* @return void
*/
Expand All @@ -91,7 +92,7 @@ public function folders(array &$data): void
$groupIDsOrFalse = posix_getgroups();
// @codeCoverageIgnoreStart
} catch (PosixException) {
$data[] = 'Error: Could not determine groups of process';
$data[] = DiagnosticData::error('Could not determine groups of process', self::class);

return;
}
Expand Down Expand Up @@ -128,25 +129,25 @@ function (int $gid): string {

if ($this->numOwnerIssues > self::MAX_ISSUE_REPORTS_PER_TYPE) {
// @codeCoverageIgnoreStart
$data[] = sprintf('Warning: %d more directories with wrong owner', $this->numOwnerIssues - self::MAX_ISSUE_REPORTS_PER_TYPE);
$data[] = DiagnosticData::warn(sprintf('%d more directories with wrong owner', $this->numOwnerIssues - self::MAX_ISSUE_REPORTS_PER_TYPE), self::class);
// @codeCoverageIgnoreEnd
}
if ($this->numPermissionIssues > self::MAX_ISSUE_REPORTS_PER_TYPE) {
// @codeCoverageIgnoreStart
$data[] = sprintf('Warning: %d more directories with wrong permissions', $this->numPermissionIssues - self::MAX_ISSUE_REPORTS_PER_TYPE);
$data[] = DiagnosticData::warn(sprintf('%d more directories with wrong permissions', $this->numPermissionIssues - self::MAX_ISSUE_REPORTS_PER_TYPE), self::class);
// @codeCoverageIgnoreEnd
}
if ($this->numAccessIssues > self::MAX_ISSUE_REPORTS_PER_TYPE) {
// @codeCoverageIgnoreStart
$data[] = sprintf('Warning: %d more inaccessible directories', $this->numAccessIssues - self::MAX_ISSUE_REPORTS_PER_TYPE);
$data[] = DiagnosticData::warn(sprintf('%d more inaccessible directories', $this->numAccessIssues - self::MAX_ISSUE_REPORTS_PER_TYPE), self::class);
// @codeCoverageIgnoreEnd
}
}

/**
* Check if user.css has the correct permissions.
*
* @param array<int,string> $data
* @param DiagnosticData[] $data
*
* @return void
*/
Expand All @@ -155,10 +156,11 @@ public function userCSS(array &$data): void
$p = Storage::disk('dist')->path('user.css');
if (!Helpers::hasPermissions($p)) {
// @codeCoverageIgnoreStart
$data[] = sprintf("Warning: '%s' does not exist or has insufficient read/write privileges.", $this->anonymize($p));
$data[] = DiagnosticData::warn(sprintf("'%s' does not exist or has insufficient read/write privileges.", $this->anonymize($p)), self::class);

$p = Storage::disk('dist')->path('');
if (!Helpers::hasPermissions($p)) {
$data[] = sprintf("Warning: '%s' has insufficient read/write privileges.", $this->anonymize($p));
$data[] = DiagnosticData::warn(sprintf("'%s' has insufficient read/write privileges.", $this->anonymize($p)), self::class);
}
// @codeCoverageIgnoreEnd
}
Expand All @@ -170,8 +172,8 @@ public function userCSS(array &$data): void
* For efficiency reasons only the directory permissions are checked,
* not the permissions of every single file.
*
* @param string $path the path of the directory or file to check
* @param string[] $data the list of errors to append to
* @param string $path the path of the directory or file to check
* @param DiagnosticData[] $data the list of errors to append to
*
* @noinspection PhpComposerExtensionStubsInspection
*/
Expand All @@ -186,7 +188,7 @@ private function checkDirectoryPermissionsRecursively(string $path, array &$data
$actualPerm = fileperms($path);
// @codeCoverageIgnoreStart
} catch (FilesystemException) {
$data[] = sprintf('Warning: Unable to determine permissions for %s' . PHP_EOL, $this->anonymize($path));
$data[] = DiagnosticData::warn(sprintf('Unable to determine permissions for %s', $this->anonymize($path)), self::class);

return;
}
Expand Down Expand Up @@ -216,7 +218,7 @@ private function checkDirectoryPermissionsRecursively(string $path, array &$data
// @codeCoverageIgnoreStart
$this->numOwnerIssues++;
if ($this->numOwnerIssues <= self::MAX_ISSUE_REPORTS_PER_TYPE) {
$data[] = sprintf('Warning: %s is owned by group %s, but should be owned by one out of %s', $this->anonymize($path), $owningGroupName, $this->groupNames);
$data[] = DiagnosticData::warn(sprintf('%s is owned by group %s, but should be owned by one out of %s', $this->anonymize($path), $owningGroupName, $this->groupNames), self::class);
}
// @codeCoverageIgnoreEnd
}
Expand All @@ -225,12 +227,7 @@ private function checkDirectoryPermissionsRecursively(string $path, array &$data
// @codeCoverageIgnoreStart
$this->numPermissionIssues++;
if ($this->numPermissionIssues <= self::MAX_ISSUE_REPORTS_PER_TYPE) {
$data[] = sprintf(
'Warning: %s has permissions %04o, but should have %04o',
$this->anonymize($path),
$actualPerm,
$expectedPerm
);
$data[] = DiagnosticData::warn(sprintf('%s has permissions %04o, but should have %04o', $this->anonymize($path), $actualPerm, $expectedPerm), self::class);
}
// @codeCoverageIgnoreEnd
}
Expand All @@ -245,7 +242,7 @@ private function checkDirectoryPermissionsRecursively(string $path, array &$data
!is_readable($path) => 'not readable',
default => '',
};
$data[] = sprintf('Error: %s is %s by %s', $this->anonymize($path), $problem, $this->groupNames);
$data[] = DiagnosticData::error(sprintf('%s is %s by %s', $this->anonymize($path), $problem, $this->groupNames), self::class);
}
// @codeCoverageIgnoreEnd
}
Expand All @@ -258,7 +255,7 @@ private function checkDirectoryPermissionsRecursively(string $path, array &$data
}
// @codeCoverageIgnoreStart
} catch (\Exception $e) {
$data[] = 'Error: ' . $e->getMessage();
$data[] = DiagnosticData::error($e->getMessage(), self::class);
Handler::reportSafely($e);
}
// @codeCoverageIgnoreEnd
Expand Down
41 changes: 6 additions & 35 deletions app/Actions/Diagnostics/Pipes/Checks/ConfigSanityCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Actions\Diagnostics\Pipes\Checks;

use App\Contracts\DiagnosticPipe;
use App\DTO\DiagnosticData;
use App\Models\Configs;
use Illuminate\Support\Facades\Schema;

Expand All @@ -12,9 +13,6 @@
*/
class ConfigSanityCheck implements DiagnosticPipe
{
/** @var array<string,string> */
private array $settings;

/**
* {@inheritDoc}
*/
Expand All @@ -24,59 +22,32 @@ public function handle(array &$data, \Closure $next): array
return $next($data);
}

// Load settings
$this->settings = Configs::get();

$this->checkKeysExistsAndSet($data);

$this->sanity($data);

$this->checkDropBoxKeyWarning($data);

return $next($data);
}

/**
* Check that a certain set of configuration exists in the database.
*
* @param array<int,string> $data
*
* @return void
*/
private function checkKeysExistsAndSet(array &$data): void
{
$keys_checked = [
'sorting_photos_col', 'sorting_albums_col',
'imagick', 'skip_duplicates', 'check_for_updates', 'version',
];

foreach ($keys_checked as $key) {
if (!isset($this->settings[$key])) {
$data[] = 'Error: ' . $key . ' not set in database';
}
}
}

/**
* Warning if the Dropbox key does not exists.
*
* @param array<int,string> $data
* @param DiagnosticData[] $data
*
* @return void
*/
private function checkDropBoxKeyWarning(array &$data): void
{
$dropbox = Configs::getValueAsString('dropbox_key');
if ($dropbox === '') {
$data[] = 'Warning: Dropbox import not working. dropbox_key is empty.';
$data[] = 'Info: To hide this Dropbox warning, set the dropbox_key to "disabled"';
$data[] = DiagnosticData::warn('Dropbox import not working. dropbox_key is empty.', self::class);
$data[] = DiagnosticData::info('To hide this Dropbox warning, set the dropbox_key to "disabled".', self::class);
}
}

/**
* Sanity check of the config.
*
* @param array<int,string> $return
* @param DiagnosticData[] $return
*/
private function sanity(array &$return): void
{
Expand All @@ -85,7 +56,7 @@ private function sanity(array &$return): void
foreach ($configs as $config) {
$message = $config->sanity($config->value);
if ($message !== '') {
$return[] = $message;
$return[] = DiagnosticData::error(str_replace('Error: ', '', $message), self::class);
}
}
}
Expand Down
10 changes: 7 additions & 3 deletions app/Actions/Diagnostics/Pipes/Checks/CountSizeVariantsCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace App\Actions\Diagnostics\Pipes\Checks;

use App\Contracts\DiagnosticPipe;
use App\DTO\DiagnosticData;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;

Expand All @@ -24,11 +25,14 @@ public function handle(array &$data, \Closure $next): array
$num = DB::table('size_variants')->where('size_variants.filesize', '=', 0)->count();
if ($num > 0) {
// @codeCoverageIgnoreStart
$data[] = sprintf('Info: Found %d small images without filesizes.', $num);
$data[] = sprintf(' You can use `php artisan lychee:variant_filesize %d` to compute them.', $num);
$data[] = DiagnosticData::info(
sprintf('Found %d small images without filesizes.', $num),
self::class,
[sprintf('You can use `php artisan lychee:variant_filesize %d` to compute them.', $num)]
);
// @codeCoverageIgnoreEnd
}

return $next($data);
}
}
}
Loading

0 comments on commit 18d2de7

Please sign in to comment.