But this is simple to solve by shuffling around a few lines of code. The patch is a net negative in terms of lines of code, of course.

CommentFileSizeAuthor
#4 2156835_4.patch3.86 KBchx
#4 interdiff.txt415 byteschx
loadByProperties.patch3.71 KBchx

Comments

chx’s picture

chx commented
Status: Active » Needs review

Assigning to tim.plunkett for review.

chx’s picture

chx commented

It'd be nice to explore injecting the entity query object in a followup. Config entities already do it.

Status: Needs review » Needs work

The last submitted patch, loadByProperties.patch, failed testing.

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt415 bytes
new 2156835_4.patch3.86 KB
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Good work. This allows for potential performance optimized entity query implementations to use it.

damiankloip’s picture

damiankloip commented
Assigned: tim.plunkett » Unassigned
damiankloip’s picture

damiankloip commented

Looks good to me too.

Only thing is we might want to put the call to entityQuery() in a wrapper function? But prob don't need this now, only when/if this class gets a dedicated unit test or something.

Also, spotted a really minor issue in the ConfigEntityUnitTest, so filed a small followup for that: #2158299: ConfigEntityUnitTest does not check properties on all loaded entities

chx’s picture

chx commented

The call to entityQuery will go away when I will convert all the classes to receive the query factory via injection as config already does.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

yched’s picture

yched commented

Would there be a way to optimize Config EFQ so that it doesn't load and loop over all config entries when the query has conditions on 'id' ?

[edit: patch at #2163919: Optimise Config EntityQueries in case there are conditions on the 'id' ]

Status: Fixed » Closed (fixed)

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