gnodet-bot commented on code in PR #28510:
URL: https://github.com/apache/camel/pull/28510#discussion_r4376067566
##########
components/camel-stripe/src/main/java/org/apache/camel/component/stripe/StripeProducer.java:
##########
@@ -43,11 +54,27 @@ public StripeEndpoint getEndpoint() {
protected void doStart() throws Exception {
super.doStart();
StripeConfiguration config = getEndpoint().getConfiguration();
- Stripe.apiKey = config.getApiKey();
+ RequestOptions.RequestOptionsBuilder builder =
RequestOptions.builder();
+
+ if (ObjectHelper.isNotEmpty(config.getApiKey())) {
+ builder.setApiKey(config.getApiKey());
+ }
Review Comment:
⚠️ **Silent fallback to global `Stripe.apiKey` when `apiKey` is empty**
The old code always set `Stripe.apiKey = config.getApiKey()` (even to
`null`). The new code only populates the `RequestOptions` builder when `apiKey`
is non-empty — meaning that if `apiKey` is not configured, the `RequestOptions`
object carries **no authentication**, and the Stripe SDK will fall back to
whatever `Stripe.apiKey` is set globally (e.g. by another endpoint, or by
application code).
This is a **silent behaviour change** that could be surprising: previously,
the global key was explicitly set (even to `null`, clearing any prior value),
preventing accidental cross-endpoint key reuse. Now, an unconfigured endpoint
silently inherits global state.
Consider documenting this in the migration notes, or throwing an explicit
error/warning when `apiKey` is empty so the user gets a clear message rather
than a cryptic auth failure at request time.
##########
components/camel-stripe/src/main/java/org/apache/camel/component/stripe/StripeProducer.java:
##########
@@ -30,6 +39,8 @@ public class StripeProducer extends DefaultProducer {
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+ private RequestOptions requestOptions;
Review Comment:
💡 **Consider making `requestOptions` `volatile`**
The field is written in `doStart()`, nulled in `doStop()`, and read from
processing threads via `getRequestOptions()`. Camel's lifecycle methods are
meant to be called before/after message processing, but there is no
memory-visibility guarantee between the thread that calls `doStart()` and the
threads that later read this field. Adding `volatile` is a low-cost
defence-in-depth measure:
```suggestion
private volatile RequestOptions requestOptions;
```
--
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]
