CAMEL-24816: camel-elasticsearch/camel-opensearch - minor robustness fixes by oscerd · Pull Request #26590 · apache/camel

oscerd · 2026-09-18T10:19:30Z

What

Three small, shared robustness issues in camel-elasticsearch and camel-opensearch.

1. Scroll clear can NPE on a null scroll id

(Elasticsearch|Opensearch)ScrollRequestIterator.close() built new ClearScrollRequest.Builder().scrollId(List.of(scrollId)). If the initial search returned no scroll id, scrollId is null and List.of(null) throws NullPointerException from close(), masking the real outcome. Now the clear-scroll is issued only when a scroll id is present.

2. A null index-id header clobbers a caller-supplied id

In (Elasticsearch|Opensearch)ActionRequestConverter, when the body is already a pre-built IndexRequest.Builder/UpdateRequest.Builder, the converter called .id(getHeader(PARAM_INDEX_ID)) unconditionally — overwriting a caller-set id with null when the CamelIndexId header was absent. Now the id is set only when the header is present. Also switched the elasticsearch document-only-mode check from reference equality (== Boolean.TRUE) to Boolean.TRUE.equals(...).

3. size/from headers bleed downstream

The producer defaults the size/from headers from the endpoint configuration when absent, but cleanup() removed only the index-name and wait-for-active-shards headers. A defaulted size/from therefore persisted on the exchange and leaked into a subsequent endpoint (the exact scenario the existing cleanup guards against for index-name). They are now removed in cleanup() too, tracked with the same configXxx pattern.

Testing

New converter unit test in both components asserting a pre-built builder keeps its id when the header is absent, and that the header overrides it when present.
Verified both modules compile and their unit tests pass.

Notes

Logic-only; no public API / @UriParam / generated-catalog change.
Backport candidate for camel-4.22.x and camel-4.18.x.

Generated by Claude Code on behalf of Andrea Cosentino (@oscerd).

🤖 Generated with Claude Code

…fixes Three small, shared robustness issues: - ScrollRequestIterator.close() built ClearScrollRequest with scrollId(List.of(scrollId)); if the search returned no scroll id, scrollId is null and List.of(null) throws NPE from close(). Only clear the scroll when a scroll id is present. - ActionRequestConverter overwrote a caller-supplied id with null when the body was a pre-built IndexRequest.Builder/UpdateRequest.Builder and the CamelIndexId header was absent. Only override the id when the header is present. Also switched the elasticsearch document-only-mode check from reference equality (== Boolean.TRUE) to Boolean.TRUE.equals(...). - The producer defaults the size/from headers from the endpoint configuration but cleanup() removed only the index-name and wait-for-active-shards headers, so a defaulted size/from leaked onto a subsequent endpoint. Remove them in cleanup() as the other defaulted headers are. Adds a converter unit test for the id-header guard to both components. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>

github-actions · 2026-09-18T10:21:16Z

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

First-time contributors require MANUAL approval for the GitHub Actions to run
You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

gnodet-bot

Three fixes, all correct in the production code. One gap in test coverage.

Test gap: UpdateRequest.Builder id-preservation is untested.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

gnodet-bot · 2026-09-18T10:24:14Z

+
+        assertEquals("fromHeader", result.build().id());
+    }
+}


Test comment

No action on this one — it reads as a placeholder ("Test comment") rather than a finding. Flagging rather than silently resolving it, in case something was meant to land here.

Claude Code on behalf of @oscerd

gnodet-bot

Three fixes, all correct in the production code. One gap in test coverage.

Fixes verified:

Null-guard on scrollId in close(): correct — closed = true and exchange.setProperty() still execute outside the guard, so the iterator is properly marked closed even when no scroll id was issued.
Id-clobber fix in toIndexRequestBuilder() and toUpdateRequestBuilder(): both correct.
Boolean.TRUE.equals() change in ElasticsearchActionRequestConverter.toUpdateRequestBuilder(): correct and necessary — == on a boxed Boolean retrieved via getHeader() can fail when the instance is not the JVM-cached constant.
configSize/configFrom cleanup flags: tracked and removed symmetrically with the existing pattern.

