Skip to content

Public getIncludes method in SourceFieldMapper#20290

Merged
sohami merged 2 commits intoopensearch-project:mainfrom
buddharajusahil:feature/source-includes-public
Dec 23, 2025
Merged

Public getIncludes method in SourceFieldMapper#20290
sohami merged 2 commits intoopensearch-project:mainfrom
buddharajusahil:feature/source-includes-public

Conversation

@buddharajusahil
Copy link
Contributor

@buddharajusahil buddharajusahil commented Dec 19, 2025

Description

This change is adding a public getter function for the includes list in SourceFieldMapper.

Related Issues

Resolves #20289

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Summary by CodeRabbit

  • New Features

    • Added a public accessor to read which source fields are included, complementing the existing exclusion accessor.
  • Refactor

    • Modernized the include/exclude accessors to return a more flexible collection-style type; external behavior and results remain unchanged.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 19, 2025

Walkthrough

Added a public getter for includes in SourceFieldMapper and changed getIncludes() and getExcludes() return types from String[] to Collection<String>; updated implementations to return immutable collections and updated CHANGELOG.

Changes

Cohort / File(s) Summary
Source field mapper API
server/src/main/java/org/opensearch/index/mapper/SourceFieldMapper.java
Added a public getter for includes; changed getIncludes() and getExcludes() signatures from String[] to Collection<String>; returns immutable collections and added java.util.Collection import.
Changelog
CHANGELOG.md
Updated Unreleased 3.x entry to document the new public getter and signature changes on SourceFieldMapper.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify all callers of getIncludes() / getExcludes() compile and handle Collection<String> (vs String[]).
  • Confirm immutability expectations and any required defensive-copy semantics or javadoc updates.
  • Ensure CHANGELOG entry placement and wording are correct.

Possibly related PRs

Suggested reviewers

  • msfroh
  • reta
  • mch2
  • sachinpkale
  • shwetathareja
  • dbwiddis
  • ashking94
  • Bukhtawar
  • owaiskazi19
  • saratvemulapalli
  • cwperks
  • kotwanikunal
  • jed326
  • anasalkouz
  • Rishikesh1159
  • CEHENKLE
  • gbbafna

Poem

🐇 I hopped through code with eager paws,
I nudged the getters, changed their laws,
Arrays became a calmer collection,
I left the source with clear direction,
A tiny hop — a tidy API cause.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Out of Scope Changes check ❓ Inconclusive While the PR adds the requested public getter, it also changes return types from String[] to Collection for both getIncludes() and getExcludes(), which extends beyond the minimal scope of exposing the includes getter. Clarify whether the return type change from String[] to Collection for both methods is intentional and approved, or if this should be a separate change addressing broader API consistency.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a public getIncludes method to SourceFieldMapper, which is the primary objective of the PR.
Description check ✅ Passed The description follows the template with all required sections completed: clear description of changes, linked issue reference, and checklist confirming testing and API documentation.
Linked Issues check ✅ Passed The PR fully satisfies the linked issue #20289 by adding a public getter for the includes list in SourceFieldMapper as requested, enabling efficient retrieval of includes.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cec8f3 and 0e2815c.

📒 Files selected for processing (1)
  • CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
  • GitHub Check: gradle-check
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: Analyze (java)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
  • GitHub Check: detect-breaking-change
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: Mend Security Check

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@buddharajusahil buddharajusahil force-pushed the feature/source-includes-public branch 2 times, most recently from 90d9d6b to eb666b3 Compare December 19, 2025 18:22
@github-actions
Copy link
Contributor

❌ Gradle check result for eb666b3: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@buddharajusahil buddharajusahil force-pushed the feature/source-includes-public branch from eb666b3 to 467d1e5 Compare December 19, 2025 19:55
@github-actions
Copy link
Contributor

❕ Gradle check result for 467d1e5: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

@codecov
Copy link

codecov bot commented Dec 19, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.17%. Comparing base (be07784) to head (0e2815c).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...org/opensearch/index/mapper/SourceFieldMapper.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #20290   +/-   ##
=========================================
  Coverage     73.16%   73.17%           
+ Complexity    71744    71732   -12     
=========================================
  Files          5795     5795           
  Lines        328304   328305    +1     
  Branches      47281    47281           
