Project

General

Profile

Actions

Bug #98148

closed

Extbase persistence erronously tries to persist unmapped private property

Added by Christian Ebert over 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Extbase
Target version:
Start date:
2022-08-16
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
11
PHP Version:
7.4
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

Description

TYPO3 11.5.14
Currently when persisting a domain model with an internal private property which should not be persisted to the database a "Cannot access private property ..." error occurs. So far as I can see this was introduced with the latest changes applied with commit e92d3212e5990d525b71ac967445add7e994b8c1 . In 11.5.13 this error not occurs. There is also no TCA Configuration for the property.

The error is triggered in:
/typo3/sysext/extbase/Classes/DomainObject/AbstractDomainObject.php line 131
at TYPO3\CMS\Extbase\DomainObject\AbstractDomainObject->_getProperty('solicitor')
in /var/www/html/www/public/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php line 598
at TYPO3\CMS\Extbase\Persistence\Generic\Backend->insertObject
in /var/www/html/www/public/typo3/sysext/extbase/Classes/Persistence/Generic/Backend.php line 281

The domain model is as follows:

class Quote extends AbstractEntity
{

    private ?\MyNamespace\Solicitor $solicitor;

}

Related issues 2 (1 open1 closed)

Related to TYPO3 Core - Bug #95819: Extbase does not support uninitialized domain object propertiesClosedAlexander Schnitzler2021-10-30

Actions
Related to TYPO3 Core - Bug #98190: Extbase fails to resolve chained validationsAccepted

Actions
Actions #1

Updated by Christian Ebert over 2 years ago

  • Description updated (diff)
Actions #3

Updated by Oliver Hader over 2 years ago

  • Related to Bug #95819: Extbase does not support uninitialized domain object properties added
Actions #4

Updated by Alexander Schnitzler over 2 years ago

I suggest filtering out private properties, just like get_object_vars() did (to be able to backport as bugfix) and reintroduce @Transient for the upcoming version 12. I am sorry that bug slipped through.

Actions #5

Updated by Oliver Hader about 2 years ago

Alexander Schnitzler wrote in #note-4:

I suggest filtering out private properties, just like get_object_vars() did (to be able to backport as bugfix) and reintroduce @Transient for the upcoming version 12. I am sorry that bug slipped through.

Can you please provide a patch for that?

Actions #6

Updated by Oliver Hader about 2 years ago

  • Status changed from New to Accepted
Actions #7

Updated by Stefan Bürk about 2 years ago

The change in #95819 generally is okay. It basicly behavious as before. This
means, that properties beginning with $_ are already ignored. However, there
was a slight oversight with the change from AbstractDomainModel->_getProperties()
to ReflectionService->getClassSchema() and the following loop in class methods
\TYPO3\CMS\Extbase\Persistence\Generic\Backend->insertObject() and
\TYPO3\CMS\Extbase\Persistence\Generic\Backend->persistObject().

As $propertyName and $propertyValue are not included, code has been added to rertieve
them in the loop. Getting the $propertyValue is however done to early and should only
be called if property is callable.

Patch will follow.

Actions #8

Updated by Gerrit Code Review about 2 years ago

  • Status changed from Accepted to Under Review

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

Actions #9

Updated by Gerrit Code Review about 2 years ago

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

Actions #10

Updated by Gerrit Code Review about 2 years ago

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

Actions #11

Updated by Stefan Bürk about 2 years ago

Beside that it worked before and broke - maybe it is wise to use the "_" prefix for the property, even if it is private and not defined in TCA. The prefix also works for "protected" properties.

Actions #12

Updated by Oliver Hader about 2 years ago

I'm getting errors in object validation when calling a detail action, like e.g. (having the patch applied).

(1/1) #1546632293 RuntimeException
Could not get value of property "ExtbaseTeam\BlogExample\Domain\Model\Blog::oida", make sure the property is either public or has a getter getOida(), a hasser hasOida() or an isser isOida().

I'm trying the approach Alex suggested with filtering the properties...

Actions #13

Updated by Gerrit Code Review about 2 years ago

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

Actions #14

Updated by Oliver Hader about 2 years ago

The annotation issue I mentioned earlier is a different topic and did exist prior to #95819 - short summary on it:

class Parent extends AbstractEntity
{
  protected Child $child;
}

use TYPO3\CMS\Extbase\Annotation\Validate;
class Child extends AbstractEntity
{
  /**
   * @Validate("StringLength", options={"minimum": 1, "maximum": 80})
   */
  public string $title = '';
}

class ParentController extends ActionController
{
  public function showAction(Parent $parent): ResponseInterface
  {
    // ...
  }
}

Action Parent.show gets argument of type Parent, which has a non-accessible property to type Child, which has a validation rule of a property. Following the validation chain is correct in general, unless some part of it cannot be accessed (which is protected property Parent::$child in this example).

In case validation for internal values is desired/expected, this probably would be something for reflection.
In case developers would "need to adjust their code", current RuntimeException (#1546632293) is semantically wrong, it has to be a LogicException instead.

→ I've create issue #98190 for that particular validation issue

Actions #15

Updated by Gerrit Code Review about 2 years ago

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

Actions #16

Updated by Gerrit Code Review about 2 years ago

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

Actions #17

Updated by Gerrit Code Review about 2 years ago

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

Actions #18

Updated by Oliver Hader about 2 years ago

  • Related to Bug #98190: Extbase fails to resolve chained validations added
Actions #19

Updated by Gerrit Code Review about 2 years ago

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

Actions #20

Updated by Gerrit Code Review about 2 years ago

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

Actions #21

Updated by Gerrit Code Review about 2 years ago

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

Actions #22

Updated by Gerrit Code Review about 2 years ago

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

Actions #23

Updated by Stefan Bürk about 2 years ago

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

Updated by Benni Mack about 2 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF