Skip to content
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

[Backport 2.x] Adds a new parameter, max_analyzer_offset, for the highlighter #4031

Merged
merged 2 commits into from
Jul 28, 2022

Conversation

hauck-jvsh
Copy link
Contributor

I was not able to edit the original pull request so I open another one

hauck-jvsh and others added 2 commits July 27, 2022 21:43
…earch-project#3893)

* opensearch-project#3842 adds a new parameter to the highlighter, the max_analyzer_offset. When this parameter is provided the highlight stops in its value. This prevents the highlighter to go beyond the index maxAnalyzedOffset.

Signed-off-by: Hauck <[email protected]>

* Adds a test for the new parameter

Signed-off-by: Hauck <[email protected]>

* Fix the test add in the previous commit;

Signed-off-by: Hauck <[email protected]>

* This was checking against the wrong field

Signed-off-by: Hauck <[email protected]>

* Only runs the test for the correct version

Signed-off-by: Hauck <[email protected]>

* Skips the test in Elasticsearch as well;

Signed-off-by: Hauck <[email protected]>

* Remove elastic 3.0 to test

Signed-off-by: Hauck <[email protected]>

* Skips all versions

Signed-off-by: Hauck <[email protected]>

* Remove unnecessary fields as pointed by @reta

Signed-off-by: Hauck <[email protected]>

* Compute if fieldMaxAnalyzedIsNotValid in the constructor as suggest by @reta

Signed-off-by: Hauck <[email protected]>

* As discussed, it is better to throws different exceptions for when the fieldMaxAnalyzed is not valid and for when it is disabled;

Signed-off-by: Hauck <[email protected]>

* hint what to do to allow highlight of bigger documents
Signed-off-by: Hauck <[email protected]>

* Let the user define the new parameter globally for all fields highlighted

Signed-off-by: Hauck <[email protected]>

* Change the  fieldMaxAnalyzedOffset Integer in order to use null when it is absent in highlight. This allows the error messages to much more precise, showing invalid for all negative numbers;

Signed-off-by: Hauck <[email protected]>

* Update javadocs and implements the stream methods for the new fields;

Signed-off-by: Hauck <[email protected]>

* builder.field do not accept null, so check before calling the method is necessary

Signed-off-by: Hauck <[email protected]>

* Only send and read the new fields if the version supports it

Signed-off-by: Hauck <[email protected]>

* the previous commit was checking the wrong field

Signed-off-by: Hauck <[email protected]>

* Check for version 3.0.0 instead of current version

Signed-off-by: Hauck <[email protected]>

* Update server/src/main/java/org/apache/lucene/search/uhighlight/CustomUnifiedHighlighter.java

Co-authored-by: Andriy Redko <[email protected]>
Signed-off-by: Hauck <[email protected]>

* Execute the test after version 3.0.0

Signed-off-by: Hauck <[email protected]>

Co-authored-by: Andriy Redko <[email protected]>
(cherry picked from commit 931813f)
Signed-off-by: Hauck <[email protected]>
@hauck-jvsh hauck-jvsh requested review from a team and reta as code owners July 28, 2022 01:00
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2022

Codecov Report

Attention: Patch coverage is 44.11765% with 19 lines in your changes missing coverage. Please review.

Project coverage is 70.51%. Comparing base (c0b1b59) to head (f213d11).
Report is 2585 commits behind head on 2.x.

Files with missing lines Patch % Lines
...tch/subphase/highlight/SearchHighlightContext.java 37.50% 4 Missing and 1 partial ⚠️
...h/fetch/subphase/highlight/UnifiedHighlighter.java 16.66% 4 Missing and 1 partial ⚠️
...ne/search/uhighlight/CustomUnifiedHighlighter.java 20.00% 2 Missing and 2 partials ⚠️
...subphase/highlight/AbstractHighlighterBuilder.java 76.92% 2 Missing and 1 partial ⚠️
...rch/fetch/subphase/highlight/HighlightBuilder.java 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x    #4031      +/-   ##
============================================
- Coverage     70.68%   70.51%   -0.18%     
+ Complexity    56489    56430      -59     
============================================
  Files          4527     4527              
  Lines        271761   271794      +33     
  Branches      39975    39983       +8     
============================================
- Hits         192092   191647     -445     
- Misses        63513    64062     +549     
+ Partials      16156    16085      -71     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants