Polishing.

Reduce duplicate code by introducing common interface.
Fix ANY flag computation.

Original Pull Request: #2113

Author: christophstrobl

src/main/asciidoc/appendix/appendix-command-reference.adoc

@@ -48,6 +48,14 @@
 |EXPIREAT |X
 |FLUSHALL |X
 |FLUSHDB |X
+|GEOADD |X
+|GEODIST |X
+|GEOHASH |X
+|GEOPOS |X
+|GEORADIUS |X
+|GEORADIUSBYMEMBER |X
+|GEOSEARCH |X
+|GEOSEARCHSTORE |X
 |GET |X
 |GETBIT |X
 |GETRANGE |X

src/main/java/org/springframework/data/redis/connection/ReactiveGeoCommands.java

@@ -672,7 +672,7 @@ public GeoRadiusCommand withFlag(Flag flag) {
 			Assert.notNull(flag, "Flag must not be null!");
 
 			GeoRadiusCommandArgs args = cloneArgs();
-			args.flags.add(flag);
+			args.getFlags().add(flag);
 
 			return new GeoRadiusCommand(getKey(), point, distance, args, store, storeDist);
 		}
@@ -735,7 +735,7 @@ public GeoRadiusCommand sort(Direction direction) {
 			Assert.notNull(direction, "Direction must not be null!");
 
 			GeoRadiusCommandArgs args = cloneArgs();
-			args.sortDirection = direction;
+			args.sort(direction);
 
 			return new GeoRadiusCommand(getKey(), point, distance, args, store, storeDist);
 		}
@@ -1007,7 +1007,7 @@ public GeoRadiusByMemberCommand withFlag(Flag flag) {
 			Assert.notNull(flag, "Flag must not be null!");
 
 			GeoRadiusCommandArgs args = cloneArgs();
-			args.flags.add(flag);
+			args.getFlags().add(flag);
 
 			return new GeoRadiusByMemberCommand(getKey(), member, distance, args, store, storeDist);
 		}
@@ -1069,7 +1069,7 @@ public GeoRadiusByMemberCommand sort(Direction direction) {
 			Assert.notNull(direction, "Direction must not be null!");
 
 			GeoRadiusCommandArgs args = cloneArgs();
-			args.sortDirection = direction;
+			args.sort(direction);
 
 			return new GeoRadiusByMemberCommand(getKey(), member, distance, args, store, storeDist);
 		}

src/main/java/org/springframework/data/redis/connection/RedisGeoCommands.java

