DefaultSelection::buildEntityQuery requires entities to have bundle key.

Good catch, but this needs tests.

IIRC there is a bundle-less entity type defined in the test modules?

+++ b/core/lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/DefaultSelection.php
@@ -445,8 +445,11 @@ protected function buildEntityQuery($match = NULL, $match_operator = 'CONTAINS')
+      $bundleKey = $entity_type->getKey('bundle');

Using hasKey() would be better here.

Nitpick: variable should be in snake case.

I think the problem is a bit deeper. Configuration value $configuration['target_bundles'] should not be an array at all.

There is a comment just above this condition

if (is_array($configuration['target_bundles'])) {

// If 'target_bundles' is NULL, all bundles are referenceable, no further
// conditions are needed.

So actually the problem is that $configuration['target_bundles'] is array instead of NULL

I traced this problem down to buildConfigurationForm method:

    if ($entity_type->hasKey('bundle')) {
      // ......
    }
    else {
      $form['target_bundles'] = [
        '#type' => 'value',
        '#value' => [],
      ];
    }

So the form sets the target_bundles as array if the entity doesn't support bundles, but instead it should use NULL

Moving back to NW for the tests.

I don't think the problem is in array. The value for target_bundles is set to NULL if it's an empty array in \Drupal\Core\Entity\Plugin\EntityReferenceSelection\DefaultSelection::validateConfigurationForm

    // If no checkboxes were checked for 'target_bundles', store NULL ("all
    // bundles are referenceable") rather than empty array ("no bundle is
    // referenceable" - typically happens when all referenceable bundles have
    // been deleted).
    if ($form_state->getValue(['settings', 'handler_settings', 'target_bundles']) === []) {
      $form_state->setValue(['settings', 'handler_settings', 'target_bundles'], NULL);
    }

Also, ::buildEntityQuery() only tries to get bundle key only if target_bundles is an array and not an empty one. The only way to reproduce this bug, intentionally providing at least one bundle for bundleless entity.

I'm attaching a patch with test that shows that everything is works properly with target_bundles: [], NULL and without value at all with User entity, which has no bundle key.

@mheip since you are mentioning a custom entity, maybe it had bundle key when you installed it? Are you sure that the installed entity schema for your custom entity doesn't have a bundle key? Maybe a field, that is using references, was configured when your entity was having a bundle key?

Test only patch doesn't catch

As a workaround, to make debug this problem easier, we can throw an exception in case when target_bundles are provided, but entity doesn't support bundles.

The test-only patch shows the OP problem, but this is unexpected. Most likely, the entity schema is broken in that case or a field widget is misconfigured.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Re-upload fixed test only patch.

Test in #21 shows exact problem from OP Drupal\Core\Entity\Query\QueryException: '' not found
but it intentionally provides invalid information. A patch from #19 proposes to throw an exception in that case with explanation.

Removing tests tag as they were added.

By adding throw new UnsupportedEntityTypeDefinitionException($message); the SelectionInterface docs will have to be updated to include a @throws tag.

Added @throws PHPDoc.

Btw, I'm not sure that exception itself is suitable in that case and properly chosen.

Thanks!

Seems like this error was caused by other code changes and patch + tests are working now. Switch back to RTBC

From #17

The only way to reproduce this bug, intentionally providing at least one bundle for bundleless entity.

So it sounds to me that this might be a support request rather than a bug. Or failing that something where we can improve developer experience, which would make it a task.

Can you confirm this is the case - i.e. it can only be reproduced by doing something unexpected.

Also, I note the test is using the `default` plugin for the user entity, which is not really how this works in core, it uses the `default:user` plugin, which has different logic.

The default plugin doesn't technically exist, because Drupal\Core\Entity\Plugin\Derivative\DefaultSelectionDeriver provides a `default:{entity_type_id}` version for every entity type.

@larowlan As proposed in #3071803-25: DefaultSelection::buildEntityQuery requires entities to have bundle key., we can throw an exception in that cases. But still, it's not clear, should we do it or not, because this sounds like a very edge case scenario.

Anyway, I can update tests to use default:user handler, but it doesn't change the tests results and outcome, because the fix is applied in DefaultSelection and will affect all derivatives. So I leave them for now, before there will be a decision what to do.

@Niklan I think we should just add the exception, without the test.

A DX improvement is worth adding, but we don't need the overhead of the test.

Flagging as a task.

There is a patch without test.

I still not sure that exception type was chosen properly. This needs a review.

If the question is if UnsupportedEntityTypeDefinitionException is a good, I think so? Will see what the committer says.

Can we get a change record here - as we're throwing a new exception.

Fine to self RTBC after that.

Created CR.

I've updated a little bit CR to make it clear that it only throws an exception when entity doesn't have bundle support (entity key) and bundles passed in settings.

Committed to 10.1.x and published the change record.

Thanks folks