Skip to content

Log a warning if param not annotated with @ProjectedPayload #3303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from

Conversation

onobc
Copy link
Contributor

@onobc onobc commented May 29, 2025

This commit introduces a warning log if a parameter is not annotated with @ProjectedPayload that this style is deprecated and that we will drop support for projections if a parameter is not annotated with @ProjectedPayload.

Resolves #3300

The output log message looks as follows:

2025-05-29 15:20:58,924 WARN eb.ProxyingHandlerMethodArgumentResolver: 271 - Parameter (method 'withModelAttribute' parameter 0) is not annotated with @ProjectedPayload - support for parameters not explicitly annotated with @ProjectedPayload (at the parameter or type level) will be dropped in a future version.

Note

If the parameter actually has a name (MethodParameter.getName() != null) then it will be included just before (method 'withModelAttribute' parameter...

  • You have read the Spring Data contribution guidelines.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

onobc added 2 commits May 27, 2025 16:56
This commit introduces a warning log if a parameter is not annotated with `@ProjectedPayload` that this style is deprecated and that we will drop support for projections if a parameter (or the parameter type) is not explicitly annotated with `@ProjectedPayload`.

Resolves #3300

Signed-off-by: Chris Bono <chris.bono@broadcom.com>
@onobc onobc added this to the 4.0 M4 (2025.1.0) milestone May 29, 2025

private final LogAccessor logger = new LogAccessor(ProxyingHandlerMethodArgumentResolver.class);

private final ConcurrentHashMap<MethodParameter, Boolean> loggedParameters = new ConcurrentHashMap<>();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My spidey-senses are typically activated when I add a map of things that could ultimately grow unbounded and OOME if something went awry. That would lead me to use the ConcurrentLruCache from framework. However, the added complexity plus the LruCache get API has no signal as to whether the cache was originally empty (this is the signal I needed). I chose simplicity over paranoia here.

What is the biggest number of method params we have encountered in the wilderness via bug reports etc..???

IOW - you good w/ this?

Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to switch to a simple static boolean? I guess a single warning is just enough. That would reduce this to a Logger declaration in ProxyingHandlerMethodArgumentResolver and a simple if?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thinking behind logging per parameter is that the user will get a more directly actionable message. Otherwise they may end up doing wak-a-mole to find the problematic parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the PR to include more context/location in the log message. It now looks like:

2025-06-02 11:18:57,706 WARN eb.ProxyingHandlerMethodArgumentResolver: 271 - Parameter 'foo' at position 0 in org.springframework.data.web.ProxyingHandlerMethodArgumentResolverUnitTests$Controller.withModelAttribute is not annotated with @ProjectedPayload - support for parameters not explicitly annotated with @ProjectedPayload (at the parameter or type level) will be dropped in a future version.

onobc added 2 commits June 2, 2025 11:19
- Move logger up to resolver

Signed-off-by: Chris Bono <chris.bono@broadcom.com>
Signed-off-by: Chris Bono <chris.bono@broadcom.com>
@mp911de mp911de self-assigned this Jun 4, 2025
@mp911de mp911de added type: bug A general bug type: enhancement A general enhancement and removed type: bug A general bug labels Jun 4, 2025
mp911de pushed a commit that referenced this pull request Jun 4, 2025
This commit introduces a warning log if a parameter is not annotated with `@ProjectedPayload` that this style is deprecated and that we will drop support for projections if a parameter (or the parameter type) is not explicitly annotated with `@ProjectedPayload`.

Resolves #3300
Original pull request: #3303

Signed-off-by: Chris Bono <chris.bono@broadcom.com>
mp911de added a commit that referenced this pull request Jun 4, 2025
Refine message. Use explicit variable types instead of var. Add tests.

See #3300
Original pull request: #3303
@mp911de mp911de closed this Jun 4, 2025
@mp911de mp911de deleted the issue/GH-3300 branch June 4, 2025 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log warning when param is not annotated with @ProjectedPayload
3 participants