CAMEL-22640: Tighten @Nullable annotations where non-null is enforced#23194
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
|
5d28f9e to
6dcb8bd
Compare
| public ResolveEndpointFailedException(@Nullable String uri, Throwable cause) { | ||
| super("Failed to resolve endpoint: " + sanitizeUri(uri) + " due to: " | ||
| public ResolveEndpointFailedException(String uri, Throwable cause) { | ||
| super("Failed to resolve endpoint: " + sanitizeUri(Objects.requireNonNull(uri, "uri")) + " due to: " |
There was a problem hiding this comment.
the Objects.requireNonNull shoudl provide a meaningful message or it is uselles and just blur the codebase
There was a problem hiding this comment.
That's how the JDK handles it, and it's consistent with the rest of the codebase (not completely, but mostly). NullPointerException("uri must not be null") doesn't add information over NullPointerException("uri") — the "must not be null" part is already conveyed by the exception type itself. The parameter name is the useful signal; the rest is noise.
There was a problem hiding this comment.
If we could come up with a real value added message, that would be different, but here, we're just checking an argument, we don't have any information to provide.
There was a problem hiding this comment.
yeah this is fine and how other frameworks also would do it - also those null values are rarely happening and if so its during development.
| public ResolveEndpointFailedException(@Nullable String uri, String message) { | ||
| super("Failed to resolve endpoint: " + sanitizeUri(uri) + " due to: " | ||
| public ResolveEndpointFailedException(String uri, String message) { | ||
| super("Failed to resolve endpoint: " + sanitizeUri(Objects.requireNonNull(uri, "uri")) + " due to: " |
There was a problem hiding this comment.
the Objects.requireNonNull shoudl provide a meaningful message or it is uselles and just blur the codebase
Remove @nullable from parameters that are already enforced as non-null or are never null in practice: - ResolveEndpointFailedException.uri: zero callers pass null, add requireNonNull enforcement - LifecycleStrategy.onThreadPoolAdd() id param: always computed by BaseExecutorServiceManager before callback (the original Javadoc "can be null in special cases" was accurate when added in CAMEL-3437, but the caller was later fixed to always generate an id) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
6dcb8bd to
59bc93a
Compare
|
LGTM |
…enforce-nonnull # Conflicts: # core/camel-api/src/main/java/org/apache/camel/ResolveEndpointFailedException.java
…eption Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CAMEL-22640
Summary
Follow-up to #23021 — removes
@Nullablefrom parameters that are already enforced as non-null or are never null in practice. The previous PR documented current reality; this PR tightens the contract where intentionality is clear.Changes
ResolveEndpointFailedException.uri: Remove@Nullablefrom field, constructors, and getter — zero callers across the entire codebase pass null. AddrequireNonNullenforcement.LifecycleStrategy.onThreadPoolAdd()idparam: Remove@Nullable—BaseExecutorServiceManager.onThreadPoolCreated()always computes a non-null id (withStringHelper.notEmpty()enforcement). The original Javadoc "(can be null in special cases)" was accurate when the method was introduced in CAMEL-3437 (theelsebranch leftidasnullwhensourcewas not aProcessorDefinitionorString), but the caller was later fixed to always generate an id using asource.getClass().getSimpleName() + hashcodefallback.Not changed
Exception
transientfields (InvalidPayloadException.type,ExpectedBodyTypeException.exchange/expectedBodyType) are kept@Nullable— although constructors enforce non-null viarequireNonNull, thetransientkeyword means these fields will be null after Java serialization/deserialization.Verification
mvn install -B -pl core/camel-api -DskipTests -am— BUILD SUCCESSmvn install -B -pl core/camel-core -DskipTests— BUILD SUCCESS (downstream compilation)mvn formatter:format impsort:sort -B -pl core/camel-api— no formatting changes