Contrib.social

TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in count()

Open on Drupal.org →
Created on 16 May 2023, over 2 years ago

Problem/Motivation

This happens, if the user submits the compare form without selecting any revisions.

TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in count() (line 143 of /var/www/html/p/.finpedia-grp/web/modules/contrib/diff/src/Plugin/views/field/DiffPluginBase.php)
#0 /var/www/html/p/.finpedia-grp/web/modules/contrib/diff/src/Plugin/views/field/DiffPluginBase.php(143): count(NULL)
#1 /var/www/html/p/.finpedia-grp/web/modules/contrib/diff/src/Plugin/views/field/DiffFrom.php(66): Drupal\diff\Plugin\views\field\DiffPluginBase->loadEntityFromDiffFormKey('')
#2 /var/www/html/p/.finpedia-grp/web/core/modules/views/src/Form/ViewsFormMainForm.php(183): Drupal\diff\Plugin\views\field\DiffFrom->viewsFormSubmit(Array, Object(Drupal\Core\Form\FormState))
#3 /var/www/html/p/.finpedia-grp/web/core/modules/views/src/Form/ViewsForm.php(195): Drupal\views\Form\ViewsFormMainForm->submitForm(Array, Object(Drupal\Core\Form\FormState))
#4 [internal function]: Drupal\views\Form\ViewsForm->submitForm(Array, Object(Drupal\Core\Form\FormState))
#5 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Form/FormSubmitter.php(114): call_user_func_array(Array, Array)
#6 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Form/FormSubmitter.php(52): Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object(Drupal\Core\Form\FormState))
#7 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Form/FormBuilder.php(597): Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object(Drupal\Core\Form\FormState))
#8 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Form/FormBuilder.php(325): Drupal\Core\Form\FormBuilder->processForm('views_form_revi...', Array, Object(Drupal\Core\Form\FormState))
#9 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Form/FormBuilder.php(224): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\views\Form\ViewsForm), Object(Drupal\Core\Form\FormState))
#10 /var/www/html/p/.finpedia-grp/web/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php(2257): Drupal\Core\Form\FormBuilder->getForm(Object(Drupal\views\Form\ViewsForm), Object(Drupal\views\ViewExecutable), Array)
#11 [internal function]: Drupal\views\Plugin\views\display\DisplayPluginBase->elementPreRender(Array)
#12 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php(101): call_user_func_array(Array, Array)
#13 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Render/Renderer.php(788): Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_ren...', 'exception', 'Drupal\\Core\\Ren...')
#14 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Render/Renderer.php(374): Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array)
#15 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Render/Renderer.php(446): Drupal\Core\Render\Renderer->doRender(Array)
#16 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Render/Renderer.php(204): Drupal\Core\Render\Renderer->doRender(Array, false)
#17 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(242): Drupal\Core\Render\Renderer->render(Array, false)
#18 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Render/Renderer.php(580): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}()
#19 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(243): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#20 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(132): Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch))
#21 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/EventSubscriber/MainContentViewSubscriber.php(90): Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch))
#22 [internal function]: Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#23 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(142): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#24 /var/www/html/p/.finpedia-grp/vendor/symfony/http-kernel/HttpKernel.php(174): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view')
#25 /var/www/html/p/.finpedia-grp/vendor/symfony/http-kernel/HttpKernel.php(81): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#26 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#27 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#29 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#30 /var/www/html/p/.finpedia-grp/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#31 /var/www/html/p/.finpedia-grp/web/core/lib/Drupal/Core/DrupalKernel.php(718): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#32 /var/www/html/p/.finpedia-grp/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#33 {main}

Proposed resolution

Adding a validate callback for the form to make sure, that revisions are selected.

🐛 Bug report
Status

Active

Version

1.0

Component

Code

Created by

🇩🇪Germany jurgenhaas Gottmadingen

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @jurgenhaas
  • Merge request !31Issue #3360589: TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in count() → (Open) created by jurgenhaas
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update over 2 years ago
    39 pass
  • Status changed to Needs review over 2 years ago
  • Comment over 2 years ago →
    🇩🇪Germany jurgenhaas Gottmadingen
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update over 2 years ago
    38 pass, 2 fail
  • Comment over 2 years ago →
    🇩🇪Germany jurgenhaas Gottmadingen

    Another related issue is this warning:

    Warning: Undefined array key 591307 in Drupal\diff\Controller\PluginRevisionController->buildRevisionsNavigation() (line 253 of /var/www/html/web/modules/contrib/diff/src/Controller/PluginRevisionController.php)

    which happens, if left and right revision are either identical or if the left revision is newer than the right revision. And that produces a watchdog record for EVERY revision ID, i.e. in our case hundreds of thousands. ;-)

    I've updated the MR to also validate this case and produce an applicable error message.

  • Comment over 2 years ago →
    🇨🇭Switzerland miro_dietiker Switzerland

    Uhm, for globally "every revision id", not even limited to the entity id?
    Or you have hundreds of thousands revision ids on this single entity?

  • Comment over 2 years ago →
    🇩🇪Germany jurgenhaas Gottmadingen

    Well, I wasn't able to verify that in all detail. What happened is in \Drupal\diff\Controller\PluginRevisionController::buildRevisionsNavigation this code block:

          $i = 0;
      // Find the previous revision.
      while ($left_revision_id > $revision_ids[$i]) {
        $i += 1;
      }

    From $revision_ids[$i] we got 756 watchdog entries in sequential order for the counter, while the node only has 2 revisions. TBH, I didn't debug how that came about, I only prevented that from happening again with the validation of the form in the MR.

  • Comment over 2 years ago →
    🇨🇭Switzerland miro_dietiker Switzerland

    Interesting, i think this part should be improved with an array sort / search, without loop.

  • Comment over 2 years ago →
    🇩🇪Germany jurgenhaas Gottmadingen

    Just checked the case which happened with a URL like /node/23017/revisions/view/166027/166014/visual_inline, so the diff between the rev IDs is 166027 - 166014 = 13, so not related to the 756 watchdog entries.

  • Comment over 2 years ago →
    🇩🇪Germany jurgenhaas Gottmadingen

    i think this part should be improved with an array sort / search, without loop.

    Yeah, I think that makes sense. Still, the form validation should still be present to let the user know when they selected something meaningless.

  • Comment over 2 years ago →
    🇨🇭Switzerland miro_dietiker Switzerland

    Sure this should be fixed.

    But from the way i used it, i thought from / to direction don't matter. As long as they are different.
    Are you sure about bigger / smaller check is needed?

    Maybe we can just swap the revisions when the sequence is wrong?
    Making it just work is better than telling the user to choose something different...

  • Comment over 2 years ago →
    🇩🇪Germany jurgenhaas Gottmadingen

    Are you sure about bigger / smaller check is needed?

    At the moment, yes, the order matters. I'm able to reproduce that. However, swapping them should resolve that automatically, I just wasn't sure if there were multiple places where this needed to be checked. However, the validation that they are not equal is still required, as we can't auto-correct that input error.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update over 2 years ago
    39 pass
  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 7.3 & MySQL 5.7
    last update over 2 years ago
    39 pass
  • First commit to issue fork.
  • Comment 11 months ago →
    heddn Nicaragua

    For this to be reviewed and accepted, please review https://git.drupalcode.org/project/diff#contribution-guidelines. Specifically we'll want to see some tests.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024