Improved PHP style

This commit is contained in:
Marcin Kurczewski 2014-09-09 12:34:57 +02:00
parent 553c33b860
commit 45e18a9ba3
20 changed files with 155 additions and 156 deletions

View file

@ -5,10 +5,10 @@ final class AutoLoader
{
public static function init()
{
spl_autoload_register([__CLASS__, '_include']);
spl_autoload_register([__CLASS__, 'includeClassName']);
}
public static function _include($className)
public static function includeClassName($className)
{
if (strpos($className, 'Szurubooru') === false)
return;
@ -19,7 +19,7 @@ final class AutoLoader
else
$className = __DIR__ . DIRECTORY_SEPARATOR . $className;
$className .= '.php';
include $className;
include($className);
}
}

View file

@ -38,14 +38,14 @@ class Config extends \ArrayObject
foreach (explode('.', $section) as $subSection)
{
if (!isset($ptr->$subSection))
$ptr->$subSection = new self();
if (!$ptr->offsetExists($subSection))
$ptr->offsetSet($subSection, new self());
$ptr = $ptr->$subSection;
}
foreach ($value as $sectionKey => $sectionValue)
$ptr->$sectionKey = $sectionValue;
$ptr->offsetSet($sectionKey, $sectionValue);
}
}
}

View file

@ -57,7 +57,7 @@ final class UserController extends AbstractController
$this->privilegeService->assertPrivilege(\Szurubooru\Privilege::REGISTER);
$formData = new \Szurubooru\FormData\RegistrationFormData($this->inputReader);
$user = $this->userService->createUser($formData);
return array_merge((array) $this->userViewProxy->fromEntity($user), ['confirmed' => $user->emailUnconfirmed == null]);
return array_merge((array) $this->userViewProxy->fromEntity($user), ['confirmed' => !$user->emailUnconfirmed]);
}
public function updateUser($userNameOrEmail)

View file

@ -7,6 +7,11 @@ abstract class AbstractViewProxy
public function fromArray($entities)
{
return array_map(function($entity) { return static::fromEntity($entity); }, $entities);
return array_map(
function($entity)
{
return static::fromEntity($entity);
},
$entities);
}
}

View file

@ -44,12 +44,12 @@ abstract class AbstractSearchService
return new \Szurubooru\Dao\SearchResult($searchFilter, $entities, $totalRecords);
}
protected function decorateFilterWithBasicTokens(&$filter, $basicTokens)
protected function decorateFilterWithBasicTokens($filter, $basicTokens)
{
throw new \BadMethodCallException('Not supported');
}
protected function decorateFilterWithComplexTokens(&$filter, $complexTokens)
protected function decorateFilterWithComplexTokens($filter, $complexTokens)
{
throw new \BadMethodCallException('Not supported');
}
@ -76,9 +76,9 @@ abstract class AbstractSearchService
foreach ($tokens as $token)
{
$token = preg_split('/,|\s+/', $token);
if (count($token) == 2)
if (count($token) === 2)
{
$orderDir = $token[1] == 'desc' ? self::ORDER_DESC : self::ORDER_ASC;
$orderDir = $token[1] === 'desc' ? self::ORDER_DESC : self::ORDER_ASC;
$orderToken = $token[0];
}
else

View file

@ -10,22 +10,12 @@ class UserSearchService extends AbstractSearchService
protected function getOrderColumn($token)
{
switch ($token)
{
case 'name':
if ($token === 'name')
return 'name';
case 'registrationDate':
case 'registrationTime':
case 'registered':
case 'joinDate':
case 'joinTime':
case 'joined':
if (in_array($token, ['registrationDate', 'registrationTime', 'registered', 'joinDate', 'joinTime', 'joined']))
return 'registrationTime';
default:
return null;
}
}
}

View file

