Proposed resolution

- Convert hook_search_info() to use plugins instead
- Define base classes and interfaces
- Remove outdated and non-working parts of the core Search functionality
- Make the core Search functionality less node-module-specific, by moving node-specific functionality into the node module.

Remaining tasks

The patch exists and has been extensively reviewed.

User interface changes

Minor: in the search admin page, the plugin titles and machine names are listed as setting options as opposed to module names.

API changes

hook_search_info is replaced by annotation-based plugin discovery using class \Drupal\search\Annotation\SearchPlugin

A new interface \Drupal\search\Plugin\SearchInterface and a base class that implements all the method as stubs \Drupal\search\Plugin\SearchPluginBase are defined. A separate interface for plugins that use the Search module's indexing is also defined: \Drupal\search\Plugin\SearchIndexingInterface (user search does not use core indexing, so the indexing-specific methods were separated out).

All the search module hooks except hook_search_preprocess() are converted to be methods on the plugins that are invoked as appropriate

The search plugins are accessed via \Drupal\search\SearchPluginManager which is available as a service 'plugin.manager.search', e.g. Drupal::service('plugin.manager.search')

Since these implementations are plugins, a single module can now provide multiple search plugins - in contrast to Drupal <= 7 where there was a limit of one search implementation per module

Node and User searches are converted to plugins

Table {search_node_links} and the related functionality are removed since it was causing extra indexing of content and the code addling to this table was broken. The consensus amongst the search module maintainers was that it was a rare edge case. See comment #133 for more information.

The SearchExpression class, which was added to Drupal 8, is not needed any more, so it is dropped. It had some functionality for injecting/extracting advanced search parameters into search keyword expressions, but this is done with query arguments now, so it is not needed. Correspondingly, since each search plugin basically sets up its own query, the setOption() method on SearchQuery is not needed, so it is also removed. Also, the function search_expression_insert()/extract() are removed.

Functionality specific to the Node module is moved from Search module to Node module or replaced by more generic functionality. For instance, search_touch_node() is replaced by search_mark_for_reindex() and the node module has a specific function node_reindex_node_search() that calls this.

In Drupal 6, hook_update_index() was called on all implementing modules. In Drupal 7, this was silently dropped to only being called on active search modules (causing some bugs). In Drupal 8, this is now a method on the SearchIndexingInterface, and thus will explicitly only be called on search plugins that use indexing.

@Berdir, would you be able to explain us how the change in the discovery would simplify this patch? I'm not 100% following what you are doing there. Also @swentel pointed me to the Routing of field_ui and we could most probably use and copy most of that code to replace the hook_menu magic that we are currently doing.

We should expose a service and give the searchPageManager as injection so that the routing can be created based on all the plugins. This should be a follow-up patch after this patch gets in.

I assumed every plugin annotation class should have "Plugin" in its name? Not sure if there is a standard here?

re: injecting the dependencies, do you have an example of a plugin doing this? So I could (?) replace the node_load() with an injected entity_manager? Not sure about how to replace the other procedural calls.

@dawehner - also, at what point is the property translated? The one above is being used in hook_menu (and the router in the future) and will get translated there, so I'm not sure if there is a risk of double translation or insertion into the DB of the pre-translated title?

@dawehner - also, at what point is the property translated? The one above is being used in hook_menu (and the router in the future) and will get translated there, so I'm not sure if there is a risk of double translation or insertion into the
DB of the pre-translated title?

Core uses a custom annotation class (Translation) in order to make it easy for l10n.drupal.org and the tools to scan for translatable tools. This is done all over the place already, so let's keep consistent.

Looking at the search module code, it seems the optional hook_update_index() was also coupled to hook_search_info(). hook_search_status() was also somewhat coupled, so it might make sense to make that code part of this plugin as well.

@tim.plunkett - I was unsure about using @Translation. Does that mean the annotation is translated before it's used? If so, that's potentially wrong since the text will be translated again as part of the menu system.

@tim.plunkett - thanks - the difference was only a couple lines to set a default.

search_get_info (or some renamed version) is still useful since it gives us the list filtered by the ones that are set active. I don't want to have to write that every time.

As we discussed in IRC, changing the keying to be the plugin ID instead of the module would be a good goal here, so then a single module could expose multiple plugins. I guess that will require a small update function to change the list of active things.

pwolanin asked me to take a look at this... It's a big patch, but I just wanted to say that in general, I think the approach is good, and the general outline of the plugin, base, and interface classes.