@@ -255,7 +255,7 @@ Long geoSearchStore(byte[] destKey, byte[] key, GeoReference<byte[]> reference,
 	 *
 	 * @since 2.6
 	 */
-	interface GeoShape {
+	interface GeoShape extends Shape {
 
 		/**
 		 * Create a shape used as predicate for geo queries from a {@link Distance radius} around the query center point.
@@ -350,17 +350,44 @@ public Metric getMetric() {
 		}
 	}
 
+	interface GeoCommandArgs {
+
+		@Nullable
+		public Direction getSortDirection();
+
+		@Nullable
+		Long getLimit();
+
+		Set<? extends GeoCommandFlag> getFlags();
+
+		default boolean hasLimit() {
+			return getLimit() != null;
+		}
+
+		default boolean hasSortDirection() {
+			return getSortDirection() != null;
+		}
+
+		default boolean hasFlags() {
+			return !getFlags().isEmpty();
+		}
+
+		public interface GeoCommandFlag {}
+	}
+
 	/**
 	 * Additional arguments (like count/sort/...) to be used with {@link RedisGeoCommands}.
 	 *
 	 * @author Mark Paluch
 	 * @since 2.6
 	 */
-	class GeoSearchCommandArgs implements Cloneable {
+	class GeoSearchCommandArgs implements GeoCommandArgs, Cloneable {
+
+		protected final Set<Flag> flags = new LinkedHashSet<>(2, 1);
 
-		Set<Flag> flags = new LinkedHashSet<>(2, 1);
-		@Nullable Long limit;
-		@Nullable Direction sortDirection;
+		@Nullable protected Long limit;
+
+		@Nullable protected Direction sortDirection;
 
 		private GeoSearchCommandArgs() {}
 
@@ -433,10 +460,7 @@ public GeoSearchCommandArgs sortDescending() {
 		 * @return never {@literal null}.
 		 */
 		public GeoSearchCommandArgs limit(long count) {
-
-			Assert.isTrue(count > 0, "Count has to positive value.");
-			limit = count;
-			return this;
+			return limit(count, false);
 		}
 
 		/**
@@ -450,7 +474,9 @@ public GeoSearchCommandArgs limit(long count, boolean any) {
 
 			Assert.isTrue(count > 0, "Count has to positive value.");
 			limit = count;
-			flags.add(Flag.ANY);
+			if (any) {
+				flags.add(Flag.ANY);
+			}
 			return this;
 		}
 
@@ -477,30 +503,18 @@ public Direction getSortDirection() {
 			return sortDirection;
 		}
 
-		public boolean hasFlags() {
-			return !flags.isEmpty();
-		}
-
-		public boolean hasSortDirection() {
-			return sortDirection != null;
-		}
-
-		public boolean hasLimit() {
-			return limit != null;
-		}
-
 		public boolean hasAnyLimit() {
 			return hasLimit() && flags.contains(Flag.ANY);
 		}
 
 		@Override
 		protected GeoSearchCommandArgs clone() {
 
-			GeoSearchCommandArgs tmp = new GeoSearchCommandArgs();
-			tmp.flags = this.flags != null ? new LinkedHashSet<>(this.flags) : new LinkedHashSet<>(2);
-			tmp.limit = this.limit;
-			tmp.sortDirection = this.sortDirection;
-			return tmp;
+			GeoSearchCommandArgs that = new GeoSearchCommandArgs();
+			that.flags.addAll(this.flags);
+			that.limit = this.limit;
+			that.sortDirection = this.sortDirection;
+			return that;
 		}
 	}
 
@@ -510,11 +524,13 @@ protected GeoSearchCommandArgs clone() {
 	 * @author Mark Paluch
 	 * @since 2.6
 	 */
-	class GeoSearchStoreCommandArgs implements Cloneable {
+	class GeoSearchStoreCommandArgs implements GeoCommandArgs, Cloneable {
+
+		private final Set<Flag> flags = new LinkedHashSet<>(2, 1);
 
-		Set<Flag> flags = new LinkedHashSet<>(2, 1);
-		@Nullable Long limit;
-		@Nullable Direction sortDirection;
+		@Nullable private Long limit;
+
+		@Nullable private Direction sortDirection;
 
 		private GeoSearchStoreCommandArgs() {}
 
@@ -576,10 +592,7 @@ public GeoSearchStoreCommandArgs sortDescending() {
 		 * @return never {@literal null}.
 		 */
 		public GeoSearchStoreCommandArgs limit(long count) {
-
-			Assert.isTrue(count > 0, "Count has to positive value.");
-			limit = count;
-			return this;
+			return limit(count, false);
 		}
 
 		/**
@@ -592,8 +605,10 @@ public GeoSearchStoreCommandArgs limit(long count) {
 		public GeoSearchStoreCommandArgs limit(long count, boolean any) {
 
 			Assert.isTrue(count > 0, "Count has to positive value.");
-			limit = count;
-			flags.add(Flag.ANY);
+			this.limit = count;
+			if (any) {
+				flags.add(Flag.ANY);
+			}
 			return this;
 		}
 
@@ -624,26 +639,18 @@ public boolean isStoreDistance() {
 			return flags.contains(Flag.STOREDIST);
 		}
 
-		public boolean hasSortDirection() {
-			return sortDirection != null;
-		}
-
-		public boolean hasLimit() {
-			return limit != null;
-		}
-
 		public boolean hasAnyLimit() {
 			return hasLimit() && flags.contains(Flag.ANY);
 		}
 
 		@Override
 		protected GeoSearchStoreCommandArgs clone() {
 
-			GeoSearchStoreCommandArgs tmp = new GeoSearchStoreCommandArgs();
-			tmp.flags = this.flags != null ? new LinkedHashSet<>(this.flags) : new LinkedHashSet<>(2);
-			tmp.limit = this.limit;
-			tmp.sortDirection = this.sortDirection;
-			return tmp;
+			GeoSearchStoreCommandArgs that = new GeoSearchStoreCommandArgs();
+			that.flags.addAll(this.flags);
+			that.limit = this.limit;
+			that.sortDirection = this.sortDirection;
+			return that;
 		}
 	}
 
@@ -729,18 +736,18 @@ public GeoRadiusCommandArgs limit(long count) {
 			return this;
 		}
 
-		public enum Flag {
+		public enum Flag implements GeoCommandFlag {
 			WITHCOORD, WITHDIST, ANY, STOREDIST
 		}
 
 		@Override
 		protected GeoRadiusCommandArgs clone() {
 
-			GeoRadiusCommandArgs tmp = new GeoRadiusCommandArgs();
-			tmp.flags = this.flags != null ? new LinkedHashSet<>(this.flags) : new LinkedHashSet<>(2);
-			tmp.limit = this.limit;
-			tmp.sortDirection = this.sortDirection;
-			return tmp;
+			GeoRadiusCommandArgs that = new GeoRadiusCommandArgs();
+			that.flags.addAll(this.flags);
+			that.limit = this.limit;
+			that.sortDirection = this.sortDirection;
+			return that;
 		}
 	}
 

src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConverters.java

@@ -30,6 +30,7 @@
 
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.dao.DataAccessException;
+import org.springframework.data.domain.Sort;
 import org.springframework.data.geo.Distance;
 import org.springframework.data.geo.GeoResult;
 import org.springframework.data.geo.GeoResults;
@@ -868,29 +869,27 @@ public static GeoArgs.Unit toGeoArgsUnit(Metric metric) {
 	 * @since 1.8
 	 */
 	public static GeoArgs toGeoArgs(GeoRadiusCommandArgs args) {
-		return toGeoArgs((GeoSearchCommandArgs) args);
+		return toGeoArgs((GeoCommandArgs) args);
 	}
 
 	/**
-	 * Convert {@link GeoRadiusCommandArgs} into {@link GeoArgs}.
+	 * Convert {@link GeoCommandArgs} into {@link GeoArgs}.
 	 *
 	 * @param args
 	 * @return
 	 * @since 2.6
 	 */
-	public static GeoArgs toGeoArgs(GeoSearchCommandArgs args) {
+	public static GeoArgs toGeoArgs(GeoCommandArgs args) {
 
 		GeoArgs geoArgs = new GeoArgs();
 
 		if (args.hasFlags()) {
-			for (GeoRadiusCommandArgs.Flag flag : args.getFlags()) {
-				switch (flag) {
-					case WITHCOORD:
-						geoArgs.withCoordinates();
-						break;
-					case WITHDIST:
-						geoArgs.withDistance();
-						break;
+			for (GeoCommandArgs.GeoCommandFlag flag : args.getFlags()) {
+				if(flag.equals(GeoRadiusCommandArgs.Flag.WITHCOORD)) {
+					geoArgs.withCoordinates();
+				}
+				else if(flag.equals(GeoRadiusCommandArgs.Flag.WITHDIST)) {
+					geoArgs.withDistance();
 				}
 			}
 		}
@@ -907,36 +906,9 @@ public static GeoArgs toGeoArgs(GeoSearchCommandArgs args) {
 		}
 
 		if (args.hasLimit()) {
-			geoArgs.withCount(args.getLimit(), args.hasAnyLimit());
+			geoArgs.withCount(args.getLimit(), args.getFlags().contains(GeoRadiusCommandArgs.Flag.ANY));
 		}
-		return geoArgs;
-	}
 
-	/**
-	 * Convert {@link GeoRadiusCommandArgs} into {@link GeoArgs}.
-	 *
-	 * @param args
-	 * @return
-	 * @since 2.6
-	 */
-	static GeoArgs toGeoArgs(GeoSearchStoreCommandArgs args) {
-
-		GeoArgs geoArgs = new GeoArgs();
-
-		if (args.hasSortDirection()) {
-			switch (args.getSortDirection()) {
-				case ASC:
-					geoArgs.asc();
-					break;
-				case DESC:
-					geoArgs.desc();
-					break;
-			}
-		}
-
-		if (args.hasLimit()) {
-			geoArgs.withCount(args.getLimit(), args.hasLimit());
-		}
 		return geoArgs;
 	}