@ -5,40 +5,53 @@ class EnumHelper
{
public static function accessRankToString($accessRank)
{
switch ($accessRank)
{
case \Szurubooru\Entities\User::ACCESS_RANK_ANONYMOUS: return 'anonymous';
case \Szurubooru\Entities\User::ACCESS_RANK_REGULAR_USER: return 'regularUser';
case \Szurubooru\Entities\User::ACCESS_RANK_POWER_USER: return 'powerUser';
case \Szurubooru\Entities\User::ACCESS_RANK_MODERATOR: return 'moderator';
case \Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR: return 'administrator';
default:
$map =
[
\Szurubooru\Entities\User::ACCESS_RANK_ANONYMOUS => 'anonymous',
\Szurubooru\Entities\User::ACCESS_RANK_REGULAR_USER => 'regularUser',
\Szurubooru\Entities\User::ACCESS_RANK_POWER_USER => 'powerUser',
\Szurubooru\Entities\User::ACCESS_RANK_MODERATOR => 'moderator',
\Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR => 'administrator',
];
if (!isset($map[$accessRank]))
throw new \DomainException('Invalid access rank!');
}
return $map[$accessRank];
}
public static function accessRankFromString($accessRankString)
{
switch (trim(strtolower($accessRankString)))
{
case 'anonymous': return \Szurubooru\Entities\User::ACCESS_RANK_ANONYMOUS;
case 'regularuser': return \Szurubooru\Entities\User::ACCESS_RANK_REGULAR_USER;
case 'poweruser': return \Szurubooru\Entities\User::ACCESS_RANK_POWER_USER;
case 'moderator': return \Szurubooru\Entities\User::ACCESS_RANK_MODERATOR;
case 'administrator': return \Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR;
default:
$map =
[
'anonymous' => \Szurubooru\Entities\User::ACCESS_RANK_ANONYMOUS,
'regularUser' => \Szurubooru\Entities\User::ACCESS_RANK_REGULAR_USER,
'powerUser' => \Szurubooru\Entities\User::ACCESS_RANK_POWER_USER,
'moderator' => \Szurubooru\Entities\User::ACCESS_RANK_MODERATOR,
'administrator' => \Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR,
];
$key = trim(strtolower($accessRankString));
if (!isset($map[$key]))
throw new \DomainException('Unrecognized access rank: ' . $accessRankString);
}
return $map[$key];
}
public static function avatarStyleFromString($avatarStyleString)
{
switch (trim(strtolower($avatarStyleString)))
{
case 'gravatar': return \Szurubooru\Entities\User::AVATAR_STYLE_GRAVATAR;
case 'manual': return \Szurubooru\Entities\User::AVATAR_STYLE_MANUAL;
case 'none':
case 'blank': return \Szurubooru\Entities\User::AVATAR_STYLE_BLANK;
}
$map =
[
'gravatar' => \Szurubooru\Entities\User::AVATAR_STYLE_GRAVATAR,
'manual' => \Szurubooru\Entities\User::AVATAR_STYLE_MANUAL,
'none' => \Szurubooru\Entities\User::AVATAR_STYLE_BLANK,
'blank' => \Szurubooru\Entities\User::AVATAR_STYLE_BLANK,
];
$key = trim(strtolower($avatarStyleString));
if (!isset($map[$key]))
throw new \DomainException('Unrecognized avatar style: ' . $avatarStyleString);
return $map[$key];
}
}

View file

@ -10,7 +10,7 @@ class HttpHelper
public function setHeader($key, $value)
{
header("$key: $value");
header($key . ': ' . $value);
}
public function output($data)

View file

@ -1,23 +1,27 @@
<?php
namespace Szurubooru\Helpers;
final class InputReader
final class InputReader extends \ArrayObject
{
public function __construct()
{
parent::setFlags(parent::ARRAY_AS_PROPS | parent::STD_PROP_LIST);
$_PUT = [];
if (isset($_SERVER['REQUEST_METHOD']) and $_SERVER['REQUEST_METHOD'] == 'PUT')
if (isset($_SERVER['REQUEST_METHOD']) and $_SERVER['REQUEST_METHOD'] === 'PUT')
parse_str(file_get_contents('php://input'), $_PUT);
foreach ([$_GET, $_POST, $_PUT] as $source)
{
foreach ($source as $key => $value)
$this->$key = $value;
$this->offsetSet($key, $value);
}
}
public function __get($key)
public function offsetGet($index)
{
if (!parent::offsetExists($index))
return null;
return parent::offsetGet($index);
}
}

View file

