From cedba9a5ef8c582c47fc059d457a6b15a37e75c5 Mon Sep 17 00:00:00 2001 From: Magnus Flor Date: Thu, 14 Mar 2019 19:12:20 +0100 Subject: [PATCH 1/2] Progress commit --- composer.json | 3 +- composer.lock | 115 +++++++++++++---- src/Commands/RedirectCommands.php | 112 +++++++++++++++-- .../AlterRedirectTableAddIgnoreQuery.php | 24 ++++ src/Database/Query.php | 7 ++ src/Repositories/RedirectRepository.php | 49 ++++++-- .../CrudController/Create/CreateTest.php | 35 ++++++ .../CrudController/Update/UpdateTest.php | 38 ++++++ .../RedirectRepository/SavingRedirectTest.php | 119 ++++++++++++++++++ .../RedirectRepositoryTestCase.php | 7 +- wp-bonnier-redirect.php | 2 +- 11 files changed, 464 insertions(+), 47 deletions(-) create mode 100644 tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php diff --git a/composer.json b/composer.json index a6dbf0d..e832bbb 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,8 @@ "require": { "php": ">=7.1", "illuminate/support": "^5.7", - "symfony/http-foundation": "^4.2" + "symfony/http-foundation": "^4.2", + "league/csv": "^9.2" }, "require-dev": { "johnpbloch/wordpress": "^4.9", diff --git a/composer.lock b/composer.lock index cd672c3..c82eba5 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "a612d321a6b4899c2e9823198964e9a3", + "content-hash": "1fc36ba6f4a20914ad5b2ed97676082e", "packages": [ { "name": "doctrine/inflector", @@ -75,7 +75,7 @@ }, { "name": "illuminate/contracts", - "version": "v5.8.3", + "version": "v5.8.4", "source": { "type": "git", "url": "https://github.com/illuminate/contracts.git", @@ -119,16 +119,16 @@ }, { "name": "illuminate/support", - "version": "v5.8.3", + "version": "v5.8.4", "source": { "type": "git", "url": "https://github.com/illuminate/support.git", - "reference": "0f0291d1bc2f036af3fceb8e46900b58812533c4" + "reference": "07062f5750872a31e086ff37a7c50ac18b8c417c" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/illuminate/support/zipball/0f0291d1bc2f036af3fceb8e46900b58812533c4", - "reference": "0f0291d1bc2f036af3fceb8e46900b58812533c4", + "url": "https://api.github.com/repos/illuminate/support/zipball/07062f5750872a31e086ff37a7c50ac18b8c417c", + "reference": "07062f5750872a31e086ff37a7c50ac18b8c417c", "shasum": "" }, "require": { @@ -176,20 +176,87 @@ ], "description": "The Illuminate Support package.", "homepage": "https://laravel.com", - "time": "2019-03-05T13:38:58+00:00" + "time": "2019-03-12T13:17:00+00:00" + }, + { + "name": "league/csv", + "version": "9.2.0", + "source": { + "type": "git", + "url": "https://github.com/thephpleague/csv.git", + "reference": "f3a3c69b6e152417e1b62d995bcad2237b053cc6" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/thephpleague/csv/zipball/f3a3c69b6e152417e1b62d995bcad2237b053cc6", + "reference": "f3a3c69b6e152417e1b62d995bcad2237b053cc6", + "shasum": "" + }, + "require": { + "ext-mbstring": "*", + "php": ">=7.0.10" + }, + "require-dev": { + "ext-curl": "*", + "friendsofphp/php-cs-fixer": "^2.12", + "phpstan/phpstan": "^0.9.2", + "phpstan/phpstan-phpunit": "^0.9.4", + "phpstan/phpstan-strict-rules": "^0.9.0", + "phpunit/phpunit": "^6.0" + }, + "suggest": { + "ext-iconv": "Needed to ease transcoding CSV using iconv stream filters" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "9.x-dev" + } + }, + "autoload": { + "psr-4": { + "League\\Csv\\": "src" + }, + "files": [ + "src/functions_include.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Ignace Nyamagana Butera", + "email": "nyamsprod@gmail.com", + "homepage": "https://github.com/nyamsprod/", + "role": "Developer" + } + ], + "description": "Csv data manipulation made easy in PHP", + "homepage": "http://csv.thephpleague.com", + "keywords": [ + "csv", + "export", + "filter", + "import", + "read", + "write" + ], + "time": "2019-03-08T06:56:16+00:00" }, { "name": "nesbot/carbon", - "version": "2.14.2", + "version": "2.16.0", "source": { "type": "git", "url": "https://github.com/briannesbitt/Carbon.git", - "reference": "a1f4f9abcde8241ce33bf5090896e9c16d0b4232" + "reference": "dd16fedc022180ea4292a03aabe95e9895677911" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/briannesbitt/Carbon/zipball/a1f4f9abcde8241ce33bf5090896e9c16d0b4232", - "reference": "a1f4f9abcde8241ce33bf5090896e9c16d0b4232", + "url": "https://api.github.com/repos/briannesbitt/Carbon/zipball/dd16fedc022180ea4292a03aabe95e9895677911", + "reference": "dd16fedc022180ea4292a03aabe95e9895677911", "shasum": "" }, "require": { @@ -236,7 +303,7 @@ "datetime", "time" ], - "time": "2019-02-28T09:07:12+00:00" + "time": "2019-03-12T09:31:40+00:00" }, { "name": "psr/container", @@ -1616,20 +1683,20 @@ }, { "name": "johnpbloch/wordpress", - "version": "4.9.9", + "version": "4.9.10", "source": { "type": "git", "url": "https://github.com/johnpbloch/wordpress.git", - "reference": "9cd9b6d8364860a36fd02b088e2fea270f7f531d" + "reference": "fa10404bdcf045704f7ff519f483af6acd4cf5ef" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/johnpbloch/wordpress/zipball/9cd9b6d8364860a36fd02b088e2fea270f7f531d", - "reference": "9cd9b6d8364860a36fd02b088e2fea270f7f531d", + "url": "https://api.github.com/repos/johnpbloch/wordpress/zipball/fa10404bdcf045704f7ff519f483af6acd4cf5ef", + "reference": "fa10404bdcf045704f7ff519f483af6acd4cf5ef", "shasum": "" }, "require": { - "johnpbloch/wordpress-core": "4.9.9", + "johnpbloch/wordpress-core": "4.9.10", "johnpbloch/wordpress-core-installer": "^1.0", "php": ">=5.3.2" }, @@ -1651,27 +1718,27 @@ "cms", "wordpress" ], - "time": "2018-12-13T03:42:59+00:00" + "time": "2019-03-13T01:13:10+00:00" }, { "name": "johnpbloch/wordpress-core", - "version": "4.9.9", + "version": "4.9.10", "source": { "type": "git", "url": "https://github.com/johnpbloch/wordpress-core.git", - "reference": "3f6081c514707ad25153b9acb90f022cd122d759" + "reference": "06dad62aa1ab1ef215034fb07d6aec4a68072313" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/johnpbloch/wordpress-core/zipball/3f6081c514707ad25153b9acb90f022cd122d759", - "reference": "3f6081c514707ad25153b9acb90f022cd122d759", + "url": "https://api.github.com/repos/johnpbloch/wordpress-core/zipball/06dad62aa1ab1ef215034fb07d6aec4a68072313", + "reference": "06dad62aa1ab1ef215034fb07d6aec4a68072313", "shasum": "" }, "require": { "php": ">=5.3.2" }, "provide": { - "wordpress/core-implementation": "4.9.9" + "wordpress/core-implementation": "4.9.10" }, "type": "wordpress-core", "notification-url": "https://packagist.org/downloads/", @@ -1691,7 +1758,7 @@ "cms", "wordpress" ], - "time": "2018-12-13T03:42:54+00:00" + "time": "2019-03-13T01:13:06+00:00" }, { "name": "johnpbloch/wordpress-core-installer", diff --git a/src/Commands/RedirectCommands.php b/src/Commands/RedirectCommands.php index 8461a93..6a33927 100644 --- a/src/Commands/RedirectCommands.php +++ b/src/Commands/RedirectCommands.php @@ -3,25 +3,40 @@ namespace Bonnier\WP\Redirect\Commands; use Bonnier\WP\Redirect\Database\DB; +use Bonnier\WP\Redirect\Database\Exceptions\DuplicateEntryException; use Bonnier\WP\Redirect\Database\Migrations\Migrate; use Bonnier\WP\Redirect\Helpers\UrlHelper; use Bonnier\WP\Redirect\Models\Redirect; use Bonnier\WP\Redirect\Repositories\RedirectRepository; use cli\progress\Bar; +use Illuminate\Support\Str; +use League\Csv\Writer; use function WP_CLI\Utils\make_progress_bar; class RedirectCommands extends \WP_CLI_Command { + private $csvHandle; + /** * Normalizes all from and to urls in the redirect table * + * ## OPTIONS + * [--output=] + * : Optional: Specify a csv filename where updates are recorded. + * * ## EXAMPLES * wp bonnier redirect redirects normalize * + * @param $args + * @param $assocArgs + * * @throws \WP_CLI\ExitException */ - public function normalize() + public function normalize($args, $assocArgs) { + if ($output = $assocArgs['output'] ?? null) { + $this->initCSV($output); + } $database = new DB(); $database->setTable(Migrate::REDIRECTS_TABLE); try { @@ -33,7 +48,7 @@ public function normalize() } if ($results->isNotEmpty()) { /** @var Bar $progress */ - $progress = make_progress_bar(sprintf('Normalizing %s redirects', $results->count()), $results->count()); + $progress = make_progress_bar(sprintf('Normalizing %s redirects', number_format($results->count())), $results->count()); $results->each(function (array $redirect) use ($database, $progress) { $redirectID = $redirect['id']; unset($redirect['id']); @@ -43,13 +58,26 @@ public function normalize() $redirect['locale'] === '' ) { $database->delete($redirectID); + $this->csvWrite([$redirectID, $redirect['from'], $redirect['to'], $redirect['locale'], 'N/A', 'N/A', 'Deleted']); } else { - $redirect['from'] = UrlHelper::normalizePath($redirect['from']); + $newFrom = UrlHelper::normalizePath($redirect['from']); + $newTo = UrlHelper::normalizeUrl($redirect['to']); + if ($newFrom !== $redirect['from'] || $newTo !== $redirect['to']) { + $this->csvWrite([$redirectID, $redirect['from'], $redirect['to'], $redirect['locale'], $newFrom, $newTo, 'Updated']); + } + $redirect['from'] = $newFrom; $redirect['from_hash'] = hash('md5', $redirect['from']); $redirect['paramless_from_hash'] = hash('md5', parse_url($redirect['from'], PHP_URL_PATH)); - $redirect['to'] = UrlHelper::normalizeUrl($redirect['to']); + $redirect['to'] = $newTo; $redirect['to_hash'] = hash('md5', $redirect['to']); - $database->update($redirectID, $redirect); + try { + $database->update($redirectID, $redirect); + } catch (DuplicateEntryException $exception) { + $database->delete($redirectID); + $this->csvWrite([$redirectID, $redirect['from'], $redirect['to'], $redirect['locale'], 'N/A', 'N/A', 'Deleted']); + } catch (\Exception $exception) { + \WP_CLI::warning(sprintf('Failed updating redirect \'%s\'', $redirectID)); + } } $progress->tick(); }); @@ -60,27 +88,36 @@ public function normalize() /** * Removes all chains in the redirect table * + * ## OPTIONS + * [--output=] + * : Optional: Specify a csv filename where updates are recorded. + * * ## EXAMPLES * wp bonnier redirect redirects unchain * * @throws \WP_CLI\ExitException */ - public function unchain() + public function unchain($args, $assocArgs) { + if ($output = $assocArgs['output'] ?? null) { + $this->initCSV($output); + } try { $repository = new RedirectRepository(new DB()); } catch (\Exception $exception) { \WP_CLI::error(sprintf('Failed instantiating RedirectRepository (%s)', $exception->getMessage())); return; } + \WP_CLI::line('Retrieving redirects...'); try { - $redirects = $repository->findAll(); + $redirects = $repository->findAll()->reverse(); } catch (\Exception $exception) { \WP_CLI::error(sprintf('Failed finding redirects (%s)', $exception->getMessage())); return; } + $redirectCount = $redirects->count(); /** @var Bar $progress */ - $progress = make_progress_bar(sprintf('Unchaining %s redirects', $redirects->count()), $redirects->count()); + $progress = make_progress_bar(sprintf('Unchaining %s redirects', number_format($redirectCount)), $redirectCount); $redirects->each(function (Redirect $redirect) use ($repository, $progress) { try { $repository->save($redirect); @@ -90,5 +127,64 @@ public function unchain() $progress->tick(); }); $progress->finish(); + if ($this->csvHandle) { + $unchainedRedirects = $repository->findAll(); + $progress = make_progress_bar(sprintf('Logging changes for %s redirects', number_format($redirectCount)), $redirectCount); + $redirects->each(function (Redirect $oldRedirect) use ($unchainedRedirects, $progress) { + /** @var Redirect|null $unchainedRedirect */ + $unchainedRedirect = $unchainedRedirects->first(function (Redirect $redirect) use ($oldRedirect) { + return $redirect->getID() === $oldRedirect->getID(); + }); + if ($unchainedRedirect) { + if ($unchainedRedirect->getFrom() !== $oldRedirect->getFrom() || + $unchainedRedirect->getTo() !== $oldRedirect->getTo() + ) { + $this->csvWrite([ + $oldRedirect->getID(), + $oldRedirect->getFrom(), + $oldRedirect->getTo(), + $oldRedirect->getLocale(), + $unchainedRedirect->getFrom(), + $unchainedRedirect->getTo(), + 'Updated' + ]); + } + } else { + $this->csvWrite([ + $oldRedirect->getID(), + $oldRedirect->getFrom(), + $oldRedirect->getTo(), + $oldRedirect->getLocale(), + 'N/A', + 'N/A', + 'Deleted' + ]); + } + $progress->tick(); + }); + $progress->finish(); + } + } + + private function initCSV(string $file) + { + $filepath = Str::finish(getcwd(), '/'); + if (Str::startsWith($file, './')) { + $filepath .= Str::after($file, './'); + } elseif (Str::startsWith($file, ['/', '~'])) { + \WP_CLI::error('Please define output file as a relative path (eg. /path/to/file.csv)'); + } else { + $filepath .= $file; + } + $this->csvHandle = Writer::createFromPath($filepath, 'w+'); + $this->csvHandle->insertOne(['ID', 'Old From', 'Old To', 'Locale', 'New From', 'New To', 'Notes']); + } + + private function csvWrite(array $row) + { + if (!$this->csvHandle) { + return; + } + $this->csvHandle->insertOne($row); } } diff --git a/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php b/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php index db5da46..850f563 100644 --- a/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php +++ b/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php @@ -3,6 +3,7 @@ namespace Bonnier\WP\Redirect\Database\Migrations; use Illuminate\Support\Str; +use League\Csv\Writer; class AlterRedirectTableAddIgnoreQuery implements Migration { @@ -15,6 +16,8 @@ public static function migrate() global $wpdb; $table = $wpdb->prefix . Migrate::REDIRECTS_TABLE; + self::deduplicate($wpdb, $table); + $sql = " ALTER TABLE `$table` ADD `keep_query` tinyint(1) DEFAULT 0, @@ -35,4 +38,25 @@ public static function verify(): bool $result = $wpdb->get_row("SHOW CREATE TABLE $table", ARRAY_A); return isset($result['Create Table']) && Str::contains($result['Create Table'], 'keep_query'); } + + private static function deduplicate(\wpdb $wpdb, string $table) + { + $csv = Writer::createFromPath(sprintf('%s_deleted_duplicates.csv', strtotime('now')), 'w+'); + $csv->insertOne(['id', 'from', 'to', 'locale']); + $wpdb->update($table, ['locale' => 'nb'], ['locale' => 'no']); + $wpdb->delete($table, ['locale' => '']); + $sql = "SELECT `locale` FROM `$table` GROUP BY `locale`"; + $locales = $wpdb->get_results($sql); + foreach ($locales as $locale) { + $results = $wpdb->get_results("SELECT `from_hash`, count(id) AS cnt FROM `$table` WHERE `locale` = '$locale->locale' GROUP BY `from` HAVING cnt > 1"); + foreach ($results as $result) { + $redirects = $wpdb->get_results("SELECT * FROM `$table` WHERE `from_hash` = '$result->from_hash' AND `locale` = '$locale->locale' ORDER BY id DESC"); + array_shift($redirects); // Keep the newest redirect + foreach ($redirects as $redirect) { + $csv->insertOne([$redirect->id, $redirect->from, $redirect->to, $redirect->locale]); + $wpdb->delete($table, ['id' => $redirect->id]); + } + } + } + } } diff --git a/src/Database/Query.php b/src/Database/Query.php index 5fec95b..3c27d32 100644 --- a/src/Database/Query.php +++ b/src/Database/Query.php @@ -146,6 +146,13 @@ public function offset(int $offset): Query return $this; } + public function groupBy(string $column): Query + { + $this->query .= " GROUP BY `$column`"; + + return $this; + } + /** * @return string * @throws \Exception diff --git a/src/Repositories/RedirectRepository.php b/src/Repositories/RedirectRepository.php index 7170a9a..0d5d47d 100644 --- a/src/Repositories/RedirectRepository.php +++ b/src/Repositories/RedirectRepository.php @@ -252,6 +252,7 @@ public function save(Redirect &$redirect, bool $updateOnDuplicate = false): Redi } } + $this->removeReverseRedirects($redirect); $this->removeChainsByRedirect($redirect); return $redirect; @@ -289,6 +290,28 @@ public function deleteMultipleByIDs(array $redirectIDs) return $this->database->deleteMultiple($redirectIDs); } + public function removeReverseRedirects(Redirect $redirect) + { + $query = $this->database->query() + ->select('*') + ->where(['from_hash', $redirect->getToHash()]) + ->andWhere(['to_hash', $redirect->getFromHash()]) + ->andWhere(['locale', $redirect->getLocale()]); + try { + $dbRedirects = collect($this->database->getResults($query)); + } catch (\Exception $exception) { + return; + } + if ($dbRedirects->isNotEmpty()) { + try { + $this->deleteMultipleByIDs($dbRedirects->map(function (array $redirect) { + return $redirect['id']; + })->toArray()); + } catch (\Exception $exception) { + } + } + } + /** * Finds all redirects that with to_hash that matches the specified redirects from hash. * I.e the redirect in the database that redirects from '/a/b' to /c/d' @@ -300,14 +323,6 @@ public function deleteMultipleByIDs(array $redirectIDs) */ public function removeChainsByRedirect(Redirect $redirect) { - if ($dbRedirects = $this->findAllBy('from_hash', $redirect->getToHash())) { - $dbRedirects->each(function (Redirect $dbRedirect) use ($redirect) { - if ($dbRedirect->getLocale() === $redirect->getLocale()) { - $redirect->setTo($dbRedirect->getTo()); - $this->updateRedirect($redirect); - } - }); - } if ($redirects = $this->findAllBy('to_hash', $redirect->getFromHash())) { $redirects->each(function (Redirect $dbRedirect) use ($redirect) { if ($dbRedirect->getID() === $redirect->getID() || @@ -319,6 +334,14 @@ public function removeChainsByRedirect(Redirect $redirect) $this->updateRedirect($dbRedirect); }); } + if ($dbRedirects = $this->findAllBy('from_hash', $redirect->getToHash())) { + $dbRedirects->each(function (Redirect $dbRedirect) use ($redirect) { + if ($dbRedirect->getLocale() === $redirect->getLocale()) { + $redirect->setTo($dbRedirect->getTo()); + $this->updateRedirect($redirect); + } + }); + } } /** @@ -339,9 +362,13 @@ private function mapRedirects(array $redirects): Collection */ private function updateRedirect(Redirect $redirect) { - $data = $redirect->toArray(); - unset($data['id']); - $this->database->update($redirect->getID(), $data); + if ($redirect->getFrom() === $redirect->getTo()) { + $this->delete($redirect); + } else { + $data = $redirect->toArray(); + unset($data['id']); + $this->database->update($redirect->getID(), $data); + } } /** diff --git a/tests/integration/Controllers/CrudController/Create/CreateTest.php b/tests/integration/Controllers/CrudController/Create/CreateTest.php index cd661d0..a066562 100644 --- a/tests/integration/Controllers/CrudController/Create/CreateTest.php +++ b/tests/integration/Controllers/CrudController/Create/CreateTest.php @@ -122,6 +122,41 @@ public function testCreatingManualRedirectUpdatesOlderToAvoidChains() ); } + public function testCreatingReverseRedirectOfExistingRedirectDeletesOldRedirect() + { + $oldRedirect = $this->createRedirect('/slug/alfa', '/slug/bravo'); + $this->assertRedirectCreated($oldRedirect); + + $request = $this->createPostRequest([ + 'redirect_from' => '/slug/bravo', + 'redirect_to' => '/slug/alfa', + 'redirect_locale' => 'da', + 'redirect_code' => 301 + ]); + + $crudController = new CrudController($this->redirectRepository, $request); + $crudController->handlePost(); + + $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); + + try { + $redirects = $this->redirectRepository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); + return; + } + + $this->assertCount(1, $redirects); + + $this->assertRedirect( + 0, + $redirects->first(), + '/slug/bravo', + '/slug/alfa', + 'manual' + ); + } + public function testCanCreateRedirectWithToWhichAlreadyExists() { $existingRedirect = $this->createRedirect('/from/something', '/to/destination'); diff --git a/tests/integration/Controllers/CrudController/Update/UpdateTest.php b/tests/integration/Controllers/CrudController/Update/UpdateTest.php index 8e98919..8edf8fa 100644 --- a/tests/integration/Controllers/CrudController/Update/UpdateTest.php +++ b/tests/integration/Controllers/CrudController/Update/UpdateTest.php @@ -113,6 +113,44 @@ public function testUpdatingRedirectWithToWhichAlreadyExistsInFromAvoidsMakingCh $this->assertSameRedirects($updatingRedirect, $redirectsAfter->last()); } + public function testUpdatingReverseRedirectOfExistingRedirectDeletesOldRedirect() + { + $oldRedirect = $this->createRedirect('/slug/alfa', '/slug/bravo'); + $this->assertRedirectCreated($oldRedirect); + $updatingRedirect = $this->createRedirect('/some/other', '/redirect'); + $this->assertRedirectCreated($updatingRedirect, 2); + + $request = $this->createPostRequest([ + 'redirect_id' => $updatingRedirect->getID(), + 'redirect_from' => '/slug/bravo', + 'redirect_to' => '/slug/alfa', + 'redirect_locale' => 'da', + 'redirect_code' => 301 + ]); + + $crudController = new CrudController($this->redirectRepository, $request); + $crudController->handlePost(); + + $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); + + try { + $redirects = $this->redirectRepository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); + return; + } + + $this->assertCount(1, $redirects); + + $this->assertRedirect( + 0, + $redirects->first(), + '/slug/bravo', + '/slug/alfa', + 'manual' + ); + } + public function testCanUpdateRedirectWithToWhichAlreadyExists() { $existingRedirect = $this->createRedirect('/from/something', '/to/destination'); diff --git a/tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php b/tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php new file mode 100644 index 0000000..6c209d6 --- /dev/null +++ b/tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php @@ -0,0 +1,119 @@ +setFrom('/from/slug') + ->setTo('/to/slug') + ->setKeepQuery(false) + ->setType('test') + ->setCode(301) + ->setLocale('da'); + + try { + $this->repository->save($redirect); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed saving the redirect (%s)', $exception->getMessage())); + return; + } + $this->assertGreaterThan(0,$redirect->getID()); + try { + $redirects = $this->repository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); + return; + } + $this->assertCount(1, $redirects); + $this->assertSameRedirects($redirect, $redirects->first()); + } + + public function testCannotSaveRedirectWhereFromAndToAreIdentical() + { + $redirect = new Redirect(); + $redirect->setFrom('/from/slug') + ->setTo('/from/slug') + ->setKeepQuery(false) + ->setType('test') + ->setCode(301) + ->setLocale('da'); + + try { + $this->repository->save($redirect); + } catch (IdenticalFromToException $exception) { + $this->assertSame('A redirect with the same from and to, cannot be created!', $exception->getMessage()); + try { + $redirects = $this->repository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); + return; + } + $this->assertEmpty($redirects); + return; + } + $this->fail('Failed throwing IdenticalFromToException!'); + } + + public function testRemovingChainsDoesNotCreateLoops() + { + $oldRedirect = $this->createRedirect('/from/a', '/to/b'); + try { + $createdRedirects = $this->repository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); + return; + } + $this->assertCount(1, $createdRedirects); + $this->assertSameRedirects($oldRedirect, $createdRedirects->first()); + + $newRedirect = new Redirect(); + $newRedirect->setFrom('/to/b') + ->setTo('/from/a') + ->setType('test') + ->setCode(301) + ->setLocale('da'); + + $database = new DB(); + $database->setTable(Migrate::REDIRECTS_TABLE); + try { + if ($redirectID = $database->insert($newRedirect->toArray())) { + $newRedirect->setID($redirectID); + } + } catch (\Exception $exception) { + $this->fail(sprintf('Failed saving redirect (%s)', $exception->getMessage())); + return; + } + + + try { + $this->repository->removeChainsByRedirect($newRedirect); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed removing redirect chains (%s)', $exception->getMessage())); + return; + } + + try { + $redirects = $this->repository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); + return; + } + + $this->assertCount(1, $redirects); + $this->assertSameRedirects($newRedirect, $redirects->first()); + } +} diff --git a/tests/integration/Repositories/RedirectRepositoryTestCase.php b/tests/integration/Repositories/RedirectRepositoryTestCase.php index f9c1b57..3805108 100644 --- a/tests/integration/Repositories/RedirectRepositoryTestCase.php +++ b/tests/integration/Repositories/RedirectRepositoryTestCase.php @@ -12,7 +12,7 @@ class RedirectRepositoryTestCase extends TestCase /** @var RedirectRepository */ protected $repository; - public function setUp() + public function setUp(bool $bootstrapRedirects = true) { parent::setUp(); try { @@ -20,7 +20,10 @@ public function setUp() } catch (\Exception $exception) { $this->fail(sprintf('Failed instantiating RedirectRepository (%s)', $exception->getMessage())); } - $this->bootstrapRedirects(); + + if ($bootstrapRedirects) { + $this->bootstrapRedirects(); + } } protected function bootstrapRedirects() diff --git a/wp-bonnier-redirect.php b/wp-bonnier-redirect.php index 5f3e0cd..b1ffb03 100644 --- a/wp-bonnier-redirect.php +++ b/wp-bonnier-redirect.php @@ -1,7 +1,7 @@ Date: Fri, 15 Mar 2019 14:04:51 +0100 Subject: [PATCH 2/2] Removing logging from scripts and prettifying tests --- src/Commands/RedirectCommands.php | 107 +++--------------- .../AlterRedirectTableAddIgnoreQuery.php | 30 ++++- .../Controllers/ControllerTestCase.php | 35 +++--- .../CrudController/Create/CreateTest.php | 64 ++--------- .../Create/NormalizationTest.php | 14 +-- .../CrudController/Create/ValidationTest.php | 24 +--- .../Update/NormalizationTest.php | 14 +-- .../CrudController/Update/UpdateTest.php | 56 ++------- .../CrudController/Update/ValidationTest.php | 14 +-- .../Controllers/ListControllerTest.php | 32 ++---- tests/integration/Models/LogTest.php | 27 ++++- tests/integration/Models/RedirectTest.php | 31 +++-- .../Observers/Category/CategoryChangeTest.php | 14 +-- .../Observers/Category/SlugChangeTest.php | 22 +--- .../Observers/Category/StatusChangeTest.php | 21 +--- .../Observers/CategoryObserverTest.php | 4 +- .../Observers/ObserverTestCase.php | 20 ++++ .../Observers/Post/CategoryChangeTest.php | 27 +---- tests/integration/Observers/Post/LogTest.php | 13 +-- .../Observers/Post/SlugChangesTest.php | 25 +--- .../Observers/Post/StatusChangeTest.php | 54 ++------- .../Observers/Tag/SlugChangeTest.php | 7 +- .../Observers/Tag/StatusChangeTest.php | 7 +- .../integration/Observers/TagObserverTest.php | 4 +- .../FindingRedirectTest.php | 14 +-- .../QueryParamsRedirectTest.php | 14 +-- .../RedirectRepository/SavingRedirectTest.php | 41 ++----- .../WildcardRedirectTest.php | 53 ++------- .../RedirectRepositoryTestCase.php | 35 ++++-- 29 files changed, 241 insertions(+), 582 deletions(-) diff --git a/src/Commands/RedirectCommands.php b/src/Commands/RedirectCommands.php index 6a33927..0559888 100644 --- a/src/Commands/RedirectCommands.php +++ b/src/Commands/RedirectCommands.php @@ -9,34 +9,20 @@ use Bonnier\WP\Redirect\Models\Redirect; use Bonnier\WP\Redirect\Repositories\RedirectRepository; use cli\progress\Bar; -use Illuminate\Support\Str; -use League\Csv\Writer; use function WP_CLI\Utils\make_progress_bar; class RedirectCommands extends \WP_CLI_Command { - private $csvHandle; - /** * Normalizes all from and to urls in the redirect table * - * ## OPTIONS - * [--output=] - * : Optional: Specify a csv filename where updates are recorded. - * * ## EXAMPLES * wp bonnier redirect redirects normalize * - * @param $args - * @param $assocArgs - * * @throws \WP_CLI\ExitException */ - public function normalize($args, $assocArgs) + public function normalize() { - if ($output = $assocArgs['output'] ?? null) { - $this->initCSV($output); - } $database = new DB(); $database->setTable(Migrate::REDIRECTS_TABLE); try { @@ -48,7 +34,10 @@ public function normalize($args, $assocArgs) } if ($results->isNotEmpty()) { /** @var Bar $progress */ - $progress = make_progress_bar(sprintf('Normalizing %s redirects', number_format($results->count())), $results->count()); + $progress = make_progress_bar( + sprintf('Normalizing %s redirects', number_format($results->count())), + $results->count() + ); $results->each(function (array $redirect) use ($database, $progress) { $redirectID = $redirect['id']; unset($redirect['id']); @@ -58,23 +47,16 @@ public function normalize($args, $assocArgs) $redirect['locale'] === '' ) { $database->delete($redirectID); - $this->csvWrite([$redirectID, $redirect['from'], $redirect['to'], $redirect['locale'], 'N/A', 'N/A', 'Deleted']); } else { - $newFrom = UrlHelper::normalizePath($redirect['from']); - $newTo = UrlHelper::normalizeUrl($redirect['to']); - if ($newFrom !== $redirect['from'] || $newTo !== $redirect['to']) { - $this->csvWrite([$redirectID, $redirect['from'], $redirect['to'], $redirect['locale'], $newFrom, $newTo, 'Updated']); - } - $redirect['from'] = $newFrom; + $redirect['from'] = UrlHelper::normalizePath($redirect['from']); $redirect['from_hash'] = hash('md5', $redirect['from']); $redirect['paramless_from_hash'] = hash('md5', parse_url($redirect['from'], PHP_URL_PATH)); - $redirect['to'] = $newTo; + $redirect['to'] = UrlHelper::normalizeUrl($redirect['to']); $redirect['to_hash'] = hash('md5', $redirect['to']); try { $database->update($redirectID, $redirect); } catch (DuplicateEntryException $exception) { $database->delete($redirectID); - $this->csvWrite([$redirectID, $redirect['from'], $redirect['to'], $redirect['locale'], 'N/A', 'N/A', 'Deleted']); } catch (\Exception $exception) { \WP_CLI::warning(sprintf('Failed updating redirect \'%s\'', $redirectID)); } @@ -88,20 +70,13 @@ public function normalize($args, $assocArgs) /** * Removes all chains in the redirect table * - * ## OPTIONS - * [--output=] - * : Optional: Specify a csv filename where updates are recorded. - * * ## EXAMPLES * wp bonnier redirect redirects unchain * * @throws \WP_CLI\ExitException */ - public function unchain($args, $assocArgs) + public function unchain() { - if ($output = $assocArgs['output'] ?? null) { - $this->initCSV($output); - } try { $repository = new RedirectRepository(new DB()); } catch (\Exception $exception) { @@ -110,14 +85,17 @@ public function unchain($args, $assocArgs) } \WP_CLI::line('Retrieving redirects...'); try { - $redirects = $repository->findAll()->reverse(); + $redirects = $repository->findAll(); } catch (\Exception $exception) { \WP_CLI::error(sprintf('Failed finding redirects (%s)', $exception->getMessage())); return; } $redirectCount = $redirects->count(); /** @var Bar $progress */ - $progress = make_progress_bar(sprintf('Unchaining %s redirects', number_format($redirectCount)), $redirectCount); + $progress = make_progress_bar( + sprintf('Unchaining %s redirects', number_format($redirectCount)), + $redirectCount + ); $redirects->each(function (Redirect $redirect) use ($repository, $progress) { try { $repository->save($redirect); @@ -127,64 +105,5 @@ public function unchain($args, $assocArgs) $progress->tick(); }); $progress->finish(); - if ($this->csvHandle) { - $unchainedRedirects = $repository->findAll(); - $progress = make_progress_bar(sprintf('Logging changes for %s redirects', number_format($redirectCount)), $redirectCount); - $redirects->each(function (Redirect $oldRedirect) use ($unchainedRedirects, $progress) { - /** @var Redirect|null $unchainedRedirect */ - $unchainedRedirect = $unchainedRedirects->first(function (Redirect $redirect) use ($oldRedirect) { - return $redirect->getID() === $oldRedirect->getID(); - }); - if ($unchainedRedirect) { - if ($unchainedRedirect->getFrom() !== $oldRedirect->getFrom() || - $unchainedRedirect->getTo() !== $oldRedirect->getTo() - ) { - $this->csvWrite([ - $oldRedirect->getID(), - $oldRedirect->getFrom(), - $oldRedirect->getTo(), - $oldRedirect->getLocale(), - $unchainedRedirect->getFrom(), - $unchainedRedirect->getTo(), - 'Updated' - ]); - } - } else { - $this->csvWrite([ - $oldRedirect->getID(), - $oldRedirect->getFrom(), - $oldRedirect->getTo(), - $oldRedirect->getLocale(), - 'N/A', - 'N/A', - 'Deleted' - ]); - } - $progress->tick(); - }); - $progress->finish(); - } - } - - private function initCSV(string $file) - { - $filepath = Str::finish(getcwd(), '/'); - if (Str::startsWith($file, './')) { - $filepath .= Str::after($file, './'); - } elseif (Str::startsWith($file, ['/', '~'])) { - \WP_CLI::error('Please define output file as a relative path (eg. /path/to/file.csv)'); - } else { - $filepath .= $file; - } - $this->csvHandle = Writer::createFromPath($filepath, 'w+'); - $this->csvHandle->insertOne(['ID', 'Old From', 'Old To', 'Locale', 'New From', 'New To', 'Notes']); - } - - private function csvWrite(array $row) - { - if (!$this->csvHandle) { - return; - } - $this->csvHandle->insertOne($row); } } diff --git a/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php b/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php index 850f563..9edb7ce 100644 --- a/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php +++ b/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php @@ -3,6 +3,7 @@ namespace Bonnier\WP\Redirect\Database\Migrations; use Illuminate\Support\Str; +use League\Csv\CannotInsertRecord; use League\Csv\Writer; class AlterRedirectTableAddIgnoreQuery implements Migration @@ -41,20 +42,41 @@ public static function verify(): bool private static function deduplicate(\wpdb $wpdb, string $table) { + if (defined('WP_ENV') && WP_ENV === 'testing') { + return; + } $csv = Writer::createFromPath(sprintf('%s_deleted_duplicates.csv', strtotime('now')), 'w+'); - $csv->insertOne(['id', 'from', 'to', 'locale']); + try { + $csv->insertOne(['id', 'from', 'to', 'locale']); + } catch (CannotInsertRecord $exception) { + return; + } $wpdb->update($table, ['locale' => 'nb'], ['locale' => 'no']); $wpdb->delete($table, ['locale' => '']); $sql = "SELECT `locale` FROM `$table` GROUP BY `locale`"; $locales = $wpdb->get_results($sql); foreach ($locales as $locale) { - $results = $wpdb->get_results("SELECT `from_hash`, count(id) AS cnt FROM `$table` WHERE `locale` = '$locale->locale' GROUP BY `from` HAVING cnt > 1"); + $results = $wpdb->get_results(" + SELECT `from_hash`, count(id) AS cnt + FROM `$table` + WHERE `locale` = '$locale->locale' + GROUP BY `from` HAVING cnt > 1 + "); foreach ($results as $result) { - $redirects = $wpdb->get_results("SELECT * FROM `$table` WHERE `from_hash` = '$result->from_hash' AND `locale` = '$locale->locale' ORDER BY id DESC"); + $redirects = $wpdb->get_results(" + SELECT * FROM `$table` + WHERE `from_hash` = '$result->from_hash' + AND `locale` = '$locale->locale' + ORDER BY id DESC + "); array_shift($redirects); // Keep the newest redirect foreach ($redirects as $redirect) { - $csv->insertOne([$redirect->id, $redirect->from, $redirect->to, $redirect->locale]); $wpdb->delete($table, ['id' => $redirect->id]); + try { + $csv->insertOne([$redirect->id, $redirect->from, $redirect->to, $redirect->locale]); + } catch (CannotInsertRecord $exception) { + continue; + } } } } diff --git a/tests/integration/Controllers/ControllerTestCase.php b/tests/integration/Controllers/ControllerTestCase.php index 08876e0..44f0c3c 100644 --- a/tests/integration/Controllers/ControllerTestCase.php +++ b/tests/integration/Controllers/ControllerTestCase.php @@ -64,23 +64,12 @@ protected function createRedirect( ->setLocale($locale) ->setCode($code); - try { - return $this->redirectRepository->save($redirect); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed saving redirect (%s)', $exception->getMessage())); - } - - return null; + return $this->save($redirect); } protected function assertRedirectCreated(Redirect $redirect, int $count = 1) { - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount($count, $redirects); $this->assertSameRedirects($redirect, $redirects->last()); } @@ -111,4 +100,24 @@ protected function assertNotices(array $expected, array $actual) $this->assertContains($notice['message'], $actual[$index][$notice['type']]); } } + + protected function save(Redirect &$redirect) + { + try { + return $this->redirectRepository->save($redirect); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed saving redirect (%s)', $exception->getMessage())); + return null; + } + } + + protected function findAllRedirects() + { + try { + return $this->redirectRepository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); + return null; + } + } } diff --git a/tests/integration/Controllers/CrudController/Create/CreateTest.php b/tests/integration/Controllers/CrudController/Create/CreateTest.php index a066562..52ee869 100644 --- a/tests/integration/Controllers/CrudController/Create/CreateTest.php +++ b/tests/integration/Controllers/CrudController/Create/CreateTest.php @@ -20,12 +20,7 @@ public function testCanCreateNewRedirect() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertManualRedirect($redirects->first(), '/example/from/slug', '/example/to/slug'); @@ -43,12 +38,7 @@ public function testCannotCreateMultipleRedirectsWithSameFrom() $crudController = new CrudController($this->redirectRepository, $request); $crudController->handlePost(); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertManualRedirect($redirects->first(), '/example/from/slug', '/example/to/slug'); $newRequest = $this->createPostRequest([ @@ -67,12 +57,7 @@ public function testCannotCreateMultipleRedirectsWithSameFrom() 'A redirect with the same \'from\' and \'locale\' already exists!' ); - try { - $newRedirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $newRedirects = $this->findAllRedirects(); $this->assertCount(1, $newRedirects); $this->assertManualRedirect($newRedirects->first(), '/example/from/slug', '/example/to/slug'); } @@ -99,12 +84,7 @@ public function testCreatingManualRedirectUpdatesOlderToAvoidChains() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $newRedirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $newRedirects = $this->findAllRedirects(); $this->assertCount(2, $newRedirects); $this->assertRedirect( @@ -139,13 +119,7 @@ public function testCreatingReverseRedirectOfExistingRedirectDeletesOldRedirect( $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); - return; - } - + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( @@ -175,12 +149,7 @@ public function testCanCreateRedirectWithToWhichAlreadyExists() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(2, $redirects); $this->assertSameRedirects($existingRedirect, $redirects->first()); $this->assertRedirect( @@ -206,12 +175,7 @@ public function testCanCreateWildcardRedirect() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( 0, @@ -238,12 +202,7 @@ public function testCanCreateRedirectThatKeepsQueryParams() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( 0, @@ -278,12 +237,7 @@ public function testCreatingRedirectWithToWhichAlreadyExistsInFromAvoidsMakingCh ]; $this->assertNotices($expectedNotices, $notices); - try { - $redirectsAfter = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirectsAfter = $this->findAllRedirects(); $this->assertCount(2, $redirectsAfter); $this->assertSameRedirects($existingRedirect, $redirectsAfter->first()); $this->assertRedirect( diff --git a/tests/integration/Controllers/CrudController/Create/NormalizationTest.php b/tests/integration/Controllers/CrudController/Create/NormalizationTest.php index 6c423e3..b0eed39 100644 --- a/tests/integration/Controllers/CrudController/Create/NormalizationTest.php +++ b/tests/integration/Controllers/CrudController/Create/NormalizationTest.php @@ -27,12 +27,7 @@ public function testCreatingRedirectsWillNormalizeFromInput(string $from, string $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( 0, @@ -63,12 +58,7 @@ public function testCreatingRedirectsWillNormalizeToInput(string $toUrl, string $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( 0, diff --git a/tests/integration/Controllers/CrudController/Create/ValidationTest.php b/tests/integration/Controllers/CrudController/Create/ValidationTest.php index a6acc2c..0935d5f 100644 --- a/tests/integration/Controllers/CrudController/Create/ValidationTest.php +++ b/tests/integration/Controllers/CrudController/Create/ValidationTest.php @@ -26,11 +26,7 @@ public function testCannotCreateRedirectWithEmptyFrom() $this->assertArrayHasKey('redirect_from', $errors); $this->assertSame('The \'from\'-value cannot be empty!', $errors['redirect_from']); - try { - $this->assertNull($this->redirectRepository->findAll()); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - } + $this->assertNull($this->findAllRedirects()); } public function testCannotCreateRedirectWithEmptyTo() @@ -52,11 +48,7 @@ public function testCannotCreateRedirectWithEmptyTo() $this->assertArrayHasKey('redirect_to', $errors); $this->assertSame('The \'to\'-value cannot be empty!', $errors['redirect_to']); - try { - $this->assertNull($this->redirectRepository->findAll()); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - } + $this->assertNull($this->findAllRedirects()); } /** @@ -84,11 +76,7 @@ public function testCannotCreateDestructiveRedirects(string $from, string $error $this->assertArrayHasKey('redirect_from', $errors); $this->assertSame($error, $errors['redirect_from']); - try { - $this->assertNull($this->redirectRepository->findAll()); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - } + $this->assertNull($this->findAllRedirects()); } /** @@ -117,11 +105,7 @@ public function testCannotCreateRedirectWithSameFromAndTo(string $fromUrl, strin $this->assertNoticeIs($crudController->getNotices(), 'error', 'From and to urls seems identical!'); - try { - $this->assertNull($this->redirectRepository->findAll()); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - } + $this->assertNull($this->findAllRedirects()); } public function destructiveRedirectsProvider() diff --git a/tests/integration/Controllers/CrudController/Update/NormalizationTest.php b/tests/integration/Controllers/CrudController/Update/NormalizationTest.php index f69bde9..4e40422 100644 --- a/tests/integration/Controllers/CrudController/Update/NormalizationTest.php +++ b/tests/integration/Controllers/CrudController/Update/NormalizationTest.php @@ -31,12 +31,7 @@ public function testCreatingRedirectsWillNormalizeFromInput(string $from, string $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirectsAfter = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirectsAfter = $this->findAllRedirects(); $this->assertCount(1, $redirectsAfter); $this->assertRedirect( 0, @@ -71,12 +66,7 @@ public function testCreatingRedirectsWillNormalizeToInput(string $toUrl, string $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirectsAfter = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirectsAfter = $this->findAllRedirects(); $this->assertCount(1, $redirectsAfter); $this->assertRedirect( 0, diff --git a/tests/integration/Controllers/CrudController/Update/UpdateTest.php b/tests/integration/Controllers/CrudController/Update/UpdateTest.php index 8edf8fa..2fa955c 100644 --- a/tests/integration/Controllers/CrudController/Update/UpdateTest.php +++ b/tests/integration/Controllers/CrudController/Update/UpdateTest.php @@ -26,12 +26,7 @@ public function testCanUpdateToRedirect() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $updatedRedirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $updatedRedirects = $this->findAllRedirects(); $this->assertCount(1, $updatedRedirects); $this->assertRedirect( 0, @@ -60,12 +55,7 @@ public function testCanUpdateFromRedirect() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $updatedRedirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $updatedRedirects = $this->findAllRedirects(); $this->assertCount(1, $updatedRedirects); $this->assertRedirect( 0, @@ -101,12 +91,7 @@ public function testUpdatingRedirectWithToWhichAlreadyExistsInFromAvoidsMakingCh $this->assertNotices($expectedNotices, $crudController->getNotices()); - try { - $redirectsAfter = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirectsAfter = $this->findAllRedirects(); $this->assertCount(2, $redirectsAfter); $this->assertSameRedirects($existingRedirect, $redirectsAfter->first()); $updatingRedirect->setTo('/final/destination'); @@ -133,12 +118,7 @@ public function testUpdatingReverseRedirectOfExistingRedirectDeletesOldRedirect( $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); @@ -171,12 +151,7 @@ public function testCanUpdateRedirectWithToWhichAlreadyExists() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(2, $redirects); $this->assertSameRedirects($existingRedirect, $redirects->first()); $this->assertRedirect( @@ -213,12 +188,7 @@ public function testUpdatingManualRedirectUpdatesOlderToAvoidChains() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $newRedirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $newRedirects = $this->findAllRedirects(); $this->assertCount(2, $newRedirects); $this->assertRedirect( @@ -254,12 +224,7 @@ public function testCanUpdateWildcardRedirect() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( 0, @@ -291,12 +256,7 @@ public function testCanUpdateRedirectThatKeepsQueryParams() $this->assertNoticeWasSaveRedirectMessage($crudController->getNotices()); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( 0, diff --git a/tests/integration/Controllers/CrudController/Update/ValidationTest.php b/tests/integration/Controllers/CrudController/Update/ValidationTest.php index 57b1411..88495ed 100644 --- a/tests/integration/Controllers/CrudController/Update/ValidationTest.php +++ b/tests/integration/Controllers/CrudController/Update/ValidationTest.php @@ -31,12 +31,7 @@ public function testCannotUpdateFromIfAlreadyExists() 'A redirect with the same \'from\' and \'locale\' already exists!' ); - try { - $redirectsAfter = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirectsAfter = $this->findAllRedirects(); $this->assertCount(2, $redirectsAfter); $this->assertSameRedirects($existingRedirect, $redirectsAfter->first()); $this->assertRedirect( @@ -78,12 +73,7 @@ public function testCannotUpdateRedirectWithSameFromAndTo(string $fromUrl, strin $this->assertNoticeIs($crudController->getNotices(), 'error', 'From and to urls seems identical!'); - try { - $redirectsAfter = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirectsAfter = $this->findAllRedirects(); $this->assertCount(1, $redirectsAfter); $this->assertSameRedirects($redirect, $redirectsAfter->first()); } diff --git a/tests/integration/Controllers/ListControllerTest.php b/tests/integration/Controllers/ListControllerTest.php index 2bdefa0..e3e8c5e 100644 --- a/tests/integration/Controllers/ListControllerTest.php +++ b/tests/integration/Controllers/ListControllerTest.php @@ -22,12 +22,7 @@ public function testCanDeleteSingleRedirect() '/new/slug' ); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( @@ -47,13 +42,14 @@ public function testCanDeleteSingleRedirect() ]); $listController = new ListController($this->redirectRepository, $request); - $listController->prepare_items(); - try { - $this->assertNull($this->redirectRepository->findAll()); + $listController->prepare_items(); } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); + $this->fail(sprintf('Failed preparing items for ListController (%s)', $exception->getMessage())); + return; } + + $this->assertNull($this->findAllRedirects()); } public function testCanBulkDeleteRedirects() @@ -73,12 +69,7 @@ public function testCanBulkDeleteRedirects() ), ]); - try { - $createdRedirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $createdRedirects = $this->findAllRedirects(); $this->assertCount(3, $createdRedirects); $createdRedirects->each(function (Redirect $redirect, int $index) use ($redirects) { /** @var Redirect $expectedRedirect */ @@ -103,12 +94,13 @@ public function testCanBulkDeleteRedirects() ]); $listController = new ListController($this->redirectRepository, $request); - $listController->prepare_items(); - try { - $this->assertNull($this->redirectRepository->findAll()); + $listController->prepare_items(); } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); + $this->fail(sprintf('Failed preparing items for ListController (%s)', $exception->getMessage())); + return; } + + $this->assertNull($this->findAllRedirects()); } } diff --git a/tests/integration/Models/LogTest.php b/tests/integration/Models/LogTest.php index a377c71..58bc5b2 100644 --- a/tests/integration/Models/LogTest.php +++ b/tests/integration/Models/LogTest.php @@ -29,7 +29,8 @@ public function testCanSaveLog() $log->setSlug('/path/to/post') ->setType('post') ->setWpID(1); - $log = $this->logRepository->save($log); + + $log = $this->save($log); $savedLog = $this->logRepository->findById($log->getID()); @@ -44,9 +45,29 @@ public function testCanSaveLogsWithSameSlug() $log->setSlug('/path/to/post') ->setType('post') ->setWpID(1); - $this->logRepository->save($log); + $this->save($log); } - $this->assertCount(10, $this->logRepository->findAll()); + $this->assertCount(10, $this->findAll()); + } + + private function save(Log &$log) + { + try { + return $this->logRepository->save($log); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed saving log (%s)', $exception->getMessage())); + return null; + } + } + + private function findAll() + { + try { + return $this->logRepository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting logs (%s)', $exception->getMessage())); + return null; + } } } diff --git a/tests/integration/Models/RedirectTest.php b/tests/integration/Models/RedirectTest.php index b431c68..ab934bf 100644 --- a/tests/integration/Models/RedirectTest.php +++ b/tests/integration/Models/RedirectTest.php @@ -91,13 +91,7 @@ public function testCanSaveRedirect() ->setTo('/my/new/slug') ->setLocale('da') ->setCode(301); - try { - $redirect = $this->redirectRepository->save($redirect); - } catch (DuplicateEntryException $e) { - $this->fail(sprintf('Failed saving the redirect (%s)', $e->getMessage())); - } catch (\Exception $e) { - $this->fail(sprintf('Failed saving the redirect (%s)', $e->getMessage())); - } + $redirect = $this->save($redirect); $this->assertNotEquals(0, $redirect->getID()); $savedRedirect = $this->redirectRepository->getRedirectById($redirect->getID()); @@ -114,11 +108,7 @@ public function testCannotSaveRedirectsWithSameFrom() ->setLocale('da') ->setCode(301); - try { - $this->redirectRepository->save($firstRedirect); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed saving redirect (%s)', $exception->getMessage())); - } + $this->save($firstRedirect); $secondRedirect = new Redirect(); $secondRedirect->setFrom('/my/old/slug') @@ -149,12 +139,7 @@ public function testCanCreateMultipleRedirectsWithSameDestination() ->setTo('/same/destination/slug') ->setLocale('da') ->setCode(301); - try { - $savedRedirect = $this->redirectRepository->save($redirect); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed saving redirect (%s)', $exception->getMessage())); - return; - } + $savedRedirect = $this->save($redirect); $this->assertInstanceOf(Redirect::class, $savedRedirect); $this->assertGreaterThan(0, $savedRedirect->getID()); } @@ -212,4 +197,14 @@ public function addingQueryParamsProvider() ] ]; } + + private function save(Redirect &$redirect) + { + try { + return $this->redirectRepository->save($redirect); + } catch (\Exception $e) { + $this->fail(sprintf('Failed saving the redirect (%s)', $e->getMessage())); + return null; + } + } } diff --git a/tests/integration/Observers/Category/CategoryChangeTest.php b/tests/integration/Observers/Category/CategoryChangeTest.php index 0a3fecf..0b87ee0 100644 --- a/tests/integration/Observers/Category/CategoryChangeTest.php +++ b/tests/integration/Observers/Category/CategoryChangeTest.php @@ -45,12 +45,7 @@ public function testCanChangeParentCategoryAndHaveRedirectsCreated() $this->assertSame('/animals/carnivorous/' . $post->post_name, $this->getPostSlug($post)); } - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(5, $redirects); $categoryRedirect = $redirects->shift(); @@ -254,12 +249,7 @@ public function testCanChangeParentCategoryAndHaveRedirectsCreatedOnMultipleLeve ); } - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(12, $redirects); foreach ($expectedFroms as $index => $expectedFrom) { diff --git a/tests/integration/Observers/Category/SlugChangeTest.php b/tests/integration/Observers/Category/SlugChangeTest.php index 8ba2a56..1746a4a 100644 --- a/tests/integration/Observers/Category/SlugChangeTest.php +++ b/tests/integration/Observers/Category/SlugChangeTest.php @@ -47,12 +47,7 @@ public function testRedirectsCreatedForCategoryAndPostsWhenCategorySlugChanges() $this->assertSame('/dinosaur-fossils/' . $post->post_name, $this->getPostSlug($post)); } - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); // One redirect per post (30) and one redirect for the category - 31 in total. $this->assertCount(31, $redirects); @@ -120,12 +115,7 @@ public function testRedirectsCreatedForAllCategoriesAndPostsWhenTopCategorySlugC $expectedTos[] = '/dinosaur-fossils/carnivorous/post-' . $index; } - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); // One redirect per post (30) and one redirect for the top category and sub category - 32 in total. $this->assertCount(32, $redirects); @@ -195,12 +185,8 @@ public function testCanHandleRedirectsForMultipleChildCategoriesOnSlugChange() // Now we should see 6 redirects for the parent category page and the 4 subcategories and the subsubcategory. // Every subcategory has 5 articles, which should also be redirected - 20 additional redirects // That should give a total of 31 redirects. - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); + $this->assertCount(31, $redirects); $this->assertCount(31, $expectedFroms); diff --git a/tests/integration/Observers/Category/StatusChangeTest.php b/tests/integration/Observers/Category/StatusChangeTest.php index 31ceca8..23c1d77 100644 --- a/tests/integration/Observers/Category/StatusChangeTest.php +++ b/tests/integration/Observers/Category/StatusChangeTest.php @@ -17,12 +17,7 @@ public function testCategoryDeletionCreatesRedirect() wp_delete_category($category->term_id); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( @@ -65,12 +60,7 @@ public function testSubcategoryDeletionCreatesRedirects() $this->assertSame('/dinosaur/' . $post->post_name, $this->getPostSlug($post)); } - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(5, $redirects); @@ -121,12 +111,7 @@ public function testCanDeleteTopCategoryAndRedirectsAreCreated() $this->assertSame('/uncategorized/' . $post->post_name, $this->getPostSlug($post)); } - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(5, $redirects); $categoryRedirect = $redirects->shift(); diff --git a/tests/integration/Observers/CategoryObserverTest.php b/tests/integration/Observers/CategoryObserverTest.php index b0398c7..3ca139f 100644 --- a/tests/integration/Observers/CategoryObserverTest.php +++ b/tests/integration/Observers/CategoryObserverTest.php @@ -34,7 +34,7 @@ public function testLogIsCreated() { $category = $this->factory()->category->create_and_get(); - $logs = $this->logRepository->findAll(); + $logs = $this->findAllLogs(); $this->assertCount(1, $logs); $this->assertSame('category', $logs->first()->getType()); $this->assertSame($category->term_id, $logs->first()->getWpID()); @@ -43,7 +43,7 @@ public function testLogIsCreated() 'slug' => 'updated-category', ]); - $logs = $this->logRepository->findAll(); + $logs = $this->findAllLogs(); $this->assertCount(2, $logs); /** @var Log $log */ $log = $logs->last(); diff --git a/tests/integration/Observers/ObserverTestCase.php b/tests/integration/Observers/ObserverTestCase.php index bfba8f0..d781e99 100644 --- a/tests/integration/Observers/ObserverTestCase.php +++ b/tests/integration/Observers/ObserverTestCase.php @@ -46,4 +46,24 @@ protected function assertLog(\WP_Post $post, Log $log, ?string $slug = null) $this->assertSame($post->ID, $log->getWpID()); $this->assertSame($post->post_type, $log->getType()); } + + protected function findAllRedirects() + { + try { + return $this->redirectRepository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); + return null; + } + } + + protected function findAllLogs() + { + try { + return $this->logRepository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed finding logs (%s)', $exception->getMessage())); + return null; + } + } } diff --git a/tests/integration/Observers/Post/CategoryChangeTest.php b/tests/integration/Observers/Post/CategoryChangeTest.php index 49fffe6..beca4ae 100644 --- a/tests/integration/Observers/Post/CategoryChangeTest.php +++ b/tests/integration/Observers/Post/CategoryChangeTest.php @@ -14,23 +14,14 @@ public function testChangingCategoryCreatesRedirects() 'post_category' => [$category->term_id], ]); - try { - $this->assertNull($this->redirectRepository->findAll()); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - } + $this->assertNull($this->findAllRedirects()); $newCategory = $this->getCategory(); $this->updatePost($post->ID, [ 'post_category' => [$newCategory->term_id], ]); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( $post->ID, @@ -66,12 +57,7 @@ public function testChangingCategoryMultipleTimesDoesNotCreateRedirectChains() ]); $newSlug = sprintf('/%s/%s', $category->slug, $post->post_name); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount($index + 1, $redirects); $redirects->each(function (Redirect $redirect, int $index) use ($post, $newSlug, $slugs) { $this->assertRedirect( @@ -112,12 +98,7 @@ public function testChangingSlugAndCategoryCreatesRedirect() $this->assertSame('/fossils/t-rex-is-awesome', $this->getPostSlug($post)); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( $post->ID, diff --git a/tests/integration/Observers/Post/LogTest.php b/tests/integration/Observers/Post/LogTest.php index 77cd061..e3a797e 100644 --- a/tests/integration/Observers/Post/LogTest.php +++ b/tests/integration/Observers/Post/LogTest.php @@ -10,12 +10,7 @@ public function testLogIsCreated() { $post = $this->getPost(); - try { - $logs = $this->logRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding logs (%s)', $exception->getMessage())); - return; - } + $logs = $this->findAllLogs(); $this->assertCount(1, $logs); $this->assertLog($post, $logs->first()); @@ -23,11 +18,7 @@ public function testLogIsCreated() 'post_name' => 'log-created', ]); - try { - $logs = $this->logRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding logs (%s)', $exception->getMessage())); - } + $logs = $this->findAllLogs(); $this->assertCount(2, $logs); // Create and update logs $this->assertLog($post, $logs->last(), '/uncategorized/log-created'); } diff --git a/tests/integration/Observers/Post/SlugChangesTest.php b/tests/integration/Observers/Post/SlugChangesTest.php index ff51b91..ec34f25 100644 --- a/tests/integration/Observers/Post/SlugChangesTest.php +++ b/tests/integration/Observers/Post/SlugChangesTest.php @@ -13,7 +13,7 @@ public function testSlugChangeCreatesRedirect() $initialSlug = '/uncategorized/' . $post->post_name; - $createdLogs = $this->logRepository->findAll(); + $createdLogs = $this->findAllLogs(); $this->assertCount(1, $createdLogs); $this->assertLog($post, $createdLogs->first(), $initialSlug); @@ -21,16 +21,11 @@ public function testSlugChangeCreatesRedirect() 'post_name' => 'new-post-slug' ]); - $logs = $this->logRepository->findAll(); + $logs = $this->findAllLogs(); $this->assertCount(2, $logs); $this->assertLog($post, $logs->last(), '/uncategorized/new-post-slug'); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( $post->ID, @@ -62,12 +57,7 @@ public function testSlugChangesDoesntCreateRedirectChains() }, array_merge([$initialPostName], $postNames)); foreach ($postNames as $index => $postName) { - try { - $redirectsBefore = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirectsBefore = $this->findAllRedirects(); if ($index === 0) { $this->assertNull($redirectsBefore); } else { @@ -79,12 +69,7 @@ public function testSlugChangesDoesntCreateRedirectChains() ]); $newSlug = '/uncategorized/' . $postName; - try { - $redirectsAfter = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirectsAfter = $this->findAllRedirects(); $this->assertCount($index + 1, $redirectsAfter); $redirectsAfter->each(function (Redirect $redirect, int $index) use ($post, $newSlug, $slugs) { $this->assertRedirect( diff --git a/tests/integration/Observers/Post/StatusChangeTest.php b/tests/integration/Observers/Post/StatusChangeTest.php index 44919fd..569717c 100644 --- a/tests/integration/Observers/Post/StatusChangeTest.php +++ b/tests/integration/Observers/Post/StatusChangeTest.php @@ -29,12 +29,7 @@ public function testTrashedPostCreatesRedirectToParentCategory() 'post_status' => 'trash', ]); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( @@ -68,12 +63,7 @@ public function testUnpublishPostCreatesRedirectToParentCategory() $this->updatePost($post->ID, [ 'post_status' => 'draft', ]); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( @@ -107,12 +97,7 @@ public function testCanUnpublishAndRepublishPostWithNewSlug() 'post_status' => 'draft' ]); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( $post->ID, @@ -129,12 +114,7 @@ public function testCanUnpublishAndRepublishPostWithNewSlug() $this->assertSame('/dinosaur/carnivorous/t-rex-is-awesome', $this->getPostSlug($post)); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( $post->ID, @@ -167,12 +147,7 @@ public function testCanUnpublishAndRepublishPostWithSameSlug() 'post_status' => 'draft' ]); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( $post->ID, @@ -188,11 +163,7 @@ public function testCanUnpublishAndRepublishPostWithSameSlug() $this->assertSame('/dinosaur/carnivorous/t-rex', $this->getPostSlug($post)); - try { - $this->assertNull($this->redirectRepository->findAll()); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - } + $this->assertNull($this->findAllRedirects()); } public function testCanDeletePostAndCreateNewPostWithSameSlug() @@ -217,12 +188,7 @@ public function testCanDeletePostAndCreateNewPostWithSameSlug() 'post_status' => 'trash' ]); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( $post->ID, @@ -240,10 +206,6 @@ public function testCanDeletePostAndCreateNewPostWithSameSlug() $this->assertSame('/dinosaur/carnivorous/t-rex', $this->getPostSlug($newPost)); - try { - $this->assertNull($this->redirectRepository->findAll()); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - } + $this->assertNull($this->findAllRedirects()); } } diff --git a/tests/integration/Observers/Tag/SlugChangeTest.php b/tests/integration/Observers/Tag/SlugChangeTest.php index 04741ff..ae5b886 100644 --- a/tests/integration/Observers/Tag/SlugChangeTest.php +++ b/tests/integration/Observers/Tag/SlugChangeTest.php @@ -22,12 +22,7 @@ public function testChangeTagSlugCreatesRedirect() $this->assertSame('/reptiles', $this->getTagSlug($tag)); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $redirect = $redirects->first(); $this->assertRedirect( diff --git a/tests/integration/Observers/Tag/StatusChangeTest.php b/tests/integration/Observers/Tag/StatusChangeTest.php index 8a36078..fd2d110 100644 --- a/tests/integration/Observers/Tag/StatusChangeTest.php +++ b/tests/integration/Observers/Tag/StatusChangeTest.php @@ -17,12 +17,7 @@ public function testDeletingTagRedirectsToFrontpage() wp_delete_term($tag->term_id, $tag->taxonomy); - try { - $redirects = $this->redirectRepository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertRedirect( $tag->term_id, diff --git a/tests/integration/Observers/TagObserverTest.php b/tests/integration/Observers/TagObserverTest.php index ca45b15..27bf89d 100644 --- a/tests/integration/Observers/TagObserverTest.php +++ b/tests/integration/Observers/TagObserverTest.php @@ -34,7 +34,7 @@ public function testLogIsCreated() { $tag = $this->factory()->tag->create_and_get(); - $logs = $this->logRepository->findAll(); + $logs = $this->findAllLogs(); $this->assertCount(1, $logs); $this->assertSame('post_tag', $logs->first()->getType()); $this->assertSame($tag->term_id, $logs->first()->getWpID()); @@ -43,7 +43,7 @@ public function testLogIsCreated() 'slug' => 'updated-tag', ]); - $logs = $this->logRepository->findAll(); + $logs = $this->findAllLogs(); $this->assertCount(2, $logs); /** @var Log $log */ $log = $logs->last(); diff --git a/tests/integration/Repositories/RedirectRepository/FindingRedirectTest.php b/tests/integration/Repositories/RedirectRepository/FindingRedirectTest.php index c238e5e..c08d23a 100644 --- a/tests/integration/Repositories/RedirectRepository/FindingRedirectTest.php +++ b/tests/integration/Repositories/RedirectRepository/FindingRedirectTest.php @@ -11,12 +11,7 @@ public function testCanFindSimpleRedirect() { $redirect = $this->createRedirect('/path/to/old/article', '/path/to/new/article'); - try { - $foundRedirect = $this->repository->findRedirectByPath('/path/to/old/article', 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } + $foundRedirect = $this->findRedirectByPath('/path/to/old/article'); $this->assertInstanceOf(Redirect::class, $foundRedirect); $this->assertSameRedirects($redirect, $foundRedirect); } @@ -31,12 +26,7 @@ public function testCanFindRedirectWithMalformedPath(string $path, string $from) { $redirect = $this->createRedirect($from, '/destination'); - try { - $foundRedirect = $this->repository->findRedirectByPath($path, 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } + $foundRedirect = $this->findRedirectByPath($path); $this->assertInstanceOf( Redirect::class, $foundRedirect, diff --git a/tests/integration/Repositories/RedirectRepository/QueryParamsRedirectTest.php b/tests/integration/Repositories/RedirectRepository/QueryParamsRedirectTest.php index 8b07770..ddc73cd 100644 --- a/tests/integration/Repositories/RedirectRepository/QueryParamsRedirectTest.php +++ b/tests/integration/Repositories/RedirectRepository/QueryParamsRedirectTest.php @@ -18,12 +18,7 @@ public function testIgnoringQueryParamsIgnoresAllQueryParams(string $path, strin { $redirect = $this->createRedirect($from, $destination); - try { - $foundRedirect = $this->repository->findRedirectByPath($path, 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } + $foundRedirect = $this->findRedirectByPath($path); $this->assertInstanceOf( Redirect::class, $foundRedirect, @@ -49,12 +44,7 @@ public function testKeepingQueryParamsActuallyKeepsAllQueryParams( ) { $redirect = $this->createRedirect($from, $destination, true); - try { - $foundRedirect = $this->repository->findRedirectByPath($path, 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } + $foundRedirect = $this->findRedirectByPath($path); $this->assertInstanceOf( Redirect::class, $foundRedirect, diff --git a/tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php b/tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php index 6c209d6..df8c3ec 100644 --- a/tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php +++ b/tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php @@ -25,19 +25,10 @@ public function testCanSaveARedirect() ->setCode(301) ->setLocale('da'); - try { - $this->repository->save($redirect); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed saving the redirect (%s)', $exception->getMessage())); - return; - } - $this->assertGreaterThan(0,$redirect->getID()); - try { - $redirects = $this->repository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); - return; - } + $this->saveRedirect($redirect); + + $this->assertGreaterThan(0, $redirect->getID()); + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertSameRedirects($redirect, $redirects->first()); } @@ -56,14 +47,10 @@ public function testCannotSaveRedirectWhereFromAndToAreIdentical() $this->repository->save($redirect); } catch (IdenticalFromToException $exception) { $this->assertSame('A redirect with the same from and to, cannot be created!', $exception->getMessage()); - try { - $redirects = $this->repository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); - return; - } - $this->assertEmpty($redirects); + $this->assertEmpty($this->findAllRedirects()); return; + } catch (\Exception $exception) { + $this->fail(sprintf('Saving redirect threw unexpected exception (%s)', $exception->getMessage())); } $this->fail('Failed throwing IdenticalFromToException!'); } @@ -71,12 +58,7 @@ public function testCannotSaveRedirectWhereFromAndToAreIdentical() public function testRemovingChainsDoesNotCreateLoops() { $oldRedirect = $this->createRedirect('/from/a', '/to/b'); - try { - $createdRedirects = $this->repository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); - return; - } + $createdRedirects = $this->findAllRedirects(); $this->assertCount(1, $createdRedirects); $this->assertSameRedirects($oldRedirect, $createdRedirects->first()); @@ -106,12 +88,7 @@ public function testRemovingChainsDoesNotCreateLoops() return; } - try { - $redirects = $this->repository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); - return; - } + $redirects = $this->findAllRedirects(); $this->assertCount(1, $redirects); $this->assertSameRedirects($newRedirect, $redirects->first()); diff --git a/tests/integration/Repositories/RedirectRepository/WildcardRedirectTest.php b/tests/integration/Repositories/RedirectRepository/WildcardRedirectTest.php index 12bccfa..6285d85 100644 --- a/tests/integration/Repositories/RedirectRepository/WildcardRedirectTest.php +++ b/tests/integration/Repositories/RedirectRepository/WildcardRedirectTest.php @@ -19,12 +19,8 @@ public function testCanFindWildcardRedirect(string $path, string $from) $this->assertTrue($redirect->isWildcard(), 'The created redirect wasn\'t a wildcard redirect!'); - try { - $foundRedirect = $this->repository->findRedirectByPath($path, 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } + $foundRedirect = $this->findRedirectByPath($path); + $this->assertInstanceOf( Redirect::class, $foundRedirect, @@ -39,21 +35,12 @@ public function testPrefersExactRedirectMatchInsteadOfWildcardRedirect() $wildcard = $this->createRedirect('/from/wildcard/*', '/destination'); $notWildcard = $this->createRedirect('/from/wildcard/exact', '/destination'); - try { - $createdRedirects = $this->repository->findAll()->take(-2); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed creating redirect (%s)', $exception->getMessage())); - return; - } + $createdRedirects = $this->findAllRedirects()->take(-2); + $this->assertSameRedirects($wildcard, $createdRedirects->first()); $this->assertSameRedirects($notWildcard, $createdRedirects->last()); - try { - $foundRedirect = $this->repository->findRedirectByPath('from/wildcard/exact/', 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } + $foundRedirect = $this->findRedirectByPath('from/wildcard/exact/'); $this->assertInstanceOf(Redirect::class, $foundRedirect); $this->assertSameRedirects($notWildcard, $foundRedirect); } @@ -63,21 +50,11 @@ public function testPrefersWildcardIfNoExactMatchExists() $wildcard = $this->createRedirect('/from/wildcard/*', '/destination'); $notWildcard = $this->createRedirect('/from/wildcard/exact', '/destination'); - try { - $createdRedirects = $this->repository->findAll()->take(-2); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed creating redirect (%s)', $exception->getMessage())); - return; - } + $createdRedirects = $this->findAllRedirects()->take(-2); $this->assertSameRedirects($wildcard, $createdRedirects->first()); $this->assertSameRedirects($notWildcard, $createdRedirects->last()); - try { - $foundRedirect = $this->repository->findRedirectByPath('from/wildcard/exact/path/', 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } + $foundRedirect = $this->findRedirectByPath('from/wildcard/exact/path/'); $this->assertInstanceOf(Redirect::class, $foundRedirect); $this->assertSameRedirects($wildcard, $foundRedirect); } @@ -86,13 +63,7 @@ public function testWildcardRedirectsKeepsQueryParams() { $redirect = $this->createRedirect('/from/*', '/destination', true); - try { - $foundRedirect = $this->repository->findRedirectByPath('/from/this/slug?c=d&a=b', 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } - + $foundRedirect = $this->findRedirectByPath('/from/this/slug?c=d&a=b'); $this->assertInstanceOf(Redirect::class, $foundRedirect); $this->assertSame($redirect->getID(), $foundRedirect->getID()); $this->assertSame('/from/*', $foundRedirect->getFrom()); @@ -103,13 +74,7 @@ public function testWildcardRedirectsIgnoresQueryParams() { $redirect = $this->createRedirect('/from/*', '/destination'); - try { - $foundRedirect = $this->repository->findRedirectByPath('/from/this/slug?c=d&a=b', 'da'); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); - return; - } - + $foundRedirect = $this->findRedirectByPath('/from/this/slug?c=d&a=b'); $this->assertInstanceOf(Redirect::class, $foundRedirect); $this->assertSameRedirects($redirect, $foundRedirect); } diff --git a/tests/integration/Repositories/RedirectRepositoryTestCase.php b/tests/integration/Repositories/RedirectRepositoryTestCase.php index 3805108..0b34994 100644 --- a/tests/integration/Repositories/RedirectRepositoryTestCase.php +++ b/tests/integration/Repositories/RedirectRepositoryTestCase.php @@ -163,12 +163,7 @@ protected function bootstrapRedirects() $this->createRedirect($url['from'], $url['to']); }); - try { - $createdRedirects = $this->repository->findAll(); - } catch (\Exception $exception) { - $this->fail(sprintf('Failed finding bootstrapped redirects (%s)', $exception->getMessage())); - return; - } + $createdRedirects = $this->findAllRedirects(); $this->assertCount(count($redirects), $createdRedirects); $createdRedirects->each(function (Redirect $redirect, int $index) use ($redirects) { $this->assertSame($redirects[$index]['from'], $redirect->getFrom()); @@ -192,11 +187,37 @@ protected function createRedirect( ->setCode($code) ->setLocale($locale); + + return $this->saveRedirect($redirect); + } + + protected function saveRedirect(Redirect &$redirect) + { try { return $this->repository->save($redirect); } catch (\Exception $exception) { $this->fail(sprintf('Failed creating redirect (%s)', $exception->getMessage())); + return null; + } + } + + protected function findAllRedirects() + { + try { + return $this->repository->findAll(); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed getting redirects (%s)', $exception->getMessage())); + return null; + } + } + + protected function findRedirectByPath(string $path, string $locale = 'da') + { + try { + return $this->repository->findRedirectByPath($path, $locale); + } catch (\Exception $exception) { + $this->fail(sprintf('Failed finding redirect (%s)', $exception->getMessage())); + return null; } - return null; } }