If deleteByQuery is called (and apachesolr is not fully configured), it causes a PHP error: Call to a member function deleteByQuery() on a non-object in /.../apachesolr/apachesolr.admin.inc on line 448.

This is because it assumes apachesolr_get_solr() will always succeed.

function apachesolr_delete_index($type = NULL) {
  // Instantiate a new Solr object.
  $solr = apachesolr_get_solr();
  if ($type) {
    $query = 'type:' . $type;
  }
  else {
    $query = '*:*';
  }
  // Allow other modules to modify the delete query.
  // For example, use the site hash so that you only delete this site's
  // content:  $query = 'hash:' . apachesolr_site_hash()
  drupal_alter('apachesolr_delete_index', $query);
  $solr->deleteByQuery($query);

apachesolr_get_solr() fails gracefully, so we can wrap all this in an if clause:

function apachesolr_delete_index($type = NULL) {
  // Instantiate a new Solr object.
  if ($solr = apachesolr_get_solr()) {
    if ($type) {
      $query = 'type:' . $type;
    }
    else {
      $query = '*:*';
    }
    // Allow other modules to modify the delete query.
    // For example, use the site hash so that you only delete this site's
    // content:  $query = 'hash:' . apachesolr_site_hash()
    drupal_alter('apachesolr_delete_index', $query);
    $solr->deleteByQuery($query);

This allows third-party modules to call deleteByQuery without having to check that apachesolr is properly configured.
Patch attached.

Comments

pwolanin’s picture

StatusFileSize
new1.39 KB

I think the factory function should instead allow the exception to be thrown.

manarth’s picture

@pwolanin - yes, that looks a better approach.

Do you think it worth continuing to use try/catch in the factory function (so that catch can log a watchdog message), and throw the caught error from the catch block?

pwolanin’s picture

I think every relevant catch block will already log a message, so it seems a bit redundant.

By the way, what exception message do you see for this?

manarth’s picture

Finally got around to a rebuild with the patch in place...

The error given is:

"0" Status: Request failed: Connection refused

pwolanin’s picture

Well, that might be ok if you are not configured at all.

manarth’s picture

Sorry, I didn't really say enough in my last message - what I meant to say was: Thanks! That patch (complete with exception) is perfect for what I'm doing...

I'm making calls to solr functions in an install profile (with the config coming from settings.php), and without the patch, the install fails - with the patch, I can just catch the exception and continue with the build.

pwolanin’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev
Status: Needs review » Patch (to be ported)

committed to 6.x-1.x

"#742998 by pwolanin: stop catching exceptions in the solr factory function."

jpmckinney’s picture

Assigned: Unassigned » jpmckinney
jpmckinney’s picture

Version: 6.x-2.x-dev » 5.x-2.x-dev
Assigned: jpmckinney » Unassigned

Committed to 6.x-2.x. http://drupal.org/cvs?commit=359996 Needs port to 5.x.

jpmckinney’s picture

jpmckinney’s picture

Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

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