Obviously there are a few hopefully minor issues to sort out, but I think this is a good idea in general.

It turns out that (we are pretty sure anyway) HEAD is broken and the fact that those two tests pass in HEAD (without the patch) is due to some faulty tests.

Here's the logic:
- When Core runs search_cron(), it invokes hook_update_index() on all modules that are enabled for search. Meaning, generically, node and user modules.
- So normally in Core, the two implementations comment_update_index() and statistics_update_index() are never run.
- These two functions are what set up the normalizations for the comment/statistics search rankings (the ones that are failing in the tests on the latest patch).
- However, this basic bug in Core Search is masked by the fact that all of the existing tests were doing an invokeAll on hook_update_index(), which invoked the comment/statistics hook implementations *during tests* even though they would never be invoked in a real world situation.

So... Basically the tests for search ranking were passing in Core because of this invocation of the comment/statistics hook implementations, which would never happen in real life. And the fact that the same tests fail on the last patch is because that test is not written wrong, and it caught an actual Core bug.

Sigh.

If we want comment and statistics search rankings to work, a different mechanism for normalizing the search rankings during cron runs needs to be found. hook_update_index() is not the right answer. hook_node_update_index() might be.

This patch cleans up the code as suggested and removes the 2 failing rankings from the test since the underlying functionality is actually broken in core as jhodgdon says. Probably are broken in 7.x too since release.

This patch also fixes the search config form for node module so it actually sets the ranking variables.

@dawehner - the potential of having the account object was one of the reasons for doing

$request->attributes->all()

e.g. if you want to do some kind of personalized search.

re: the entity controller, we are using at some points one or both the storage controller and the render controller, so it seemed the current constructor was easier for handling both.

+++ b/core/modules/search/lib/Drupal/search/Tests/SearchRankingTest.phpundefined@@ -91,6 +91,11 @@ function testRankings() {+ // @todo - comments and views are removed from the array since they are+ // broken in core. Those modules expected hook_update_index() to be called+ // even though it was only called on modules that implemented a search type.+ array_pop($node_ranks);... foreach ($node_ranks as $node_rank) {

This is a little cryptic so getting this in core this way is quite weird. However, I understand it's a complicated situation as explained by jhogdon and it'll get fixed in a separate issue. As soon as the test is green this can be marked as RTBC and I will.

@tim.plunkett - I think it's both much less readable and very likely (while minor) less performant.

This patch has the manager provide an array of active plugins which makes the module code cleaner, and makes some other minor changes with an eye towards a possible follow-up to maintain a list of active plugin IDs, instead of allowing only one plugin per module. We need a follow-up in any case in handle the routing conversion.

Per discussion with Tim.plunkett, we shouldn't get this in unless it's keying plugins by plugin ID instead of module. Currently a module can only provide one plugin, which is rather counter to the whole concept.

This patch shifts the settings, keying, etc to be by ID to resolve that bug.

It also adds use of AccessibleInterface and (previously missing) PluginInspectionInterface

These @params should have types and must have descriptions. The @return description is a bit obtuse as well, and the first line should start with Sets instead of Set, and should probably say "parameters" instead of "params". (getSearchParams should have the same change?)

c) It would be nice to have a description somewhere in all of this of what a "param" is vs. an "attribute" of a search. A good place would be in the doc header for SearchInterface maybe? I have no idea when reading this class's docs what these two terms mean in terms of how they affect searches, how they'd be set up, etc.

The type should be "bool" here not "boolean". And in the description, it should say something like "TRUE if the search settings are valid, and FALSE if it isn't". Note all caps on TRUE/FALSE.

e) SearchInterface has some more methods whose docs start with the wrong verb person: execute(), buildResults(), etc etc. Tests/SearchConfigSettingsFormTest.php does too. And Search/SearchExtraTypeSearch.php.

f) The word wrapping is not good in updateIndex() docs. Minor problem, maybe don't bother to fix.

g) search_reindex() in search.module has a typo where it says $plugin_is instead of $plugin_id in the docs header:

+ * specified, $plugin_is must also be given. Omit both $sid and $plugin_id to clear

h) search_cron() docs still refer to hook_update_index():

+ * Fires hook_update_index() in all plugins and cleans up dirty words.

i) in search_index():

- * @param $module- * The machine-readable name of the module that this item comes from (a- * module that implements hook_search_info()).+ * @param $type+ * The entity type or other machine-readable type of this item.

