Skip to content

Commit

Permalink
Polish for Issue spring-projects#2655 and PR spring-projects#2672.
Browse files Browse the repository at this point in the history
See spring-projects#2655
Original pull request: spring-projects#2672
  • Loading branch information
jxblum committed Sep 14, 2023
1 parent 5c87bf0 commit 4cb843d
Show file tree
Hide file tree
Showing 11 changed files with 90 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ public Mono<Long> add(K key, Map<V, Point> memberCoordinateMap) {

Mono<List<GeoLocation<ByteBuffer>>> serializedList = Flux
.fromIterable(() -> memberCoordinateMap.entrySet().iterator())
.map(entry -> new GeoLocation<>(rawValue(entry.getKey()), entry.getValue())).collectList();
.map(entry -> new GeoLocation<>(rawValue(entry.getKey()), entry.getValue()))
.collectList();

return serializedList.flatMap(list -> geoCommands.geoAdd(rawKey(key), list));
});
Expand All @@ -106,7 +107,8 @@ public Mono<Long> add(K key, Iterable<GeoLocation<V>> geoLocations) {
return createMono(geoCommands -> {

Mono<List<GeoLocation<ByteBuffer>>> serializedList = Flux.fromIterable(geoLocations)
.map(location -> new GeoLocation<>(rawValue(location.getName()), location.getPoint())).collectList();
.map(location -> new GeoLocation<>(rawValue(location.getName()), location.getPoint()))
.collectList();

return serializedList.flatMap(list -> geoCommands.geoAdd(rawKey(key), list));
});
Expand Down Expand Up @@ -220,7 +222,7 @@ public Flux<GeoResult<GeoLocation<V>>> radius(K key, V member, double radius) {

return createFlux(geoCommands ->
geoCommands.geoRadiusByMember(rawKey(key), rawValue(member), new Distance(radius)) //
.map(this::readGeoResult));
.map(this::readGeoResult));
}

@Override
Expand Down Expand Up @@ -265,7 +267,7 @@ public Mono<Boolean> delete(K key) {

Assert.notNull(key, "Key must not be null");

return template.doCreateMono(connection -> connection.keyCommands().del(rawKey(key))).map(l -> l != 0);
return template.doCreateMono(connection -> connection.keyCommands().del(rawKey(key))).map(count -> count != 0);
}

