elastic / elastic/elasticsearch-dsl-ruby

DSL AST initialisation recursively searches up namespaces for consts

Open
#16 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Ruby
Stars
18
Forks
16
PR merge metrics
No merged PRs in 30d

Description

When the DSL AST initialises nodes, it performs a `.const_get` on the relative module. This has problems, as it will look recursively up the entire module tree for a relating const that it will then instantiate.

For example, in `method_missing` on [`Elasticsearch::DSL::Search::Query` line 39](https://github.com/elastic/elasticsearch-ruby/blob/master/elasticsearch-dsl/lib/elasticsearch/dsl/search/query.rb#L39), it calls `const_defined?` and `const_get` on `Queries`. If you happen to have say, a class defined in the root namespace called `Report` and you call `report`, it'll find this class, make a new instance and give it to you. This creates some strangeness with the other conditional, where it defers to `binding` to allow you to access methods in the outer context.

I am using the DSL to dynamically create queries, often using instances of this model, which I have sensibly named `report` in the context I am using the DSL. However, when the `arity == 0`, and I try to access this `report`, it hits the `const_defined?` and initializes a new instance of `::Report`.

```ruby
class Report
end

class Example
include Elasticsearch::DSL

attr_accessor :report

def initialize(report)
@report = report
end

def generate_query
search do
query do
# Now anywhere in here report will be a Report.new everytime, instead of deferring to the outer caller's report.
end
aggregation :wicked_metric do
# Similarly here, the same issue arises.
end
end
end
end
```

I don't believe this is the intended behavior, or if it is intended, perhaps ensuring the found class is within the `Elasticsearch::DSL::Search` namespace would be sufficient safety.

```rb
Queries.const_defined? klass, false
```

I can just use the arity to prevent the issue, but that's pretty ugly on larger queries and creates a lot of clutter.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.