davsclaus commented on code in PR #24320:
URL: https://github.com/apache/camel/pull/24320#discussion_r3547095594
##########
components/camel-opensearch/src/main/java/org/apache/camel/component/opensearch/OpensearchProducer.java:
##########
@@ -154,12 +162,17 @@ private OpensearchOperation resolveOperation(Exchange
exchange) {
@Override
public boolean process(Exchange exchange, AsyncCallback callback) {
try {
- if (configuration.isDisconnect() && client == null) {
- startClient();
+ OpenSearchTransport transport;
+ if (openSearchClient == null) {
+ if (configuration.isDisconnect() && client == null &&
!isCustomClient) {
Review Comment:
Inside this `if (openSearchClient == null)` branch, the `&& !isCustomClient`
check is always true (since `isCustomClient` is only true when
`openSearchClient != null`). This is redundant and can be simplified to:
```suggestion
if (configuration.isDisconnect() && client == null) {
```
##########
components/camel-opensearch/pom.xml:
##########
@@ -90,6 +90,12 @@
<version>${project.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>${mockito-version}</version>
Review Comment:
Minor: the child elements use 2-space indentation here, but the rest of the
POM uses 4-space indentation for dependency children. Running `mvn
formatter:format` should fix this automatically.
```suggestion
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>${mockito-version}</version>
<scope>test</scope>
```
Actually, please just run `mvn formatter:format` on this module to align
with the project style.
##########
components/camel-opensearch/src/main/java/org/apache/camel/component/opensearch/OpensearchProducer.java:
##########
@@ -91,11 +91,19 @@ class OpensearchProducer extends DefaultAsyncProducer {
protected final OpensearchConfiguration configuration;
private volatile RestClient client;
private Sniffer sniffer;
+ private final OpenSearchClient openSearchClient;
+ private boolean isCustomClient;
public OpensearchProducer(OpensearchEndpoint endpoint,
OpensearchConfiguration configuration) {
super(endpoint);
this.configuration = configuration;
this.client = endpoint.getClient();
+ this.openSearchClient = endpoint.getOpenSearchClient();
+ if(this.openSearchClient != null) {
+ isCustomClient = true;
Review Comment:
The `isCustomClient` field is always equivalent to `openSearchClient !=
null` — both are set once in the constructor and never modified. This field
adds unnecessary complexity.
The if/else block also has formatting issues (missing space after `if`,
missing spaces around `else`).
Suggestion: remove the `isCustomClient` field entirely and use
`openSearchClient != null` directly at the 3 call sites.
```suggestion
this.openSearchClient = endpoint.getOpenSearchClient();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