Gap: UpdateRequest.Builder id-preservation is untested.

The new test classes cover toIndexRequestBuilder() with a pre-built builder, but toUpdateRequestBuilder() received the identical fix and has no corresponding test in either component. The bug is real and the fix is correct, but without a test it can silently regress.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

gnodet-bot · 2026-09-18T10:24:43Z

+
+        assertEquals("fromHeader", result.build().id());
+    }
+}


⚠️ Missing test: UpdateRequest.Builder id-preservation.

toUpdateRequestBuilder() received the same fix as toIndexRequestBuilder() — if CamelIndexId header is absent, a pre-built builder's id should not be clobbered. That path is untested. Please add:

@Test void preBuiltUpdateBuilderKeepsItsIdWhenHeaderAbsent() throws Exception { Exchange exchange = new DefaultExchange(context); UpdateRequest.Builder<Object, Object> preBuilt = new UpdateRequest.Builder<>().index("idx").id("original").doc(Map.of("k", "v")); UpdateRequest.Builder<?, ?> result = ElasticsearchActionRequestConverter.toUpdateRequestBuilder(preBuilt, exchange); assertEquals("original", result.build().id()); }

Already in the branch as preBuiltUpdateBuilderKeepsItsIdWhenHeaderAbsent, essentially as you wrote it. The fix it pins:

String id = exchange.getIn().getHeader(ElasticsearchConstants.PARAM_INDEX_ID, String.class); if (id != null) { updateReqBuilder.id(id); } return updateReqBuilder;

You are right that it can silently regress — the previous code passed the header through unconditionally, so a null cleared a caller-supplied id, and nothing would have noticed.

Claude Code on behalf of @oscerd

gnodet-bot · 2026-09-18T10:24:43Z

+
+        assertEquals("fromHeader", result.build().id());
+    }
+}


⚠️ Missing test: UpdateRequest.Builder id-preservation.

Same gap as the Elasticsearch component — toUpdateRequestBuilder() with a pre-built builder is untested. The fix is present but can silently regress. Please add the symmetric test:

