Require [articles] setting in elision filter

[romseygeek]

We should throw an exception at construction time if a list of
articles is not provided, otherwise we can get random NPEs during
indexing.

Fixes #43002

[elasticmachine]

Pinging @elastic/es-search

[telendt]

If it requires setting, shouldn't elision filter provider be wrapped in requiresAnalysisSettings at the time of registering?

Also - why are those settings required for _analyze but not in a "regular" analyzer?

(I do not have any elision specific settings in my index and everything works fine there)

[romseygeek]

If it requires setting, shouldn't elision filter provider be wrapped in requiresAnalysisSettings at the time of registering?

It should, thanks for catching!

(I do not have any elision specific settings in my index and everything works fine there)

Are you sure you're actually using the elision filter during indexing? If I set things up without any settings, then I get NPEs during indexing:

PUT test
{
  "settings": {
    "analysis": {
      "filter": {
        "elision" : {
          "type" : "elision"
        }
      },
      "analyzer": {
        "default": {
          "tokenizer": "standard",
          "filter": [
            "elision"
          ]
        }
      }
    }
  }
}

POST test/_doc/1 
{
  "field1" : "l'avion"
}

{
  "error": {
    "root_cause": [
      {
        "type": "null_pointer_exception",
        "reason": null
      }
    ],
    "type": "null_pointer_exception",
    "reason": null
  },
  "status": 500
}

[romseygeek]

Aha, but there's a prebuilt elision filter that by default uses French articles, so if you don't define any filters at all but still add elision to your list of filters, then you get that prebuilt filter; and indeed, the _analyze API doesn't appear to pick this up. So we do need to add this PR, but there is more work to do afterwards.

[telendt]

Aha, but there's a prebuilt elision filter that by default uses French articles, so if you don't define any filters at all but still add elision to your list of filters, then you get that prebuilt filter

Yes, that's how it's used in my index. Sorry for imprecise description.

[romseygeek]

I opened #43568 as a follow-up to deal with pre-configured filters in the Analyze API

[cbuescher]

@romseygeek change LGTM, but can you briefly clarify how this will be handled when backporting to 7.3? The way I currently understand your comments here is that there an elision filter without an explicite definition in the settings will still use the built-in "french" one? Just want to make sure we're not breaking existing mappings, even if they might be silently behave buggy, on a minor when backporting.

[romseygeek]

an elision filter without an explicit definition in the settings will still use the built-in "french" one

That's correct. The only change we're making here is to capture a mis-configured filter early - currently, if you set up an elision filter in an explicit mapping but don't give it an articles set, then it will throw a NullPointerException when it is first used. Now, you'll get an error when you create the mapping.

[jimczi]

Thanks @romseygeek , I left a comment

[jimczi]

It's hidden but it's also possible to set articles_path to reference a file so we should also check this alternative.
I am not sure that a lot of users are aware of this and I doubt that it is very useful but we should handle it for bwc.

[romseygeek]

articles_path isn't documented, I'll add something to the tokenfilter docs about it as well.

[jimczi]

Thanks for updating, I left a minor comment but feel free to ignore.

[jimczi]

Can we check that parseArticles doesn't return a null value instead ?