Problem/Motivation

Suppose you have some arbitrary $entity_type_id and you want to know if there are any entities stored. Sadly:

\Drupal::entityQuery($entity_type_id)->count()->execute()

isn't guaranteed to work, because ContentEntityNullStorage::getQueryServiceName() throws an exception. Another alternative is to use the countFieldData() method and count the values for the field holding the entity ID:

$entity_manager->getStorage($entity_type_id)->countFieldData($entity_manager->getFieldStorageDefinitions($entity_type_id)[$entity_type->getKey('id')], TRUE);

but that's pretty ugly and only works for fieldable entities.

This is a major bug because:

During a configuration import, the Entity system performs validation to ensure that bundles are not deleted if they are in use on the target site. The current code for this validation assumes that the storage for all validated bundles will return a usable query via the getQuery() method. However, this is not the case for bundles using ContentEntityNullStorage (which appears to be used by contact forms). Such bundles will cause a QueryException to be thrown, breaking the import process.

Proposed resolution

Option 1: Require storage handlers to always implement a working entity query service. Even for NULL storage, which can always return empty/0 as appropriate.
Option 2: Add a hasData() method or similar to EntityStorageInterface.

Remaining tasks

  • Improve tests
  • Review
  • Commit

User interface changes

None.

API changes

None.

Suggested commit message

git commit -m 'Issue #2337753 by alexpott, ndewhurst: ContentEntityNullStorage does not implement a query service'

@ndewhurst deserves credit for their work on #2409013: BundleConfigImportValidate Attempts to Query Un-Queryable Storages

Files: 
CommentFileSizeAuthor
#16 2337753.16.patch8.23 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,526 pass(es). View
#16 14-16-interdiff.txt604 bytesalexpott
#14 2337753.14.patch8.21 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,435 pass(es). View
#14 9-14-interdiff.txt6.78 KBalexpott
#9 2337753.9.test-only.patch2.63 KBalexpott
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,492 pass(es), 0 fail(s), and 1 exception(s). View
#9 2337753.9.patch7.45 KBalexpott
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,497 pass(es). View

Comments

effulgentsia’s picture

I'd prefer option 1, because it seems odd to me for EntityQuery to not be a reliable service to use.

effulgentsia’s picture

Issue summary: View changes
effulgentsia’s picture

Issue summary: View changes
effulgentsia’s picture

Issue summary: View changes
chx’s picture

Obviously 1.

chx’s picture

Title: Decide on canonical API for querying if there are any entities of a given type » ContentEntityNullStorage does not implement a query service
Category: Task » Bug report
chx’s picture

Assigned: Unassigned » chx
Berdir’s picture

alexpott’s picture

Priority: Normal » Major
Status: Active » Needs review
Issue tags: +Configuration system, +Configurables
FileSize
7.45 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,497 pass(es). View
2.63 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,492 pass(es), 0 fail(s), and 1 exception(s). View

Here is an implementation of a null query service for null storage and a test for #2409013: BundleConfigImportValidate Attempts to Query Un-Queryable Storages which I think we should close in favour of this issue.

Bumping to major because of how this breaks configuration import validation.

alexpott’s picture

Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 9: 2337753.9.test-only.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
chx’s picture

Assigned: chx » Unassigned

return FALSE should be return 0 shouldn't it? Otherwise looks great.

alexpott’s picture

FileSize
6.78 KB
8.21 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,435 pass(es). View

Fixing #13 and improving the test coverage.

chx’s picture

> Tests importing a delete of contact message.

My English parser gave up on this.

alexpott’s picture

FileSize
604 bytes
8.23 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,526 pass(es). View

Fixing my english.

chx’s picture

Status: Needs review » Reviewed & tested by the community

In a minor followup we should add a create method to ConfigImporter to avoid writing all those gets like five times in core already.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 1776b01 on 8.0.x
    Issue #2337753 by alexpott: ContentEntityNullStorage does not implement...

Status: Fixed » Closed (fixed)

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