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 · 0059ccf · 0059ccf
1 parent a0a2356
commit 0059ccf
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 30 deletions.
diff --git a/src/main/java/com/lambdaworks/redis/output/GeoWithinListOutput.java b/src/main/java/com/lambdaworks/redis/output/GeoWithinListOutput.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2017 the original author or authors.
+ * Copyright 2011-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -101,6 +101,7 @@ public void complete(int depth) {
             distance = null;
             geohash = null;
             coordinates = null;
+            x = null;
         }
     }
 

diff --git a/src/test/java/com/lambdaworks/redis/cluster/commands/GeoClusterCommandTest.java b/src/test/java/com/lambdaworks/redis/cluster/commands/GeoClusterCommandTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2016 the original author or authors.
+ * Copyright 2011-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -71,22 +71,22 @@ protected RedisCommands<String, String> connect() {
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geoaddWithTransaction() throws Exception {
+    public void geoaddInTransaction() throws Exception {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geoaddMultiWithTransaction() throws Exception {
+    public void geoaddMultiInTransaction() throws Exception {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void georadiusWithTransaction() throws Exception {
+    public void georadiusInTransaction() throws Exception {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geodistWithTransaction() throws Exception {
+    public void geodistInTransaction() throws Exception {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
@@ -96,16 +96,16 @@ public void georadiusWithArgsAndTransaction() throws Exception {
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void georadiusbymemberWithArgsAndTransaction() throws Exception {
+    public void georadiusbymemberWithArgsInTransaction() throws Exception {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geoposWithTransaction() throws Exception {
+    public void geoposInTransaction() throws Exception {
     }
 
     @Ignore("MULTI not available on Redis Cluster")
     @Override
-    public void geohashWithTransaction() throws Exception {
+    public void geohashInTransaction() throws Exception {
     }
 }
diff --git a/src/test/java/com/lambdaworks/redis/commands/GeoCommandTest.java b/src/test/java/com/lambdaworks/redis/commands/GeoCommandTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2017 the original author or authors.
+ * Copyright 2011-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -31,6 +31,9 @@
 import com.lambdaworks.redis.*;
 import com.lambdaworks.redis.api.StatefulRedisConnection;
 
+/**
+ * @author Mark Paluch
+ */
 public class GeoCommandTest extends AbstractRedisClientTest {
 
     @Rule
@@ -57,7 +60,7 @@ public void geoadd() throws Exception {
     }
 
     @Test
-    public void geoaddWithTransaction() throws Exception {
+    public void geoaddInTransaction() throws Exception {
 
         redis.multi();
         redis.geoadd(key, -73.9454966, 40.747533, "lic market");
@@ -75,7 +78,7 @@ public void geoaddMulti() throws Exception {
     }
 
     @Test
-    public void geoaddMultiWithTransaction() throws Exception {
+    public void geoaddMultiInTransaction() throws Exception {
 
         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() throws Exception {
         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() throws Exception {
 
@@ -106,7 +104,7 @@ public void georadius() throws Exception {
     }
 
     @Test
-    public void georadiusWithTransaction() throws Exception {
+    public void georadiusInTransaction() throws Exception {
 
         prepareGeo();
 
@@ -122,6 +120,22 @@ public void georadiusWithTransaction() throws Exception {
         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() throws Exception {
 
@@ -144,7 +158,7 @@ public void geodistMissingElements() throws Exception {
     }
 
     @Test
-    public void geodistWithTransaction() throws Exception {
+    public void geodistInTransaction() throws Exception {
 
         prepareGeo();
 
@@ -175,7 +189,7 @@ public void geopos() throws Exception {
     }
 
     @Test
-    public void geoposWithTransaction() throws Exception {
+    public void geoposInTransaction() throws Exception {
 
         prepareGeo();
 
@@ -282,7 +296,7 @@ public void geohashUnknownKey() throws Exception {
     }
 
     @Test
-    public void geohashWithTransaction() throws Exception {
+    public void geohashInTransaction() throws Exception {
 
         prepareGeo();
 
@@ -434,7 +448,7 @@ public void georadiusbymemberWithArgs() throws Exception {
     }
 
     @Test
-    public void georadiusbymemberWithArgsAndTransaction() throws Exception {
+    public void georadiusbymemberWithArgsInTransaction() throws Exception {
 
         prepareGeo();
 
@@ -488,4 +502,17 @@ public void georadiusStorebymemberWithNullArgs() throws Exception {
         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/com/lambdaworks/redis/commands/transactional/GeoTxCommandTest.java b/src/test/java/com/lambdaworks/redis/commands/transactional/GeoTxCommandTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2016 the original author or authors.
+ * Copyright 2011-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,22 +32,22 @@ protected RedisCommands<String, String> connect() {
 
     @Ignore
     @Override
-    public void georadiusbymemberWithArgsAndTransaction() throws Exception {
+    public void georadiusbymemberWithArgsInTransaction() throws Exception {
     }
 
     @Ignore
     @Override
-    public void geoaddWithTransaction() throws Exception {
+    public void geoaddInTransaction() throws Exception {
     }
 
     @Ignore
     @Override
-    public void geoaddMultiWithTransaction() throws Exception {
+    public void geoaddMultiInTransaction() throws Exception {
     }
 
     @Ignore
     @Override
-    public void geoposWithTransaction() throws Exception {
+    public void geoposInTransaction() throws Exception {
     }
 
     @Ignore
@@ -57,16 +57,16 @@ public void georadiusWithArgsAndTransaction() throws Exception {
 
     @Ignore
     @Override
-    public void georadiusWithTransaction() throws Exception {
+    public void georadiusInTransaction() throws Exception {
     }
 
     @Ignore
     @Override
-    public void geodistWithTransaction() throws Exception {
+    public void geodistInTransaction() throws Exception {
     }
 
     @Ignore
     @Override
-    public void geohashWithTransaction() throws Exception {
+    public void geohashInTransaction() throws Exception {
     }
 }