Reset coordinate state of GeoWithinListOutput after completing an item …

…#805 GeoWithinListOutput now resets its longitude (x) coordinate state after completing a GeoWithin item. Previously, the longitude coordinate was never reset and subsequent items used the first reported longitude coordinate which rendered an incorrect result.
redis · Jun 21, 2018 · 84c3fdb · 84c3fdb
1 parent bda3018
commit 84c3fdb
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 27 deletions.
diff --git a/src/main/java/io/lettuce/core/output/GeoWithinListOutput.java b/src/main/java/io/lettuce/core/output/GeoWithinListOutput.java
@@ -101,6 +101,7 @@ public void complete(int depth) {
             distance = null;
             geohash = null;
             coordinates = null;
+            x = null;
         }
     }
 

diff --git a/src/test/java/io/lettuce/core/cluster/commands/GeoClusterCommandTest.java b/src/test/java/io/lettuce/core/cluster/commands/GeoClusterCommandTest.java
@@ -71,22 +71,22 @@ protected RedisCommands<String, String> connect() {
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geoaddWithTransaction() {
+    public void geoaddInTransaction() {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geoaddMultiWithTransaction() {
+    public void geoaddMultiInTransaction() {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void georadiusWithTransaction() {
+    public void georadiusInTransaction() {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geodistWithTransaction() {
+    public void geodistInTransaction() {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
@@ -96,16 +96,16 @@ public void georadiusWithArgsAndTransaction() {
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void georadiusbymemberWithArgsAndTransaction() {
+    public void georadiusbymemberWithArgsInTransaction() {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geoposWithTransaction() {
+    public void geoposInTransaction() {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geohashWithTransaction() {
+    public void geohashInTransaction() {
     }
 }
diff --git a/src/test/java/io/lettuce/core/commands/GeoCommandTest.java b/src/test/java/io/lettuce/core/commands/GeoCommandTest.java
@@ -31,6 +31,9 @@
 import io.lettuce.core.*;
 import io.lettuce.core.api.StatefulRedisConnection;
 
+/**
+ * @author Mark Paluch
+ */
 public class GeoCommandTest extends AbstractRedisClientTest {
 
     @Rule
@@ -57,7 +60,7 @@ public void geoadd() {
     }
 
     @Test
-    public void geoaddWithTransaction() {
+    public void geoaddInTransaction() {
 
         redis.multi();
         redis.geoadd(key, -73.9454966, 40.747533, "lic market");
@@ -75,7 +78,7 @@ public void geoaddMulti() {
     }
 
     @Test
-    public void geoaddMultiWithTransaction() {
+    public void geoaddMultiInTransaction() {
 
         redis.multi();
         redis.geoadd(key, 8.6638775, 49.5282537, "Weinheim", 8.3796281, 48.9978127, "EFS9", 8.665351, 49.553302, "Bahn");
@@ -88,11 +91,6 @@ public void geoaddMultiWrongArgument() {
         redis.geoadd(key, 49.528253);
     }
 
-    protected void prepareGeo() {
-        redis.geoadd(key, 8.6638775, 49.5282537, "Weinheim");
-        redis.geoadd(key, 8.3796281, 48.9978127, "EFS9", 8.665351, 49.553302, "Bahn");
-    }
-
     @Test
     public void georadius() {
 
@@ -106,7 +104,7 @@ public void georadius() {
     }
 
     @Test
-    public void georadiusWithTransaction() {
+    public void georadiusInTransaction() {
 
         prepareGeo();
 
@@ -122,6 +120,22 @@ public void georadiusWithTransaction() {
         assertThat(largerGeoradius).hasSize(2).contains("Weinheim").contains("Bahn");
     }
 
+    @Test
+    public void georadiusWithCoords() {
+
+        prepareGeo();
+
+        List<GeoWithin<String>> georadius = redis.georadius(key, 8.6582861, 49.5285695, 100, GeoArgs.Unit.km,
+                GeoArgs.Builder.coordinates());
+
+        assertThat(georadius).hasSize(3);
+        assertThat(getX(georadius, 0)).isBetween(8.66, 8.67);
+        assertThat(getY(georadius, 0)).isBetween(49.52, 49.53);
+
+        assertThat(getX(georadius, 2)).isBetween(8.37, 8.38);
+        assertThat(getY(georadius, 2)).isBetween(48.99, 49.00);
+    }
+
     @Test
     public void geodist() {
 
@@ -144,7 +158,7 @@ public void geodistMissingElements() {
     }
 
     @Test
-    public void geodistWithTransaction() {
+    public void geodistInTransaction() {
 
         prepareGeo();
 
@@ -175,7 +189,7 @@ public void geopos() {
     }
 
     @Test
-    public void geoposWithTransaction() {
+    public void geoposInTransaction() {
 
         prepareGeo();
 
@@ -282,7 +296,7 @@ public void geohashUnknownKey() {
     }
 
     @Test
-    public void geohashWithTransaction() {
+    public void geohashInTransaction() {
 
         prepareGeo();
 
@@ -434,7 +448,7 @@ public void georadiusbymemberWithArgs() {
     }
 
     @Test
-    public void georadiusbymemberWithArgsAndTransaction() {
+    public void georadiusbymemberWithArgsInTransaction() {
 
         prepareGeo();
 
@@ -488,4 +502,17 @@ public void georadiusStorebymemberWithNullArgs() {
         redis.georadiusbymember(key, "Bahn", 1, GeoArgs.Unit.km, (GeoRadiusStoreArgs<String>) null);
     }
 
+    protected void prepareGeo() {
+        redis.geoadd(key, 8.6638775, 49.5282537, "Weinheim");
+        redis.geoadd(key, 8.3796281, 48.9978127, "EFS9", 8.665351, 49.553302, "Bahn");
+    }
+
+    private static double getY(List<GeoWithin<String>> georadius, int i) {
+        return georadius.get(i).getCoordinates().getY().doubleValue();
+    }
+
+    private static double getX(List<GeoWithin<String>> georadius, int i) {
+        return georadius.get(i).getCoordinates().getX().doubleValue();
+    }
+
 }
diff --git a/src/test/java/io/lettuce/core/commands/reactive/GeoReactiveCommandTest.java b/src/test/java/io/lettuce/core/commands/reactive/GeoReactiveCommandTest.java
@@ -58,6 +58,6 @@ public void geopos() {
     @Test
     @Ignore("API differences")
     @Override
-    public void geoposWithTransaction() {
+    public void geoposInTransaction() {
     }
 }
diff --git a/src/test/java/io/lettuce/core/commands/transactional/GeoTxCommandTest.java b/src/test/java/io/lettuce/core/commands/transactional/GeoTxCommandTest.java
@@ -32,22 +32,22 @@ protected RedisCommands<String, String> connect() {
 
     @Ignore
     @Override
-    public void georadiusbymemberWithArgsAndTransaction() {
+    public void georadiusbymemberWithArgsInTransaction() {
     }
 
     @Ignore
     @Override
-    public void geoaddWithTransaction() {
+    public void geoaddInTransaction() {
     }
 
     @Ignore
     @Override
-    public void geoaddMultiWithTransaction() {
+    public void geoaddMultiInTransaction() {
     }
 
     @Ignore
     @Override
-    public void geoposWithTransaction() {
+    public void geoposInTransaction() {
     }
 
     @Ignore
@@ -57,16 +57,16 @@ public void georadiusWithArgsAndTransaction() {
 
     @Ignore
     @Override
-    public void georadiusWithTransaction() {
+    public void georadiusInTransaction() {
     }
 
     @Ignore
     @Override
-    public void geodistWithTransaction() {
+    public void geodistInTransaction() {
     }
 
     @Ignore
     @Override
-    public void geohashWithTransaction() {
+    public void geohashInTransaction() {
     }
 }