In ChainStoreResolver priority of the service is a key of resolvers array, this causes a problem when two or more resolver services have the same priority. In this case only 1 resolver will be triggered, because all other will overwrite the array with the same key.

  /**
   * {@inheritdoc}
   */
  public function addResolver(StoreResolverInterface $resolver, $priority) {
    $this->resolvers[$priority] = $resolver;
  }

The same chain is used for price resolvers but there is no $priority as a key of array.

Comments

a.dmitriiev created an issue. See original summary.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented

Can you test and prove this? Because the priority should be adjusted by the container to prevent conflicts.

EDIT: The logic is here \Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass::process

If there is an issue, then you either need to manually resolve, or Core will need to..

// Line 127, gets priority or defaults to zero; this would need to be magical.
$handlers[$id] = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
..
        // Sort all handlers by priority.
        arsort($handlers, SORT_NUMERIC);
agoradesign’s picture

agoradesign CreditAttribution: agoradesign commented

If there is an issue, then you either need to manually resolve, or Core will need to..

The price resolvers definitely work without setting the prio as array key (with correctly ordered resolvers). So if we can prove, that explicitely setting the key is causing problems, why don't we simply change the chain store resolver to behave like the price resolver?

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented

Yeah, lets' do that. I remember when dealing with priority I was a bit inept at understanding out how worked. We shouldn't pass the priority parameter and just [] it, as the container adds.

agoradesign’s picture

agoradesign CreditAttribution: agoradesign commented

haha, I know that.. was the same with my price resolvers. I had to play around with different priority values of my custom resolver in order to understand Drupal's/Symfony's behaviour here

a.dmitriiev’s picture

a.dmitriiev CreditAttribution: a.dmitriiev as a volunteer and at 1xINTERNET commented

Please add to the next commit :) Because now it is a bit confusing.

Thanks.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Title: Chain Store Resolver $priority » Remove $priority from resolvers since the implementation is incomplete
Category: Bug report » Task

Okay, so we basically want to revert #2796005: Chain resolvers do not support tagged service priority..
I'm fine with that, since we aren't doing our own sorting by priority and deduplicating anyway.

Sure would be nice to have a single ChainResolver. Need to look into that again.

  • bojanz committed e4d6636 on 8.x-2.x authored by steveoliver 
    Issue #2833140: Remove $priority from resolvers since the implementation...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Active » Fixed

Done. Gave the commit credit to steveoliver cause I owe him one from #2832595: Payments tab on orders no longer shows up.

Status: Fixed » Closed (fixed)

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