From 8a227ee168a603832d32e1eb73e92c4903d066a4 Mon Sep 17 00:00:00 2001 From: Vladimir Finko Date: Fri, 30 Jun 2017 11:30:54 +0200 Subject: [PATCH 1/3] don't sort sites in the constructor --- library/CM/Site/SiteFactory.php | 30 ++++++++++++++--------- tests/library/CM/Site/SiteFactoryTest.php | 3 ++- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/library/CM/Site/SiteFactory.php b/library/CM/Site/SiteFactory.php index ce00c3d12..a829f8509 100644 --- a/library/CM/Site/SiteFactory.php +++ b/library/CM/Site/SiteFactory.php @@ -5,6 +5,9 @@ class CM_Site_SiteFactory { /** @var CM_Site_Abstract[] */ private $_siteList; + /** @var CM_Site_Abstract[]|null */ + private $_siteListSorted = null; + /** * @param CM_Site_Abstract[]|null $siteList */ @@ -12,16 +15,6 @@ public function __construct(array $siteList = null) { if (null === $siteList) { $siteList = (new CM_Paging_Site_All())->getItems(); } - - usort($siteList, function (CM_Site_Abstract $site1, CM_Site_Abstract $site2) { - $length1 = mb_strlen($site1->getUrlString()); - $length2 = mb_strlen($site2->getUrlString()); - if ($length1 == $length2) { - return 0; - } - return $length1 > $length2 ? -1 : 1; - }); - $this->_siteList = $siteList; } @@ -31,7 +24,8 @@ public function __construct(array $siteList = null) { */ public function findSite(CM_Http_Request_Abstract $request) { $request = clone $request; - foreach ($this->_siteList as $site) { + $this->_sortSiteList(); + foreach ($this->_siteListSorted as $site) { if ($site->match($request)) { return $site; } @@ -121,4 +115,18 @@ public function getDefaultSite() { return $defaultSite; } + protected function _sortSiteList() { + if (null === $this->_siteListSorted) { + $siteList = $this->_siteList; + usort($siteList, function (CM_Site_Abstract $site1, CM_Site_Abstract $site2) { + $length1 = mb_strlen($site1->getUrlString()); + $length2 = mb_strlen($site2->getUrlString()); + if ($length1 == $length2) { + return 0; + } + return $length1 > $length2 ? -1 : 1; + }); + $this->_siteListSorted = $siteList; + } + } } diff --git a/tests/library/CM/Site/SiteFactoryTest.php b/tests/library/CM/Site/SiteFactoryTest.php index 38fd1cda9..ac1a9e3e1 100644 --- a/tests/library/CM/Site/SiteFactoryTest.php +++ b/tests/library/CM/Site/SiteFactoryTest.php @@ -17,9 +17,10 @@ public function testSiteListSorted() { ksort($unsorted); $siteFactory = new CM_Site_SiteFactory($unsorted); + $this->callProtectedMethod($siteFactory, '_sortSiteList'); $reflection = new ReflectionClass($siteFactory); - $property = $reflection->getProperty('_siteList'); + $property = $reflection->getProperty('_siteListSorted'); $property->setAccessible(true); $this->assertEquals($sorted, $property->getValue($siteFactory)); } From 281220d81d528ff0047e107f42ed01770530476e Mon Sep 17 00:00:00 2001 From: Vladimir Finko Date: Fri, 30 Jun 2017 14:08:50 +0200 Subject: [PATCH 2/3] use getters --- library/CM/Site/SiteFactory.php | 30 +++++++++++++++-------- tests/library/CM/Site/SiteFactoryTest.php | 8 ++---- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/library/CM/Site/SiteFactory.php b/library/CM/Site/SiteFactory.php index a829f8509..bf315ae37 100644 --- a/library/CM/Site/SiteFactory.php +++ b/library/CM/Site/SiteFactory.php @@ -12,9 +12,6 @@ class CM_Site_SiteFactory { * @param CM_Site_Abstract[]|null $siteList */ public function __construct(array $siteList = null) { - if (null === $siteList) { - $siteList = (new CM_Paging_Site_All())->getItems(); - } $this->_siteList = $siteList; } @@ -24,8 +21,7 @@ public function __construct(array $siteList = null) { */ public function findSite(CM_Http_Request_Abstract $request) { $request = clone $request; - $this->_sortSiteList(); - foreach ($this->_siteListSorted as $site) { + foreach ($this->_getSiteListSorted() as $site) { if ($site->match($request)) { return $site; } @@ -52,7 +48,7 @@ public function getSite(CM_Http_Request_Abstract $request) { */ public function findSiteById($id) { $id = (string) $id; - return \Functional\first($this->_siteList, function (CM_Site_Abstract $site) use ($id) { + return \Functional\first($this->_getSiteList(), function (CM_Site_Abstract $site) use ($id) { return $site->getId() === $id; }); } @@ -81,7 +77,7 @@ public function getSiteById($id) { */ public function findSiteByType($type) { $type = (int) $type; - return \Functional\first($this->_siteList, function (CM_Site_Abstract $site) use ($type) { + return \Functional\first($this->_getSiteList(), function (CM_Site_Abstract $site) use ($type) { return $site->getType() === $type; }); } @@ -106,7 +102,7 @@ public function getSiteByType($type) { * @throws CM_Exception_Invalid */ public function getDefaultSite() { - $defaultSite = \Functional\first($this->_siteList, function (CM_Site_Abstract $site) { + $defaultSite = \Functional\first($this->_getSiteList(), function (CM_Site_Abstract $site) { return true === $site->getDefault(); }); if (null === $defaultSite) { @@ -115,9 +111,22 @@ public function getDefaultSite() { return $defaultSite; } - protected function _sortSiteList() { + /** + * @return CM_Site_Abstract[] + */ + protected function _getSiteList() { + if (null === $this->_siteList) { + $this->_siteList = (new CM_Paging_Site_All())->getItems(); + } + return $this->_siteList; + } + + /** + * @return CM_Site_Abstract[] + */ + protected function _getSiteListSorted() { if (null === $this->_siteListSorted) { - $siteList = $this->_siteList; + $siteList = $this->_getSiteList(); usort($siteList, function (CM_Site_Abstract $site1, CM_Site_Abstract $site2) { $length1 = mb_strlen($site1->getUrlString()); $length2 = mb_strlen($site2->getUrlString()); @@ -128,5 +137,6 @@ protected function _sortSiteList() { }); $this->_siteListSorted = $siteList; } + return $this->_siteListSorted; } } diff --git a/tests/library/CM/Site/SiteFactoryTest.php b/tests/library/CM/Site/SiteFactoryTest.php index ac1a9e3e1..2b458b0d4 100644 --- a/tests/library/CM/Site/SiteFactoryTest.php +++ b/tests/library/CM/Site/SiteFactoryTest.php @@ -17,12 +17,8 @@ public function testSiteListSorted() { ksort($unsorted); $siteFactory = new CM_Site_SiteFactory($unsorted); - $this->callProtectedMethod($siteFactory, '_sortSiteList'); - - $reflection = new ReflectionClass($siteFactory); - $property = $reflection->getProperty('_siteListSorted'); - $property->setAccessible(true); - $this->assertEquals($sorted, $property->getValue($siteFactory)); + $siteListSorted = $this->callProtectedMethod($siteFactory, '_getSiteListSorted'); + $this->assertEquals($sorted, $siteListSorted); } public function testFindSite() { From ce32bd901567b62438346f67bf40402ca5e1706d Mon Sep 17 00:00:00 2001 From: Vladimir Finko Date: Fri, 30 Jun 2017 16:31:27 +0200 Subject: [PATCH 3/3] docu's changes --- library/CM/Site/SiteFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/CM/Site/SiteFactory.php b/library/CM/Site/SiteFactory.php index bf315ae37..98d24b53a 100644 --- a/library/CM/Site/SiteFactory.php +++ b/library/CM/Site/SiteFactory.php @@ -2,7 +2,7 @@ class CM_Site_SiteFactory { - /** @var CM_Site_Abstract[] */ + /** @var CM_Site_Abstract[]|null */ private $_siteList; /** @var CM_Site_Abstract[]|null */