Fix incorrectly added functions.

[baezzys]

Correct that hashcode and equals were added in the chunk iterator class instead of in the chunk class.

This PR is related to #4314

[fmbenhassine]

Thank you for your PR. While testing it, I noticed there is test failure in FaultTolerantChunkProcessorTests.testWriteRetryOnTwoExceptions. Can you check please?

Also , please run ./mvnw spring-javaformat:apply to fix code formatting as well.

[baezzys]

I'll fix it

[baezzys]

@fmbenhassine

This is a problem related to Spring retry.

    private void registerContext(RetryContext context, RetryState state) {
        if (state != null) {
            Object key = state.getKey();
            if (key != null) {
                if (context.getRetryCount() > 1 && !this.retryContextCache.containsKey(key)) {
                    throw new RetryException("Inconsistent state for failed item key: cache key has changed. Consider whether equals() or hashCode() for the key might be inconsistent, or if you need to supply a better key");
                }

                this.retryContextCache.put(key, context);
            }
        }

    }

In this code, the chunk class is being used as the key value in the retryContextCache.

At the point where the error occurs, the item of class chunk has changed. Because this changes the hash code, it goes inside the if conditional, unlike before.

That's why the error occurs in the 4th assertEquals method of testWriteRetryOnTwoExceptions().

This is because it throws a RetryException, which is not the expected error.

How would you suggest fixing it, exclude the item from the hash code and equals? Or would it be better to modify the test code?

[fmbenhassine]

Thank you for tracing the issue down to Spring Retry!

In Spring Batch, failed items are retried without any modification (it does not make sense to change an item before retrying it). So if the test changes the items before retrying them, then the test is incorrect and should be fixed. Do not hesitate to adapt it as needed. Let me know if you need help on this. Thank you!

[baezzys]

Thank you for your review. I'll try to modify the test code according to your idea.

[baezzys]

Done.

[fmbenhassine]

The test is actually correct. Your changes of the test makes it inconsistent with the test name.

This is actually deeper than I thought. The issue is not with Spring Retry. The issue is with Spring Batch, which is using the Chunk itself as a key in the HashMap-based retry context cache here. This is different from another code path where items themselves are used as keys in the context here.

So we are in a case where the Chunk is used as a key of the HashMap of the retry context, and which is changing on retry attempts (debugging the case shows that the chunk is shrinking one item at a time when scanning is performed). Therefore, the hash code is changing on each attempt which leads to the error.

The correct way to fix this issue is by using items in the retry context, not the Chunk itself. This makes all the tests of FaultTolerantChunkProcessorTests pass without any modification, but unfortunately makes some other tests to fail.

At first, I thought this is related to the item buffering feature, but it is not (the test in question is still failing with or without buffering after adding equals/hashcode to Chunk). So I believe this is an old hidden bug that is now popping up after adding equals and hashcode to the Chunk class. For that reason, I created #4370 to dig further and fix the issue correctly.

For your PR, I took the commit that adds equals/hashcode, but I omitted the commit that changes the test (because the test is correct as explained previously).

Thank you for your contribution!

[fmbenhassine]

Rebased and merged as 30e8766.