Fix: [MongoDB Connector] Missing aggregation pipeline crashes sync job #495
Conversation
…we should default to an empty pipeline (as it's valid according to the filtering validation) so the sync job doesn't crash (MongoDB Ruby client doesn't provide a default parameter for the pipeline, only for options).
lib/connectors/mongodb/connector.rb
Outdated
| options = extract_options(aggregate) | ||
|
|
||
| if !pipeline.nil? && pipeline.empty? && !options.present? | ||
| if !pipeline.present? && !options.present? |
There was a problem hiding this comment.
I think it doesn't make sense to use present? and empty? will be enough. You have an inline if-statement in line 156 so there is no way that pipeline can be nil.
The same applies to options. You just need to make sure that you have the right object type to respond to .empty? method.
Both suggestions are optional because I just advocate stopping the usage of ActiveSupport lib :)
There was a problem hiding this comment.
Makes totally sense, adjusted it 👍 What's the rationale behind stopping to use ActiveSupport?
There was a problem hiding this comment.
What's the rationale behind stopping to use ActiveSupport?
ActiveSupport is a part of RubyOnRails framework where performance and clean code are not super important (not saying it's bad). A recent example was HashWithIndifferentAccess which adds unnecessary overhead.
I don't like extending the language without any reasonable need because this kind of libs often brings overhead.
#495) When a pipeline is nil (not specified in the advanced rules UI) we should default to an empty pipeline (as it's valid according to the filtering validation) so the sync job doesn't crash (MongoDB Ruby client doesn't provide a default parameter for the pipeline, only for options).
Closes https://github.com/elastic/enterprise-search-team/issues/3487
We now check if a pipeline is nil and set it to an empty array, if that's the case, so the sync job won't crash.
Checklists
Pre-Review Checklist