From 768daa08f3fc79955eb5b855241dc41bf16cfb03 Mon Sep 17 00:00:00 2001 From: Mojmir Fendek Date: Tue, 4 Feb 2020 14:45:43 +1300 Subject: [PATCH] Top page feature --- README.md | 72 ++++++ src/Extensions/ElementalPageExtension.php | 6 +- src/Models/BaseElement.php | 1 + src/TopPage/DataExtension.php | 216 +++++++++++++++++ src/TopPage/FluentExtension.php | 43 ++++ src/TopPage/SiteTreeExtension.php | 248 +++++++++++++++++++ tests/TopPage/TestBlockPage.php | 28 +++ tests/TopPage/TestChildPage.php | 36 +++ tests/TopPage/TestContent.php | 45 ++++ tests/TopPage/TestList.php | 41 ++++ tests/TopPage/TopPageTest.php | 281 ++++++++++++++++++++++ tests/TopPage/TopPageTest.yml | 55 +++++ 12 files changed, 1068 insertions(+), 4 deletions(-) create mode 100644 src/TopPage/DataExtension.php create mode 100644 src/TopPage/FluentExtension.php create mode 100644 src/TopPage/SiteTreeExtension.php create mode 100644 tests/TopPage/TestBlockPage.php create mode 100644 tests/TopPage/TestChildPage.php create mode 100644 tests/TopPage/TestContent.php create mode 100644 tests/TopPage/TestList.php create mode 100644 tests/TopPage/TopPageTest.php create mode 100644 tests/TopPage/TopPageTest.yml diff --git a/README.md b/README.md index dcbe06f4c..703ab337e 100644 --- a/README.md +++ b/README.md @@ -323,6 +323,78 @@ globally in your command line. **Note:** If adding or modifying colours, spacing, font sizes etc. please try and use an appropriate variable from the silverstripe/admin module if available. +## Top page reference feature (optional performance improvement) + +In some cases your project setup may have deeply nested blocks, for example: + +``` +Page + ElementalArea + RowBlock (represents grid row on frontend) + ElementalArea + AccordionBlock (block which can contain other content blocks) + ElementalArea + ContetnBlock +``` + +It's quite common to use top page lookups from block context, i.e. a block is querying data from the page that the block belongs to. + +Most common cases are: + +* `CMS fields` - block level conditional logic depends on page data +* `templates` - block level render logic depends on page data + +This module uses some in-memory caching but this isn't good enough for such deeply nested data structures by default. + +In such cases it is recommended to turn on this feature which stores the top page reference on individual blocks and elemental areas. +This speeds up data lookup significantly. + +Please note that this feature only works with project setups which don't allow `block sharing`, i.e. `one block can only belong to a single page`. + +To turn the feature on simply apply following extensions, like this: + +``` +DNADesign\Elemental\Models\BaseElement: + extensions: + topPage: DNADesign\Elemental\TopPage\DataExtension + +DNADesign\Elemental\Models\ElementalArea: + extensions: + topPage: DNADesign\Elemental\TopPage\DataExtension + +Page: + extensions: + topPage: DNADesign\Elemental\TopPage\SiteTreeExtension +``` + +If your project setup uses Fluent module it is recommended to use following configuration instead: + +``` +DNADesign\Elemental\Models\BaseElement: + extensions: + topPage: DNADesign\Elemental\TopPage\FluentExtension + +DNADesign\Elemental\Models\ElementalArea: + extensions: + topPage: DNADesign\Elemental\TopPage\FluentExtension + +Page: + extensions: + topPage: DNADesign\Elemental\TopPage\SiteTreeExtension +``` + +This will store the locale of the top page on blocks which simplifies top page lookup in case the locale is unknown at the time of page lookup from block context. + +The page reference data on the blocks can also be used for maintenance dev tasks as it's easy to identify which blocks belong to which pages in which locale. + +### Top page reference data during object duplication + +Duplicating data object will not duplicate the top page reference data. +Instead, the newly created data objects will have a new top page data assigned based on the context. + +For example, duplicating a page with all of its blocks will create a new page and new blocks. +All the new blocks will have the new page stored as their top page reference. This works even if the deplication tree contains other pages as tree nodes. + ## Integration with other modules * [Multiple languages with tractorcow/silverstripe-fluent](docs/en/advanced_setup.md) diff --git a/src/Extensions/ElementalPageExtension.php b/src/Extensions/ElementalPageExtension.php index 995422c6b..feeb93d11 100644 --- a/src/Extensions/ElementalPageExtension.php +++ b/src/Extensions/ElementalPageExtension.php @@ -2,16 +2,14 @@ namespace DNADesign\Elemental\Extensions; -use Exception; use DNADesign\Elemental\Models\ElementalArea; use SilverStripe\Control\Controller; use SilverStripe\View\Parsers\HTML4Value; -use SilverStripe\Core\Config\Config; use SilverStripe\View\SSViewer; /** - * @method ElementalArea ElementalArea - * @property ElementalArea ElementalArea + * @method ElementalArea ElementalArea() + * @property int ElementalAreaID */ class ElementalPageExtension extends ElementalAreasExtension { diff --git a/src/Models/BaseElement.php b/src/Models/BaseElement.php index 05cbca3c6..d9983f218 100644 --- a/src/Models/BaseElement.php +++ b/src/Models/BaseElement.php @@ -39,6 +39,7 @@ * @property int $Sort * @property string $ExtraClass * @property string $Style + * @property int $ParentID * * @method ElementalArea Parent() */ diff --git a/src/TopPage/DataExtension.php b/src/TopPage/DataExtension.php new file mode 100644 index 000000000..d8bc4f055 --- /dev/null +++ b/src/TopPage/DataExtension.php @@ -0,0 +1,216 @@ + Page::class, + ]; + + /** + * @config + * @var array + */ + private static $indexes = [ + 'TopPageID' => true, + ]; + + /** + * @var bool + */ + private $skipTopPageUpdate = false; + + /** + * Exension point in @see DataObject::onAfterWrite() + */ + public function onAfterWrite(): void + { + $this->setTopPage(); + } + + /** + * Exension point in @see DataObject::duplicate() + */ + public function onBeforeDuplicate(): void + { + $this->clearTopPage(); + } + + /** + * Exension point in @see DataObject::duplicate() + */ + public function onAfterDuplicate(): void + { + $this->updateTopPage(); + } + + /** + * Find top level page of a block or elemental area + * this is very useful in case blocks are deeply nested + * + * for example: + * page -> elemental area -> block -> elemental area -> block + * + * this lookup is very performant as is safe to use in a template as well + * + * @return Page|null + * @throws ValidationException + */ + public function getTopPage(): ?Page + { + $list = [$this->owner]; + + while (count($list) > 0) { + /** @var DataObject|DataExtension $item */ + $item = array_shift($list); + + if ($item instanceof Page) { + // trivial case + return $item; + } + + if ($item->hasExtension(DataExtension::class) && $item->TopPageID > 0) { + // top page is stored inside data object - just fetch it via cached call + $page = Page::get_by_id($item->TopPageID); + + if ($page !== null && $page->exists()) { + return $page; + } + } + + if ($item instanceof BaseElement) { + // parent lookup via block + $parent = $item->Parent(); + + if ($parent !== null && $parent->exists()) { + array_push($list, $parent); + } + + continue; + } + + if ($item instanceof ElementalArea) { + // parent lookup via elemental area + $parent = $item->getOwnerPage(); + + if ($parent !== null && $parent->exists()) { + array_push($list, $parent); + } + + continue; + } + } + + return null; + } + + /** + * @param Page|null $page + * @throws ValidationException + */ + public function setTopPage(?Page $page = null): void + { + if ($this->skipTopPageUpdate) { + return; + } + + /** @var BaseElement|ElementalArea|Versioned|DataExtension $owner */ + $owner = $this->owner; + + if (!$owner->hasExtension(DataExtension::class)) { + return; + } + + if ($owner->TopPageID > 0) { + return; + } + + $page = $page ?? $owner->getTopPage(); + + if ($page === null) { + return; + } + + // set the page to properties in case this object is re-used later + $this->assignTopPage($page); + + if ($owner->hasExtension(Versioned::class)) { + $owner->writeWithoutVersion(); + + return; + } + + $owner->write(); + } + + /** + * Use this to wrap any code which is supposed to run without doing any top page updates + * + * @param callable $callback + * @return mixed + */ + public function withoutTopPageUpdate(callable $callback) + { + $this->skipTopPageUpdate = true; + + try { + return $callback(); + } finally { + $this->skipTopPageUpdate = false; + } + } + + /** + * Register the object for top page update + * this is a little bit roundabout way to do it, but it's necessary because when cloned object is written + * the relations are not yet written so it's impossible to do a parent lookup at that time + */ + protected function updateTopPage(): void + { + /** @var SiteTreeExtension $extension */ + $extension = singleton(SiteTreeExtension::class); + $extension->addDuplicatedObject($this->owner); + } + + protected function assignTopPage(Page $page): void + { + $this->owner->TopPageID = (int) $page->ID; + } + + /** + * Clears top page relation, this is useful when duplicating object as the new object doesn't necessarily + * belong to the original page + */ + protected function clearTopPage(): void + { + $this->owner->TopPageID = 0; + } +} diff --git a/src/TopPage/FluentExtension.php b/src/TopPage/FluentExtension.php new file mode 100644 index 000000000..64509a4b0 --- /dev/null +++ b/src/TopPage/FluentExtension.php @@ -0,0 +1,43 @@ + 'Varchar', + ]; + + protected function assignTopPage(Page $page): void + { + parent::assignTopPage($page); + + $this->owner->TopPageLocale = FluentState::singleton()->getLocale(); + } + + protected function clearTopPage(): void + { + parent::clearTopPage(); + + $this->owner->TopPageLocale = null; + } +} diff --git a/src/TopPage/SiteTreeExtension.php b/src/TopPage/SiteTreeExtension.php new file mode 100644 index 000000000..f20a76bae --- /dev/null +++ b/src/TopPage/SiteTreeExtension.php @@ -0,0 +1,248 @@ +setTopPageForElementalArea(); + $this->processDuplicationFromOriginal(); + } + + /** + * Exension point in @see DataObject::duplicate() + * + * @param Page $original + */ + public function onBeforeDuplicate(Page $original): void + { + $this->initDuplication($original); + } + + /** + * Exension point in @see DataObject::duplicate() + * + * @param Page $original + * @param bool $doWrite + */ + public function onAfterDuplicate(Page $original, $doWrite): void + { + $this->processDuplication($original, (bool) $doWrite); + } + + public function getDuplicationKey(): ?string + { + $owner = $this->owner; + + if (!$owner->isInDB()) { + return null; + } + + return sprintf('%s-%d', $owner->ClassName, $owner->ID); + } + + /** + * This is a very roundabout way on how to update top page reference for data objects + * the main reason why we're doing it like this is the fact that duplication process goes top-down + * when it comes to model creation, but it goes bottom-up when it comes to writes + * as a consequence the relations are not available when model is written + * + * Instead of updating the page reference during model write, we will simply push the object into a list + * and update the reference later when page is available + * + * note that when going top-down on the relationship tree the models are not written yet so we can't use + * page IDs as identifiers + * instead we use IDs of the original objects that we are cloning from + * + * Additionally, the duplication process has to support nested pages which makes things more complicated + * this is handled by using a stack like data structure to keep track on multiple pages + * each with it's own duplication list + * + * @param DataObject $object + */ + public function addDuplicatedObject(DataObject $object): void + { + if (!$object->hasExtension(DataExtension::class)) { + return; + } + + $key = $this->getDuplicatedPageKey(); + + if ($key === null) { + return; + } + + if (array_key_exists($key, $this->duplicatedObjects)) { + array_unshift($this->duplicatedObjects[$key], $object); + + return; + } + + $this->duplicatedObjects[$key] = [$object]; + } + + /** + * Find currently duplicated page + * note: this doesn't change any stored data + * + * @return string|null + */ + protected function getDuplicatedPageKey(): ?string + { + $pages = $this->duplicatedPages; + + if (count($pages) === 0) { + return null; + } + + return array_shift($pages); + } + + /** + * @param Page|SiteTreeExtension $original + */ + protected function initDuplication(Page $original): void + { + $key = $original->getDuplicationKey(); + + if ($key === null) { + return; + } + + if (in_array($key, $this->duplicatedPages)) { + // this should never happen as it would indicate a duplication loop + return; + } + + array_unshift($this->duplicatedPages, $key); + } + + protected function processDuplication(Page $original, bool $written): void + { + if ($written) { + $this->writeDuplication($original); + + return; + } + + + // write may not be triggered as the page maybe have come up via relation + // in this case we have to delay the processing until the page is written + // store the origin reference on the object (in memory only) so we can pick it up later + $this->owner->duplicationOriginal = $original; + } + + /** + * Relevant only for duplicated object that were not written at the time of duplication + */ + protected function processDuplicationFromOriginal(): void + { + $owner = $this->owner; + + if (!isset($owner->duplicationOriginal)) { + return; + } + + $original = $owner->duplicationOriginal; + + if (!$original instanceof Page) { + return; + } + + unset($owner->duplicationOriginal); + $this->writeDuplication($original); + } + + /** + * @param Page|SiteTreeExtension $original + */ + protected function writeDuplication(Page $original): void + { + $key = $original->getDuplicationKey(); + $currentKey = $this->getDuplicatedPageKey(); + + if ($key !== $currentKey) { + // should never happen but it indicates that the nesting hierarchy was incorrect + return; + } + + if (array_key_exists($key, $this->duplicatedObjects)) { + $objects = $this->duplicatedObjects[$key]; + + /** @var DataObject|DataExtension $object */ + foreach ($objects as $object) { + // attach current page ID to the object + $object->setTopPage($this->owner); + } + } + + // mark page as processed + array_shift($this->duplicatedPages); + } + + /** + * Elemental area is created before related page is written so we have to set top page explicitly + * after page is written and the relations are available + */ + protected function setTopPageForElementalArea(): void + { + /** @var Page|ElementalPageExtension $owner */ + $owner = $this->owner; + + if (!$owner->hasExtension(ElementalPageExtension::class)) { + return; + } + + if (!$owner->ElementalAreaID) { + return; + } + + /** @var ElementalArea|DataExtension $area */ + $area = $owner->ElementalArea(); + + if (!$area->exists()) { + return; + } + + if (!$area->hasExtension(DataExtension::class)) { + return; + } + + $area->setTopPage($owner); + } +} diff --git a/tests/TopPage/TestBlockPage.php b/tests/TopPage/TestBlockPage.php new file mode 100644 index 000000000..6569f18a2 --- /dev/null +++ b/tests/TopPage/TestBlockPage.php @@ -0,0 +1,28 @@ + TestContent::class . '.ChildPage', + ]; + + /** + * @var array + */ + private static $extensions = [ + ElementalPageExtension::class, + ]; +} diff --git a/tests/TopPage/TestContent.php b/tests/TopPage/TestContent.php new file mode 100644 index 000000000..e0c790358 --- /dev/null +++ b/tests/TopPage/TestContent.php @@ -0,0 +1,45 @@ + 'Varchar', + ]; + + private static $has_one = [ + 'ChildPage' => TestChildPage::class, + ]; + + private static $cascade_duplicates = [ + 'ChildPage', + ]; + + private static $cascade_deletes = [ + 'ChildPage', + ]; + + public function getType(): string + { + return 'Test element with content'; + } +} diff --git a/tests/TopPage/TestList.php b/tests/TopPage/TestList.php new file mode 100644 index 000000000..2c1c443d7 --- /dev/null +++ b/tests/TopPage/TestList.php @@ -0,0 +1,41 @@ + ElementalArea::class + ]; + + private static $owns = [ + 'Elements' + ]; + + private static $cascade_deletes = [ + 'Elements' + ]; + + private static $cascade_duplicates = [ + 'Elements' + ]; + + private static $extensions = [ + ElementalAreasExtension::class + ]; + + public function getType(): string + { + return 'Test element list'; + } +} diff --git a/tests/TopPage/TopPageTest.php b/tests/TopPage/TopPageTest.php new file mode 100644 index 000000000..d4873739d --- /dev/null +++ b/tests/TopPage/TopPageTest.php @@ -0,0 +1,281 @@ + [ + TopPage\SiteTreeExtension::class, + ], + ElementalArea::class => [ + TopPage\DataExtension::class, + ], + BaseElement::class => [ + TopPage\DataExtension::class, + ], + ]; + + /** + * @var array + */ + protected static $extra_dataobjects = [ + TestContent::class, + TestList::class, + TestBlockPage::class, + TestChildPage::class, + ]; + + protected function setUp(): void + { + /** @var TopPage\DataExtension $extension */ + $extension = singleton(TopPage\DataExtension::class); + + // this makes sure that the data objects start with no top page data + $extension->withoutTopPageUpdate(function () { + parent::setUp(); + }); + } + + /** + * @param string $pageIdentifier + * @param string $pageClass + * @param string $objectIdentifier + * @param string $objectClass + * @dataProvider objectsProvider + */ + public function testTestGetTopPage( + string $pageIdentifier, + string $pageClass, + string $objectIdentifier, + string $objectClass + ): void { + /** @var Page|TopPage\SiteTreeExtension $content */ + $page = $this->objFromFixture($pageClass, $pageIdentifier); + + /** @var DataObject|TopPage\DataExtension $object */ + $object = $this->objFromFixture($objectClass, $objectIdentifier); + + $topPage = $object->getTopPage(); + + $this->assertNotNull($topPage); + $this->assertEquals((int) $page->ID, (int) $topPage->ID); + } + + /** + * @param string $pageIdentifier + * @param string $pageClass + * @param string $objectIdentifier + * @param string $objectClass + * @dataProvider objectsProvider + */ + public function testTestUpdateTopPageEmptyCache( + string $pageIdentifier, + string $pageClass, + string $objectIdentifier, + string $objectClass + ): void { + /** @var Page|TopPage\SiteTreeExtension $content */ + $page = $this->objFromFixture($pageClass, $pageIdentifier); + + /** @var DataObject|TopPage\DataExtension $object */ + $object = $this->objFromFixture($objectClass, $objectIdentifier); + + $this->assertEquals(0, (int) $object->TopPageID); + + $object->forceChange(); + $id = $object->write(); + $object = DataObject::get($object->ClassName)->byID($id); + + $this->assertEquals((int) $page->ID, (int) $object->TopPageID); + + // do a second write to make sure that we won't override existing top page + $object->forceChange(); + $id = $object->write(); + $object = DataObject::get($object->ClassName)->byID($id); + + $this->assertEquals((int) $page->ID, (int) $object->TopPageID); + } + + public function testNewPage(): void + { + $page = TestBlockPage::create(); + $page->Title = 'New page test'; + $page->write(); + + /** @var ElementalArea|TopPage\DataExtension $area */ + $area = $page->ElementalArea(); + $this->assertEquals((int) $page->ID, (int) $area->TopPageID); + } + + /** + * @param bool $populateTopPage + * @dataProvider populateTopPageProvider + */ + public function testNewBlock(bool $populateTopPage): void + { + if ($populateTopPage) { + $this->populateTopPageForAllObjects(); + } + + /** @var TestBlockPage $page */ + $page = $this->objFromFixture(TestBlockPage::class, 'block-page1'); + + /** @var ElementalArea $area */ + $area = $this->objFromFixture(ElementalArea::class, 'area3'); + + /** @var TestContent|TopPage\DataExtension $content */ + $content = TestContent::create(); + $content->Title = 'Fresh block'; + + $area->Elements()->add($content); + $content = DataObject::get($content->ClassName)->byID($content->ID); + + $this->assertEquals((int) $page->ID, (int) $content->TopPageID); + } + + public function testPageDuplication(): void + { + $this->populateTopPageForAllObjects(); + + /** @var TestBlockPage $page */ + $page = $this->objFromFixture(TestBlockPage::class, 'block-page1'); + + /** @var TestChildPage $childPage */ + $childPage = $this->objFromFixture(TestChildPage::class, 'child-page1'); + + $page->duplicate(); + $pages = TestBlockPage::get()->filter(['Title' => 'BlockPage1'])->sort('ID', 'DESC'); + + $this->assertCount(2, $pages); + + $pageClone = $pages->first(); + $childPages = TestChildPage::get()->filter(['Title' => 'ChildPage1'])->sort('ID', 'DESC'); + + $this->assertCount(2, $childPages); + + $childClone = $childPages->first(); + + $this->assertNotEquals((int) $childPage->ID, (int) $childClone->ID); + + $objects = [ + [TestList::class, 'List1', $pageClone], + [TestContent::class, 'Content1', $pageClone], + [TestList::class, 'List2', $childClone], + [TestContent::class, 'Content2', $childClone], + ]; + + foreach ($objects as $objectData) { + $class = array_shift($objectData); + $title = array_shift($objectData); + $page = array_shift($objectData); + + $items = DataObject::get($class)->filter(['Title' => $title])->sort('ID', 'DESC'); + + $this->assertCount(2, $items); + + /** @var DataObject|TopPage\DataExtension $objectClone */ + $objectClone = $items->first(); + + $this->assertEquals((int) $page->ID, (int) $objectClone->TopPageID); + + /** @var ElementalArea|TopPage\DataExtension $area */ + $area = $objectClone->Parent(); + + $this->assertEquals((int) $page->ID, (int) $area->TopPageID); + } + } + + public function objectsProvider(): array + { + return [ + [ + 'block-page1', + TestBlockPage::class, + 'content1', + TestContent::class, + ], + [ + 'block-page1', + TestBlockPage::class, + 'list1', + TestList::class, + ], + [ + 'block-page1', + TestBlockPage::class, + 'area1', + ElementalArea::class, + ], + [ + 'block-page1', + TestBlockPage::class, + 'area3', + ElementalArea::class, + ], + [ + 'child-page1', + TestChildPage::class, + 'content2', + TestContent::class, + ], + [ + 'child-page1', + TestChildPage::class, + 'list2', + TestList::class, + ], + [ + 'child-page1', + TestChildPage::class, + 'area2', + ElementalArea::class, + ], + [ + 'child-page1', + TestChildPage::class, + 'area4', + ElementalArea::class, + ], + ]; + } + + public function populateTopPageProvider(): array + { + return [ + [true], + [false], + ]; + } + + private function populateTopPageForAllObjects(): void + { + $list = $this->objectsProvider(); + + foreach ($list as $objects) { + array_shift($objects); + array_shift($objects); + $identifier = array_shift($objects); + $class = array_shift($objects); + + $object = $this->objFromFixture($class, $identifier); + $object->forceChange(); + $object->write(); + } + } +} diff --git a/tests/TopPage/TopPageTest.yml b/tests/TopPage/TopPageTest.yml new file mode 100644 index 000000000..bfc11ba62 --- /dev/null +++ b/tests/TopPage/TopPageTest.yml @@ -0,0 +1,55 @@ +# Deeply nested setup +# +# Block page +# - Elemental area +# -- TestList +# --- ElementalArea +# ---- TestContent +# ----- ChildPage +# ------- ElementalArea +# -------- TestList +# --------- ElementalArea +# ---------- TestContent + +DNADesign\Elemental\Models\ElementalArea: + area1: + OwnerClassName: DNADesign\Elemental\Tests\TopPage\TestBlockPage + area2: + OwnerClassName: DNADesign\Elemental\Tests\TopPage\TestChildPage + area3: + OwnerClassName: DNADesign\Elemental\Tests\TopPage\TestList + area4: + OwnerClassName: DNADesign\Elemental\Tests\TopPage\TestList + +DNADesign\Elemental\Tests\TopPage\TestList: + list1: + Title: List1 + ParentID: =>DNADesign\Elemental\Models\ElementalArea.area1 + ElementsID: =>DNADesign\Elemental\Models\ElementalArea.area3 + list2: + Title: List2 + ParentID: =>DNADesign\Elemental\Models\ElementalArea.area2 + ElementsID: =>DNADesign\Elemental\Models\ElementalArea.area4 + +DNADesign\Elemental\Tests\TopPage\TestBlockPage: + block-page1: + Title: BlockPage1 + URLSegment: block-page1 + ElementalAreaID: =>DNADesign\Elemental\Models\ElementalArea.area1 + +DNADesign\Elemental\Tests\TopPage\TestChildPage: + child-page1: + Title: ChildPage1 + URLSegment: child-page1 + ElementalAreaID: =>DNADesign\Elemental\Models\ElementalArea.area2 + +DNADesign\Elemental\Tests\TopPage\TestContent: + content1: + Title: Content1 + Content: 'Some content' + ParentID: =>DNADesign\Elemental\Models\ElementalArea.area3 + ChildPageID: =>DNADesign\Elemental\Tests\TopPage\TestChildPage.child-page1 + content2: + Title: Content2 + Content: 'Some other content' + ParentID: =>DNADesign\Elemental\Models\ElementalArea.area4