Creation of dynamic property Drupal\feeds\Feeds\Item\SyndicationItem::$parent:fid is deprecated

'parent:fid' is actually a dynamic property. It is generated in \Drupal\feeds\Feeds\Source\BasicFieldSource. Besides 'parent:fid', you also have 'parent:uid', 'parent:created', 'parent:title' and many others.

So simply declaring $parent_fid as a variable here is not a sustainable solution.

So I think that BaseItem needs to check in methods `get()` and `set()` if the property to set exists. And if not, write to `$data` property like DynamicItem does. I don't know however if there any drawbacks in that approach, but I think the risk for that is low.

Almost. BaseItem should check first if the requested field exists as property in the class. And if not, fallback to the $data array.

@MegaChriz Tried to fix the review. Please check and do let me know, if anymore changes are needed.

• I think that property_exists() should be used in both BaseItem::get() and BaseItem::set().
• BaseItem::get() should check if $this->data[$field] exists before returning that.
• BaseItem::toArray() should merge the object vars with $data array and then remove the extra data array from it.
• In Syndicati‎onItem.php‎ there is a blank line removed that should not have been removed.
• Note that the code is currently failing tests.

Reverted the blank line code on Syndicati‎onItem.php for failing tests. I will working on the remaining.

Reviewed the MR!174 and suggested few changes

cc: @MegaChriz

I created a new MR that just tells PHP it's OK to have dynamic properties on the item. This is a simple approach. I'm not sure if this is really the best long term solution.

@bkosborne
Thanks for your contribution. This could be a viable solution too, though it is possible that it will no longer be supported in a future version of PHP (PHP 9?). I'm not sure yet though if it is going to be removed, it's not clear from https://www.php.net/manual/en/language.oop5.properties.php#language.oop5...

Meanwhile, I've updated the other MR:

• I've addressed my own remarks from #11.
• I made sure that the data property cannot be get or set directly. An UnexpectedValueException will be thrown when trying to do so. Added test coverage for this change.

Tested MR !174 on 3 custom Feeds Importers that extend the BaseItem class.

• Drupal Core 11.1.3
• PHP 8.3.15
• Feeds v3.0.0

Works as expected and no PHP deprecation notices now. RTBC for me

This also significantly sped up the batch process.

Another confirmation that this patch does the trick. My specs:

Drupal Core 10.4.6
PHP 8.3.20
Feeds v3.0.0

I adjusted the code a bit, because I did not like that 'data' became a preserved key. The changes ensure that a field called 'data' can be set without overwriting the complete $data property.

I also expanded the test coverage a bit.

AI helped a bit with this.

I looked through the code one more time, did not spot anything suspicious. Merged it. Thanks all who helped on this issue.

Any chance we could get a 3.1 (or 3.0.1) release that includes this fix? This generates a lot of unnecessary noise in logs.

We are seeing deprecation warnings on cron run after updating to PHP 8.3.21 and clearing caches. These are only notice errors, I think, so we should still be fine. But there are about 500 of them after running cron from the admin UI.

    Deprecated function: Creation of dynamic property 
    Drupal\feeds\Feeds\Item\SyndicationItem::$blog 
    is deprecated in Drupal\feeds\Feeds\Item\BaseItem->set() 
    (line 21 of modules/contrib/feeds/src/Feeds/Item/BaseItem.php).

    Deprecated function: Creation of dynamic property 
    Drupal\feeds\Feeds\Item\SyndicationItem::$parent:field_group 
    is deprecated in Drupal\Core\Queue\DatabaseQueue->claimItem() 
    (line 150 of core/lib/Drupal/Core/Queue/DatabaseQueue.php).

https://git.drupalcode.org/project/feeds/-/merge_requests/174.diff

Works perfect for me.

Thanks!