=========================================
+ Hits         240216   240236   +20     
+ Misses        68822    68805   -17     
+ Partials      19266    19264    -2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@buddharajusahil buddharajusahil force-pushed the feature/source-includes-public branch from 467d1e5 to 40a70f7 Compare December 19, 2025 23:14
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
server/src/main/java/org/opensearch/index/mapper/SourceFieldMapper.java (1)

286-292: Consider adding JavaDoc for these public API methods.

The implementation correctly returns immutable collections using List.of(), which provides good encapsulation by preventing external modification of the internal state. However, since this class is marked with @PublicApi, these methods should include JavaDoc comments describing their purpose and return values.

📝 Suggested JavaDoc addition
+    /**
+     * Returns the list of fields to include in the source.
+     *
+     * @return an immutable collection of field patterns to include
+     */
     public Collection<String> getIncludes() {
         return List.of(includes);
     }

+    /**
+     * Returns the list of fields to exclude from the source.
+     *
+     * @return an immutable collection of field patterns to exclude
+     */
     public Collection<String> getExcludes() {
         return List.of(excludes);
     }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 467d1e5 and 40a70f7.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • server/src/main/java/org/opensearch/index/mapper/SourceFieldMapper.java (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: gradle-check
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: Analyze (java)
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: detect-breaking-change
🔇 Additional comments (2)
CHANGELOG.md (1)

10-10: LGTM!

The CHANGELOG entry accurately describes the addition of the public getter for the included field and is consistent with the existing format.

server/src/main/java/org/opensearch/index/mapper/SourceFieldMapper.java (1)

60-60: LGTM!

The import addition is necessary to support the new return type.

Signed-off-by: Sahil Buddharaju <sahilbud@amazon.com>
@buddharajusahil buddharajusahil force-pushed the feature/source-includes-public branch from 40a70f7 to 0cec8f3 Compare December 19, 2025 23:56
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 40a70f7 and 0cec8f3.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • server/src/main/java/org/opensearch/index/mapper/SourceFieldMapper.java (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/src/main/java/org/opensearch/index/mapper/SourceFieldMapper.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: gradle-check
  • GitHub Check: precommit (21, windows-2025, true)
  • GitHub Check: precommit (25, macos-15-intel)
  • GitHub Check: precommit (21, ubuntu-latest)
  • GitHub Check: precommit (25, macos-15)
  • GitHub Check: precommit (25, windows-latest)
  • GitHub Check: precommit (21, windows-latest)
  • GitHub Check: precommit (25, ubuntu-24.04-arm)
  • GitHub Check: precommit (21, macos-15)
  • GitHub Check: precommit (21, macos-15-intel)
  • GitHub Check: precommit (25, ubuntu-latest)
  • GitHub Check: precommit (21, ubuntu-24.04-arm)
  • GitHub Check: Analyze (java)
  • GitHub Check: assemble (25, ubuntu-latest)
  • GitHub Check: assemble (25, ubuntu-24.04-arm)
  • GitHub Check: assemble (21, ubuntu-latest)
  • GitHub Check: assemble (21, windows-latest)
  • GitHub Check: assemble (25, windows-latest)
  • GitHub Check: assemble (21, ubuntu-24.04-arm)
  • GitHub Check: detect-breaking-change

@github-actions
Copy link
Contributor

✅ Gradle check result for 0cec8f3: SUCCESS

Signed-off-by: sahil <61558528+buddharajusahil@users.noreply.github.com>
@github-actions
Copy link
Contributor

✅ Gradle check result for 0e2815c: SUCCESS

@sohami sohami merged commit b351381 into opensearch-project:main Dec 23, 2025
34 of 35 checks passed
mohit10011999 pushed a commit to mohit10011999/OpenSearch that referenced this pull request Dec 30, 2025
)

Signed-off-by: Sahil Buddharaju <sahilbud@amazon.com>
Signed-off-by: sahil <61558528+buddharajusahil@users.noreply.github.com>
Co-authored-by: Sahil Buddharaju <sahilbud@amazon.com>
mohit10011999 pushed a commit to mohit10011999/OpenSearch that referenced this pull request Dec 30, 2025
)

Signed-off-by: Sahil Buddharaju <sahilbud@amazon.com>
Signed-off-by: sahil <61558528+buddharajusahil@users.noreply.github.com>
Co-authored-by: Sahil Buddharaju <sahilbud@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement or improvement to existing feature or request _No response_

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Make includes publicly accessible in SourceFieldMapper

4 participants