From 2adbe4cbc7022a87e915de91c195ce4b4fa1af94 Mon Sep 17 00:00:00 2001 From: Anton Fedurtsya Date: Sat, 2 Dec 2023 01:25:55 +0200 Subject: [PATCH] Cleanup code duplication in filters Signed-off-by: Anton Fedurtsya --- src/ChangeFilter/CategoryChangeFilter.php | 48 ++++++-------- src/ChangeFilter/ChangeFilterTemplate.php | 65 +++++++++++++++++++ src/ChangeFilter/ContentChangeFilter.php | 48 ++++++-------- src/ChangeFilter/ProductChangeFilter.php | 48 ++++++-------- src/Settings/ShopSettings.php | 1 - .../ChangeFilter/CategoryChangeFilterTest.php | 6 +- .../ChangeFilter/ContentChangeFilterTest.php | 5 +- .../ChangeFilter/ProductChangeFilterTest.php | 6 +- 8 files changed, 135 insertions(+), 92 deletions(-) create mode 100644 src/ChangeFilter/ChangeFilterTemplate.php diff --git a/src/ChangeFilter/CategoryChangeFilter.php b/src/ChangeFilter/CategoryChangeFilter.php index 01ac056..56b3f22 100644 --- a/src/ChangeFilter/CategoryChangeFilter.php +++ b/src/ChangeFilter/CategoryChangeFilter.php @@ -9,29 +9,32 @@ namespace FreshAdvance\Sitemap\ChangeFilter; -use DateTime; -use Doctrine\DBAL\Connection; -use FreshAdvance\Sitemap\DataStructure\ObjectUrl; -use FreshAdvance\Sitemap\DataStructure\PageUrl; +use Doctrine\DBAL\ForwardCompatibility\Result; use OxidEsales\Eshop\Application\Model\Category; -use OxidEsales\EshopCommunity\Internal\Framework\Database\ConnectionProviderInterface; -class CategoryChangeFilter implements ChangeFilterInterface +class CategoryChangeFilter extends ChangeFilterTemplate implements ChangeFilterInterface { - protected Connection $connection; + public function getObjectType(): string + { + return 'category'; + } - public function __construct( - ConnectionProviderInterface $connectionProvider - ) { - $this->connection = $connectionProvider->get(); + protected function getModelClass(): string + { + return Category::class; } - public function getObjectType(): string + protected function getChangeFrequency(): string { - return 'category'; + return 'daily'; } - public function getUpdatedUrls(int $limit): iterable + protected function getPriority(): float + { + return 0.7; + } + + protected function filterAndQueryItems(int $limit): Result { $query = "SELECT c.OXID FROM oxcategories c @@ -43,6 +46,7 @@ public function getUpdatedUrls(int $limit): iterable ORDER BY c.OXTIMESTAMP ASC LIMIT {$limit}"; + /** @var Result $result */ $result = $this->connection->executeQuery( $query, [ @@ -51,20 +55,6 @@ public function getUpdatedUrls(int $limit): iterable ] ); - while ($data = $result->fetchAssociative()) { - $item = oxNew(Category::class); - $item->load((string)$data['OXID']); // @phpstan-ignore-line - - yield new ObjectUrl( - objectId: $item->getId(), - objectType: $this->getObjectType(), - url: new PageUrl( - location: (string)$item->getLink(), - lastModified: new DateTime($item->getFieldData('oxtimestamp')), // @phpstan-ignore-line - changeFrequency: 'daily', - priority: 0.9 - ) - ); - } + return $result; } } diff --git a/src/ChangeFilter/ChangeFilterTemplate.php b/src/ChangeFilter/ChangeFilterTemplate.php new file mode 100644 index 0000000..caa1326 --- /dev/null +++ b/src/ChangeFilter/ChangeFilterTemplate.php @@ -0,0 +1,65 @@ +connection = $connectionProvider->get(); + } + + abstract public function getObjectType(): string; + + public function getUpdatedUrls(int $limit): iterable + { + $result = $this->filterAndQueryItems($limit); + + while ($data = $result->fetchAssociative()) { + /** @var IUrl&BaseModel $item */ + $item = oxNew($this->getModelClass()); + $item->load((string)$data['OXID']); // @phpstan-ignore-line + + yield new ObjectUrl( + objectId: $item->getId(), + objectType: $this->getObjectType(), + url: new PageUrl( + location: (string)$item->getLink(), + lastModified: new DateTime($item->getFieldData('oxtimestamp')), // @phpstan-ignore-line + changeFrequency: $this->getChangeFrequency(), + priority: $this->getPriority() + ) + ); + } + } + + abstract protected function filterAndQueryItems(int $limit): Result; + + /** + * @return class-string + */ + abstract protected function getModelClass(): string; + + abstract protected function getChangeFrequency(): string; + + abstract protected function getPriority(): float; +} diff --git a/src/ChangeFilter/ContentChangeFilter.php b/src/ChangeFilter/ContentChangeFilter.php index 1325c65..d0b81fc 100644 --- a/src/ChangeFilter/ContentChangeFilter.php +++ b/src/ChangeFilter/ContentChangeFilter.php @@ -9,29 +9,17 @@ namespace FreshAdvance\Sitemap\ChangeFilter; -use DateTime; -use Doctrine\DBAL\Connection; -use FreshAdvance\Sitemap\DataStructure\ObjectUrl; -use FreshAdvance\Sitemap\DataStructure\PageUrl; +use Doctrine\DBAL\ForwardCompatibility\Result; use OxidEsales\Eshop\Application\Model\Content; -use OxidEsales\EshopCommunity\Internal\Framework\Database\ConnectionProviderInterface; -class ContentChangeFilter implements ChangeFilterInterface +class ContentChangeFilter extends ChangeFilterTemplate implements ChangeFilterInterface { - protected Connection $connection; - - public function __construct( - ConnectionProviderInterface $connectionProvider - ) { - $this->connection = $connectionProvider->get(); - } - public function getObjectType(): string { return 'content'; } - public function getUpdatedUrls(int $limit): \Generator + protected function filterAndQueryItems(int $limit): Result { $query = "SELECT c.OXID FROM oxcontents c @@ -44,6 +32,7 @@ public function getUpdatedUrls(int $limit): \Generator ORDER BY c.OXTIMESTAMP ASC LIMIT {$limit}"; + /** @var Result $result */ $result = $this->connection->executeQuery( $query, [ @@ -53,20 +42,21 @@ public function getUpdatedUrls(int $limit): \Generator ] ); - while ($data = $result->fetchAssociative()) { - $item = oxNew(Content::class); - $item->load((string)$data['OXID']); // @phpstan-ignore-line + return $result; + } - yield new ObjectUrl( - objectId: $item->getId(), - objectType: $this->getObjectType(), - url: new PageUrl( - location: (string)$item->getLink(), - lastModified: new DateTime($item->getFieldData('oxtimestamp')), // @phpstan-ignore-line - changeFrequency: 'never', - priority: 0.5 - ) - ); - } + protected function getModelClass(): string + { + return Content::class; + } + + protected function getChangeFrequency(): string + { + return 'weekly'; + } + + protected function getPriority(): float + { + return 0.3; } } diff --git a/src/ChangeFilter/ProductChangeFilter.php b/src/ChangeFilter/ProductChangeFilter.php index 28cb6c0..842607b 100644 --- a/src/ChangeFilter/ProductChangeFilter.php +++ b/src/ChangeFilter/ProductChangeFilter.php @@ -9,29 +9,17 @@ namespace FreshAdvance\Sitemap\ChangeFilter; -use DateTime; -use FreshAdvance\Sitemap\DataStructure\ObjectUrl; -use FreshAdvance\Sitemap\DataStructure\PageUrl; +use Doctrine\DBAL\ForwardCompatibility\Result; use OxidEsales\Eshop\Application\Model\Article; -use OxidEsales\EshopCommunity\Internal\Framework\Database\ConnectionProviderInterface; -use Doctrine\DBAL\Connection; -class ProductChangeFilter implements ChangeFilterInterface +class ProductChangeFilter extends ChangeFilterTemplate implements ChangeFilterInterface { - protected Connection $connection; - - public function __construct( - ConnectionProviderInterface $connectionProvider - ) { - $this->connection = $connectionProvider->get(); - } - public function getObjectType(): string { return 'product'; } - public function getUpdatedUrls(int $limit): iterable + protected function filterAndQueryItems(int $limit): Result { $query = "SELECT a.OXID FROM oxarticles a @@ -43,6 +31,7 @@ public function getUpdatedUrls(int $limit): iterable ORDER BY a.OXTIMESTAMP ASC LIMIT {$limit}"; + /** @var Result $result */ $result = $this->connection->executeQuery( $query, [ @@ -51,20 +40,21 @@ public function getUpdatedUrls(int $limit): iterable ] ); - while ($data = $result->fetchAssociative()) { - $item = oxNew(Article::class); - $item->load((string)$data['OXID']); // @phpstan-ignore-line + return $result; + } - yield new ObjectUrl( - objectId: $item->getId(), - objectType: $this->getObjectType(), - url: new PageUrl( - location: (string)$item->getLink(), - lastModified: new DateTime($item->getFieldData('oxtimestamp')), // @phpstan-ignore-line - changeFrequency: 'daily', - priority: 0.7 - ) - ); - } + protected function getModelClass(): string + { + return Article::class; + } + + protected function getChangeFrequency(): string + { + return 'daily'; + } + + protected function getPriority(): float + { + return 0.5; } } diff --git a/src/Settings/ShopSettings.php b/src/Settings/ShopSettings.php index 5857449..cc303ed 100644 --- a/src/Settings/ShopSettings.php +++ b/src/Settings/ShopSettings.php @@ -9,7 +9,6 @@ namespace FreshAdvance\Sitemap\Settings; -use FreshAdvance\Sitemap\Settings\ShopSettingsInterface; use OxidEsales\Eshop\Core\Config; class ShopSettings implements ShopSettingsInterface diff --git a/tests/Integration/ChangeFilter/CategoryChangeFilterTest.php b/tests/Integration/ChangeFilter/CategoryChangeFilterTest.php index b3f3331..b38eef4 100644 --- a/tests/Integration/ChangeFilter/CategoryChangeFilterTest.php +++ b/tests/Integration/ChangeFilter/CategoryChangeFilterTest.php @@ -14,6 +14,10 @@ use FreshAdvance\Sitemap\Tests\Integration\IntegrationTestCase; use OxidEsales\Eshop\Application\Model\Category; +/** + * @covers \FreshAdvance\Sitemap\ChangeFilter\ChangeFilterTemplate + * @covers \FreshAdvance\Sitemap\ChangeFilter\CategoryChangeFilter + */ class CategoryChangeFilterTest extends IntegrationTestCase { public function testGetUpdatedUrls(): void @@ -62,6 +66,6 @@ protected function checkCurrentUrlItem(ObjectUrlInterface $objectUrl, string $va $this->assertSame('http://localhost.local/' . $value . '/', $url->getLocation()); $this->assertNotEmpty($url->getLastModified()); $this->assertSame('daily', $url->getChangeFrequency()); - $this->assertSame(0.9, $url->getPriority()); + $this->assertSame(0.7, $url->getPriority()); } } diff --git a/tests/Integration/ChangeFilter/ContentChangeFilterTest.php b/tests/Integration/ChangeFilter/ContentChangeFilterTest.php index 21abd30..bc0c2f7 100644 --- a/tests/Integration/ChangeFilter/ContentChangeFilterTest.php +++ b/tests/Integration/ChangeFilter/ContentChangeFilterTest.php @@ -18,6 +18,7 @@ use OxidEsales\EshopCommunity\Application\Model\Content; /** + * @covers \FreshAdvance\Sitemap\ChangeFilter\ChangeFilterTemplate * @covers \FreshAdvance\Sitemap\ChangeFilter\ContentChangeFilter */ class ContentChangeFilterTest extends IntegrationTestCase @@ -68,7 +69,7 @@ protected function checkCurrentUrlItem(ObjectUrlInterface $objectUrl, string $va $url = $objectUrl->getUrl(); $this->assertSame('http://localhost.local/' . $value . '/', $url->getLocation()); $this->assertNotEmpty($url->getLastModified()); - $this->assertSame('never', $url->getChangeFrequency()); - $this->assertSame(0.5, $url->getPriority()); + $this->assertSame('weekly', $url->getChangeFrequency()); + $this->assertSame(0.3, $url->getPriority()); } } diff --git a/tests/Integration/ChangeFilter/ProductChangeFilterTest.php b/tests/Integration/ChangeFilter/ProductChangeFilterTest.php index ed73360..5d0905a 100644 --- a/tests/Integration/ChangeFilter/ProductChangeFilterTest.php +++ b/tests/Integration/ChangeFilter/ProductChangeFilterTest.php @@ -13,6 +13,10 @@ use FreshAdvance\Sitemap\DataStructure\ObjectUrlInterface; use OxidEsales\Eshop\Application\Model\Article; +/** + * @covers \FreshAdvance\Sitemap\ChangeFilter\ChangeFilterTemplate + * @covers \FreshAdvance\Sitemap\ChangeFilter\ProductChangeFilter + */ class ProductChangeFilterTest extends \FreshAdvance\Sitemap\Tests\Integration\IntegrationTestCase { public function testSomething() @@ -60,6 +64,6 @@ protected function checkCurrentUrlItem(ObjectUrlInterface $objectUrl, string $va $this->assertSame('http://localhost.local/' . $value . '.html', $url->getLocation()); $this->assertNotEmpty($url->getLastModified()); $this->assertSame('daily', $url->getChangeFrequency()); - $this->assertSame(0.7, $url->getPriority()); + $this->assertSame(0.5, $url->getPriority()); } }