From b7da3e98ac859cfad6a5ef3145962919d86fe0bb Mon Sep 17 00:00:00 2001 From: Matias Griese Date: Wed, 9 May 2018 12:24:01 +0300 Subject: [PATCH] Code cleanup --- classes/Twig/AdminTwigExtension.php | 7 +- classes/adminbasecontroller.php | 156 ++++++++++++++-------------- classes/gpm.php | 22 ++-- classes/popularity.php | 30 +++--- classes/utils.php | 4 +- twig/AdminTwigExtension.php | 6 +- 6 files changed, 111 insertions(+), 114 deletions(-) diff --git a/classes/Twig/AdminTwigExtension.php b/classes/Twig/AdminTwigExtension.php index 838c3e0d..516ff2ba 100644 --- a/classes/Twig/AdminTwigExtension.php +++ b/classes/Twig/AdminTwigExtension.php @@ -3,6 +3,7 @@ namespace Grav\Plugin\Admin\Twig; use Grav\Common\Grav; use Grav\Common\Language\Language; +use Grav\Common\Page\Page; use Symfony\Component\Yaml\Yaml; use Symfony\Component\Yaml\Parser; @@ -47,7 +48,7 @@ class AdminTwigExtension extends \Twig_Extension return clone $obj; } - public function getPageUrl($context, $page) + public function getPageUrl($context, Page $page) { $page_route = trim($page->rawRoute(), '/'); $page_lang = $page->language(); @@ -55,7 +56,7 @@ class AdminTwigExtension extends \Twig_Extension $base_url_simple = $context['base_url_simple']; $admin_lang = Grav::instance()['session']->admin_lang ?: 'en'; - if ($page_lang && $page_lang != $admin_lang) { + if ($page_lang && $page_lang !== $admin_lang) { $page_url = $base_url_simple . '/' . $page_lang . '/' . $context['admin_route'] . '/pages/' . $page_route; } else { $page_url = $base_url . '/pages/' . $page_route; @@ -169,7 +170,7 @@ class AdminTwigExtension extends \Twig_Extension $periods[$j] = $this->grav['admin']->translate($periods[$j], null, true); - return "$difference $periods[$j] {$tense}"; + return "{$difference} {$periods[$j]} {$tense}"; } } diff --git a/classes/adminbasecontroller.php b/classes/adminbasecontroller.php index 5fed86f6..5803a91c 100644 --- a/classes/adminbasecontroller.php +++ b/classes/adminbasecontroller.php @@ -69,14 +69,14 @@ class AdminBaseController protected $redirectCode; protected $upload_errors = [ - 0 => "There is no error, the file uploaded with success", - 1 => "The uploaded file exceeds the max upload size", - 2 => "The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML", - 3 => "The uploaded file was only partially uploaded", - 4 => "No file was uploaded", - 6 => "Missing a temporary folder", - 7 => "Failed to write file to disk", - 8 => "A PHP extension stopped the file upload" + 0 => 'There is no error, the file uploaded with success', + 1 => 'The uploaded file exceeds the max upload size', + 2 => 'The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML', + 3 => 'The uploaded file was only partially uploaded', + 4 => 'No file was uploaded', + 6 => 'Missing a temporary folder', + 7 => 'Failed to write file to disk', + 8 => 'A PHP extension stopped the file upload' ]; /** @var array */ @@ -89,7 +89,7 @@ class AdminBaseController */ public function execute() { - if (in_array($this->view, $this->blacklist_views)) { + if (in_array($this->view, $this->blacklist_views, true)) { return false; } @@ -101,7 +101,7 @@ class AdminBaseController if (method_exists($this, $method)) { try { - $success = call_user_func([$this, $method]); + $success = $this->{$method}(); } catch (\RuntimeException $e) { $success = true; $this->admin->setMessage($e->getMessage(), 'error'); @@ -125,30 +125,43 @@ class AdminBaseController protected function validateNonce() { - if (method_exists('Grav\Common\Utils', 'getNonce')) { - if (strtolower($_SERVER['REQUEST_METHOD']) == 'post') { - if (isset($this->post['admin-nonce'])) { - $nonce = $this->post['admin-nonce']; - } else { - $nonce = $this->grav['uri']->param('admin-nonce'); + if (strtolower($_SERVER['REQUEST_METHOD']) === 'post') { + if (isset($this->post['admin-nonce'])) { + $nonce = $this->post['admin-nonce']; + } else { + $nonce = $this->grav['uri']->param('admin-nonce'); + } + + if (!$nonce || !Utils::verifyNonce($nonce, 'admin-form')) { + if ($this->task === 'addmedia') { + + $message = sprintf($this->admin->translate('PLUGIN_ADMIN.FILE_TOO_LARGE', null), + ini_get('post_max_size')); + + //In this case it's more likely that the image is too big than POST can handle. Show message + $this->admin->json_response = [ + 'status' => 'error', + 'message' => $message + ]; + + return false; } - if (!$nonce || !Utils::verifyNonce($nonce, 'admin-form')) { - if ($this->task == 'addmedia') { + $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN'), 'error'); + $this->admin->json_response = [ + 'status' => 'error', + 'message' => $this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN') + ]; - $message = sprintf($this->admin->translate('PLUGIN_ADMIN.FILE_TOO_LARGE', null), - ini_get('post_max_size')); - - //In this case it's more likely that the image is too big than POST can handle. Show message - $this->admin->json_response = [ - 'status' => 'error', - 'message' => $message - ]; - - return false; - } - - $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN'), 'error'); + return false; + } + unset($this->post['admin-nonce']); + } else { + if ($this->task === 'logout') { + $nonce = $this->grav['uri']->param('logout-nonce'); + if (null === $nonce || !Utils::verifyNonce($nonce, 'logout-form')) { + $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN'), + 'error'); $this->admin->json_response = [ 'status' => 'error', 'message' => $this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN') @@ -156,32 +169,17 @@ class AdminBaseController return false; } - unset($this->post['admin-nonce']); } else { - if ($this->task == 'logout') { - $nonce = $this->grav['uri']->param('logout-nonce'); - if (!isset($nonce) || !Utils::verifyNonce($nonce, 'logout-form')) { - $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN'), - 'error'); - $this->admin->json_response = [ - 'status' => 'error', - 'message' => $this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN') - ]; + $nonce = $this->grav['uri']->param('admin-nonce'); + if (null === $nonce || !Utils::verifyNonce($nonce, 'admin-form')) { + $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN'), + 'error'); + $this->admin->json_response = [ + 'status' => 'error', + 'message' => $this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN') + ]; - return false; - } - } else { - $nonce = $this->grav['uri']->param('admin-nonce'); - if (!isset($nonce) || !Utils::verifyNonce($nonce, 'admin-form')) { - $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN'), - 'error'); - $this->admin->json_response = [ - 'status' => 'error', - 'message' => $this->admin->translate('PLUGIN_ADMIN.INVALID_SECURITY_TOKEN') - ]; - - return false; - } + return false; } } } @@ -209,7 +207,7 @@ class AdminBaseController */ public function taskFilesUpload() { - if (!$this->authorizeTask('save', $this->dataPermissions()) || !isset($_FILES)) { + if (null === $_FILES || !$this->authorizeTask('save', $this->dataPermissions())) { return false; } @@ -230,7 +228,7 @@ class AdminBaseController $filename = trim($upload->file->name); // Handle bad filenames. - if (strtr($filename, "\t\n\r\0\x0b", '_____') !== $filename || rtrim($filename, ". ") !== $filename || preg_match('|\.php|', $filename)) { + if (strtr($filename, "\t\n\r\0\x0b", '_____') !== $filename || rtrim($filename, '. ') !== $filename || preg_match('|\.php|', $filename)) { $this->admin->json_response = [ 'status' => 'error', 'message' => sprintf($this->admin->translate('PLUGIN_ADMIN.FILEUPLOAD_UNABLE_TO_UPLOAD', null), @@ -591,8 +589,8 @@ class AdminBaseController // now the first 4 chars of base contain the lang code. // if redirect path already contains the lang code, and is != than the base lang code, then use redirect path as-is - if (Utils::pathPrefixedByLangCode($base) && Utils::pathPrefixedByLangCode($this->redirect) && substr($base, - 0, 4) != substr($this->redirect, 0, 4) + if (Utils::pathPrefixedByLangCode($base) && Utils::pathPrefixedByLangCode($this->redirect) + && 0 !== strpos($this->redirect, substr($base, 0, 4)) ) { $redirect = $this->redirect; } else { @@ -666,7 +664,7 @@ class AdminBaseController if (is_array($source)) { foreach ($source as $key => $value) { - $key = str_replace('%5B', '[', str_replace('%5D', ']', $key)); + $key = str_replace(['%5B', '%5D'], ['[', ']'], $key); if (is_array($value)) { $out[$key] = $this->cleanDataKeys($value); } else { @@ -710,11 +708,11 @@ class AdminBaseController unset($files[$destination]['tmp_name']); } - if ($this->view == 'pages') { + if ($this->view === 'pages') { $keys = explode('.', preg_replace('/^header./', '', $key)); $init_key = array_shift($keys); if (count($keys) > 0) { - $new_data = isset($obj->header()->$init_key) ? $obj->header()->$init_key : []; + $new_data = isset($obj->header()->{$init_key}) ? $obj->header()->{$init_key} : []; Utils::setDotNotation($new_data, implode('.', $keys), $files, true); } else { $new_data = $files; @@ -745,7 +743,7 @@ class AdminBaseController return false; } - $data = $this->view == 'pages' ? $this->admin->page(true) : $this->prepareData([]); + $data = $this->view === 'pages' ? $this->admin->page(true) : $this->prepareData([]); $settings = $data->blueprints()->schema()->getProperty($this->post['name']); if (isset($settings['folder'])) { @@ -755,7 +753,7 @@ class AdminBaseController } // Do not use self@ outside of pages - if ($this->view != 'pages' && in_array($folder, ['@self', 'self@'])) { + if ($this->view !== 'pages' && in_array($folder, ['@self', 'self@', '@self@'])) { $this->admin->json_response = [ 'status' => 'error', 'message' => sprintf($this->admin->translate('PLUGIN_ADMIN.FILEUPLOAD_PREVENT_SELF', null), $folder) @@ -863,7 +861,7 @@ class AdminBaseController $this->taskRemoveMedia(); - if ($type == 'pages') { + if ($type === 'pages') { $page = $this->admin->page(true, $proute); $keys = explode('.', preg_replace('/^header./', '', $field)); $header = (array)$page->header(); @@ -878,8 +876,8 @@ class AdminBaseController $page->save(); } else { - $blueprint_prefix = $type == 'config' ? '' : $type . '.'; - $blueprint_name = str_replace('/blueprints', '', str_replace('config/', '', $blueprint)); + $blueprint_prefix = $type === 'config' ? '' : $type . '.'; + $blueprint_name = str_replace(['config/', '/blueprints'], '', $blueprint); $blueprint_field = $blueprint_prefix . $blueprint_name . '.' . $field; $files = $this->grav['config']->get($blueprint_field); @@ -941,7 +939,7 @@ class AdminBaseController $fileParts = pathinfo($filename); foreach (scandir($fileParts['dirname']) as $file) { - $regex_pattern = "/" . preg_quote($fileParts['filename']) . "@\d+x\." . $fileParts['extension'] . "(?:\.meta\.yaml)?$|" . preg_quote($fileParts['basename']) . "\.meta\.yaml$/"; + $regex_pattern = '/' . preg_quote($fileParts['filename'], '/') . "@\d+x\." . $fileParts['extension'] . "(?:\.meta\.yaml)?$|" . preg_quote($fileParts['basename'], '/') . "\.meta\.yaml$/"; if (preg_match($regex_pattern, $file)) { $path = $fileParts['dirname'] . '/' . $file; @unlink($path); @@ -963,18 +961,18 @@ class AdminBaseController } return true; - } else { - if ($this->grav['uri']->extension() === 'json') { - $this->admin->json_response = [ - 'status' => 'success', - 'message' => $this->admin->translate('PLUGIN_ADMIN.REMOVE_FAILED') - ]; - } else { - $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.REMOVE_FAILED'), 'error'); - } - - return false; } + + if ($this->grav['uri']->extension() === 'json') { + $this->admin->json_response = [ + 'status' => 'success', + 'message' => $this->admin->translate('PLUGIN_ADMIN.REMOVE_FAILED') + ]; + } else { + $this->admin->setMessage($this->admin->translate('PLUGIN_ADMIN.REMOVE_FAILED'), 'error'); + } + + return false; } /** diff --git a/classes/gpm.php b/classes/gpm.php index 79052399..acddb088 100644 --- a/classes/gpm.php +++ b/classes/gpm.php @@ -111,13 +111,13 @@ class Gpm throw new \RuntimeException($msg); } - if (count($packages) == 1) { + if (count($packages) === 1) { $message = Installer::getMessage(); if ($message) { return $message; - } else { - $messages .= $message; } + + $messages .= $message; } } @@ -173,7 +173,7 @@ class Gpm // Check destination Installer::isValidDestination($location); - if (Installer::lastErrorCode() === Installer::IS_LINK && !$options['ignore_symlinks']) { + if (!$options['ignore_symlinks'] && Installer::lastErrorCode() === Installer::IS_LINK) { return false; } @@ -185,7 +185,7 @@ class Gpm throw new \RuntimeException($msg); } - if (count($packages) == 1) { + if (count($packages) === 1) { $message = Installer::getMessage(); if ($message) { return $message; @@ -236,7 +236,7 @@ class Gpm return Admin::translate('PLUGIN_ADMIN.NOT_VALID_GRAV_PACKAGE'); } - if ($type == 'grav') { + if ($type === 'grav') { Installer::isValidDestination(GRAV_ROOT . '/system'); if (Installer::IS_LINK === Installer::lastErrorCode()) { Folder::delete($tmp_source); @@ -258,14 +258,14 @@ class Gpm $is_update = file_exists($install_path); Installer::isValidDestination(GRAV_ROOT . DS . $install_path); - if (Installer::lastErrorCode() == Installer::IS_LINK) { + if (Installer::lastErrorCode() === Installer::IS_LINK) { Folder::delete($tmp_source); Folder::delete($tmp_zip); return Admin::translate('PLUGIN_ADMIN.CANNOT_OVERWRITE_SYMLINKS'); } Installer::install($zip, GRAV_ROOT, - ['install_path' => $install_path, 'theme' => (($type == 'theme')), 'is_update' => $is_update], + ['install_path' => $install_path, 'theme' => $type === 'theme', 'is_update' => $is_update], $extracted); } @@ -378,10 +378,6 @@ class Gpm Folder::delete($tmp); - if ($errorCode & (Installer::ZIP_OPEN_ERROR | Installer::ZIP_EXTRACT_ERROR)) { - return false; - } - - return true; + return !($errorCode & (Installer::ZIP_OPEN_ERROR | Installer::ZIP_EXTRACT_ERROR)); } } diff --git a/classes/popularity.php b/classes/popularity.php index 12ecd05a..54d2f0eb 100644 --- a/classes/popularity.php +++ b/classes/popularity.php @@ -56,7 +56,7 @@ class Popularity $relative_url = str_replace(Grav::instance()['base_url_relative'], '', $page->url()); // Don't track error pages or pages that have no route - if ($page->template() == 'error' || !$page->route()) { + if ($page->template() === 'error' || !$page->route()) { return; } @@ -92,13 +92,13 @@ class Popularity // get the daily access count if (array_key_exists($day_month_year, $this->daily_data)) { - $this->daily_data[$day_month_year] = intval($this->daily_data[$day_month_year]) + 1; + $this->daily_data[$day_month_year] = (int)$this->daily_data[$day_month_year] + 1; } else { $this->daily_data[$day_month_year] = 1; } // keep correct number as set by history - $count = intval($this->config->get('plugins.admin.popularity.history.daily', 30)); + $count = (int)$this->config->get('plugins.admin.popularity.history.daily', 30); $total = count($this->daily_data); if ($total > $count) { @@ -117,7 +117,7 @@ class Popularity $this->daily_data = $this->getData($this->daily_file); } - $limit = intval($this->config->get('plugins.admin.popularity.dashboard.days_of_stats', 7)); + $limit = (int)$this->config->get('plugins.admin.popularity.dashboard.days_of_stats', 7); $chart_data = array_slice($this->daily_data, -$limit, $limit); $labels = []; @@ -144,9 +144,9 @@ class Popularity if (isset($this->daily_data[date(self::DAILY_FORMAT)])) { return $this->daily_data[date(self::DAILY_FORMAT)]; - } else { - return 0; } + + return 0; } /** @@ -163,7 +163,7 @@ class Popularity foreach (array_reverse($this->daily_data) as $daily) { $total += $daily; $day++; - if ($day == 7) { + if ($day === 7) { break; } } @@ -181,9 +181,9 @@ class Popularity } if (isset($this->monthly_data[date(self::MONTHLY_FORMAT)])) { return $this->monthly_data[date(self::MONTHLY_FORMAT)]; - } else { - return 0; } + + return 0; } protected function updateMonthly() @@ -197,13 +197,13 @@ class Popularity // get the monthly access count if (array_key_exists($month_year, $this->monthly_data)) { - $this->monthly_data[$month_year] = intval($this->monthly_data[$month_year]) + 1; + $this->monthly_data[$month_year] = (int)$this->monthly_data[$month_year] + 1; } else { $this->monthly_data[$month_year] = 1; } // keep correct number as set by history - $count = intval($this->config->get('plugins.admin.popularity.history.monthly', 12)); + $count = (int)$this->config->get('plugins.admin.popularity.history.monthly', 12); $total = count($this->monthly_data); $this->monthly_data = array_slice($this->monthly_data, $total - $count, $count); @@ -242,7 +242,7 @@ class Popularity // get the totals for this url if (array_key_exists($url, $this->totals_data)) { - $this->totals_data[$url] = intval($this->totals_data[$url]) + 1; + $this->totals_data[$url] = (int)$this->totals_data[$url] + 1; } else { $this->totals_data[$url] = 1; } @@ -264,7 +264,7 @@ class Popularity $visitors = $this->visitors_data; arsort($visitors); - $count = intval($this->config->get('plugins.admin.popularity.history.visitors', 20)); + $count = (int)$this->config->get('plugins.admin.popularity.history.visitors', 20); $this->visitors_data = array_slice($visitors, 0, $count, true); file_put_contents($this->visitors_file, json_encode($this->visitors_data)); @@ -279,9 +279,9 @@ class Popularity { if (file_exists($path)) { return (array)json_decode(file_get_contents($path), true); - } else { - return []; } + + return []; } diff --git a/classes/utils.php b/classes/utils.php index b6c3f05c..957b2bc8 100644 --- a/classes/utils.php +++ b/classes/utils.php @@ -21,12 +21,12 @@ class Utils public static function findUserByEmail($email) { $account_dir = Grav::instance()['locator']->findResource('account://'); - $files = array_diff(scandir($account_dir), ['.', '..']); + $files = array_diff(scandir($account_dir, SCANDIR_SORT_ASCENDING), ['.', '..']); foreach ($files as $file) { if (strpos($file, '.yaml') !== false) { $user = User::load(trim(substr($file, 0, -5))); - if ($user['email'] == $email) { + if ($user['email'] === $email) { return $user; } } diff --git a/twig/AdminTwigExtension.php b/twig/AdminTwigExtension.php index bee710f3..6e5f3408 100644 --- a/twig/AdminTwigExtension.php +++ b/twig/AdminTwigExtension.php @@ -1,4 +1,6 @@