Refactors SpanTermQueryBuilder. #11005
Conversation
| public SpanTermQueryBuilder queryName(String queryName) { | ||
| this.queryName = queryName; | ||
| return this; | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override | ||
| public void doXContent(XContentBuilder builder, Params params) throws IOException { | ||
| builder.startObject(SpanTermQueryParser.NAME); |
There was a problem hiding this comment.
Not sure, but if we use parserName() instead of the constant here, can we move the whole doXContent method do the BaseTermQueryBuilder?
MaineC
force-pushed
the
feature/span-term-query-refactoring
branch
from
e20b6b4 to
24d7ae5
Compare
| private static IndexQueryParserService queryParserService; | ||
| private static Index index; | ||
| protected static IndexQueryParserService queryParserService; | ||
| protected static Index index; |
There was a problem hiding this comment.
I think those two can stay private, or do you need them in the concrete test classes? They should also be accesible via the context you get from createContext() if you need them.
There was a problem hiding this comment.
This must have been a merge artifact - will revert, thanks for spotting this.
|
@cbuescher Thanks for your comments - changed the code accordingly. |
|
Thanks, LGTM, but lets see if anybody else has an opinion. |
| /* | ||
| * Added class name to the hashcode so a {@link SpanTermQueryBuilder} and a {@link TermQueryBuilder} aren't considered the same | ||
| * if they have the same parameters set. | ||
| */ |
There was a problem hiding this comment.
this makes sense, wonder if the comment is even needed, seems more like a commit message ;)
|
left some comments |
MaineC
force-pushed
the
feature/span-term-query-refactoring
branch
from
8908571 to
da2eb5a
Compare
|
@javanna Thanks for your comments. Tried to incorporate them in the changes. |
| } | ||
|
|
||
| /** Writes the given parameters. */ | ||
| public void writeTo(StreamOutput out) throws IOException { |
MaineC
force-pushed
the
feature/span-term-query-refactoring
branch
from
560ea69 to
beab4c1
Compare
| * @param fieldName The name of the field | ||
| * @param value The value of the term | ||
| */ | ||
| @SuppressWarnings("boxing") |
There was a problem hiding this comment.
Added this after enabling (un-)boxing warnings. Let me know if this is too paranoid.
There was a problem hiding this comment.
I think it is :) especially given that we don't suppress this type of warnings anywhere in our codebase, not sure we should introduce this here. We tend to suppress stuff around generics mainly, in general warnings that show up with default IDE's setup (which IDE? :P )
|
left a very minor comment, LGTM though, please push ;) |
MaineC
force-pushed
the
feature/span-term-query-refactoring
branch
2 times, most recently
from
e5a1e63 to
5150237
Compare
This attempts to do to SpanTermQueryBuilder what has already been changed for TermQueryBuilder. The commit tries to avoid code duplication where possible by pulling what is the same for both QueryBuilders and tests into separate classes. Relates to elastic#10217
One final refactoring of the SpanTermQuery - makes sure the class hierarchy works again. Relates to elastic#10217
MaineC
force-pushed
the
feature/span-term-query-refactoring
branch
from
5150237 to
7a7c7f4
Compare
…ring Refactors SpanTermQueryBuilder. Due to similarities with TermQueryBuilder a lot of code was moved into separate abstract classes that can be used by both - TermQueryBuilder and SpanTermQueryBuilder. Relates to #10217
…refactoring Refactors SpanTermQueryBuilder. Due to similarities with TermQueryBuilder a lot of code was moved into separate abstract classes that can be used by both - TermQueryBuilder and SpanTermQueryBuilder. Relates to elastic#10217
clintongormley
added
:Search/Search
Refactors SpanTermQueryBuilder analogous to TermQueryBuilder. Factors common code between the two into separate classes.
Relates to #10217
This goes against the feature/query-refactoring branch.