Dhairya-007 opened a new pull request, #29419:
URL: https://github.com/apache/camel/pull/29419
Two options carry `defaultValue = "true"` in their annotation but declare
the field without an initialiser, so the effective default is `false` and the
documented behaviour never happens unless the user sets the option explicitly.
### camel-netty-http — `logWarnOnBadRequest`
```java
@UriParam(label = "consumer,advanced", defaultValue = "true")
private boolean logWarnOnBadRequest;
```
`HttpServerChannelHandler` is the only reader:
```java
DecoderResult decoderResult = request.decoderResult();
if (decoderResult != null && decoderResult.cause() != null) {
if (getConsumer().getConfiguration().isLogWarnOnBadRequest()) {
LOG.warn("Netty request decoder failure due: {} returning HTTP
Status 400 to client", ...);
```
So a malformed HTTP request gets its 400 returned silently today, while the
option documents the warning as on by default — and the option only exists so
it can be turned off.
Every other `defaultValue = "true"` option in `NettyHttpConfiguration`
initialises its field (`mapHeaders`, `throwExceptionOnFailure`,
`send503whenSuspended`, `useRelativePath`); `logWarnOnBadRequest` is the one
that does not. Nothing else assigns it — the component constructor sets `sync`,
`reuseAddress` and the initializer factories, and `createEndpoint` either
copies a user-supplied configuration or news one up.
### camel-http — `LoggingHttpActivityListener.multiline`
```java
@Metadata(defaultValue = "true", description = "If enabled then each
information is outputted as separate LOG events")
private boolean multiline;
```
`multiline` picks between one LOG event per line and a single joined event,
so the listener currently defaults to the joined form while documenting the
opposite. This is the only `defaultValue = "true"` field in the class without
`= true` — `showRouteId`, `showRouteGroup`, `showExchangeId`, `showBody`,
`showHeaders`, `showStreams` and `sourceLocationLoggerName` all have it, and
`showBinary`, which documents `false`, correctly has none.
### How I found them, and what I left alone
I compared every `@UriParam`/`@Metadata` `defaultValue` of a `boolean` field
against the field's initialiser. Four fields came back as documented `true`
with no initialiser; the other two are fine in practice and I did not touch
them:
- `ResourceEndpoint.contentCache` — each component propagates its own
component-level `contentCache` (default `true`) onto the endpoint, e.g.
`VelocityComponent.createEndpoint` does `answer.setContentCache(cache)`.
- `FileWatchEndpoint.useFileHashing` — `FileWatchEndpoint(uri, component)`
does `setUseFileHashing(component.isUseFileHashing())`, and the component field
is `true`.
`LoggingHttpActivityListener.loggingLevel` looks similar (`defaultValue =
"INFO"`, field left null) but is handled at the point of use with `if
(loggingLevel != null && !loggingLevel.equals("INFO"))`, so it is only the
getter that reads back null. I left it as it is.
### Verification
Building these modules locally means building most of Camel at the current
SNAPSHOT, so the changes come with tests instead:
`NettyHttpComponentWithConfigurationTest.testNettyComponentDocumentedBooleanDefaults`
asserts all five documented-`true` options on a freshly created endpoint, and
a new `LoggingHttpActivityListenerTest` asserts the documented defaults of the
listener. Both fail on `isLogWarnOnBadRequest()` / `isMultiline()` against the
current code.
No generated metadata changes, since the annotations already said `true`. No
existing test reads `LoggingHttpActivityListener`, and the
`logWarnOnBadRequest` branch is reached only on `decoderResult.cause() !=
null`, which no netty-http test exercises, so the only effect is the WARN line
the option already promises.
--
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]
