Comments

stijndm’s picture

StatusFileSize
new13 KB

In attachment is an updated version of the search_api_attachements module that incorporates the method used by apachesolr_attachments for indexing/parsing documents with a remote Tika library. It is not the cleanest implementation, but it works. I do recommend doing some code cleanup before committing. It also incorporates some other fixes (ie: catching exceptions when parsing) and the changes reported in #1253824: search_api callback causes undefined property errors in SearchApiAttachmentsAlterSettings.

The Solr config is the same as used by apachesolr_attachments, so you'll have to take a peak at the installation instruction of that module. The best thing to do is merge the instructions to this module.

Enjoy.

stijndm’s picture

Status: Active » Needs work
miiimooo’s picture

@stijndm: I've activated this zip and get an error:

Fatal error: Call to undefined function search_api_list_servers() in /var/www/web334/web/drupal_files/modules/search_api_attachments/search_api_attachments.admin.inc on line 51

What version of search_api was this written for? I'm using a fairly current dev version.

miiimooo’s picture

@stijndm: I've activated this zip and get an error:

Fatal error: Call to undefined function search_api_list_servers() in /var/www/web334/web/drupal_files/modules/search_api_attachments/search_api_attachments.admin.inc on line 51

What version of search_api was this written for? I'm using a fairly current dev version.

miiimooo’s picture

@stijndm: I've activated this zip and get an error:

Fatal error: Call to undefined function search_api_list_servers() in /var/www/web334/web/drupal_files/modules/search_api_attachments/search_api_attachments.admin.inc on line 51

What version of search_api was this written for? I'm using a fairly current dev version.

regilero’s picture

I've made a proper patch.
Note it miss some documentations about the way to configure a remote solr extractor, I'll add this data soon.

This patch cannot work if the patch http://drupal.org/files/search_api_solr-allow_abitrary_query-1580118-1.p... (#1580118 Allow arbitrary solr requests) is not apply on the search_api_solr module.

