Problem/Motivation

As of #3075695: Move search_index() and related functions to a service Drupal core changed the way search indexing works.
This module was never updated to match the new system.
This means that anything you try to index with core 8.8.x silently "works", but the {search_totals} table is never updated.
If you start with a clean search index on an 8.8.x core site, you'll never get any search results displayed, since the query for search results JOINs against {search_totals}, which is empty.

Calling this "critical" since the fundamental functionality of the module is now broken.

Proposed resolution

Update src/Plugin/Search/SearchExcludeNodeSearch.php to use core's new API.

In particular:

SearchExcludeNodeSearch::updateIndex() currently does this:

    $node_storage = $this->entityManager->getStorage('node');
    foreach ($node_storage->loadMultiple($nids) as $node) {
      $this->indexNode($node);
    }

Core's NodeSearch::updateIndex() now looks more like this:

    $node_storage = $this->entityTypeManager->getStorage('node');
    $words = [];
    try {
      foreach ($node_storage->loadMultiple($nids) as $node) {
        $words += $this->indexNode($node);
      }
    }
    finally {
      $this->searchIndex->updateWordWeights($words);
    }

Remaining tasks

  1. Update the search_exclude plugin (see #2), ideally in such a way that it still works with both 8.7.x core and 8.8.x (see #3).
  2. Consider adding automated tests to this module so we would have noticed this sooner.
  3. Review.
  4. RTBC.
  5. Commit.

User interface changes

None.

API changes

TBD.

Data model changes

None.

Release notes snippet

TBD.

CommentFileSizeAuthor
#3 3108145_2_3.interdiff.txt430 bytesdww
#3 3108145-3.patch762 bytesdww
#2 3108145-2.patch711 bytesdww
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww created an issue. See original summary.

dww’s picture

Assigned: dww » Unassigned
Status: Active » Needs review
FileSize
711 bytes

Initial patch that works okay in local testing. Pretty sure this will blow up spectacularly on 8.7.x core sites. So either this needs to go into a new 8.x-2.x branch, or it needs to be modified to conditionally do this or not based on what version of the core search API it discovers (e.g. checking for the existence of the $this->searchIndex protected variable or something.

But it's a start. ;) If you don't care about 8.7.x, this patch might be all you need to get your search index working again with 8.8.x core.

dww’s picture

Issue summary: View changes
FileSize
762 bytes
430 bytes

This works on both 8.8.x and 8.7.x.

dww’s picture

In Slack, @kim.pepper pointed out #3087407: Search indexing calls are inefficient as also related...

andypost’s picture

As quickfix it looks good, but strange that no language passed to indexNode()
++ To add a basic test

shderuiter’s picture

Customer site (Drupal 8.8.3) using this module had problems with certain search terms. Using patch 3 fixed the problem.

johne’s picture

#3 works great.

DanielVeza’s picture

Tested this manually + 2 positive comments about the patch solving issues.

++ To adding tests. I'll open that as a seperate ticket

  • DanielVeza committed e5a9d99 on 8.x-1.x authored by dww
    Issue #3108145 by dww, andypost, shderuiter, johne, DanielVeza:...
DanielVeza’s picture

Status: Needs review » Fixed

Thanks everyone.

DanielVeza’s picture

dww’s picture

Great, thanks! Following #3149523 now.

Status: Fixed » Closed (fixed)

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

PapaGrande’s picture

@DanielVeza, could we get another release that includes this fix? Thanks.