Add Search API support

I would *definitely* be interested in a patch for that!

Yeah, that's the problem, I see it all the time. I'll give it a spin locally. Thanks.

Would you mind reworking this so that it applies to the main module? I don't think it needs to be a submodule.

This improves some of the comments.

After reviewing the functionality, this needs to be extended to use metatag_get_info() to list all meta tags provided by the module, rather than hardcoding it to only work with the useless 'keywords' field.

The last patch missed some files.

Yes, this is definitely going in the right direction, it just needs to dynamically load the meta tags via the APIs rather than hardcoding the definitions for a few tags.

Thinking on it further, is the goal to index the strings that are being manually entered, i.e. overridden values that would be found in the {metatag} table, or the final processed meta tags that would appear on the HTML page?

Sorry, I didn't mean the actual HTML tags, but the final values with all tokens processed.

The next question is: should it exclude values that are just tokens?

+++ b/metatag.module
@@ -2974,6 +2974,19 @@ function metatag_translations_delete($metatags, $context) {
 /**
+ * Implements hook_search_api_alter_callback_info().
+ */
+function metatag_search_api_alter_callback_info() {
+  return array(
+    'search_api_metatag_alter_callback' => array(
+      'name' => t('Metatag keywords'),
+      'description' => t("Adds the item's Metatag keywords to the indexed data."),
+      'class' => 'MetatagSearchAlterCallback',
+    ),
+  );
+}

This should be able to go into metatag.search_api.inc as well, since search_api implements hook_hook_info().

The question about tokens was related to the fact that just pulling the $entity->metatags[LANGCODE][METATAG]['value'] variable will give you the meta tag values before they're processed, so there could be raw tokens. To get the final meta tag strings we'd need to take a different approach, and I'm honestly not sure off-hand the best way of doing this.

Thinking on it further, I think should be handled as a custom data type via hook_search_api_data_type_info rather than a data alternation thing. It might then list all available meta tags individually, or maybe it could work as a "related field" that then makes all of the tags available?

Yes, this is definitely the kind of improvement I was looking for, thank you.

I'll test it out next week.

I don't know about the Metatag-specific code, of course, but as far as integration with the Search API is concerned, the latest patch looks quite good. Just a few things I noticed:

1. +++ b/metatag.search_api.inc
   @@ -0,0 +1,78 @@
   +    // Only works on entities.
   +    return $index->datasource() instanceof SearchApiEntityDataSourceController;
   

   You can use (bool) $index->getEntityType() instead. That's more portable than checking for a specific class.

2. +++ b/metatag.search_api.inc
   @@ -0,0 +1,78 @@
   +    $tags = metatag_get_info()['tags'];
   

   I could be mistaken, but I don't think that's valid in PHP 5.2.

3. +++ b/metatag.search_api.inc
   @@ -0,0 +1,78 @@
   +        if (isset($item->metatags[$item->language][$tag])) {
   

   This will trigger an error if $item->language isn't set.

Those should all be fixed in the attached patch. Regarding field_get_items(), I'm not completely sure – it should do what you want, but I'm not 100% certain. Please try it out.

The test looks also sensible, though I wonder why you're using the UI for creating the server and node, but not for the index. Personally, I'd create all three programmatically, but at least it should be consistent – in my opinion. But not a real "error", of course, so that's up to you.

Did you forget to include the metatag.search_api.test file? I don't see it in patch #57.

I've re-queued the last patch for review.

There were unwanted + signs in the info file ;-)

Ah! It's because we're adding new test dependencies, I forgot about this issue.

The patch in #71 will add the test dependencies, we'll commit that and then do a new patch with the rest.

And the final tests, which should work fine.

BTW I updated the test file to match the other "with [modulename]" tests, but otherwise it's 100% yours.

Forgot to add the metatag.info changes back again.

The testbot failure are being caused by #2737555: Test failures in contrib modules when other projects include forked versions of said contrib modules, so hopefully that'll be fixed soon.

BTW would you mind adding a section to the README.txt file explaining how to use the new functionality? I think that'd really help people a lot. Thanks.

The tests are passing again (thanks drumm!), so I think a little extra bit to the README.txt file and it'll be good to go.

Duh, there's already some instructions in the README.txt file.

And it's committed! Woohoo! Great work, stefan.r!