Skip to content

Conversation

@anthologia
Copy link
Contributor

Related to: #10083

  • Replace org.springframework.lang.Nullable with org.jspecify.annotations.Nullable
  • Migrate package-info.java files to use @NullMarked annotation
  • Add @SuppressWarnings("NullAway.Init") for fields initialized in lifecycle methods

For the array fields BaseWsOutboundGatewaySpec.messageSenders and BaseWsOutboundGatewaySpec.gatewayInterceptors, I applied @SuppressWarnings("NullAway.Init") instead of @Nullable because adding @Nullable still produces the same NullAway warnings.

…pecify

Related to: spring-projects#10083

- Replace `org.springframework.lang.Nullable` with `org.jspecify.annotations.Nullable`
- Migrate `package-info.java` files to use `@NullMarked` annotation
- Add `@SuppressWarnings("NullAway.Init")` for fields initialized in lifecycle methods

Signed-off-by: Jooyoung Pyoung <[email protected]>
@anthologia
Copy link
Contributor Author

I found some packages don't have @org.jspecify.annotations.NullMarked applied.. I'll add them in a follow-up commit !

@artembilan
Copy link
Member

I'll add them in a follow-up commit !

Right. Let's finish with the current change you have so far and then we will go from there for others.
If that is more convenient for you, we may even go separate PRs.

Thanks

- Use proper array nullability: `WebServiceMessageSender @nullable []`
- Fix inner class annotation placement: `DefaultUriBuilderFactory.@nullable EncodingMode`
- Set default Jaxb2Marshaller for gatewayMarshaller field

Signed-off-by: Jooyoung Pyoung <[email protected]>
@anthologia
Copy link
Contributor Author

anthologia commented Jun 11, 2025

I've applied all the requested changes!

  • Set gatewayMarshaller to use Jaxb2Marshaller as default since it's an implementation of Marshaller. Thanks!
  • Updated all @Nullable annotations to be placed between access modifier and type for consistency.
    • Should we standardize @Nullable placement convention project-wide? 🤔 Currently there seems to be mixed usage.

I'll add commits tomorrow for other packages in the ws module to this PR rather than separate PRs! Is that fine?

@artembilan
Copy link
Member

Currently there seems to be mixed usage.

I know, but the easier way to proceed is to replace imports and leave as is.
We can clean up the proper placement as we will go eventually.
If that is fine for you, feel free to make it as it is recommended by JSpecify, but I'm not picky on that.

It is fine to continue in this PR.

Thanks.

* Add `@NullMarked` to all the `ws` packages
* Add `@Nullable` or `@SuppressWarnings("NullAway.Init")` whenever it is requested
* Add defensive null checks for better safety (`DefaultSoapHeaderMapper`, `SimpleWebServiceOutboundGateway`)

Signed-off-by: Jooyoung Pyoung <[email protected]>
@artembilan
Copy link
Member

Hey @anthologia !

Any updates, please?

* Add constructor overloads without messageFactory parameter
* Clean up unnecessary @SuppressWarnings("NullAway") annotations

Signed-off-by: Jooyoung Pyoung <[email protected]>
@anthologia
Copy link
Contributor Author

@artembilan

Sorry for the delay ! I just pushed the updates 😊

I noticed something while working on this - the WebServiceTemplate(WebServiceMessageFactory messageFactory) ctor param is marked as non-null, but internally it calls initDefaultStrategies() which handles null cases by initializing default strategies.

Should the messageFactory param actually be @Nullable in the WebServiceTemplate ctor? It seems like the implementation already supports null values through the default initialization mechanism.

What do you think?

@artembilan
Copy link
Member

The logic there is like this:

	/** Creates a new {@code WebServiceTemplate} using default settings. */
	public WebServiceTemplate() {
		initDefaultStrategies();
	}

	/**
	 * Creates a new {@code WebServiceTemplate} based on the given message factory.
	 * @param messageFactory the message factory to use
	 */
	public WebServiceTemplate(WebServiceMessageFactory messageFactory) {
		setMessageFactory(messageFactory);
		initDefaultStrategies();
	}

So, the contract enforces us to provide not-null if we chose that specific ctor.

You may raise a GH issue in Spring WS if you still think that it is OK to have it nullable in the second ctor since the first one simply assumes that it really can be null.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Thank you for an update.
Please, give me some feedback on those comments I have left.
As a fix or reject with respective arguments.

…ressions

* Use `@Nullable` `WebServiceMessageFactory` with conditional `webServiceTemplate` creation
* Mark `uri` and webServiceMessageFactory fields as `@Nullable`, keep template as final
* Remove all `@SuppressWarnings("NullAway")` annotations
* Remove unnecessary ctor overloads from previous approach

Signed-off-by: Jooyoung Pyoung <[email protected]>
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 the original author or authors.
Copy link
Member

Choose a reason for hiding this comment

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

This is not the first time.
So, after merging your PR, I'm going to apply our new -present style for Copyright.
That would make our life easier 😄

@artembilan artembilan merged commit 2b949cb into spring-projects:main Jun 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants