From bf79f2474c94a8c870c1b5deb76f6cf975a6e93a Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Tue, 24 Sep 2019 01:00:22 +0200 Subject: [PATCH] Cleanup code from #1876 - Extract a method for email address generation - Consistent types - No docblocks for types where superfluous - Tweak console output - Don't inherit from integration test's base class in unit test --- src/Install/BaseUrl.php | 47 ++++++++------------ src/Install/Console/UserDataProvider.php | 13 +++--- src/Install/Controller/InstallController.php | 6 +-- src/Install/Installation.php | 6 +-- src/Install/Steps/StoreConfig.php | 5 ++- tests/unit/Install/BaseUrlTest.php | 40 ++++++++++++++--- 6 files changed, 66 insertions(+), 51 deletions(-) diff --git a/src/Install/BaseUrl.php b/src/Install/BaseUrl.php index b2e801454..d35b39904 100644 --- a/src/Install/BaseUrl.php +++ b/src/Install/BaseUrl.php @@ -15,60 +15,49 @@ use Psr\Http\Message\UriInterface; final class BaseUrl { - /** @var UriInterface|string */ - private $baseUrl; + /** @var string */ + private $normalized; - /** - * @param UriInterface|string $baseUrl - */ - private function __construct($baseUrl) + private function __construct(string $baseUrl) { - $this->baseUrl = $this->normalise($baseUrl); + $this->normalized = $this->normalize($baseUrl); } - /** - * @param string $baseUrl - * @return \Flarum\Install\BaseUrl - */ public static function fromString(string $baseUrl): self { return new self($baseUrl); } - /** - * @param \Psr\Http\Message\UriInterface $baseUrl - * @return \Flarum\Install\BaseUrl - */ public static function fromUri(UriInterface $baseUrl): self { - return self::fromString((string) $baseUrl); + return new self((string) $baseUrl); } - /** - * @return string - */ public function __toString(): string { - return $this->baseUrl; + return $this->normalized; } - /** - * @param UriInterface|string $baseUrl - * @return string - */ - private function normalise($baseUrl): string + public function toEmail(string $mailbox): string + { + $host = preg_replace('/^www\./i', '', parse_url($this->normalized, PHP_URL_HOST)); + + return "$mailbox@$host"; + } + + private function normalize(string $baseUrl): string { // Empty base url is still valid if (empty($baseUrl)) { return ''; } - $normalisedBaseUrl = trim($baseUrl, '/'); - if (! preg_match('#^https?://#i', $normalisedBaseUrl)) { - $normalisedBaseUrl = sprintf('http://%s', $normalisedBaseUrl); + $normalizedBaseUrl = trim($baseUrl, '/'); + if (! preg_match('#^https?://#i', $normalizedBaseUrl)) { + $normalizedBaseUrl = sprintf('http://%s', $normalizedBaseUrl); } - $parseUrl = parse_url($normalisedBaseUrl); + $parseUrl = parse_url($normalizedBaseUrl); $path = $parseUrl['path'] ?? null; if (isset($parseUrl['path']) && strrpos($parseUrl['path'], '.php') !== false) { diff --git a/src/Install/Console/UserDataProvider.php b/src/Install/Console/UserDataProvider.php index b87dbd399..8fb8d94ae 100644 --- a/src/Install/Console/UserDataProvider.php +++ b/src/Install/Console/UserDataProvider.php @@ -27,6 +27,7 @@ class UserDataProvider implements DataProviderInterface protected $questionHelper; + /** @var BaseUrl */ protected $baseUrl; public function __construct(InputInterface $input, OutputInterface $output, QuestionHelper $questionHelper) @@ -40,7 +41,7 @@ class UserDataProvider implements DataProviderInterface { return $installation ->debugMode(false) - ->baseUrl(BaseUrl::fromString($this->getBaseUrl())) + ->baseUrl($this->getBaseUrl()) ->databaseConfig($this->getDatabaseConfiguration()) ->adminUser($this->getAdminUser()) ->settings($this->getSettings()); @@ -66,15 +67,17 @@ class UserDataProvider implements DataProviderInterface ); } - private function getBaseUrl(): string + private function getBaseUrl(): BaseUrl { - return $this->baseUrl = $this->ask('Base URL:(Default: http://flarum.local)', 'http://flarum.local'); + $baseUrl = $this->ask('Base URL (Default: http://flarum.local):', 'http://flarum.local'); + + return $this->baseUrl = BaseUrl::fromString($baseUrl); } private function getAdminUser(): AdminUser { return new AdminUser( - $this->ask('Admin username:(Default: admin)', 'admin'), + $this->ask('Admin username (Default: admin):', 'admin'), $this->askForAdminPassword(), $this->ask('Admin email address (required):') ); @@ -107,7 +110,7 @@ class UserDataProvider implements DataProviderInterface return [ 'forum_title' => $title, - 'mail_from' => 'noreply@'.preg_replace('/^www\./i', '', parse_url($this->baseUrl, PHP_URL_HOST)), + 'mail_from' => $this->baseUrl->toEmail('noreply'), 'welcome_title' => 'Welcome to '.$title, ]; } diff --git a/src/Install/Controller/InstallController.php b/src/Install/Controller/InstallController.php index 51fa5358c..28f17de13 100644 --- a/src/Install/Controller/InstallController.php +++ b/src/Install/Controller/InstallController.php @@ -53,16 +53,16 @@ class InstallController implements RequestHandlerInterface public function handle(Request $request): ResponseInterface { $input = $request->getParsedBody(); - $baseUrl = $request->getUri(); + $baseUrl = BaseUrl::fromUri($request->getUri()); try { $pipeline = $this->installation - ->baseUrl(BaseUrl::fromUri($baseUrl)) + ->baseUrl($baseUrl) ->databaseConfig($this->makeDatabaseConfig($input)) ->adminUser($this->makeAdminUser($input)) ->settings([ 'forum_title' => Arr::get($input, 'forumTitle'), - 'mail_from' => 'noreply@'.preg_replace('/^www\./i', '', $baseUrl->getHost()), + 'mail_from' => $baseUrl->toEmail('noreply'), 'welcome_title' => 'Welcome to '.Arr::get($input, 'forumTitle'), ]) ->build(); diff --git a/src/Install/Installation.php b/src/Install/Installation.php index 835370950..8ef92d6b7 100644 --- a/src/Install/Installation.php +++ b/src/Install/Installation.php @@ -63,13 +63,9 @@ class Installation return $this; } - /** - * @param \Flarum\Install\BaseUrl $baseUrl - * @return $this - */ public function baseUrl(BaseUrl $baseUrl) { - $this->baseUrl = (string) $baseUrl; + $this->baseUrl = $baseUrl; return $this; } diff --git a/src/Install/Steps/StoreConfig.php b/src/Install/Steps/StoreConfig.php index 5c4832c5a..67fbd79da 100644 --- a/src/Install/Steps/StoreConfig.php +++ b/src/Install/Steps/StoreConfig.php @@ -9,6 +9,7 @@ namespace Flarum\Install\Steps; +use Flarum\Install\BaseUrl; use Flarum\Install\DatabaseConfig; use Flarum\Install\ReversibleStep; use Flarum\Install\Step; @@ -23,7 +24,7 @@ class StoreConfig implements Step, ReversibleStep private $configFile; - public function __construct($debugMode, DatabaseConfig $dbConfig, $baseUrl, $configFile) + public function __construct($debugMode, DatabaseConfig $dbConfig, BaseUrl $baseUrl, $configFile) { $this->debugMode = $debugMode; $this->dbConfig = $dbConfig; @@ -55,7 +56,7 @@ class StoreConfig implements Step, ReversibleStep return [ 'debug' => $this->debugMode, 'database' => $this->dbConfig->toArray(), - 'url' => $this->baseUrl, + 'url' => (string) $this->baseUrl, 'paths' => $this->getPathsConfig(), ]; } diff --git a/tests/unit/Install/BaseUrlTest.php b/tests/unit/Install/BaseUrlTest.php index af7363b14..c4f1ca58d 100644 --- a/tests/unit/Install/BaseUrlTest.php +++ b/tests/unit/Install/BaseUrlTest.php @@ -12,14 +12,13 @@ namespace Flarum\Tests\unit; use Flarum\Install\BaseUrl; -use Flarum\Tests\integration\TestCase; +use PHPUnit\Framework\TestCase; +use Zend\Diactoros\Uri; class BaseUrlTest extends TestCase { /** * @dataProvider urlProvider - * @param $uri - * @param $expected */ public function test_base_url_simulating_cli_installer($uri, $expected) { @@ -28,14 +27,23 @@ class BaseUrlTest extends TestCase /** * @dataProvider urlProvider - * @param $uri - * @param $expected */ public function test_base_url_simulating_web_installer($uri, $expected) { - $request = $this->request('get', $uri); + $uri = new Uri($uri); - $this->assertEquals($expected, BaseUrl::fromUri($request->getUri())); + $this->assertEquals($expected, BaseUrl::fromUri($uri)); + } + + /** + * @dataProvider emailProvider + */ + public function test_default_email_generation($uri, $expected) + { + $this->assertEquals( + $expected, + BaseUrl::fromString($uri)->toEmail('noreply') + ); } public function urlProvider() @@ -56,4 +64,22 @@ class BaseUrlTest extends TestCase ['http://sub.flarum.org', 'http://sub.flarum.org'], ]; } + + public function emailProvider() + { + return [ + ['flarum.org', 'noreply@flarum.org'], + ['flarum.org/', 'noreply@flarum.org'], + ['http://flarum.org', 'noreply@flarum.org'], + ['http://flarum.org/', 'noreply@flarum.org'], + ['https://flarum.org', 'noreply@flarum.org'], + ['http://flarum.org/index.php', 'noreply@flarum.org'], + ['http://flarum.org/index.php/', 'noreply@flarum.org'], + ['http://flarum.org/flarum', 'noreply@flarum.org'], + ['http://flarum.org/flarum/index.php', 'noreply@flarum.org'], + ['http://flarum.org/flarum/index.php/', 'noreply@flarum.org'], + ['sub.flarum.org', 'noreply@sub.flarum.org'], + ['http://sub.flarum.org', 'noreply@sub.flarum.org'], + ]; + } }