@ -49,7 +49,7 @@ class AuthService
$this->validateUser($user);
$passwordHash = $this->passwordService->getHash($password);
if ($user->passwordHash != $passwordHash)
if ($user->passwordHash !== $passwordHash)
throw new \InvalidArgumentException('Specified password is invalid.');
$this->loginToken = $this->createAndSaveLoginToken($user);
@ -60,7 +60,7 @@ class AuthService
public function loginFromToken($loginTokenName)
{
$loginToken = $this->tokenService->getByName($loginTokenName);
if ($loginToken->purpose != \Szurubooru\Entities\Token::PURPOSE_LOGIN)
if ($loginToken->purpose !== \Szurubooru\Entities\Token::PURPOSE_LOGIN)
throw new \Exception('This token is not a login token.');
$user = $this->userService->getById($loginToken->additionalData);

View file

@ -19,6 +19,7 @@ class FileService
$daysToLive = isset($options->daysToLive)
? $options->daysToLive
: 7;
$secondsToLive = $daysToLive * 24 * 60 * 60;
$lastModified = filemtime($finalSource);
$eTag = md5(file_get_contents($finalSource)); //todo: faster
@ -29,7 +30,7 @@ class FileService
$eTagHeader = isset($_SERVER['HTTP_IF_NONE_MATCH'])
? trim($_SERVER['HTTP_IF_NONE_MATCH'], "\" \t\r\n")
: false;
: null;
$this->httpHelper->setHeader('ETag', '"' . $eTag . '"');
$this->httpHelper->setHeader('Last-Modified', gmdate('D, d M Y H:i:s \G\M\T', $lastModified));
@ -38,20 +39,15 @@ class FileService
$this->httpHelper->setHeader('Expires', gmdate('D, d M Y H:i:s \G\M\T', time() + $secondsToLive));
if (isset($options->customFileName))
{
$this->httpHelper->setHeader('Content-Disposition', 'inline; filename="' . $options->customFileName . '"');
}
if (isset($options->mimeType))
{
$this->httpHelper->setHeader('Content-Type', $options->mimeType);
}
else
{
$this->httpHelper->setHeader('Content-Type', mime_content_type($finalSource));
}
$this->httpHelper->setHeader(
'Content-Type',
isset($options->mimeType)
? $options->mimeType
: mime_content_type($finalSource));
if (strtotime($ifModifiedSince) == $lastModified or $eTagHeader == $eTag)
if (strtotime($ifModifiedSince) === $lastModified or $eTagHeader === $eTag)
{
$this->httpHelper->setResponseCode(304);
}

View file

@ -58,16 +58,16 @@ class PrivilegeService
$loggedInUser = $this->authService->getLoggedInUser();
if ($userIdentifier instanceof \Szurubooru\Entities\User)
{
return $loggedInUser->name == $userIdentifier->name;
return $loggedInUser->name === $userIdentifier->name;
}
elseif (is_string($userIdentifier))
{
if ($loggedInUser->email)
{
if ($loggedInUser->email == $userIdentifier)
if ($loggedInUser->email === $userIdentifier)
return true;
}
return $loggedInUser->name == $userIdentifier;
return $loggedInUser->name === $userIdentifier;
}
else
{

View file

@ -59,19 +59,19 @@ class ImageGdThumbnailGenerator implements IThumbnailGenerator
if (($dstHeight / $dstWidth) > ($srcHeight / $srcWidth))
{
$h = $srcHeight;
$w = $h * $dstWidth / $dstHeight;
$cropHeight = $srcHeight;
$cropWidth = $srcHeight * $dstWidth / $dstHeight;
}
else
{
$w = $srcWidth;
$h = $w * $dstHeight / $dstWidth;
$cropWidth = $srcWidth;
$cropHeight = $srcWidth * $dstHeight / $dstWidth;
}
$x = ($srcWidth - $w) / 2;
$y = ($srcHeight - $h) / 2;
$cropX = ($srcWidth - $cropWidth) / 2;
$cropY = ($srcHeight - $cropHeight) / 2;
$dstImage = imagecreatetruecolor($dstWidth, $dstHeight);
imagecopyresampled($dstImage, $srcImage, 0, 0, $x, $y, $dstWidth, $dstHeight, $w, $h);
imagecopyresampled($dstImage, $srcImage, 0, 0, $cropX, $cropY, $dstWidth, $dstHeight, $cropWidth, $cropHeight);
return $dstImage;
}
@ -82,17 +82,17 @@ class ImageGdThumbnailGenerator implements IThumbnailGenerator
if (($dstHeight / $dstWidth) < ($srcHeight / $srcWidth))
{
$h = $dstHeight;
$w = $h * $srcWidth / $srcHeight;
$cropHeight = $dstHeight;
$cropWidth = $dstHeight * $srcWidth / $srcHeight;
}
else
{
$w = $dstWidth;
$h = $w * $srcHeight / $srcWidth;
$cropWidth = $dstWidth;
$cropHeight = $dstWidth * $srcHeight / $srcWidth;
}
$dstImage = imagecreatetruecolor($w, $h);
imagecopyresampled($dstImage, $srcImage, 0, 0, 0, 0, $w, $h, $srcWidth, $srcHeight);
$dstImage = imagecreatetruecolor($cropWidth, $cropHeight);
imagecopyresampled($dstImage, $srcImage, 0, 0, 0, 0, $cropWidth, $cropHeight, $srcWidth, $srcHeight);
return $dstImage;
}
}

