From 4b8c157b6c5e1373be4c3eefef8557e05cc5dd6b Mon Sep 17 00:00:00 2001 From: "Buster \"Silver Eagle\" Neece" Date: Wed, 30 Jan 2019 23:25:55 -0600 Subject: [PATCH] Code cleanup from static analysis. --- phpstan.neon | 19 +++++ src/Acl.php | 8 +- src/ApiUtilities.php | 4 +- src/Auth.php | 76 +++++++++---------- src/Controller/Admin/IndexController.php | 12 +-- src/Controller/Api/AbstractCrudController.php | 19 ++--- .../Api/AbstractGenericCrudController.php | 8 +- .../Api/Admin/SettingsController.php | 6 +- src/Controller/Frontend/AccountController.php | 7 +- .../Frontend/DashboardController.php | 4 +- .../Stations/Files/EditController.php | 2 +- .../Stations/Reports/OverviewController.php | 7 +- src/Sync/Task/NowPlaying.php | 13 ++-- 13 files changed, 102 insertions(+), 83 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 142cf86ec..1e1573f98 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,7 +4,26 @@ parameters: paths: - src + fileExtensions: + - php + bootstrap: %rootDir%/../../../util/phpstan.php + universalObjectCratesClasses: + - Azura\Session\NamespaceInterface + - Azura\View + + ignoreErrors: + # Caused by Symfony Validator (perhaps wrongly) returning the interface. + - '#Cannot cast Symfony\\Component\\Validator\\ConstraintViolationListInterface to string.#' + + # Supervisor functions that do exist but aren't annotated properly in the connector lib. + - '#Call to an undefined method Supervisor\\Supervisor::reloadConfig().#' + - '#Call to an undefined method Supervisor\\Supervisor::tailProcessLog().#' + - '#Call to an undefined method Supervisor\\Supervisor::signalProcess().#' + + # PHPDocs + - '#PHPDoc tag .*#' + includes: - vendor/phpstan/phpstan-doctrine/extension.neon diff --git a/src/Acl.php b/src/Acl.php index 8dcda6a2e..02a312653 100644 --- a/src/Acl.php +++ b/src/Acl.php @@ -50,7 +50,7 @@ class Acl * Check if a specified User entity is allowed to perform an action (or array of actions). * * @param Entity\User|null $user - * @param $action + * @param string|array $action * @param null $station_id * @return bool */ @@ -142,7 +142,7 @@ class Acl * Wrapper around the 'userAllowed' function that throws a UI-friendly exception upon failure. * * @param Entity\User|null $user - * @param $action + * @param string|array $action * @param null $station_id * @throws Exception\NotLoggedIn * @throws Exception\PermissionDenied @@ -199,8 +199,8 @@ class Acl } /** - * @param $permission_name - * @param $is_global + * @param string $permission_name + * @param bool $is_global * @return bool */ public static function isValidPermission($permission_name, $is_global): bool diff --git a/src/ApiUtilities.php b/src/ApiUtilities.php index fc53b7aa3..3645e29f4 100644 --- a/src/ApiUtilities.php +++ b/src/ApiUtilities.php @@ -40,8 +40,8 @@ class ApiUtilities /** * Get the album art URL for a given unique StationMedia identifier. * - * @param $station_id - * @param $media_unique_id + * @param int $station_id + * @param string $media_unique_id * @param UriInterface|null $base_url * @return UriInterface */ diff --git a/src/Auth.php b/src/Auth.php index 2b02d74e0..faeaf7b98 100644 --- a/src/Auth.php +++ b/src/Auth.php @@ -15,10 +15,10 @@ class Auth protected $_user_repo; /** @var User|null */ - protected $_user = null; + protected $_user; /** @var User|null */ - protected $_masqueraded_user = null; + protected $_masqueraded_user; public function __construct(Session $session, UserRepository $user_repo) { @@ -31,24 +31,23 @@ class Auth /** * Authenticate a given username and password combination against the User repository. * - * @param $username - * @param $password - * @return bool + * @param string $username + * @param string $password + * @return User|null */ - public function authenticate($username, $password) + public function authenticate($username, $password): ?User { $user_auth = $this->_user_repo->authenticate($username, $password); if ($user_auth instanceof User) { $this->setUser($user_auth); - - return true; - } else { - return false; + return $user_auth; } + + return null; } - public function logout() + public function logout(): void { unset($this->_session->user_id); unset($this->_session->masquerade_user_id); @@ -63,7 +62,7 @@ class Auth * * @return bool */ - public function isLoggedIn() + public function isLoggedIn(): bool { if (APP_IS_COMMAND_LINE && !APP_TESTING_MODE) { return false; @@ -78,33 +77,32 @@ class Auth * Get the currently logged in user. * * @param bool $real_user_only - * @return bool|User|null|object + * @return User|null * @throws \Azura\Exception */ - public function getLoggedInUser($real_user_only = false) + public function getLoggedInUser($real_user_only = false): ?User { - if ($this->isMasqueraded() && !$real_user_only) { + if (!$real_user_only && $this->isMasqueraded()) { return $this->getMasquerade(); - } else { - return $this->getUser(); } + + return $this->getUser(); } /** * Get the authenticated user entity. * - * @return bool|User|null + * @return User|null * @throws \Azura\Exception */ - public function getUser() + public function getUser(): ?User { if ($this->_user === null) { $user_id = (int)$this->_session->user_id; - if ($user_id == 0) { + if (0 === $user_id) { $this->_user = false; - - return false; + return null; } $user = $this->_user_repo->find($user_id); @@ -119,25 +117,20 @@ class Auth } } - return $this->_user; + return ($this->_user instanceof User) + ? $this->_user + : null; } /** * Set the currently authenticated user. * * @param User $user - * @return bool */ - public function setUser(User $user) + public function setUser(User $user): void { - // Prevent any previous identity from being used. - // @session_regenerate_id(true); - $this->_session->user_id = $user->getId(); - $this->_user = $user; - - return true; } /** @@ -147,14 +140,18 @@ class Auth /** * Become a different user across the application. * - * @param $user_info + * @param array $user_info */ - public function masqueradeAsUser($user_info) + public function masqueradeAsUser($user_info): void { if (!($user_info instanceof User)) { $user_info = $this->_user_repo->findOneBy($user_info); } + if (!($user_info instanceof User)) { + throw new \Azura\Exception('Invalid user!'); + } + $this->_session->masquerade_user_id = $user_info->getId(); $this->_masqueraded_user = $user_info; } @@ -162,7 +159,7 @@ class Auth /** * Return to the regular authenticated account. */ - public function endMasquerade() + public function endMasquerade(): void { unset($this->_session->masquerade_user_id); $this->_masqueraded_user = null; @@ -173,7 +170,7 @@ class Auth * * @return User|null */ - public function getMasquerade() + public function getMasquerade(): ?User { return $this->_masqueraded_user; } @@ -183,11 +180,10 @@ class Auth * * @return bool */ - public function isMasqueraded() + public function isMasqueraded(): bool { if (!$this->isLoggedIn()) { $this->_masqueraded_user = false; - return false; } @@ -196,7 +192,7 @@ class Auth $this->_masqueraded_user = false; } else { $mask_user_id = (int)$this->_session->masquerade_user_id; - if ($mask_user_id != 0) { + if (0 !== $mask_user_id) { $user = $this->_user_repo->find($mask_user_id); } else { $user = null; @@ -205,9 +201,7 @@ class Auth if ($user instanceof User) { $this->_masqueraded_user = $user; } else { - unset($this->_session->user_id); - unset($this->_session->masquerade_user_id); - + unset($this->_session->user_id, $this->_session->masquerade_user_id); $this->_masqueraded_user = false; } } diff --git a/src/Controller/Admin/IndexController.php b/src/Controller/Admin/IndexController.php index 0437529fe..dacfa9e6b 100644 --- a/src/Controller/Admin/IndexController.php +++ b/src/Controller/Admin/IndexController.php @@ -55,25 +55,24 @@ class IndexController public function syncAction(Request $request, Response $response, $type): ResponseInterface { $view = $request->getView(); - $view->sidebar = null; $handler = new TestHandler(Logger::DEBUG, false); $this->logger->pushHandler($handler); switch ($type) { - case "long": + case 'long': $this->sync->syncLong(true); break; - case "medium": + case 'medium': $this->sync->syncMedium(true); break; - case "short": + case 'short': $this->sync->syncShort(true); break; - case "nowplaying": + case 'nowplaying': default: $this->sync->syncNowplaying(true); break; @@ -82,7 +81,8 @@ class IndexController $this->logger->popHandler(); return $view->renderToResponse($response, 'system/log_view', [ - 'title' => __('Sync Task Output'), + 'sidebar' => null, + 'title' => __('Sync Task Output'), 'log_records' => $handler->getRecords(), ]); } diff --git a/src/Controller/Api/AbstractCrudController.php b/src/Controller/Api/AbstractCrudController.php index 4780453a7..cd1c1523b 100644 --- a/src/Controller/Api/AbstractCrudController.php +++ b/src/Controller/Api/AbstractCrudController.php @@ -7,6 +7,7 @@ use Doctrine\ORM\Query; use Symfony\Component\Serializer\Normalizer\AbstractNormalizer; use Symfony\Component\Serializer\Normalizer\ObjectNormalizer; use Symfony\Component\Serializer\Serializer; +use Symfony\Component\Validator\ConstraintViolationList; use Symfony\Component\Validator\Validator\ValidatorInterface; abstract class AbstractCrudController @@ -39,7 +40,7 @@ abstract class AbstractCrudController } /** - * @param $record + * @param object $record * @param Router $router * @return mixed */ @@ -71,7 +72,7 @@ abstract class AbstractCrudController } /** - * @param $object + * @param object $object * @return mixed */ protected function _displayShortenedObject($object) @@ -84,8 +85,8 @@ abstract class AbstractCrudController } /** - * @param $data - * @param null $record + * @param array $data + * @param object|null $record * @return object * @throws \App\Exception\Validation * @throws \Doctrine\ORM\ORMException @@ -101,8 +102,8 @@ abstract class AbstractCrudController } /** - * @param $data - * @param $record + * @param array $data + * @param object $record * @return object * @throws \App\Exception\Validation * @throws \Doctrine\ORM\ORMException @@ -130,8 +131,8 @@ abstract class AbstractCrudController } /** - * @param $data - * @param $record + * @param array $data + * @param object $record * @param array $context */ protected function _denormalizeToRecord($data, $record, array $context = []): void @@ -142,7 +143,7 @@ abstract class AbstractCrudController } /** - * @param $record + * @param object $record * @throws \Doctrine\ORM\ORMException * @throws \Doctrine\ORM\OptimisticLockException */ diff --git a/src/Controller/Api/AbstractGenericCrudController.php b/src/Controller/Api/AbstractGenericCrudController.php index 45ec75e12..b521eee6f 100644 --- a/src/Controller/Api/AbstractGenericCrudController.php +++ b/src/Controller/Api/AbstractGenericCrudController.php @@ -50,7 +50,7 @@ abstract class AbstractGenericCrudController extends AbstractCrudController /** * @param Request $request * @param Response $response - * @param $record_id + * @param mixed $record_id * @return ResponseInterface */ public function getAction(Request $request, Response $response, $record_id): ResponseInterface @@ -64,7 +64,7 @@ abstract class AbstractGenericCrudController extends AbstractCrudController /** * @param Request $request * @param Response $response - * @param $record_id + * @param mixed $record_id * @return ResponseInterface */ public function editAction(Request $request, Response $response, $record_id): ResponseInterface @@ -85,7 +85,7 @@ abstract class AbstractGenericCrudController extends AbstractCrudController /** * @param Request $request * @param Response $response - * @param $record_id + * @param mixed $record_id * @return ResponseInterface */ public function deleteAction(Request $request, Response $response, $record_id): ResponseInterface @@ -104,7 +104,7 @@ abstract class AbstractGenericCrudController extends AbstractCrudController } /** - * @param $record_id + * @param mixed $record_id * @return object|null * @throws \Doctrine\ORM\ORMException * @throws \Doctrine\ORM\OptimisticLockException diff --git a/src/Controller/Api/Admin/SettingsController.php b/src/Controller/Api/Admin/SettingsController.php index 744d35c47..d9aa81e06 100644 --- a/src/Controller/Api/Admin/SettingsController.php +++ b/src/Controller/Api/Admin/SettingsController.php @@ -40,9 +40,11 @@ class SettingsController $this->serializer = $serializer; $this->validator = $validator; - $this->settings_repo = $this->em->getRepository(Entity\Settings::class); + /** @var Entity\Repository\SettingsRepository $settings_repo */ + $settings_repo = $em->getRepository(Entity\Settings::class); - $all_settings = $this->settings_repo->fetchAll(); + $this->settings_repo = $settings_repo; + $all_settings = $settings_repo->fetchAll(); $this->api_settings = $this->serializer->denormalize($all_settings, Entity\Api\Admin\Settings::class); } diff --git a/src/Controller/Frontend/AccountController.php b/src/Controller/Frontend/AccountController.php index a01934fe8..644173eec 100644 --- a/src/Controller/Frontend/AccountController.php +++ b/src/Controller/Frontend/AccountController.php @@ -76,10 +76,9 @@ class AccountController return $response->withRedirect($request->getUri()->getPath()); } - $login_success = $this->auth->authenticate($_POST['username'], $_POST['password']); - - if ($login_success) { + $user = $this->auth->authenticate($_POST['username'], $_POST['password']); + if ($user instanceof User) { // Regenerate session ID $session->regenerate(); @@ -87,8 +86,6 @@ class AccountController $this->acl->reload(); // Persist user as database entity. - $user = $this->auth->getLoggedInUser(); - $this->em->persist($user); $this->em->flush(); diff --git a/src/Controller/Frontend/DashboardController.php b/src/Controller/Frontend/DashboardController.php index f7a0624e0..6ef5ee9bb 100644 --- a/src/Controller/Frontend/DashboardController.php +++ b/src/Controller/Frontend/DashboardController.php @@ -87,8 +87,8 @@ class DashboardController } // Get administrator notifications. - $notification_event = $this->dispatcher->dispatch(GetNotifications::NAME, new GetNotifications($user)); - + $notification_event = new GetNotifications($user); + $this->dispatcher->dispatch(GetNotifications::NAME, $notification_event); $notifications = $notification_event->getNotifications(); diff --git a/src/Controller/Stations/Files/EditController.php b/src/Controller/Stations/Files/EditController.php index 01183888b..cf4205dc3 100644 --- a/src/Controller/Stations/Files/EditController.php +++ b/src/Controller/Stations/Files/EditController.php @@ -91,7 +91,7 @@ class EditController extends FilesControllerAbstract // Detect rename. if ($data['path'] !== $media->getPath()) { $path_full = 'media://'.$data['path']; - $fs->rename($media->getFullPath(), $path_full); + $fs->rename($media->getPathUri(), $path_full); } if (!empty($custom_fields)) { diff --git a/src/Controller/Stations/Reports/OverviewController.php b/src/Controller/Stations/Reports/OverviewController.php index 38df17480..2f7c1094a 100644 --- a/src/Controller/Stations/Reports/OverviewController.php +++ b/src/Controller/Stations/Reports/OverviewController.php @@ -116,9 +116,13 @@ class OverviewController // Compile the above data. $song_totals = []; + + /** @var Entity\Repository\SongRepository $song_repo */ + $song_repo = $this->em->getRepository(Entity\Song::class); + foreach ($song_totals_raw as $total_type => $total_records) { foreach ($total_records as $total_record) { - $song = $this->em->getRepository(Entity\Song::class)->findAsArray($total_record['song_id']); + $song = $song_repo->findAsArray($total_record['song_id']); $total_record['song'] = $song; $song_totals[$total_type][] = $total_record; @@ -128,7 +132,6 @@ class OverviewController } /* Song "Deltas" (Changes in Listener Count) */ - $threshold = strtotime('-2 weeks'); // Get all songs played in timeline. diff --git a/src/Sync/Task/NowPlaying.php b/src/Sync/Task/NowPlaying.php index 16cde873d..0952798b1 100644 --- a/src/Sync/Task/NowPlaying.php +++ b/src/Sync/Task/NowPlaying.php @@ -81,12 +81,15 @@ class NowPlaying extends AbstractTask implements EventSubscriberInterface $this->event_dispatcher = $event_dispatcher; $this->influx = $influx; - $this->history_repo = $this->em->getRepository(Entity\SongHistory::class); - $this->song_repo = $this->em->getRepository(Entity\Song::class); - $this->listener_repo = $this->em->getRepository(Entity\Listener::class); + $this->history_repo = $em->getRepository(Entity\SongHistory::class); + $this->song_repo = $em->getRepository(Entity\Song::class); + $this->listener_repo = $em->getRepository(Entity\Listener::class); - $this->settings_repo = $this->em->getRepository(Entity\Settings::class); - $this->analytics_level = $this->settings_repo->getSetting('analytics', Entity\Analytics::LEVEL_ALL); + /** @var Entity\Repository\SettingsRepository $settings_repo */ + $settings_repo = $em->getRepository(Entity\Settings::class); + + $this->settings_repo = $settings_repo; + $this->analytics_level = $settings_repo->getSetting('analytics', Entity\Analytics::LEVEL_ALL); } public static function getSubscribedEvents()