In #1184272: Remove deprecated $conditions support from entity controller we removed $conditions
from entity_load_multiple()
. Interestingly though, instead of actually removing it after it has already been marked as @deprecated in D7 we added a new method to EntityStorageControllerInterface
called EntityStorageControllerInterface::loadByProperties()
as well as another wrapper called entity_load_multiple_by_properties(). Additionally, this comes with another utility method named ->buildPropertyQuery()
. So, to summarize: Instead of curing the problem with $conditions
we actually just moved it to somewhere else by introducing a new method as well as a wrapper, which, by the way is just a wrapper for entity_query()
itself anyways (which itself is a wrapper for the DIC)...
CMI currently has a custom work-around for this to bypass EFQ but once #1846454: Add Entity query to Config entities we should remove this stuff altogether and simply tell people to use EFQ for loading things by properties.
Also related:
#1893442: Move BlockStorageController::loadByProperties() into ConfigStorageController
Comment | File | Size | Author |
---|---|---|---|
#2 | drupal-1887058-2.patch | 517 bytes | dawehner |
Comments
Comment #0.0
fubhy CreditAttribution: fubhy commentedUpdating issue summary.
Comment #0.1
fubhy CreditAttribution: fubhy commentedUpdated issue summary.
Comment #1
tim.plunkettThis is currently needed by blocks, and is being moved up a level for all config entities: #1893442: Move BlockStorageController::loadByProperties() into ConfigStorageController
So, postponing on #1846454: Add Entity query to Config entities, since both storage controllers would need entity_query support first.
Comment #2
dawehnerAs issues still maybe add usage to that function / maybe contrib already start to port stuff (as an example #916388: Convert menu links into entities)
I would suggest to add an @deprecated for now and slowly remove all the usage to finally remove it.
Comment #3
tstoecklerThat sounds like a plan. :-)
Comment #4
sunI do not understand this issue. Can we get a better + more concise summary? To clarify what the actual problem is?
Comment #5
fubhy CreditAttribution: fubhy commented@sun: There is no point in having multiple levels of these pointless wrappers. We got entity field query, let's simply use it directly. In fact, I would even vote for removing the loadByProperties() method. It's simply not the job of the storage controller to do that.
Comment #6
ParisLiakos CreditAttribution: ParisLiakos commented#2: drupal-1887058-2.patch queued for re-testing.
Comment #7
BerdirNot convinced.
This does use EFQ internally, so the problem that $conditions had (not being able to cache it properly), is solved. So the issue summary is not correct, we *did* cure the $conditions problem, without enforcing everyone to add at least 4 lines of code.
EFQ got a lot easier to use since this got in, that's true, but we also have 54 calls to this function that would have to be updated and inlined.
Comment #8
fubhy CreditAttribution: fubhy commentedGranted. It's still yet another procedural wrapper. We should stop with adding those at some point.
Comment #8.0
fubhy CreditAttribution: fubhy commentedUpdated issue summary.
Comment #10
Mile23entity_load_multiple_by_properties()
is marked deprecated for removal before Drupal 9, andEntityStorageController
is a handler now: https://www.drupal.org/node/2200867EntityStorageInterface::loadByProperties()
exists and is not deprecated.Comment #11
cilefen CreditAttribution: cilefen commentedComment #22
aiphesI encounter an issue to update my code using this function. Someone could help me ?
https://drupal.stackexchange.com/questions/309417/render-custom-region-i...
Comment #25
dimitriskr CreditAttribution: dimitriskr commentedQuick search of these two functions on latest 11.x returned no results. If it's still an issue, please re-open it with the findings