DefaultPollableMessageSource should execute `postSend` and `afterSendCompletion` interceptor methods

[JordiMartinezVicent]

Describe the issue

An object of the class DefaultPollableMessageSource can contain a list of ChannelInterceptor.

Currently, when a PollableMessageSource does poll, only the method preSend of the interceptors is called. See invoke method.

In order to be consistent with a "normal" consumer, it would be nice that the methods:

• postSend
• afterSendCompletion

will be executed.

It would be nice that, preSend is executed before the user code is executed, and preSend and afterSendCompletion after the user code is executed.

Version of the framework

spring-cloud-stream: 4.2.0 spring-boot: 3.4.7

[artembilan]

I'm not sure that send part is correct here at all. We talk about a MessageSource which has only a receive operation. So, it looks like better to rework the logic to the receive part from ChannelInterceptor API:

	/**
	 * Invoked as soon as receive is called and before a Message is
	 * actually retrieved. If the return value is 'false', then no
	 * Message will be retrieved. This only applies to PollableChannels.
	 */
	default boolean preReceive(MessageChannel channel) {
		return true;
	}

	/**
	 * Invoked immediately after a Message has been retrieved but before
	 * it is returned to the caller. The Message may be modified if
	 * necessary; {@code null} aborts further interceptor invocations.
	 * This only applies to PollableChannels.
	 */
	default @Nullable Message<?> postReceive(Message<?> message, MessageChannel channel) {
		return message;
	}

	/**
	 * Invoked after the completion of a receive regardless of any exception that
	 * have been raised thus allowing for proper resource cleanup.
	 * <p>Note that this will be invoked only if {@link #preReceive} successfully
	 * completed and returned {@code true}.
	 * @since 4.1
	 */
	default void afterReceiveCompletion(@Nullable Message<?> message, MessageChannel channel,
			@Nullable Exception ex) {
	}

[JordiMartinezVicent]

I'm not sure that send part is correct here at all. We talk about a MessageSource which has only a receive operation. So, it looks like better to rework the logic to the receive part from ChannelInterceptor API:

	/**
	 * Invoked as soon as receive is called and before a Message is
	 * actually retrieved. If the return value is 'false', then no
	 * Message will be retrieved. This only applies to PollableChannels.
	 */
	default boolean preReceive(MessageChannel channel) {
		return true;
	}

	/**
	 * Invoked immediately after a Message has been retrieved but before
	 * it is returned to the caller. The Message may be modified if
	 * necessary; {@code null} aborts further interceptor invocations.
	 * This only applies to PollableChannels.
	 */
	default @Nullable Message<?> postReceive(Message<?> message, MessageChannel channel) {
		return message;
	}

	/**
	 * Invoked after the completion of a receive regardless of any exception that
	 * have been raised thus allowing for proper resource cleanup.
	 * <p>Note that this will be invoked only if {@link #preReceive} successfully
	 * completed and returned {@code true}.
	 * @since 4.1
	 */
	default void afterReceiveCompletion(@Nullable Message<?> message, MessageChannel channel,
			@Nullable Exception ex) {
	}

Just take into account the following:

• When a ChannelInterceptor is added to a MessageChannel, the methods "preSend", etc, are called. Even though the channel is baking a input binding.
• I guess, not calling to preSend would be a breaking change, as the current interceptors added to pollables expect the preSend method to be called.

[artembilan]

When a ChannelInterceptor is added to a MessageChannel, the methods "preSend", etc, are called.

Sure! But that is for a MessageChannel. Here we deal with a PollableMessageSource which really bypasses a MessageChannel abstraction and just calls provided MessageHandler in its poll() contract. Honestly, the ChannelInterceptor was wrong a choice from day first for this PollableMessageSource abstraction.

pollables expect the preSend method to be called.

And since it is pollable I would expect receive API from the interceptor to be called. So, feels like personal opinion and we cannot be sure what else other users of these two API think.

I guess, not calling to preSend would be a breaking change,

I believe it would be even better to deprecate ChannelInterceptor option on this PollableMessageSource at all since that API is misleading and there is no real hooks where it could be called internally. It might be better to come up with some other abstraction for intercepting, even if it is very generic MethodInterceptor.

However, I think there was a reason why ChannelInterceptor was chosen here as well. Perhaps there is some Spring Cloud Stream infrastructure which relies on ChannelInterceptor, and its functionality has to be called from the PollableMessageSource as well. And therefore, perhaps there is only a preSend phase implemented in that out-of-the-box ChannelInterceptor. So, feels like the behavior is what was intended. Would be great to get some confirmation on the matter. And with that in mind, it won't hurt to implement the rest of ChannelInterceptor contracts in the PollableMessageSource, even if it would be used only from the custom end-user interceptor. In the end I will just bite a bullet for my contracts incompatibility concern if that is what the framework expects from its internals.

Thanks