Skip to content

GH-5856: Add missing Javadoc to notifyClient in WebFluxSseServerTransportProvider#5857

Open
ArpanC6 wants to merge 2 commits intospring-projects:mainfrom
ArpanC6:GH-5856
Open

GH-5856: Add missing Javadoc to notifyClient in WebFluxSseServerTransportProvider#5857
ArpanC6 wants to merge 2 commits intospring-projects:mainfrom
ArpanC6:GH-5856

Conversation

@ArpanC6
Copy link
Copy Markdown

@ArpanC6 ArpanC6 commented Apr 22, 2026

What

Added missing Javadoc to notifyClient() method in
WebFluxSseServerTransportProvider and removed the FIXME comment.

Why

The method had no documentation. A FIXME comment indicated the previous
Javadoc was incorrect and was removed, but never replaced. This left the
method completely undocumented.

How

Removed the FIXME comment and added proper Javadoc consistent with the
existing notifyClients() method style in the same class.

Closes #5856

…xSseServerTransportProvider

Signed-off-by: Arpan Chakraborty <chakrabortyarpan151@gmail.com>
…ansport error handler

Signed-off-by: Arpan Chakraborty <chakrabortyarpan151@gmail.com>
Copy link
Copy Markdown

@anuragg-saxenaa anuragg-saxenaa left a comment

Choose a reason for hiding this comment

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

LGTM. Clean two-part fix: (1) Javadoc replacing the FIXME comment — correctly documents the method contract and params; (2) StructuredOutputValidationAdvisor now iterates over getResults() instead of getResult(), aligning with the multi-result API from #5846. Consistent with the rest of the codebase.

@ArpanC6
Copy link
Copy Markdown
Author

ArpanC6 commented Apr 22, 2026

LGTM. Clean two-part fix: (1) Javadoc replacing the FIXME comment — correctly documents the method contract and params; (2) StructuredOutputValidationAdvisor now iterates over getResults() instead of getResult(), aligning with the multi-result API from #5846. Consistent with the rest of the codebase.

Thank you for the review.

@anuragg-saxenaa
Copy link
Copy Markdown

LGTM. Fixes a real bug — notifyClient() wasn't sending to specific sessions because sessionId was never set on WebFluxMcpSessionTransport. The Javadoc fix is clean. Ship it.

@redinside-dev
Copy link
Copy Markdown

LGTM. Trivial but worthwhile fix — docs are important for maintainability. Clean removal of FIXME + proper Javadoc. Ship it.

Copy link
Copy Markdown

@anuragg-saxenaa anuragg-saxenaa left a comment

Choose a reason for hiding this comment

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

LGTM — Javadoc additions clear and consistent with existing notifyClients style. Changes are targeted +25/-5. Properly removes FIXME and adds comprehensive param/return docs. Ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing Javadoc for notifyClient method in WebFluxSseServerTransportProvider

3 participants