Problem/Motivation
Entity Query sucks! Really! erm. Where was I? Yes. Currently you need to implement all five classes (QueryFactory, Query, QueryAggregate, Tables, Condition) to add a single line to code to, say Tables. This takes unnecessary boilerplate to a new level.
Proposed resolution
Make only an empty QueryFactory mandatory. Then grab the namespace of it and the namespace of any ancestors and use the first class that exists within those namespaces. For eg. if Drupal\user\Entity\Query\Sql\QueryFactory
extends Drupal\Core\Entity\Query\Sql\QueryFactory
then we will look for the rest (Tables, Condition, Query, QueryAggregate) first in Drupal\user\Entity\Query\Sql
then in Drupal\Core\Entity\Query\Sql
. Much like DBTNG drivers just a bit more flexible.
Remaining tasks
Commit this.
User interface changes
None.
API changes
If you were extending Entity Query, you can drop a little boilerblate.
Related Issues
#1751274: Do not query user_roles directly
#2038707: Entity query sql backend limits storage controllers changes in contrib
Comment | File | Size | Author |
---|---|---|---|
#9 | 2096271_4.patch | 8.93 KB | chx |
#4 | 2096271_4.patch | 8.94 KB | chx |
#4 | interdiff.txt | 1.7 KB | chx |
better_eq.patch | 8.54 KB | chx | |
Comments
Comment #1
chx CreditAttribution: chx commentedNote that when this and #1751274: Do not query user_roles directly both is in, we can drop the three new classes from that issue or commit this first and don't even commit those. The test in that issue is the test for this issue too.
Comment #2
dawehnerNice!
Comment #3
alexpottWe're missing the docblock for $namespace parameter.
Why are we using self here? Let's add a comment.
Comment #4
chx CreditAttribution: chx commentedComment #5
chx CreditAttribution: chx commentedComment #6
dawehnerI can't set myself a RTBC under a code in drupal which uses final, sorry.
Comment #7
chx CreditAttribution: chx commentedErm, QueryFactory hardwires QueryBase (because it kind of needs to). If a class extends QueryBase and changes this method then we have a hilarious situation where the query is finding different classes to the factory. Let's not allow for that. For that matter, most of our utility classes are final as well.
Comment #8
dawehnerEvery final is wrong and just makes the life of people who actually cares about unit testing a HELL, let me repeat it a HELL. No matter whether there are good intentions behind the usage of it.
Comment #9
chx CreditAttribution: chx commentedFine.
Comment #10
dawehnerThank you!
Comment #11
alexpottCommitted 6976d52 and pushed to 8.x. Thanks!
We need a change notification that is basically the content on the issue summary and can point to #1751274: Do not query user_roles directly as an example of the benefits of this change.
Comment #12
chx CreditAttribution: chx commentedWe already had a change notice I just needed to update it https://drupal.org/node/2070303
Comment #13.0
(not verified) CreditAttribution: commentedUpdated issue summary.