Is this accurate? Wouldn't it be the plugin ID? If not, I think it needs more information on what $type would be, because if I had this question... probably others would too?

j) Somewhere near the end of search.module -- probably in a @defgroup:

RE #115 - really? I think that "type" referred to a search type/module, didn't it? Which was "node"? Because when doing a search of a given "type" in the old system, I think the query had a condition on the type column... let's see....

@jhodgdon - yes, it used module before, but it's unclear since there was a 1:1 mapping of module name to entity type to search "type"

Of course, this code predates a clear concept of entity types even!

My initial thought when making hte conversion to key by plugin ID was to use the plugin ID as the "type", and then I changed my mind to assume it was entity type. I can re-roll it the other way if you think that makes more sense in terms of the search module API.

This is definitely meant to be search type and not entity type, because there could be several different modules providing search plugins/types for searching content of entity type node.

Let me see if I can explain what I mean. For example, let's say I have search plugin A that indexes nodes, and search plugin B that also indexes nodes. Both will use the node ID as "sid". Both call search_index() to index the node, but each one indexes different text (for instance, different fields etc.).

The search module needs to then store each plugin's presumably different interpretation of the text for that node separately, so it cannot use "node" as the "type" field, it needs to use plugin A or B's machine name.

Does that make sense? Each search type/module/plugin needs to have a scheme for having its own search IDs (sid), and then when it passes this to search_index(), the Search module needs to store that information so that when a search result has a match for that "sid" value, the plugin can match that to the particular content item of whatever it's indexing.

/** * Sets the keywords, parameters, and attributes to be used by execute(). * * The Search module calls this method to pass in information entered by * the user when a search is requested on this search type. * * @param string $keywords * The keywords to use in a search.... */ public function setSearch($keywords, array $parameters, array $attributes);

c) I think we should pare down SearchInterface to the bare minimum methods that the Search module actually requires plugins to have. Which I think are just:
- setSearch
- isSearchExecutable
- buildResults
right? The rest should just be on SearchPluginBase but not on SearchInterface because they aren't needed by all search plugins, so they shouldn't have to implement them.

pwolanin thinks we need a few more but I think there are too many that aren't really needed for every search plugin. Let's cut them down as much as possible.

d) And let's add to the docs for isSearchExecutable() and buildResults() to say when the Search module calls them.

e) And then somewhere, we also need to document the correspondence between the index and the search query extender.

search_index() should say any module using the core search index needs to pass in a unique ID there for $type (and suggest using the plugin ID). And then the SearchQuery extender needs to document that in searchExpression(), you pass in the same $type for the second argument.

I think of the index/retrieval part of the Search module as a provider of indexing and retrieval services for other modules that provide searches.... Here's a description of how I think it should work:

The core Search module's Search Indexer (SI) and Search Query Extender (SQE) provide a service that an independent search provider module (SPM) (provider of a plugin that defines a search type) can use to index and search whatever it wants to define as "content":
- The SPM needs a way to identify a piece of content with an ID that it understands, independent of other SPMs.
- The SPM needs a way to add content to the SI, independent of other SPMs, with the ID attached to it.
- The SPM needs a way to tell the SI that content has been invalidated or needs to be reindexed, independent of other SPMs, by providing the ID to the SI.
- The SPM needs a way to use the SQE to search only its own content, independent of other SPMs, and have each result identified by its ID.
- No SPM should interfere with or be allowed to search any other SPM's data.
- Not all SPMs will use the SI/SQE system to do their searching (for instance, the User SPM does not use the SI/SQE system, but instead directly queries the user table).
- If we could manage to define the SI/SQE system using the Dependency Injection Container, then another module like Solr could define itself as a provider of SI/SQE services as well, and you could have the choice of turning on Solr and overriding the core SI/SQE provider. This is probably beyond the scope of what we can do for Drupal 8 though... but it would be cool if it could happen.

So, under this view, the Node module happens to use the node ID as its unique identifier of content, and in the example of #124, what is really happening here is that the node module as a SPM is saying, when a node is updated, tell the SI that the content owned by the node SPM with a particular ID needs to be reindexed. If another SPM happens also to include nodes as content, then that SPM needs to also detect that a node has been updated and invalidate whatever ID it happens to have for that particular node. Each SPM has to take responsibility for the content it gives to the SI, and the node SPM shouldn't be trying to do anything to other SPM's data.

At least... If the core Search module is going to be usable as a generic search index/retrieval framework, that is how I see it working. That is basically how it worked in Drupal 7 as well.

