The getmultiple method in ComponentConfigResolver.php expects an array of ids to be passed in to give a list of components. This method needs to be able to return all components if requested. The $this->entityStorage->loadMultiple method can return all config entities if null is passed, but the method in question cannot accept a null argument, Submitting a reallly simple patch to solve this issue.

Thanks
Sukanya

CommentFileSizeAuthor
#4 2818219-4.patch1.37 KBTR
#2 ComponentConfigResolverGetMultipleMethod-2818219-1.patch635 bytessukanya.ramakrishnan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sukanya.ramakrishnan created an issue. See original summary.

sukanya.ramakrishnan’s picture

sukanya.ramakrishnan CreditAttribution: sukanya.ramakrishnan commented
FileSize
ComponentConfigResolverGetMultipleMethod-2818219-1.patch635 bytes

Submitting a really simple patch for this issue!

TR’s picture

TR CreditAttribution: TR commented
Status: Active » Needs review
TR’s picture

TR CreditAttribution: TR commented
FileSize
2818219-4.patch1.37 KB

When you post a patch you should set the status to "Needs review" to trigger the testbot to evaluate your patch and to indicate to reviewers that there is a patch that needs to be reviewed!

The interface needs to be changed too ... here's a new patch.

Status: Needs review » Needs work

The last submitted patch, 4: 2818219-4.patch, failed testing. View results

TR’s picture

TR CreditAttribution: TR commented
Category: Feature request » Task
Status: Needs work » Postponed

Well, it was a good idea, but as you can see if you modify the signature of RulesComponentResolverInterface::getMultiple(), like you must, then there are two component resolvers that must be changed. Your patch changes only one - ComponentConfigResolver. The other one is EventComponentResolver.

Looking at the source code for EventComponentResolver::getMultiple() you will see that it CAN'T accept a NULL argument instead of an array of IDs, because of how it operates.

The motivation for the patch is that the IMPLEMENTATION of ComponentConfigResolver::getMultiple() calls $this->entityStorage->loadMultiple(), but as the implementation can change or be different for different resolvers, the thing that defines the behavior must be RulesComponentResolverInterface and in this case we can't change the API of that interface to return ALL components because that won't work and is not what we want in the case of the EventComponentResolver

Actually, this does point out an inconsistency in the API - the semantics of that method are different for the two resolvers - in one the array of ids is COMPONENT ids and in the other it is EVENT ids. So this should really be re-thought ...

Regardless, this patch is not something we can do. I'm postponing this issue and renaming it a task - we need to look at the design of that interface and make sure it can be used consistently.