Problem/Motivation

The entity.query, entity.query.sql and the entity.query.keyvalue services are factories and yet they are not named so.

Proposed resolution

Rename them to *.factory making them in line with cache.factory, config.factory, logger.factory etc etc

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#35 entity-query-2156883-35.patch26.31 KBGaelan
#31 the_entity_query-2156883-31.patch26.25 KBdimaro
#26 2156883-26.patch26.14 KBheykarthikwithu
#23 0001-2156883-entity.query-service-name-is-very-confusing..patch29.54 KBdarol100
#22 the_entity_query-2156883-22.patch25.03 KBdeepakaryan1988
#19 the_entity_query-2156883-19.patch24.89 KBdimaro
#19 interdiff-2156883-14-19.txt905 bytesdimaro
#14 2156883-14.patch24.88 KBalexandre.todorov
#10 2156883_10.patch25.09 KBchx
#9 2156883-entity-query-factory-9.patch25.09 KBrichardcanoe
#7 2156883-entity-query-factory-7.patch25.09 KBrichardcanoe
eqf.patch19.67 KBchx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

This matches config.factory (the other factory I interact with on a daily basis).
+1, RTBC if green

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

eqf.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

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

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review

eqf.patch queued for re-testing.

Status: Needs review » Needs work

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

chx’s picture

chx CreditAttribution: chx commented
Issue tags: +Novice, +Needs reroll
richardcanoe’s picture

richardcanoe CreditAttribution: richardcanoe commented
Status: Needs work » Needs review
FileSize
2156883-entity-query-factory-7.patch25.09 KB

Rerolled & extended the patch, changed all instances of entity.query to entity.query.factory

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Needs work
Issue tags: -Needs reroll

Thanks for the additional work! I think entity.query.factory.config looks strange and doesn't adhere to the pattern established and should read entity.factory.config.query and the same for sql and keyvalue.

richardcanoe’s picture

richardcanoe CreditAttribution: richardcanoe commented
Status: Needs work » Needs review
FileSize
2156883-entity-query-factory-9.patch25.09 KB

Changed config, sql and keyvalue as requested. Now I'm confused!

chx’s picture

chx CreditAttribution: chx commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
2156883_10.patch25.09 KB

OMG. What did I write. I am so wrong and I am so sorry! Please accept my apologies and this corrected patch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/core.services.yml
@@ -573,20 +573,20 @@ services:
-  entity.query:
+  entity.query.factory:

Can we add an alias for entity.query to the entity.query.factory. This makes the change far far less disruptive for existing contrib. Can we also include a comment that this service will be removed in Drupal 9.

Also since this is disruptive a beta evaluation would be great. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

chx’s picture

chx CreditAttribution: chx commented

If that's the case then why don't we just postpone the whole thing to D9? There's hardly any point AFAIK.

alexandre.todorov’s picture

alexandre.todorov CreditAttribution: alexandre.todorov commented

Hi,
I'm working on this issue in Drupal Dev Days Mentor Sprint Montpellier (France) 2015

alexandre.todorov’s picture

alexandre.todorov CreditAttribution: alexandre.todorov commented
FileSize
2156883-14.patch24.88 KB

Rerolled the patch #10 and added service aliases for existing contrib.

dimaro’s picture

dimaro CreditAttribution: dimaro at La Drupalera by Emergya commented
Status: Needs work » Needs review

Change status to Needs review.

Status: Needs review » Needs work

The last submitted patch, 14: 2156883-14.patch, failed testing.

Status: Needs work » Needs review

dimaro queued 14: 2156883-14.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 14: 2156883-14.patch, failed testing.

dimaro’s picture

dimaro CreditAttribution: dimaro at La Drupalera by Emergya commented
Status: Needs work » Needs review
FileSize
interdiff-2156883-14-19.txt905 bytes
the_entity_query-2156883-19.patch24.89 KB

Correct the service aliases.

Hope it works.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Category: Bug report » Task
Status: Needs review » Needs work
Issue tags: +Needs beta evaluation, +Needs reroll
deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
CreditAttribution: deepakaryan1988 at Srijan | A Material+ Company commented
Assigned: Unassigned » deepakaryan1988
Status: Needs work » Active
deepakaryan1988’s picture

deepakaryan1988
Primary language Hindi
CreditAttribution: deepakaryan1988 at Srijan | A Material+ Company commented
Assigned: deepakaryan1988 » Unassigned
Status: Active » Needs review
Issue tags: -Needs reroll
FileSize
the_entity_query-2156883-22.patch25.03 KB

Removing needs reroll tag
and I am re-rolling the patch #19 .

darol100’s picture

darol100 CreditAttribution: darol100 as a volunteer and commented
FileSize
0001-2156883-entity.query-service-name-is-very-confusing..patch29.54 KB

I think we should fix the comments as well ?

If so here are few things that are needs work.

d8/core/lib/Drupal/Core/Entity/EntityStorageBase.php

  • Line 477: // Access the service directly rather than entity.query factory so the
  • Line 486: // Access the service directly rather than entity.query factory so the

d8/core/lib/Drupal/Core/Entity/Query/QueryFactory.php

  • Line 22: * @todo https://www.drupal.org/node/2389335 remove entity.query service and

In this patch I change entity.query -> entity.query.factory in comments as well. If we do not have to change entity.query -> entity.query.factory in comments, the patch #22 is ready to go.

dimaro’s picture

dimaro CreditAttribution: dimaro at La Drupalera by Emergya commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

What is the status of this issue?
At the moment, Needs reroll.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Assigned: Unassigned » heykarthikwithu
heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Assigned: heykarthikwithu » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2156883-26.patch26.14 KB

Status: Needs review » Needs work

The last submitted patch, 26: 2156883-26.patch, failed testing.

darol100’s picture

darol100 CreditAttribution: darol100 as a volunteer and commented
Version: 8.0.x-dev » 8.1.x-dev

This looks to me like an API change, if so we can't do it. I will move it to next minor release just in case, even though this might not get committed.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Minor releases can't do API changes either, only additions.

Possibly we just want to deprecate this in favor of getStorage('node')->getQuery() and remove it in 9.x.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dimaro’s picture

dimaro CreditAttribution: dimaro at La Drupalera by Emergya commented
Status: Needs work » Needs review
FileSize
the_entity_query-2156883-31.patch26.25 KB

Rerolled #26. Patch to 8.2.x.

Status: Needs review » Needs work

The last submitted patch, 31: the_entity_query-2156883-31.patch, failed testing.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dimaro’s picture

dimaro CreditAttribution: dimaro at La Drupalera by Emergya commented
Issue tags: +Needs reroll

The latest patch should be rerolled against 8.3.x

Gaelan’s picture

Gaelan CreditAttribution: Gaelan as a volunteer and at Google Code-In commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
entity-query-2156883-35.patch26.31 KB

Rerolled.

Status: Needs review » Needs work

The last submitted patch, 35: entity-query-2156883-35.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Issue tags: -Novice, -Needs beta evaluation

As I wrote above, we shouldn't be changing those services IMHO instead we should just remove them. See #2389335: Deprecate entity.query service and replace with using the entity storage's getQuery() method. I'd say we should close this as a duplicate of that issue.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs work » Postponed

The mentioned issue just removes the main service, the others remain. So not a duplicate, but postponed on that, most of it would go away after that.

And unlike the current patch, might actually be possible in 8.x, not sure.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.