RE #130 - I don't see the problem? Whatever the node search plugin does during indexing (whatever "type" it passes to search_index(), that is) is the same thing that needs to be put in as the 'type' value in that query in #124. And definitely, the ID that node search plugin uses for the "sid" when indexing content should be the node ID.

So... What's happening (this references D7's behavior) is that search_index() currently has a heck of a lot of code that is specific to D5-era nodes and books in it. What it does is that if someone has a link like:

<a href="http://example.com/node/12345">pizza</a>

in node/98776 that is being indexed (or, startlingly, also links like book/12334!?!), then during the indexing of node 98766:
- "pizza" is added to table {search_node_links}
- node/12345 is marked for reindexing.

And then when node/12345 is reindexed, function search_node_update_index(), which node.module invokes when preparing content for indexing, adds "pizza" as a link (with no HREF attribute) to the end of the content of node/12345.

So basically if you then search for "pizza", you will get node 98766 because the word pizza is in it, and also node 12345 because 98766 linked to it using link title "pizza".

A lot of messy code that has outdated cruft in it (like the book/* links?!?), just to accomplish that the 2nd node comes up in searches for pizza, when most likely that second node had the word pizza in its title anyway.

Furthermore, search_index() will do this not only when node.module is indexing nodes, but also when other modules' search providers are indexing their own content, whatever that might be. *!?!?

So, I think pwolanin and I both agree that:
- no one probably knows about this functionality
- no one probably cares about it or is relying on it
- it is not a great idea that search.module is so tied up in node-specific stuff
- it shouldn't be mixing up various search modules's stuff, ever.
- if we really wanted to make this general, search_index() could make a list of links it finds in content it is asked to index, and call a method on the search plugin that requested the search to give it the information and let it deal with it, but this would be kind of complicated. Or node.module could scan through content as it calls search_index(), find links, and do this itself (no reason it needs to be inside search_index()).

===> But really, we probably don't need this functionality at all -- it's an unnecessary complication and an edge case, so let's just drop it. This will greatly simplify search_index() -- it took pwolanin and me about 20 minutes or more in IRC just to figure out what the code was doing anyway. And we could get rid of the search_node_links() table.

Oh. We also think that search.module should have a function called something like
search_mark_for_reindex($type, $sid)
and probably one called
search_remove_from_index($type, $sid)
if it doesn't already.

Oh, one more thing: I also think that if possible, all of the search-related stuff in both node and user modules should be moved into separate modules. Maybe it's not so necessary for user.module, because I think all the search stuff (after this patch) will be in the plugin class anyway. But for node.module, there will need to be some hooks (like when a node is updated, it needs to be marked for reindex I think maybe?)... and it would be good if all the search-related stuff was in its own module. Maybe?

I'm not sure splitting out search-related code is worth a separate module given that it's only a small amount of code. This patch just moves certain hook implementations from search module to node module, as well as killing all the link-scoring code and removing that table an using the plugin ID as the search index type.

- Is 'user' the node author? Should we in that case call it "author"?
- In 'term'... isn't there a 'vid' needed here? Maybe not... whoah. I guess that the taxonomy_index table does not use 'vid'. Hm. OK. Scratch this comment.
- Should we document what these array elements are, and maybe what the keys/values represent? The docs for $advanced are not illuminating.

c) In the NodeSearch::execute() method, I don't understand why all the "f" parameters (and can't you document them better than calling them "f" things?) are first concatenated into one string and then parsed out. At a minimum, this needs some explanation in a code comment, because it seems to me (naively?) like we start with an array of key => value, and then have to parse out the keys... why not just not concatenate? But I'm probably missing something. I guess I am not sure what format $parameters['f'] would be in maybe? Anyway the code is a bit obtuse and I can't review it because I don't understand it.

d) I would think we need to take care of the @todos that reference variable_get() (node rankings) before committing this?

Way to decouple search.module from being so node.module centric!! However, shouldn't this be using the node.module function node_reindex_node_search() instead, which verifies search.module is enabled (which is what the actual node_comment_update() implementations are doing)?

f) The first line docs for SearchIndexingInterface do not actually say what it is for. How about:

Defines an optional interface for SearchPlugin objects that use the core Search index.

g) Generally/minor: a lot of the docs are not word wrapped to 80 character lines.

i) I think comments can now be attached to other entities besides nodes. If this is correct, the hooks in node.module that respond to comment CRUD need to check whether the comment is actually attached to a node and only update the node search index if that is the case.

re: d) we just moved the code in here - I'd rather leave it for a follow-up
re: e) we could probably kill it, but it seems advertised as a search.module feature
re: f): HEAD to HEAD updates are not supported at this point in the cycle.
re: g) maybe due to being copied from old code? Point out any obvious places to wrap

Re-roll for minor conflict in user.module and to respond to comments above.

re: i) the Comment entity only seems to be able to be associated with nodes.

Added comments to NodeSearch re the f query string, but here's more explanation:

$_GET['f'] is an array that looks like this in the URL:

?f[]=type:page&f[]=term:27&f[]=term:13&f[]=langcode:en

And this in code:

$f= array('type:page', 'term:27', 'term:13', 'langcode:en');

so imploding that gives a string like we used to have in the advanced search keywords:

'type:page term:27 term:13 langcode:en'

doing implode() plus preg_match_all() seemed simpler and more efficient that a foreach loop over all of $f for each possible "option" (field name), especially since we want to group them for the OR condition.

Agreed, we cannot deal with comments on non-node entities yet. Just something to be aware of if that patch lands before this one; alternatively, that other issue will need to be updating search-related hooks if comments can be added to non-node entities.

Anyway... It looks like we have a working patch. I'm going to have to make some time to give it a full review.

This is semantically wrong. It should just be calling function search_index(), which is not a hook, just an ordinary function. Why was this done? Either it needs a comment explaining why, or it should just be replaced by an ordinary function call to search_index(). This method should not be called if search.module is not enabled, and there are other things that will fail miserably in the updateIndex() and other methods if it isn't enabled, like queries against tables that will not exist...

g) In NodeSearch::execute(), there is an implicit assumption that everything that comes through in the facets in the URL is a known type of advanced search. I added a comment there to that effect, but it seems like that assumption should be checked in the code?

h) Does NodeSearch::searchFormSubmit() need to be sanitizing the values it is reading directly from $form_state?

i) I noticed that the search tables search_dataset and search_index have langcode fields. Shouldn't search_mark_for_reindex() also take an optional $langcode argument? search_reindex() does... I guess it si not such a big deal because at worst it will be inefficient if several items are marked for reindexing, but still.

j) Along the same vein, I'm wondering if NodeSearch::updateIndex() should be doing some kind of DISTINCT or something because if a node has multiple languages, its nid will show up multiple times in {search_dataset} marked for reindexing, so I think we will be reindexing the node multiple times? I'm not sure about this.

k) We have functions search_expression_insert()/extract() and a SearchExpression class with these methods, and then the NodeSearch plugin goes ahead and does its own parsing for the search facets... Wow, do we really need to have all of that? I think we should get rid of the search_expression* functions (they are just wrappers on the methods), and I think that the NodeSearch plugin should use the SearchExpression class to add/extract facets. Either that or we should get rid of the SearchExpression's insert/extract methods entirely.

l) I noticed that on other plugin annotations, items that are translatables have

RE #147 - well if you're going to do that, you should do it all over all kinds of methods in NodeSearch. Because many of them are assuming that certain database tables exist, so you'd get PDO exceptions if you tried to run those methods without search.module being enabled.

So we would either need to put that all over NodeSearch plugin, or I think it's just safe to call that function in the NodeIndex method.

I misunderstood. I was responding to the question about that particular usage of invokeHook, and missed the forest for the trees.
Since search plugins are provided by the search module, you theoretically should be able to rely on the module being enabled.

But the best course of action would be to turn that function into a method on a service.

That way if anyone wanted to reuse the search plugins without search.module in contrib, they'd just have to provide their own service with a null implementation or something.

Moving search_index() to a service should be opened as follow-up and referenced by issue number in an @todo

Yeah, a service for indexing would be the holy grail that would allow Solr etc. to just take over on searching. Not happening on this issue. :) A @todo comment in the code sounds like a good idea.

So I think the answer to #145 (f) is that all that invokehook nonsense should just be replaced by a simple function call to search_index(). Status to needs work accordingly.

The other comments I had on #145 (G-L)... I am not sure about all of them and am certainly willing to be convinced (which is why I did not set that patch to "needs work"). But if the answer is "convince Jennifer" then maybe some comments ought to be added to the code to clarify those questions?

re: #145::h searchFormSubmit() is simply processing the values and putting them into the redirect URL. So, there's no reason to try to sanitize, since the user can just as well type anything they like into the URL.

Adding a DISTINCT per #145::j

re: #145::l, the title is not actually translated - it's indeed a string since it's passed through the menu system which later translates it. This may change later depending on local task plugin conversion.

I think that the only comment from #145 that still hasn't been addressed is:

g) In NodeSearch::execute(), there is an implicit assumption that everything that comes through in the facets in the URL is a known type of advanced search. I added a comment there to that effect, but it seems like that assumption should be checked in the code?

// Each searchable node that we created contains values in the body field++<<<<<<< HEAD + // in one or more languages. + $search_result = node_search_execute($node->body->value);++=======+ // in one or more languages. Let's pick the last language variant from the+ // body array and execute a search using that as a search keyword.+ $body_language_variant = end($node->body);+ $plugin->setSearch($body_language_variant[0]['value'], array(), array());+ // Do the search and assert the results.+ $search_result = $plugin->execute();++>>>>>>> applied patch

Actually wondering if we didn't lose test coverage there. Restored the previous functionality using the new API there, see interdiff.

Also removed a few now unecessary getBCEntity() and calls that relied on the BC decorator, let's see if there's more.

Dries, xjm, and I quickly glanced over this patch, and at a high level, it makes a lot of sense to modernize the Search API to OO. However, it looks like this patch makes more changes than just moving code from a pseudohooks to interface methods. For example, dropping the {search_node_links} table, and changing search_touch_node() to node_reindex_node_search(). Can the issue summary be updated with the complete set of API changes being made here? We'll need that for a change notice anyway.

It looks like upgrade path tests are missing. Because _search_update_8000() is changed to something non-trivial it could really use an upgrade path test. Other than the missing upgrade path tests, this patch looks RTBC. I understand it's no fun to keep re-rolling this patch -- I will add it to the top of my TODO list once the upgrade path tests have been added. I think we're really close -- thank you for your efforts to date.

Ok, well I guess that was a useful exercise, since we missed the need to call $config->save() in the update function helper and some wonkiness around config merging.

Includes a little tweak also to avoid an array_flip() warning, though I'm 99.9% sure that's only a HEAD-to-HEAD problem. Also tweaks the config yml file so the default looks the same as what's written out when you submit the form.

I (gasp) edited the patch file directly to change those two words to the correct spelling (no other changes, verified with diff but I'm not making an interdiff file) and re-uploaded. Assuming it turns green, I think this is good to go.

Tried to commit this but I noticed some spelling mistakes (would have fixed during commit) but I also noticed that the removal of the search_node_links means that the views.view.backlinks.yml will be impossible to do in core since this linked to the said table in Drupal 7. Patch attached removes it. Talked it over with @xjm and she agreed that if this issue removes this table then it needs to do something with the backlinks view.

If the search_node_links table was needed, how come there wasn't a test that failed when we removed it?

The code that was finding those links was deeply flawed and not accurate in Drupal 7 anyway... and the links table, even if it was working, was not really doing anything for searches, as discussed previously, except making the whole process less efficient.

That said, if someone wants to make a table of link A goes to link B (which is what that table was purporting to do), you could take the code we took out of the search indexing code, fix it to make it actually work correctly, and make a hook_cron to generate the table of links.

What spelling mistakes did you notice? We've all been through the code multiple times and didn't notice them... Happy to fix them but editing your own writing or writing you've looked at multiple times, you don't notice spelling problems.

Tried to commit this but I noticed some spelling mistakes (would have fixed during commit) but I also noticed that the removal of the search_node_links means that the views.view.backlinks.yml will be impossible to do in core since this linked to the said table in Drupal 7. Patch attached removes it. Talked it over with @xjm and she agreed that if this issue removes this table then it needs to do something with the backlinks view.

This view does not have tests at the moment is because the search integration into views wasn't part of the initial commit of views, and it haven't made it in yet. The approach is fine,

So... I woke up at 5 AM this morning worried there was perhaps one more thing we needed to do in this patch -- but it was already committed.... We may need a quick follow-up on the update function.

Here's what I was worried about.

I think in the search_index and search_dataset tables in D7 and previous, we were indexing nodes with the type column set to "node" [type was the module name], and in D8 we are using the plugin name, which is now "node_search".

So I think we need to in the update_8000 function also do a database query that translates the type column in those tables from the module name to the plugin ID?

It seems like _search_update_8000_modules_mapto_plugins() needs a few lines inside the