All Zend Framework feed plugins are configured with shareByDefault off if using the Zend Service manager. In order to work properly in the Symfony service container, they should be registered in *.services.yml as prototype scope, which is the equivalent setting.

This causes problems like #2348339: Aggregator parses only the first element of an atom feed.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because some services are registered incorrectly and won't work as expected.
Issue priority Normal because only the aggregator is affected in practice as far as we know.
Prioritized changes The main goal of this issue is fixing a service registration bug.
CommentFileSizeAuthor
#17 zend_feed_plugins_are-2151103-17.patch5.3 KBtwistor
#17 zend_feed_plugins_are-2151103-17-should-fail.patch2.38 KBtwistor
#17 interdiff.txt1.03 KBtwistor
#10 zend_feed_plugins_are-2151103-10.patch5.37 KBkerby70
#7 zend_feed_plugins_are-2151103-7.patch5.38 KBkerby70
#4 drupal-prototype_scope_zend_feed-2151103-4.patch5.38 KBParisLiakos
#1 drupal-prototype_scope_zend_feed-2151103-1.patch5.39 KBParisLiakos
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Active » Needs review
FileSize
drupal-prototype_scope_zend_feed-2151103-1.patch5.39 KB

@twistor should take credit for this as well

anavarre queued 1: drupal-prototype_scope_zend_feed-2151103-1.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: drupal-prototype_scope_zend_feed-2151103-1.patch, failed testing.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Status: Needs work » Needs review
FileSize
drupal-prototype_scope_zend_feed-2151103-4.patch5.38 KB

reroll

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented
Related issues: +#2348339: Aggregator parses only the first element of an atom feed
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Needs review » Needs work
Issue tags: +Needs reroll
kerby70’s picture

kerby70 CreditAttribution: kerby70 at Blink Reaction (now part of FFW) commented
Issue tags: -Needs reroll
FileSize
zend_feed_plugins_are-2151103-7.patch5.38 KB

Reroll attached.

kerby70’s picture

kerby70 CreditAttribution: kerby70 commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: zend_feed_plugins_are-2151103-7.patch, failed testing.

kerby70’s picture

kerby70 CreditAttribution: kerby70 at Blink Reaction (now part of FFW) commented
FileSize
zend_feed_plugins_are-2151103-10.patch5.37 KB

Reroll.

kerby70’s picture

kerby70 CreditAttribution: kerby70 commented
Status: Needs work » Needs review
kerby70’s picture

kerby70 CreditAttribution: kerby70 commented
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes

+1 for this issue.

cilefen queued 10: zend_feed_plugins_are-2151103-10.patch for re-testing.

cilefen’s picture

cilefen CreditAttribution: cilefen commented 
  1. +++ b/core/core.services.yml
@@ -1041,49 +1041,74 @@ services:
+# All plugins should be set as prototype scope
+# See Zend\Feed\Reader\ExtensionPluginManager::shareByDefault

    I am not sure we need this comment. But I could be wrong. 

  2. +++ b/core/core.services.yml
@@ -1041,49 +1041,74 @@ services:
+# All plugins should be set as prototype scope
+# See Zend\Feed\Writer\ExtensionPluginManager::shareByDefault

    This is the same as #1.

Could the next patch uploader please provide a tests-only patch?

twistor’s picture

twistor CreditAttribution: twistor as a volunteer commented
FileSize
interdiff.txt1.03 KB
zend_feed_plugins_are-2151103-17-should-fail.patch2.38 KB
zend_feed_plugins_are-2151103-17.patch5.3 KB

The last submitted patch, 17: zend_feed_plugins_are-2151103-17-should-fail.patch, failed testing.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

The test proves the service registration problem is real. I am satisfied.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed ae32aaa and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed ae32aaa on 8.0.x 
    Issue #2151103 by twistor, kerby70, ParisLiakos, cilefen: Zend feed...
Fabianx’s picture

Fabianx CreditAttribution: Fabianx for Drupal Association commented

Oh,

scopes are deprecated in 2.8:

- https://github.com/symfony/symfony/blob/2.8/UPGRADE-2.8.md#dependencyinj...

but shared: false does not become available until 2.8.

So we'll need to support a forward compatibility fix for #2497243: Replace Symfony container with a Drupal one, stored in cache in the mean time.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.