Skip to content

Refactor getBulkOperationForAction into BulkOperationFactory#6748

Merged
dinujoh merged 3 commits into
opensearch-project:mainfrom
dinujoh:feature/opensearch-bulk-operation-factory
Apr 13, 2026
Merged

Refactor getBulkOperationForAction into BulkOperationFactory#6748
dinujoh merged 3 commits into
opensearch-project:mainfrom
dinujoh:feature/opensearch-bulk-operation-factory

Conversation

@dinujoh

@dinujoh dinujoh commented Apr 10, 2026

Copy link
Copy Markdown
Member

Description

Extract bulk operation creation logic from OpenSearchSink into a dedicated BulkOperationFactory class for improved testability and separation of concerns. Update unit tests to test the factory directly.

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

@dinujoh dinujoh force-pushed the feature/opensearch-bulk-operation-factory branch from 5d9f19a to 67fd627 Compare April 10, 2026 14:53
Extract bulk operation creation logic from OpenSearchSink into a
dedicated BulkOperationFactory class for improved testability and
separation of concerns.

Signed-off-by: Dinu John <86094133+dinujoh@users.noreply.github.com>
@dinujoh dinujoh force-pushed the feature/opensearch-bulk-operation-factory branch 2 times, most recently from e08b0dc to f281fce Compare April 10, 2026 15:23
dlvenable
dlvenable previously approved these changes Apr 10, 2026

@dlvenable dlvenable left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @dinujoh !

Add BulkOperationFactoryTest covering create, index, update, upsert,
delete, default action, optional fields, document filters, and pipeline
setting. Update OpenSearchSinkScriptTest to test BulkOperationFactory
directly instead of going through OpenSearchSink.

Signed-off-by: Dinu John <86094133+dinujoh@users.noreply.github.com>
@dinujoh dinujoh force-pushed the feature/opensearch-bulk-operation-factory branch from f281fce to d8bfbec Compare April 13, 2026 17:09
graytaylor0
graytaylor0 previously approved these changes Apr 13, 2026
Pass filteredJsonNode instead of jsonNode to scriptManager.buildScript
so that params.doc in the script reflects the post-filter document,
consistent with how builder.document() and builder.upsert() use the
filtered version.

Signed-off-by: Dinu John <86094133+dinujoh@users.noreply.github.com>
@dinujoh dinujoh merged commit 8a14fd7 into opensearch-project:main Apr 13, 2026
93 of 97 checks passed
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