-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Make PreConfiguredTokenFilter harder to misuse #24572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There are now three public static method to build instances of PreConfiguredTokenFilter and the ctor is private. I chose static methods instead of constructors because those allow us to change out the implementation returned if we so desire. Relates to elastic#23658
rjernst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| */ | ||
| public final class PreConfiguredTokenFilter implements AnalysisModule.AnalysisProvider<TokenFilterFactory> { | ||
| /** | ||
| * Set up a pre-configured token filter that may no vary at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: no -> not
| */ | ||
| public final class PreConfiguredTokenFilter implements AnalysisModule.AnalysisProvider<TokenFilterFactory> { | ||
| /** | ||
| * Set up a pre-configured token filter that may no vary at all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "Set up" -> "Create"
|
From a categorization perspective this issue is about the plugin API more than analysis so I dropped the |
There are now three public static method to build instances of
PreConfiguredTokenFilter and the ctor is private. I chose static
methods instead of constructors because those allow us to change
out the implementation returned if we so desire.
Also moves over two more pre-configured token filters. There still
a bunch to go!
Relates to #23658