spring-projects / spring-data-redis

Provides support to increase developer productivity in Java when using Redis, a key-value store. Uses familiar Spring concepts such as a template classes for core API usage and lightweight repository style data access.
https://spring.io/projects/spring-data-redis/
Apache License 2.0
1.77k stars 1.17k forks source link

Negative maxlen for XAddOptions is ignored #2982

Closed jinkshower closed 2 months ago

jinkshower commented 2 months ago

This issue refers to a comment in #2936

As we now can use XAddOptions in streamOperations,

XAddOptions options = XAddOptions.maxlen(-1);
redisTemplate.opsForStream("myStream", Map.of("key", "value"), options);

This executes without throwing an error or warning the client. Instead, the result is that the maxlen option is not applied.

IMO, this is because the XAddOptions hasMaxlen() method gets called before every time XAddOptions is used, and it checks maxlen > 0, which results in ignoring negative values for maxlen.

@ParameterizedRedisTest
void addMinusMaxlenDoesNotThrowsException() {
    K key = keyFactory.instance();
    HV value = hashValueFactory.instance();
    XAddOptions options = XAddOptions.maxlen(-1).approximateTrimming(false);
    assertDoesNotThrow(() -> redisTemplate.opsForStream().add(StreamRecords.objectBacked(value).withStreamKey(key), options));
    assertThat(redisTemplate.opsForStream().range(key, Range.unbounded())).hasSize(1);
}

image

mp911de commented 2 months ago

We have a similar case with XPendingOptions while StreamReadOptions perform assertions on count.

jinkshower commented 2 months ago

@mp911de, I’d like to continue and finish what I started for this issue. Would you mind if I handle this issue? Also, do you want the PR to cover the XPendingOptions count as well?

mp911de commented 2 months ago

Sounds good, feel free to address both concerns within a single pull request.