Magento\Cms\Api\Data\PageSearchResultsInterface::getItems() returns wrong type

[redelschaap]

According to the documentation, \Magento\Cms\Api\Data\PageSearchResultsInterface::getItems() should return an array of \Magento\Cms\Api\Data\PageInterface objects, while it currently returns an array of arrays.

Preconditions

I am using Magento 2.1.

Steps to reproduce

$sortOrder      = $this->sortOrderBuilder->setField(\Magento\Cms\Model\Page::TITLE)
                                         ->setAscendingDirection()
                                         ->create();
$searchCriteria = $this->searchCriteriaBuilder->addFilter('store_id', $storeId)
                                              ->addSortOrder($sortOrder)
                                              ->create();

$pageList = $this->pageRepository->getList($searchCriteria);
$pages    = $pageList->getItems();

Expected result

According to the documentation, I would expect $pages to be an array of \Magento\Cms\Api\Data\PageInterface objects.

Actual result

$pages is array of data arrays:

Array
(
    [0] => Array
        (
            [identifier] => [page identifier]
            [title] => [page title]
            [page_layout] => [page title]
            [meta_keywords] => 
            [meta_description] => 
            [content] => [page content]
            [creation_time] => 2016-10-06 14:22:20
            [update_time] => 2016-10-06 14:29:20
            [sort_order] => 0
            [layout_update_xml] => 
            [custom_theme] => 
            [custom_root_template] => 
            [custom_layout_update_xml] => 
            [custom_theme_from] => 2016-10-06
            [custom_theme_to] => 2016-10-06
            [active] => 1
        )
)

[schmengler]

Same with \Magento\Cms\Api\Data\BlockSearchResultsInterface::getItems()

Notably, the ID fields are missing, so it is not even possible to create model instances from this array.

[dersam]

Still present in 2.1.7.

[marius-wieschollek]

The issue is in the \Magento\Cms\Model\PageRepository::getList() method.
The following part of the code is wrong:

$pages = [];
/** @var Page $pageModel */
foreach ($collection as $pageModel) {
    $pageData = $this->dataPageFactory->create();
    $this->dataObjectHelper->populateWithArray(
        $pageData,
        $pageModel->getData(),
        'Magento\Cms\Api\Data\PageInterface'
    );
    $pages[] = $this->dataObjectProcessor->buildOutputDataArray(
        $pageData,
        'Magento\Cms\Api\Data\PageInterface'
    );
}
$searchResults->setItems($pages);

It takes every PageInterface object from the collection. Then it creates a new PageInterface model and populates it with the data from the original object. Then the newly created object is converted to an array and put into the $pages array which contains the result.

This makes no sense to me at all. $searchResults->setItems($collection->getItems()); would do the job just fine. This conversion process instead results in the wrong return type and also deletes additional fields from the page objects. Also without this step, the PageFactory would be unnecessary, which is a good thing since it does not work properly as well. ($this->dataPageFactory->create() should be able to take the properties of the resulting object as argument but that does not work. It is always empty and so they use $this->dataObjectHelper->populateWithArray())

Of course you can't just patch the code since others who use this broken method rely on the wrong return type. You need to write a Plugin or your own implementation of the PageRepository to fix this.

The same goes for the BlockRepository. The code is just copy+paste over there.

[dmitriyprime]

Hi, @redelschaap . Thanks for the feedback. Unfortunately, I could not reproduce the issue. Can you check your problem on the latest version of Magento 2?

[korostii]

Hi @dmitriyprime, there indeed seems to be a fix present on the develop (2.2.0) branch, which makes me think you were trying to reproduce the issue on 2.2.0. Is that correct?

But the issue is reported on the 2.1.x branch. and the latest released version to date is 2.1.7. As @dersam confirms, the issue seems to be still present as of 2.1.7.

Would you mind trying to reproduce it to 2.1.7 given the same step?

[redelschaap]

@dmitriyprime I currently can't check this on Magento 2.2, but I just checked on 2.1.7 and on that version it is indeed still present. It definitely should be fixed in the 2.1.x branch.

[dmitriyprime]

@redelschaap , thanks for the feedback. We've created internal ticket MAGETWO-70381 to address this issue.

[magento-engcom-team]

@redelschaap, thank you for your report.
The issue is already fixed in 2.2.0

[redelschaap]

Doesn't this need to be backported to 2.1.x?

[marius-wieschollek]

@redelschaap I think the issue with backporting this to 2.1.x would be that it is a breaking change. Fixing this in a minor bugfix release of Magento would break any module that depends on it. It is probably better to have a known bug than yet another BC break.

[korostii]

@marius-wieschollek I don't see any @api annotation on this class. Besides, any 3rd-party modules should depend on interfaces (also known as service contracts) rather then their implementations.

It seems that issue was moved to Done in project branch [2.1-develop] by mistake.
@magento-engcom-team would you mind double-checking that?