Skip to content

CAMEL-22640: Tighten @Nullable annotations where non-null is enforced#23194

Merged
gnodet merged 3 commits into
apache:mainfrom
gnodet:CAMEL-22640-nullaway-enforce-nonnull
May 14, 2026
Merged

CAMEL-22640: Tighten @Nullable annotations where non-null is enforced#23194
gnodet merged 3 commits into
apache:mainfrom
gnodet:CAMEL-22640-nullaway-enforce-nonnull

Conversation

@gnodet
Copy link
Copy Markdown
Contributor

@gnodet gnodet commented May 13, 2026

CAMEL-22640

Summary

Follow-up to #23021 — removes @Nullable from 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 @Nullable from field, constructors, and getter — zero callers across the entire codebase pass null. Add requireNonNull enforcement.
  • LifecycleStrategy.onThreadPoolAdd() id param: Remove @NullableBaseExecutorServiceManager.onThreadPoolCreated() always computes a non-null id (with StringHelper.notEmpty() enforcement). The original Javadoc "(can be null in special cases)" was accurate when the method was introduced in CAMEL-3437 (the else branch left id as null when source was not a ProcessorDefinition or String), but the caller was later fixed to always generate an id using a source.getClass().getSimpleName() + hashcode fallback.

Not changed

Exception transient fields (InvalidPayloadException.type, ExpectedBodyTypeException.exchange/expectedBodyType) are kept @Nullable — although constructors enforce non-null via requireNonNull, the transient keyword means these fields will be null after Java serialization/deserialization.

Verification

  • mvn install -B -pl core/camel-api -DskipTests -am — BUILD SUCCESS
  • mvn install -B -pl core/camel-core -DskipTests — BUILD SUCCESS (downstream compilation)
  • mvn formatter:format impsort:sort -B -pl core/camel-api — no formatting changes

@gnodet gnodet requested review from apupier and davsclaus May 13, 2026 12:18
@github-actions github-actions Bot added the core label May 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🌟 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 13, 2026

🧪 CI tested the following changed modules:

  • core/camel-api

ℹ️ Dependent modules were not tested because the total number of affected modules exceeded the threshold (50). Use the test-dependents label to force testing all dependents.


⚙️ View full build and test results

@gnodet gnodet marked this pull request as draft May 13, 2026 12:44
@gnodet gnodet force-pushed the CAMEL-22640-nullaway-enforce-nonnull branch from 5d28f9e to 6dcb8bd Compare May 13, 2026 12:55
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: "
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the Objects.requireNonNull shoudl provide a meaningful message or it is uselles and just blur the codebase

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: "
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
@gnodet gnodet force-pushed the CAMEL-22640-nullaway-enforce-nonnull branch from 6dcb8bd to 59bc93a Compare May 13, 2026 14:13
@gnodet gnodet marked this pull request as ready for review May 13, 2026 14:48
@davsclaus
Copy link
Copy Markdown
Contributor

LGTM

gnodet and others added 2 commits May 14, 2026 23:15
…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>
@gnodet gnodet merged commit 34f6bd8 into apache:main May 14, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants