Stopwords processor

Thanks! I think this will make a great addition, so will add it to the main module.
However, there are a few issues that will have to be addressed first (although the processor works quite well already, so great job!):

• First off, some small style issues in .info and .module, addressed in the attached patch. Only real change is that the stopwords processor should have a higher weight than the tokenizer.
  Oh, and I corrected postprocessSearchResults() to use array_merge() instead of the + operator, as the latter won't work with numeric indexes.
• I think it's more user-friendly to let the words be separated by white-space (including newlines), not commas. After all, you can't specify stop words containing spaces in any case. Then, use preg_split() to get the individual words. (Quick test shows this shouldn't be a performance issue, taking 130ms for a 1MB string.)
• Even better would be a way to use a file containing the stop words. Since you probably aren't keen on implementing some smart uploading, merging and what-not, a simple text field for a URL (which can either be local, "http://", "private://", …) should suffice. If left empty (or the file can't be opened), fall back to the text area.
• Instead of using configurationFormValidate(), you can probably just set #required on the form element. This of course would be moot when you add another way of specifying stop words.

•   // @todo Do we need to make sure there is a full text field?
    public function preprocessSearchQuery(SearchApiQuery $query) {
      $stopwords = $this->getStopWords();
      if (!is_array($stopwords)) {
        return;
      }
      $keys = $query->getKeys();
      foreach ($keys as $key => $value) {
        if (is_numeric($key) && in_array($value, $stopwords)) {
          unset($keys[$key]);
          $this->ignored[] = $value;
        }
      }
    }
  

  • Regarding the @todo: Yes, you should check whether $keys isn't empty. It could also be a string, which will have to also be checked. (You could maybe use a preg_replace() for this case, as well as for dealing with untokenized text at index time.)
  • is_array($stopwords) will always be TRUE (barring acts of God), you should check for empty($stopwords) instead.
  • As said, check whether keys are set and deal with string keys. Also, $value in the loop can be an array, which should be processed recursively to be completely correct. Removing the stopwords inside of phrase queries with preg_replace() would probably also be a good idea.
• Generally, I think this won't perform that well for large numbers of keywords. Instead of using in_array($v, $stopword), you could, e.g., flip the array and use isset($stopwords[$v]).
• I don't think that overriding preprocessIndexItems(), as well as the rest of this system, is necessary. Just overriding processFieldValue() should suffice, and would even free you from explicitly handling tokenized data. Which you don't do completely correctly at the moment, anyways. (I admit it is pretty hard to do, had also a number of bugs in there. But that's exactly why I provided a system that can be overridden in many places to free implementers from most of the work.)
• As said, using preg_replace() instead of processWords() to deal with words inside of text should also be looked at, in my opinion. Don't know how well that would perform for large numbers of stop words, though. In any case, this should be the code to do it:
  

  $delimiters = array_fill(0, count($stopwords), '/');
  $regex = '/\b(' . implode('|', array_map('preg_quote', $stopwords, $delimiters)) . ')\b\s*/';
  $text = preg_replace($regex, '', $text);
  

  Based on a quick test, it doesn't perform too shabby, either. Maybe we can provide a checkbox to disable this, if users run into performance problems with it.

Oh, you should also check whether $this->ignored and $response['ignored'] are even set in postprocessSearchResults().

I agree, maintaining stopword lists inside the module wouldn't be a good idea.

But, with the idea of providing localization support at some future point, if you have ideas on how to build a repository of files for any language people want to contribute, that would be good to work in.

For this use case, you could later just provide some syntax to specify those files. E.g., specify "stopword.*.txt" and the asterisk will automatically be replaced with the language code (or "und"). I don't think that's much of a problem.

What configuration will give me an array in $value? Is that coming from a facet, or from some other module?

Those can be used for more complex queries, e.g. with negated terms. I don't think it's currently used, but it's in the specification and therefore should be minded.
But there is also a processKey() method which will be called for all keywords in the default processor class, and which you can use so you don't have to take care of all of this yourself. You should probably just use that.

Is phrase searching supported by anything yet?

It's supported by the Solr backend, and I think by the Xapian backend, too.

Other comments:

• Instead of static, you should use an object property for caching in getStopwords(), as static will cache the same version for all objects. In the (admittedly unlikely, but still possible) event that two searches on different indexes (with both using differently configured stopwords processors) are executed on the same page request, this would filter out the wrong term for all searches but the first.
• You could also, instead of using getStopwords(), use an filterStopwords($word) method right away. This would help you not duplicate code for fields and search keys. (Or, you can just override the process() method, if you'd do the same things in processFieldValue() and processKey() anyways. This would, by the way, also take care of search filters set on fulltext fields, which you are currently ignoring.)
• Please don't use $query->keys() to set the processed search keys, as this will mess up the "original keys" property. Instead, save the return value of getKeys() as a reference and operate on $keys directly.

• +++ b/includes/processor_stopwords.inc
  @@ -0,0 +1,138 @@
  +      else $response['ignore'] = $this->ignored;
  

  Please always wrap the body of if/else/… in braces.

• You can maybe speed up the regex variant with the /…/S flag, and by saving the regex so it hasn't got to be constructed anew for each single value. The major benefit however isn't speed, but correctness: as far as I can see, your code currently will only find words separated by spaces on both sides, but not those separated by commas, periods, other special characters, newlines, etc.
  If you think the difference in performance is too large, we should maybe just ignore untokenized inout altogether and specify that people should tokenize the input (i.e., use the "Tokenizer" processor) before using the stopwords processor. Or we could implement the regex variant for untokenized input (or tokens that still contain whitespace, which we should then also check) and note in the processor description (or the configuration form) that performance will improve when tokenizing the input before filtering out stopwords.
  However, as I see it, both run times are too low to really care about the performance, even if the difference is a factor of about 100. In the end, in most cases this will still be a rather insignificant addition to the overall runtime.
• In several locations (when saving the form, when loading stop words, …) your code assumes the file will always be set. You should check whether the setting is even present before trying to use it as a file URI.
• Also, at least for me, files with the "http://" scheme don't work yet. I think the reason is that functions like is_file()/file_exists() (in contrast to, e.g., file_get_contents()) only work for local files. Better just try to read the file and see if this works, instead of using is_file().

Powered by Dreditor.

+++ b/includes/processor_stopwords.inc
@@ -0,0 +1,138 @@
+      else $response['ignore'] = $this->ignored;

There's also a typo ("ignore" instead of "ignored"), resulting in ignored words not showing properly.

Powered by Dreditor.

I fit should run after the tokenizer, this should be clearly stated.
This also had a bug that if a keyword was removed, there would be an empty string as the key. The abstract default processor should take care of unsetting such keys.
Lastly, we still cannot just use explode(), as that just ignores too many cases for my taste. We should at least use preg_split() to split on any whitespace.

The attached patch should fix all of this.

OK, then let's just wait a couple of days whether someone else wants to review, and then add this.
In any case, thanks again for your contribution!

Just committed this, thanks again!