This patch contains :

  • some cleanup on bad self:: usage for methods which are not declared static
  • modifications from #1148162 Use real path instead of url to retrieve file to allow for private files extraction
  • an external extraction based on the existing solution on apachesolr_attachments, requesting /extract/tika on the remote server
  • A simplified local cache of the extracted content, to prevent redoing the solr extraction each time the node gets re-indexed (I use filedpot, with several files attached, I do not want to re-extract each file when I add one file). The extraction cache is emptied in hiik_file_update or hook_file_delete (so it's really a simplified version). This cache is applied to all attachments, not only the ones from distant solr.
regilero’s picture

Version: 7.x-1.0 » 7.x-1.x-dev
Status: Needs work » Needs review

Forgot to alter th status of the patch. I think this needs more reviews than work.

bblake’s picture

Patch didn't work for me, made a few changes and it now works for me.

pwolanin’s picture

Minor, but I would swap md5 for a sha2 hash function, since md5 shouldn't be used in Drupal 7+. Need to fix that in apachesolr_attachments

sutharsan’s picture

Status: Needs review » Needs work
StatusFileSize
new14.42 KB

Patch fails to apply due to too many whitespace errors. Attached patch is clean.

But now the following error occurs:

Fatal error: Call to undefined method SearchApiSolrConnection::sendHttpRequest() in /Users/erik/www/iriscorp/sites/all/modules/contrib/search_api_attachments/includes/callback_attachments_settings.inc on line 215

The method "sendHttpRequest" is not found in the search_api (7.x-1.2) or search_ap_solr (7.x-1.0-rc2).

simon georges’s picture

@Sutharsan, It's mentionned in #6 that you need the patch http://drupal.org/files/search_api_solr-allow_abitrary_query-1580118-1.p... on SearchAPI Solr to have this working.

sutharsan’s picture

Status: Needs work » Needs review

I was too quick, thanks. So this issue is depending on #1580118: Allow arbitrary solr requests -- useful for search_api_attachments. Back to needs review.

gp.mazzola’s picture

Hi, the new release of Search API Solr Search module, to RC4, breaks this patch. Could you update it to work with the new version? Dependency on the SolrPhpClient library has been dropped

andypost’s picture

Status: Needs review » Needs work
+++ b/includes/callback_attachments_settings.incundefined
@@ -89,20 +89,60 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
+    watchdog('Search API Solr Attachments', $file['uri'], NULL, WATCHDOG_NOTICE);
...
+        watchdog('Search API Solr Attachments', 'CACHED file extraction (size :size)', array(':size' => strlen($result[0]->body)), WATCHDOG_NOTICE);
...
+    watchdog('Search API Solr Attachments', 'file extraction performed (size :size)', array(':size' => strlen($ret)), WATCHDOG_NOTICE);

Too much writes to watchdog

+++ b/includes/callback_attachments_settings.incundefined
@@ -89,20 +89,60 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
-  protected function extract_simple($file) {
...
+  protected function extract_simple($filepath) {

@@ -110,9 +150,9 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
-  protected function extract_exif($file) {
+  protected function extract_exif($filepath) {

@@ -124,8 +164,7 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
-  protected function extract_tika($file) {
...
+  protected function extract_tika($file,$filepath) {

@@ -141,6 +180,51 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
+  protected function extract_solr($file,$filepath) {

Any reason to have different set of arguments?

+++ b/includes/callback_attachments_settings.incundefined
@@ -141,6 +180,51 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
+      $conditions = array('class' => 'search_api_solr_service', 'enabled' => TRUE);
+      foreach (search_api_server_load_multiple(FALSE, $conditions) as $server) {
+        $solr = $server->getSolrConnection();
+        break;
...
+      $result = $solr->sendHttpRequest('extract/tika'. $query_string, 'POST', $headers, $data, FALSE);

looks like you send data to random server

+++ b/includes/callback_attachments_settings.incundefined
@@ -141,6 +180,51 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
+      $query_string = $this->httpBuildQuery($params);

probably this depends on solr version...

torpy’s picture

I completely rewrote this to make use of the makeServlet() function in the solr connection class as hinted by drunken monkey in #1580118: Allow arbitrary solr requests -- useful for search_api_attachments. Provided you have Solr setup properly (more instructions soon), the following patch should work.

Also, rewrote the caching logic to use Drupal's Cache API instead of the custom code that was there before.

Unfortunately, this requires the following patches to be installed in order to apply cleanly:
#1148162: Use real path instead of url to retrieve file
#1307720: Newly added node files not added properly
#1895966: Handle (Ignore) Missing Files

torpy’s picture

Status: Needs work » Needs review
torpy’s picture

Now with a better README.txt explaining what changes need to happen in the solr config.

gp.mazzola’s picture

Hi @Torpy,
thank you for the new patch. I am trying to test it. Which version of the search_api_attachments module should I use?
I have installed the latest 7.x-1.x-dev and I also downloaded the other 3 patches that I have to apply first, according to your post.
However the first one

#1148162: Use real path instead of url to retrieve file

fails to apply.

Thank you,
GP

torpy’s picture

I used dev and the 'realpath' patch in comment #10 (https://drupal.org/files/search_api_attachments-realpath-1148162-10.patch). Just tried it, should apply fine.

Actually, looking back, I realised that this patch incorporates the 3 I mentioned :). So just patching dev with this one should do!

gp.mazzola’s picture

@Torpy

Thank you for your answer. I applied your patch at #17 against the last 7.x-1.x-dev of the module, and it worked nicely.
I have then tested it then and it works fine for me! Documents are passed to the remote SOLR server, extracted and cached properly.
I have not found any bug yet.
Thank you a lot for the patch! :)
I hope it can be committed in the next release!

GP

izus’s picture

Status: Needs review » Needs work

Thank you very much for the work done so far, lets try to clean this patch and have this awsome feature in.
I juste looked at the patch for the moment but i'm not reviewing it in details yet, i think it needs some cleanup before a detailed review so that we will focus on this task purpose and forget about related tasks code that are already merged :

1) it would be awsome if we make the patch work for the 7.x-1.x branch as new patches are merged there in preparation of next release in August/September and it would be awsome to have this patch in :)

2) regarding #15, we need to clean this patch deleting the code alreay merged of #1148162: Use real path instead of url to retrieve file and #1895966: Handle (Ignore) Missing Files
but about #1307720: Newly added node files not added properly, my tests didn't help understand the real issue, i just considered the issue was not accurate no more. can you please explain why it is needed for this patch to work or confirm it's not neede anymore with the last code base from 7.x-1.x branch

Thank you again :)

torpy’s picture

Sorry, been very busy recently. I'll get you an updated patch within the next few days, if not today!

torpy’s picture

Updated patch against HEAD attached.

torpy’s picture

Status: Needs work » Needs review
StatusFileSize
new15.02 KB

Issue status updated.

Also tested point 2 from comment #21 and I can't reproduce it. I've removed that line from this patch.

gp.mazzola’s picture

@torpy

Hi! I have just tried the patch at #24 against latest dev release of the module, but for me it fails to apply

izus’s picture

@gp.mazzola
please try it with 7.x-1.x branch
https://drupal.org/node/1059436/git-instructions/7.x-1.x

gp.mazzola’s picture

@izus

Thank you! Following your suggestion it worked and patched applied nicely agaist 7.x-1.x branch
I've tested then it a little adding some PDF attachment, re-indexing, deleting nodes... and it seems to work fine.
Text inside the attachment is indexed and related cache table in the DB is populated. On the other side deleting the node correctly update SOLR index and remove entry from cache table.

pounard’s picture

We're using it since some monthes now, so I guess it's RTBC.

pounard’s picture

Status: Needs review » Reviewed & tested by the community

Oups forgotten state.

izus’s picture

Assigned: Unassigned » izus

assigning to me for final review and merge

hefox’s picture

+++ b/includes/callback_attachments_settings.inc
@@ -89,16 +92,40 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
+    $file = (array)$file;

Think there's suppose to be a space here (multiple places)

+++ b/includes/callback_attachments_settings.inc
@@ -89,16 +92,40 @@ class SearchApiAttachmentsAlterSettings extends SearchApiAbstractAlterCallback {
+      $cid = 'cached_extraction_' . ':' . $file['fid'];

_' . ':' => _:' (multiple places)

torpy’s picture

Updated patch attached.

Changes in this patch:

$ diff search_api_attachments-1289222-remote_document_processing-24.patch search_api_attachments-1289222-remote_document_processing-32.patch 
64c64
< index 6698d36..ce3e18e 100644
---
> index 6698d36..0626e01 100644
84c84
< +    $file = (array)$file;
---
> +    $file = (array) $file;
90c90
< +      $cid = 'cached_extraction_' . ':' . $file['fid'];
---
> +      $cid = 'cached_extraction_:' . $file['fid'];
353c353
< index 4542eb9..e8d6f64 100644
---
> index 4542eb9..008987e 100644
367,368c367,368
< +  $file = (array)$file;
< +  $cid = 'cached_extraction_' . ':' . $file['fid'];
---
> +  $file = (array) $file;
> +  $cid = 'cached_extraction_:' . $file['fid'];
378,379c378,379
< +  $file = (array)$file;
< +  $cid = 'cached_extraction_' . ':' . $file['fid'];
---
> +  $file = (array) $file;
> +  $cid = 'cached_extraction_:' . $file['fid'];
deadbeef’s picture

Updated patch so that path of extract request servlet is relative - original doesn't work with multiple cores.

izus’s picture

Assigned: izus » Unassigned
Status: Reviewed & tested by the community » Fixed

Hi,
i tested #33 and just merged it to 7.x-1.x
Thanks all for this great patch
you rock

Status: Fixed » Closed (fixed)

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