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..0559888 100644 --- a/src/Commands/RedirectCommands.php +++ b/src/Commands/RedirectCommands.php @@ -3,6 +3,7 @@ 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; @@ -33,7 +34,10 @@ 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']); @@ -49,7 +53,13 @@ public function normalize() $redirect['paramless_from_hash'] = hash('md5', parse_url($redirect['from'], PHP_URL_PATH)); $redirect['to'] = UrlHelper::normalizeUrl($redirect['to']); $redirect['to_hash'] = hash('md5', $redirect['to']); - $database->update($redirectID, $redirect); + try { + $database->update($redirectID, $redirect); + } catch (DuplicateEntryException $exception) { + $database->delete($redirectID); + } catch (\Exception $exception) { + \WP_CLI::warning(sprintf('Failed updating redirect \'%s\'', $redirectID)); + } } $progress->tick(); }); @@ -73,14 +83,19 @@ public function unchain() \WP_CLI::error(sprintf('Failed instantiating RedirectRepository (%s)', $exception->getMessage())); return; } + \WP_CLI::line('Retrieving redirects...'); try { $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', $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); diff --git a/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php b/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php index db5da46..9edb7ce 100644 --- a/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php +++ b/src/Database/Migrations/AlterRedirectTableAddIgnoreQuery.php @@ -3,6 +3,8 @@ namespace Bonnier\WP\Redirect\Database\Migrations; use Illuminate\Support\Str; +use League\Csv\CannotInsertRecord; +use League\Csv\Writer; class AlterRedirectTableAddIgnoreQuery implements Migration { @@ -15,6 +17,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 +39,46 @@ 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) + { + if (defined('WP_ENV') && WP_ENV === 'testing') { + return; + } + $csv = Writer::createFromPath(sprintf('%s_deleted_duplicates.csv', strtotime('now')), 'w+'); + 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 + "); + 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) { + $wpdb->delete($table, ['id' => $redirect->id]); + try { + $csv->insertOne([$redirect->id, $redirect->from, $redirect->to, $redirect->locale]); + } catch (CannotInsertRecord $exception) { + continue; + } + } + } + } + } } 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/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 cd661d0..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( @@ -122,6 +102,35 @@ 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()); + + $redirects = $this->findAllRedirects(); + $this->assertCount(1, $redirects); + + $this->assertRedirect( + 0, + $redirects->first(), + '/slug/bravo', + '/slug/alfa', + 'manual' + ); + } + public function testCanCreateRedirectWithToWhichAlreadyExists() { $existingRedirect = $this->createRedirect('/from/something', '/to/destination'); @@ -140,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( @@ -171,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, @@ -203,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, @@ -243,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 8e98919..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,18 +91,46 @@ 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'); $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()); + + $redirects = $this->findAllRedirects(); + + $this->assertCount(1, $redirects); + + $this->assertRedirect( + 0, + $redirects->first(), + '/slug/bravo', + '/slug/alfa', + 'manual' + ); + } + public function testCanUpdateRedirectWithToWhichAlreadyExists() { $existingRedirect = $this->createRedirect('/from/something', '/to/destination'); @@ -133,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( @@ -175,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( @@ -216,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, @@ -253,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 new file mode 100644 index 0000000..df8c3ec --- /dev/null +++ b/tests/integration/Repositories/RedirectRepository/SavingRedirectTest.php @@ -0,0 +1,96 @@ +setFrom('/from/slug') + ->setTo('/to/slug') + ->setKeepQuery(false) + ->setType('test') + ->setCode(301) + ->setLocale('da'); + + $this->saveRedirect($redirect); + + $this->assertGreaterThan(0, $redirect->getID()); + $redirects = $this->findAllRedirects(); + $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()); + $this->assertEmpty($this->findAllRedirects()); + return; + } catch (\Exception $exception) { + $this->fail(sprintf('Saving redirect threw unexpected exception (%s)', $exception->getMessage())); + } + $this->fail('Failed throwing IdenticalFromToException!'); + } + + public function testRemovingChainsDoesNotCreateLoops() + { + $oldRedirect = $this->createRedirect('/from/a', '/to/b'); + $createdRedirects = $this->findAllRedirects(); + $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; + } + + $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 f9c1b57..0b34994 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() @@ -160,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()); @@ -189,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; } } 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 @@