Skip files that fail indexing

Here's what I am proposing:

- Add a column to the apachesolr_attachments_files table named "failed" of type tinyint default value 0
- possibly also add columns for attempts (count of attempts to index the file) and last_attempted (timestamp)
- Modify the apachesolr_attachments_add_documents() such that failed files are excluded in the query
- Add an admin screen that lists the failed files, with options to re-index each one or many

Any thoughts or suggestions?

Here's a patch that adds the functionality described in #1.

The admin screen that lists the failed files can be accessed at admin/reports/apachesolr/attachments.

Not based on what I was seeing. My understanding is that the workflow is like this:

- Get all the noes to index
$rows = apachesolr_get_nodes_to_index('apachesolr_attachments', $cron_try);

- Index the nodes
$success = apachesolr_index_nodes($rows, 'apachesolr_attachments');

This calls the defined callbacks, one of which is apachesolr_attachments_add_documents(). This function gets all the files for each node, and then attempts to extract the text for each file, but it never checks if a text is empty. For each file, it calls:

apachesolr_attachments_get_attachment_text($file)

This function does the following:

$cached = db_fetch_array(db_query("SELECT * FROM {apachesolr_attachments_files} WHERE fid = %d", $file->fid));

  if (!is_null($cached['body']) && ($cached['sha1'] == $sha1)) {
    // No need to re-extract.
    return $cached['body'];
  }

Because there is no body, an attempt is made to extract the text from the file. If the file has things that Tika doesn't like, exceptions are thrown, and the row doesn't get updated. As a result, the same file will get parsed on each and every cron run, and will fail every time.

I would also say that just because a document didn't produce any text, that doesn't necessarily mean that an error didn't occur. IMO, there should be a distinction between not having any text and having exceptions thrown. Do you agree? One benefit to the patch I posted is that admins can identify files that had problems, fix the files, and re-index the nodes. This would certainly be helpful if you needed to replace several files, and didn't want to delete the old attachment and re-upload a new one.

What do you mean when you say more complex? I was thinking that it might be nice to add another column to display details about why the file failed; as an administrator, you only know that a file failed, but not why.

What would you like to see axed from the patch?

I'm fine with that as long as the column matches the intent - the schema change I proposed already has a "failed" column, which will be either 0 or 1. The intent behind the number of attempts column was so that administrators could potentially differentiate between a one-time server failure vs. something wrong with the file itself. You're right though - it's probably not necessary, and I would be fine dropping that field and supporting code.