@Override
Expand All @@ -276,8 +278,8 @@ public Flux<GeoResult<GeoLocation<V>>> search(K key, GeoReference<V> reference,
Assert.notNull(reference, "GeoReference must not be null");
GeoReference<ByteBuffer> rawReference = getGeoReference(reference);

return createFlux(geoCommands -> geoCommands
.geoSearch(rawKey(key), rawReference, geoPredicate, args).map(this::readGeoResult));
return createFlux(geoCommands -> geoCommands.geoSearch(rawKey(key), rawReference, geoPredicate, args)
.map(this::readGeoResult));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.springframework.data.redis.connection.ReactiveHashCommands;
import org.springframework.data.redis.connection.convert.Converters;
import org.springframework.data.redis.serializer.RedisSerializationContext;
import org.springframework.data.redis.util.RedisAssertions;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -211,7 +212,7 @@ public Flux<HV> values(H key) {

Assert.notNull(key, "Key must not be null");

return createFlux(connection -> connection.hVals(rawKey(key)) //
return createFlux(hashCommands -> hashCommands.hVals(rawKey(key)) //
.map(this::readRequiredHashValue));
}

Expand Down Expand Up @@ -276,30 +277,20 @@ private HK readHashKey(ByteBuffer value) {

private HK readRequiredHashKey(ByteBuffer buffer) {

HK hashKey = readHashKey(buffer);

if (hashKey == null) {
throw new InvalidDataAccessApiUsageException("Deserialized hash key is null");
}

return hashKey;
return RedisAssertions.requireNonNull(readHashKey(buffer),

This comment has been minimized.

Copy link
@mp911de

mp911de Sep 15, 2023

RedisAssertions adds a lot of noise and isn't otherwise helpful. Please revert those changes across the board.

This comment has been minimized.

Copy link
@jxblum

jxblum Sep 15, 2023

Author Owner

Adds a lot of noise... How? It simplified 5 lines into 1 with a single return statement and is very readable. I think it helped.

This comment has been minimized.

Copy link
@jxblum

jxblum Sep 15, 2023

Author Owner

How is this any different?

public void someMethod(String argument) {

    Assert.hasText(argument,  () -> "Argument [%s] is required", argument);

   // go on to use argument
}
() -> new InvalidDataAccessApiUsageException("Deserialized hash key is null"));
}

@SuppressWarnings("unchecked")
@Nullable
private HV readHashValue(@Nullable ByteBuffer value) {
return (HV) (value == null ? null : serializationContext.getHashValueSerializationPair().read(value));
return value != null ? (HV) serializationContext.getHashValueSerializationPair().read(value) : null;
}

private HV readRequiredHashValue(ByteBuffer buffer) {

HV hashValue = readHashValue(buffer);

if (hashValue == null) {
throw new InvalidDataAccessApiUsageException("Deserialized hash value is null");
}

return hashValue;
return RedisAssertions.requireNonNull(readHashValue(buffer),
() -> new InvalidDataAccessApiUsageException("Deserialized hash value is null"));
}

private Map.Entry<HK, HV> deserializeHashEntry(Map.Entry<ByteBuffer, ByteBuffer> source) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.springframework.data.redis.connection.ReactiveListCommands.LPosCommand;
import org.springframework.data.redis.connection.RedisListCommands.Position;
import org.springframework.data.redis.serializer.RedisSerializationContext;
import org.springframework.data.redis.util.RedisAssertions;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -351,12 +352,7 @@ private V readValue(ByteBuffer buffer) {

private V readRequiredValue(ByteBuffer buffer) {

V v = readValue(buffer);

if (v == null) {
throw new InvalidDataAccessApiUsageException("Deserialized list value is null");
}

return v;
return RedisAssertions.requireNonNull(readValue(buffer),
() -> new InvalidDataAccessApiUsageException("Deserialized list value is null"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.redis.connection.ReactiveSetCommands;
import org.springframework.data.redis.serializer.RedisSerializationContext;
import org.springframework.data.redis.util.RedisAssertions;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -424,12 +425,7 @@ private V readValue(ByteBuffer buffer) {

private V readRequiredValue(ByteBuffer buffer) {

V v = readValue(buffer);

if (v == null) {
throw new InvalidDataAccessApiUsageException("Deserialized set value is null");
}

return v;
return RedisAssertions.requireNonNull(readValue(buffer),
() -> new InvalidDataAccessApiUsageException("Deserialized set value is null"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.springframework.data.redis.core.types.Expiration;
import org.springframework.data.redis.serializer.RedisSerializationContext;
import org.springframework.data.redis.serializer.RedisSerializationContext.SerializationPair;
import org.springframework.data.redis.util.RedisAssertions;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -336,13 +337,8 @@ private V readValue(ByteBuffer buffer) {

private V readRequiredValue(ByteBuffer buffer) {

V v = readValue(buffer);

if (v == null) {
throw new InvalidDataAccessApiUsageException("Deserialized value is null");
}

return v;
return RedisAssertions.requireNonNull(readValue(buffer),
() -> new InvalidDataAccessApiUsageException("Deserialized value is null"));
}

private SerializationPair<String> stringSerializationPair() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.springframework.data.redis.core.ZSetOperations.TypedTuple;
import org.springframework.data.redis.serializer.RedisSerializationContext;
import org.springframework.data.redis.util.ByteUtils;
import org.springframework.data.redis.util.RedisAssertions;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

Expand Down Expand Up @@ -744,13 +745,8 @@ private V readValue(ByteBuffer buffer) {

private V readRequiredValue(ByteBuffer buffer) {

V v = readValue(buffer);

if (v == null) {
throw new InvalidDataAccessApiUsageException("Deserialized sorted set value is null");
}

return v;
return RedisAssertions.requireNonNull(readValue(buffer),
() -> new InvalidDataAccessApiUsageException("Deserialized sorted set value is null"));
}

private TypedTuple<V> readTypedTuple(Tuple raw) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.springframework.data.redis.serializer.RedisElementReader;
import org.springframework.data.redis.serializer.RedisElementWriter;
import org.springframework.data.redis.serializer.RedisSerializationContext;
import org.springframework.data.redis.util.RedisAssertions;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
Expand All @@ -61,12 +62,12 @@
* {@link org.springframework.data.redis.serializer.RedisElementReader#read(ByteBuffer)} returns {@code null} for a
* particular element as Reactive Streams prohibit the usage of {@code null} values.
*
* @param <K> the Redis key type against which the template works (usually a String)
* @param <V> the Redis value type against which the template works
* @author Mark Paluch
* @author Christoph Strobl
* @author Petromir Dzhunev
* @since 2.0
* @param <K> the Redis key type against which the template works (usually a String)
* @param <V> the Redis value type against which the template works
*/
public class ReactiveRedisTemplate<K, V> implements ReactiveRedisOperations<K, V> {

Expand Down Expand Up @@ -678,12 +679,7 @@ private K readKey(ByteBuffer buffer) {

private K readRequiredKey(ByteBuffer buffer) {

K key = readKey(buffer);

if (key == null) {
throw new InvalidDataAccessApiUsageException("Deserialized key is null");
}

return key;
return RedisAssertions.requireNonNull(readKey(buffer),
() -> new InvalidDataAccessApiUsageException("Deserialized key is null"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.springframework.data.redis.serializer.RedisElementWriter;
import org.springframework.data.redis.serializer.RedisSerializationContext;
import org.springframework.data.redis.serializer.RedisSerializationContext.SerializationPair;
import org.springframework.data.redis.util.RedisAssertions;
import org.springframework.util.Assert;

/**
Expand Down Expand Up @@ -105,45 +106,33 @@ protected <T> Flux<T> eval(ReactiveRedisConnection connection, RedisScript<T> sc

Flux<T> result = connection.scriptingCommands().evalSha(script.getSha1(), returnType, numKeys, keysAndArgs);

result = result.onErrorResume(e -> {
result = result.onErrorResume(cause -> {

if (ScriptUtils.exceptionContainsNoScriptError(e)) {
if (ScriptUtils.exceptionContainsNoScriptError(cause)) {
return connection.scriptingCommands().eval(scriptBytes(script), returnType, numKeys, keysAndArgs);
}

return Flux
.error(e instanceof RuntimeException ? (RuntimeException) e : new RedisSystemException(e.getMessage(), e));
return Flux.error(cause instanceof RuntimeException ? cause
: new RedisSystemException(cause.getMessage(), cause));
});

return script.returnsRawValue() ? result : deserializeResult(resultReader, result);
}

@SuppressWarnings("Convert2MethodRef")
@SuppressWarnings({ "Convert2MethodRef", "rawtypes", "unchecked" })
protected ByteBuffer[] keysAndArgs(RedisElementWriter argsWriter, List<K> keys, List<?> args) {

return Stream.concat(keys.stream().map(t -> keySerializer().getWriter().write(t)),
args.stream().map(t -> argsWriter.write(t))).toArray(size -> new ByteBuffer[size]);
}

/**
* @param script
* @return
*/
protected ByteBuffer scriptBytes(RedisScript<?> script) {
return serializationContext.getStringSerializationPair().getWriter().write(script.getScriptAsString());
}

protected <T> Flux<T> deserializeResult(RedisElementReader<T> reader, Flux<T> result) {
return result.map(it -> {

T value = ScriptUtils.deserializeResult(reader, it);

if (value == null) {
throw new InvalidDataAccessApiUsageException("Deserialized script result is null");
}

return value;
});
return result.map(it -> RedisAssertions.requireNonNull(ScriptUtils.deserializeResult(reader, it),
() -> new InvalidDataAccessApiUsageException("Deserialized script result is null")));
}

protected SerializationPair<K> keySerializer() {
Expand All @@ -169,6 +158,6 @@ private <T> Flux<T> execute(ReactiveRedisCallback<T> action) {
}

public ReactiveRedisConnectionFactory getConnectionFactory() {
return connectionFactory;
return this.connectionFactory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,25 @@ public static <T> T requireNonNull(@Nullable T target, Supplier<String> message)
return target;
}

/**
* Asserts the given {@link Object} is not {@literal null} throwing the given {@link RuntimeException}
* if {@link Object} is {@literal null}.
*
* @param <T> {@link Class type} of {@link Object} being asserted.
* @param target {@link Object} to evaluate.
* @param cause {@link Supplier} of a {@link RuntimeException} to throw
* if the given {@link Object} is {@literal null}.
* @return the given {@link Object}.
*/
public static <T> T requireNonNull(@Nullable T target, RuntimeExceptionSupplier cause) {

if (target == null) {
throw cause.get();
}

return target;
}

/**
* Asserts the given {@link Object} is not {@literal null}.
*
Expand Down Expand Up @@ -85,4 +104,7 @@ public static <T> T requireState(@Nullable T target, Supplier<String> message) {
Assert.state(target != null, message);
return target;
}

public interface RuntimeExceptionSupplier extends Supplier<RuntimeException> { }

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
@ExtendWith(LettuceConnectionFactoryExtension.class)
public class ReactiveStringRedisTemplateIntegrationTests {

private ReactiveRedisConnectionFactory connectionFactory;
private final ReactiveRedisConnectionFactory connectionFactory;

private ReactiveStringRedisTemplate template;

Expand Down Expand Up @@ -66,17 +66,16 @@ void keysFailsOnNullElements() {
template.opsForValue().set("a", "1").as(StepVerifier::create).expectNext(true).verifyComplete();
template.opsForValue().set("b", "1").as(StepVerifier::create).expectNext(true).verifyComplete();

RedisElementWriter<String> writer = RedisElementWriter.from(StringRedisSerializer.UTF_8);
RedisElementReader<String> reader = RedisElementReader.from(StringRedisSerializer.UTF_8);
RedisElementWriter<String> writer = RedisElementWriter.from(StringRedisSerializer.UTF_8);

RedisSerializationContext<String, String> nullReadingContext = RedisSerializationContext
.<String, String> newSerializationContext(StringRedisSerializer.UTF_8).key(buffer -> {
.<String, String>newSerializationContext(StringRedisSerializer.UTF_8).key(buffer -> {

String read = reader.read(buffer);
if ("a".equals(read)) {
return null;
}

return read;
return "a".equals(read) ? null : read;

}, writer).build();

ReactiveRedisTemplate<String, String> customTemplate = new ReactiveRedisTemplate<>(template.getConnectionFactory(),
Expand Down
Loading

0 comments on commit 4cb843d

Please sign in to comment.