The value of releasePartialSequences is not propagated to the underlying SequenceSizeReleaseStrategy

[mjd507]

taking a example from unit test (which is disabled due to Time sensitive issue)
test method reference org.springframework.integration.aggregator.AggregatorTests#testAggPerfDefaultPartial

@Test
@Disabled("Time sensitive")
public void testAggPerfDefaultPartial() throws InterruptedException, ExecutionException,TimeoutException {
	AggregatingMessageHandler handler = new AggregatingMessageHandler(new DefaultAggregatingMessageGroupProcessor());
	handler.setCorrelationStrategy(message -> "foo");
	handler.setReleasePartialSequences(true);
        ....
}

in above test, we set releasePartialSequences to true, which will set releaseStrategy to SequenceSizeReleaseStrategy.

// class AbstractCorrelatingMessageHandler

public void setReleasePartialSequences(boolean releasePartialSequences) {
	if (!this.releaseStrategySet && releasePartialSequences) {
		setReleaseStrategy(new SequenceSizeReleaseStrategy());
	}
	this.releasePartialSequences = releasePartialSequences;
}

however, the releasePartialSequences value is never propagated to SequenceSizeReleaseStrategy.

actually it is propagated in onInit() method, inside afterPropertiesSet(), which means only we call afterPropertiesSet(), it will be propagated.

// class AbstractCorrelatingMessageHandler

@Override
protected void onInit() {
	super.onInit();
        // ...
	if (this.releasePartialSequences) {
		Assert.isInstanceOf(SequenceSizeReleaseStrategy.class, this.releaseStrategy, () ->
				"Release strategy of type [" + this.releaseStrategy.getClass().getSimpleName() +
						"] cannot release partial sequences. Use a SequenceSizeReleaseStrategy instead.");
		((SequenceSizeReleaseStrategy) this.releaseStrategy)
				.setReleasePartialSequences(this.releasePartialSequences);
	}
        // ...
}

---

I think it is our unit test issue, also the assertion is not correct. even it passes when we enabled the test.

for fix, I prefer remove the value propagation in method onInit(), and add this logic in setReleasePartialSequences()

// class AbstractCorrelatingMessageHandler
public void setReleasePartialSequences(boolean releasePartialSequences) {
	if (!this.releaseStrategySet && releasePartialSequences) {
		SequenceSizeReleaseStrategy sequenceSizeReleaseStrategy = new SequenceSizeReleaseStrategy();
		sequenceSizeReleaseStrategy.setReleasePartialSequences(releasePartialSequences);
		setReleaseStrategy(sequenceSizeReleaseStrategy);
	}
	this.releasePartialSequences = releasePartialSequences;
}

kindly confirm.

[artembilan]

I think the bug is indeed in the setReleasePartialSequences(), where we have to do this:

setReleaseStrategy(new SequenceSizeReleaseStrategy(releasePartialSequences));

However, the check in the onInit() still has to be there.
There is no guarantee which setter is going to be called first.
So, if setReleaseStrategy() first, then this.releaseStrategySet = true;, and next setReleasePartialSequences() will ignore the SequenceSizeReleaseStrategy population.
The onIni() logic is to ensure a consistency between those to setters.
We could do the logic in the setter, but why if we have it in the onInit().

Unlike other possible use-case for IntegrationObjectSupport, the instance of the AbstractCorrelatingMessageHandler really has to be a bean.

So, I suggest only the fix for this issue as I've shown in the begging: just this setReleaseStrategy(new SequenceSizeReleaseStrategy(releasePartialSequences));.
Not sure, though, that it is going to help that disabled testAggPerfDefaultPartial()...