GH-8678: Make Buffer Overflow Strategy Configurable in IntegrationWebSocketContainer

[ColoredCarrot]

Implement #8678: Make the send buffer overflow strategy configurable in IntegrationWebSocketContainer.

Adds a simple setter that is the missing analogue to setSendTimeLimit and setSendBufferSizeLimit.

[artembilan]

Thank you for the contribution.

A couple more comments to whatever I have already reviewed:

1. Any chances to have a simple unit test to new property is propagated properly to the decorated WebSocketSession. We can verify that via TestWebSocketListener in the ClientWebSocketContainerTests.
2. Add your name to the @author list of the affected classes.

[artembilan]

I don't think we need this extra property for a default value: we simply can initialize our sendBufferOverflowStrategy with that ConcurrentWebSocketSessionDecorator.OverflowStrategy.TERMINATE as default value.
Or better to have this as null and chose an appropriate ctor when we instantiate ConcurrentWebSocketSessionDecorator

[ColoredCarrot]

I considered the latter approach as well. I thought that having the default be explicit in this class, in addition to ConcurrentWebSocketSessionDecorator, would relieve users of having to open up ConcurrentWebSocketSessionDecorator to see the default.
We could mention that TERMINATE is the default in the Javadoc, but I feel like that would more than likely not get updated if the default is ever changed in ConcurrentWebSocketSessionDecorator. Thoughts?

[artembilan]

Well, to have a single place of truth, I'd prefer a @see ConcurrentWebSocketSessionDecorator in our new setter for this strategy.
Right, user would need to look there for more info, but then we would not not to worry about breaking change ever.

[ColoredCarrot]

Fair enough, I'll get that implemented :)

[artembilan]

Our preferences do not have blank lines in method Javadocs: https://github.com/spring-projects/spring-framework/wiki/Code-Style#javadoc-formatting

[ColoredCarrot]

Thank you for the detailed review & comments!

About the unit test: Unlike sendTimeLimit and bufferSizeLimit, ConcurrentWebSocketSessionDecorator's overflowStrategy has no getter (and is private). This seems like an oversight. Since ConcurrentWebSocketSessionDecorator isn't part of Spring Integration, we (probably?) can't fix that as part of this ticket.

In the meantime, the only way I can see to implement this test is by testing the actual behavior (I'm assuming using reflection to get the field value is out of the question). That would essentially duplicate this test case of Spring WebSocket.
Definitely possible; is that within the scope, or should we await the getter?

[artembilan]

or should we await the getter?

No, we should not.

See the mentioned ClientWebSocketContainerTests and its TestWebSocketListener with an afterSessionStarted() impl.
That's where that ConcurrentWebSocketSessionDecorator is propagated.
To get access to that private ConcurrentWebSocketSessionDecorator.overflowStrategy property we just can use our TestUtils.getPropertyValue() API which indeed is based on reflection anyway, but much easier to use.

[ColoredCarrot]

Right, I just wasn't aware that extracting private values like that was common practice. Won't be an issue then, thanks for the heads up.

[artembilan]

I'm merging with with some refactoring to be able to back-port this down to 5.5.x where we cannot use record yet.

[artembilan]

Merged as be53593.