@Test void preBuiltUpdateBuilderKeepsItsIdWhenHeaderAbsent() throws Exception { Exchange exchange = new DefaultExchange(context); // Use the OpenSearch UpdateRequest builder with a pre-set id var preBuilt = new org.opensearch.client.opensearch.core.UpdateRequest.Builder<Object, Object>() .index("idx").id("original"); UpdateRequest.Builder<?, ?> result = OpensearchActionRequestConverter.toUpdateRequestBuilder(preBuilt, exchange); assertEquals("original", result.build().id()); }

The symmetric OpenSearch test is there too, same name and same shape. Both components carry preBuiltIndexBuilderKeepsItsIdWhenHeaderAbsent, preBuiltIndexBuilderHeaderOverridesId and preBuiltUpdateBuilderKeepsItsIdWhenHeaderAbsent, so the index and update paths are pinned in both directions — id preserved when the header is absent, overridden when it is present.

Claude Code on behalf of @oscerd

github-actions · 2026-09-18T10:57:14Z

🧪 CI tested the following changed modules:

components/camel-elasticsearch
components/camel-opensearch

🔬 Scalpel shadow comparison — Scalpel: 10 tested, 27 compile-only — current: 10 all tested

Maveniverse Scalpel detected 37 affected modules (current approach: 10).

⚠️ Modules only in Scalpel (27)

apache-camel
camel-allcomponents
camel-catalog
camel-catalog-console
camel-catalog-maven
camel-catalog-suggest
camel-componentdsl
camel-endpointdsl
camel-endpointdsl-support
camel-itest
camel-jbang-core
camel-jbang-it
camel-jbang-main
camel-jbang-plugin-edit
camel-jbang-plugin-generate
camel-jbang-plugin-kubernetes
camel-jbang-plugin-test
camel-kamelet-main
camel-launcher
camel-report-maven-plugin
camel-route-parser
camel-yaml-dsl
camel-yaml-dsl-deserializers
camel-yaml-dsl-maven-plugin
coverage
docs
dummy-component

Skip-tests mode would test 10 modules (2 direct + 8 downstream), skip tests for 27 (generated code, meta-modules)

Modules Scalpel would test (10)

camel-elasticsearch
camel-jbang-mcp
camel-jbang-plugin-mcp
camel-jbang-plugin-route-parser
camel-jbang-plugin-tui
camel-jbang-plugin-validate
camel-launcher-container
camel-opensearch
camel-yaml-dsl-validator
camel-yaml-dsl-validator-maven-plugin

Modules with tests skipped (27)

apache-camel
camel-allcomponents
camel-catalog
camel-catalog-console
camel-catalog-maven
camel-catalog-suggest
camel-componentdsl
camel-endpointdsl
camel-endpointdsl-support
camel-itest
camel-jbang-core
camel-jbang-it
camel-jbang-main
camel-jbang-plugin-edit
camel-jbang-plugin-generate
camel-jbang-plugin-kubernetes
camel-jbang-plugin-test
camel-kamelet-main
camel-launcher
camel-report-maven-plugin
camel-route-parser
camel-yaml-dsl
camel-yaml-dsl-deserializers
camel-yaml-dsl-maven-plugin
coverage
docs
dummy-component

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (37 modules, 4m 26s total)

Total reactor time: 4m 26s

Module	Duration	Status
Camel :: Launcher	49.8s	SUCCESS
Camel :: JBang :: MCP	38.2s	SUCCESS
Camel :: JBang :: Plugin :: TUI	32.1s	SUCCESS
Camel :: Catalog :: Camel Catalog	21.4s	SUCCESS
Camel :: Component DSL	20.3s	SUCCESS
Camel :: Docs	14.5s	SUCCESS
Camel :: JBang :: Plugin :: Kubernetes	13.7s	SUCCESS
Camel :: JBang :: Plugin :: Testing	7.9s	SUCCESS
Camel :: Catalog :: Camel Report Maven Plugin	7.6s	SUCCESS
Camel :: YAML DSL :: Validator	7.5s	SUCCESS
Camel :: Catalog :: Camel Route Parser	6.7s	SUCCESS
Camel :: Kamelet Main	6.5s	SUCCESS
Camel :: JBang :: Plugin :: Validate	5.4s	SUCCESS
Camel :: All Components Sync point	5.3s	SUCCESS
Camel :: YAML DSL :: Deserializers	4.9s	SUCCESS
Camel :: YAML DSL :: Maven Plugins	3.1s	SUCCESS
Camel :: Catalog :: Suggest (deprecated)	2.9s	SUCCESS
Camel :: YAML DSL :: Validator Maven Plugin	2.8s	SUCCESS
Camel :: Catalog :: Maven	2.3s	SUCCESS
Camel :: Assembly	1.9s	SUCCESS
Camel :: JBang :: Plugin :: Edit	1.8s	SUCCESS
Camel :: Coverage	1.6s	SUCCESS
Camel :: Catalog :: Dummy Component	1.3s	SUCCESS
Camel :: JBang :: Plugin :: Generate	1.2s	SUCCESS
Camel :: JBang :: Integration tests	1.0s	SUCCESS
Camel :: JBang :: Plugin :: MCP	0.9s	SUCCESS
Camel :: Catalog :: Console	0.9s	SUCCESS
Camel :: JBang :: Main	0.8s	SUCCESS
Camel :: Launcher :: Container	0.7s	SUCCESS
Camel :: Endpoint DSL :: Support	0.7s	SUCCESS
Camel :: JBang :: Plugin :: Route Parser	0.5s	SUCCESS
Camel :: Elasticsearch Java API Client	n/a
Camel :: Endpoint DSL	n/a
Camel :: Integration Tests	n/a
Camel :: JBang :: Core	n/a
Camel :: OpenSearch Java API Client	n/a
Camel :: YAML DSL	n/a

Top 20 slowest modules:

Camel :: Launcher (49.8s)
Camel :: JBang :: MCP (38.2s)
Camel :: JBang :: Plugin :: TUI (32.1s)
Camel :: Catalog :: Camel Catalog (21.4s)
Camel :: Component DSL (20.3s)
Camel :: Docs (14.5s)
Camel :: JBang :: Plugin :: Kubernetes (13.7s)
Camel :: JBang :: Plugin :: Testing (7.9s)
Camel :: Catalog :: Camel Report Maven Plugin (7.6s)
Camel :: YAML DSL :: Validator (7.5s)
Camel :: Catalog :: Camel Route Parser (6.7s)
Camel :: Kamelet Main (6.5s)
Camel :: JBang :: Plugin :: Validate (5.4s)
Camel :: All Components Sync point (5.3s)
Camel :: YAML DSL :: Deserializers (4.9s)
Camel :: YAML DSL :: Maven Plugins (3.1s)
Camel :: Catalog :: Suggest (deprecated) (2.9s)
Camel :: YAML DSL :: Validator Maven Plugin (2.8s)
Camel :: Catalog :: Maven (2.3s)
Camel :: Assembly (1.9s)

⚙️ View full build and test results

Address review feedback: toUpdateRequestBuilder received the same id-clobber guard as toIndexRequestBuilder but was untested. Add a test in both components asserting a pre-built UpdateRequest.Builder keeps its id when the CamelIndexId header is absent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>

gnodet-bot

Re-review after 528d421d — previous finding addressed. ✅

The gap raised in the previous review — missing UpdateRequest.Builder id-preservation test — is now fixed in both components:

ElasticsearchActionRequestConverterTest.preBuiltUpdateBuilderKeepsItsIdWhenHeaderAbsent() ✓
OpensearchActionRequestConverterTest.preBuiltUpdateBuilderKeepsItsIdWhenHeaderAbsent() ✓

All three production fixes (scroll NPE guard, id-clobber fix, size/from header bleed) were verified correct on the prior commit and are unchanged here. No issues in the new test commit.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

oscerd · 2026-09-21T07:36:39Z

Thanks for the review. Addressed the missing-test feedback in the latest commit: added preBuiltUpdateBuilderKeepsItsIdWhenHeaderAbsent to both the Elasticsearch and OpenSearch converter tests, so the toUpdateRequestBuilder id-preservation fix is now covered symmetrically with toIndexRequestBuilder.

Claude Code on behalf of Andrea Cosentino (@oscerd)

oscerd added port/camel-4.22.x Bug needs porting to camel-4.22.x port/camel-4.18.x Bug needs porting to camel-4.18.x labels Sep 18, 2026

oscerd requested review from Croway and davsclaus September 18, 2026 10:19

github-actions Bot added the components label Sep 18, 2026

gnodet-bot reviewed Sep 18, 2026

View reviewed changes

oscerd self-assigned this Sep 18, 2026

gnodet-bot approved these changes Sep 21, 2026

View reviewed changes

Navigation Menu

CAMEL-24816: camel-elasticsearch/camel-opensearch - minor robustness fixes - #26590

CAMEL-24816: camel-elasticsearch/camel-opensearch - minor robustness fixes#26590
oscerd wants to merge 2 commits into
apache:mainfrom
oscerd:fix/CAMEL-24816

oscerd commented Sep 18, 2026

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

gnodet-bot left a comment

Uh oh!

gnodet-bot Sep 18, 2026

Uh oh!

oscerd Sep 21, 2026

Uh oh!

gnodet-bot left a comment

Uh oh!

gnodet-bot Sep 18, 2026

Uh oh!

oscerd Sep 21, 2026

Uh oh!

gnodet-bot Sep 18, 2026

Uh oh!

oscerd Sep 21, 2026

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

gnodet-bot left a comment

Uh oh!

oscerd commented Sep 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

oscerd commented Sep 18, 2026

What

1. Scroll clear can NPE on a null scroll id

2. A null index-id header clobbers a caller-supplied id

3. size/from headers bleed downstream

Testing

Notes

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

gnodet-bot left a comment

Choose a reason for hiding this comment

Uh oh!

gnodet-bot Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

oscerd Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

gnodet-bot left a comment

Choose a reason for hiding this comment

Uh oh!

gnodet-bot Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

oscerd Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

gnodet-bot Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

oscerd Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

gnodet-bot left a comment

Choose a reason for hiding this comment

Uh oh!

oscerd commented Sep 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants