Leverage extraction support of Search API Solr integration, fix incompatibilities for various solr service providers incl. Acquia Search

ok
thanks for this :)

Attaching a patch based on @cristiroma's work (so all credits to him) that fix this from the module's side by enforcing the adapter type and fix the name of the file referenced.

IMHO basename($file) should go into this module, but the enforcement of the adapter should be handled in the backend code, since this use case could be specific to Acquia Cloud (I have not tested with standalone solr installations).

Regardless I'll attach the full patch here, so we can take it from there.

hi,

i tested and here is what i noticed

1. +++ b/src/Plugin/SearchApiAttachmentsTextExtractor/SolrExtractor.php
   @@ -39,6 +39,7 @@ class SolrExtractor extends TextExtractorPluginBase {
   +    $client->setAdapter('Solarium\Core\Client\Adapter\Http');
   

   somehow this doesn't work for me and the value i have with ->getAdapter is Solarium\Core\Client\Adapter\Curl

2. +++ b/src/Plugin/SearchApiAttachmentsTextExtractor/SolrExtractor.php
   @@ -58,7 +59,7 @@ class SolrExtractor extends TextExtractorPluginBase {
   +    $xml_data = $array_data[basename($filepath)];
   

   i tested and the Json::decode($json_data) has the $filepath and so basename($filepath) is not found

So I found that this is a combination of issues:

• A known problem with Solarium: https://github.com/solariumphp/solarium/pull/413
• Some incompatibilities with search_api_attachments with the latest Search API/Search API Solr beta versions.

To fix, I tried this combination:

• Search API 8.x-1.0-beta4
• Search API Solr 8.x-1.0-beta1
• Acquia Connector 8.x-1.7
• the attached patch for search_api_attachments-8.x-1.x
• the patch from #2831801 (because I used search_api_solr-8.x-beta1.. note this patch is already in search_api_solr-8.x-1.x-dev)
• Patched solarium with the pull request https://github.com/solariumphp/solarium/pull/413 (Note: you can download the github pull request as a patch by appending .patch to the pull request's URL)
• Since I'm using Acquia Search's Tika, I did need to change the "Tika path" in the Search API Attachments configuration form to "extract/tika" (you do this at /admin/config/search/search_api_attachments)

Screenshots are great. Here's one!

Please review.

I have had 3 different customers/sites try out this patch (along with the patched Solarium as mentioned above) and it fixed the issue for them.

Because of that I am going to mark this as RTBC. We just need some noise on the Solarium PR so that it gets fixed there too!

Please drop a line here when the solarium PR is merged, so that we can merge this issue as well.
Big Thank to all contributors.

1. +++ b/src/Plugin/search_api_attachments/SolrExtractor.php
   @@ -70,9 +70,9 @@ class SolrExtractor extends TextExtractorPluginBase {
   -    $client = $backend->getSolrConnection();
   +    $client = $backend->getSolrConnector();
   ...
   -    $query = $client->createExtract();
   +    $query = $client->getExtractQuery();
   

   This part is covered by #2862528: Solr Extractor causes Error with wrong method call and is not related to Acquia Search in any way.

2. +++ b/src/Plugin/search_api_attachments/SolrExtractor.php
   @@ -91,7 +91,7 @@ class SolrExtractor extends TextExtractorPluginBase {
   -    $xml_data = $array_data[$filepath];
   +    $xml_data = $array_data[basename($filepath)];
   

   What I can see from the new tests included in search_api solr itself, that basepath() operation is not required for a local Solr server. Maybe I didn't recognize a detailed explanation here, but I don't understand why it should be done. Just like @izus in #4

+++ b/src/Plugin/SearchApiAttachmentsTextExtractor/SolrExtractor.php
@@ -39,6 +39,7 @@ class SolrExtractor extends TextExtractorPluginBase {
+    $client->setAdapter('Solarium\Core\Client\Adapter\Http');

somehow this doesn't work for me and the value i have with ->getAdapter is Solarium\Core\Client\Adapter\Curl

Meanwhile this is handled within the Solr Connectors. The default connector I wrote uses curl while the acquia connector I wrote uses "http".

After reviewing SolrExtractor::extract() completely, I think we should do something else here. Most of the code should be moved to search_api_solr, some parts to the backend, some parts to the connector. Then Acquia is able to fix their stuff in their own connector. And solr_attachments can avoid some configuration options which aren't required anymore. For examplt the 'solr_tika_path' is absolutely specific for the connector plugin and shouldn't be configurable.

OK, I provided the required methods by #2870333: Support connector specific tika based extraction.

The attached patch now simplifies the extractor implementation here.
And it enables Acquia to overwrite SolrConnectorPluginBase::getContentFromExtractResult() in their own connector.

Makes sense to me.

Thanks all.
This is now merged and will be part of today's release ( first beta)

Note that the Solarium fix is progressing: https://github.com/solariumphp/solarium/pull/484 was merged into the develop branch, hopefully will be included in new release soon.

@mkalkbrenner can you post the versions of each module you are using that makes it work? We're not seeing it functioning on Acquia Search and suspect some of the code "pushed" from #5 to other modules didn't make it.

I've confirmed #5 will at least get you through the /admin/config/search/search_api_attachments configuration screen with the following versions of modules / patches installed:

search_api 8.x-1.0-beta4
search_api_solr 8.x-1.0-beta1
acquia_connector 8.x-1.7
search_api_attachments 8.x-1.0-alpha5 (with the following patches applied)

1. Search Api Attachments Patch: https://www.drupal.org/files/issues/drupal_8_acquia_search-2696901-5.patch
2. Solarium Patch: https://patch-diff.githubusercontent.com/raw/solariumphp/solarium/pull/4...

@greggmarshall I can't test with Acquia Search. All that I can tell is that our tests on travis.org cover the extraction as well and they're always passing: https://travis-ci.org/mkalkbrenner/search_api_solr
Others second that it's working on their local Solr installations.

But as @janusman is pointing out here, there're some steps required to get things to work on Acquia Search. Therefor I suggest to open an issue at https://www.drupal.org/project/acquia_connector

The good news are that any fixes required could be done by themselves in their connector.

Thank you both. @jaydubb181 from our team found a combination of modules and patches and posted them in #18 for future Google visitors.

But as @janusman is pointing out here, there're some steps required to get things to work on Acquia Search.

Let me try to explain the reasoning...

IIUC, the problem will show up when using any (not just Acquia's) backend via the "Solr Extractor" (e.g. Extract-via-HTTP-request) method, because of these reasons:

• Solarium has a bug that does not send the proper multipart headers (fixed in Solarium's dev branch, in pull 484) which will break (IIUC) with any Tika-via-HTTP attempts
• If you patch the stable Solarium with the above PR, the request now succeeds, but the returned data is not being found by search_api_attachments within the returned XML...
• The reason is that the mentioned Solarium change includes a change to now use basename($filename) instead of the complete $filename (which includes the complete path to the local file). Therefore, if you patch Solarium without then patching search_api_attachments, even when the Tika extraction worked and returned data, the search_api_attachments module won't find the proper element within the returned XML and search_api_attachments declares failure to extract.

So, either we try to:

1. Submit a PR to fix Solarium to not use basename($file) in the dev branch, so that things won't break in the future: https://github.com/solariumphp/solarium/pull/518
2. Or, we embrace that Solarium change, and add the basename($file) to search_api_attachments as mentioned in https://www.drupal.org/node/2870333#comment-12060394

Of course, if my assumption that all "Solr Extractor" requests are broken without the Solarium patch is wrong... then I understand if this has a lower urgency. I don't see how a Tika-via-http-request method would ever work given the brokenness of the multipart handling in Solarium fixed in https://github.com/solariumphp/solarium/pull/484 .

If, however, my assumption is true, then maybe we should document that patching to Solarium is needed? Or should the composer.json file included with this module specify patching from https://github.com/solariumphp/solarium/pull/484 ?

I did more digging and found that:

• The Travis testsfor search_api_solr only use the "Curl" Solarium adapter during text extraction
• Acquia Search's connector uses the Solarium "Http" adapter insted of the above Solarium "Curl" adapter.
• Since Search API Solr only tests extraction using the Curl adapter, it doesn't catch the Solarium bug when using the Http adapter for Solr extraction method.
• Thus, what's actually needed (IMO) is a different Solarium patch: https://github.com/solariumphp/solarium/pull/519 that is compatible with Acquia. (The earlier-suggested Solarium pull request 484 did fix the Http adapter but it broke the "Curl" adapter in the process.)

@janusman,

It's unclear what the current state of this is for the 1.0-beta3 version. I've patched Solarium with pull/519, but it none of the search_api_attachments patches apply.

Have they been included in beta3 releases?

Can we please reopen this issue? This patch doesn't apply to 8.6 and needs a reroll

reopened the issue
Contributions/patches/documentation are welcome
community rocks !

I'm not sure why this needs re-opening? The patches in this thread are already in the latest release. All I did was install this module (beta11 at this point in time) and then the solarium patch listed in #23: https://patch-diff.githubusercontent.com/raw/solariumphp/solarium/pull/5...

Confirmed working on Acquia solr.

@manningpete: Just guessing, but if I take literally what you have said: "This patch doesn't apply to 8.6" ... could it be that you tried to apply the patch to Drupal Core 8.6 instead of the module code of search_api_attachments?

#27 is clear about the solarium patch
idk if something can be done for this in search_api_solr
but in search_api_attachments i'm sure we can do nothing more for it
Thanks all for your contributions