Bug #93484

TYPO3 Workspace, Preview module raising exception for versioned plugin records

Added by Gabriel Kaufmann / Typoworx NewMedia 7 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Workspaces
Target version:
-
Start date:
2021-02-10
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
9
PHP Version:
7.2
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

I noticed a very strange error. We had to migrate a TYPO3 instance to TYPO3 v9 which was previously was using TYPO3 Workspaces.

Our customer noticed an Exception trying to use the "Page Preview" Module in a draft workspace on a page which contains a plugin with (workspace ready) plugin records.

The exception aims that the workspace versioning function for some reason tries to access a "uid_local" field which does not exist on the given record! After hours of debugging I noticed that this is caused by $source of type TYPO3\CMS\Extbase\Persistence\Generic\Qom\Join given to TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbBackend::overlayLanguageAndWorkspace. For some reason the original core function is not able to build a valid query and simply returns the original rows (even through there still is some kind of logic which seems to try catch this case).

I was able to create a patch which seems to work fine so far. I'm XClassing around Typo3DbBackend and before calling the parent::overlayLanguageAndWorkspace I'm checking if the given $source is a Join!

If this is the case I'm breaking down the whole thing by re-creating a new $source of type Selector (simple query without join) giving this one to the original function. I think the rest is validly handled by the workspace-versioned model/record itself. So far I was not able to find side-effects of breaking of the Join into a simple "Select" Query for the table without the join (in my case it was for sys-category).

```
namespace Sit\SitConfigGeneric\Core\Persistence\Generic\Storage;

use TYPO3\CMS\Extbase\Persistence\QueryInterface;
use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper;
use TYPO3\CMS\Extbase\Persistence\Generic\Qom\Join;
use TYPO3\CMS\Extbase\Persistence\Generic\Qom\SourceInterface;
use TYPO3\CMS\Extbase\Persistence\Generic\Qom\QueryObjectModelFactory;

/** * Class Typo3DbBackend * @package TYPOworx\CorePatches\Core * * @author Gabriel Kaufmann <>
*/
class Typo3DbBackend extends \TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbBackend {
/** * Performs workspace and language overlay on the given row array. The language and workspace id is automatically * detected (depending on FE or BE context). You can also explicitly set the language/workspace id. * * @param SourceInterface|\TYPO3\CMS\Extbase\Persistence\Generic\Qom\Join $source The source (selector or join) * @param array $rows * @param QueryInterface $query * @param int|null $workspaceUid * @return array * @throws \TYPO3\CMS\Core\Context\Exception\AspectNotFoundException * @throws \Throwable
*/
protected function overlayLanguageAndWorkspace(SourceInterface $source, array $rows, QueryInterface $query, int $workspaceUid = null): array {
try {
/** * For unknwon reason the TYPO3 Core dev's didn't expect * that some join-types would skrew the ::overlayLanguageAndWorkspace, * because it mistakenly uses the right table, but fields that belong to * the Join'ed relation :sic:
*/
if($source instanceof Join) {
/*
$sourceModel = '\\' . ltrim( get_class($source), '\\');

$refactoredSource = $this->objectManager->get(
$sourceModel,
$this->getTableNameFromModel($query->getType()),
$query->getType()
);
*/
$qob = $this->objectManager->get(QueryObjectModelFactory::class);
$refactoredSource = $qob->selector($this->getTableNameFromModel($query->getType()), $query->getType());
}
else {
$refactoredSource = $source;
} {
throw $e;
}
}
$workspaceRows = parent::overlayLanguageAndWorkspace($refactoredSource, $rows, $query, $workspaceUid);
return $workspaceRows;
}
catch (\Throwable $e)
/**
 * @param $model
 * @return string|null
 * @throws \TYPO3\CMS\Extbase\Persistence\Generic\Exception
*/
protected function getTableNameFromModel($model) :? string {
if(is_object($model)) {
$model = get_class($model);
}
$model = '\\' . ltrim($model, '\\');
$dataMapper = $this->objectManager->get(DataMapper::class);
return $dataMapper->getDataMap($model)->getTableName();
}
}
```

I'm not perfectly shure - but I think it makes sense to apply the same patch around ::doLanguageAndWorkspaceOverlay as well.

Please discuss if it makes sense to apply this to the original core-class. I know workspaces in TYPO3 v9 and upwards are still unstable! But may be this will help to "harden" this feature a little bit more.


Files

Auswahl_006.jpg (76 KB) Auswahl_006.jpg Gabriel Kaufmann / Typoworx NewMedia, 2021-02-10 18:18
Typo3DbBackend.php.patch (5.45 KB) Typo3DbBackend.php.patch Gabriel Kaufmann / Typoworx NewMedia, 2021-02-11 10:37

Related issues

Related to TYPO3 Core - Bug #94140: PHP Warning appears when preview in workspace and item of mm relation is disabledNew2021-05-14