View file

@ -41,19 +41,19 @@ class ImageImagickThumbnailGenerator implements IThumbnailGenerator
if (($dstHeight / $dstWidth) > ($srcHeight / $srcWidth))
{
$h = $dstHeight;
$w = $h * $srcWidth / $srcHeight;
$cropHeight = $dstHeight;
$cropWidth = $dstHeight * $srcWidth / $srcHeight;
}
else
{
$w = $dstWidth;
$h = $w * $srcHeight / $srcWidth;
$cropWidth = $dstWidth;
$cropHeight = $dstWidth * $srcHeight / $srcWidth;
}
$x = ($srcWidth - $w) / 2;
$y = ($srcHeight - $h) / 2;
$cropX = ($cropWidth - $dstWidth) >> 1;
$cropY = ($cropHeight - $dstHeight) >> 1;
$srcImage->resizeImage($w, $h, \imagick::FILTER_LANCZOS, 0.9);
$srcImage->cropImage($dstWidth, $dstHeight, ($w - $dstWidth) >> 1, ($h - $dstHeight) >> 1);
$srcImage->resizeImage($cropWidth, $cropHeight, \imagick::FILTER_LANCZOS, 0.9);
$srcImage->cropImage($dstWidth, $dstHeight, $cropX, $cropY);
$srcImage->setImagePage(0, 0, 0, 0);
}
@ -64,15 +64,15 @@ class ImageImagickThumbnailGenerator implements IThumbnailGenerator
if (($dstHeight / $dstWidth) < ($srcHeight / $srcWidth))
{
$h = $dstHeight;
$w = $h * $srcWidth / $srcHeight;
$cropHeight = $dstHeight;
$cropWidth = $dstHeight * $srcWidth / $srcHeight;
}
else
{
$w = $dstWidth;
$h = $w * $srcHeight / $srcWidth;
$cropWidth = $dstWidth;
$cropHeight = $dstWidth * $srcHeight / $srcWidth;
}
$srcImage->resizeImage($w, $h, \imagick::FILTER_LANCZOS, 0.9);
$srcImage->resizeImage($cropWidth, $cropHeight, \imagick::FILTER_LANCZOS, 0.9);
}
}

View file

