silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
722 stars 821 forks source link

StandardRelatedDataService not working for complex relations #11067

Open sb-relaxt-at opened 10 months ago

sb-relaxt-at commented 10 months ago

Affected Version

Any version including StandardRelatedDataService (Silverstripe 4 and 5).

Description

There are multiple cases in which the StandardRelatedDataService does not work (Undefined index) or return wrong results. I do understand that the DataService is marked for internal use and might be not intended for use with other DataObjects than Files. Nevertheless these issues also occur if Files are used in complex relationships.

Case 1

There is no support for polymorphic relations besides the Parent relation. If polymorphic relations are involved there will be no filtering by class, resulting in wrong related data returned.

https://github.com/silverstripe/silverstripe-framework/blob/26c8c0f4f63783d8d1968941a388cc9e78726c1a/src/ORM/RelatedData/StandardRelatedDataService.php#L120

https://github.com/silverstripe/silverstripe-framework/blob/26c8c0f4f63783d8d1968941a388cc9e78726c1a/src/ORM/RelatedData/StandardRelatedDataService.php#L231

https://github.com/silverstripe/silverstripe-framework/blob/26c8c0f4f63783d8d1968941a388cc9e78726c1a/src/ORM/RelatedData/StandardRelatedDataService.php#L318

https://github.com/silverstripe/silverstripe-framework/blob/26c8c0f4f63783d8d1968941a388cc9e78726c1a/src/ORM/RelatedData/StandardRelatedDataService.php#L371

Case 2

Given the the following DataObject structure for a Many-many-through Relation between X and Y by class B:

A:
  has_one:
    x: X
B: #extends A
  has_one:
    y: Y

The service won't be able to find the configuration for x as it uses the uninherited config of B which does not include the relation to X, resulting in an Undefined index see:

https://github.com/silverstripe/silverstripe-framework/blob/26c8c0f4f63783d8d1968941a388cc9e78726c1a/src/ORM/RelatedData/StandardRelatedDataService.php#L417

Edit: Fixed a wrong relation in the example.

emteknetnz commented 8 months ago

Case 1 will be fixed in 5.2.0 with this PR - https://github.com/silverstripe/silverstripe-framework/pull/11107

sb-relaxt-at commented 8 months ago

@emteknetnz I am not sure that your PR addresses all references to Parent in StandardRelatedDataService, I suppose there are at least four of them.