Problem/Motivation

When using Facets in Drupal 9 attempting to create a facet with a machine name that already existed causes a PHP error.

Steps to reproduce

  1. Install Drupal 9 and the facets and search API modules.
  2. Configure a search API source.
  3. Add a facet.
  4. Attempt to add another facet using the same machine name.
  5. Confirm "The website encountered an unexpected error. Please try again later." message (or exception output).

This is tested by \Drupal\Tests\facets\Functional\IntegrationTest::addFacetDuplicate -- that tests is failing with Drupal 9 and using the manual steps outlined above. This was discovered while working on #3248297: Default tests failing.

Example backtrace

TypeError: Return value of Drupal\Core\Entity\EntityStorageBase::getEntityClass() must be of the type string, null returned in Drupal\Core\Entity\EntityStorageBase->getEntityClass() (line 115 of /app/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php)

#0 /app/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php(454): Drupal\Core\Entity\EntityStorageBase->getEntityClass()
#1 /app/web/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php(182): Drupal\Core\Entity\EntityStorageBase->mapFromStorageRecords(Array)
#2 /app/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php(346): Drupal\Core\Config\Entity\ConfigEntityStorage->doLoadMultiple(Array)
#3 /app/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php(296): Drupal\Core\Entity\EntityStorageBase->loadMultiple(Array)
#4 [internal function]: Drupal\Core\Entity\EntityStorageBase->load('author_name', Array, Object(Drupal\Core\Form\FormState))
#5 /app/web/core/lib/Drupal/Core/Render/Element/MachineName.php(275): call_user_func(Array, 'author_name', Array, Object(Drupal\Core\Form\FormState))
#6 [internal function]: Drupal\Core\Render\Element\MachineName::validateMachineName(Array, Object(Drupal\Core\Form\FormState), Array)
#7 /app/web/core/lib/Drupal/Core/Form/FormValidator.php(282): call_user_func_array(Array, Array)
#8 /app/web/core/lib/Drupal/Core/Form/FormValidator.php(238): Drupal\Core\Form\FormValidator->doValidateForm(Array, Object(Drupal\Core\Form\FormState))
#9 /app/web/core/lib/Drupal/Core/Form/FormValidator.php(118): Drupal\Core\Form\FormValidator->doValidateForm(Array, Object(Drupal\Core\Form\FormState), 'facets_facet_fo...')
#10 /app/web/core/lib/Drupal/Core/Form/FormBuilder.php(588): Drupal\Core\Form\FormValidator->validateForm('facets_facet_fo...', Array, Object(Drupal\Core\Form\FormState))
#11 /app/web/core/lib/Drupal/Core/Form/FormBuilder.php(320): Drupal\Core\Form\FormBuilder->processForm('facets_facet_fo...', Array, Object(Drupal\Core\Form\FormState))
#12 /app/web/core/lib/Drupal/Core/Controller/FormController.php(73): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\facets\Form\FacetSettingsForm), Object(Drupal\Core\Form\FormState))
#13 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch))
#14 /app/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#15 /app/web/core/lib/Drupal/Core/Render/Renderer.php(564): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#16 /app/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#17 /app/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#18 /app/web/vendor/symfony/http-kernel/HttpKernel.php(158): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#19 /app/web/vendor/symfony/http-kernel/HttpKernel.php(80): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#20 /app/web/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#21 /app/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /app/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /app/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#24 /app/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#25 /app/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#26 /app/web/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#27 /app/web/core/lib/Drupal/Core/DrupalKernel.php(708): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /app/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#29 {main}

Proposed resolution

Determine the source of the issue and fix it (:

Remaining tasks

Determine the issue and create a patch.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#2 3248533-2.patch2.95 KBwells

Comments

wells created an issue. See original summary.

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
StatusFileSize
new 3248533-2.patch2.95 KB

This is interesting... some combination of AJAX, machine name validation, and the facet storage class used by the form is causing this issue. I think it's actually a core bug but I've exhausted all I can think of trying to determine where the issue lies.

Basically what happens is:

  1. \Drupal\facets\Form\FacetSettingsForm::buildEntityForm creates the id form field with $form['id']['#machine_name']['exists'] = [$this->facetStorage, 'load'].
  2. When a facet source is selected the form does its AJAX thing.
  3. At some point after a field is selected the the user clicks "Save" $this->facetStorage has a NULL value for baseEntityClass.

I used xdebug to walk through the code and at all points leading up to the form submission $this->facetStorage::baseEntityClass is correctly set (i.e., not NULL). The same approach is used by \Drupal\facets_summary\Entity\FacetsSummary and \Drupal\image\Entity\ImageStyle (in core) but in both of those cases the forms for adding those entities do not use AJAX.

The issue in this case can be solved in one of two ways:

  1. By changing $form['id']['#machine_name']['exists'] to [\Drupal\facets\Entity\Facet::class, 'load'] (or any similar variation).
  2. By adding a small \Drupal\facets\Form\FacetSettingsForm::load method that just does return $this->facetStorage->load($id); and changing $form['id']['#machine_name']['exists'] to [$this, 'load'].

I'm attaching a patch for the first option as it seems to be a common pattern in core and contrib anyway.

Note: I think setting this to "Needs Review" will get kicked back to "Needs Work" because of #3248297: Default tests failing. The primary concern is that \Drupal\Tests\facets\Functional\IntegrationTest::testFramework should pass for D8.9/MySQL 5.7 and D9/MySQL 5.7 tests because it runs \Drupal\Tests\facets\Functional\IntegrationTest::addFacetDuplicate.

Status: Needs review » Needs work

The last submitted patch, 2: 3248533-2.patch, failed testing. View results

wells’s picture

wells
he/him/his
Primary language English
Location Seattle, WA
commented

Well \Drupal\Tests\facets\Functional\IntegrationTest::testFramework still fails but it does get past \Drupal\Tests\facets\Functional\IntegrationTest::addFacetDuplicate, hah. It is now \Drupal\Tests\facets\Functional\BlockTestTrait::deleteBlock that is failing in D9. That is a separate issue or belongs with #3248297: Default tests failing.

Appreciate if someone can confirm and RTBC this fix so it doesn't get stuck in Needs work.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Status: Needs work » Reviewed & tested by the community

  • mkalkbrenner committed 520baf6 on 8.x-1.x authored by wells 
    Issue #3248533 by wells, mkalkbrenner: Attempting to create a facet with...
mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Status: Reviewed & tested by the community » Fixed
mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented
Version: 8.x-1.x-dev » 2.0.x-dev

Status: Fixed » Closed (fixed)

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