| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | entity system |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
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
Comments
#1
This 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.
#2
As 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.
#3
That sounds like a plan. :-)
#4
I do not understand this issue. Can we get a better + more concise summary? To clarify what the actual problem is?
#5
@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.
#6
#2: drupal-1887058-2.patch queued for re-testing.
#7
Not 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.
#8
Granted. It's still yet another procedural wrapper. We should stop with adding those at some point.