@ -67,7 +67,7 @@ class UserService
public function getFiltered(\Szurubooru\FormData\SearchFormData $formData)
{
$pageSize = intval($this->config->users->usersPerPage);
$this->validator->validateNumber($formData->page);
$this->validator->validateNumber($formData->pageNumber);
$searchFilter = new \Szurubooru\Dao\SearchFilter($pageSize, $formData);
return $this->userSearchService->getFiltered($searchFilter);
}
@ -95,7 +95,7 @@ class UserService
$user->lastLoginTime = null;
$user->avatarStyle = \Szurubooru\Entities\User::AVATAR_STYLE_GRAVATAR;
$this->sendActivationEmailIfNeeded($user);
$user = $this->sendActivationEmailIfNeeded($user);
return $this->userDao->save($user);
}
@ -109,11 +109,11 @@ class UserService
$this->fileService->saveFromBase64($formData->avatarContent, $this->getCustomAvatarSourcePath($user));
}
if ($formData->userName !== null and $formData->userName != $user->name)
if ($formData->userName !== null and $formData->userName !== $user->name)
{
$this->validator->validateUserName($formData->userName);
$userWithThisEmail = $this->userDao->getByName($formData->userName);
if ($userWithThisEmail and $userWithThisEmail->id != $user->id)
if ($userWithThisEmail and $userWithThisEmail->id !== $user->id)
throw new \DomainException('User with this name already exists.');
$user->name = $formData->userName;
@ -125,13 +125,14 @@ class UserService
$user->passwordHash = $this->passwordService->getHash($formData->password);
}
if ($formData->email !== null and $formData->email != $user->email)
if ($formData->email !== null and $formData->email !== $user->email)
{
$this->validator->validateEmail($formData->email);
if ($this->userDao->getByEmail($formData->email))
throw new \DomainException('User with this e-mail already exists.');
$user->emailUnconfirmed = $formData->email;
$user = $this->sendActivationEmailIfNeeded($user);
}
if ($formData->accessRank !== null)
@ -148,9 +149,6 @@ class UserService
$user->browsingSettings = $formData->browsingSettings;
}
if ($formData->email !== null)
$this->sendActivationEmailIfNeeded($user);
return $this->userDao->save($user);
}
@ -185,7 +183,7 @@ class UserService
public function finishPasswordReset($tokenName)
{
$token = $this->tokenService->getByName($tokenName);
if ($token->purpose != \Szurubooru\Entities\Token::PURPOSE_PASSWORD_RESET)
if ($token->purpose !== \Szurubooru\Entities\Token::PURPOSE_PASSWORD_RESET)
throw new \Exception('This token is not a password reset token.');
$user = $this->getByName($token->additionalData);
@ -205,27 +203,29 @@ class UserService
public function finishActivation($tokenName)
{
$token = $this->tokenService->getByName($tokenName);
if ($token->purpose != \Szurubooru\Entities\Token::PURPOSE_ACTIVATE)
if ($token->purpose !== \Szurubooru\Entities\Token::PURPOSE_ACTIVATE)
throw new \Exception('This token is not an activation token.');
$user = $this->getByName($token->additionalData);
$this->confirmEmail($user);
$user = $this->confirmEmail($user);
$this->userDao->save($user);
$this->tokenService->invalidateByName($token->name);
}
private function sendActivationEmailIfNeeded(\Szurubooru\Entities\User &$user)
private function sendActivationEmailIfNeeded(\Szurubooru\Entities\User $user)
{
if ($user->accessRank == \Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR or !$this->config->security->needEmailActivationToRegister)
if ($user->accessRank === \Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR or !$this->config->security->needEmailActivationToRegister)
{
$this->confirmEmail($user);
$user = $this->confirmEmail($user);
}
else
{
$this->sendActivationEmail($user);
}
return $user;
}
private function confirmEmail(\Szurubooru\Entities\User &$user)
private function confirmEmail(\Szurubooru\Entities\User $user)
{
//security issue:
//1. two users set their unconfirmed mail to godzilla@empire.gov
@ -240,6 +240,6 @@ class UserService
$user->email = $user->emailUnconfirmed;
$user->emailUnconfirmed = null;
$this->userDao->save($user);
return $user;
}
}

View file

@ -10,8 +10,10 @@ class Validator
$this->config = $config;
}
public function validateNumber(&$subject) {
$subject = intval($subject);
public function validateNumber($subject)
{
if (!preg_match('/^-?[0-9]+$/', $subject))
throw new \DomainException(subject . ' does not look like a number.');
}
public function validateNonEmpty($subject, $subjectName = 'Object')
@ -38,10 +40,8 @@ class Validator
throw new \DomainException($subjectName . ' must have at most ' . $maxLength . ' character(s).');
}
public function validateUserName(&$userName)
public function validateUserName($userName)
{
$userName = trim($userName);
$minUserNameLength = intval($this->config->users->minUserNameLength);
$maxUserNameLength = intval($this->config->users->maxUserNameLength);
$this->validateNonEmpty($userName, 'User name');

View file

@ -1,5 +1,6 @@
<?php
$dataDirectory = __DIR__ . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR . 'data';
return [
\Szurubooru\Config::class => DI\object()->constructor([
$dataDirectory . DIRECTORY_SEPARATOR . 'config.ini',
@ -9,11 +10,11 @@ return [
\Szurubooru\ControllerRepository::class => DI\object()->constructor(DI\link('controllers')),
'controllers' => DI\factory(function (DI\container $c) {
'controllers' => DI\factory(function (DI\container $container) {
return [
$c->get(\Szurubooru\Controllers\AuthController::class),
$c->get(\Szurubooru\Controllers\UserController::class),
$c->get(\Szurubooru\Controllers\UserAvatarController::class),
$container->get(\Szurubooru\Controllers\AuthController::class),
$container->get(\Szurubooru\Controllers\UserController::class),
$container->get(\Szurubooru\Controllers\UserAvatarController::class),
];
}),
];

View file

@ -62,16 +62,6 @@ final class ValidatorTest extends \Szurubooru\Tests\AbstractTestCase
$validator->validateUserName($userName);
}
public function testUserNameWithSpaces()
{
$this->configMock->set('users/minUserNameLength', 0);
$this->configMock->set('users/maxUserNameLength', 100);
$userName = ' godzilla ';
$validator = $this->getValidator();
$validator->validateUserName($userName);
$this->assertEquals('godzilla', $userName);
}
public function testUserNameWithInvalidCharacters()
{
$this->configMock->set('users/minUserNameLength', 0);