Actions
#1

Updated by Benni Mack 7 months ago

Sounds reasonable. Care to create a patch for this?

#2

Updated by Gabriel Kaufmann / Typoworx NewMedia 7 months ago

Benni Mack wrote in #note-1:

Sounds reasonable. Care to create a patch for this?

Thanks for the positive Feedback. I've attached the patch. It was a little bit more tricky than my XClass solution. I had to do some minor refactoring. But the good news I think it would have made sense anyway as some code-logic was redundant in ::doLanguageAndWorkspaceOverlay and ::overlayLanguageAndWorkspace anyway.

I decided to implement this logic in the upper method ::getObjectDataByQuery calling either the method ::doLanguageAndWorkspaceOverlay or ::overlayLanguageAndWorkspace giving the rewritten SourceInterface $source to it. Another reason for this way solving it was that I would had to refactor ::doLanguageAndWorkspaceOverlay taking QueryInterface instead of QuerySettingsInterface. And I'm not a friend of breaking such things if not really avoidable.

A side note to keep in mind:
The $query->getSource() method is marked removed for Extbase7. How can this be sensefully replaced to keep this working? I was missing any kind of code-hint near the deprecation message which could help to migrate it for Extbase7.

Any feedback welcome.

#3

Updated by Gabriel Kaufmann / Typoworx NewMedia 7 months ago

Patch file didn't upload with my previous update. So trying again.

Feedback for my refactoring suggestion is welcome. I hope it will help to improve TYPO3 and Workspace feature.

#4

Updated by Gerrit Code Review 7 months ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67924

#5

Updated by Gerrit Code Review 7 months ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67924

#6

Updated by Gerrit Code Review 7 months ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67924

#7

Updated by Nikita Hovratov 7 months ago

Hi, I took the freedom to push your patch to the review system. I would still need some help with the failing tests as I don't know how to debug functional tests.

#8

Updated by Gerrit Code Review 7 months ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67924

#9

Updated by Gerrit Code Review 7 months ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67924

#10

Updated by Gerrit Code Review 7 months ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/67924

#11

Updated by Gabriel Kaufmann / Typoworx NewMedia 7 months ago

Nikita Hovratov wrote in #note-7:

Hi, I took the freedom to push your patch to the review system. I would still need some help with the failing tests as I don't know how to debug functional tests.

Sorry didn't have the spare time yet to review this yet. I'll keep it in mind in hope being able to look after this as soon as possible.

#12

Updated by Gabriel Kaufmann / Typoworx NewMedia 6 months ago

Nikita Hovratov wrote in #note-7:

Hi, I took the freedom to push your patch to the review system. I would still need some help with the failing tests as I don't know how to debug functional tests.

I found a sponsor for working on the patch and getting the unit-tests "happy". The company www.sit.nrw - one of my current clients, will sponsor me working on the patch to make it ready for core-integration.

@Nikita Hovratov
For the core unit tests do they simply rely on this instructions?
https://docs.typo3.org/m/typo3/reference-coreapi/master/en-us/Testing/CoreTesting.html

Or is there something special to consider for local testing the tests which didn't pass yet?

#13

Updated by Nikita Hovratov 6 months ago

Hi Gabriel,

actually the tests are working now! After investing some more time I found another solution, which doesn't break the tests.
It's really simple in fact: Just remove the additional columns from the join query by comparing the tca.
Now we are only waiting for approval. I would love you to review the patch.
Regarding the tests: Yes, this is basically how local testing works. Functional tests however require a lot of resources (setting up complete frontend, database, etc.).

#14

Updated by Gabriel Kaufmann / Typoworx NewMedia 6 months ago

Nikita Hovratov wrote in #note-13:

Hi Gabriel,

actually the tests are working now!

Thanks for investigating! Great news - that sounds like I've would have done it nearly the same.

#15

Updated by Gerrit Code Review 5 months ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68915

#16

Updated by Gerrit Code Review 5 months ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68915

#17

Updated by Gerrit Code Review 5 months ago

Patch set 1 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68935

#18

Updated by Gerrit Code Review 5 months ago

Patch set 2 for branch 10.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68935

#19

Updated by Gerrit Code Review 5 months ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/68915

#20

Updated by Benni Mack 5 months ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#21

Updated by Benni Mack 4 months ago

  • Status changed from Resolved to Closed
#22

Updated by Nikita Hovratov 4 months ago

  • Related to Bug #94140: PHP Warning appears when preview in workspace and item of mm relation is disabled added
#23

Updated by Gabriel Kaufmann / Typoworx NewMedia 3 months ago

Thanks for solving and using my provided patch.

Can I somewhere see for which TYPO3 Versions and Release-Version the patch is applied? I've been using a Polyfill using XClass in the meantime and want to disable it for newer TYPO3-Releases which already provide the final solution from this ticket.

Thanks in advice.

Also available in: Atom PDF